[lldb/DWARF] Bypass the compres^Wconstruction of DIERefs in debug_names (#93296)

DebugNamesDWARFIndex was jumping through hoops to construct a DIERef
from an index entry only to jump through them back a short while later
to construct a DWARFDIE.

This used to be necessary as the index lookup was a two stage process,
where we first enumerated all matches, and then examined them (so it was
important that the enumeration was cheap -- does not trigger unnecessary
parsing). However, now that the processing is callback based, we are
always immediately examining the DWARFDIE right after finding the entry,
and the DIERef just gets in the way.

Author: labath

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

@@ -284,8 +284,12 @@ void AppleDWARFIndex::GetFunctions(
   for (const auto &entry : m_apple_names_up->equal_range(name)) {
     DIERef die_ref(std::nullopt, DIERef::Section::DebugInfo,
                    *entry.getDIESectionOffset());
-    if (!ProcessFunctionDIE(lookup_info, die_ref, dwarf, parent_decl_ctx,
-                            callback))
+    DWARFDIE die = dwarf.GetDIE(die_ref);
+    if (!die) {
+      ReportInvalidDIERef(die_ref, name);
+      continue;
+    }
+    if (!ProcessFunctionDIE(lookup_info, die, parent_decl_ctx, callback))
       return;
   }
 }

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

@@ -24,16 +24,11 @@ using namespace lldb_private::plugin::dwarf;
 DWARFIndex::~DWARFIndex() = default;
 
 bool DWARFIndex::ProcessFunctionDIE(
-    const Module::LookupInfo &lookup_info, DIERef ref, SymbolFileDWARF &dwarf,
+    const Module::LookupInfo &lookup_info, DWARFDIE die,
     const CompilerDeclContext &parent_decl_ctx,
     llvm::function_ref<bool(DWARFDIE die)> callback) {
   llvm::StringRef name = lookup_info.GetLookupName().GetStringRef();
   FunctionNameType name_type_mask = lookup_info.GetNameTypeMask();
-  DWARFDIE die = dwarf.GetDIE(ref);
-  if (!die) {
-    ReportInvalidDIERef(ref, name);
-    return true;
-  }
 
   if (!(name_type_mask & eFunctionNameTypeFull)) {
     ConstString name_to_match_against;

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

@@ -81,11 +81,10 @@ class DWARFIndex {
   StatsDuration m_index_time;
 
   /// Helper function implementing common logic for processing function dies. If
-  /// the function given by "ref" matches search criteria given by
-  /// "parent_decl_ctx" and "name_type_mask", it is inserted into the "dies"
-  /// vector.
-  bool ProcessFunctionDIE(const Module::LookupInfo &lookup_info, DIERef ref,
-                          SymbolFileDWARF &dwarf,
+  /// the function given by "die" matches search criteria given by
+  /// "parent_decl_ctx" and "name_type_mask", it calls the callback with the
+  /// given die.
+  bool ProcessFunctionDIE(const Module::LookupInfo &lookup_info, DWARFDIE die,
                           const CompilerDeclContext &parent_decl_ctx,
                           llvm::function_ref<bool(DWARFDIE die)> callback);
 

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

@@ -64,27 +64,25 @@ DebugNamesDWARFIndex::GetNonSkeletonUnit(const DebugNames::Entry &entry) const {
   return cu ? &cu->GetNonSkeletonUnit() : nullptr;
 }
 
-std::optional<DIERef>
-DebugNamesDWARFIndex::ToDIERef(const DebugNames::Entry &entry) const {
+DWARFDIE DebugNamesDWARFIndex::GetDIE(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(unit->GetSymbolFileDWARF().GetFileIndex(),
-                  DIERef::Section::DebugInfo, unit->GetOffset() + *die_offset);
-
-  return std::nullopt;
+  std::optional<uint64_t> die_offset = entry.getDIEUnitOffset();
+  if (!unit || !die_offset)
+    return DWARFDIE();
+  if (DWARFDIE die = unit->GetDIE(unit->GetOffset() + *die_offset))
+    return die;
+
+  m_module.ReportErrorIfModifyDetected(
+      "the DWARF debug information has been modified (bad offset {0:x} in "
+      "debug_names section)\n",
+      *die_offset);
+  return DWARFDIE();
 }
 
 bool DebugNamesDWARFIndex::ProcessEntry(
     const DebugNames::Entry &entry,
     llvm::function_ref<bool(DWARFDIE die)> callback) {
-  std::optional<DIERef> ref = ToDIERef(entry);
-  if (!ref)
-    return true;
-  SymbolFileDWARF &dwarf = *llvm::cast<SymbolFileDWARF>(
-      m_module.GetSymbolFile()->GetBackingSymbolFile());
-  DWARFDIE die = dwarf.GetDIE(*ref);
+  DWARFDIE die = GetDIE(entry);
   if (!die)
     return true;
   // Clang erroneously emits index entries for declaration DIEs in case when the
@@ -187,27 +185,22 @@ void DebugNamesDWARFIndex::GetCompleteObjCClass(
     llvm::function_ref<bool(DWARFDIE die)> callback) {
   // Keep a list of incomplete types as fallback for when we don't find the
   // complete type.
-  DIEArray incomplete_types;
+  std::vector<DWARFDIE> incomplete_types;
 
   for (const DebugNames::Entry &entry :
        m_debug_names_up->equal_range(class_name.GetStringRef())) {
     if (entry.tag() != DW_TAG_structure_type &&
         entry.tag() != DW_TAG_class_type)
       continue;
 
-    std::optional<DIERef> ref = ToDIERef(entry);
-    if (!ref)
-      continue;
-
-    DWARFUnit *cu = m_debug_info.GetUnit(*ref);
-    if (!cu || !cu->Supports_DW_AT_APPLE_objc_complete_type()) {
-      incomplete_types.push_back(*ref);
+    DWARFDIE die = GetDIE(entry);
+    if (!die) {
+      // Report invalid
       continue;
     }
-
-    DWARFDIE die = m_debug_info.GetDIE(*ref);
-    if (!die) {
-      ReportInvalidDIERef(*ref, class_name.GetStringRef());
+    DWARFUnit *cu = die.GetCU();
+    if (!cu->Supports_DW_AT_APPLE_objc_complete_type()) {
+      incomplete_types.push_back(die);
       continue;
     }
 
@@ -216,12 +209,11 @@ void DebugNamesDWARFIndex::GetCompleteObjCClass(
       callback(die);
       return;
     }
-    incomplete_types.push_back(*ref);
+    incomplete_types.push_back(die);
   }
 
-  auto dierefcallback = DIERefCallback(callback, class_name.GetStringRef());
-  for (DIERef ref : incomplete_types)
-    if (!dierefcallback(ref))
+  for (DWARFDIE die : incomplete_types)
+    if (!callback(die))
       return;
 
   m_fallback.GetCompleteObjCClass(class_name, must_be_implementation, callback);
@@ -383,8 +375,8 @@ void DebugNamesDWARFIndex::GetFunctions(
     if (tag != DW_TAG_subprogram && tag != DW_TAG_inlined_subroutine)
       continue;
 
-    if (std::optional<DIERef> ref = ToDIERef(entry)) {
-      if (!ProcessFunctionDIE(lookup_info, *ref, dwarf, parent_decl_ctx,
+    if (DWARFDIE die = GetDIE(entry)) {
+      if (!ProcessFunctionDIE(lookup_info, die, parent_decl_ctx,
                               [&](DWARFDIE die) {
                                 if (!seen.insert(die.GetDIE()).second)
                                   return true;

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

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