[Lldb-commits] [lldb] Add more ways to find the .dwp file. (PR #81067)

Greg Clayton via lldb-commits lldb-commits at lists.llvm.org
Thu Feb 8 22:32:56 PST 2024


https://github.com/clayborg updated https://github.com/llvm/llvm-project/pull/81067

>From 3c2f6039cf0e253d78b5193098b311028daaea72 Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayborg at gmail.com>
Date: Wed, 7 Feb 2024 16:43:50 -0800
Subject: [PATCH 1/3] Add more ways to find the .dwp file.

When using split DWARF we can run into many different ways to store debug info:
- lldb loads "<exe>" which contains skeleton DWARF and needs to find "<exe>.dwp"
- lldb loads "<exe>" which is stripped but has .gnu_debuglink pointing to "<exe>.debug" with skeleton DWARF and needs to find "<exe>.dwp"
- lldb loads "<exe>" which is stripped but has .gnu_debuglink pointing to "<exe>.debug" with skeleton DWARF and needs to find "<exe>.debug.dwp"
- lldb loads "<exe>.debug" and needs to find "<exe>.dwp

Previously we only handled the first two cases. This patch adds support for the latter two.
---
 lldb/include/lldb/Utility/FileSpecList.h      |  4 ++
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp      | 61 +++++++++++++------
 .../DWARF/x86/dwp-separate-debug-file.cpp     | 30 +++++++++
 3 files changed, 78 insertions(+), 17 deletions(-)

diff --git a/lldb/include/lldb/Utility/FileSpecList.h b/lldb/include/lldb/Utility/FileSpecList.h
index 49edc667ddd5b6..6eb3bb9971f13a 100644
--- a/lldb/include/lldb/Utility/FileSpecList.h
+++ b/lldb/include/lldb/Utility/FileSpecList.h
@@ -238,6 +238,10 @@ class FileSpecList {
   const_iterator begin() const { return m_files.begin(); }
   const_iterator end() const { return m_files.end(); }
 
+  llvm::iterator_range<const_iterator> files() const {
+    return llvm::make_range(begin(), end());
+  }
+
 protected:
   collection m_files; ///< A collection of FileSpec objects.
 };
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 781f5c5a436778..487961fa7437fb 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -4349,26 +4349,53 @@ SymbolFileDWARFDebugMap *SymbolFileDWARF::GetDebugMapSymfile() {
 
 const std::shared_ptr<SymbolFileDWARFDwo> &SymbolFileDWARF::GetDwpSymbolFile() {
   llvm::call_once(m_dwp_symfile_once_flag, [this]() {
+    // Create a list of files to try and append .dwp to
+    FileSpecList symfiles;
+    // Append the module's object file path.
+    const FileSpec module_fspec = m_objfile_sp->GetModule()->GetFileSpec();
+    symfiles.Append(module_fspec);
+    // Append the object file for this SymbolFile only if it is different from
+    // the module's file path. Our main module could be "a.out", our symbol file
+    // could be "a.debug" and our ".dwp" file might be "a.debug.dwp" instead of
+    // "a.out.dwp".
+    const FileSpec symfile_fspec(m_objfile_sp->GetFileSpec());
+    if (symfile_fspec != module_fspec) {
+      symfiles.Append(symfile_fspec);
+    } else {
+      // If we don't have a separate debug info file, then try stripping the
+      // extension. We main module could be "a.debug" and the .dwp file could be
+      // "a.dwp" instead of "a.debug.dwp".
+      ConstString filename_no_ext =
+          module_fspec.GetFileNameStrippingExtension();
+      if (filename_no_ext != module_fspec.GetFilename()) {
+        FileSpec module_spec_no_ext(module_fspec);
+        module_spec_no_ext.SetFilename(filename_no_ext);
+        symfiles.Append(module_spec_no_ext);
+      }
+    }
+
+    FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
     ModuleSpec module_spec;
     module_spec.GetFileSpec() = m_objfile_sp->GetFileSpec();
-    module_spec.GetSymbolFileSpec() =
-        FileSpec(m_objfile_sp->GetModule()->GetFileSpec().GetPath() + ".dwp");
-
     module_spec.GetUUID() = m_objfile_sp->GetUUID();
-    FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths();
-    FileSpec dwp_filespec =
-        PluginManager::LocateExecutableSymbolFile(module_spec, search_paths);
-    if (FileSystem::Instance().Exists(dwp_filespec)) {
-      DataBufferSP dwp_file_data_sp;
-      lldb::offset_t dwp_file_data_offset = 0;
-      ObjectFileSP dwp_obj_file = ObjectFile::FindPlugin(
-          GetObjectFile()->GetModule(), &dwp_filespec, 0,
-          FileSystem::Instance().GetByteSize(dwp_filespec), dwp_file_data_sp,
-          dwp_file_data_offset);
-      if (!dwp_obj_file)
-        return;
-      m_dwp_symfile = std::make_shared<SymbolFileDWARFDwo>(
-          *this, dwp_obj_file, DIERef::k_file_index_mask);
+    for (const auto &symfile : symfiles.files()) {
+      module_spec.GetSymbolFileSpec() =
+          FileSpec(symfile.GetPath() + ".dwp", symfile.GetPathStyle());
+      FileSpec dwp_filespec =
+          PluginManager::LocateExecutableSymbolFile(module_spec, search_paths);
+      if (FileSystem::Instance().Exists(dwp_filespec)) {
+        DataBufferSP dwp_file_data_sp;
+        lldb::offset_t dwp_file_data_offset = 0;
+        ObjectFileSP dwp_obj_file = ObjectFile::FindPlugin(
+            GetObjectFile()->GetModule(), &dwp_filespec, 0,
+            FileSystem::Instance().GetByteSize(dwp_filespec), dwp_file_data_sp,
+            dwp_file_data_offset);
+        if (!dwp_obj_file)
+          return;
+        m_dwp_symfile = std::make_shared<SymbolFileDWARFDwo>(
+            *this, dwp_obj_file, DIERef::k_file_index_mask);
+        break;
+      }
     }
   });
   return m_dwp_symfile;
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-separate-debug-file.cpp b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-separate-debug-file.cpp
index a47209931c3840..408bdcb3fbd99c 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-separate-debug-file.cpp
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-separate-debug-file.cpp
@@ -1,5 +1,6 @@
 // REQUIRES: lld
 
+// Now test with DWARF5
 // RUN: %clang -target x86_64-pc-linux -gsplit-dwarf -gdwarf-5 -c %s -o %t.dwarf5.o
 // RUN: ld.lld %t.dwarf5.o -o %t.dwarf5
 // RUN: llvm-dwp %t.dwarf5.dwo -o %t.dwarf5.dwp
@@ -34,6 +35,19 @@
 // RUN:   -o "statistics dump" \
 // RUN:   %t.dwarf5 -b | FileCheck %s -check-prefix=CACHED
 
+// Make sure that if we load the "%t.dwarf5.debug" file, that we can find and
+// load the .dwo file from the .dwp when it is "%t.dwarf5.dwp"
+// RUN: %lldb %t.dwarf5.debug -o "b main" -b | FileCheck %s -check-prefix=DEBUG
+
+// Make sure that if we load the "%t.dwarf5" file, that we can find and
+// load the .dwo file from the .dwp when it is "%t.dwarf5.debug.dwp"
+// RUN: mv %t.dwarf5.dwp %t.dwarf5.debug.dwp
+// RUN: %lldb %t.dwarf5.debug -o "b main" -b | FileCheck %s -check-prefix=DEBUG
+
+// Make sure that if we load the "%t.dwarf5.debug" file, that we can find and
+// load the .dwo file from the .dwp when it is "%t.dwarf5.debug.dwp"
+// RUN: %lldb %t.dwarf5.debug -o "b main" -b | FileCheck %s -check-prefix=DEBUG
+
 // Now test with DWARF4
 // RUN: %clang -target x86_64-pc-linux -gsplit-dwarf -gdwarf-4 -c %s -o %t.dwarf4.o
 // RUN: ld.lld %t.dwarf4.o -o %t.dwarf4
@@ -69,6 +83,19 @@
 // RUN:   -o "statistics dump" \
 // RUN:   %t.dwarf4 -b | FileCheck %s -check-prefix=CACHED
 
+// Make sure that if we load the "%t.dwarf4.debug" file, that we can find and
+// load the .dwo file from the .dwp when it is "%t.dwarf4.dwp"
+// RUN: %lldb %t.dwarf4.debug -o "b main" -b | FileCheck %s -check-prefix=DEBUG
+
+// Make sure that if we load the "%t.dwarf4" file, that we can find and
+// load the .dwo file from the .dwp when it is "%t.dwarf4.debug.dwp"
+// RUN: mv %t.dwarf4.dwp %t.dwarf4.debug.dwp
+// RUN: %lldb %t.dwarf4.debug -o "b main" -b | FileCheck %s -check-prefix=DEBUG
+
+// Make sure that if we load the "%t.dwarf4.debug" file, that we can find and
+// load the .dwo file from the .dwp when it is "%t.dwarf4.debug.dwp"
+// RUN: %lldb %t.dwarf4.debug -o "b main" -b | FileCheck %s -check-prefix=DEBUG
+
 // CHECK: (A) a = (x = 47)
 
 // CACHE: script lldb.target.modules[0].FindTypes('::A').GetTypeAtIndex(0)
@@ -83,6 +110,9 @@
 // CACHED-NEXT: }
 // CACHED: "totalDebugInfoIndexLoadedFromCache": 1
 
+// Make sure debug information was loaded by verifying that the
+// DEBUG: Breakpoint 1: where = dwp-separate-debug-file.cpp.tmp.dwarf{{[45]}}.debug`main + {{[0-9]+}} at dwp-separate-debug-file.cpp:{{[0-9]+}}:{{[0-9]+}}, address = {{0x[0-9a-fA-F]+}}
+
 struct A {
   int x = 47;
 };

>From 9d58f41457fc2c9e54b1409c64f3028fdaededf1 Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayborg at gmail.com>
Date: Thu, 8 Feb 2024 22:29:23 -0800
Subject: [PATCH 2/3] Address review comments.

Fixed:
- added tests for missing .dwp files for both DWARF4 and DWARF5
- added a tests where we create a file with a GNU build ID where the stripped executable and debug file have UUIDs and the .dwp doesn't and make sure it loads the .dwp file.
- fix a dwarf4 and dwarf5 test that was testing the wrong binary
- fixed comments
- don't use break + return
---
 .../SymbolFile/DWARF/SymbolFileDWARF.cpp      | 12 +++----
 .../DWARF/x86/dwp-separate-debug-file.cpp     | 31 +++++++++++++++++--
 2 files changed, 34 insertions(+), 9 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index 487961fa7437fb..e48531e663e8c8 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -4349,7 +4349,7 @@ SymbolFileDWARFDebugMap *SymbolFileDWARF::GetDebugMapSymfile() {
 
 const std::shared_ptr<SymbolFileDWARFDwo> &SymbolFileDWARF::GetDwpSymbolFile() {
   llvm::call_once(m_dwp_symfile_once_flag, [this]() {
-    // Create a list of files to try and append .dwp to
+    // Create a list of files to try and append .dwp to.
     FileSpecList symfiles;
     // Append the module's object file path.
     const FileSpec module_fspec = m_objfile_sp->GetModule()->GetFileSpec();
@@ -4390,11 +4390,11 @@ const std::shared_ptr<SymbolFileDWARFDwo> &SymbolFileDWARF::GetDwpSymbolFile() {
             GetObjectFile()->GetModule(), &dwp_filespec, 0,
             FileSystem::Instance().GetByteSize(dwp_filespec), dwp_file_data_sp,
             dwp_file_data_offset);
-        if (!dwp_obj_file)
-          return;
-        m_dwp_symfile = std::make_shared<SymbolFileDWARFDwo>(
-            *this, dwp_obj_file, DIERef::k_file_index_mask);
-        break;
+        if (dwp_obj_file) {
+          m_dwp_symfile = std::make_shared<SymbolFileDWARFDwo>(
+              *this, dwp_obj_file, DIERef::k_file_index_mask);
+          break;
+        }
       }
     }
   });
diff --git a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-separate-debug-file.cpp b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-separate-debug-file.cpp
index 408bdcb3fbd99c..dbb8aed54ee8aa 100644
--- a/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-separate-debug-file.cpp
+++ b/lldb/test/Shell/SymbolFile/DWARF/x86/dwp-separate-debug-file.cpp
@@ -42,12 +42,17 @@
 // Make sure that if we load the "%t.dwarf5" file, that we can find and
 // load the .dwo file from the .dwp when it is "%t.dwarf5.debug.dwp"
 // RUN: mv %t.dwarf5.dwp %t.dwarf5.debug.dwp
-// RUN: %lldb %t.dwarf5.debug -o "b main" -b | FileCheck %s -check-prefix=DEBUG
+// RUN: %lldb %t.dwarf5 -o "b main" -b | FileCheck %s -check-prefix=DEBUG
 
 // Make sure that if we load the "%t.dwarf5.debug" file, that we can find and
 // load the .dwo file from the .dwp when it is "%t.dwarf5.debug.dwp"
 // RUN: %lldb %t.dwarf5.debug -o "b main" -b | FileCheck %s -check-prefix=DEBUG
 
+// Make sure that if we remove the .dwp file we see an appropriate error.
+// RUN: rm %t.dwarf5.debug.dwp
+// RUN: %lldb %t.dwarf5 -o "b main" -b 2>&1 | FileCheck %s -check-prefix=NODWP
+// RUN: %lldb %t.dwarf5.debug -o "b main" -b 2>&1 | FileCheck %s -check-prefix=NODWP
+
 // Now test with DWARF4
 // RUN: %clang -target x86_64-pc-linux -gsplit-dwarf -gdwarf-4 -c %s -o %t.dwarf4.o
 // RUN: ld.lld %t.dwarf4.o -o %t.dwarf4
@@ -90,12 +95,27 @@
 // Make sure that if we load the "%t.dwarf4" file, that we can find and
 // load the .dwo file from the .dwp when it is "%t.dwarf4.debug.dwp"
 // RUN: mv %t.dwarf4.dwp %t.dwarf4.debug.dwp
-// RUN: %lldb %t.dwarf4.debug -o "b main" -b | FileCheck %s -check-prefix=DEBUG
+// RUN: %lldb %t.dwarf4 -o "b main" -b | FileCheck %s -check-prefix=DEBUG
 
 // Make sure that if we load the "%t.dwarf4.debug" file, that we can find and
 // load the .dwo file from the .dwp when it is "%t.dwarf4.debug.dwp"
 // RUN: %lldb %t.dwarf4.debug -o "b main" -b | FileCheck %s -check-prefix=DEBUG
 
+// Make sure that if we remove the .dwp file we see an appropriate error.
+// RUN: rm %t.dwarf4.debug.dwp
+// RUN: %lldb %t.dwarf4 -o "b main" -b 2>&1 | FileCheck %s -check-prefix=NODWP
+// RUN: %lldb %t.dwarf4.debug -o "b main" -b 2>&1 | FileCheck %s -check-prefix=NODWP
+
+// Test if we have a GNU build ID in our main executable and in our debug file,
+// and we have a .dwp file that doesn't, that we can still load our .dwp file.
+// RUN: %clang -target x86_64-pc-linux -gsplit-dwarf -gdwarf-5 -c %s -o %t.o
+// RUN: ld.lld %t.o --build-id=md5 -o %t
+// RUN: llvm-dwp %t.dwo -o %t.dwp
+// RUN: rm %t.dwo
+// RUN: llvm-objcopy --only-keep-debug %t %t.debug
+// RUN: llvm-objcopy --strip-all --add-gnu-debuglink=%t.debug %t
+// RUN: %lldb %t -o "target variable a" -b | FileCheck %s
+
 // CHECK: (A) a = (x = 47)
 
 // CACHE: script lldb.target.modules[0].FindTypes('::A').GetTypeAtIndex(0)
@@ -111,7 +131,12 @@
 // CACHED: "totalDebugInfoIndexLoadedFromCache": 1
 
 // Make sure debug information was loaded by verifying that the
-// DEBUG: Breakpoint 1: where = dwp-separate-debug-file.cpp.tmp.dwarf{{[45]}}.debug`main + {{[0-9]+}} at dwp-separate-debug-file.cpp:{{[0-9]+}}:{{[0-9]+}}, address = {{0x[0-9a-fA-F]+}}
+// DEBUG: Breakpoint 1: where = dwp-separate-debug-file.cpp.tmp.dwarf{{[45]}}{{(\.debug)?}}`main + {{[0-9]+}} at dwp-separate-debug-file.cpp:{{[0-9]+}}:{{[0-9]+}}, address = {{0x[0-9a-fA-F]+}}
+
+// Make sure if we load the stripped binary or the debug info file with no .dwp
+// nor any .dwo files that we are not able to fine the .dwp or .dwo files.
+// NODWP: unable to locate separate debug file (dwo, dwp). Debugging will be degraded.
+
 
 struct A {
   int x = 47;

>From 214fe9773a4549b4893fd2ce2f2f9d22ba476fe8 Mon Sep 17 00:00:00 2001
From: Greg Clayton <clayborg at gmail.com>
Date: Thu, 8 Feb 2024 22:32:30 -0800
Subject: [PATCH 3/3] Fix comment.

---
 lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
index e48531e663e8c8..7e64744e34fda0 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -4363,8 +4363,8 @@ const std::shared_ptr<SymbolFileDWARFDwo> &SymbolFileDWARF::GetDwpSymbolFile() {
       symfiles.Append(symfile_fspec);
     } else {
       // If we don't have a separate debug info file, then try stripping the
-      // extension. We main module could be "a.debug" and the .dwp file could be
-      // "a.dwp" instead of "a.debug.dwp".
+      // extension. The main module could be "a.debug" and the .dwp file could
+      // be "a.dwp" instead of "a.debug.dwp".
       ConstString filename_no_ext =
           module_fspec.GetFileNameStrippingExtension();
       if (filename_no_ext != module_fspec.GetFilename()) {



More information about the lldb-commits mailing list