From 82d134f6679e967c7c86def334ba600fb90cc46b Mon Sep 17 00:00:00 2001 From: Kevin Frei Date: Mon, 25 Mar 2024 08:33:58 -0700 Subject: [PATCH] Reapply "DebugInfoD tests + fixing issues exposed by tests (#85693)" This reverts commit 7fc2fbb3f1961e0ad0722c2d749ddd6264195a1c. --- .../Python/lldbsuite/test/make/Makefile.rules | 33 +++++++++++++++- .../SymbolFile/DWARF/SymbolFileDWARF.cpp | 38 ++++++++++++------- .../Plugins/SymbolLocator/CMakeLists.txt | 7 +++- .../SymbolVendor/ELF/SymbolVendorELF.cpp | 15 ++++++-- lldb/test/API/debuginfod/Normal/Makefile | 25 ++++++++++++ .../API/debuginfod/Normal/TestDebuginfod.py | 4 +- lldb/test/API/debuginfod/Normal/main.c | 7 ++++ lldb/test/API/debuginfod/SplitDWARF/Makefile | 28 ++++++++++++++ .../SplitDWARF/TestDebuginfodDWP.py | 2 +- lldb/test/API/debuginfod/SplitDWARF/main.c | 7 ++++ 10 files changed, 144 insertions(+), 22 deletions(-) create mode 100644 lldb/test/API/debuginfod/Normal/Makefile create mode 100644 lldb/test/API/debuginfod/Normal/main.c create mode 100644 lldb/test/API/debuginfod/SplitDWARF/Makefile create mode 100644 lldb/test/API/debuginfod/SplitDWARF/main.c diff --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules index bd8eea3d6f5a04..0aa94bdc0b5c8a 100644 --- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules +++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules @@ -51,7 +51,7 @@ LLDB_BASE_DIR := $(THIS_FILE_DIR)/../../../../../ # # GNUWin32 uname gives "windows32" or "server version windows32" while # some versions of MSYS uname return "MSYS_NT*", but most environments -# standardize on "Windows_NT", so we'll make it consistent here. +# standardize on "Windows_NT", so we'll make it consistent here. # When running tests from Visual Studio, the environment variable isn't # inherited all the way down to the process spawned for make. #---------------------------------------------------------------------- @@ -210,6 +210,12 @@ else ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES" DSYM = $(EXE).debug endif + + ifeq "$(MAKE_DWP)" "YES" + MAKE_DWO := YES + DWP_NAME = $(EXE).dwp + DYLIB_DWP_NAME = $(DYLIB_NAME).dwp + endif endif LIMIT_DEBUG_INFO_FLAGS = @@ -358,6 +364,7 @@ ifneq "$(OS)" "Darwin" OBJCOPY ?= $(call replace_cc_with,objcopy) ARCHIVER ?= $(call replace_cc_with,ar) + DWP ?= $(call replace_cc_with,dwp) override AR = $(ARCHIVER) endif @@ -528,6 +535,10 @@ ifneq "$(CXX)" "" endif endif +ifeq "$(GEN_GNU_BUILD_ID)" "YES" + LDFLAGS += -Wl,--build-id +endif + #---------------------------------------------------------------------- # DYLIB_ONLY variable can be used to skip the building of a.out. # See the sections below regarding dSYM file as well as the building of @@ -566,11 +577,25 @@ else endif else ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES" +ifeq "$(SAVE_FULL_DEBUG_BINARY)" "YES" + cp "$(EXE)" "$(EXE).unstriped"" +endif $(OBJCOPY) --only-keep-debug "$(EXE)" "$(DSYM)" $(OBJCOPY) --strip-debug --add-gnu-debuglink="$(DSYM)" "$(EXE)" "$(EXE)" endif +ifeq "$(MAKE_DWP)" "YES" + $(DWP) -o "$(DWP_NAME)" $(DWOS) +endif endif + +#---------------------------------------------------------------------- +# Support emitting the content of the GNU build-id into a file +# This needs to be used in conjunction with GEN_GNU_BUILD_ID := YES +#---------------------------------------------------------------------- +$(EXE).uuid : $(EXE) + $(OBJCOPY) --dump-section=.note.gnu.build-id=$@ $< + #---------------------------------------------------------------------- # Make the dylib #---------------------------------------------------------------------- @@ -611,9 +636,15 @@ endif else $(LD) $(DYLIB_OBJECTS) $(LDFLAGS) -shared -o "$(DYLIB_FILENAME)" ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES" + ifeq "$(SAVE_FULL_DEBUG_BINARY)" "YES" + cp "$(DYLIB_FILENAME)" "$(DYLIB_FILENAME).unstripped" + endif $(OBJCOPY) --only-keep-debug "$(DYLIB_FILENAME)" "$(DYLIB_FILENAME).debug" $(OBJCOPY) --strip-debug --add-gnu-debuglink="$(DYLIB_FILENAME).debug" "$(DYLIB_FILENAME)" "$(DYLIB_FILENAME)" endif +ifeq "$(MAKE_DWP)" "YES" + $(DWP) -o $(DYLIB_DWP_FILE) $(DYLIB_DWOS) +endif endif #---------------------------------------------------------------------- diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp index f2ff3a8b259fa4..e09c9a86478bb7 100644 --- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -4299,26 +4299,38 @@ const std::shared_ptr &SymbolFileDWARF::GetDwpSymbolFile() { FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths(); ModuleSpec module_spec; module_spec.GetFileSpec() = m_objfile_sp->GetFileSpec(); + FileSpec dwp_filespec; for (const auto &symfile : symfiles.files()) { module_spec.GetSymbolFileSpec() = FileSpec(symfile.GetPath() + ".dwp", symfile.GetPathStyle()); LLDB_LOG(log, "Searching for DWP using: \"{0}\"", module_spec.GetSymbolFileSpec()); - FileSpec dwp_filespec = + dwp_filespec = PluginManager::LocateExecutableSymbolFile(module_spec, search_paths); if (FileSystem::Instance().Exists(dwp_filespec)) { - LLDB_LOG(log, "Found DWP file: \"{0}\"", 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) { - m_dwp_symfile = std::make_shared( - *this, dwp_obj_file, DIERef::k_file_index_mask); - break; - } + break; + } + } + if (!FileSystem::Instance().Exists(dwp_filespec)) { + LLDB_LOG(log, "No DWP file found locally"); + // Fill in the UUID for the module we're trying to match for, so we can + // find the correct DWP file, as the Debuginfod plugin uses *only* this + // data to correctly match the DWP file with the binary. + module_spec.GetUUID() = m_objfile_sp->GetUUID(); + dwp_filespec = + PluginManager::LocateExecutableSymbolFile(module_spec, search_paths); + } + if (FileSystem::Instance().Exists(dwp_filespec)) { + LLDB_LOG(log, "Found DWP file: \"{0}\"", 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) { + m_dwp_symfile = std::make_shared( + *this, dwp_obj_file, DIERef::k_file_index_mask); } } if (!m_dwp_symfile) { diff --git a/lldb/source/Plugins/SymbolLocator/CMakeLists.txt b/lldb/source/Plugins/SymbolLocator/CMakeLists.txt index ca969626f4ffc4..3367022639ab85 100644 --- a/lldb/source/Plugins/SymbolLocator/CMakeLists.txt +++ b/lldb/source/Plugins/SymbolLocator/CMakeLists.txt @@ -1,5 +1,10 @@ +# Order matters here: the first symbol locator prevents further searching. +# For DWARF binaries that are both stripped and split, the Default plugin +# will return the stripped binary when asked for the ObjectFile, which then +# prevents an unstripped binary from being requested from the Debuginfod +# provider. +add_subdirectory(Debuginfod) add_subdirectory(Default) if (CMAKE_SYSTEM_NAME MATCHES "Darwin") add_subdirectory(DebugSymbols) endif() -add_subdirectory(Debuginfod) diff --git a/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp b/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp index a881218a56cef3..f296e655cc466d 100644 --- a/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp +++ b/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp @@ -55,8 +55,8 @@ static bool IsDwpSymbolFile(const lldb::ModuleSP &module_sp, dwp_file_data_sp, dwp_file_data_offset); // The presence of a debug_cu_index section is the key identifying feature of // a DWP file. Make sure we don't fill in the section list on dwp_obj_file - // (by calling GetSectionList(false)) as this is invoked before we may have - // all the symbol files collected and available. + // (by calling GetSectionList(false)) as this function could be called before + // we may have all the symbol files collected and available. return dwp_obj_file && ObjectFileELF::classof(dwp_obj_file.get()) && dwp_obj_file->GetSectionList(false)->FindSectionByType( eSectionTypeDWARFDebugCuIndex, false); @@ -105,8 +105,15 @@ SymbolVendorELF::CreateInstance(const lldb::ModuleSP &module_sp, FileSpecList search_paths = Target::GetDefaultDebugFileSearchPaths(); FileSpec dsym_fspec = PluginManager::LocateExecutableSymbolFile(module_spec, search_paths); - if (!dsym_fspec) - return nullptr; + if (!dsym_fspec || IsDwpSymbolFile(module_sp, dsym_fspec)) { + // If we have a stripped binary or if we got a DWP file, we should prefer + // symbols in the executable acquired through a plugin. + ModuleSpec unstripped_spec = + PluginManager::LocateExecutableObjectFile(module_spec); + if (!unstripped_spec) + return nullptr; + dsym_fspec = unstripped_spec.GetFileSpec(); + } DataBufferSP dsym_file_data_sp; lldb::offset_t dsym_file_data_offset = 0; diff --git a/lldb/test/API/debuginfod/Normal/Makefile b/lldb/test/API/debuginfod/Normal/Makefile new file mode 100644 index 00000000000000..bd2fa623df473d --- /dev/null +++ b/lldb/test/API/debuginfod/Normal/Makefile @@ -0,0 +1,25 @@ +C_SOURCES := main.c + +# For normal (non DWP) Debuginfod tests, we need: + +# * The "full" binary: a.out.debug +# Produced by Makefile.rules with KEEP_FULL_DEBUG_BINARY set to YES and +# SPLIT_DEBUG_SYMBOLS set to YES + +# * The stripped binary (a.out) +# Produced by Makefile.rules with SPLIT_DEBUG_SYMBOLS set to YES + +# * The 'only-keep-debug' binary (a.out.dbg) +# Produced below + +# * The .uuid file (for a little easier testing code) +# Produced below + +# Don't strip the debug info from a.out: +SPLIT_DEBUG_SYMBOLS := YES +SAVE_FULL_DEBUG_BINARY := YES +GEN_GNU_BUILD_ID := YES + +all: a.out.uuid a.out + +include Makefile.rules diff --git a/lldb/test/API/debuginfod/Normal/TestDebuginfod.py b/lldb/test/API/debuginfod/Normal/TestDebuginfod.py index a662fb9fc6e686..4bbb43782118c1 100644 --- a/lldb/test/API/debuginfod/Normal/TestDebuginfod.py +++ b/lldb/test/API/debuginfod/Normal/TestDebuginfod.py @@ -63,7 +63,7 @@ def test_debuginfod_symbols(self): Test behavior with the full binary available from Debuginfod as 'debuginfo' from the plug-in. """ - test_root = self.config_test(["a.out"], "a.out.full") + test_root = self.config_test(["a.out"], "a.out.unstripped") self.try_breakpoint(True) def test_debuginfod_executable(self): @@ -71,7 +71,7 @@ def test_debuginfod_executable(self): Test behavior with the full binary available from Debuginfod as 'executable' from the plug-in. """ - test_root = self.config_test(["a.out"], None, "a.out.full") + test_root = self.config_test(["a.out"], None, "a.out.unstripped") self.try_breakpoint(True) def test_debuginfod_okd_symbols(self): diff --git a/lldb/test/API/debuginfod/Normal/main.c b/lldb/test/API/debuginfod/Normal/main.c new file mode 100644 index 00000000000000..4c7184609b4536 --- /dev/null +++ b/lldb/test/API/debuginfod/Normal/main.c @@ -0,0 +1,7 @@ +// This is a dump little pair of test files + +int func(int argc, const char *argv[]) { + return (argc + 1) * (argv[argc][0] + 2); +} + +int main(int argc, const char *argv[]) { return func(0, argv); } diff --git a/lldb/test/API/debuginfod/SplitDWARF/Makefile b/lldb/test/API/debuginfod/SplitDWARF/Makefile new file mode 100644 index 00000000000000..266d74cf906298 --- /dev/null +++ b/lldb/test/API/debuginfod/SplitDWARF/Makefile @@ -0,0 +1,28 @@ +C_SOURCES := main.c + +# For split-dwarf Debuginfod tests, we need: + +# * A .DWP file (a.out.dwp) +# Produced by Makefile.rules with MAKE_DWO and MERGE_DWOS both set to YES + +# * The "full" binary: it's missing things that live in .dwo's (a.out.debug) +# Produced by Makefile.rules with KEEP_FULL_DEBUG_BINARY set to YES and +# SPLIT_DEBUG_SYMBOLS set to YES + +# * The stripped binary (a.out) +# Produced by Makefile.rules + +# * The 'only-keep-debug' binary (a.out.dbg) +# Produced below + +# * The .uuid file (for a little easier testing code) +# Produced here in the rule below + +MAKE_DWP := YES +SPLIT_DEBUG_SYMBOLS := YES +SAVE_FULL_DEBUG_BINARY := YES +GEN_GNU_BUILD_ID := YES + +all: a.out.uuid a.out + +include Makefile.rules diff --git a/lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py b/lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py index fda54e15d8a847..53a19c0a8454d1 100644 --- a/lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py +++ b/lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py @@ -80,7 +80,7 @@ def test_debuginfod_both_symfiles_from_service(self): Test behavior with a stripped binary, with the unstripped binary and dwp symbols from Debuginfod. """ - self.config_test(["a.out"], "a.out.dwp", "a.out.full") + self.config_test(["a.out"], "a.out.dwp", "a.out.unstripped") self.try_breakpoint(True) def test_debuginfod_both_okd_symfiles_from_service(self): diff --git a/lldb/test/API/debuginfod/SplitDWARF/main.c b/lldb/test/API/debuginfod/SplitDWARF/main.c new file mode 100644 index 00000000000000..4c7184609b4536 --- /dev/null +++ b/lldb/test/API/debuginfod/SplitDWARF/main.c @@ -0,0 +1,7 @@ +// This is a dump little pair of test files + +int func(int argc, const char *argv[]) { + return (argc + 1) * (argv[argc][0] + 2); +} + +int main(int argc, const char *argv[]) { return func(0, argv); }