# Summary
This PR updates `SBProcess::GetNumThreads()` and
`SBProcess::GetThreadAtIndex()` to listen to the stop locker.
`SBProcess::GetNumThreads()` will return 0 if the process is running.
## Problem Description
Recently upon debugging a program with thousands of threads in VS Code,
lldb-dap would hang at a `threads` request sent right after receiving
the `configurationDone` response. Soon after it will end the debug
session with the following error
```
Process <pid> exited with status = -1 (0xffffffff) lost connection
```
This is because LLDB is still in the middle of resuming all the threads.
And requesting threads will end up interrupt the process on Linux. From
the gdb-remote log it ended up getting `lldb::StateType::eStateInvalid`
and just exit with status -1.
I don't think it's reasonable to allow getting threads from a running
process. There are a few approaches to fix this:
1) Send the stopped event to IDE after `configurationDone`. This aligns
with the CLI behavior.
2) However, the above approach will break the existing user facing
behavior. The alternative will be reject the `threads` request if the
process is not stopped.
3) Improve the run lock. This is a synchronize issue where process was
in the middle of resuming while lldb-dap attempts to interrupt it.
**This PR implements the option 3**
## HOWEVER
This fixed the "lost connection" issue below but new issue has surfaced.
From testing, and also from checking the [VSCode source
code](174af221c9/src/vs/workbench/contrib/debug/browser/debugSession.ts (L791)),
it expects having threadID to perform `pause`. So after attaching,
without any threads reported to the client, the user will not be able to
pause the attached process. `setBreakpoint` will still work and once we
make a stop at the bp (or any stop that will report threads, client can
perform pause again.
## NEXT
1) Made an attempt to return initial thread list so that VSCode can
pause (second commit in the PR)
2) Investigate why threads will trigger unwinding the second frame of a
thread, which leads to sending the interrupt
3) Decided if we want to support `stopOnEntry` for attaching, given
i. This is not an official specification
ii. If enable stopOnEntry, we need to fix attaching on Linux, to send
only one stopped event. Currently, all threads upon attaching will have
stop reason `SIGSTOP` and lldb-dap will send `stopped` event for each
one of them. Every `stopped` will trigger the client request for
threads.
iii. Alternatively, we can support auto continue correspond to `(lldb)
process attach --continue`. This require the ii above.
### Additionally
lldb-dap will not send a `continued` event after `configurationDone`
because it checks `dap.focus_tid == LLDB_INVALID_THREAD_ID` (so that we
don't send it for `launch` request). Notice `dap.focus_tid` will only
get assigned when handling stop or stepping.
According to DAP
> Please note: a debug adapter is not expected to send this event in
response to a request that implies that execution continues, e.g. launch
or continue.
It is only necessary to send a continued event if there was no previous
request that implied this.
So I guess we are not violating DAP if we don't send `continued` event.
But I'd like to get some sense about this.
## Test Plan
Used following program for testing:
https://gist.github.com/kusmour/1729d2e07b7b1063897db77de194e47d
**NOTE: Utilize stdin to get pid and attach AFTER hitting enter. Attach
should happen when all the threads start running.**
DAP messages before the change
<img width="1165" alt="image"
src="https://github.com/user-attachments/assets/a9ad85fb-81ce-419c-95e5-612639905c66"
/>
DAP message after the change - report zero threads after attaching
<img width="1165" alt="image"
src="https://github.com/user-attachments/assets/a1179e18-6844-437a-938c-0383702294cd"
/>
---------
Co-authored-by: Jonas Devlieghere <jonas@devlieghere.com>
When you call the `SBTarget::ReadInstructions` with flavor from lldb
crashes. This is because the wrong order of the `DisassemblyBytes`
constructor this fixes that
---------
Signed-off-by: Ebuka Ezike <yerimyah1@gmail.com>
The motivation for this patch is that in Statistics.cpp we [check to see
if the module symfile is
loaded](990a086d9d/lldb/source/Target/Statistics.cpp (L353C60-L353C75))
to calculate how much debug info has been loaded. I have an external
utility that only wants to look at the loaded debug info, which isn't
exposed by the SBAPI.
Expose u target API mutex through the SB API. This is motivated by
lldb-dap, which is built on top of the SB API and needs a way to execute
a series of SB API calls in an atomic manner (see #131242).
We can solve this problem by either introducing an additional layer of
locking at the DAP level or by exposing the existing locking at the SB
API level. This patch implements the second approach.
This was discussed in an RFC on Discourse [0]. The original
implementation exposed a move-only lock rather than a mutex [1] which
doesn't work well with SWIG 4.0 [2]. This implement the alternative
solution of exposing the mutex rather than the lock. The SBMutex
conforms to the BasicLockable requirement [3] (which is why the methods
are called `lock` and `unlock` rather than Lock and Unlock) so it can be
used as `std::lock_guard<lldb::SBMutex>` and
`std::unique_lock<lldb::SBMutex>`.
[0]: https://discourse.llvm.org/t/rfc-exposing-the-target-api-lock-through-the-sb-api/85215/6
[1]: https://github.com/llvm/llvm-project/pull/131404
[2]: https://discourse.llvm.org/t/rfc-bumping-the-minimum-swig-version-to-4-1-0/85377/9
[3]: https://en.cppreference.com/w/cpp/named_req/BasicLockable
The class was taking ownership of the SBCommandPluginInterface pointer
it was passed in, by wrapping it in a shared pointer. This causes a
double free in the unit test when the object is destroyed and the same
pointer gets freed once when the SBCommandPluginInterface goes away and
then again when the shared pointer hits a zero refcount.
This reverts commit
87b7f63a11,
reapplying
7e66cf74fb
with a small (and probably temporary)
change to generate more debug info to help with diagnosing buildbot
issues.
Calling the public API `SBLineEntry::SetColumn()` sets the row instead
of the column.
This probably should be backported as it has been since version 3.4.
---------
Co-authored-by: Jonas Devlieghere <jonas@devlieghere.com>
Remove support for coalescing progress reports in LLDB. This
functionality was motivated by Xcode, which wanted to listen for less
frequent, aggregated progress events at the cost of losing some detail.
See the original RFC [1] for more details. Since then, they've
reevaluated this trade-off and opted to listen for the regular, full
fidelity progress events and do any post processing on their end.
rdar://146425487
This patch pushes the error handling boundary for the GetBitSize()
methods from Runtime into the Type and CompilerType APIs. This makes it
easier to diagnose problems thanks to more meaningful error messages
being available. GetBitSize() is often the first thing LLDB asks about a
type, so this method is particularly important for a better user
experience.
rdar://145667239
This patch adds a finalize method which destroys the underlying RAII
SBProgress. My primary motivation for this is so I can write better
tests that are non-flaky, but after discussing with @clayborg in my DAP
message improvement patch (#124648) this is probably an essential API
despite that I originally argued it wasn't.
I am using VSCode with the official vscode-lldb extension. When I try to
list the breakpoints in the debug console get the message:
```
br list
can't evaluate expressions when the process is running.
```
I know that this is wrong and you need to use
```
`br list
(lldb) br list
No breakpoints currently set.
```
but the error message is misleading. I cleaned up the code and now the
error message is
```
br list
sbframe object is not valid.
```
which is still not perfect, but at least it's not misleading.
Currently, configuring lldb with cmake with separated stdout and stderr
gives some unnecessary output in stderr which happens since message
statement is used without `[mode]` (See
https://cmake.org/cmake/help/v3.31/command/message.html). This patch
adds mode `STATUS` instead of explicit `--` at the beginning of
messages.
There are a lot of lldb commands whose result is really one or more
ValueObjects that we then print with the ValueObjectPrinter. Now that we
have the ability to access the SBCommandReturnObject through a callback
(#125006), we can store the resultant ValueObjects in the return object,
allowing an IDE to access the SBValues and do its own rich formatting.
rdar://143965453
This makes GetOutputStreamSP and GetErrorStreamSP protected members of
Debugger. Users who want to print to the debugger's stream should use
GetAsyncOutputStreamSP and GetAsyncErrorStreamSP instead and the few
remaining stragglers have been migrated.
This patch adds support for template arguments of
`clang::TemplateArgument::ArgKind::StructuralValue` kind (added in
https://github.com/llvm/llvm-project/pull/78041). These are used for
non-type template parameters such as floating point constants. When LLDB
created `clang::NonTypeTemplateParmDecl`s, it previously assumed
integral values, this patch accounts for structural values too.
Anywhere LLDB assumed a `DW_TAG_template_value_parameter` was
`Integral`, it will now also check for `StructuralValue`, and will
unpack the `TemplateArgument` value and type accordingly.
We can rely on the fact that any `TemplateArgument` of `StructuralValue`
kind that the `DWARFASTParserClang` creates will have a valid value,
because it gets those from `DW_AT_const_value`.
This patch adds a new API to `SBType` to retrieve the value of a
template parameter given an index. We re-use the
`TypeSystemClang::GetIntegralTemplateArgument` for this and thus
currently only supports integral non-type template parameters. Types
like float/double are not supported yet.
rdar://144395216
Remove Debugger::GetOutputStream and Debugger::GetErrorStream in
preparation for replacing both with a new variant that needs to be
locked and hence can't be handed out like we do right now.
The patch replaces most uses with GetAsyncOutputStream and
GetAsyncErrorStream respectively. There methods return new StreamSP
objects that automatically get flushed on destruction.
See #126630 for more details.
This typedef doesn't match the signature below, specifically the
signature takes a `lldb:SBDebugger` vs this was defined as
`lldb:SBDebugger&`.
```
lldb/source/API/SBDebugger.cpp:199:13: runtime error: call to function lldb::PluginInitialize(lldb::SBDebugger) through pointer to incorrect function type 'bool (*)(lldb::SBDebugger &)'
.../CustomPlugin.cpp:134: note: lldb::PluginInitialize(lldb::SBDebugger) defined here
```
The WatchAddress API includes a flag to indicate if watchpoint should be
for read, modify or both. This API uses 2 booleans, but the 'modify'
flag was ignored and WatchAddress unconditionally watched write
(actually modify).
We now only watch for modify when the modify flag is true.
---
The included test fails prior to this patch and succeeds after. That is
previously specifying `False` for `modify` would still stop on _write_,
but after the patch correctly only stops on _read_
Xcode uses a pseudoterminal for the debugger console.
- The upside of this apporach is that it means that it can rely on
LLDB's IOHandlers for multiline and script input.
- The downside of this approach is that the command output is printed to
the PTY and you don't get a SBCommandReturnObject. Adrian added support
for inline diagnostics (#110901) and we'd like to access those from the
IDE.
This patch adds support for registering a callback in the command
interpreter that gives access to the `(SB)CommandReturnObject` right
before it will be printed. The callback implementation can choose
whether it likes to handle printing the result or defer to lldb. If the
callback indicated it handled the result, the command interpreter will
skip printing the result.
We considered a few other alternatives to solve this problem:
- The most obvious one is using `HandleCommand`, which returns a
`SBCommandReturnObject`. The problem with this approach is the multiline
input mentioned above. We would need a way to tell the IDE that it
should expect multiline input, which isn't known until LLDB starts
handling the command.
- To address the multiline issue,we considered exposing (some of the)
IOHandler machinery through the SB API. To solve this particular issue,
that would require reimplementing a ton of logic that already exists
today in the CommandInterpeter. Furthermore that seems like overkill
compared to the proposed solution.
rdar://141254310
As suggested in #125006. Depending on which PR lands first, I'll update
`TestCommandInterepterPrintCallback.py` to check that the
`CommandReturnObject` passed to the callback has the correct command.
This reverts commit a774de807e.
This is the same changes as last time, plus:
* We load the binary into the target object so that on Windows, we can
resolve the locations of the functions.
* We now assert that each required breakpoint has at least 1 location,
to prevent an issue like that in the future.
* We are less strict about the unsupported error message, because it
prints "error: windows" on Windows instead of "error: gdb-remote".
Reverts llvm/llvm-project#123945
Has failed on the Windows on Arm buildbot:
https://lab.llvm.org/buildbot/#/builders/141/builds/5865
```
********************
Unresolved Tests (2):
lldb-api :: functionalities/reverse-execution/TestReverseContinueBreakpoints.py
lldb-api :: functionalities/reverse-execution/TestReverseContinueWatchpoints.py
********************
Failed Tests (1):
lldb-api :: functionalities/reverse-execution/TestReverseContinueNotSupported.py
```
Reverting while I reproduce locally.
This reverts commit 22561cfb44 and fixes
b7b9ccf449 (#112079).
The problem is that x86_64 and Arm 32-bit have memory regions above the
stack that are readable but not writeable. First Arm:
```
(lldb) memory region --all
<...>
[0x00000000fffcf000-0x00000000ffff0000) rw- [stack]
[0x00000000ffff0000-0x00000000ffff1000) r-x [vectors]
[0x00000000ffff1000-0xffffffffffffffff) ---
```
Then x86_64:
```
$ cat /proc/self/maps
<...>
7ffdcd148000-7ffdcd16a000 rw-p 00000000 00:00 0 [stack]
7ffdcd193000-7ffdcd196000 r--p 00000000 00:00 0 [vvar]
7ffdcd196000-7ffdcd197000 r-xp 00000000 00:00 0 [vdso]
ffffffffff600000-ffffffffff601000 --xp 00000000 00:00 0 [vsyscall]
```
Compare this to AArch64 where the test did pass:
```
$ cat /proc/self/maps
<...>
ffffb87dc000-ffffb87dd000 r--p 00000000 00:00 0 [vvar]
ffffb87dd000-ffffb87de000 r-xp 00000000 00:00 0 [vdso]
ffffb87de000-ffffb87e0000 r--p 0002a000 00:3c 76927217 /usr/lib/aarch64-linux-gnu/ld-linux-aarch64.so.1
ffffb87e0000-ffffb87e2000 rw-p 0002c000 00:3c 76927217 /usr/lib/aarch64-linux-gnu/ld-linux-aarch64.so.1
fffff4216000-fffff4237000 rw-p 00000000 00:00 0 [stack]
```
To solve this, look up the memory region of the stack pointer (using
https://lldb.llvm.org/resources/lldbgdbremote.html#qmemoryregioninfo-addr)
and constrain the read to within that region. Since we know the stack is
all readable and writeable.
I have also added skipIfRemote to the tests, since getting them working
in that context is too complex to be worth it.
Memory write failures now display the range they tried to write, and
register write errors will show the name of the register where possible.
The patch also includes a workaround for a an issue where the test code
could mistake an `x` response that happens to begin with an `O` for an
output packet (stdout). This workaround will not be necessary one we
start using the [new
implementation](https://discourse.llvm.org/t/rfc-fixing-incompatibilties-of-the-x-packet-w-r-t-gdb/84288)
of the `x` packet.
---------
Co-authored-by: Pavel Labath <pavel@labath.sk>
I wanted to clarify the semantics around SBProgress. Given the nature of
Progress events, copying seems like the wrong idea. Making SBProgress
move-only (like SBStream) seems like the better choice here.
This commit adds support for a
`SBProcess::ContinueInDirection()` API. A user-accessible command for
this will follow in a later commit.
This feature depends on a gdbserver implementation (e.g. `rr`) providing
support for the `bc` and `bs` packets. `lldb-server` does not support
those packets, and there is no plan to change that. For testing
purposes, this commit adds a Python implementation of *very limited*
record-and-reverse-execute functionality, implemented as a proxy between
lldb and lldb-server in `lldbreverse.py`. This should not (and in
practice cannot) be used for anything except testing.
The tests here are quite minimal but we test that simple breakpoints and
watchpoints work as expected during reverse execution, and that
conditional breakpoints and watchpoints work when the condition calls a
function that must be executed in the forward direction.
Recently I've been working on a lot of internal Python tooling, and in
certain cases I want to report async to the script over DAP. Progress.h
already handles this, so I've exposed Progress via the SB API so Python
scripts can also update progress objects.
I actually have no idea how to test this, so I just wrote a [toy command
to test
it](https://gist.github.com/Jlalond/48d85e75a91f7a137e3142e6a13d0947)
I also copied the first section of the extensive Progress.h class
documentation to the docstrings.
I think the only issue here was that we would erroneously consider
functions which are "in the middle" of the function were stepping to as
a part of the function, and would try to step into them (likely stepping
out of the function instead) instead of giving up early.
This patch adds the ability to get a thread at a give index, based on
insertion order, for SBSaveCore Options. This is primarily to benefit
scripts using SBSaveCore, and remove the need to have both options and a
second collection if your script is tracking what threads need to be
saved. Such as if you want to collect the source of all the threads to
be saved after the Core is generated.
The main change is to permit the disassembler class to process/store
multiple (discontinuous) ranges of addresses. The result is not
ambiguous because each instruction knows its size (in addition to its
address), so we can check for discontinuity by looking at whether the
next instruction begins where the previous ends.
This patch doesn't handle the "disassemble" CLI command, which uses a
more elaborate mechanism for disassembling and printing instructions.
Lots of code around LLDB was directly accessing the target's section
load list. This NFC patch makes the section load list private so the
Target class can access it, but everyone else now uses accessor
functions. This allows us to control the resolving of addresses and will
allow for functionality in LLDB which can lazily resolve addresses in
JIT plug-ins with a future patch.
Many calls to Function::GetAddressRange() were not interested in the
range itself. Instead they wanted to find the address of the function
(its entry point) or the base address for relocation of function-scoped
entities (technically, the two don't need to be the same, but there's
isn't good reason for them not to be). This PR creates a separate
function for retrieving this, and changes the existing
(non-controversial) uses to call that instead.
Building on top of previous work that exposed expression diagnostics via
SBCommandReturnObject, this patch generalizes the support to expose any
SBError as machine-readable structured data. One use-case of this is to
allow IDEs to better visualize expression diagnostics.
rdar://139997604
Currently, we arbitrarily paginate editline completions to 40 elements.
On large terminals, that leaves some real-estate unused. On small
terminals, it's pretty annoying to not see the first completions. We can
address both issues by using the terminal height for pagination.
This builds on the improvements of #116456.
Compared to the python version, this also does type checking and error
handling, so it's slightly longer, however, it's still comfortably
under 500 lines.
Relanding with more explicit type conversions.
This reverts commit f6012a209d.
Revert "[lldb] Add cast to fix compile error on 32-but platforms"
This reverts commit d300337e93.
Revert "[lldb] Improve log message to include missing strings"
This reverts commit 0be3348485.
Revert "[lldb] Add comment"
This reverts commit e2bb47443d.
Revert "[lldb] Implement a formatter bytecode interpreter in C++"
This reverts commit 9a9c1d4a61.
Compared to the python version, this also does type checking and error
handling, so it's slightly longer, however, it's still comfortably
under 500 lines.
This reverts commit 2526d5b168, reapplying
ba14dac481 after fixing the conflict with
#117532. The change is that Function::GetAddressRanges now recomputes
the returned value instead of returning the member. This means it now
returns a value instead of a reference type.
SBFunction::GetEndAddress doesn't really make sense for discontinuous
functions, so I'm declaring it deprecated. GetStartAddress sort of makes
sense, if one uses it to find the functions entry point, so I'm keeping
that undeprecated.
I've made the test a Shell tests because these make it easier to create
discontinuous functions regardless of the host os and architecture. They
do make testing the python API harder, but I think I've managed to come
up with something not entirely unreasonable.
The old behavior was to return a null string in the error case,when
refactoring the error handling I thought it would be a good idea to
print the error in the description, but that breaks clients that try to
print a description first and then do something else in the error case.
The API is not great but it's clear that in-band errors are also not a
good idea.
rdar://133956263
