Skip to content

Conversation

@clayborg
Copy link
Collaborator

@clayborg clayborg commented Dec 2, 2024

The ManualDWARFIndex class can create a index cache if the LLDB index cache is enabled. This used to save the index to the same file, regardless of wether the cache was a full index (no .debug_names) or a partial index (have .debug_names, but not all .o files were had .debug_names). So we could end up saving an index cache that was partial, and then later load that partial index as if it were a full index if the user set the 'settings set plugin.symbol-file.dwarf.ignore-file-indexes true'. This would cause us to ignore the .debug_names section, and if the index cache was enabled, we could end up loading the partial index as if it were a full DWARF index.

This patch detects when the ManualDWARFIndex is being used with .debug_names, and saves out a cache file with a suffix of "-full" or "-partial" to avoid this issue.

The ManualDWARFIndex class can create a index cache if the LLDB index cache is enabled. This used to save the index to the same file, regardless of wether the cache was a full index (no .debug_names) or a partial index (have .debug_names, but not all .o files were had .debug_names). So we could end up saving an index cache that was partial, and then later load that partial index as if it were a full index if the user set the 'settings set plugin.symbol-file.dwarf.ignore-file-indexes true'. This would cause us to ignore the .debug_names section, and if the index cache was enabled, we could end up loading the partial index as if it were a full DWARF index.

This patch detects when the ManualDWARFIndex is being used with .debug_names, and saves out a cache file with a suffix of "-full" or "-partial" to avoid this issue.
@llvmbot
Copy link
Member

llvmbot commented Dec 2, 2024

@llvm/pr-subscribers-lldb

Author: Greg Clayton (clayborg)

Changes

The ManualDWARFIndex class can create a index cache if the LLDB index cache is enabled. This used to save the index to the same file, regardless of wether the cache was a full index (no .debug_names) or a partial index (have .debug_names, but not all .o files were had .debug_names). So we could end up saving an index cache that was partial, and then later load that partial index as if it were a full index if the user set the 'settings set plugin.symbol-file.dwarf.ignore-file-indexes true'. This would cause us to ignore the .debug_names section, and if the index cache was enabled, we could end up loading the partial index as if it were a full DWARF index.

This patch detects when the ManualDWARFIndex is being used with .debug_names, and saves out a cache file with a suffix of "-full" or "-partial" to avoid this issue.


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

3 Files Affected:

  • (modified) lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp (+23-1)
  • (modified) lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h (+10)
  • (added) lldb/test/Shell/SymbolFile/DWARF/x86/dwp-index-cache.cpp (+62)
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
index 1220e6115a2a95..0be19ab29ef082 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.cpp
@@ -706,6 +706,11 @@ bool ManualDWARFIndex::Encode(DataEncoder &encoder) const {
   return true;
 }
 
+bool ManualDWARFIndex::IsPartial() const {
+  // If we have units or type units to skip, then this index is partial.
+  return !m_units_to_avoid.empty() || !m_type_sigs_to_avoid.empty();
+}
+
 std::string ManualDWARFIndex::GetCacheKey() {
   std::string key;
   llvm::raw_string_ostream strm(key);
@@ -713,9 +718,26 @@ std::string ManualDWARFIndex::GetCacheKey() {
   // module can have one object file as the main executable and might have
   // another object file in a separate symbol file, or we might have a .dwo file
   // that claims its module is the main executable.
+
+  // This class can be used to index all of the DWARF, or part of the DWARF
+  // when there is a .debug_names index where some compile or type units were
+  // built without .debug_names. So we need to know when we have a full manual
+  // DWARF index or a partial manual DWARF index and save them to different
+  // cache files. Before this fix we might end up debugging a binary with
+  // .debug_names where some of the compile or type units weren't indexed, and
+  // find an issue with the .debug_names tables (bugs or being incomplete), and
+  // then we disable loading the .debug_names by setting a setting in LLDB by
+  // running "settings set plugin.symbol-file.dwarf.ignore-file-indexes 0" in
+  // another LLDB instance. The problem arose when there was an index cache from
+  // a previous run where .debug_names was enabled and it had saved a cache file
+  // that only covered the missing compile and type units from the .debug_names,
+  // and with the setting that disables the loading of the cache files we would
+  // load partial cache index cache. So we need to pick a unique cache suffix
+  // name that indicates if the cache is partial or full to avoid this problem.
+  llvm::StringRef dwarf_index_suffix(IsPartial() ? "partial-" : "full-");
   ObjectFile *objfile = m_dwarf->GetObjectFile();
   strm << objfile->GetModule()->GetCacheKey() << "-dwarf-index-"
-      << llvm::format_hex(objfile->GetCacheHash(), 10);
+       << dwarf_index_suffix << llvm::format_hex(objfile->GetCacheHash(), 10);
   return key;
 }
 
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
index d8c4a22ab21f7b..6a52c88a99220f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
+++ b/lldb/source/Plugins/SymbolFile/DWARF/ManualDWARFIndex.h
@@ -168,6 +168,16 @@ class ManualDWARFIndex : public DWARFIndex {
                             const lldb::LanguageType cu_language,
                             IndexSet &set);
 
+  /// Return true if this manual DWARF index is covering only part of the DWARF.
+  ///
+  /// An instance of this class will be used to index all of the DWARF, but also
+  /// when we have .debug_names we will use one to index any compile or type
+  /// units that are not covered by the .debug_names table.
+  ///
+  /// \return
+  ///   True if this index is a partial index, false otherwise.
+  bool IsPartial() const;
+
   /// The DWARF file which we are indexing.
   SymbolFileDWARF *m_dwarf;
   /// Which dwarf units should we skip while building the index.
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-index-cache.cpp b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-index-cache.cpp
new file mode 100644
index 00000000000000..3e97c3fb1ebc24
--- /dev/null
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-index-cache.cpp
@@ -0,0 +1,62 @@
+// REQUIRES: lld
+
+// Test if we build a mixed binary where one .o file has a .debug_names and
+// another doesn't have one, that we save a full or partial index cache.
+// Previous versions of LLDB would have ManualDWARFIndex.cpp that would save out
+// an index cache to the same file regardless of wether the index cache was a
+// full DWARF manual index, or just the CUs and TUs that were missing from any
+// .debug_names tables. If the user had a .debug_names table and debugged once
+// with index caching enabled, then debugged again but set the setting to ignore
+// .debug_names ('settings set plugin.symbol-file.dwarf.ignore-file-indexes 1')
+// this could cause LLDB to load the index cache from the previous run which
+// was incomplete and it only contained the manually indexed DWARF from the run
+// where we used .debug_names, but it would now load it as if it were the
+// complete DWARF index.
+
+// Test that if we don't have .debug_names, that we save a full DWARF index.
+// RUN: %clang -target x86_64-pc-linux -gsplit-dwarf -gdwarf-5 -DMAIN=1 -c %s -o %t.main.o
+// RUN: %clang -target x86_64-pc-linux -gsplit-dwarf -gdwarf-5 -DMAIN=0 -c %s -o %t.foo.o
+// RUN: ld.lld %t.main.o %t.foo.o -o %t.nonames
+// RUN: llvm-dwp %t.main.dwo %t.foo.dwo -o %t.nonames.dwp
+// RUN: rm %t.main.dwo %t.foo.dwo
+// Run one time with the index cache enabled to populate the index cache. When
+// we populate the index cache we have to parse all of the DWARF debug info
+// and it is always available.
+// RUN: rm -rf %t.lldb-index-cache
+// RUN: %lldb \
+// RUN:   -O 'settings set symbols.enable-lldb-index-cache true' \
+// RUN:   -O 'settings set symbols.lldb-index-cache-path %t.lldb-index-cache' \
+// RUN:   -O 'settings set target.preload-symbols true' \
+// RUN:   %t.nonames -b
+
+// Make sure there is a file with "dwarf-index-full" in its filename
+// RUN: ls %t.lldb-index-cache | FileCheck %s -check-prefix=FULL
+// FULL: {{dwp-index-cache.cpp.tmp.nonames.*-dwarf-index-full-}}
+
+// Test that if we have one .o file with .debug_names and one without, that we
+// save a partial DWARF index.
+// RUN: %clang -target x86_64-pc-linux -gsplit-dwarf -gdwarf-5 -DMAIN=1 -c %s -o %t.main.o -gpubnames
+// RUN: %clang -target x86_64-pc-linux -gsplit-dwarf -gdwarf-5 -DMAIN=0 -c %s -o %t.foo.o
+// RUN: ld.lld %t.main.o %t.foo.o -o %t.somenames
+// RUN: llvm-dwp %t.main.dwo %t.foo.dwo -o %t.somenames.dwp
+// RUN: rm %t.main.dwo %t.foo.dwo
+// Run one time with the index cache enabled to populate the index cache. When
+// we populate the index cache we have to parse all of the DWARF debug info
+// and it is always available.
+// RUN: rm -rf %t.lldb-index-cache
+// RUN: %lldb \
+// RUN:   -O 'settings set symbols.enable-lldb-index-cache true' \
+// RUN:   -O 'settings set symbols.lldb-index-cache-path %t.lldb-index-cache' \
+// RUN:   -O 'settings set target.preload-symbols true' \
+// RUN:   %t.somenames -b
+
+// Make sure there is a file with "dwarf-index-full" in its filename
+// RUN: ls %t.lldb-index-cache | FileCheck %s -check-prefix=PARTIAL
+// PARTIAL: {{dwp-index-cache.cpp.tmp.somenames.*-dwarf-index-partial-}}
+
+#if MAIN
+extern int foo();
+int main() { return foo(); }
+#else
+int foo() { return 0; }
+#endif

@clayborg clayborg changed the title Fix the DWARF index cache when index is partial. [lldb] Fix the DWARF index cache when index is partial. Dec 2, 2024
Comment on lines +20 to +21
// RUN: llvm-dwp %t.main.dwo %t.foo.dwo -o %t.nonames.dwp
// RUN: rm %t.main.dwo %t.foo.dwo
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these steps necessary. We should be able to build/save the index from dwo files alone, right?

Comment on lines +41 to +42
// RUN: llvm-dwp %t.main.dwo %t.foo.dwo -o %t.somenames.dwp
// RUN: rm %t.main.dwo %t.foo.dwo
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

@clayborg clayborg merged commit f44bee7 into llvm:main Dec 3, 2024
9 checks passed
@clayborg clayborg deleted the manual-index-partial branch December 3, 2024 18:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants