[Lldb-commits] [lldb] [lldb][DWARF] Change GetAttributes to always visit current DIE before recursing (PR #123261)
Michael Buch via lldb-commits
lldb-commits at lists.llvm.org
Thu Jan 16 16:41:21 PST 2025
https://github.com/Michael137 updated https://github.com/llvm/llvm-project/pull/123261
>From 69b53a2a6290733ee90e4d75521f4c2fd6bd1401 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Thu, 16 Jan 2025 15:16:36 +0000
Subject: [PATCH 1/3] [lldb][DWARF] Change GetAttributes to always visit
current DIE before recursing
`GetAttributes` returns all attributes on a given DIE, including any attributes in referenced `DW_AT_abstract_origin` or `DW_AT_specification`. However, if an attribute exists on both the referring DIE and the referenced DIE, the first one encountered will be the one that takes precendence when querying the returned `DWARFAttributes`. There is currently no good way of ensuring that an attribute of a definition doesn't get shadowed by one found on the declaration. One use-case where we don't want this to happen is for `DW_AT_object_pointer` (which can exist on both definitions and declarations, see https://github.com/llvm/llvm-project/pull/123089).
This patch makes sure we visit the current DIE's attributes before
following DIE references. I tried keeping as much of the original
`GetAttributes` unchanged and just add an outer `GetAttributes` that
keeps track of the DIEs we need to visit next.
There's precendent for this iteration order in
`llvm::DWARFDie::findRecursively` and also
`lldb_private::ElaboratingDIEIterator`. We could use the latter to
implement `GetAttributes`, though it also follows `DW_AT_signature` so I
decided to leave it for follow-up.
---
.../SymbolFile/DWARF/DWARFDebugInfoEntry.cpp | 82 ++-
.../SymbolFile/DWARF/DWARFDebugInfoEntry.h | 25 +-
.../SymbolFile/DWARF/DWARFDIETest.cpp | 602 ++++++++++++++++++
3 files changed, 680 insertions(+), 29 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
index 6d073411de876c..d26d302c33dd28 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -281,22 +281,28 @@ bool DWARFDebugInfoEntry::GetDIENamesAndRanges(
return !ranges.empty();
}
-// Get all attribute values for a given DIE, including following any
-// specification or abstract origin attributes and including those in the
-// results. Any duplicate attributes will have the first instance take
-// precedence (this can happen for declaration attributes).
-void DWARFDebugInfoEntry::GetAttributes(DWARFUnit *cu,
- DWARFAttributes &attributes,
- Recurse recurse,
- uint32_t curr_depth) const {
- const auto *abbrevDecl = GetAbbreviationDeclarationPtr(cu);
- if (!abbrevDecl) {
- attributes.Clear();
- return;
- }
+bool DWARFDebugInfoEntry::GetAttributes(
+ DWARFUnit const *cu, llvm::SmallVector<DWARFDIE> &worklist,
+ llvm::SmallSet<DWARFDebugInfoEntry const *, 3> &seen,
+ DWARFAttributes &attributes) const {
+ assert(!worklist.empty());
+ assert(cu);
+
+ DWARFDIE current = worklist.pop_back_val();
+
+ const auto *entry = current.GetDIE();
+ if (!entry)
+ return false;
+
+ const auto *abbrevDecl =
+ entry->GetAbbreviationDeclarationPtr(current.GetCU());
+ if (!abbrevDecl)
+ return false;
const DWARFDataExtractor &data = cu->GetData();
- lldb::offset_t offset = GetFirstAttributeOffset();
+ lldb::offset_t offset = current.GetDIE()->GetFirstAttributeOffset();
+
+ const bool is_first_die = seen.size() == 1;
for (const auto &attribute : abbrevDecl->attributes()) {
DWARFFormValue form_value(cu);
@@ -309,10 +315,10 @@ void DWARFDebugInfoEntry::GetAttributes(DWARFUnit *cu,
switch (attr) {
case DW_AT_sibling:
case DW_AT_declaration:
- if (curr_depth > 0) {
+ if (seen.size() > 1 && !is_first_die) {
// This attribute doesn't make sense when combined with the DIE that
// references this DIE. We know a DIE is referencing this DIE because
- // curr_depth is not zero
+ // we've visited more than one DIE already.
break;
}
[[fallthrough]];
@@ -321,13 +327,12 @@ void DWARFDebugInfoEntry::GetAttributes(DWARFUnit *cu,
break;
}
- if (recurse == Recurse::yes &&
- ((attr == DW_AT_specification) || (attr == DW_AT_abstract_origin))) {
+ if (attr == DW_AT_specification || attr == DW_AT_abstract_origin) {
if (form_value.ExtractValue(data, &offset)) {
- DWARFDIE spec_die = form_value.Reference();
- if (spec_die)
- spec_die.GetDIE()->GetAttributes(spec_die.GetCU(), attributes,
- recurse, curr_depth + 1);
+ if (DWARFDIE spec_die = form_value.Reference()) {
+ if (seen.insert(spec_die.GetDIE()).second)
+ worklist.push_back(spec_die);
+ }
}
} else {
const dw_form_t form = form_value.Form();
@@ -339,6 +344,39 @@ void DWARFDebugInfoEntry::GetAttributes(DWARFUnit *cu,
DWARFFormValue::SkipValue(form, data, &offset, cu);
}
}
+
+ return true;
+}
+
+DWARFAttributes DWARFDebugInfoEntry::GetAttributes(DWARFUnit *cu,
+ Recurse recurse) const {
+ // FIXME: use ElaboratingDIEIterator to follow specifications/abstract origins
+ // instead of maintaining our own worklist/seen list.
+
+ DWARFAttributes attributes;
+
+ llvm::SmallVector<DWARFDIE, 3> worklist;
+ worklist.emplace_back(cu, this);
+
+ // Keep track if DIEs already seen to prevent infinite recursion.
+ // Value of '3' was picked for the same reason that
+ // DWARFDie::findRecursively does.
+ llvm::SmallSet<DWARFDebugInfoEntry const *, 3> seen;
+ seen.insert(this);
+
+ while (!worklist.empty()) {
+ if (!GetAttributes(cu, worklist, seen, attributes)) {
+ attributes.Clear();
+ break;
+ }
+
+ // We visited the current DIE already and were asked not to check the
+ // rest of the worklist. So bail out.
+ if (recurse == Recurse::no)
+ break;
+ }
+
+ return attributes;
}
// GetAttributeValue
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
index de6bbf1d527899..28381114634319 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -52,12 +52,21 @@ class DWARFDebugInfoEntry {
lldb::offset_t *offset_ptr);
using Recurse = DWARFBaseDIE::Recurse;
+
+ // Get all attribute values for a given DIE, including following any
+ // specification or abstract origin attributes and including those in the
+ // results.
+ //
+ // When following specifications/abstract origins, the attributes
+ // on the referring DIE are guaranteed to be visited before the attributes of
+ // the referenced DIE.
+ //
+ // \param[in]
+ // \param[in]
+ //
+ // \returns output may have duplicate entries
DWARFAttributes GetAttributes(DWARFUnit *cu,
- Recurse recurse = Recurse::yes) const {
- DWARFAttributes attrs;
- GetAttributes(cu, attrs, recurse, 0 /* curr_depth */);
- return attrs;
- }
+ Recurse recurse = Recurse::yes) const;
dw_offset_t GetAttributeValue(const DWARFUnit *cu, const dw_attr_t attr,
DWARFFormValue &formValue,
@@ -180,8 +189,10 @@ class DWARFDebugInfoEntry {
dw_tag_t m_tag = llvm::dwarf::DW_TAG_null;
private:
- void GetAttributes(DWARFUnit *cu, DWARFAttributes &attrs, Recurse recurse,
- uint32_t curr_depth) const;
+ // Helper for the public \ref DWARFDebugInfoEntry::GetAttributes API.
+ bool GetAttributes(DWARFUnit const *cu, llvm::SmallVector<DWARFDIE> &worklist,
+ llvm::SmallSet<DWARFDebugInfoEntry const *, 3> &seen,
+ DWARFAttributes &attributes) const;
};
} // namespace dwarf
} // namespace lldb_private::plugin
diff --git a/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp b/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp
index 1e4c8f3ba07787..4f544656a6312c 100644
--- a/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp
+++ b/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp
@@ -394,3 +394,605 @@ TEST(DWARFDIETest, GetContextInFunction) {
EXPECT_THAT(foo_struct_die.GetTypeLookupContext(),
testing::ElementsAre(make_struct("struct_t")));
}
+
+struct GetAttributesTestFixture : public testing::TestWithParam<dw_attr_t> {};
+
+TEST_P(GetAttributesTestFixture, TestGetAttributes_IterationOrder) {
+ // Tests that we accumulate all current DIE's attributes first
+ // before checking the attributes of the specification.
+
+ const char *yamldata = R"(
+--- !ELF
+FileHeader:
+ Class: ELFCLASS64
+ Data: ELFDATA2LSB
+ Type: ET_EXEC
+ Machine: EM_AARCH64
+DWARF:
+ debug_str:
+ - func
+ debug_abbrev:
+ - ID: 0
+ Table:
+ - Code: 0x1
+ Tag: DW_TAG_compile_unit
+ Children: DW_CHILDREN_yes
+ Attributes:
+ - Attribute: DW_AT_language
+ Form: DW_FORM_data2
+ - Code: 0x2
+ Tag: DW_TAG_subprogram
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_high_pc
+ Form: DW_FORM_data4
+ - Attribute: DW_AT_name
+ Form: DW_FORM_strp
+ - Attribute: DW_AT_declaration
+ Form: DW_FORM_flag_present
+ - Attribute: DW_AT_external
+ Form: DW_FORM_flag_present
+ - Attribute: DW_AT_low_pc
+ Form: DW_FORM_data4
+ - Code: 0x3
+ Tag: DW_TAG_subprogram
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_high_pc
+ Form: DW_FORM_data4
+ - Attribute: {0}
+ Form: DW_FORM_ref4
+ - Attribute: DW_AT_low_pc
+ Form: DW_FORM_data4
+ debug_info:
+ - Version: 5
+ UnitType: DW_UT_compile
+ AddrSize: 8
+ Entries:
+
+# DW_TAG_compile_unit
+# DW_AT_language [DW_FORM_data2] (DW_LANG_C_plus_plus)
+
+ - AbbrCode: 0x1
+ Values:
+ - Value: 0x04
+
+# DW_TAG_subprogram
+# DW_AT_high_pc [DW_FORM_data4]
+# DW_AT_name [DW_FORM_strp] ("func")
+# DW_AT_low_pc [DW_FORM_data4]
+ - AbbrCode: 0x2
+ Values:
+ - Value: 0xdeadbeef
+ - Value: 0x0
+ - Value: 0x1
+ - Value: 0x1
+ - Value: 0xdeadbeef
+
+# DW_TAG_subprogram
+# DW_AT_high_pc [DW_FORM_data4]
+# DW_AT_specification [DW_FORM_ref4] ("func")
+# DW_AT_low_pc [DW_FORM_data4]
+ - AbbrCode: 0x3
+ Values:
+ - Value: 0xf00dcafe
+ - Value: 0xf
+ - Value: 0xf00dcafe
+
+ - AbbrCode: 0x0
+...
+)";
+ YAMLModuleTester t(llvm::formatv(yamldata, GetParam()).str());
+
+ DWARFUnit *unit = t.GetDwarfUnit();
+ ASSERT_NE(unit, nullptr);
+ const DWARFDebugInfoEntry *cu_entry = unit->DIE().GetDIE();
+ ASSERT_EQ(cu_entry->Tag(), DW_TAG_compile_unit);
+ ASSERT_EQ(unit->GetDWARFLanguageType(), DW_LANG_C_plus_plus);
+ DWARFDIE cu_die(unit, cu_entry);
+
+ auto declaration = cu_die.GetFirstChild();
+ ASSERT_TRUE(declaration.IsValid());
+ ASSERT_EQ(declaration.Tag(), DW_TAG_subprogram);
+
+ auto definition = declaration.GetSibling();
+ ASSERT_TRUE(definition.IsValid());
+ ASSERT_EQ(definition.Tag(), DW_TAG_subprogram);
+ ASSERT_FALSE(definition.GetAttributeValueAsOptionalUnsigned(DW_AT_external));
+
+ auto attrs = definition.GetAttributes(DWARFDebugInfoEntry::Recurse::yes);
+ EXPECT_EQ(attrs.Size(), 7U);
+
+ // Check that the attributes on the definition (that are also present
+ // on the declaration) take precedence.
+ for (auto attr : {DW_AT_low_pc, DW_AT_high_pc}) {
+ auto idx = attrs.FindAttributeIndex(attr);
+ EXPECT_NE(idx, UINT32_MAX);
+
+ DWARFFormValue form_value;
+ auto success = attrs.ExtractFormValueAtIndex(idx, form_value);
+ EXPECT_TRUE(success);
+
+ EXPECT_EQ(form_value.Unsigned(), 0xf00dcafe);
+ }
+}
+
+TEST_P(GetAttributesTestFixture, TestGetAttributes_Cycle) {
+ // Tests that GetAttributes can deal with cycles in
+ // specifications/abstract origins.
+
+ const char *yamldata = R"(
+--- !ELF
+FileHeader:
+ Class: ELFCLASS64
+ Data: ELFDATA2LSB
+ Type: ET_EXEC
+ Machine: EM_AARCH64
+DWARF:
+ debug_abbrev:
+ - ID: 0
+ Table:
+ - Code: 0x1
+ Tag: DW_TAG_compile_unit
+ Children: DW_CHILDREN_yes
+ Attributes:
+ - Attribute: DW_AT_language
+ Form: DW_FORM_data2
+ - Code: 0x2
+ Tag: DW_TAG_subprogram
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: {0}
+ Form: DW_FORM_ref4
+ debug_info:
+ - Version: 5
+ UnitType: DW_UT_compile
+ AddrSize: 8
+ Entries:
+
+ - AbbrCode: 0x1
+ Values:
+ - Value: 0x04
+
+ - AbbrCode: 0x2
+ Values:
+ - Value: 0x19
+
+ - AbbrCode: 0x2
+ Values:
+ - Value: 0xf
+
+ - AbbrCode: 0x2
+ Values:
+ - Value: 0x14
+
+ - AbbrCode: 0x0
+...
+)";
+ YAMLModuleTester t(llvm::formatv(yamldata, GetParam()).str());
+
+ DWARFUnit *unit = t.GetDwarfUnit();
+ ASSERT_NE(unit, nullptr);
+ const DWARFDebugInfoEntry *cu_entry = unit->DIE().GetDIE();
+ ASSERT_EQ(cu_entry->Tag(), DW_TAG_compile_unit);
+ ASSERT_EQ(unit->GetDWARFLanguageType(), DW_LANG_C_plus_plus);
+ DWARFDIE cu_die(unit, cu_entry);
+
+ auto func = cu_die.GetFirstChild();
+ ASSERT_TRUE(func.IsValid());
+ ASSERT_EQ(func.Tag(), DW_TAG_subprogram);
+
+ auto attrs = func.GetAttributes(DWARFDebugInfoEntry::Recurse::yes);
+ EXPECT_EQ(attrs.Size(), 3U);
+
+ // TODO: check that we do form a cycle with the DIEs
+}
+
+TEST_P(GetAttributesTestFixture,
+ TestGetAttributes_SkipNonApplicableAttributes) {
+ // Tests that GetAttributes will omit attributes found through
+ // specifications/abstract origins which are not applicable.
+
+ const char *yamldata = R"(
+--- !ELF
+FileHeader:
+ Class: ELFCLASS64
+ Data: ELFDATA2LSB
+ Type: ET_EXEC
+ Machine: EM_AARCH64
+DWARF:
+ debug_str:
+ - func
+ debug_abbrev:
+ - ID: 0
+ Table:
+ - Code: 0x1
+ Tag: DW_TAG_compile_unit
+ Children: DW_CHILDREN_yes
+ Attributes:
+ - Attribute: DW_AT_language
+ Form: DW_FORM_data2
+ - Code: 0x2
+ Tag: DW_TAG_subprogram
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_declaration
+ Form: DW_FORM_flag_present
+ - Attribute: DW_AT_name
+ Form: DW_FORM_strp
+ - Attribute: DW_AT_sibling
+ Form: DW_FORM_ref4
+ - Code: 0x3
+ Tag: DW_TAG_subprogram
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_declaration
+ Form: DW_FORM_flag_present
+ - Attribute: {0}
+ Form: DW_FORM_ref4
+ - Attribute: DW_AT_sibling
+ Form: DW_FORM_ref4
+ debug_info:
+ - Version: 5
+ UnitType: DW_UT_compile
+ AddrSize: 8
+ Entries:
+
+# DW_TAG_compile_unit
+# DW_AT_language [DW_FORM_data2] (DW_LANG_C_plus_plus)
+
+ - AbbrCode: 0x1
+ Values:
+ - Value: 0x04
+
+# DW_TAG_subprogram
+# DW_AT_declaration
+# DW_AT_name [DW_FORM_strp] ("func")
+# DW_AT_sibling
+ - AbbrCode: 0x2
+ Values:
+ - Value: 0x1
+ - Value: 0x0
+ - Value: 0x18
+
+# DW_TAG_subprogram
+# DW_AT_declaration
+# DW_AT_specification [DW_FORM_ref4] ("func")
+# DW_AT_sibling
+ - AbbrCode: 0x3
+ Values:
+ - Value: 0x1
+ - Value: 0xf
+ - Value: 0xdeadbeef
+
+ - AbbrCode: 0x0
+...
+)";
+ YAMLModuleTester t(llvm::formatv(yamldata, GetParam()).str());
+
+ DWARFUnit *unit = t.GetDwarfUnit();
+ ASSERT_NE(unit, nullptr);
+ const DWARFDebugInfoEntry *cu_entry = unit->DIE().GetDIE();
+ ASSERT_EQ(cu_entry->Tag(), DW_TAG_compile_unit);
+ ASSERT_EQ(unit->GetDWARFLanguageType(), DW_LANG_C_plus_plus);
+ DWARFDIE cu_die(unit, cu_entry);
+
+ auto declaration = cu_die.GetFirstChild();
+ ASSERT_TRUE(declaration.IsValid());
+ ASSERT_EQ(declaration.Tag(), DW_TAG_subprogram);
+
+ auto definition = declaration.GetSibling();
+ ASSERT_TRUE(definition.IsValid());
+ ASSERT_EQ(definition.Tag(), DW_TAG_subprogram);
+
+ auto attrs = definition.GetAttributes(DWARFDebugInfoEntry::Recurse::yes);
+ EXPECT_EQ(attrs.Size(), 4U);
+ EXPECT_NE(attrs.FindAttributeIndex(DW_AT_name), UINT32_MAX);
+ EXPECT_NE(attrs.FindAttributeIndex(GetParam()), UINT32_MAX);
+
+ auto sibling_idx = attrs.FindAttributeIndex(DW_AT_sibling);
+ EXPECT_NE(sibling_idx, UINT32_MAX);
+
+ DWARFFormValue form_value;
+ auto success = attrs.ExtractFormValueAtIndex(sibling_idx, form_value);
+ ASSERT_TRUE(success);
+
+ EXPECT_EQ(form_value.Unsigned(), 0xdeadbeef);
+}
+
+TEST_P(GetAttributesTestFixture, TestGetAttributes_NoRecurse) {
+ // Tests that GetAttributes will not recurse if Recurse::No is passed to it.
+
+ const char *yamldata = R"(
+--- !ELF
+FileHeader:
+ Class: ELFCLASS64
+ Data: ELFDATA2LSB
+ Type: ET_EXEC
+ Machine: EM_AARCH64
+DWARF:
+ debug_str:
+ - func
+ debug_abbrev:
+ - ID: 0
+ Table:
+ - Code: 0x1
+ Tag: DW_TAG_compile_unit
+ Children: DW_CHILDREN_yes
+ Attributes:
+ - Attribute: DW_AT_language
+ Form: DW_FORM_data2
+ - Code: 0x2
+ Tag: DW_TAG_subprogram
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_name
+ Form: DW_FORM_strp
+ - Code: 0x3
+ Tag: DW_TAG_subprogram
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_low_pc
+ Form: DW_FORM_data4
+ - Attribute: {0}
+ Form: DW_FORM_ref4
+ debug_info:
+ - Version: 5
+ UnitType: DW_UT_compile
+ AddrSize: 8
+ Entries:
+
+# DW_TAG_compile_unit
+# DW_AT_language [DW_FORM_data2] (DW_LANG_C_plus_plus)
+
+ - AbbrCode: 0x1
+ Values:
+ - Value: 0x04
+
+# DW_TAG_subprogram
+# DW_AT_name [DW_FORM_strp] ("func")
+ - AbbrCode: 0x2
+ Values:
+ - Value: 0x0
+
+# DW_TAG_subprogram
+# DW_AT_low_pc [DW_FORM_data4]
+# DW_AT_specification [DW_FORM_ref4]
+ - AbbrCode: 0x3
+ Values:
+ - Value: 0xdeadbeef
+ - Value: 0xf
+
+ - AbbrCode: 0x0
+...
+)";
+ YAMLModuleTester t(llvm::formatv(yamldata, GetParam()).str());
+
+ DWARFUnit *unit = t.GetDwarfUnit();
+ ASSERT_NE(unit, nullptr);
+ const DWARFDebugInfoEntry *cu_entry = unit->DIE().GetDIE();
+ ASSERT_EQ(cu_entry->Tag(), DW_TAG_compile_unit);
+ ASSERT_EQ(unit->GetDWARFLanguageType(), DW_LANG_C_plus_plus);
+ DWARFDIE cu_die(unit, cu_entry);
+
+ auto declaration = cu_die.GetFirstChild();
+ ASSERT_TRUE(declaration.IsValid());
+ ASSERT_EQ(declaration.Tag(), DW_TAG_subprogram);
+
+ auto definition = declaration.GetSibling();
+ ASSERT_TRUE(definition.IsValid());
+ ASSERT_EQ(definition.Tag(), DW_TAG_subprogram);
+
+ auto attrs = definition.GetAttributes(DWARFDebugInfoEntry::Recurse::no);
+ EXPECT_EQ(attrs.Size(), 2U);
+ EXPECT_EQ(attrs.FindAttributeIndex(DW_AT_name), UINT32_MAX);
+ EXPECT_NE(attrs.FindAttributeIndex(GetParam()), UINT32_MAX);
+ EXPECT_NE(attrs.FindAttributeIndex(DW_AT_low_pc), UINT32_MAX);
+}
+
+TEST_P(GetAttributesTestFixture, TestGetAttributes_InvalidSpec) {
+ // Test that GetAttributes doesn't try following invalid
+ // specifications (but still add it to the list of attributes).
+
+ const char *yamldata = R"(
+--- !ELF
+FileHeader:
+ Class: ELFCLASS64
+ Data: ELFDATA2LSB
+ Type: ET_EXEC
+ Machine: EM_AARCH64
+DWARF:
+ debug_str:
+ - func
+ debug_abbrev:
+ - ID: 0
+ Table:
+ - Code: 0x1
+ Tag: DW_TAG_compile_unit
+ Children: DW_CHILDREN_yes
+ Attributes:
+ - Attribute: DW_AT_language
+ Form: DW_FORM_data2
+ - Code: 0x2
+ Tag: DW_TAG_subprogram
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_name
+ Form: DW_FORM_strp
+ - Code: 0x3
+ Tag: DW_TAG_subprogram
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: {0}
+ Form: DW_FORM_ref4
+ debug_info:
+ - Version: 5
+ UnitType: DW_UT_compile
+ AddrSize: 8
+ Entries:
+
+# DW_TAG_compile_unit
+# DW_AT_language [DW_FORM_data2] (DW_LANG_C_plus_plus)
+
+ - AbbrCode: 0x1
+ Values:
+ - Value: 0x04
+
+# DW_TAG_subprogram
+# DW_AT_name [DW_FORM_strp] ("func")
+ - AbbrCode: 0x2
+ Values:
+ - Value: 0x0
+
+# DW_TAG_subprogram
+# DW_AT_specification [DW_FORM_ref4]
+ - AbbrCode: 0x3
+ Values:
+ - Value: 0xdeadbeef
+
+ - AbbrCode: 0x0
+...
+)";
+ YAMLModuleTester t(llvm::formatv(yamldata, GetParam()).str());
+
+ DWARFUnit *unit = t.GetDwarfUnit();
+ ASSERT_NE(unit, nullptr);
+ const DWARFDebugInfoEntry *cu_entry = unit->DIE().GetDIE();
+ ASSERT_EQ(cu_entry->Tag(), DW_TAG_compile_unit);
+ ASSERT_EQ(unit->GetDWARFLanguageType(), DW_LANG_C_plus_plus);
+ DWARFDIE cu_die(unit, cu_entry);
+
+ auto declaration = cu_die.GetFirstChild();
+ ASSERT_TRUE(declaration.IsValid());
+ ASSERT_EQ(declaration.Tag(), DW_TAG_subprogram);
+
+ auto definition = declaration.GetSibling();
+ ASSERT_TRUE(definition.IsValid());
+ ASSERT_EQ(definition.Tag(), DW_TAG_subprogram);
+
+ auto attrs = definition.GetAttributes(DWARFDebugInfoEntry::Recurse::yes);
+ EXPECT_EQ(attrs.Size(), 1U);
+ EXPECT_EQ(attrs.FindAttributeIndex(DW_AT_name), UINT32_MAX);
+ EXPECT_NE(attrs.FindAttributeIndex(GetParam()), UINT32_MAX);
+}
+
+TEST(DWARFDIETest, TestGetAttributes_Worklist) {
+ // Test that GetAttributes will follow both the abstract origin
+ // and specification on a single DIE correctly (omitting non-applicable
+ // attributes in the process).
+
+ // Contrived example where
+ // f1---> f2 --> f4
+ // `-> f3 `-> f5
+ //
+ const char *yamldata = R"(
+--- !ELF
+FileHeader:
+ Class: ELFCLASS64
+ Data: ELFDATA2LSB
+ Type: ET_EXEC
+ Machine: EM_AARCH64
+DWARF:
+ debug_str:
+ - foo
+ - bar
+ debug_abbrev:
+ - ID: 0
+ Table:
+ - Code: 0x1
+ Tag: DW_TAG_compile_unit
+ Children: DW_CHILDREN_yes
+ Attributes:
+ - Attribute: DW_AT_language
+ Form: DW_FORM_data2
+ - Code: 0x2
+ Tag: DW_TAG_subprogram
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_specification
+ Form: DW_FORM_ref4
+ - Attribute: DW_AT_abstract_origin
+ Form: DW_FORM_ref4
+ - Code: 0x3
+ Tag: DW_TAG_subprogram
+ Children: DW_CHILDREN_no
+ Attributes:
+ - Attribute: DW_AT_declaration
+ Form: DW_FORM_flag_present
+ - Attribute: DW_AT_artificial
+ Form: DW_FORM_flag_present
+
+ debug_info:
+ - Version: 5
+ UnitType: DW_UT_compile
+ AddrSize: 8
+ Entries:
+
+ - AbbrCode: 0x1
+ Values:
+ - Value: 0x04
+
+# DW_TAG_subprogram ("f1")
+# DW_AT_specification [DW_FORM_ref4] ("f2")
+# DW_AT_abstract_origin [DW_FORM_ref4] ("f3")
+ - AbbrCode: 0x2
+ Values:
+ - Value: 0x18
+ - Value: 0x21
+
+# DW_TAG_subprogram ("f2")
+# DW_AT_specification [DW_FORM_ref4] ("f4")
+# DW_AT_abstract_origin [DW_FORM_ref4] ("f5")
+ - AbbrCode: 0x2
+ Values:
+ - Value: 0x22
+ - Value: 0x23
+
+# DW_TAG_subprogram ("f3")
+# DW_AT_declaration [DW_FORM_flag_present]
+# DW_AT_artificial [DW_FORM_flag_present]
+ - AbbrCode: 0x3
+ Values:
+ - Value: 0x1
+ - Value: 0x1
+
+# DW_TAG_subprogram ("f4")
+# DW_AT_declaration [DW_FORM_flag_present]
+# DW_AT_artificial [DW_FORM_flag_present]
+ - AbbrCode: 0x3
+ Values:
+ - Value: 0x1
+ - Value: 0x1
+
+# DW_TAG_subprogram ("f5")
+# DW_AT_declaration [DW_FORM_flag_present]
+# DW_AT_artificial [DW_FORM_flag_present]
+ - AbbrCode: 0x3
+ Values:
+ - Value: 0x1
+ - Value: 0x1
+
+ - AbbrCode: 0x0
+...
+)";
+ YAMLModuleTester t(yamldata);
+
+ DWARFUnit *unit = t.GetDwarfUnit();
+ ASSERT_NE(unit, nullptr);
+ const DWARFDebugInfoEntry *cu_entry = unit->DIE().GetDIE();
+ ASSERT_EQ(cu_entry->Tag(), DW_TAG_compile_unit);
+ ASSERT_EQ(unit->GetDWARFLanguageType(), DW_LANG_C_plus_plus);
+ DWARFDIE cu_die(unit, cu_entry);
+
+ auto f1 = cu_die.GetFirstChild();
+ ASSERT_TRUE(f1.IsValid());
+ ASSERT_EQ(f1.Tag(), DW_TAG_subprogram);
+
+ auto attrs = f1.GetAttributes(DWARFDebugInfoEntry::Recurse::yes);
+ EXPECT_EQ(attrs.Size(), 7U);
+ EXPECT_EQ(attrs.FindAttributeIndex(DW_AT_declaration), UINT32_MAX);
+}
+
+INSTANTIATE_TEST_SUITE_P(GetAttributeTests, GetAttributesTestFixture,
+ testing::Values(DW_AT_specification,
+ DW_AT_abstract_origin));
>From b48e5614bbcb7f44059b3e755c376f6d976d1389 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Fri, 17 Jan 2025 00:30:50 +0000
Subject: [PATCH 2/3] fixup! fix comments; make function static
---
.../SymbolFile/DWARF/DWARFDebugInfoEntry.cpp | 13 ++++---
.../SymbolFile/DWARF/DWARFDebugInfoEntry.h | 39 ++++++++++---------
.../SymbolFile/DWARF/DWARFDIETest.cpp | 2 -
3 files changed, 27 insertions(+), 27 deletions(-)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
index d26d302c33dd28..ed8e12ea3038fe 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -281,10 +281,11 @@ bool DWARFDebugInfoEntry::GetDIENamesAndRanges(
return !ranges.empty();
}
-bool DWARFDebugInfoEntry::GetAttributes(
- DWARFUnit const *cu, llvm::SmallVector<DWARFDIE> &worklist,
- llvm::SmallSet<DWARFDebugInfoEntry const *, 3> &seen,
- DWARFAttributes &attributes) const {
+/// Helper for the public \ref DWARFDebugInfoEntry::GetAttributes API.
+static bool GetAttributes(DWARFUnit const *cu,
+ llvm::SmallVector<DWARFDIE> &worklist,
+ llvm::SmallSet<DWARFDebugInfoEntry const *, 3> &seen,
+ DWARFAttributes &attributes) {
assert(!worklist.empty());
assert(cu);
@@ -348,7 +349,7 @@ bool DWARFDebugInfoEntry::GetAttributes(
return true;
}
-DWARFAttributes DWARFDebugInfoEntry::GetAttributes(DWARFUnit *cu,
+DWARFAttributes DWARFDebugInfoEntry::GetAttributes(const DWARFUnit *cu,
Recurse recurse) const {
// FIXME: use ElaboratingDIEIterator to follow specifications/abstract origins
// instead of maintaining our own worklist/seen list.
@@ -365,7 +366,7 @@ DWARFAttributes DWARFDebugInfoEntry::GetAttributes(DWARFUnit *cu,
seen.insert(this);
while (!worklist.empty()) {
- if (!GetAttributes(cu, worklist, seen, attributes)) {
+ if (!::GetAttributes(cu, worklist, seen, attributes)) {
attributes.Clear();
break;
}
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
index 28381114634319..16f1a5aede47d9 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -53,19 +53,26 @@ class DWARFDebugInfoEntry {
using Recurse = DWARFBaseDIE::Recurse;
- // Get all attribute values for a given DIE, including following any
- // specification or abstract origin attributes and including those in the
- // results.
- //
- // When following specifications/abstract origins, the attributes
- // on the referring DIE are guaranteed to be visited before the attributes of
- // the referenced DIE.
- //
- // \param[in]
- // \param[in]
- //
- // \returns output may have duplicate entries
- DWARFAttributes GetAttributes(DWARFUnit *cu,
+ /// Get all attribute values for a given DIE, including following any
+ /// specification or abstract origin attributes and including those in the
+ /// results.
+ ///
+ /// When following specifications/abstract origins, the attributes
+ /// on the referring DIE are guaranteed to be visited before the attributes of
+ /// the referenced DIE.
+ ///
+ /// \param[in] cu DWARFUnit that this entry belongs to.
+ ///
+ /// \param[in] recurse If set to \c Recurse::yes, will include attributes
+ /// on DIEs referenced via \c DW_AT_specification and \c DW_AT_abstract_origin
+ /// (including across multiple levels of indirection).
+ ///
+ /// \returns DWARFAttributes that include all attributes found on this DIE
+ /// (and possibly referenced DIEs). Attributes may appear multiple times
+ /// (e.g., if a declaration and definition both specify the same attribute).
+ /// On failure, the returned DWARFAttributes will be empty.
+ ///
+ DWARFAttributes GetAttributes(const DWARFUnit *cu,
Recurse recurse = Recurse::yes) const;
dw_offset_t GetAttributeValue(const DWARFUnit *cu, const dw_attr_t attr,
@@ -187,12 +194,6 @@ class DWARFDebugInfoEntry {
/// A copy of the DW_TAG value so we don't have to go through the compile
/// unit abbrev table
dw_tag_t m_tag = llvm::dwarf::DW_TAG_null;
-
-private:
- // Helper for the public \ref DWARFDebugInfoEntry::GetAttributes API.
- bool GetAttributes(DWARFUnit const *cu, llvm::SmallVector<DWARFDIE> &worklist,
- llvm::SmallSet<DWARFDebugInfoEntry const *, 3> &seen,
- DWARFAttributes &attributes) const;
};
} // namespace dwarf
} // namespace lldb_private::plugin
diff --git a/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp b/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp
index 4f544656a6312c..718c733ae91051 100644
--- a/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp
+++ b/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp
@@ -584,8 +584,6 @@ TEST_P(GetAttributesTestFixture, TestGetAttributes_Cycle) {
auto attrs = func.GetAttributes(DWARFDebugInfoEntry::Recurse::yes);
EXPECT_EQ(attrs.Size(), 3U);
-
- // TODO: check that we do form a cycle with the DIEs
}
TEST_P(GetAttributesTestFixture,
>From 638731dd5c63b534e51871e65ccc6d26e05ce8c2 Mon Sep 17 00:00:00 2001
From: Michael Buch <michaelbuch12 at gmail.com>
Date: Fri, 17 Jan 2025 00:38:22 +0000
Subject: [PATCH 3/3] fixup! add more assertions to cycle test
---
.../SymbolFile/DWARF/DWARFDIETest.cpp | 48 +++++++++++++++++--
1 file changed, 44 insertions(+), 4 deletions(-)
diff --git a/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp b/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp
index 718c733ae91051..3f61d1607073cf 100644
--- a/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp
+++ b/lldb/unittests/SymbolFile/DWARF/DWARFDIETest.cpp
@@ -520,6 +520,13 @@ TEST_P(GetAttributesTestFixture, TestGetAttributes_IterationOrder) {
TEST_P(GetAttributesTestFixture, TestGetAttributes_Cycle) {
// Tests that GetAttributes can deal with cycles in
// specifications/abstract origins.
+ //
+ // Contrived example:
+ //
+ // func1 -> func3
+ // ^ |
+ // | v
+ // +------func2
const char *yamldata = R"(
--- !ELF
@@ -578,12 +585,45 @@ TEST_P(GetAttributesTestFixture, TestGetAttributes_Cycle) {
ASSERT_EQ(unit->GetDWARFLanguageType(), DW_LANG_C_plus_plus);
DWARFDIE cu_die(unit, cu_entry);
- auto func = cu_die.GetFirstChild();
- ASSERT_TRUE(func.IsValid());
- ASSERT_EQ(func.Tag(), DW_TAG_subprogram);
+ auto func1 = cu_die.GetFirstChild();
+ ASSERT_TRUE(func1.IsValid());
+ ASSERT_EQ(func1.Tag(), DW_TAG_subprogram);
+
+ auto func2 = func1.GetSibling();
+ ASSERT_TRUE(func2.IsValid());
+ ASSERT_EQ(func2.Tag(), DW_TAG_subprogram);
+
+ auto func3 = func2.GetSibling();
+ ASSERT_TRUE(func3.IsValid());
+ ASSERT_EQ(func3.Tag(), DW_TAG_subprogram);
- auto attrs = func.GetAttributes(DWARFDebugInfoEntry::Recurse::yes);
+ auto attrs = func1.GetAttributes(DWARFDebugInfoEntry::Recurse::yes);
EXPECT_EQ(attrs.Size(), 3U);
+
+ // Confirm that the specifications do form a cycle.
+ {
+ DWARFFormValue form_value;
+ auto success = attrs.ExtractFormValueAtIndex(0, form_value);
+ ASSERT_TRUE(success);
+
+ EXPECT_EQ(form_value.Reference(), func3);
+ }
+
+ {
+ DWARFFormValue form_value;
+ auto success = attrs.ExtractFormValueAtIndex(1, form_value);
+ ASSERT_TRUE(success);
+
+ EXPECT_EQ(form_value.Reference(), func2);
+ }
+
+ {
+ DWARFFormValue form_value;
+ auto success = attrs.ExtractFormValueAtIndex(2, form_value);
+ ASSERT_TRUE(success);
+
+ EXPECT_EQ(form_value.Reference(), func1);
+ }
}
TEST_P(GetAttributesTestFixture,
More information about the lldb-commits
mailing list