[lldb][DWARF] Change GetAttributes to always visit current DIE before recursing #123261
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… 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 llvm#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.
|
@llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) Changes
This patch makes sure we visit the current DIE's attributes before following DIE references. I tried keeping as much of the original There's precendent for this iteration order in Patch is 25.23 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/123261.diff 3 Files Affected:
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:
+ ...
[truncated]
|
labath
approved these changes
Jan 17, 2025
Co-authored-by: Pavel Labath <[email protected]>
Co-authored-by: Pavel Labath <[email protected]>
|
@labath the test |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
GetAttributesreturns all attributes on a given DIE, including any attributes that the DIE references viaDW_AT_abstract_originandDW_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 returnedDWARFAttributes. But there was no guarantee in which order those attributes get visited. That means there's no convenient 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 forDW_AT_object_pointer(which can exist on both definitions and declarations, see #123089).This patch makes sure we visit the current DIE's attributes before following DIE references. I tried keeping as much of the original
GetAttributesunchanged and just add an outerGetAttributesthat keeps track of the DIEs we need to visit next.There's precendent for this iteration order in
llvm::DWARFDie::findRecursivelyand alsolldb_private::ElaboratingDIEIterator. We could use the latter to implementGetAttributes, though it also followsDW_AT_signatureso I decided to leave it for follow-up.