LibDebug: Don't expose AttributeValue internals, use getters instead

This will be needed when we add `DW_FORM_strx*` and `DW_FORM_addrx*`
support, which requires us to fetch `DW_AT_str_offsets_base` and
`DW_AT_addr_base` attributes from the parent compilation unit. This
can't be done as we read the values, because it would create infinite
recursion (as we might try to parse the compilation unit's
`DW_FORM_strx*` encoded name before we get to the attribute). Having
getters ensures that we will perform lookups if they are needed.

Userland/Libraries/LibDebug/DebugInfo.cpp

@@ -54,15 +54,18 @@ void DebugInfo::parse_scopes_impl(Dwarf::DIE const& die)
         VariablesScope scope {};
         scope.is_function = (child.tag() == Dwarf::EntryTag::SubProgram);
         if (name.has_value())
-            scope.name = name.value().data.as_string;
+            scope.name = name.value().as_string();
 
         if (!child.get_attribute(Dwarf::Attribute::LowPc).has_value()) {
             dbgln_if(SPAM_DEBUG, "DWARF: Couldn't find attribute LowPc for scope");
             return;
         }
-        scope.address_low = child.get_attribute(Dwarf::Attribute::LowPc).value().data.as_addr;
-        // The attribute name HighPc is confusing. In this context, it seems to actually be a positive offset from LowPc
-        scope.address_high = scope.address_low + child.get_attribute(Dwarf::Attribute::HighPc).value().data.as_addr;
+        scope.address_low = child.get_attribute(Dwarf::Attribute::LowPc).value().as_addr();
+        auto high_pc = child.get_attribute(Dwarf::Attribute::HighPc);
+        if (high_pc->type() == Dwarf::AttributeValue::Type::Address)
+            scope.address_high = high_pc->as_addr();
+        else
+            scope.address_high = scope.address_low + high_pc->as_unsigned();
 
         child.for_each_child([&](Dwarf::DIE const& variable_entry) {
             if (!(variable_entry.tag() == Dwarf::EntryTag::Variable
@@ -187,12 +190,12 @@ static Optional<Dwarf::DIE> parse_variable_type_die(Dwarf::DIE const& variable_d
     if (!type_die_offset.has_value())
         return {};
 
-    VERIFY(type_die_offset.value().type == Dwarf::AttributeValue::Type::DieReference);
+    VERIFY(type_die_offset.value().type() == Dwarf::AttributeValue::Type::DieReference);
 
-    auto type_die = variable_die.compilation_unit().get_die_at_offset(type_die_offset.value().data.as_unsigned);
+    auto type_die = variable_die.compilation_unit().get_die_at_offset(type_die_offset.value().as_unsigned());
     auto type_name = type_die.get_attribute(Dwarf::Attribute::Name);
     if (type_name.has_value()) {
-        variable_info.type_name = type_name.value().data.as_string;
+        variable_info.type_name = type_name.value().as_string();
     } else {
         dbgln("Unnamed DWARF type at offset: {}", type_die.offset());
         variable_info.type_name = "[Unnamed Type]";
@@ -211,13 +214,13 @@ static void parse_variable_location(Dwarf::DIE const& variable_die, DebugInfo::V
     if (!location_info.has_value())
         return;
 
-    switch (location_info.value().type) {
+    switch (location_info.value().type()) {
     case Dwarf::AttributeValue::Type::UnsignedNumber:
         variable_info.location_type = DebugInfo::VariableInfo::LocationType::Address;
-        variable_info.location_data.address = location_info.value().data.as_addr;
+        variable_info.location_data.address = location_info.value().as_unsigned();
         break;
     case Dwarf::AttributeValue::Type::DwarfExpression: {
-        auto expression_bytes = ReadonlyBytes { location_info.value().data.as_raw_bytes.bytes, location_info.value().data.as_raw_bytes.length };
+        auto expression_bytes = location_info.value().as_raw_bytes();
         auto value = Dwarf::Expression::evaluate(expression_bytes, regs);
 
         if (value.type != Dwarf::Expression::Type::None) {
@@ -228,7 +231,7 @@ static void parse_variable_location(Dwarf::DIE const& variable_die, DebugInfo::V
         break;
     }
     default:
-        dbgln("Warning: unhandled Dwarf location type: {}", (int)location_info.value().type);
+        dbgln("Warning: unhandled Dwarf location type: {}", (int)location_info.value().type());
     }
 }
 
@@ -245,22 +248,22 @@ OwnPtr<DebugInfo::VariableInfo> DebugInfo::create_variable_info(Dwarf::DIE const
     NonnullOwnPtr<VariableInfo> variable_info = make<VariableInfo>();
     auto name_attribute = variable_die.get_attribute(Dwarf::Attribute::Name);
     if (name_attribute.has_value())
-        variable_info->name = name_attribute.value().data.as_string;
+        variable_info->name = name_attribute.value().as_string();
 
     auto type_die = parse_variable_type_die(variable_die, *variable_info);
 
     if (variable_die.tag() == Dwarf::EntryTag::Enumerator) {
         auto constant = variable_die.get_attribute(Dwarf::Attribute::ConstValue);
         VERIFY(constant.has_value());
-        switch (constant.value().type) {
+        switch (constant.value().type()) {
         case Dwarf::AttributeValue::Type::UnsignedNumber:
-            variable_info->constant_data.as_u32 = constant.value().data.as_unsigned;
+            variable_info->constant_data.as_u32 = constant.value().as_unsigned();
             break;
         case Dwarf::AttributeValue::Type::SignedNumber:
-            variable_info->constant_data.as_i32 = constant.value().data.as_signed;
+            variable_info->constant_data.as_i32 = constant.value().as_signed();
             break;
         case Dwarf::AttributeValue::Type::String:
-            variable_info->constant_data.as_string = constant.value().data.as_string;
+            variable_info->constant_data.as_string = constant.value().as_string();
             break;
         default:
             VERIFY_NOT_REACHED();
@@ -294,7 +297,7 @@ void DebugInfo::add_type_info_to_variable(Dwarf::DIE const& type_die, PtraceRegi
         if (is_array_type && member.tag() == Dwarf::EntryTag::SubRangeType) {
             auto upper_bound = member.get_attribute(Dwarf::Attribute::UpperBound);
             VERIFY(upper_bound.has_value());
-            auto size = upper_bound.value().data.as_unsigned + 1;
+            auto size = upper_bound.value().as_unsigned() + 1;
             type_info->dimension_sizes.append(size);
             return;
         }
@@ -428,12 +431,12 @@ Optional<Dwarf::LineProgram::DirectoryAndFile> DebugInfo::get_source_path_of_inl
     if (caller_file.has_value()) {
         u32 file_index = 0;
 
-        if (caller_file->type == Dwarf::AttributeValue::Type::UnsignedNumber) {
-            file_index = caller_file->data.as_unsigned;
-        } else if (caller_file->type == Dwarf::AttributeValue::Type::SignedNumber) {
+        if (caller_file->type() == Dwarf::AttributeValue::Type::UnsignedNumber) {
+            file_index = caller_file->as_unsigned();
+        } else if (caller_file->type() == Dwarf::AttributeValue::Type::SignedNumber) {
             // For some reason, the file_index is sometimes stored as a signed number.
-            VERIFY(caller_file->data.as_signed >= 0);
-            file_index = (u32)caller_file->data.as_signed;
+            VERIFY(caller_file->as_signed() >= 0);
+            file_index = (u32)caller_file->as_signed();
         } else {
             return {};
         }
@@ -449,9 +452,9 @@ Optional<uint32_t> DebugInfo::get_line_of_inline(Dwarf::DIE const& die) const
     if (!caller_line.has_value())
         return {};
 
-    if (caller_line->type != Dwarf::AttributeValue::Type::UnsignedNumber)
+    if (caller_line->type() != Dwarf::AttributeValue::Type::UnsignedNumber)
         return {};
-    return caller_line.value().data.as_unsigned;
+    return caller_line.value().as_unsigned();
 }
 
 }

Userland/Libraries/LibDebug/Dwarf/AttributeValue.h

@@ -6,12 +6,18 @@
 
 #pragma once
 
+#include <AK/Span.h>
 #include <AK/Types.h>
 #include <LibDebug/Dwarf/DwarfTypes.h>
 
 namespace Debug::Dwarf {
 
-struct AttributeValue {
+class CompilationUnit;
+
+class AttributeValue {
+    friend class DwarfInfo;
+
+public:
     enum class Type : u8 {
         UnsignedNumber,
         SignedNumber,
@@ -21,21 +27,33 @@ struct AttributeValue {
         DwarfExpression,
         SecOffset,
         RawBytes,
-    } type;
+        Address
+    };
+
+    Type type() const { return m_type; }
+    AttributeDataForm form() const { return m_form; }
 
+    FlatPtr as_addr() const { return m_data.as_addr; }
+    u64 as_unsigned() const { return m_data.as_unsigned; }
+    i64 as_signed() const { return m_data.as_signed; }
+    const char* as_string() const { return m_data.as_string; }
+    bool as_bool() const { return m_data.as_bool; }
+    ReadonlyBytes as_raw_bytes() const { return m_data.as_raw_bytes; }
+
+private:
+    Type m_type;
     union {
         FlatPtr as_addr;
         u64 as_unsigned;
         i64 as_signed;
         const char* as_string; // points to bytes in the memory mapped elf image
         bool as_bool;
-        struct {
-            u32 length;
-            const u8* bytes; // points to bytes in the memory mapped elf image
-        } as_raw_bytes;
-    } data {};
+        ReadonlyBytes as_raw_bytes;
+    } m_data {};
+
+    AttributeDataForm m_form {};
 
-    AttributeDataForm form {};
+    CompilationUnit const* m_compilation_unit { nullptr };
 };
 
 }

Userland/Libraries/LibDebug/Dwarf/CompilationUnit.cpp

@@ -38,7 +38,7 @@ Optional<FlatPtr> CompilationUnit::base_address() const
     auto die = root_die();
     auto res = die.get_attribute(Attribute::LowPc);
     if (res.has_value()) {
-        m_cached_base_address = res->data.as_addr;
+        m_cached_base_address = res->as_addr();
     }
     m_has_cached_base_address = true;
     return m_cached_base_address;

Userland/Libraries/LibDebug/Dwarf/DIE.cpp

@@ -81,7 +81,7 @@ void DIE::for_each_child(Function<void(DIE const& child)> callback) const
         auto sibling = current_child.get_attribute(Attribute::Sibling);
         u32 sibling_offset = 0;
         if (sibling.has_value()) {
-            sibling_offset = sibling.value().data.as_unsigned;
+            sibling_offset = sibling.value().as_unsigned();
         } else {
             // NOTE: According to the spec, the compiler doesn't have to supply the sibling information.
             // When it doesn't, we have to recursively iterate the current child's children to find where they end

Userland/Libraries/LibDebug/Dwarf/DwarfInfo.cpp

@@ -66,14 +66,14 @@ AttributeValue DwarfInfo::get_attribute_value(AttributeDataForm form, ssize_t im
     InputMemoryStream& debug_info_stream, const CompilationUnit* unit) const
 {
     AttributeValue value;
-    value.form = form;
+    value.m_form = form;
+    value.m_compilation_unit = unit;
 
     auto assign_raw_bytes_value = [&](size_t length) {
-        value.data.as_raw_bytes.length = length;
-        value.data.as_raw_bytes.bytes = reinterpret_cast<const u8*>(debug_info_data().data()
-            + debug_info_stream.offset());
+        value.m_data.as_raw_bytes = { reinterpret_cast<const u8*>(debug_info_data().data() + debug_info_stream.offset()), length };
 
         debug_info_stream.discard_or_error(length);
+        VERIFY(!debug_info_stream.has_any_error());
     };
 
     switch (form) {
@@ -81,95 +81,95 @@ AttributeValue DwarfInfo::get_attribute_value(AttributeDataForm form, ssize_t im
         u32 offset;
         debug_info_stream >> offset;
         VERIFY(!debug_info_stream.has_any_error());
-        value.type = AttributeValue::Type::String;
+        value.m_type = AttributeValue::Type::String;
 
         auto strings_data = debug_strings_data();
-        value.data.as_string = reinterpret_cast<const char*>(strings_data.data() + offset);
+        value.m_data.as_string = reinterpret_cast<const char*>(strings_data.data() + offset);
         break;
     }
     case AttributeDataForm::Data1: {
         u8 data;
         debug_info_stream >> data;
         VERIFY(!debug_info_stream.has_any_error());
-        value.type = AttributeValue::Type::UnsignedNumber;
-        value.data.as_unsigned = data;
+        value.m_type = AttributeValue::Type::UnsignedNumber;
+        value.m_data.as_unsigned = data;
         break;
     }
     case AttributeDataForm::Data2: {
         u16 data;
         debug_info_stream >> data;
         VERIFY(!debug_info_stream.has_any_error());
-        value.type = AttributeValue::Type::UnsignedNumber;
-        value.data.as_signed = data;
+        value.m_type = AttributeValue::Type::UnsignedNumber;
+        value.m_data.as_signed = data;
         break;
     }
     case AttributeDataForm::Addr: {
         FlatPtr address;
         debug_info_stream >> address;
         VERIFY(!debug_info_stream.has_any_error());
-        value.type = AttributeValue::Type::UnsignedNumber;
-        value.data.as_addr = address;
+        value.m_type = AttributeValue::Type::Address;
+        value.m_data.as_addr = address;
         break;
     }
     case AttributeDataForm::SData: {
         i64 data;
         debug_info_stream.read_LEB128_signed(data);
         VERIFY(!debug_info_stream.has_any_error());
-        value.type = AttributeValue::Type::SignedNumber;
-        value.data.as_signed = data;
+        value.m_type = AttributeValue::Type::SignedNumber;
+        value.m_data.as_signed = data;
         break;
     }
     case AttributeDataForm::UData: {
         u64 data;
         debug_info_stream.read_LEB128_unsigned(data);
         VERIFY(!debug_info_stream.has_any_error());
-        value.type = AttributeValue::Type::UnsignedNumber;
-        value.data.as_unsigned = data;
+        value.m_type = AttributeValue::Type::UnsignedNumber;
+        value.m_data.as_unsigned = data;
         break;
     }
     case AttributeDataForm::SecOffset: {
         u32 data;
         debug_info_stream >> data;
         VERIFY(!debug_info_stream.has_any_error());
-        value.type = AttributeValue::Type::SecOffset;
-        value.data.as_unsigned = data;
+        value.m_type = AttributeValue::Type::SecOffset;
+        value.m_data.as_unsigned = data;
         break;
     }
     case AttributeDataForm::Data4: {
         u32 data;
         debug_info_stream >> data;
         VERIFY(!debug_info_stream.has_any_error());
-        value.type = AttributeValue::Type::UnsignedNumber;
-        value.data.as_unsigned = data;
+        value.m_type = AttributeValue::Type::UnsignedNumber;
+        value.m_data.as_unsigned = data;
         break;
     }
     case AttributeDataForm::Data8: {
         u64 data;
         debug_info_stream >> data;
         VERIFY(!debug_info_stream.has_any_error());
-        value.type = AttributeValue::Type::UnsignedNumber;
-        value.data.as_unsigned = data;
+        value.m_type = AttributeValue::Type::UnsignedNumber;
+        value.m_data.as_unsigned = data;
         break;
     }
     case AttributeDataForm::Ref4: {
         u32 data;
         debug_info_stream >> data;
         VERIFY(!debug_info_stream.has_any_error());
-        value.type = AttributeValue::Type::DieReference;
+        value.m_type = AttributeValue::Type::DieReference;
         VERIFY(unit);
-        value.data.as_unsigned = data + unit->offset();
+        value.m_data.as_unsigned = data + unit->offset();
         break;
     }
     case AttributeDataForm::FlagPresent: {
-        value.type = AttributeValue::Type::Boolean;
-        value.data.as_bool = true;
+        value.m_type = AttributeValue::Type::Boolean;
+        value.m_data.as_bool = true;
         break;
     }
     case AttributeDataForm::ExprLoc: {
         size_t length;
         debug_info_stream.read_LEB128_unsigned(length);
         VERIFY(!debug_info_stream.has_any_error());
-        value.type = AttributeValue::Type::DwarfExpression;
+        value.m_type = AttributeValue::Type::DwarfExpression;
         assign_raw_bytes_value(length);
         break;
     }
@@ -178,12 +178,12 @@ AttributeValue DwarfInfo::get_attribute_value(AttributeDataForm form, ssize_t im
         u32 str_offset = debug_info_stream.offset();
         debug_info_stream >> str;
         VERIFY(!debug_info_stream.has_any_error());
-        value.type = AttributeValue::Type::String;
-        value.data.as_string = reinterpret_cast<const char*>(str_offset + debug_info_data().data());
+        value.m_type = AttributeValue::Type::String;
+        value.m_data.as_string = reinterpret_cast<const char*>(str_offset + debug_info_data().data());
         break;
     }
     case AttributeDataForm::Block1: {
-        value.type = AttributeValue::Type::RawBytes;
+        value.m_type = AttributeValue::Type::RawBytes;
         u8 length;
         debug_info_stream >> length;
         VERIFY(!debug_info_stream.has_any_error());
@@ -191,7 +191,7 @@ AttributeValue DwarfInfo::get_attribute_value(AttributeDataForm form, ssize_t im
         break;
     }
     case AttributeDataForm::Block2: {
-        value.type = AttributeValue::Type::RawBytes;
+        value.m_type = AttributeValue::Type::RawBytes;
         u16 length;
         debug_info_stream >> length;
         VERIFY(!debug_info_stream.has_any_error());
@@ -199,7 +199,7 @@ AttributeValue DwarfInfo::get_attribute_value(AttributeDataForm form, ssize_t im
         break;
     }
     case AttributeDataForm::Block4: {
-        value.type = AttributeValue::Type::RawBytes;
+        value.m_type = AttributeValue::Type::RawBytes;
         u32 length;
         debug_info_stream >> length;
         VERIFY(!debug_info_stream.has_any_error());
@@ -207,7 +207,7 @@ AttributeValue DwarfInfo::get_attribute_value(AttributeDataForm form, ssize_t im
         break;
     }
     case AttributeDataForm::Block: {
-        value.type = AttributeValue::Type::RawBytes;
+        value.m_type = AttributeValue::Type::RawBytes;
         size_t length;
         debug_info_stream.read_LEB128_unsigned(length);
         VERIFY(!debug_info_stream.has_any_error());
@@ -218,16 +218,16 @@ AttributeValue DwarfInfo::get_attribute_value(AttributeDataForm form, ssize_t im
         u32 offset;
         debug_info_stream >> offset;
         VERIFY(!debug_info_stream.has_any_error());
-        value.type = AttributeValue::Type::String;
+        value.m_type = AttributeValue::Type::String;
 
         auto strings_data = debug_line_strings_data();
-        value.data.as_string = reinterpret_cast<const char*>(strings_data.data() + offset);
+        value.m_data.as_string = reinterpret_cast<const char*>(strings_data.data() + offset);
         break;
     }
     case AttributeDataForm::ImplicitConst: {
         /* Value is part of the abbreviation record. */
-        value.type = AttributeValue::Type::SignedNumber;
-        value.data.as_signed = implicit_const_value;
+        value.m_type = AttributeValue::Type::SignedNumber;
+        value.m_data.as_signed = implicit_const_value;
         break;
     }
     default:
@@ -247,9 +247,9 @@ void DwarfInfo::build_cached_dies() const
         auto ranges = die.get_attribute(Attribute::Ranges);
         if (ranges.has_value()) {
             // TODO Support DW_FORM_rnglistx
-            if (ranges->form != AttributeDataForm::SecOffset)
+            if (ranges->form() != AttributeDataForm::SecOffset)
                 return {};
-            auto offset = ranges->data.as_addr;
+            auto offset = ranges->as_unsigned();
             AddressRanges address_ranges(debug_range_lists_data(), offset, die.compilation_unit());
             Vector<DIERange> entries;
             address_ranges.for_each_range([&entries](auto range) {
@@ -264,18 +264,18 @@ void DwarfInfo::build_cached_dies() const
         if (!start.has_value() || !end.has_value())
             return {};
 
-        VERIFY(start->form == Dwarf::AttributeDataForm::Addr);
+        VERIFY(start->type() == Dwarf::AttributeValue::Type::Address);
 
         // DW_AT_high_pc attribute can have different meanings depending on the attribute form.
         // (Dwarf version 5, section 2.17.2).
 
         uint32_t range_end = 0;
-        if (end->form == Dwarf::AttributeDataForm::Addr)
-            range_end = end->data.as_addr;
+        if (end->form() == Dwarf::AttributeDataForm::Addr)
+            range_end = end->as_addr();
         else
-            range_end = start->data.as_unsigned + end->data.as_unsigned;
+            range_end = start->as_addr() + end->as_unsigned();
 
-        return { DIERange { (FlatPtr)start.value().data.as_addr, range_end } };
+        return { DIERange { start.value().as_addr(), range_end } };
     };
 
     // If we simply use a lambda, type deduction fails because it's used recursively.

Userland/Libraries/LibDebug/Dwarf/LineProgram.cpp

@@ -60,10 +60,10 @@ void LineProgram::parse_path_entries(Function<void(PathEntry& entry)> callback,
                 auto value = m_dwarf_info.get_attribute_value(format_description.form, 0, m_stream);
                 switch (format_description.type) {
                 case ContentType::Path:
-                    entry.path = value.data.as_string;
+                    entry.path = value.as_string();
                     break;
                 case ContentType::DirectoryIndex:
-                    entry.directory_index = value.data.as_unsigned;
+                    entry.directory_index = value.as_unsigned();
                     break;
                 default:
                     dbgln_if(DWARF_DEBUG, "Unhandled path list attribute: {}", (int)format_description.type);