When multiple threads are stopped at the same breakpoint, LLDB currently
steps each thread over the breakpoint one at a time. Each step requires
disabling the breakpoint, single-stepping one thread, and re-enabling
it, resulting in N disable/enable cycles and N individual vCont packets
for N threads. This is a common scenario for hot breakpoints in
multithreaded programs and scales poorly.
This patch batches the step-over so that all threads at the same
breakpoint site are stepped together in a single vCont packet, with the
breakpoint disabled once at the start and re-enabled once after the last
thread finishes.
At the top of WillResume, any leftover StepOverBreakpoint plans from a
previous cycle are popped with their re-enable side effect suppressed
via SetReenabledBreakpointSite, giving a clean slate.
SetupToStepOverBreakpointIfNeeded then creates fresh plans for all
threads that still need to step over a breakpoint, and these are grouped
by breakpoint address.
For groups with multiple threads, each plan is set to defer its
re-enable through SetDeferReenableBreakpointSite. Instead of re-enabling
the breakpoint directly when a plan completes, it calls
ThreadFinishedSteppingOverBreakpoint, which decrements a per-address
tracking count. The breakpoint is only re-enabled when the count reaches
zero.
All threads in the largest group are resumed together in a single
batched vCont packet. If some threads don't complete their step in one
cycle, the pop-and-recreate logic naturally re-batches the remaining
threads on the next WillResume call.
For 10 threads at the same breakpoint, this reduces the operation from
10 z0/Z0 pairs and 10 vCont packets to 1 z0 + 1 Z0 and a few
progressively smaller batched vCont packets.
EDIT:
Tried to merge this PR twice, the first time the test was flaky so we
had to revert. The second time, we broke 2 tests on windows machine:
https://lab.llvm.org/buildbot/#/builders/141/builds/15798
The tests that were failing were failing because the cleanup code in
`WillResume` was popping **ALL** `StepOverBreakpoint` plans, including
non-deferred ones from incomplete single-steps.
The issue was:
1) Multiple threads hit the same breakpoint. One thread's breakpoint
condition evaluates to false, so it needs to auto-continue.
2) A `StepOverBreakpoint` plan is created for that thread
(non-deferred).
3) On the next WillResume, the cleanup pops that non-deferred plan.
4) Now the `StopOthers` scan finds no thread with a StopOthers() plan,
so thread_to_run stays null.
5) The else branch runs, calling `SetupToStepOverBreakpointIfNeeded` on
**ALL** threads, including the thread that legitimately hit the
breakpoint with a true condition.
6) That thread gets a new `StepOverBreakpoint` plan pushed, which
overwrites its breakpoint stop reason with trace when the step
completes.
The error `trace (2) != breakpoint (3)` confirms this, the thread that
should have reported breakpoint as its stop reason instead reports
trace, because an unwanted `StepOverBreakpoint` plan was pushed on it
and completed.
The newly added code fixes it by only popping plans that have
`GetDeferReenableBreakpointSite() == true`
Co-authored-by: Bar Soloveychik <barsolo@fb.com>
Re-land https://github.com/llvm/llvm-project/pull/180101 since it was
reverted here https://github.com/llvm/llvm-project/pull/182431 because
of a flaky test. This PR include the modified test that should pass from
https://github.com/llvm/llvm-project/pull/182415 :
When multiple threads are stopped at the same breakpoint, LLDB currently
steps each thread over the breakpoint one at a time. Each step requires
disabling the breakpoint, single-stepping one thread, and re-enabling
it, resulting in N disable/enable cycles and N individual vCont packets
for N threads. This is a common scenario for hot breakpoints in
multithreaded programs and scales poorly.
This patch batches the step-over so that all threads at the same
breakpoint site are stepped together in a single vCont packet, with the
breakpoint disabled once at the start and re-enabled once after the last
thread finishes.
At the top of WillResume, any leftover StepOverBreakpoint plans from a
previous cycle are popped with their re-enable side effect suppressed
via SetReenabledBreakpointSite, giving a clean slate.
SetupToStepOverBreakpointIfNeeded then creates fresh plans for all
threads that still need to step over a breakpoint, and these are grouped
by breakpoint address.
For groups with multiple threads, each plan is set to defer its
re-enable through SetDeferReenableBreakpointSite. Instead of re-enabling
the breakpoint directly when a plan completes, it calls
ThreadFinishedSteppingOverBreakpoint, which decrements a per-address
tracking count. The breakpoint is only re-enabled when the count reaches
zero.
All threads in the largest group are resumed together in a single
batched vCont packet. If some threads don't complete their step in one
cycle, the pop-and-recreate logic naturally re-batches the remaining
threads on the next WillResume call.
For 10 threads at the same breakpoint, this reduces the operation from
10 z0/Z0 pairs and 10 vCont packets to 1 z0 + 1 Z0 and a few
progressively smaller batched vCont packets.
Co-authored-by: Bar Soloveychik <barsolo@fb.com>
Following up from
https://discourse.llvm.org/t/improving-performance-of-multiple-threads-stepping-over-the-same-breakpoint/89637
When multiple threads are stopped at the same breakpoint, LLDB currently
steps each thread over the breakpoint one at a time. Each step requires
disabling the breakpoint, single-stepping one thread, and re-enabling
it, resulting in N disable/enable cycles and N individual vCont packets
for N threads.
Now we batch the step-over so that all threads at the same breakpoint
site are stepped together in a single vCont packet, with the breakpoint
disabled once at the start and re-enabled once after the last thread
finishes.
When we hit `WillResume` any leftover `StepOverBreakpoint` plans from a
previous cycle are popped with their re-enable side effect suppressed
via `SetReenabledBreakpointSite`, giving a clean slate.
`SetupToStepOverBreakpointIfNeeded` then creates fresh plans for all
threads that still need to step over a breakpoint, and these are grouped
by breakpoint address.
For groups with multiple threads, each plan is set to defer its
re-enable through `SetDeferReenableBreakpointSite`. Instead of
re-enabling the breakpoint directly when a plan completes, it calls
`ThreadFinishedSteppingOverBreakpoint`, which decrements a tracking
count per address. The breakpoint is only re-enabled when the count
reaches zero.
All threads in the largest group are resumed together in a single
batched vCont packet. If some threads don't complete their step in one
cycle, the pop-and-recreate logic naturally re-batches the remaining
threads on the next WillResume call.
Claude AI assisted in syntex fixes and making the comments more
understandable.
---------
Co-authored-by: Bar Soloveychik <barsolo@fb.com>
This changes Python API tests to use a single build shared across all
test functions, instead of the previous default behavior of a separate
build dir for each test function.
This build behavior opt-out, tests can use the previous behavior of one
individual (unshared) build directory per test function, by setting
`SHARED_BUILD_TESTCASE` to False (in the test class).
The motivation is to make the test suite more efficient, by not
repeatedly building the same test source. When running tests on my macOS
machine, this reduces the time of `ninja check-lldb-api` by almost 60%
(sample numbers: from ~492s down to ~207s = 58%). Almost 5min time
saved.
Each test function still calls `self.build()`, but only the first call
will do a build, in the subsequent tests `make` will be a no-op because
the sources won't have changed.
Ran my python script from
https://github.com/llvm/llvm-project/pull/97043 over the repo again and
there were 2 duplicate test-cases that have been introduced since I last
did this.
Also one of the WASM classes had a duplicate method which I just
removed.
When a server is unable to allocate memory for the `_M` packet, it may
respond with an error code. In this case,
`GDBRemoteCommunicationClient::AllocateMemory()` sets
`m_supports_alloc_dealloc_memory` to `eLazyBoolYes`; `eLazyBoolNo` is
only used if the server cannot handle the packet at all. Before this
patch, `ProcessGDBRemote::DoAllocateMemory()` checked this flag and
returned `LLDB_INVALID_ADDRESS` without setting an error, which caused
`Process::CanJIT()` to set `m_can_jit = eCanJITYes`, resulting in
`IRMemoryMap::FindSpace()` attempting to allocate memory in the inferior
process and failing. With the patch,
`ProcessGDBRemote::DoAllocateMemory()` returns an error and `m_can_jit`
is set to `eCanJITNo`.
Example debug session:
```
(lldb) platform connect...
(lldb) file test
(lldb) br set...
(lldb) run
Process 100 launched:...
Process 100 stopped
* thread #1,...
(lldb) expr $x0
error: Couldn't allocate space for materialized struct: Couldn't malloc: address space is full
error: errored out in virtual lldb_private::LLVMUserExpression::DoExecute, couldn't PrepareToExecuteJITExpression
```
When connected to a GDB remote platform, we always use "gdb-remote" as
the process plugin when attaching. This means that the `--plugin`
argument to `process attach` is effectively ignored. This patch makes it
so that "gdb-remote" remains the default, while still honoring the
process plugin name specified in the attach info. The same thing applies
to launching a process.
rdar://167845923
We weren't setting `m_should_detach` when going through the
`DoConnectRemote` code path. This meant that when you would attaches to
a remote process with `gdb-remote <port>` and use Ctrl+D, it would kill
the process instead of detach from it.
rdar://156111423
While debugging the tests for #155000 I found it helpful to have both
sides
of the simulated gdb-rsp traffic rather than just the responses so I've
extended
the packetLog in MockGDBServerResponder to record traffic in both
directions.
Tests have been updated accordingly
This started as me being annoyed that I got loads of this when
inspecting memory regions on Mac:
Modified memory (dirty) page list provided, 0 entries.
So I thought I should test the existing behaviour, which led me to
refactor the existing test to run the same checks on all regions.
In the process I realised that the output is not wrong. There is a
difference between knowing that no pages are dirty and not knowing
anything about dirty pages. We print that there are 0 entries so the
user knows that difference.
The test case now checks "memory region" output as well as API use.
There were also some checks only run on certain regions, like page size,
which now run for all of them.
Due to a fallback in GDBRemoteCommunicationClient.cpp, on Linux we will
assume a PID of 1 if the remote does not respond in some way that tells
us the real PID.
So if PID 1 happened to be a process that the current user could read,
we would try to debug that instead. On my current machine, PID 1 was
sshd run by root so we would ignore it. If I changed the fallback to
some process ID run by my user, the test would fail.
To prevent this, select the remote-linux platform before creating the
target. This means we won't attempt any host lookups.
Fixes#155895
This PR implements a register context for Wasm, which uses virtual
registers to resolve Wasm local, globals and stack values. The registers
are used to implement support for `DW_OP_WASM_location` in the DWARF
expression evaluator (#151010). This also adds a more comprehensive
test, showing that we can use this to show local variables.
Extend support in LLDB for WebAssembly. This PR adds a new Process
plugin (ProcessWasm) that extends ProcessGDBRemote for WebAssembly
targets. It adds support for WebAssembly's memory model with separate
address spaces, and the ability to fetch the call stack from the
WebAssembly runtime.
I have tested this change with the WebAssembly Micro Runtime (WAMR,
https://github.com/bytecodealliance/wasm-micro-runtime) which implements
a GDB debug stub and supports the qWasmCallStack packet.
```
(lldb) process connect --plugin wasm connect://localhost:4567
Process 1 stopped
* thread #1, name = 'nobody', stop reason = trace
frame #0: 0x40000000000001ad
wasm32_args.wasm`main:
-> 0x40000000000001ad <+3>: global.get 0
0x40000000000001b3 <+9>: i32.const 16
0x40000000000001b5 <+11>: i32.sub
0x40000000000001b6 <+12>: local.set 0
(lldb) b add
Breakpoint 1: where = wasm32_args.wasm`add + 28 at test.c:4:12, address = 0x400000000000019c
(lldb) c
Process 1 resuming
Process 1 stopped
* thread #1, name = 'nobody', stop reason = breakpoint 1.1
frame #0: 0x400000000000019c wasm32_args.wasm`add(a=<unavailable>, b=<unavailable>) at test.c:4:12
1 int
2 add(int a, int b)
3 {
-> 4 return a + b;
5 }
6
7 int
(lldb) bt
* thread #1, name = 'nobody', stop reason = breakpoint 1.1
* frame #0: 0x400000000000019c wasm32_args.wasm`add(a=<unavailable>, b=<unavailable>) at test.c:4:12
frame #1: 0x40000000000001e5 wasm32_args.wasm`main at test.c:12:12
frame #2: 0x40000000000001fe wasm32_args.wasm
```
This PR is based on an unmerged patch from Paolo Severini:
https://reviews.llvm.org/D78801. I intentionally stuck to the
foundations to keep this PR small. I have more PRs in the pipeline to
support the other features/packets.
My motivation for supporting Wasm is to support debugging Swift compiled
to WebAssembly:
https://www.swift.org/documentation/articles/wasm-getting-started.html
This fixes issues found in e066f35c6981c720e3a7e5883efc40c861b3b7, which
was later reverted. The problem was with "k" message which was sent with
sync_on_timeout flag set to true, so lldb was waiting for response,
which is currently not being sent by lldb-server. Not waiting for
response at all seems to be not a solution, because on MAC OS X lldb
waits for response from "k" to gracefully kill inferior.
Patch fixes the sync-on-timeout logic in lldb and switches to qEcho
based ping, instead of qC. This fixes vRun message case, when there is
no process yet and qC returns an error.
Extending the conditionals in `AugmentRegisterInfo` to support
alternative names for lldb.
Fixes#124023
There is an exception with register `X8` which is not covered here but
more details can be found in the issue
https://github.com/llvm/llvm-project/issues/127900.
This fixes the two test suite failures that I missed in the PR:
https://github.com/llvm/llvm-project/pull/112939
One was a poorly written test case - it assumed that on connect to a
gdb-remote with a running process, lldb MUST have fetched all the frame
0 registers. In fact, there's no need for it to do so (as the CallSite
patch showed...) and if we don't need to we shouldn't. So I fixed the
test to only expect a `g` packet AFTER calling read_registers.
The other was a place where some code had used 0 when it meant
LLDB_INVALID_LINE_NUMBER, which I had fixed but missed one place where
it was still compared to 0.
Fixes#107864
QEMU decided that when SVE is enabled it will only tell us about SVE
registers in the XML, and not include Neon registers. On the grounds
that the Neon V registers can be read from the bottom 128 bits of a SVE
Z register (SVE's vector length is always >= 128 bits).
To support this we create sub-registers just as we do for S and D
registers of the V registers. Except this time we use part of the Z
registers.
This change also updates our fallback for registers with unknown types
that are > 128 bit. This is detailed in
https://github.com/llvm/llvm-project/issues/87471, though that covers
more than this change fixes.
We'll now treat any register of unknown type that is >= 128 bit as a
vector of bytes. So that the user gets to see something
even if the order might be wrong.
And until lldb supports vector and union types for registers, this is
also the only way we can get a value to apply the sub-reg to, to make
the V registers.
(based on a conversation I had with @labath yesterday in
https://github.com/llvm/llvm-project/pull/106442)
Most APIs that currently vend a Status would be better served by
returning llvm::Expected<> instead. If possibles APIs should be
refactored to avoid Status. The only legitimate long-term uses of Status
are objects that need to store an error for a long time (which should be
questioned as a design decision, too).
This patch makes the transition to llvm::Error easier by making the
places that cannot switch to llvm::Error explicit: They are marked with
a call to Status::clone(). Every other API can and should be refactored
to use llvm::Expected. In the end Status should only be used in very few
places.
Whenever an unchecked Error is dropped by Status it logs this to the
verbose API channel.
Implementation notes:
This patch introduces two new kinds of error_category as well as new
llvm::Error types. Here is the mapping of lldb::ErrorType to
llvm::Errors:
```
(eErrorTypeInvalid)
eErrorTypeGeneric llvm::StringError
eErrorTypePOSIX llvm::ECError
eErrorTypeMachKernel MachKernelError
eErrorTypeExpression llvm::ErrorList<ExpressionError>
eErrorTypeWin32 Win32Error
```
Relanding with built-in cloning support for llvm::ECError, and support
for initializing a Windows error with a NO_ERROR error code, and
modifying TestGDBRemotePlatformFile.py to support different renderings
of ENOSYS.
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
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 d9e659c538516036e40330b6a98160cbda4ff100.
I could not reproduce the Mac OS ASAN failure locally but I narrowed it
down to the test `test_many_fields_same_enum`. This test shares an enum
between x0, which is 64 bit, and cpsr, which is 32 bit.
My theory is that when it does `register read x0`, an enum type is
created where the undlerying enumerators are 64 bit, matching the
register size.
Then it does `register read cpsr` which used the cached enum type, but
this register is 32 bit. This caused lldb to try to read an 8 byte value
out of a 4 byte allocation:
READ of size 8 at 0x60200014b874 thread T0
<...>
=>0x60200014b800: fa fa fd fa fa fa fd fa fa fa fd fa fa fa[04]fa
To fix this I've added the register's size in bytes to the constructed
enum type's name. This means that x0 uses:
__lldb_register_fields_enum_some_enum_8
And cpsr uses:
__lldb_register_fields_enum_some_enum_4
If any other registers use this enum and are read, they will use the
cached type as long as their size matches, otherwise we make a new type.
In one of my recent PRs I mistakenly had two test-cases with the same
name, preventing one of them to run. Since it's an easy mistake to make
(e.g., copy pasting existing test-cases), I ran following sanity-check
script over `lldb/test/API`, which found couple of tests which were
losing coverage because of this (or in some cases simply had duplicate
tests):
```
import ast
import sys
filename = sys.argv[1]
print(f'Checking {filename}...')
tree = ast.parse(open(filename, 'r').read())
for node in ast.walk(tree):
if not isinstance(node, ast.ClassDef):
continue
func_names = []
for child in ast.iter_child_nodes(node):
if isinstance(child, ast.FunctionDef):
func_names.append(child.name)
seen_func_names = set()
duplicate_func_names = []
for name in func_names:
if name in seen_func_names:
duplicate_func_names.append(name)
else:
seen_func_names.add(name)
if len(duplicate_func_names) != 0:
print(f'Multiple func names found:\n\t{duplicate_func_names}\n\tclass {node.name}\n\tfile: {filename}')
```
This patch fixes these cases.
Fixes#92541
When e69a3d18f48bc0d81b5dd12e735a2ec898ce64d added fallback register
layouts, it assumed that the choices were target XML with registers, or
no target XML at all.
In the linked issue, a user has a debug stub that does have target XML,
but it's missing register information.
This caused us to finalize the register information using an empty set
of registers got from target XML, then fail an assert when we attempted
to add the fallback set. Since we think we've already completed the
register information.
This change adds a check to prevent that first call and expands the
existing tests to check each architecture without target XML and with
target XML missing register information.
This teaches lldb to parse the enum XML elements sent by lldb-server,
and make use of the information in `register read` and `register info`.
The format is described in
https://sourceware.org/gdb/current/onlinedocs/gdb.html/Enum-Target-Types.html.
The target XML parser will drop any invalid enum or evalue. If we find
multiple evalue for the same value, we will use the last one we find.
The order of evalues from the XML is preserved as there may be good
reason they are not in numerical order.
from PEP8
(https://peps.python.org/pep-0008/#programming-recommendations):
> Comparisons to singletons like None should always be done with is or
is not, never the equality operators.
Co-authored-by: Eisuke Kawashima <e-kwsm@users.noreply.github.com>
The tests `test_file_permissions` and `test_file_permissions_fallback`
are disabled for Windows target. These tests use MockGDBServerResponder
and do not depend on the real target. These tests failed in case of
Windows host and Linux target. Disable them for Windows host too.
The tests TestPty and TestPtyServer use the Unix specific python builtin
module termios. They are failed in case of Windows host and Linux
target. Disable them for Windows host too.
Any time we see the pattern `assertEqual(value, bool)`, we can replace
that with `assert<bool>(value)`. Likewise for `assertNotEqual`.
Technically this relaxes the test a bit, as we may want to make sure
`value` is either `True` or `False`, and not something that implicitly
converts to a bool. For example, `assertEqual("foo", True)` will fail,
but `assertTrue("foo")` will not. In most cases, this distinction is not
important.
There are two such places that this patch does **not** transform, since
it seems intentional that we want the result to be a bool:
*
5daf2001a1/lldb/test/API/python_api/sbstructureddata/TestStructuredDataAPI.py (L90)
*
5daf2001a1/lldb/test/API/commands/settings/TestSettings.py (L940)
Followup to 9c2468821ec51defd09c246fea4a47886fff8c01. I patched `teyit`
with a `visit_assertEqual` node handler to generate this.
This uses [teyit](https://pypi.org/project/teyit/) to modernize asserts,
as recommended by the [unittest release
notes](https://docs.python.org/3.12/whatsnew/3.12.html#id3).
For example, `assertTrue(a == b)` is replaced with `assertEqual(a, b)`.
This produces better error messages, e.g. `error: unexpectedly found 1
and 2 to be different` instead of `error: False`.
assertEquals is a deprecated alias for assertEqual and has been removed
in Python 3.12. This wasn't an issue previously because we used a
vendored version of the unittest module. Now that we use the built-in
version this gets updated together with the Python version used to run
the test suite.
This reverts commit a7b78cac9a77e3ef6bbbd8ab1a559891dc693401.
With updates to the tests.
TestWatchTaggedAddress.py: Updated the expected watchpoint types,
though I'm not sure there should be a differnt default for the two
ways of setting them, that needs to be confirmed.
TestStepOverWatchpoint.py: Skipped this everywhere because I think
what used to happen is you couldn't put 2 watchpoints on the same
address (after alignment). I guess that this is now allowed because
modify watchpoints aren't accounted for, but likely should be.
Needs investigating.
This reverts commit 933ad5c897ee366759a54869b35b2d7285a92137.
This caused 1 test failure and an unexpected pass on AArch64 Linux:
https://lab.llvm.org/buildbot/#/builders/96/builds/45765
Wasn't reported because the bot was already red at the time.
