[lldb/dwarf] Fix DW_IDX_parent processing for split dwarf (#92745)

DWARFDebugInfo only knows how to resolve references in its own file, but
in split dwarf, the index entries will refer to DIEs in the separate
(DWO) file. To resolve the DIERef correctly we'd either need to go
through the SymbolFileDWARF to get the full logic for resolving a
DIERef, or use the fact that ToDIERef already looks up the correct unit
while computing its result.

This patch does the latter.

This bug manifested itself in not being able to find type definitions
for types in namespaces, so I've modified one of our type resolving test
cases to run with debug_names, and added a namespaced class into it (it
originally contained only a top-level class).

Author: labath

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp

@@ -259,9 +259,3 @@ DWARFDebugInfo::GetDIE(const DIERef &die_ref) {
     return cu->GetNonSkeletonUnit().GetDIE(die_ref.die_offset());
   return DWARFDIE(); // Not found
 }
-
-llvm::StringRef DWARFDebugInfo::PeekDIEName(const DIERef &die_ref) {
-  if (DWARFUnit *cu = GetUnit(die_ref))
-    return cu->GetNonSkeletonUnit().PeekDIEName(die_ref.die_offset());
-  return llvm::StringRef();
-}

lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h

@@ -44,11 +44,6 @@ class DWARFDebugInfo {
   bool ContainsTypeUnits();
   DWARFDIE GetDIE(const DIERef &die_ref);
 
-  /// Returns the AT_Name of this DIE, if it exists, without parsing the entire
-  /// compile unit. An empty is string is returned upon error or if the
-  /// attribute is not present.
-  llvm::StringRef PeekDIEName(const DIERef &die_ref);
-
   enum {
     eDumpFlag_Verbose = (1 << 0),  // Verbose dumping
     eDumpFlag_ShowForm = (1 << 1), // Show the DW_form type

lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp

@@ -48,26 +48,30 @@ DebugNamesDWARFIndex::GetUnits(const DebugNames &debug_names) {
   return result;
 }
 
-std::optional<DIERef>
-DebugNamesDWARFIndex::ToDIERef(const DebugNames::Entry &entry) const {
+DWARFUnit *
+DebugNamesDWARFIndex::GetNonSkeletonUnit(const DebugNames::Entry &entry) const {
   // Look for a DWARF unit offset (CU offset or local TU offset) as they are
   // both offsets into the .debug_info section.
   std::optional<uint64_t> unit_offset = entry.getCUOffset();
   if (!unit_offset) {
     unit_offset = entry.getLocalTUOffset();
     if (!unit_offset)
-      return std::nullopt;
+      return nullptr;
   }
 
   DWARFUnit *cu =
       m_debug_info.GetUnitAtOffset(DIERef::Section::DebugInfo, *unit_offset);
-  if (!cu)
-    return std::nullopt;
+  return cu ? &cu->GetNonSkeletonUnit() : nullptr;
+}
 
-  cu = &cu->GetNonSkeletonUnit();
+std::optional<DIERef>
+DebugNamesDWARFIndex::ToDIERef(const DebugNames::Entry &entry) const {
+  DWARFUnit *unit = GetNonSkeletonUnit(entry);
+  if (!unit)
+    return std::nullopt;
   if (std::optional<uint64_t> die_offset = entry.getDIEUnitOffset())
-    return DIERef(cu->GetSymbolFileDWARF().GetFileIndex(),
-                  DIERef::Section::DebugInfo, cu->GetOffset() + *die_offset);
+    return DIERef(unit->GetSymbolFileDWARF().GetFileIndex(),
+                  DIERef::Section::DebugInfo, unit->GetOffset() + *die_offset);
 
   return std::nullopt;
 }
@@ -306,10 +310,10 @@ bool DebugNamesDWARFIndex::SameParentChain(
     auto maybe_dieoffset = entry.getDIEUnitOffset();
     if (!maybe_dieoffset)
       return false;
-    auto die_ref = ToDIERef(entry);
-    if (!die_ref)
+    DWARFUnit *unit = GetNonSkeletonUnit(entry);
+    if (!unit)
       return false;
-    return name == m_debug_info.PeekDIEName(*die_ref);
+    return name == unit->PeekDIEName(unit->GetOffset() + *maybe_dieoffset);
   };
 
   // If the AT_name of any parent fails to match the expected name, we don't

lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h

@@ -84,6 +84,7 @@ class DebugNamesDWARFIndex : public DWARFIndex {
   std::unique_ptr<DebugNames> m_debug_names_up;
   ManualDWARFIndex m_fallback;
 
+  DWARFUnit *GetNonSkeletonUnit(const DebugNames::Entry &entry) const;
   std::optional<DIERef> ToDIERef(const DebugNames::Entry &entry) const;
   bool ProcessEntry(const DebugNames::Entry &entry,
                     llvm::function_ref<bool(DWARFDIE die)> callback);

lldb/test/API/lang/cpp/limit-debug-info/Makefile

@@ -1,5 +1,3 @@
 CXX_SOURCES = main.cpp derived.cpp base.cpp
 
-CFLAGS_EXTRAS = $(LIMIT_DEBUG_INFO_FLAGS)
-
 include Makefile.rules

lldb/test/API/lang/cpp/limit-debug-info/TestWithLimitDebugInfo.py

@@ -5,13 +5,8 @@
 
 
 class TestWithLimitDebugInfo(TestBase):
-    @add_test_categories(["dwarf", "dwo"])
-    def test_limit_debug_info(self):
-        self.build()
-
-        src_file = os.path.join(self.getSourceDir(), "main.cpp")
-        src_file_spec = lldb.SBFileSpec(src_file)
-        self.assertTrue(src_file_spec.IsValid(), "breakpoint file")
+    def _run_test(self, build_dict):
+        self.build(dictionary=build_dict)
 
         # Get the path of the executable
         exe_path = self.getBuildArtifact("a.out")
@@ -21,9 +16,11 @@ def test_limit_debug_info(self):
         self.assertTrue(target.IsValid(), VALID_TARGET)
 
         # Break on main function
-        breakpoint = target.BreakpointCreateBySourceRegex("break here", src_file_spec)
-        self.assertTrue(
-            breakpoint.IsValid() and breakpoint.GetNumLocations() >= 1, VALID_BREAKPOINT
+        lldbutil.run_break_set_by_file_and_line(
+            self, "derived.h", line_number("derived.h", "// break1")
+        )
+        lldbutil.run_break_set_by_file_and_line(
+            self, "derived.h", line_number("derived.h", "// break2")
         )
 
         # Launch the process
@@ -32,14 +29,23 @@ def test_limit_debug_info(self):
 
         # Get the thread of the process
         self.assertEqual(process.GetState(), lldb.eStateStopped, PROCESS_STOPPED)
-        thread = lldbutil.get_stopped_thread(process, lldb.eStopReasonBreakpoint)
-        thread.StepInto()
 
-        # Get frame for current thread
-        frame = thread.GetSelectedFrame()
+        self.expect_expr("1", result_type="int", result_value="1")
+        self.expect_expr("this", result_type="Foo *")
+        self.expect_expr("this->x", result_type="int", result_value="12345")
+
+        self.runCmd("continue")
 
         self.expect_expr("1", result_type="int", result_value="1")
+        self.expect_expr("this", result_type="ns::Foo2 *")
+        self.expect_expr("this->x", result_type="int", result_value="23456")
 
-        v2 = frame.EvaluateExpression("this")
-        self.assertTrue(v2.IsValid(), "'expr this' results in a valid SBValue object")
-        self.assertSuccess(v2.GetError(), "'expr this' succeeds without an error.")
+    @add_test_categories(["dwarf", "dwo"])
+    def test_default(self):
+        self._run_test(dict(CFLAGS_EXTRAS="$(LIMIT_DEBUG_INFO_FLAGS)"))
+
+    @add_test_categories(["dwarf", "dwo"])
+    def test_debug_names(self):
+        self._run_test(
+            dict(CFLAGS_EXTRAS="$(LIMIT_DEBUG_INFO_FLAGS) -gdwarf-5 -gpubnames")
+        )

lldb/test/API/lang/cpp/limit-debug-info/base.cpp

@@ -1,8 +1,7 @@
 #include "base.h"
 
-FooNS::FooNS() : x(12345) {}
-
-void FooNS::bar() {
-    x = 54321;
-}
+FooBase::FooBase() : x(12345) {}
+ns::Foo2Base::Foo2Base() : x(23456) {}
 
+void FooBase::bar() {}
+void ns::Foo2Base::bar() {}

lldb/test/API/lang/cpp/limit-debug-info/base.h

@@ -1,12 +1,22 @@
-class FooNS
-{
+class FooBase {
 public:
-    virtual void bar();
-    virtual char baz() = 0;
+  virtual void bar();
 
 protected:
-    FooNS();
+  FooBase();
 
-    int x;
+  int x;
 };
 
+namespace ns {
+class Foo2Base {
+public:
+  virtual void bar();
+
+protected:
+  Foo2Base();
+
+  int x;
+};
+
+} // namespace ns

lldb/test/API/lang/cpp/limit-debug-info/derived.cpp

@@ -1,11 +1,10 @@
 #include "derived.h"
 
-Foo foo1;
-Foo foo2;
-
 Foo::Foo() { a = 12345; }
+ns::Foo2::Foo2() { a = 23456; }
 
-char Foo::baz() {
-    return (char)(x&0xff);
-}
+Foo foo1;
+Foo foo2;
 
+ns::Foo2 foo2_1;
+ns::Foo2 foo2_2;

lldb/test/API/lang/cpp/limit-debug-info/derived.h

@@ -1,19 +1,36 @@
 #include "base.h"
 
-class Foo : public FooNS
-{
+class Foo : public FooBase {
 public:
-    Foo();
+  Foo();
 
-    // Deliberately defined by hand.
-    Foo &operator=(const Foo &rhs) {
-      a = rhs.a;
-      return *this;
-    }
+  // Deliberately defined by hand.
+  Foo &operator=(const Foo &rhs) {
+    x = rhs.x; // break1
+    a = rhs.a;
+    return *this;
+  }
+  int a;
+};
+
+namespace ns {
+class Foo2 : public Foo2Base {
+public:
+  Foo2();
 
-    char baz() override;
-    int a;
+  // Deliberately defined by hand.
+  Foo2 &operator=(const Foo2 &rhs) {
+    x = rhs.x; // break2
+    a = rhs.a;
+    return *this;
+  }
+
+  int a;
 };
+} // namespace ns
 
 extern Foo foo1;
 extern Foo foo2;
+
+extern ns::Foo2 foo2_1;
+extern ns::Foo2 foo2_2;