[Lldb-commits] [PATCH] D107659: [nfc] [lldb] Assertions for D106270 - [DWARF5] Fix offset check when using .debug_names

Jan Kratochvil via Phabricator via lldb-commits Fri, 06 Aug 2021 12:03:22 -0700

jankratochvil updated this revision to Diff 364861.
jankratochvil added a comment.


In D107659#2931836 <https://reviews.llvm.org/D107659#2931836>, @clayborg wrote:

> Can we not just grab the skeleton unit when/if needed instead of asserting in 
> many places?

I agree it could be unified for API simplicity to be always the skeleton unit, 
thanks for the idea. Done in this patch. Still I believe it should be asserted 
as unfortunately with the current Unit types it cannot be compile-time checked.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D107659/new/

https://reviews.llvm.org/D107659

Files:
  lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
  lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
  lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
  lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.h
  lldb/test/Shell/SymbolFile/DWARF/x86/Inputs/find-variable-file-3.cpp
  lldb/test/Shell/SymbolFile/DWARF/x86/find-variable-file.cpp

Index: lldb/test/Shell/SymbolFile/DWARF/x86/find-variable-file.cpp
===================================================================
--- lldb/test/Shell/SymbolFile/DWARF/x86/find-variable-file.cpp
+++ lldb/test/Shell/SymbolFile/DWARF/x86/find-variable-file.cpp
@@ -31,12 +31,15 @@
 // the compile unit using the name index if it is split.
 // RUN: %clang -c -o %t-1.o --target=x86_64-pc-linux -gdwarf-5 -gsplit-dwarf -gpubnames %s
 // RUN: %clang -c -o %t-2.o --target=x86_64-pc-linux -gdwarf-5 -gsplit-dwarf -gpubnames %S/Inputs/find-variable-file-2.cpp
-// RUN: ld.lld %t-1.o %t-2.o -o %t
+// RUN: %clang -c -o %t-3.o --target=x86_64-pc-linux -gdwarf-5 -gsplit-dwarf -gpubnames %S/Inputs/find-variable-file-3.cpp
+// RUN: ld.lld %t-1.o %t-2.o %t-3.o -o %t
 // RUN: llvm-readobj --sections %t | FileCheck %s --check-prefix NAMES
 // RUN: lldb-test symbols --file=find-variable-file.cpp --find=variable %t | \
 // RUN:   FileCheck --check-prefix=ONE %s
 // RUN: lldb-test symbols --file=find-variable-file-2.cpp --find=variable %t | \
 // RUN:   FileCheck --check-prefix=TWO %s
+// RUN: lldb-test symbols --file=find-variable-file-3.cpp --find=variable \
+// RUN:   --name=notexists %t
 
 // NAMES: Name: .debug_names
 
Index: lldb/test/Shell/SymbolFile/DWARF/x86/Inputs/find-variable-file-3.cpp
===================================================================
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/x86/Inputs/find-variable-file-3.cpp
@@ -0,0 +1,2 @@
+// No variable should exist in this file.
+void f() {}
Index: lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.h
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.h
+++ lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.h
@@ -38,8 +38,9 @@
   bool Find(const lldb_private::RegularExpression &regex,
             llvm::function_ref<bool(DIERef ref)> callback) const;
 
+  /// \a unit must be the skeleton unit if possible, not GetNonSkeletonUnit().
   void
-  FindAllEntriesForUnit(const DWARFUnit &unit,
+  FindAllEntriesForUnit(DWARFUnit &unit,
                         llvm::function_ref<bool(DIERef ref)> callback) const;
 
   void
Index: lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/NameToDIE.cpp
@@ -45,15 +45,16 @@
 }
 
 void NameToDIE::FindAllEntriesForUnit(
-    const DWARFUnit &unit,
-    llvm::function_ref<bool(DIERef ref)> callback) const {
+    DWARFUnit &s_unit, llvm::function_ref<bool(DIERef ref)> callback) const {
+  lldbassert(!s_unit.GetSymbolFileDWARF().GetDwoNum());
+  const DWARFUnit &ns_unit = s_unit.GetNonSkeletonUnit();
   const uint32_t size = m_map.GetSize();
   for (uint32_t i = 0; i < size; ++i) {
     const DIERef &die_ref = m_map.GetValueAtIndexUnchecked(i);
-    if (unit.GetSymbolFileDWARF().GetDwoNum() == die_ref.dwo_num() &&
-        unit.GetDebugSection() == die_ref.section() &&
-        unit.GetOffset() <= die_ref.die_offset() &&
-        die_ref.die_offset() < unit.GetNextUnitOffset()) {
+    if (ns_unit.GetSymbolFileDWARF().GetDwoNum() == die_ref.dwo_num() &&
+        ns_unit.GetDebugSection() == die_ref.section() &&
+        ns_unit.GetOffset() <= die_ref.die_offset() &&
+        die_ref.die_offset() < ns_unit.GetNextUnitOffset()) {
       if (!callback(die_ref))
         return;
     }
Index: lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -359,9 +359,9 @@
 
 void ManualDWARFIndex::GetGlobalVariables(
     DWARFUnit &unit, llvm::function_ref<bool(DWARFDIE die)> callback) {
+  lldbassert(!unit.GetSymbolFileDWARF().GetDwoNum());
   Index();
-  m_set.globals.FindAllEntriesForUnit(unit.GetNonSkeletonUnit(),
-                                      DIERefCallback(callback));
+  m_set.globals.FindAllEntriesForUnit(unit, DIERefCallback(callback));
 }
 
 void ManualDWARFIndex::GetObjCMethods(
Index: lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/DebugNamesDWARFIndex.cpp
@@ -124,6 +124,7 @@
 
 void DebugNamesDWARFIndex::GetGlobalVariables(
     DWARFUnit &cu, llvm::function_ref<bool(DWARFDIE die)> callback) {
+  lldbassert(!cu.GetSymbolFileDWARF().GetDwoNum());
   uint64_t cu_offset = cu.GetOffset();
   bool found_entry_for_cu = false;
   for (const DebugNames::NameIndex &ni: *m_debug_names_up) {
Index: lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
+++ lldb/source/Plugins/SymbolFile/DWARF/DWARFIndex.h
@@ -34,6 +34,7 @@
   virtual void
   GetGlobalVariables(const RegularExpression &regex,
                      llvm::function_ref<bool(DWARFDIE die)> callback) = 0;
+  /// \a cu must be the skeleton unit if possible, not GetNonSkeletonUnit().
   virtual void
   GetGlobalVariables(DWARFUnit &cu,
                      llvm::function_ref<bool(DWARFDIE die)> callback) = 0;
Index: lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
===================================================================
--- lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
+++ lldb/source/Plugins/SymbolFile/DWARF/AppleDWARFIndex.cpp
@@ -79,6 +79,7 @@
   if (!m_apple_names_up)
     return;
 
+  lldbassert(!cu.GetSymbolFileDWARF().GetDwoNum());
   const DWARFUnit &non_skeleton_cu = cu.GetNonSkeletonUnit();
   DWARFMappedHash::DIEInfoArray hash_data;
   m_apple_names_up->AppendAllDIEsInRange(non_skeleton_cu.GetOffset(),

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

[Lldb-commits] [PATCH] D107659: [nfc] [lldb] Assertions for D106270 - [DWARF5] Fix offset check when using .debug_names

Reply via email to