Add support for parsing type unit entries in .debug_names.

[clayborg]

This is a follow up patch after .debug_names can now emit local type unit entries when we compile with type units + DWARF5 + .debug_names. The pull request that added this functionality was:

#70515

This patch makes sure that the DebugNamesDWARFIndex in LLDB will not manually need to parse type units if they have a valid index. It also fixes the index to be able to correctly extract name entries that reference type unit DIEs. Added a test to verify things work as expected.

[llvmbot]

@llvm/pr-subscribers-debuginfo

@llvm/pr-subscribers-lldb

Author: Greg Clayton (clayborg)

Changes

This is a follow up patch after .debug_names can now emit local type unit entries when we compile with type units + DWARF5 + .debug_names. The pull request that added this functionality was:

#70515

This patch makes sure that the DebugNamesDWARFIndex in LLDB will not manually need to parse type units if they have a valid index. It also fixes the index to be able to correctly extract name entries that reference type unit DIEs. Added a test to verify things work as expected.

---

Full diff: https://github.com/llvm/llvm-project/pull/72952.diff

4 Files Affected:

• (modified) lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp (+14-4)
• (added) lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-debug-names.cpp (+48)
• (modified) llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h (+18-1)
• (modified) llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp (+17-1)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index 4fc3866a3b608fd..62c5417191a124d 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -37,19 +37,29 @@ llvm::DenseSet<dw_offset_t>
 DebugNamesDWARFIndex::GetUnits(const DebugNames &debug_names) {
   llvm::DenseSet<dw_offset_t> result;
   for (const DebugNames::NameIndex &ni : debug_names) {
-    for (uint32_t cu = 0; cu < ni.getCUCount(); ++cu)
+    const uint32_t num_cus = ni.getCUCount();
+    for (uint32_t cu = 0; cu < num_cus; ++cu)
       result.insert(ni.getCUOffset(cu));
+    const uint32_t num_tus = ni.getLocalTUCount();
+    for (uint32_t tu = 0; tu < num_tus; ++tu)
+      result.insert(ni.getLocalTUOffset(tu));
   }
   return result;
 }
 
 std::optional<DIERef>
 DebugNamesDWARFIndex::ToDIERef(const DebugNames::Entry &entry) {
+  // Look for a CU offset or a local TU offset as they are both offsets into
+  // the .debug_info section.
   std::optional<uint64_t> cu_offset = entry.getCUOffset();
-  if (!cu_offset)
-    return std::nullopt;
+  if (!cu_offset) {
+    cu_offset = entry.getLocalTUOffset();
+    if (!cu_offset)
+      return std::nullopt;
+  }
 
-  DWARFUnit *cu = m_debug_info.GetUnitAtOffset(DIERef::Section::DebugInfo, *cu_offset);
+  DWARFUnit *cu =
+      m_debug_info.GetUnitAtOffset(DIERef::Section::DebugInfo, *cu_offset);
   if (!cu)
     return std::nullopt;
 
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-debug-names.cpp b/lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-debug-names.cpp
new file mode 100644
index 000000000000000..2b7a928c89a8f71
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/debug-types-debug-names.cpp
@@ -0,0 +1,48 @@
+// Test that we can use .debug_names to lookup a type that is only referenced
+// from within a type unit. In the code below the type named "stype" is only
+// referenced within the type unit itself and when we enable .debug_names, we
+// expect the have an entry for this and to be able to find this type when
+// we do a lookup.
+
+// REQUIRES: lld
+
+// RUN: %clang %s -target x86_64-pc-linux -gdwarf-5 -fdebug-types-section \
+// RUN:   -gpubnames -fno-limit-debug-info -c -o %t.o
+// RUN: ld.lld %t.o -o %t
+// RUN: %lldb %t -o "type lookup stype" -b | FileCheck %s --check-prefix=BASE
+// RUN: %lldb %t -o "type lookup bar::stype" -b | FileCheck %s --check-prefix=PART
+// RUN: %lldb %t -o "type lookup foo::bar::stype" -b | FileCheck %s --check-prefix=FULL
+
+// BASE: (lldb) type lookup stype
+// BASE-NEXT: int
+
+// PART: (lldb) type lookup bar::stype
+// PART-NEXT: int
+
+// FULL: (lldb) type lookup foo::bar::stype
+// FULL-NEXT: int
+
+namespace foo {
+class bar {
+public:
+  typedef unsigned utype;
+  // This type is only referenced from within the type unit and we need to
+  // make sure we can find it with the new type unit support in .debug_names.
+  typedef int stype;
+
+private:
+  utype m_unsigned;
+
+public:
+  bar(utype u) : m_unsigned(u) {}
+
+  utype get() const { return m_unsigned; }
+  void set(utype u) { m_unsigned = u; }
+  stype gets() const { return (stype)m_unsigned; }
+};
+} // namespace foo
+
+int main() {
+  foo::bar b(12);
+  return 0;
+}
diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
index 1ba555a061904cc..b89536bc0c7230c 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
@@ -56,6 +56,14 @@ class DWARFAcceleratorTable {
     /// recorded in this Accelerator Entry.
     virtual std::optional<uint64_t> getCUOffset() const = 0;
 
+    /// Returns the Offset of the Type Unit associated with this
+    /// Accelerator Entry or std::nullopt if the Type Unit offset is not
+    /// recorded in this Accelerator Entry.
+    virtual std::optional<uint64_t> getLocalTUOffset() const {
+      // Default return for accelerator tables that don't support type units.
+      return std::nullopt;
+    }
+
     /// Returns the Tag of the Debug Info Entry associated with this
     /// Accelerator Entry or std::nullopt if the Tag is not recorded in this
     /// Accelerator Entry.
@@ -424,6 +432,7 @@ class DWARFDebugNames : public DWARFAcceleratorTable {
 
   public:
     std::optional<uint64_t> getCUOffset() const override;
+    std::optional<uint64_t> getLocalTUOffset() const override;
     std::optional<dwarf::Tag> getTag() const override { return tag(); }
 
     /// Returns the Index into the Compilation Unit list of the owning Name
@@ -433,9 +442,17 @@ class DWARFDebugNames : public DWARFAcceleratorTable {
     /// which will handle that check itself). Note that entries in NameIndexes
     /// which index just a single Compilation Unit are implicitly associated
     /// with that unit, so this function will return 0 even without an explicit
-    /// DW_IDX_compile_unit attribute.
+    /// DW_IDX_compile_unit attribute, unless there is a DW_IDX_type_unit
+    /// attribute.
     std::optional<uint64_t> getCUIndex() const;
 
+    /// Returns the Index into the Local Type Unit list of the owning Name
+    /// Index or std::nullopt if this Accelerator Entry does not have an
+    /// associated Type Unit. It is up to the user to verify that the
+    /// returned Index is valid in the owning NameIndex (or use
+    /// getLocalTUOffset(), which will handle that check itself).
+    std::optional<uint64_t> getLocalTUIndex() const;
+
     /// .debug_names-specific getter, which always succeeds (DWARF v5 index
     /// entries always have a tag).
     dwarf::Tag tag() const { return Abbr->Tag; }
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp b/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
index 8302cbbf231aedf..0f9c8ef485d456e 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
@@ -621,7 +621,10 @@ std::optional<uint64_t> DWARFDebugNames::Entry::getCUIndex() const {
   if (std::optional<DWARFFormValue> Off = lookup(dwarf::DW_IDX_compile_unit))
     return Off->getAsUnsignedConstant();
   // In a per-CU index, the entries without a DW_IDX_compile_unit attribute
-  // implicitly refer to the single CU.
+  // implicitly refer to the single CU, but only if we don't have a
+  // DW_IDX_type_unit.
+  if (lookup(dwarf::DW_IDX_type_unit).has_value())
+    return std::nullopt;
   if (NameIdx->getCUCount() == 1)
     return 0;
   return std::nullopt;
@@ -634,6 +637,19 @@ std::optional<uint64_t> DWARFDebugNames::Entry::getCUOffset() const {
   return NameIdx->getCUOffset(*Index);
 }
 
+std::optional<uint64_t> DWARFDebugNames::Entry::getLocalTUOffset() const {
+  std::optional<uint64_t> Index = getLocalTUIndex();
+  if (!Index || *Index >= NameIdx->getLocalTUCount())
+    return std::nullopt;
+  return NameIdx->getLocalTUOffset(*Index);
+}
+
+std::optional<uint64_t> DWARFDebugNames::Entry::getLocalTUIndex() const {
+  if (std::optional<DWARFFormValue> Off = lookup(dwarf::DW_IDX_type_unit))
+    return Off->getAsUnsignedConstant();
+  return std::nullopt;
+}
+
 void DWARFDebugNames::Entry::dump(ScopedPrinter &W) const {
   W.startLine() << formatv("Abbrev: {0:x}\n", Abbr->Code);
   W.startLine() << formatv("Tag: {0}\n", Abbr->Tag);

[jeffreytan81]

I would rename cu_offset to unit_offset because it can either be CU or TU now.

[clayborg]

Good idea

[jeffreytan81]

I wonder why do we really need this check?
With this check, when there is no DW_IDX_compile_unit but DW_IDX_type_unit presents, getCUIndex will return nullopt. Per my understanding, this is split dwarf (per-CU index) + TU scenario which we should return 0 instead of nulptr, right?

[clayborg]

We only return 0 if this DebugName::Entry doesn't have a DW_IDX_type_unit and if there is no valid CU index from a DW_IDX_compile_unit . The code is correct.

[jeffreytan81]

Do we want to add a test with duplicate type names? Otherwise, LGTM.

[ayermolo]

What happens when Linker tombstones the tu local entry to -1, or will that be in a separate patch after LLD changes land?

[clayborg]

What happens when Linker tombstones the tu local entry to -1, or will that be in a separate patch after LLD changes land?

I can will do another PR for that once it lands.

[clayborg]

What happens when Linker tombstones the tu local entry to -1, or will that be in a separate patch after LLD changes land?

Actually this patch will work for tombstoned units already. I will make an inline comment and tag you with details.

[clayborg]

This will return a NULL cu if the *unit_offset is UINT32_MAX, so it will work with tombstoned units. Though we can make a patch to `DebugNames::Entry::getLocalTUOffset()' to return std::nullopt for tombstoned units as a better fix.