[Lldb-commits] [lldb] Add more ways to find the .dwp file. (PR #81067)
clayborg wrote: > Will this now work with .dwp files not having UUID? I actually added a test for this and it works just fine if the main executable (`a.out`) has a UUID and the debug info file (`a.out.debug`) also has the UUID, but the .dwp file doesn't, we _are_ able to load the .dwp file just fine. https://github.com/llvm/llvm-project/pull/81067 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add more ways to find the .dwp file. (PR #81067)
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 Date: Wed, 7 Feb 2024 16:43:50 -0800 Subject: [PATCH 1/4] 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 "" which contains skeleton DWARF and needs to find ".dwp" - lldb loads "" which is stripped but has .gnu_debuglink pointing to ".debug" with skeleton DWARF and needs to find ".dwp" - lldb loads "" which is stripped but has .gnu_debuglink pointing to ".debug" with skeleton DWARF and needs to find ".debug.dwp" - lldb loads ".debug" and needs to find ".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 49edc667ddd5b..6eb3bb9971f13 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 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 781f5c5a43677..487961fa7437f 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 ::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(), _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( - *this, dwp_obj_file, DIERef::k_file_index_mask); +for (const auto : 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(), _filespec, 0, +FileSystem::Instance().GetByteSize(dwp_filespec), dwp_file_data_sp, +
[Lldb-commits] [lldb] Add more ways to find the .dwp file. (PR #81067)
github-actions[bot] wrote: :warning: C/C++ code formatter, clang-format found issues in your code. :warning: You can test this locally with the following command: ``bash git-clang-format --diff c13e271a38363d354294e2af1651470bed8facb3 9d58f41457fc2c9e54b1409c64f3028fdaededf1 -- lldb/include/lldb/Utility/FileSpecList.h lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp lldb/test/Shell/SymbolFile/DWARF/x86/dwp-separate-debug-file.cpp `` View the diff from clang-format here. ``diff 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 dbb8aed54e..4fe20832d2 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 @@ -137,7 +137,6 @@ // 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; }; `` https://github.com/llvm/llvm-project/pull/81067 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add more ways to find the .dwp file. (PR #81067)
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 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 "" which contains skeleton DWARF and needs to find ".dwp" - lldb loads "" which is stripped but has .gnu_debuglink pointing to ".debug" with skeleton DWARF and needs to find ".dwp" - lldb loads "" which is stripped but has .gnu_debuglink pointing to ".debug" with skeleton DWARF and needs to find ".debug.dwp" - lldb loads ".debug" and needs to find ".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 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 ::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(), _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( - *this, dwp_obj_file, DIERef::k_file_index_mask); +for (const auto : 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(), _filespec, 0, +FileSystem::Instance().GetByteSize(dwp_filespec), dwp_file_data_sp, +
[Lldb-commits] [lldb] Add more ways to find the .dwp file. (PR #81067)
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 Date: Wed, 7 Feb 2024 16:43:50 -0800 Subject: [PATCH 1/2] 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 "" which contains skeleton DWARF and needs to find ".dwp" - lldb loads "" which is stripped but has .gnu_debuglink pointing to ".debug" with skeleton DWARF and needs to find ".dwp" - lldb loads "" which is stripped but has .gnu_debuglink pointing to ".debug" with skeleton DWARF and needs to find ".debug.dwp" - lldb loads ".debug" and needs to find ".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 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 ::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(), _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( - *this, dwp_obj_file, DIERef::k_file_index_mask); +for (const auto : 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(), _filespec, 0, +FileSystem::Instance().GetByteSize(dwp_filespec), dwp_file_data_sp, +
[Lldb-commits] [lldb] Add more ways to find the .dwp file. (PR #81067)
clayborg wrote: > FWIW, I think we should be opinionated (& consistent with whatever gdb does, > if it has some precedent here - if it is less opinionated, then maybe we have > to be accepting) when it comes to whether x.debug goes with x.dwp or > x.debug.dwp - we shouldn't support both unless there's some prior precedent > that's unavoidable to support. It'd be better for everyone if there was one > option we encourage people to follow. (so I think we shouldn't support (2) > and (3) if we can help it - we should pick one) There currently is no enforcement on any of this from the compiler or linker driver side and everyone must do the stripping of the executable and creating the .dwp file manually. Since there is no actual enforcement, it seems like we should support both IMHO. More on this below... > I'm not sure I understand the "lldb loads .debug and needs to find .dwp" > case. the .debug file usually only has the debug info, not the executable > code, so you can't load it directly, right? (see the documentation in > https://sourceware.org/gdb/current/onlinedocs/gdb.html/Separate-Debug-Files.html > - `objcopy --only-keep-debug foo foo.debug`). If we have a build system that creates: - `a.out` which is stripped and shipped for distribution - `a.out.debug` which contains debug info only with the skeleton units, address table, line tables, accelerator tables - `a.out.dwp` or `a.out.debug.dwp` which contains the .dwo files for `a.out.debug` We might request to download debug info for this build using a GNU build ID or some other build number for say a symbolication server which wants to use LLDB. If we get this debug info, we might only download `a.out.debug` and the associated `.dwp` file (either `a.out.dwp` or `a.out.debug.dwp`). It would be a shame to require that we download the `a.out` binary as well just so that we can load this into a debugger and get the .dwp file to load automatically. With this change people _can_ load `a.out.debug` into LLDB and peruse the debug information and perform symbolication without having to download the original `a.out`, a file which is not required for symbolication. If we downloaded `a.out.debug` + `a.out.dwp`, unless a user knows to rename `a.out.dwp` to `a.out.debug.dwp`, or create a symlink, then they won't get any info from the .dwo files in the .dwp file. If we don't fix this, then people will get confused and not know how to fix things and get full debug info to get loaded. No one besides debugger, compiler, and linker engineers and very few other people actually know what split DWARF does and or means and if they can't get things to work, they give up or waste time figuring out the magic things that need to happen to make us able to load the right debug info. So I prefer to allow things to work more seamlessly without trying to enforce something that is left up to users to do in the first place. I am someone that often responds to peoples posts when I am on call and I need to help people figure out how to load their debug symbols correrctl. With split DWARF it is even worse because if they get the debug info file with the skeleton compile units but the .dwp file isn't named consistent with GDB conventions, then they can set file and line breakpoints and hit them, but as soon as they stop there, they get no variable information because the .dwo file doesn't get loaded. They don't know what a .dwo or a .dwp file is or why it is needed. We get a few posts a week with people trying to use split DWARF symbols and not being able to get things to work. So IMHO there is no harm in making all scenarios work without having to require some naming convention that isn't actually enforced by _any_ compiler or linker driver, and when things don't work, we put the pressure on the user to find some wiki somewhere that says "here is how you were supposed to do it" and figure out how to fix the issue themselves or by seeking help from us. https://github.com/llvm/llvm-project/pull/81067 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add more ways to find the .dwp file. (PR #81067)
dwblaikie wrote: > If the DWO ID is just a hash of the file path or something that isn't > guaranteed to be unique with each new build, then we need the UUID in the > .dwp file. Nah, the DWO ID, as per spec, is a semantic hash of the DWARF contents. It should change, generally, if any part of the DWARF changes. https://github.com/llvm/llvm-project/blob/1d4fc381d3da4317cc2cfa59b2d59d53decddf71/llvm/lib/CodeGen/AsmPrinter/DIEHash.cpp#L399 https://github.com/llvm/llvm-project/pull/81067 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add more ways to find the .dwp file. (PR #81067)
clayborg wrote: > > > Will this now work with .dwp files not having UUID? > > > > > > No. If binairies have UUIDs (GNU build IDs), they need to match right now. > > That is larger fix that involves adding a "enum UUIDFlavor" to the UUIDs so > > we can ensure we aren't comparing two different things. > > What Alexander is talking about is if we have a GNU build ID in ``, > > the `.debug` file will have the same UUID, but llvm-dwp currently > > doesn't copy the GNU build ID over into the `.dwp` file. This causes LLDB > > to not allow the .dwp file to be loaded. The problem is if the .dwp file > > doesn't have a UUID, it will make one up by calculating a CRC of the file > > itself, and then we will compare a GNU build ID from `` to the CRC > > calculated by the `.dwp` file and they won't match. > > @dwblaikie do you know how accurate the DWO ID is? Can we avoid relying on > > matching up the UUID on the .dwp file and solely rely on allowing it to be > > loaded and rely on the DWO IDs matching between the skeleton unit and the > > .dwo unit? If so, there is an easy fix I can make to this patch to solve > > that problem. > > Not sure I follow. For .dwo files path is described in Skeleton CU: > DW_AT_comp_dir/DW_AT_dwo_name. The DWP file can have multiple CUs with > different DWO IDs. Exactly. The question is, if a have one `a.out.dwp` file that is out of date, and one of the .dwo files was recompiled into the `a.out` binary, but the `a.out.dwp` file wasn't recreated and if it was actually allowed to be loaded without comparing the GNU build ID, will the DWO ID be unique if the skeleton unit has a `DW_AT_dwo_name` with "foo.o" that has a DWO ID of `0x123` and if we lookup up "foo.o" in the old .dwp file, will the DWO ID mismatch be enough for us to ensure we don't load the .dwo info. If the answer is yes, which implied the DWO ID is some sort of checksum or signature that always changes when a specific file is rebuilt, then it is ok to not match a UUID on a .dwp file. Seeing as no .dwp files actually have a GNU build ID right now unless people manually add it, it seems like we should allow loading any .dwp regardless of the lack of a GNU build ID and deal with the mismatches from the DWO ID. If the DWO ID is just a hash of the file path or something that isn't guaranteed to be unique with each new build, then we need the UUID in the .dwp file. https://github.com/llvm/llvm-project/pull/81067 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add more ways to find the .dwp file. (PR #81067)
dwblaikie wrote: FWIW, I think we should be opinionated (& consistent with whatever gdb does, if it has some precedent here - if it is less opinionated, then maybe we have to be accepting) when it comes to whether x.debug goes with x.dwp or x.debug.dwp - we shouldn't support both unless there's some prior precedent that's unavoidable to support. It'd be better for everyone if there was one option we encourage people to follow. (so I think we shouldn't support (2) and (3) if we can help it - we should pick one) I'm not sure I understand the "lldb loads .debug and needs to find .dwp" case. the .debug file usually only has the debug info, not the executable code, so you can't load it directly, right? (see the documentation in https://sourceware.org/gdb/current/onlinedocs/gdb.html/Separate-Debug-Files.html - `objcopy --only-keep-debug foo foo.debug`). https://github.com/llvm/llvm-project/pull/81067 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add more ways to find the .dwp file. (PR #81067)
@@ -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 adrian-prantl wrote: Should there be some negative check that removes the dwp and verifies that it fails, so we know for sure that the dwp code path was taken? https://github.com/llvm/llvm-project/pull/81067 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add more ways to find the .dwp file. (PR #81067)
@@ -4349,26 +4349,53 @@ SymbolFileDWARFDebugMap *SymbolFileDWARF::GetDebugMapSymfile() { const std::shared_ptr ::GetDwpSymbolFile() { llvm::call_once(m_dwp_symfile_once_flag, [this]() { +// Create a list of files to try and append .dwp to adrian-prantl wrote: ```suggestion // Create a list of files to try and append .dwp to. ``` https://github.com/llvm/llvm-project/pull/81067 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add more ways to find the .dwp file. (PR #81067)
@@ -4349,26 +4349,53 @@ SymbolFileDWARFDebugMap *SymbolFileDWARF::GetDebugMapSymfile() { const std::shared_ptr ::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(), _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( - *this, dwp_obj_file, DIERef::k_file_index_mask); +for (const auto : 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(), _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( +*this, dwp_obj_file, DIERef::k_file_index_mask); +break; adrian-prantl wrote: Either use break or return for both? https://github.com/llvm/llvm-project/pull/81067 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add more ways to find the .dwp file. (PR #81067)
@@ -4349,26 +4349,53 @@ SymbolFileDWARFDebugMap *SymbolFileDWARF::GetDebugMapSymfile() { const std::shared_ptr ::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 adrian-prantl wrote: "The" main module? https://github.com/llvm/llvm-project/pull/81067 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add more ways to find the .dwp file. (PR #81067)
ayermolo wrote: > > Will this now work with .dwp files not having UUID? > > No. If binairies have UUIDs (GNU build IDs), they need to match right now. > That is larger fix that involves adding a "enum UUIDFlavor" to the UUIDs so > we can ensure we aren't comparing two different things. > > What Alexander is talking about is if we have a GNU build ID in ``, the > `.debug` file will have the same UUID, but llvm-dwp currently doesn't > copy the GNU build ID over into the `.dwp` file. This causes LLDB to not > allow the .dwp file to be loaded. The problem is if the .dwp file doesn't > have a UUID, it will make one up by calculating a CRC of the file itself, and > then we will compare a GNU build ID from `` to the CRC calculated by the > `.dwp` file and they won't match. > > @dwblaikie do you know how accurate the DWO ID is? Can we avoid relying on > matching up the UUID on the .dwp file and solely rely on allowing it to be > loaded and rely on the DWO IDs matching between the skeleton unit and the > .dwo unit? If so, there is an easy fix I can make to this patch to solve that > problem. Not sure I follow. For .dwo files path is described in Skeleton CU: DW_AT_comp_dir/DW_AT_dwo_name. The DWP file can have multiple CUs with different DWO IDs. https://github.com/llvm/llvm-project/pull/81067 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add more ways to find the .dwp file. (PR #81067)
clayborg wrote: > Will this now work with .dwp files not having UUID? No. If binairies have UUIDs (GNU build IDs), they need to match right now. That is larger fix that involves adding a "enum UUIDFlavor" to the UUIDs so we can ensure we aren't comparing two different things. What Alexander is talking about is if we have a GNU build ID in ``, the `.debug` file will have the same UUID, but llvm-dwp currently doesn't copy the GNU build ID over into the `.dwp` file. This causes LLDB to not allow the .dwp file to be loaded. The problem is if the .dwp file doesn't have a UUID, it will make one up by calculating a CRC of the file itself, and then we will compare a GNU build ID from `` to the CRC calculated by the `.dwp` file and they won't match. @dwblaikie do you know how accurate the DWO ID is? Can we avoid relying on matching up the UUID on the .dwp file and solely rely on allowing it to be loaded and rely on the DWO IDs matching between the skeleton unit and the .dwo unit? If so, there is an easy fix I can make to this patch to solve that problem. https://github.com/llvm/llvm-project/pull/81067 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add more ways to find the .dwp file. (PR #81067)
kevinfrei wrote: > Will this now work with .dwp files not having UUID? The lack of UUID is kinda why this is so important. The connection is strictly based on the filename. This just expands the variety of filenames that can be supported. One thing that's helpful is that the .gnu_debuglink can be a relative/absolute path, so it enables putting the files in a different location than right beside the binary, which is definitely an improvement. https://github.com/llvm/llvm-project/pull/81067 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add more ways to find the .dwp file. (PR #81067)
ayermolo wrote: Will this now work with .dwp files not having UUID? https://github.com/llvm/llvm-project/pull/81067 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add more ways to find the .dwp file. (PR #81067)
https://github.com/clayborg edited https://github.com/llvm/llvm-project/pull/81067 ___ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
[Lldb-commits] [lldb] Add more ways to find the .dwp file. (PR #81067)
llvmbot wrote: @llvm/pr-subscribers-lldb Author: Greg Clayton (clayborg) Changes 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. --- Full diff: https://github.com/llvm/llvm-project/pull/81067.diff 3 Files Affected: - (modified) lldb/include/lldb/Utility/FileSpecList.h (+4) - (modified) lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp (+44-17) - (modified) lldb/test/Shell/SymbolFile/DWARF/x86/dwp-separate-debug-file.cpp (+30) ``diff diff --git a/lldb/include/lldb/Utility/FileSpecList.h b/lldb/include/lldb/Utility/FileSpecList.h index 49edc667ddd5b..6eb3bb9971f13 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 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 781f5c5a43677..487961fa7437f 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 ::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(), _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( - *this, dwp_obj_file, DIERef::k_file_index_mask); +for (const auto : 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(), _filespec, 0, +FileSystem::Instance().GetByteSize(dwp_filespec), dwp_file_data_sp, +dwp_file_data_offset); +if (!dwp_obj_file) + return; +
[Lldb-commits] [lldb] Add more ways to find the .dwp file. (PR #81067)
https://github.com/clayborg created https://github.com/llvm/llvm-project/pull/81067 When using split DWARF we can run into many different ways to store debug info: - lldb loads "" which contains skeleton DWARF and needs to find ".dwp" - lldb loads "" which is stripped but has .gnu_debuglink pointing to ".debug" with skeleton DWARF and needs to find ".dwp" - lldb loads "" which is stripped but has .gnu_debuglink pointing to ".debug" with skeleton DWARF and needs to find ".debug.dwp" - lldb loads ".debug" and needs to find ".dwp Previously we only handled the first two cases. This patch adds support for the latter two. >From 3c2f6039cf0e253d78b5193098b311028daaea72 Mon Sep 17 00:00:00 2001 From: Greg Clayton Date: Wed, 7 Feb 2024 16:43:50 -0800 Subject: [PATCH] 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 "" which contains skeleton DWARF and needs to find ".dwp" - lldb loads "" which is stripped but has .gnu_debuglink pointing to ".debug" with skeleton DWARF and needs to find ".dwp" - lldb loads "" which is stripped but has .gnu_debuglink pointing to ".debug" with skeleton DWARF and needs to find ".debug.dwp" - lldb loads ".debug" and needs to find ".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 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 ::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(), _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( - *this, dwp_obj_file, DIERef::k_file_index_mask); +for (const auto : symfiles.files()) { + module_spec.GetSymbolFileSpec() = +