Skip to content

Commit

Permalink
LLDB Debuginfod tests and a fix or two (#90622)
Browse files Browse the repository at this point in the history
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 <freik@meta.com>
  • Loading branch information
kevinfrei and kevinfrei committed May 2, 2024
1 parent e4bb663 commit 2d4acb0
Show file tree
Hide file tree
Showing 12 changed files with 519 additions and 17 deletions.
26 changes: 25 additions & 1 deletion lldb/packages/Python/lldbsuite/test/make/Makefile.rules
Original file line number Diff line number Diff line change
Expand Up @@ -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.
#----------------------------------------------------------------------
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -566,10 +577,17 @@ else
endif
else
ifeq "$(SPLIT_DEBUG_SYMBOLS)" "YES"
ifeq "$(SAVE_FULL_DEBUG_BINARY)" "YES"
cp "$(EXE)" "$(EXE).unstripped"
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


#----------------------------------------------------------------------
# Make the dylib
Expand Down Expand Up @@ -611,9 +629,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

#----------------------------------------------------------------------
Expand Down
38 changes: 25 additions & 13 deletions lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4378,26 +4378,38 @@ 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);
}
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) {
Expand Down
7 changes: 6 additions & 1 deletion lldb/source/Plugins/SymbolLocator/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -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)
29 changes: 27 additions & 2 deletions lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,24 @@ 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);
// 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 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);
}

// CreateInstance
//
// Platforms can register a callback to use when creating symbol vendors to
Expand Down Expand Up @@ -87,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;
Expand Down
19 changes: 19 additions & 0 deletions lldb/test/API/debuginfod/Normal/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
C_SOURCES := main.c

# For normal (non DWP) Debuginfod tests, we need:

# * The full binary: a.out.unstripped
# Produced by Makefile.rules with SAVE_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.debug)
# Produced below

SPLIT_DEBUG_SYMBOLS := YES
SAVE_FULL_DEBUG_BINARY := YES
GEN_GNU_BUILD_ID := YES

include Makefile.rules
183 changes: 183 additions & 0 deletions lldb/test/API/debuginfod/Normal/TestDebuginfod.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
import os
import shutil
import tempfile

import lldb
from lldbsuite.test.decorators import *
import lldbsuite.test.lldbutil as lldbutil
from lldbsuite.test.lldbtest import *


"""
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
"""


class DebugInfodTests(TestBase):
# No need to try every flavor of debug inf.
NO_DEBUG_INFO_TESTCASE = True

def setUp(self):
TestBase.setUp(self)
# Don't run these tests if we don't have Debuginfod support
if "Debuginfod" not in configuration.enabled_plugins:
self.skipTest("The Debuginfod SymbolLocator plugin is not enabled")

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.unstripped")
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.unstripped")
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 = self.getUUID("a.out")
if not uuid:
self.fail("Could not get UUID for a.out")
return
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
)

def getUUID(self, filename):
try:
target = self.dbg.CreateTarget(self.getBuildArtifact(filename))
module = target.GetModuleAtIndex(0)
uuid = module.GetUUIDString().replace("-", "").lower()
self.dbg.DeleteTarget(target)
return uuid if len(uuid) == 40 else None
except:
return None
7 changes: 7 additions & 0 deletions lldb/test/API/debuginfod/Normal/main.c
Original file line number Diff line number Diff line change
@@ -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); }
Loading

0 comments on commit 2d4acb0

Please sign in to comment.