This fixes https://github.com/llvm/llvm-project/issues/56125 and
https://github.com/vadimcn/codelldb/issues/666, as well as the
downstream issue in our binary ninja debugger:
https://github.com/Vector35/debugger/issues/535
Basically, lldb does not claim to support the `swbreak` packet so the
gdbserver would not use it. As a result, the gdbserver always sends the
unmodified program counter value which, on systems like x86, causes the
program counter to be off-by-one (or otherwise wrong). For reference,
the lldb-server always sends the modified program counter value so it
works perfectly with lldb.
https://sourceware.org/gdb/current/onlinedocs/gdb.html/Stop-Reply-Packets.html#swbreak-stop-reason
No new code is added to add support `swbreak`, since the way lldb works
already expects the remote to have adjusted the program counter. The
change just lets the gdbserver know that lldb supports it, so that it
will send the adjusted program counter.
To test this PR, you can use lldb to connect to a gdbserver running on
e.g., Ubuntu 22.04, and see the program counter is off-by-one without
the patch. With the patch, things work as expected
This patch is a follow-up to #97263 that fix ambigous abbreviated
command resolution.
When multiple commands are resolved, instead of failing to pick a
command to
run, this patch changes to resolution logic to check if there is a
single
alias match and if so, it will run the alias instead of the other
matches.
This has as a side-effect that we don't need to make aliases for every
substring of aliases to support abbrivated alias resolution.
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
This patch loosen the parsing requirement to allow parsing not only
JSON dictionaries but also valid JSON type (integer, float, string,
bool, array, null).
Signed-off-by: Med Ismail Bennani <ismail@bennani.ma>
@walter-erquinigo found the the [PR with testing and a fix for
DebugInfoD](https://github.com/llvm/llvm-project/pull/98344) caused an
issue when working with stripped binaries.
The issue is that when you're working with split-dwarf, there are *3*
possible files: The stripped binary the user is debugging, the
"only-keep-debug" *or* unstripped binary, plus the `.dwp` file. The
debuginfod plugin should provide the unstripped/OKD binary. However, if
the debuginfod plugin fails, the default symbol locator plugin will just
return the stripped binary, which doesn't help. So, to address that, the
SymbolVendorELF code checks to see if the SymbolLocator's
ExecutableObjectFile request returned the same file, and bails if that's
the case. You can see the specific diff as the second commit in the PR.
I'm investigating adding a test: I can't quite get a simple repro, and
I'm unwilling to make any additional changes to Makefile.rules to this
diff, for Pavlovian reasons.
This PR introduces a new `ThreadPlanSingleThreadTimeout` that will be
used to address potential deadlock during single-thread stepping.
While debugging a target with a non-trivial number of threads (around
5000 threads in one example target), we noticed that a simple step over
can take as long as 10 seconds. Enabling single-thread stepping mode
significantly reduces the stepping time to around 3 seconds. However,
this can introduce deadlock if we try to step over a method that depends
on other threads to release a lock.
To address this issue, we introduce a new
`ThreadPlanSingleThreadTimeout` that can be controlled by the
`target.process.thread.single-thread-plan-timeout` setting during
single-thread stepping mode. The concept involves counting the elapsed
time since the last internal stop to detect overall stepping progress.
Once a timeout occurs, we assume the target is not making progress due
to a potential deadlock, as mentioned above. We then send a new async
interrupt, resume all threads, and `ThreadPlanSingleThreadTimeout`
completes its task.
To support this design, the major changes made in this PR are:
1. `ThreadPlanSingleThreadTimeout` is popped during every internal stop
and reset (re-pushed) to the top of the stack (as a leaf node) during
resume. This is achieved by always returning `true` from
`ThreadPlanSingleThreadTimeout::DoPlanExplainsStop()` and
`ThreadPlanSingleThreadTimeout::MischiefManaged()`.
2. A new thread-specific async interrupt stop is introduced, which can
be detected/consumed by `ThreadPlanSingleThreadTimeout`.
3. The clearing of branch breakpoints in the range thread plan has been
moved from `DoPlanExplainsStop()` to `ShouldStop()`, as it is not
guaranteed that it will be called.
The detailed design is discussed in the RFC below:
[https://discourse.llvm.org/t/improve-single-thread-stepping/74599](https://discourse.llvm.org/t/improve-single-thread-stepping/74599)
---------
Co-authored-by: jeffreytan81 <jeffreytan@fb.com>
This makes use of the changes introduced in D134604, in order to
instantiate alias templates witn a final sugared substitution.
This comes at no additional relevant cost.
Since we don't track / unique them in specializations, we wouldn't be
able to resugar them later anyway.
Differential Revision: https://reviews.llvm.org/D136565
In #100443, Mach-o and Minidump now only call process API's that take a
`SaveCoreOption` as the container for the style and information if a
thread should be included in the core or not. This introduced a bug
where in subsequent method calls we were not honoring the defaults of
both implementations.
~~To solve this I have made a copy of each SaveCoreOptions that is
mutable by the respective plugin. Originally I wanted to leave the
SaveCoreOptions as non const so these default value mutations could be
shown back to the user. Changing that behavior is outside of the scope
of this bugfix, but is context for why we are making a copy.~~
Removed const on the savecoreoptions so defaults can be inspected by the
user
CC: @Michael137
In #98403 I enabled the SBSaveCoreOptions object, which allows users via
the scripting API to define what they want saved into their core file.
As the first option I've added a threadlist, so users can scan and
identify which threads and corresponding stacks they want to save.
In order to support this, I had to add a new method to `Process.h` on
how we identify which threads are to be saved, and I had to change the
book keeping in minidump to ensure we don't double save the stacks.
Important to @jasonmolenda I also changed the MachO coredump to accept
these new APIs.
This is used by various system routines (the capabilities checker and
dyld to name a few) to add extra color to an abort. This patch adds a
frame recognizer so people can easily see the details, and also adds the
information to the ExtendedCrashInformation dictionary.
I also had to rework how the dictionary is held; previously it was
created on demand, but that was inconvenient since it meant all the
entries had to be produced at that same time. That didn't work for the
recognizer.
This PR splits the test assertion that verifies we're on the correct
line and have the correct value of `val` to make the error message more
clear. At present it just shows `Assertion error: True != False`
Co-authored-by: kendal <kendal@thebrowser.company>
This introduces a `target.object-map` which allows us to remap module
locations, much in the same way as source mapping works today. This is
useful, for instance, when debugging coredumps, so we can replace some
of the locations where LLDB attempts to load shared libraries and
executables from, without having to setup an entire sysroot.
This test has been flaky lately
(https://github.com/llvm/llvm-project/issues/101162) and I disabled it
everywhere initially.
I found that it always uses "x86_64" for the program architecture so the
test was "passing" elsewhere but I don't think it was meant to. So I
have added a define to pass on the host's architecture when compiling.
This makes it work on AArch64 as well.
While I'm here I've fixed the uint64_t formatting warnings by using the
defined formats that'll work everywhere.
In addition, I found that the function names include "()" on Linux, so
now we check for "foo" or "foo()".
The test cpp file has never been, or was only partially formatted so
I've not formatted the changes, just kept to the local style.
I've removed the Linux skip to see if any of this helps the timeouts,
and to verify the build command changes. If the timeouts come back I'll
disable it again.
Depends on https://github.com/llvm/llvm-project/pull/100674
Currently, we treat VLAs declared as `int[]` and `int[0]` identically.
I.e., we create them as `IncompleteArrayType`s. However, the
`DW_AT_count` for `int[0]` *does* exist, and is retrievable without an
execution context. This patch decouples the notion of "has 0 elements"
from "has no known `DW_AT_count`".
This aligns with how Clang represents `int[0]` in the AST (it treats it
as a `ConstantArrayType` of 0 size).
This issue was surfaced when adapting LLDB to
https://github.com/llvm/llvm-project/issues/93069. There, the
`__compressed_pair_padding` type has a `char[0]` member. If we
previously got the `__compressed_pair_padding` out of a module (where
clang represents `char[0]` as a `ConstantArrayType`), and try to merge
the AST with one we got from DWARF (where LLDB used to represent
`char[0]` as an `IncompleteArrayType`), the AST structural equivalence
check fails, resulting in silent ASTImporter failures. This manifested
in a failure in `TestNonModuleTypeSeparation.py`.
**Implementation**
1. Adjust `ParseChildArrayInfo` to store the element counts of each VLA
dimension as an `optional<uint64_t>`, instead of a regular `uint64_t`.
So when we pass this down to `CreateArrayType`, we have a better
heuristic for what is an `IncompleteArrayType`.
2. In `TypeSystemClang::GetBitSize`, if we encounter a
`ConstantArrayType` simply return the size that it was created with. If
we couldn't determine the authoritative bound from DWARF during parsing,
we would've created an `IncompleteArrayType`. This ensures that
`GetBitSize` on arrays with `DW_AT_count 0x0` returns `0` (whereas
previously it would've returned a `std::nullopt`, causing that
`FieldDecl` to just get dropped during printing)
It has been markad as XFAIL here
de2ddc8f3146bd87152ea86b533541039541efe1, but I haven't found the reason
for that, and apparently, it passes against the Linux target.
HostProcessWindows::Terminate() correctly uses m_process which type is
process_t (HANDLE) to call ::TerminateProcess(). But Host::Kill() uses a
cast from pid, which is wrong.
This patch fixes#51793
This reverts commit 05f0e86cc895181b3d2210458c78938f83353002.
The debuginfo dexter tests are failing, probably because the way
stepping over breakpoints has changed with my patches. And there
are two API tests fails on the ubuntu-arm (32-bit) bot. I'll need
to investigate both of these, neither has an obvious failure reason.
lldb today has two rules: When a thread stops at a BreakpointSite, we
set the thread's StopReason to be "breakpoint hit" (regardless if we've
actually hit the breakpoint, or if we've merely stopped *at* the
breakpoint instruction/point and haven't tripped it yet). And second,
when resuming a process, any thread sitting at a BreakpointSite is
silently stepped over the BreakpointSite -- because we've already
flagged the breakpoint hit when we stopped there originally.
In this patch, I change lldb to only set a thread's stop reason to
breakpoint-hit when we've actually executed the instruction/triggered
the breakpoint. When we resume, we only silently step past a
BreakpointSite that we've registered as hit. We preserve this state
across inferior function calls that the user may do while stopped, etc.
Also, when a user adds a new breakpoint at $pc while stopped, or changes
$pc to be the address of a BreakpointSite, we will silently step past
that breakpoint when the process resumes. This is purely a UX call, I
don't think there's any person who wants to set a breakpoint at $pc and
then hit it immediately on resuming.
One non-intuitive UX from this change, but I'm convinced it is
necessary: If you're stopped at a BreakpointSite that has not yet
executed, you `stepi`, you will hit the breakpoint and the pc will not
yet advance. This thread has not completed its stepi, and the thread
plan is still on the stack. If you then `continue` the thread, lldb will
now stop and say, "instruction step completed", one instruction past the
BreakpointSite. You can continue a second time to resume execution. I
discussed this with Jim, and trying to paper over this behavior will
lead to more complicated scenarios behaving non-intuitively. And mostly
it's the testsuite that was trying to instruction step past a breakpoint
and getting thrown off -- and I changed those tests to expect the new
behavior.
The bugs driving this change are all from lldb dropping the real stop
reason for a thread and setting it to breakpoint-hit when that was not
the case. Jim hit one where we have an aarch64 watchpoint that triggers
one instruction before a BreakpointSite. On this arch we are notified of
the watchpoint hit after the instruction has been unrolled -- we disable
the watchpoint, instruction step, re-enable the watchpoint and collect
the new value. But now we're on a BreakpointSite so the watchpoint-hit
stop reason is lost.
Another was reported by ZequanWu in
https://discourse.llvm.org/t/lldb-unable-to-break-at-start/78282 we
attach to/launch a process with the pc at a BreakpointSite and
misbehave. Caroline Tice mentioned it is also a problem they've had with
putting a breakpoint on _dl_debug_state.
The change to each Process plugin that does execution control is that
1. If we've stopped at a BreakpointSite that has not been executed yet,
we will call Thread::SetThreadStoppedAtUnexecutedBP(pc) to record
that. When the thread resumes, if the pc is still at the same site, we
will continue, hit the breakpoint, and stop again.
2. When we've actually hit a breakpoint (enabled for this thread or not),
the Process plugin should call Thread::SetThreadHitBreakpointSite().
When we go to resume the thread, we will push a step-over-breakpoint
ThreadPlan before resuming.
The biggest set of changes is to StopInfoMachException where we
translate a Mach Exception into a stop reason. The Mach exception codes
differ in a few places depending on the target (unambiguously), and I
didn't want to duplicate the new code for each target so I've tested
what mach exceptions we get for each action on each target, and
reorganized StopInfoMachException::CreateStopReasonWithMachException to
document these possible values, and handle them without specializing
based on the target arch.
rdar://123942164
In #98403 some of the tests were transitioned to the new
`SBProcess::SaveCore(SBSaveCoreOptions)` API, but were not detected in
testing. This patch addresses that.
This PR adds `SBSaveCoreOptions`, which is a container class for options
when LLDB is taking coredumps. For this first iteration this container
just keeps parity with the extant API of `file, style, plugin`. In the
future this options object can be extended to allow users to take a
subset of their core dumps.
Currently, if we execute 'process load' with remote debugging, it uses
the host's path delimiter to look up files on a target machine. If we
run remote debugging of Linux target on Windows and execute "process
load C:\foo\a.so", lldb-server tries to load \foo\a.so instead of
/foo/a.so on the remote.
It affects several API tests.
This commit fixes that error. Also, it contains minor fixes for
TestLoadUnload.py for testing on Windows host and Linux target.
The strings this test is using seem to consistently fail to match
against the expected values when built & run targeting Windows amd64.
This PR updates them to the expected values.
To fix the test and avoid over-specifying for a specific platform, use
`assertIn(<target-substring>,...)` to see if we've got the correct
target label instead of comparing the demangler output for an exact
string match.
---------
Co-authored-by: kendal <kendal@thebrowser.company>
This patch adds an interface GetLastInstrSize to get information about
the size of last tried to be decoded instruction and uses it to set
software breakpoint if the memory can be decoded as instruction.
RISC-V architecture instruction format specifies the length of
instruction in first bits, so we can set a breakpoint for these cases.
This is needed as RISCV have a lot of extensions, that are not supported
by `EmulateInstructionRISCV`.
This is motivated by the upcoming refactor of libc++'s
`__compressed_pair` in https://github.com/llvm/llvm-project/pull/76756
As this will require changes to numerous LLDB libc++ data-formatters
(see early draft https://github.com/llvm/llvm-project/pull/96538), it
would be nice to have a test-suite that will actually exercise both the
old and new layout. We have a matrix bot that tests old versions of
Clang (but currently those only date back to Clang-15). Having them in
the test-suite will give us quicker signal on what broke.
We have an existing test that exercises various layouts of `std::string`
over time in `TestDataFormatterLibcxxStringSimulator.py`, but that's the
only STL type we have it for. This patch proposes a new
`libcxx-simulators` directory which will take the same approach for all
the STL types that we can feasibly support in this way (as @labath
points out, for some types this might just not be possible due to their
implementation complexity). Nonetheless, it'd be great to have a record
of how the layout of libc++ types changed over time.
Some related discussion:
*
https://github.com/llvm/llvm-project/pull/97568#issuecomment-2213426804
This reverts commit 2fa1220a37a3f55b76a29803d8333b3a3937d53a.
This reverts commit b9496a74eb4029629ca2e440c5441614e766f773.
The patch #98344 causes a crash in LLDB when parsing some files like `numpy.libs/libgfortran-daac5196.so.5.0.0` on graviton (you can download it in https://drive.google.com/file/d/12ygLjJwWpzdYsrzBPp1JGiFHxcgM0-XY/view?usp=drive_link if you want to troubleshoot yourself).
The assert that is hit is the following:
```
llvm-project/lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp:2452: std::pair<unsigned int, std::map<long unsigned int, lldb_private::AddressClass> > ObjectFileELF::ParseSymbolTable(lldb_private::Symtab*, lldb::user_id_t, lldb_private::Section*): Assertion `strtab->GetObjectFile() == this' failed.
[383588:383636:20240716,025305.572639:ERROR crashpad_client_linux.cc:780] Crashpad isn't enabled
```
This object file doesn't have apparently a strings table but LLDB still tries to process it due to the code that is being reverted.
Frequently, while troubleshooting user's debugging issues in VScode, we
would like to know lldb version so that we can confirm if certain
patch/feature is in or not.
This PR adds version string into `initialize` response so that telemetry
can track it.
---------
Co-authored-by: jeffreytan81 <jeffreytan@fb.com>
A new file was added to the python_api/events test, but I forgot to
git add it before making the PR. The commit was:
44d9692e6a657ec46e98e4912ac56417da67cfee
This fixes a bug where Process events were being delivered to secondary
listeners when the Private state thread listener was processing the
event. That meant the secondary listener could get an event before the
Primary listener did. That in turn meant the state when the secondary
listener got the event wasn't right yet. Plus it meant that the
secondary listener saw more events than the primary (not all events get
forwarded from the private to the public Process listener.)
This bug became much more evident when we had a stop hook that did some
work, since that delays the Primary listener event delivery. So I also
added a stop-hook to the test, and put a little delay in as well.
This test sets a breakpoint on malloc, as a way to stop early in
dyld's setting up code, before the system libraries are initialized
so we can confirm that we don't fetch the Objective-C class table
before it's initialized.
In macOS 15 (macOS Sonoma), dyld doesn't call malloc any longer,
so this heuristic/trick isn't working. It does call other things
called *alloc though, so I'm changing this to use a regex breakpoint
on that, to keep the test working.
This should disable the failing test on the ubuntu build bots
@JDevlieghere (I forgot to disable the 'baseline' test, as it tests the
debugger's basic handling of DWP files, but again, the API
infrastructure doesn't quite support DWP generation)
https://github.com/llvm/llvm-project/pull/98344#issuecomment-2221000566
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](https://github.com/llvm/llvm-project/pull/90622)
[revert](https://github.com/llvm/llvm-project/pull/87676)
[five](https://github.com/llvm/llvm-project/pull/86812)
[times](https://github.com/llvm/llvm-project/pull/85693)
[before](https://github.com/llvm/llvm-project/pull/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.
On Windows the function name is the full prototype including
the calling convention, all we care about is that the last part
is correct.
This also reverts the xfail added by 07b3e2c0c68b93a3d4d89426dc7fd14cc31ca6be.
this test is occasionally (~3%) failing on an emulator target. The value
used by the test (one second) is quite aggressive given that we set the timeout
for a single gdb packet to 60 seconds.
Bumping it to five to resolve flakyness.
This improves the handling of `$` (dollar) characters in summary strings in the
following ways:
1. When a `$` is not followed by an open paren (`{`), it should be treated as a literal
character and preserved in the output. Previously, the dollar would be consumed by the
parser and not shown in the output.
2. When a `$` is the last character of a format string, this change eliminates the
infinite loop lldb would enter into.
rdar://131392446
Clang's v2 output appears to be missing a key DW_AT_type attribute,
and this causes the "max" of the unsigned enum to appear as -1 instead
of "max" aka 3.
```
(BitfieldStruct) $0 = {
signed_min = min
signed_other = -1
signed_max = max
unsigned_min = min
unsigned_other = 1
unsigned_max = -1
}
```
Test added by #96202.
In async mode, the test terminates sooner than it should
(`run_to_source_breakpoint` does not work in this mode), and then the
test crashes due to #76057. Most of the time, the test does not fail
because its already XFAILed, but the crash still registers as a failure.
Reverts llvm/llvm-project#96802
Attempt #5 fails. It's been 6 months. I despise Makefile.rules and have
no ability to even *detect* these failures without _landing_ a diff. In
the mean time, we have no testing for DWP files at all (and a regression
that was introduced, that I fix with this diff) so I'm going to just
remove some of the tests and try to land it again, but with less testing
I guess.
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](https://github.com/llvm/llvm-project/pull/90622)
[previous](https://github.com/llvm/llvm-project/pull/87676)
[four](https://github.com/llvm/llvm-project/pull/86812)
[diffs](https://github.com/llvm/llvm-project/pull/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.
Depends on https://github.com/llvm/llvm-project/pull/97687
Similar to https://github.com/llvm/llvm-project/pull/97579, this patch
simplifies the way in which we retrieve the key/value pair of a
`std::map` (in this case of the `std::map::iterator`).
We do this for the same reason: not only was the old logic hard to
follow, and encoded the libc++ layout in non-obvious ways, it was also
fragile to alignment miscalculations
(https://github.com/llvm/llvm-project/pull/97443); this would break once
the new layout of std::map landed as part of
https://github.com/llvm/llvm-project/issues/93069.
Instead, this patch simply casts the `__iter_pointer` to the
`__node_pointer` and uses a straightforward
`GetChildMemberWithName("__value_")` to get to the key/value we care
about.
We can eventually re-use the core-part of the `std::map` and
`std::map::iterator` formatters. But it will be an easier to change to
review once both simplifications landed.
