[Lldb-commits] [lldb] [lldb] Remove DWARFDebugInfo DIERef footguns (PR #92894)

Pavel Labath via lldb-commits lldb-commits at lists.llvm.org
Tue May 21 04:03:16 PDT 2024


https://github.com/labath created https://github.com/llvm/llvm-project/pull/92894

DWARFDebugInfo doesn't know how to resolve the "file_index" component of a DIERef. This patch removes GetUnit (in favor of existing GetUnitContainingDIEOffset) and changes GetDIE to take only the components it actually uses.

In the process it fixes GetCompleteObjCClass, although that bug was unlikely to be noticed, since ObjC is only really used on darwin, and split DWARF isn't a thing there.

>From e416051b09147b1083765795f711bb972e42a893 Mon Sep 17 00:00:00 2001
From: Pavel Labath <pavel at labath.sk>
Date: Mon, 20 May 2024 14:51:52 +0000
Subject: [PATCH] [lldb] Remove DWARFDebugInfo DIERef footguns

DWARFDebugInfo doesn't know how to resolve the "file_index" component of
a DIERef. This patch removes GetUnit (in favor of existing
GetUnitContainingDIEOffset) and changes GetDIE to take only the
components it actually uses.

In the process it fixes GetCompleteObjCClass, although that bug was
unlikely to be noticed, since ObjC is only really used on darwin, and
split DWARF isn't a thing there.
---
 .../Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp    | 11 +++--------
 .../Plugins/SymbolFile/DWARF/DWARFDebugInfo.h      |  3 +--
 .../SymbolFile/DWARF/DebugNamesDWARFIndex.cpp      | 14 +++++++-------
 .../Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp   |  7 ++++---
 .../SymbolFile/DWARF/SymbolFileDWARFDwo.cpp        |  2 +-
 5 files changed, 16 insertions(+), 21 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
index d28da728728e5..c37cc91e08ed1 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.cpp
@@ -222,10 +222,6 @@ DWARFUnit *DWARFDebugInfo::GetUnitAtOffset(DIERef::Section section,
   return result;
 }
 
-DWARFUnit *DWARFDebugInfo::GetUnit(const DIERef &die_ref) {
-  return GetUnitContainingDIEOffset(die_ref.section(), die_ref.die_offset());
-}
-
 DWARFUnit *
 DWARFDebugInfo::GetUnitContainingDIEOffset(DIERef::Section section,
                                            dw_offset_t die_offset) {
@@ -253,9 +249,8 @@ bool DWARFDebugInfo::ContainsTypeUnits() {
 //
 // Get the DIE (Debug Information Entry) with the specified offset.
 DWARFDIE
-DWARFDebugInfo::GetDIE(const DIERef &die_ref) {
-  DWARFUnit *cu = GetUnit(die_ref);
-  if (cu)
-    return cu->GetNonSkeletonUnit().GetDIE(die_ref.die_offset());
+DWARFDebugInfo::GetDIE(DIERef::Section section, dw_offset_t die_offset) {
+  if (DWARFUnit *cu = GetUnitContainingDIEOffset(section, die_offset))
+    return cu->GetNonSkeletonUnit().GetDIE(die_offset);
   return DWARFDIE(); // Not found
 }
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
index 456ebd908ccb2..4706b55d38ea9 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugInfo.h
@@ -38,11 +38,10 @@ class DWARFDebugInfo {
                              uint32_t *idx_ptr = nullptr);
   DWARFUnit *GetUnitContainingDIEOffset(DIERef::Section section,
                                         dw_offset_t die_offset);
-  DWARFUnit *GetUnit(const DIERef &die_ref);
   DWARFUnit *GetSkeletonUnit(DWARFUnit *dwo_unit);
   DWARFTypeUnit *GetTypeUnitForHash(uint64_t hash);
   bool ContainsTypeUnits();
-  DWARFDIE GetDIE(const DIERef &die_ref);
+  DWARFDIE GetDIE(DIERef::Section section, dw_offset_t die_offset);
 
   enum {
     eDumpFlag_Verbose = (1 << 0),  // Verbose dumping
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
index 79400e36e04f3..31bbad3e56269 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -195,17 +195,17 @@ void DebugNamesDWARFIndex::GetCompleteObjCClass(
     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);
-      continue;
-    }
-
-    DWARFDIE die = m_debug_info.GetDIE(*ref);
+    SymbolFileDWARF &dwarf = *llvm::cast<SymbolFileDWARF>(
+        m_module.GetSymbolFile()->GetBackingSymbolFile());
+    DWARFDIE die = dwarf.GetDIE(*ref);
     if (!die) {
       ReportInvalidDIERef(*ref, class_name.GetStringRef());
       continue;
     }
+    if (!die.GetCU()->Supports_DW_AT_APPLE_objc_complete_type()) {
+      incomplete_types.push_back(*ref);
+      continue;
+    }
 
     if (die.GetAttributeValueAsUnsigned(DW_AT_APPLE_objc_complete_type, 0)) {
       // If we find the complete version we're done.
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index f6f152726bf74..2bca27936c76b 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -1748,7 +1748,8 @@ SymbolFileDWARF::GetDIE(const DIERef &die_ref) {
     if (SymbolFileDWARFDebugMap *debug_map = GetDebugMapSymfile()) {
       symbol_file = debug_map->GetSymbolFileByOSOIndex(*file_index); // OSO case
       if (symbol_file)
-        return symbol_file->DebugInfo().GetDIE(die_ref);
+        return symbol_file->DebugInfo().GetDIE(die_ref.section(),
+                                               die_ref.die_offset());
       return DWARFDIE();
     }
 
@@ -1765,7 +1766,7 @@ SymbolFileDWARF::GetDIE(const DIERef &die_ref) {
   if (symbol_file)
     return symbol_file->GetDIE(die_ref);
 
-  return DebugInfo().GetDIE(die_ref);
+  return DebugInfo().GetDIE(die_ref.section(), die_ref.die_offset());
 }
 
 /// Return the DW_AT_(GNU_)dwo_id.
@@ -3773,7 +3774,7 @@ SymbolFileDWARF::FindBlockContainingSpecification(
   // Give the concrete function die specified by "func_die_offset", find the
   // concrete block whose DW_AT_specification or DW_AT_abstract_origin points
   // to "spec_block_die_offset"
-  return FindBlockContainingSpecification(DebugInfo().GetDIE(func_die_ref),
+  return FindBlockContainingSpecification(GetDIE(func_die_ref),
                                           spec_block_die_offset);
 }
 
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
index 85e1afd0d8976..71c9997e4c82c 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDwo.cpp
@@ -145,7 +145,7 @@ SymbolFileDWARFDwo::GetTypeSystemForLanguage(LanguageType language) {
 DWARFDIE
 SymbolFileDWARFDwo::GetDIE(const DIERef &die_ref) {
   if (die_ref.file_index() == GetFileIndex())
-    return DebugInfo().GetDIE(die_ref);
+    return DebugInfo().GetDIE(die_ref.section(), die_ref.die_offset());
   return GetBaseSymbolFile().GetDIE(die_ref);
 }
 



More information about the lldb-commits mailing list