LLDB Debuginfod tests and a fix or two #90622
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
| 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() | ||
This file contains hidden or 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
This file contains hidden or 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
| 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 |
This file contains hidden or 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
| 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)) | ||
|
There was a problem hiding this comment. You don't actually need to create a target for this. Something like: ought to suffice. There was a problem hiding this comment. I think this check is being too strict (and could be the cause of the reported failures). There's no requirement that the build id be exactly 10 bytes long, and in fact different linkers will use different build id lengths (corresponding to different checksum algorithms). If all you wanted was to rule out lldb's fake crc-based UUIDs, then I suggest something like |
||
| module = target.GetModuleAtIndex(0) | ||
| uuid = module.GetUUIDString().replace("-", "").lower() | ||
| self.dbg.DeleteTarget(target) | ||
| return uuid if len(uuid) == 40 else None | ||
| except: | ||
| return None | ||
This file contains hidden or 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
| 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); } |
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something about this if block has broken
Expr/TestStringLiteralExpr.teston Arm 32 bit, a platform that has been sensitive to changes in this area in the past. The test is built without debug info and something happens when the test file comes through here. For ld.so and the libc it's all the same as before.Last time this happened it was because we lost the symbols that let us call
mmap, but those are in ld.so usually so I'm not sure what's the problem this time.This means we fall back the interpreter, which we didn't need to do before.
I've reverted this PR (327bfc9) and the follow up while I investigate locally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We try to allocate memory to JIT the expression and calling
mmapfails with a signal. In #68987 this was because we had lost the section info that told us whether to call it as Thumb or Arm, if we get that wrong it causes a SIGILL, so it could be the same thing again.I will look into it more next week and assuming I find a fix, reland the changes for you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@DavidSpickett if there's anything I can do to help, please ping me. Feel free to use my github handle at hotmail to ping me, as I'm going to be on a "doing nothing in nicer weather" vacation next week.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what I've figured out so far. It's like the previous issue I mentioned, except it's related to the program file, not
ld.so.If we look at the memory regions before this PR we see:
After we just have:
Which means we have lost, or never loaded, the section information for this program file. Which makes sense given that it's compiled without debug information. We'll ask a plugin for the debug info and none of those will have it, so we exit early.
Problem is, Arm has 2 execution modes. Arm and Thumb. To know how to break properly in different code we look at markers on the sections. These set the
AddressClassof the address value in LLDB to the right value.In this case, we need to call
mmap. This is in a Thumb section inld.so, and we have it's section information. We don't need to break here though, only set the control register (cpsr) bit to indicate we want to run in Thumb mode, then write the PC to point to this location.For the end of
mmap, we need to return somewhere. So lldb says, why not_start, it's something we can count on existing on Linux. So LLDB needs to set the link register to that address, then place a breakpoint on the address to catch the program just asmmapfinishes.Problem is, after this PR we don't have section information to tell us that this area is Thumb. This means that
Platform::GetSoftwareBreakpointTrapOpcodechooses an Arm breakpoint code. Now I don't know exactly what goes wrong there, I think in Thumb mode the program sees this encoding as some kind of backwards branch, and it ends up basically in the zero page and segfaults from there. This is why the call tommapfails and we fall back to the interpreter.So ideally we want to split the two concepts of 1. Section information and 2. Symbol file. So that we can load both without resetting the other. I'm going to look into that next.
And for some context, I think Arm is the only platform this section information is crucial. Potentially MIPS but also we dropped Linux MIPS support, and I don't know if those platforms mixed modes ever. So it's not surprising you didn't see this testing on more modern platforms.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#91585 fixes the underlying issue, if/when that's proven to work, I'll put this PR back in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That PR is in, so at least Arm Linux is happy for this to go back in.