[Lldb-commits] [lldb] [lldb] Recurse through DW_AT_signature when looking for attributes (PR #107241)

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Thu Sep 5 04:19:10 PDT 2024


https://github.com/labath updated https://github.com/llvm/llvm-project/pull/107241

>From 2fd6f97ed01eee17b28c1c843f0d891a460a2fb3 Mon Sep 17 00:00:00 2001
From: Pavel Labath <pavel at labath.sk>
Date: Wed, 4 Sep 2024 13:36:07 +0200
Subject: [PATCH 1/2] [lldb] Recurse through DW_AT_signature when looking for
 attributes

This allows e.g. DWARFDIE::GetName() to return the name of the type when
looking at its declaration (which contains only
DW_AT_declaration+DW_AT_signature). This is similar to how we recurse
through DW_AT_specification when looking for a function name. Llvm dwarf
parser has obtained the same functionality through #99495.

This fixes a bug where we would confuse a type like NS::Outer::Struct
with NS::Struct (because NS::Outer (and its name) was in a type unit).
---
 .../Plugins/SymbolFile/DWARF/DWARFDIE.cpp     | 18 +----
 .../SymbolFile/DWARF/DWARFDebugInfoEntry.cpp  | 78 ++++++++-----------
 .../SymbolFile/DWARF/DWARFDebugInfoEntry.h    | 57 +++++++-------
 .../DWARF/x86/type-unit-same-basename.cpp     | 45 +++++++++++
 4 files changed, 110 insertions(+), 88 deletions(-)
 create mode 100644 lldb/test/Shell/SymbolFile/DWARF/x86/type-unit-same-basename.cpp

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
index 0a13c457a307ae..e94b3198c63d90 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -50,7 +50,8 @@ class ElaboratingDIEIterator
     m_worklist.pop_back();
 
     // And add back any items that elaborate it.
-    for (dw_attr_t attr : {DW_AT_specification, DW_AT_abstract_origin}) {
+    for (dw_attr_t attr :
+         {DW_AT_specification, DW_AT_abstract_origin, DW_AT_specification}) {
       if (DWARFDIE d = die.GetReferencedDIE(attr))
         if (m_seen.insert(die.GetDIE()).second)
           m_worklist.push_back(d);
@@ -129,10 +130,10 @@ DWARFDIE
 DWARFDIE::GetAttributeValueAsReferenceDIE(const dw_attr_t attr) const {
   if (IsValid()) {
     DWARFUnit *cu = GetCU();
-    const bool check_specification_or_abstract_origin = true;
+    const bool check_elaborating_dies = true;
     DWARFFormValue form_value;
     if (m_die->GetAttributeValue(cu, attr, form_value, nullptr,
-                                 check_specification_or_abstract_origin))
+                                 check_elaborating_dies))
       return form_value.Reference();
   }
   return DWARFDIE();
@@ -377,11 +378,6 @@ static void GetDeclContextImpl(DWARFDIE die,
       die = spec;
       continue;
     }
-    // To find the name of a type in a type unit, we must follow the signature.
-    if (DWARFDIE spec = die.GetReferencedDIE(DW_AT_signature)) {
-      die = spec;
-      continue;
-    }
 
     // Add this DIE's contribution at the end of the chain.
     auto push_ctx = [&](CompilerContextKind kind, llvm::StringRef name) {
@@ -434,12 +430,6 @@ static void GetTypeLookupContextImpl(DWARFDIE die,
                                      std::vector<CompilerContext> &context) {
   // Stop if we hit a cycle.
   while (die && seen.insert(die.GetID()).second) {
-    // To find the name of a type in a type unit, we must follow the signature.
-    if (DWARFDIE spec = die.GetReferencedDIE(DW_AT_signature)) {
-      die = spec;
-      continue;
-    }
-
     // Add this DIE's contribution at the end of the chain.
     auto push_ctx = [&](CompilerContextKind kind, llvm::StringRef name) {
       context.push_back({kind, ConstString(name)});
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
index e2660735ea7dec..77ef59d605c9c4 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.cpp
@@ -355,8 +355,7 @@ void DWARFDebugInfoEntry::GetAttributes(DWARFUnit *cu,
 // would be a compile unit header).
 dw_offset_t DWARFDebugInfoEntry::GetAttributeValue(
     const DWARFUnit *cu, const dw_attr_t attr, DWARFFormValue &form_value,
-    dw_offset_t *end_attr_offset_ptr,
-    bool check_specification_or_abstract_origin) const {
+    dw_offset_t *end_attr_offset_ptr, bool check_elaborating_dies) const {
   if (const auto *abbrevDecl = GetAbbreviationDeclarationPtr(cu)) {
     std::optional<uint32_t> attr_idx = abbrevDecl->findAttributeIndex(attr);
 
@@ -380,25 +379,18 @@ dw_offset_t DWARFDebugInfoEntry::GetAttributeValue(
     }
   }
 
-  if (check_specification_or_abstract_origin) {
-    if (GetAttributeValue(cu, DW_AT_specification, form_value)) {
+  if (check_elaborating_dies) {
+    for (dw_attr_t elaborating_attr :
+         {DW_AT_specification, DW_AT_abstract_origin, DW_AT_signature}) {
+      if (!GetAttributeValue(cu, elaborating_attr, form_value))
+        continue;
       DWARFDIE die = form_value.Reference();
-      if (die) {
-        dw_offset_t die_offset = die.GetDIE()->GetAttributeValue(
-            die.GetCU(), attr, form_value, end_attr_offset_ptr, false);
-        if (die_offset)
-          return die_offset;
-      }
-    }
-
-    if (GetAttributeValue(cu, DW_AT_abstract_origin, form_value)) {
-      DWARFDIE die = form_value.Reference();
-      if (die) {
-        dw_offset_t die_offset = die.GetDIE()->GetAttributeValue(
-            die.GetCU(), attr, form_value, end_attr_offset_ptr, false);
-        if (die_offset)
-          return die_offset;
-      }
+      if (!die)
+        continue;
+      dw_offset_t die_offset = die.GetDIE()->GetAttributeValue(
+          die.GetCU(), attr, form_value, end_attr_offset_ptr, false);
+      if (die_offset)
+        return die_offset;
     }
   }
   return 0;
@@ -412,10 +404,9 @@ dw_offset_t DWARFDebugInfoEntry::GetAttributeValue(
 // doesn't change.
 const char *DWARFDebugInfoEntry::GetAttributeValueAsString(
     const DWARFUnit *cu, const dw_attr_t attr, const char *fail_value,
-    bool check_specification_or_abstract_origin) const {
+    bool check_elaborating_dies) const {
   DWARFFormValue form_value;
-  if (GetAttributeValue(cu, attr, form_value, nullptr,
-                        check_specification_or_abstract_origin))
+  if (GetAttributeValue(cu, attr, form_value, nullptr, check_elaborating_dies))
     return form_value.AsCString();
   return fail_value;
 }
@@ -425,10 +416,9 @@ const char *DWARFDebugInfoEntry::GetAttributeValueAsString(
 // Get the value of an attribute as unsigned and return it.
 uint64_t DWARFDebugInfoEntry::GetAttributeValueAsUnsigned(
     const DWARFUnit *cu, const dw_attr_t attr, uint64_t fail_value,
-    bool check_specification_or_abstract_origin) const {
+    bool check_elaborating_dies) const {
   DWARFFormValue form_value;
-  if (GetAttributeValue(cu, attr, form_value, nullptr,
-                        check_specification_or_abstract_origin))
+  if (GetAttributeValue(cu, attr, form_value, nullptr, check_elaborating_dies))
     return form_value.Unsigned();
   return fail_value;
 }
@@ -436,10 +426,9 @@ uint64_t DWARFDebugInfoEntry::GetAttributeValueAsUnsigned(
 std::optional<uint64_t>
 DWARFDebugInfoEntry::GetAttributeValueAsOptionalUnsigned(
     const DWARFUnit *cu, const dw_attr_t attr,
-    bool check_specification_or_abstract_origin) const {
+    bool check_elaborating_dies) const {
   DWARFFormValue form_value;
-  if (GetAttributeValue(cu, attr, form_value, nullptr,
-                        check_specification_or_abstract_origin))
+  if (GetAttributeValue(cu, attr, form_value, nullptr, check_elaborating_dies))
     return form_value.Unsigned();
   return std::nullopt;
 }
@@ -450,20 +439,18 @@ DWARFDebugInfoEntry::GetAttributeValueAsOptionalUnsigned(
 // relative offsets as needed.
 DWARFDIE DWARFDebugInfoEntry::GetAttributeValueAsReference(
     const DWARFUnit *cu, const dw_attr_t attr,
-    bool check_specification_or_abstract_origin) const {
+    bool check_elaborating_dies) const {
   DWARFFormValue form_value;
-  if (GetAttributeValue(cu, attr, form_value, nullptr,
-                        check_specification_or_abstract_origin))
+  if (GetAttributeValue(cu, attr, form_value, nullptr, check_elaborating_dies))
     return form_value.Reference();
   return {};
 }
 
 uint64_t DWARFDebugInfoEntry::GetAttributeValueAsAddress(
     const DWARFUnit *cu, const dw_attr_t attr, uint64_t fail_value,
-    bool check_specification_or_abstract_origin) const {
+    bool check_elaborating_dies) const {
   DWARFFormValue form_value;
-  if (GetAttributeValue(cu, attr, form_value, nullptr,
-                        check_specification_or_abstract_origin))
+  if (GetAttributeValue(cu, attr, form_value, nullptr, check_elaborating_dies))
     return form_value.Address();
   return fail_value;
 }
@@ -474,12 +461,13 @@ uint64_t DWARFDebugInfoEntry::GetAttributeValueAsAddress(
 // pc>.
 //
 // Returns the hi_pc or fail_value.
-dw_addr_t DWARFDebugInfoEntry::GetAttributeHighPC(
-    const DWARFUnit *cu, dw_addr_t lo_pc, uint64_t fail_value,
-    bool check_specification_or_abstract_origin) const {
+dw_addr_t
+DWARFDebugInfoEntry::GetAttributeHighPC(const DWARFUnit *cu, dw_addr_t lo_pc,
+                                        uint64_t fail_value,
+                                        bool check_elaborating_dies) const {
   DWARFFormValue form_value;
   if (GetAttributeValue(cu, DW_AT_high_pc, form_value, nullptr,
-                        check_specification_or_abstract_origin)) {
+                        check_elaborating_dies)) {
     dw_form_t form = form_value.Form();
     if (form == DW_FORM_addr || form == DW_FORM_addrx ||
         form == DW_FORM_GNU_addr_index)
@@ -499,12 +487,11 @@ dw_addr_t DWARFDebugInfoEntry::GetAttributeHighPC(
 // Returns true or sets lo_pc and hi_pc to fail_value.
 bool DWARFDebugInfoEntry::GetAttributeAddressRange(
     const DWARFUnit *cu, dw_addr_t &lo_pc, dw_addr_t &hi_pc,
-    uint64_t fail_value, bool check_specification_or_abstract_origin) const {
+    uint64_t fail_value, bool check_elaborating_dies) const {
   lo_pc = GetAttributeValueAsAddress(cu, DW_AT_low_pc, fail_value,
-                                     check_specification_or_abstract_origin);
+                                     check_elaborating_dies);
   if (lo_pc != fail_value) {
-    hi_pc = GetAttributeHighPC(cu, lo_pc, fail_value,
-                               check_specification_or_abstract_origin);
+    hi_pc = GetAttributeHighPC(cu, lo_pc, fail_value, check_elaborating_dies);
     if (hi_pc != fail_value)
       return true;
   }
@@ -514,8 +501,7 @@ bool DWARFDebugInfoEntry::GetAttributeAddressRange(
 }
 
 DWARFRangeList DWARFDebugInfoEntry::GetAttributeAddressRanges(
-    DWARFUnit *cu, bool check_hi_lo_pc,
-    bool check_specification_or_abstract_origin) const {
+    DWARFUnit *cu, bool check_hi_lo_pc, bool check_elaborating_dies) const {
 
   DWARFFormValue form_value;
   if (GetAttributeValue(cu, DW_AT_ranges, form_value))
@@ -526,7 +512,7 @@ DWARFRangeList DWARFDebugInfoEntry::GetAttributeAddressRanges(
     dw_addr_t lo_pc = LLDB_INVALID_ADDRESS;
     dw_addr_t hi_pc = LLDB_INVALID_ADDRESS;
     if (GetAttributeAddressRange(cu, lo_pc, hi_pc, LLDB_INVALID_ADDRESS,
-                                 check_specification_or_abstract_origin)) {
+                                 check_elaborating_dies)) {
       if (lo_pc < hi_pc)
         ranges.Append(DWARFRangeList::Entry(lo_pc, hi_pc - lo_pc));
     }
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
index 3816c6500717af..20c07c95ec47b5 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfoEntry.h
@@ -60,44 +60,45 @@ class DWARFDebugInfoEntry {
     return attrs;
   }
 
-  dw_offset_t
-  GetAttributeValue(const DWARFUnit *cu, const dw_attr_t attr,
-                    DWARFFormValue &formValue,
-                    dw_offset_t *end_attr_offset_ptr = nullptr,
-                    bool check_specification_or_abstract_origin = false) const;
+  dw_offset_t GetAttributeValue(const DWARFUnit *cu, const dw_attr_t attr,
+                                DWARFFormValue &formValue,
+                                dw_offset_t *end_attr_offset_ptr = nullptr,
+                                bool check_elaborating_dies = false) const;
 
-  const char *GetAttributeValueAsString(
-      const DWARFUnit *cu, const dw_attr_t attr, const char *fail_value,
-      bool check_specification_or_abstract_origin = false) const;
+  const char *
+  GetAttributeValueAsString(const DWARFUnit *cu, const dw_attr_t attr,
+                            const char *fail_value,
+                            bool check_elaborating_dies = false) const;
 
-  uint64_t GetAttributeValueAsUnsigned(
-      const DWARFUnit *cu, const dw_attr_t attr, uint64_t fail_value,
-      bool check_specification_or_abstract_origin = false) const;
+  uint64_t
+  GetAttributeValueAsUnsigned(const DWARFUnit *cu, const dw_attr_t attr,
+                              uint64_t fail_value,
+                              bool check_elaborating_dies = false) const;
 
   std::optional<uint64_t> GetAttributeValueAsOptionalUnsigned(
       const DWARFUnit *cu, const dw_attr_t attr,
-      bool check_specification_or_abstract_origin = false) const;
+      bool check_elaborating_dies = false) const;
 
-  DWARFDIE GetAttributeValueAsReference(
-      const DWARFUnit *cu, const dw_attr_t attr,
-      bool check_specification_or_abstract_origin = false) const;
+  DWARFDIE
+  GetAttributeValueAsReference(const DWARFUnit *cu, const dw_attr_t attr,
+                               bool check_elaborating_dies = false) const;
 
-  uint64_t GetAttributeValueAsAddress(
-      const DWARFUnit *cu, const dw_attr_t attr, uint64_t fail_value,
-      bool check_specification_or_abstract_origin = false) const;
+  uint64_t
+  GetAttributeValueAsAddress(const DWARFUnit *cu, const dw_attr_t attr,
+                             uint64_t fail_value,
+                             bool check_elaborating_dies = false) const;
 
-  dw_addr_t
-  GetAttributeHighPC(const DWARFUnit *cu, dw_addr_t lo_pc, uint64_t fail_value,
-                     bool check_specification_or_abstract_origin = false) const;
+  dw_addr_t GetAttributeHighPC(const DWARFUnit *cu, dw_addr_t lo_pc,
+                               uint64_t fail_value,
+                               bool check_elaborating_dies = false) const;
 
-  bool GetAttributeAddressRange(
-      const DWARFUnit *cu, dw_addr_t &lo_pc, dw_addr_t &hi_pc,
-      uint64_t fail_value,
-      bool check_specification_or_abstract_origin = false) const;
+  bool GetAttributeAddressRange(const DWARFUnit *cu, dw_addr_t &lo_pc,
+                                dw_addr_t &hi_pc, uint64_t fail_value,
+                                bool check_elaborating_dies = false) const;
 
-  DWARFRangeList GetAttributeAddressRanges(
-      DWARFUnit *cu, bool check_hi_lo_pc,
-      bool check_specification_or_abstract_origin = false) const;
+  DWARFRangeList
+  GetAttributeAddressRanges(DWARFUnit *cu, bool check_hi_lo_pc,
+                            bool check_elaborating_dies = false) const;
 
   const char *GetName(const DWARFUnit *cu) const;
 
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/type-unit-same-basename.cpp b/lldb/test/Shell/SymbolFile/DWARF/x86/type-unit-same-basename.cpp
new file mode 100644
index 00000000000000..f7f5a30aaba9ef
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/type-unit-same-basename.cpp
@@ -0,0 +1,45 @@
+// Test that we can correctly disambiguate a nested type (where the outer type
+// is in a type unit) from a non-nested type with the same basename. Failure to
+// do so can cause us to think a type is a member of itself, which caused
+// infinite recursion (crash) in the past.
+
+// REQUIRES: lld
+
+// RUN: %clang --target=x86_64-pc-linux -c %s -o %t-a.o -g -fdebug-types-section -flimit-debug-info -DFILE_A
+// RUN: %clang --target=x86_64-pc-linux -c %s -o %t-b.o -g -fdebug-types-section -flimit-debug-info -DFILE_B
+// RUN: ld.lld -z undefs %t-a.o %t-b.o -o %t
+// RUN: %lldb %t -o "target variable x" -o exit | FileCheck %s
+
+// CHECK: (lldb) target variable
+// CHECK-NEXT: (const X) x = {
+// CHECK-NEXT:   NS::Outer::Struct = {
+// CHECK-NEXT:     x = 42
+// CHECK-NEXT:     o = (x = 47)
+// CHECK-NEXT:     y = 24
+// CHECK-NEXT:   }
+// CHECK-NEXT: }
+
+namespace NS {
+struct Struct {
+  int x = 47;
+  virtual void anchor();
+};
+} // namespace NS
+
+#ifdef FILE_A
+namespace NS {
+struct Outer {
+  struct Struct {
+    int x = 42;
+    NS::Struct o;
+    int y = 24;
+  };
+};
+} // namespace NS
+
+struct X : NS::Outer::Struct {};
+extern constexpr X x = {};
+#endif
+#ifdef FILE_B
+void NS::Struct::anchor() {}
+#endif

>From 9840edd94d6b35ff0cd04a29f94ea6d07293a894 Mon Sep 17 00:00:00 2001
From: Pavel Labath <pavel at labath.sk>
Date: Thu, 5 Sep 2024 13:00:13 +0200
Subject: [PATCH 2/2] fix elaborating iterator

---
 lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
index e94b3198c63d90..d83740f8e2113b 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDIE.cpp
@@ -25,9 +25,9 @@ using namespace lldb_private::plugin::dwarf;
 namespace {
 
 /// Iterate through all DIEs elaborating (i.e. reachable by a chain of
-/// DW_AT_specification and DW_AT_abstract_origin attributes) a given DIE. For
-/// convenience, the starting die is included in the sequence as the first
-/// item.
+/// DW_AT_specification, DW_AT_abstract_origin and/or DW_AT_signature
+/// attributes) a given DIE. For convenience, the starting die is included in
+/// the sequence as the first item.
 class ElaboratingDIEIterator
     : public llvm::iterator_facade_base<
           ElaboratingDIEIterator, std::input_iterator_tag, DWARFDIE,
@@ -51,7 +51,7 @@ class ElaboratingDIEIterator
 
     // And add back any items that elaborate it.
     for (dw_attr_t attr :
-         {DW_AT_specification, DW_AT_abstract_origin, DW_AT_specification}) {
+         {DW_AT_specification, DW_AT_abstract_origin, DW_AT_signature}) {
       if (DWARFDIE d = die.GetReferencedDIE(attr))
         if (m_seen.insert(die.GetDIE()).second)
           m_worklist.push_back(d);



More information about the lldb-commits mailing list