https://github.com/labath created 
https://github.com/llvm/llvm-project/pull/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 examening 
the DWARFDIE right after finding the entry, and the DIERef just gets in the way.

>From 2621a2c474ae18bd71b897919e75f23f9526f72b Mon Sep 17 00:00:00 2001
From: Pavel Labath <pa...@labath.sk>
Date: Fri, 24 May 2024 08:14:27 +0000
Subject: [PATCH] [lldb/DWARF] Bypass the compres^Wconstruction of DIERefs in
 debug_names

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 examening the DWARFDIE right after finding the entry,
and the DIERef just gets in the way.
---
 .../SymbolFile/DWARF/AppleDWARFIndex.cpp      |  8 ++-
 .../Plugins/SymbolFile/DWARF/DWARFIndex.cpp   |  7 +--
 .../Plugins/SymbolFile/DWARF/DWARFIndex.h     |  9 ++-
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.cpp | 58 ++++++++-----------
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.h   |  2 +-
 5 files changed, 37 insertions(+), 47 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
index 33537df4f5076..1703597a7cd2f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
+++ b/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;
   }
 }
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
index 20c07a94b5076..30fb5d5ebdb0d 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.cpp
+++ b/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;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
index 0551b07100a96..cb3ae8a06d788 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
+++ b/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);
 
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp 
b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index 79400e36e04f3..90e42be7202d8 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/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;
   return callback(die);
@@ -183,7 +181,7 @@ 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())) {
@@ -191,19 +189,14 @@ void DebugNamesDWARFIndex::GetCompleteObjCClass(
         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;
     }
 
@@ -212,12 +205,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);
@@ -379,8 +371,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;
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h 
b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
index 81fb8f88b805a..a27a414ecdd19 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.h
+++ b/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);
 

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to