DebugInfoD tests + fixing issues exposed by tests #85693
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kevinfrei
force-pushed
the
debuginfod-issues
branch
from
915f7e0
to
b00e998
Compare
|
✅ With the latest revision this PR passed the Python code formatter. |
|
@llvm/pr-subscribers-lldb Author: Kevin Frei (kevinfrei) ChangesFinally getting back to Debuginfod tests, I've migrated my tests from shell to API (at @JDevlieghere's suggestion) and addressed a couple issues that came about during testing. Patch is 24.53 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/85693.diff 10 Files Affected:
diff --git a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
index bfd249ccd43f2e..370db60ab7f7d6 100644
--- a/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
+++ b/lldb/packages/Python/lldbsuite/test/make/Makefile.rules
@@ -210,6 +210,12 @@ else
ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES"
DSYM = $(EXE).debug
endif
+
+ ifeq "$(MERGE_DWOS)" "YES"
+ MAKE_DWO := YES
+ DWP_NAME = $(EXE).dwp
+ DYLIB_DWP_NAME = $(DYLIB_NAME).dwp
+ endif
endif
LIMIT_DEBUG_INFO_FLAGS =
@@ -357,6 +363,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
@@ -527,6 +534,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
@@ -565,11 +576,25 @@ else
endif
else
ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES"
+ifeq "$(SAVE_FULL_DEBUG_BINARY)" "YES"
+ cp "$(EXE)" "$(EXE).full"
+endif
$(OBJCOPY) --only-keep-debug "$(EXE)" "$(DSYM)"
$(OBJCOPY) --strip-debug --add-gnu-debuglink="$(DSYM)" "$(EXE)" "$(EXE)"
endif
+ifeq "$(MERGE_DWOS)" "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
#----------------------------------------------------------------------
@@ -610,9 +635,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).full"
+ 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 "$(MERGE_DWOS)" "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 5f67658f86ea96..0b7e86743f9f9f 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
@@ -4377,26 +4377,40 @@ const std::shared_ptr<SymbolFileDWARFDwo> &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<SymbolFileDWARFDwo>(
- *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);
+ // Set it back so it's not outliving the m_objfile_sp shared pointer.
+ module_spec.GetUUID() = {};
+ }
+ 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<SymbolFileDWARFDwo>(
+ *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 b5fe35d71032a8..cd9259a8c21630 100644
--- a/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
+++ b/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
@@ -44,6 +44,27 @@ llvm::StringRef SymbolVendorELF::GetPluginDescriptionStatic() {
"executables.";
}
+// If this is needed elsewhere, it can be exported/moved.
+static bool IsDwpSymbolFile(const lldb::ModuleSP &module_sp,
+ const FileSpec &file_spec) {
+ DataBufferSP dwp_file_data_sp;
+ lldb::offset_t dwp_file_data_offset = 0;
+ // Try to create an ObjectFile from the file_spec.
+ ObjectFileSP dwp_obj_file = ObjectFile::FindPlugin(
+ module_sp, &file_spec, 0, FileSystem::Instance().GetByteSize(file_spec),
+ dwp_file_data_sp, dwp_file_data_offset);
+ if (!ObjectFileELF::classof(dwp_obj_file.get()))
+ return false;
+ // 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.
+ if (!dwp_obj_file || !dwp_obj_file->GetSectionList(false)->FindSectionByType(
+ eSectionTypeDWARFDebugCuIndex, false))
+ return false;
+ return true;
+}
+
// CreateInstance
//
// Platforms can register a callback to use when creating symbol vendors to
@@ -87,8 +108,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
new file mode 100644
index 00000000000000..78fa8aaeea2e59
--- /dev/null
+++ b/lldb/test/API/debuginfod/Normal/TestDebuginfod.py
@@ -0,0 +1,161 @@
+import os
+import shutil
+import tempfile
+import struct
+
+import lldb
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+def getUUID(aoutuuid):
+ """
+ Pull the 20 byte UUID out of the .note.gnu.build-id section that was dumped
+ to a file already, as part of the build.
+ """
+ with open(aoutuuid, "rb") as f:
+ data = f.read(36)
+ if len(data) != 36:
+ return None
+ header = struct.unpack_from("<4I", data)
+ if len(header) != 4:
+ return None
+ # 4 element 'prefix', 20 bytes of uuid, 3 byte long string: 'GNU':
+ if header[0] != 4 or header[1] != 20 or header[2] != 3 or header[3] != 0x554e47:
+ return None
+ return data[16:].hex()
+
+
+"""
+Test support for the DebugInfoD network symbol acquisition protocol.
+This one is for simple / no split-dwarf scenarios.
+
+For no-split-dwarf scenarios, there are 2 variations:
+1 - A stripped binary with it's corresponding unstripped binary:
+2 - A stripped binary with a corresponding --only-keep-debug symbols file
+"""
+@skipUnlessPlatform(["linux", "freebsd"])
+class DebugInfodTests(TestBase):
+ # No need to try every flavor of debug inf.
+ NO_DEBUG_INFO_TESTCASE = True
+
+ def test_normal_no_symbols(self):
+ """
+ Validate behavior with no symbols or symbol locator.
+ ('baseline negative' behavior)
+ """
+ test_root = self.config_test(["a.out"])
+ self.try_breakpoint(False)
+
+ def test_normal_default(self):
+ """
+ Validate behavior with symbols, but no symbol locator.
+ ('baseline positive' behavior)
+ """
+ test_root = self.config_test(["a.out", "a.out.debug"])
+ self.try_breakpoint(True)
+
+ 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")
+ self.try_breakpoint(True)
+
+ 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")
+ self.try_breakpoint(True)
+
+ def test_debuginfod_okd_symbols(self):
+ """
+ Test behavior with the 'only-keep-debug' symbols available from Debuginfod.
+ """
+ test_root = self.config_test(["a.out"], "a.out.debug")
+ self.try_breakpoint(True)
+
+ def try_breakpoint(self, should_have_loc):
+ """
+ This function creates a target from self.aout, sets a function-name
+ breakpoint, and checks to see if we have a file/line location,
+ as a way to validate that the symbols have been loaded.
+ should_have_loc specifies if we're testing that symbols have or
+ haven't been loaded.
+ """
+ target = self.dbg.CreateTarget(self.aout)
+ self.assertTrue(target and target.IsValid(), "Target is valid")
+
+ bp = target.BreakpointCreateByName("func")
+ self.assertTrue(bp and bp.IsValid(), "Breakpoint is valid")
+ self.assertEqual(bp.GetNumLocations(), 1)
+
+ loc = bp.GetLocationAtIndex(0)
+ self.assertTrue(loc and loc.IsValid(), "Location is valid")
+ addr = loc.GetAddress()
+ self.assertTrue(addr and addr.IsValid(), "Loc address is valid")
+ line_entry = addr.GetLineEntry()
+ self.assertEqual(should_have_loc, line_entry != None and line_entry.IsValid(), "Loc line entry is valid")
+ if should_have_loc:
+ self.assertEqual(line_entry.GetLine(), 4)
+ self.assertEqual(line_entry.GetFileSpec().GetFilename(), self.main_source_file.GetFilename())
+ self.dbg.DeleteTarget(target)
+ shutil.rmtree(self.tmp_dir)
+
+ def config_test(self, local_files, debuginfo = None, executable = None):
+ """
+ Set up a test with local_files[] copied to a different location
+ so that we control which files are, or are not, found in the file system.
+ Also, create a stand-alone file-system 'hosted' debuginfod server with the
+ provided debuginfo and executable files (if they exist)
+
+ Make the filesystem look like:
+
+ /tmp/<tmpdir>/test/[local_files]
+
+ /tmp/<tmpdir>/cache (for lldb to use as a temp cache)
+
+ /tmp/<tmpdir>/buildid/<uuid>/executable -> <executable>
+ /tmp/<tmpdir>/buildid/<uuid>/debuginfo -> <debuginfo>
+ Returns the /tmp/<tmpdir> path
+ """
+
+ self.build()
+
+ uuid = getUUID(self.getBuildArtifact("a.out.uuid"))
+
+ self.main_source_file = lldb.SBFileSpec("main.c")
+ self.tmp_dir = tempfile.mkdtemp()
+ test_dir = os.path.join(self.tmp_dir, "test")
+ os.makedirs(test_dir)
+
+ self.aout = ""
+ # Copy the files used by the test:
+ for f in local_files:
+ shutil.copy(self.getBuildArtifact(f), test_dir)
+ # The first item is the binary to be used for the test
+ if (self.aout == ""):
+ self.aout = os.path.join(test_dir, f)
+
+ use_debuginfod = debuginfo != None or executable != None
+
+ # Populated the 'file://... mocked' Debuginfod server:
+ if use_debuginfod:
+ os.makedirs(os.path.join(self.tmp_dir, "cache"))
+ uuid_dir = os.path.join(self.tmp_dir, "buildid", uuid)
+ os.makedirs(uuid_dir)
+ if debuginfo:
+ shutil.copy(self.getBuildArtifact(debuginfo), os.path.join(uuid_dir, "debuginfo"))
+ if executable:
+ shutil.copy(self.getBuildArtifact(executable), os.path.join(uuid_dir, "executable"))
+
+ # Configure LLDB for the test:
+ self.runCmd("settings set symbols.enable-external-lookup %s" % str(use_debuginfod).lower())
+ self.runCmd("settings clear plugin.symbol-locator.debuginfod.server-urls")
+ if use_debuginfod:
+ self.runCmd("settings set plugin.symbol-locator.debuginfod.cache-path %s/cache" % self.tmp_dir)
+ self.runCmd("settings insert-before plugin.symbol-locator.debuginfod.server-urls 0 file://%s" % self.tmp_dir)
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..89e00378fa2aa9
--- /dev/null
+++ b/lldb/test/API/debuginfod/SplitDWARF/Makefile
@@ -0,0 +1,29 @@
+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_DWO := YES
+MERGE_DWOS := 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
new file mode 100644
index 00000000000000..638b4aa7207fb9
--- /dev/null
+++ b/lldb/test/API/debuginfod/SplitDWARF/TestDebuginfodDWP.py
@@ -0,0 +1,169 @@
+"""
+Test support for the DebugInfoD network symbol acquisition protocol.
+"""
+import os
+import shutil
+import tempfile
+import struct
+
+import lldb
+from lldbsuite.test.decorators import *
+import lldbsuite.test.lldbutil as lldbutil
+from lldbsuite.test.lldbtest import *
+
+
+def getUUID(aoutuuid):
+ """
+ Pull the 20 byte UUID out of the .note.gnu.build-id section that was dumped
+ to a file already, as part of the build.
+ """
+ with open(aoutuuid, "rb") as f:
+ data = f.read(36)
+ if len(data) != 36:
+ return None
+ header = struct.unpack_from("<4I", data)
+ if len(header) != 4:
+ return None
+ # 4 element 'prefix', 20 bytes of uuid, 3 byte long string: 'GNU':
+ if header[0] != 4 or header[1] != 20 or header[2] != 3 or header[3] != 0x554e47:
+ return None
+ return data[16:].hex()
+
+"""
+Test support for the DebugInfoD network symbol acquisition protocol.
+This file is for split-dwarf (dwp) scenarios.
+
+1 - A split binary target with it's corresponding DWP file
+2 - A stripped, split binary target with an unstripped binary and a DWP file
+3 - A stripped, split binary target with an --only-keep-debug symbols file and a DWP file
+"""
+@skipUnlessPlatform(["linux", "freebsd"])
+class DebugInfodDWPTests(TestBase):
+ # No need to try every flavor of debug inf.
+ NO_DEBUG_INFO_TESTCASE = True
+
+ def test_normal_stripped(self):
+ """
+ Validate behavior with a stripped binary, no symbols or symbol locator.
+ """
+ self.config_test(["a.out"])
+ self.try_breakpoint(False)
+
+ def test_normal_stripped_split_with_dwp(self):
+ """
+ Validate behavior with symbols, but no symbol locator.
+ """
+ self.config_test(["a.out", "a.out.debug", "a.out.dwp"])
+ self.try_breakpoint(True)
+
+ def test_normal_stripped_only_dwp(self):
+ """
+ Validate behavior *with* dwp symbols only, but missing other symbols,
+ but no symbol locator. This shouldn't work: without the other symbols
+ DWO's appear mostly useless.
+ """
+ self.config_test(["a.out", "a.out.dwp"])
+ self.try_breakpoint(False)
+
+ def test_debuginfod_dwp_from_service(self):
+ """
+ Test behavior with the unstripped binary, and DWP from the service.
+ """
+ self.config_test(["a.out.debug"], "a.out.dwp")
+ self.try_breakpoint(True)
+
+ def test_debuginfod_both_symfiles_from_service(self):
+ """
+ Test behavior with a stripped binary, with the unstrippe...
[truncated]
|
kevinfrei
force-pushed
the
debuginfod-issues
branch
from
5cd4c52
to
b97ed48
Compare
|
@JDevlieghere ping :) |
Summary: While adding tests for DebugInfoD integration, there were a couple issues that came up: DWP from /debuginfo for a stripped binary needs to return symbols from /executable Test Plan: Added some API tests Reviewers: gclayton, hyubo Subscribers: Tasks: T179375937 Tags: debuginfod Differential Revision: https://phabricator.intern.facebook.com/D54953389
kevinfrei
force-pushed
the
debuginfod-issues
branch
from
031a234
to
ba22dea
Compare
clayborg
reviewed
Mar 21, 2024
clayborg
approved these changes
Mar 21, 2024
|
I am seeing some errors related to this PR in our builds. Just want to confirm if the fix forward you have up for review will fix it. Otherwise, could you revert this change? Thanks! |
clayborg
pushed a commit
that referenced
this pull request
Mar 22, 2024
@GeorgeHuyubo noticed an unchecked shared pointer result in #85693. This is the fix for that issue. Co-authored-by: Kevin Frei <[email protected]>
|
both commits from this PR broke LLDB buildbots on Arm and AArch64 Linux. I will be reverting Kindly remerge after fixing issues. Thanks! |
omjavaid
added a commit
that referenced
this pull request
Mar 22, 2024
This reverts commit 6d939a6. This broke following LLDB bots: https://lab.llvm.org/buildbot/#/builders/96/builds/54867 https://lab.llvm.org/buildbot/#/builders/17/builds/50824
chencha3
pushed a commit
to chencha3/llvm-project
that referenced
this pull request
Mar 23, 2024
Finally getting back to Debuginfod tests: I've migrated the tests in my [earlier PR](llvm#79181) from shell to API (at @JDevlieghere's suggestion) and addressed a couple issues that came about during testing. The tests first test the "normal" situation (no DebugInfoD involvement, just normal debug files sitting around), then the "no debug info" situation (to make sure the test is seeing failure properly), then it tests to validate that when Debuginfod returns the symbols, things work properly. This is duplicated for DWP/split-dwarf scenarios. --------- Co-authored-by: Kevin Frei <[email protected]>
kevinfrei
added a commit
to kevinfrei/llvm
that referenced
this pull request
Apr 30, 2024
This reverts commit 7fc2fbb.
clayborg
pushed a commit
that referenced
this pull request
May 2, 2024
I'm taking yet another swing at getting these tests going, on the hypothesis that the problems with buildbots & whatnot are because they're not configured with CURL support, which I've confirmed would cause the previous tests to fail. (I have no access to an ARM64 linux system, but I did repro the failure on MacOS configured without CURL support) So, the only difference between this diff and [previous](#85693) [diffs](#87676) that have already been approved is that I've added a condition to the tests to only run if Debuginfod capabilities should be built into the binary. I had done this for these tests when they were [Shell tests](#79181) and not API tests, but I couldn't find a direct analog in any API test, so I used the "plugins" model used by the intel-pt tests as well. --------- Co-authored-by: Kevin Frei <[email protected]>
kevinfrei
added a commit
to kevinfrei/llvm
that referenced
this pull request
May 17, 2024
This reverts commit 7fc2fbb.
clayborg
pushed a commit
that referenced
this pull request
May 22, 2024
Here we go with attempt number five. Again, no changes to the LLDB code diff, which has been reviewed several times. For the tests, I added a `@skipIfCurlSupportMissing` annotation so that the Debuginfod mocked server stuff won't run, and I also disabled non-Linux/FreeBSD hosts altogether, as they fail for platform reasons on macOS and Windows. In addition, I updated the process for extracting the GNU BuildID to no create a target, per some feedback on the previous diff. For reference, previous PR's (landed, backed out after the fact for various reasons) #90622, #87676, #86812, #85693 --------- Co-authored-by: Kevin Frei <[email protected]>
kevinfrei
added a commit
to kevinfrei/llvm
that referenced
this pull request
Jun 26, 2024
This reverts commit 7fc2fbb.
kevinfrei
added a commit
to kevinfrei/llvm
that referenced
this pull request
Jun 27, 2024
This reverts commit 7fc2fbb.
kevinfrei
added a commit
to kevinfrei/llvm
that referenced
this pull request
Jun 28, 2024
This reverts commit 7fc2fbb.
kevinfrei
added a commit
to kevinfrei/llvm
that referenced
this pull request
Jul 2, 2024
This reverts commit 7fc2fbb.
kevinfrei
added a commit
to kevinfrei/llvm
that referenced
this pull request
Jul 2, 2024
This reverts commit 7fc2fbb.
clayborg
pushed a commit
that referenced
this pull request
Jul 8, 2024
This is the same diff I've put up at many times before. I've been trying to add some brand new functionality to the LLDB test infrastucture (create split-dwarf files!), and we all know that no good deed goes unpunished. The last attempt was reverted because it didn't work on the Fuchsia build. There are no code differences between this and [the](#90622) [previous](#87676) [four](#86812) [diffs](#85693) landed & reverted (due to testing infra failures). The only change in this one is the way `dwp` is being identified in `Makefile.rules`. Thanks to @petrhosek for helping me figure out how the fuchsia builders are configured. I now prefer to use llvm-dwp and fall back to gnu's dwp if the former isn't found. Hopefully this will work everywhere it needs to.
kevinfrei
added a commit
to kevinfrei/llvm
that referenced
this pull request
Jul 10, 2024
This reverts commit 7fc2fbb.
JDevlieghere
pushed a commit
that referenced
this pull request
Jul 10, 2024
This is all the tests and fixes I've had percolating since my first attempt at this in January. After 6 months of trying, I've given up on adding the ability to test DWP files in LLDB API tests. I've left both the tests (disabled) and the changes to Makefile.rules in place, in the hopes that someone who can configure the build bots will be able to enable the tests once a non-borked dwp tool is widely available. Other than disabling the DWP tests, this continues to be the same diff that I've tried to land and [not](#90622) [revert](#87676) [five](#86812) [times](#85693) [before](#96802). There are a couple of fixes that the testing exposed, and I've abandoned the DWP tests because I want to get those fixes finally upstreamed, as without them DebugInfoD is less useful.
aaryanshukla
pushed a commit
to aaryanshukla/llvm-project
that referenced
this pull request
Jul 14, 2024
This is the same diff I've put up at many times before. I've been trying to add some brand new functionality to the LLDB test infrastucture (create split-dwarf files!), and we all know that no good deed goes unpunished. The last attempt was reverted because it didn't work on the Fuchsia build. There are no code differences between this and [the](llvm#90622) [previous](llvm#87676) [four](llvm#86812) [diffs](llvm#85693) landed & reverted (due to testing infra failures). The only change in this one is the way `dwp` is being identified in `Makefile.rules`. Thanks to @petrhosek for helping me figure out how the fuchsia builders are configured. I now prefer to use llvm-dwp and fall back to gnu's dwp if the former isn't found. Hopefully this will work everywhere it needs to.
aaryanshukla
pushed a commit
to aaryanshukla/llvm-project
that referenced
this pull request
Jul 14, 2024
…#98344) This is all the tests and fixes I've had percolating since my first attempt at this in January. After 6 months of trying, I've given up on adding the ability to test DWP files in LLDB API tests. I've left both the tests (disabled) and the changes to Makefile.rules in place, in the hopes that someone who can configure the build bots will be able to enable the tests once a non-borked dwp tool is widely available. Other than disabling the DWP tests, this continues to be the same diff that I've tried to land and [not](llvm#90622) [revert](llvm#87676) [five](llvm#86812) [times](llvm#85693) [before](llvm#96802). There are a couple of fixes that the testing exposed, and I've abandoned the DWP tests because I want to get those fixes finally upstreamed, as without them DebugInfoD is less useful.
kevinfrei
added a commit
to kevinfrei/llvm
that referenced
this pull request
Jul 16, 2024
This reverts commit 7fc2fbb.
kevinfrei
added a commit
to kevinfrei/llvm
that referenced
this pull request
Jul 17, 2024
Reapply "DebugInfoD tests + fixing issues exposed by tests (llvm#85693)" This reverts commit 7fc2fbb. Switched to using LLDB to get UUID at @clayborg's suggestion Disable tests on non-x86 Linux platforms, as they appear to fail on AArch64/Arm buildbots Moved the @skipIf to each test, off of the class itself Added CURL dependency to lit configuration 'stuff' Fixed comments in the tests Fix stupid formatter issue Updated to respond to (very late) code review feedback Fixed the script to acquire the UUID without creating a target Updated tests to not run on Mac/Windows Added LLVM_ENABLE_CURL to the config.h.cmake to work in test's @skipIf thing Attempting to prefer llvm-dwp over gnu's dwp Disabled the DWP tests Missed disabling the baseline test for DWP stuff
kevinfrei
added a commit
to kevinfrei/llvm
that referenced
this pull request
Jul 17, 2024
This reverts commit 7fc2fbb.
kevinfrei
added a commit
to kevinfrei/llvm
that referenced
this pull request
Jul 19, 2024
Reapply "DebugInfoD tests + fixing issues exposed by tests (llvm#85693)" This reverts commit 7fc2fbb. Switched to using LLDB to get UUID at @clayborg's suggestion Disable tests on non-x86 Linux platforms, as they appear to fail on AArch64/Arm buildbots Moved the @skipIf to each test, off of the class itself Added CURL dependency to lit configuration 'stuff' Fixed comments in the tests Fix stupid formatter issue Updated to respond to (very late) code review feedback Fixed the script to acquire the UUID without creating a target Updated tests to not run on Mac/Windows Added LLVM_ENABLE_CURL to the config.h.cmake to work in test's @skipIf thing Attempting to prefer llvm-dwp over gnu's dwp Disabled the DWP tests Missed disabling the baseline test for DWP stuff
kevinfrei
added a commit
to kevinfrei/llvm
that referenced
this pull request
Jul 19, 2024
Reapply "DebugInfoD tests + fixing issues exposed by tests (llvm#85693)" This reverts commit 7fc2fbb. Switched to using LLDB to get UUID at @clayborg's suggestion Disable tests on non-x86 Linux platforms, as they appear to fail on AArch64/Arm buildbots Moved the @skipIf to each test, off of the class itself Added CURL dependency to lit configuration 'stuff' Fixed comments in the tests Fix stupid formatter issue Updated to respond to (very late) code review feedback Fixed the script to acquire the UUID without creating a target Updated tests to not run on Mac/Windows Added LLVM_ENABLE_CURL to the config.h.cmake to work in test's @skipIf thing Attempting to prefer llvm-dwp over gnu's dwp Disabled the DWP tests Missed disabling the baseline test for DWP stuff
kevinfrei
added a commit
to kevinfrei/llvm
that referenced
this pull request
Sep 9, 2024
This reverts commit 7fc2fbb.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Finally getting back to Debuginfod tests:
I've migrated the tests in my earlier PR from shell to API (at @JDevlieghere's suggestion) and addressed a couple issues that came about during testing.
The tests first test the "normal" situation (no DebugInfoD involvement, just normal debug files sitting around), then the "no debug info" situation (to make sure the test is seeing failure properly), then it tests to validate that when Debuginfod returns the symbols, things work properly. This is duplicated for DWP/split-dwarf scenarios.