This patch implements a workaround for a VSCode bug that causes it to
send disassemble requests with empty memory reference. You can find more
detailed description
[here](https://github.com/microsoft/vscode/pull/270361). I propose to
allow empty memory reference and return invalid instructions when this
occurs.
Error log example:
```
1759923554.517830610 (stdio) --> {"command":"disassemble","arguments":{"memoryReference":"","offset":0,"instructionOffset":-50,"instructionCount":50,"resolveSymbols":true},"type":"request","seq":3}
1759923554.518007517 (stdio) queued (command=disassemble seq=3)
1759923554.518254757 (stdio) <-- {"body":{"error":{"format":"invalid arguments for request 'disassemble': malformed memory reference at arguments.memoryReference\n{\n \"instructionCount\": 50,\n \"instructionOffset\": -50,\n \"memoryReference\": /* error: malformed memory reference */ \"\",\n \"offset\": 0,\n \"resolveSymbols\": true\n}","id":3,"showUser":true}},"command":"disassemble","request_seq":3,"seq":0,"success":false,"type":"response"}
```
I am not sure that we should add workaround here when bug on VSCode
side, but I think this bug affects our users. WDYT?
Adjusting logs in a few ways:
* The `DAP_LOG` and `DAP_LOG_ERROR` macros now include the file/line of
the log statement.
* Added support for creating a log with a prefix. This simplifies how we
create logs for the `lldb_dap::DAP` instance and `lldb_dap::Transport`
instance, allowing us to not have to pass the client name around as
much.
* Updated logging usage to take the `lldb_dap::Log` as a reference but
it now defaults to `llvm::raw_null_stream` if not configured. This
ensures more uniform access to the logger, even if its not written
anywhere.
The logs now look like:
```
1764896564.038788080 (stdio) --> {"command":"initialize","arguments":{...},"type":"request","seq":1}
1764896564.039064884 DAP.cpp:1007 (stdio) queued (command=initialize seq=1)
1764896564.039768934 (stdio) <-- {"body":{...},"command":"initialize","request_seq":1,"seq":1,"success":true,"type":"response"}
```
Some months ago, the LookupInfo constructor logic was refactored to not
depend on language specific logic, and use languages plugins instead. In
this refactor, when the language type is unknown, a single LookupInfo
object will handle multiple languages. This doesn't work well, as
multiple languages might want to configure the LookupInfo object in
different ways. For example, different languages might want to set the
m_lookup_name differently from each other, but the previous
implementation would pick the first name a language provided, and
effectively ignored every other language. Other fields of the LookupInfo
object are also configured in incompatible ways.
This approach doesn't seem to be a problem upstream, since only the
C++/Objective-C language plugins are available, but it broke downstream
on the Swift fork, as adding Swift to the list of default languages when
the language type is unknown breaks C++ tests.
This patch makes it so instead of building a single LookupInfo object
for multiple languages, one LookupInfo object is built per language
instead.
rdar://159531216
On Mac `unsigned long long` corresponds to `uint64_t` so `%llu` is
needed.
Simply always using `%llu` and upcasting to `unsigned long long` should
make it work fine.
Updates `InitializeRequestArguments` to correctly follow the spec, see
https://microsoft.github.io/debug-adapter-protocol/specification#Requests_Initialize.
This should correct which fields are tracked as optional and simplifies
some of the types to make sure they're meaningful (e.g. an
`optional<bool>` isn't anymore helpful than a `bool` since undefined and
false are basically equivalent and it requires us to handle interpreting undefined as the default value in all the places we use the `optional<bool>`).
When creating a stack frame in JSONUtils.cpp CreateStackFrame() the code
constructs a std::string from module.GetUUIDString(), which can return
nullptr in some cases (as documented in the implementation of
SBModule::GetUUIDString()). This causes a segmentation fault when passed
to the std::string constructor.
This fix adds a null check before constructing the UUID string, falling
back to an empty string if nullptr is returned. The existing empty check
ensures the moduleId field is omitted from the JSON when no UUID exists.
rdar://163811812
---------
Co-authored-by: Ebuka Ezike <yerimyah1@gmail.com>
Add a "shared_cache_path" key-value to the jGetSharedCacheInfo response,
if we can fetch the shared cache path.
If debugserver and the inferior process are running with the same shared
cache UUID, there is a simple SPI to get debugserver's own shared cache
filepath and we will return that.
On newer OSes, there are SPI we can use to get the inferior process'
shared cache filepath, use that if necessary and the SPI are available.
The response for the jGetSharedCacheInfo packet will now look like
{"shared_cache_base_address":6609256448,"shared_cache_uuid":"B69FF43C-DBFD-3FB1-B4FE-A8FE32EA1062","no_shared_cache":false,"shared_cache_private_cache":false,"shared_cache_path":"/System/Volumes/Preboot/Cryptexes/OS/System/Library/dyld/dyld_shared_cache_arm64e"}
when we have the full information about the shared cache in the
inferior. There are three possible types of responses:
1. inferior has not yet mapped in a shared cache (read: when stopped at
dyld_start and dyld hasn't started executing yet). In this case, no
"shared_cache_path" is listed. ("shared_cache_base_address" will be 0,
"shared_cache_uuid" will be all-zeroes uuid)
2. inferior has a shared cache, but it is different than debugserver's
and we do not have the new SPI to query the shared cache filepath. No
"shared_cache_path" is listed.
3. We were able to find the shared cache filepath, and it is included in
the response, as above.
I'm not using this information in lldb yet, but changes that build on
this will be forthcoming.
rdar://148939795
This patch adds support for `dataBreakpointInfoBytes` capability from
DAP. You can test this feature in VSCode (`Add data breakpoint at
address` button in breakpoints tab).
## Summary:
This change introduces a `DAPSessionManager` to enable multiple DAP
sessions to share debugger instances when needed, for things like child
process debugging and some scripting hooks that create dynamically new
targets.
Changes include:
- Add `DAPSessionManager` singleton to track and coordinate all active DAP
sessions
- Support attaching to an existing target via its globally unique target
ID (targetId parameter)
- Share debugger instances across sessions when new targets are created
dynamically
- Refactor event thread management to allow sharing event threads
between sessions and move event thread and event thread handlers to `EventHelpers`
- Add `eBroadcastBitNewTargetCreated` event to notify when new targets are
created
- Extract session names from target creation events
- Defer debugger initialization from 'initialize' request to
'launch'/'attach' requests. The only time the debugger is used currently
in between its creation in `InitializeRequestHandler` and the `Launch`
or `Attach` requests is during the `TelemetryDispatcher` destruction
call at the end of the `DAP::HandleObject` call, so this is safe.
This enables scenarios when new targets are created dynamically so that
the debug adapter can automatically start a new debug session for the
spawned target while sharing the debugger instance.
## Tests:
The refactoring maintains backward compatibility. All existing DAP test
cases pass.
Also added a few basic unit tests for DAPSessionManager
```
>> ninja DAPTests
>> ./tools/lldb/unittests/DAP/DAPTests
>>./bin/llvm-lit -v ../llvm-project/lldb/test/API/tools/lldb-dap/
```
* original change #162730
* with windows fix#164843
* remove timeout that was pointed out in the comment above
* Remove test that starts and listens on a socket to avoid timeout
issues
Adding structured types for the evaluate request handler.
This should be mostly a non-functional change. I did catch some spelling
mistakes in our tests ('variable' vs 'variables').
This was getting joined in ShutDownExcecptionThread (sic) but not
cleared. So this function was not safe to call twice, since you aren't
supposed to join a thread twice. Sadly, this was called in
MachTask::Clear and MachProcess::Destroy, which are both called when you
tell debugserver to detach.
This didn't seem to cause problems IRL, but the most recent ASAN detects
this as an error and calls ASAN::Die, which was causing all the tests
that ran detach to fail.
I fixed that by moving the clear & test for m_exception_thread to
ShutDownExceptionThread. I also fixed the spelling of that routine. And
that routine was claiming to return a kern_return_t which no one was
checking. It actually returns a kern_return_t if there was a Mach
failure and a Posix error if there was a join failure. Since there's
really nothing you can do but exit if this fails, which is always what
you are in the process of doing when you call this, and since we have
already done all the useful logging in ShutDownExceptionThread, I just
removed the return value.
Summary
-------
While dogfooding lldb-dap, I observed that VSCode frequently displays
certain stack frames as greyed out. Although these frames have valid
debug information, double-clicking them shows disassembly instead of
source code. However, running `bt` from the LLDB command line correctly
displays source file and line information for these same frames,
indicating this is an lldb-dap specific issue.
Root Cause
----------
Investigation revealed that `DAP::ResolveSource()` incorrectly uses a
frame's PC address directly to determine whether valid source line
information exists. This approach works for leaf frames, but fails for
non-leaf (caller) frames where the PC points to the return address
immediately after a call instruction. This return address may fall into
compiler-generated code with no associated line information, even though
the actual call site has valid source location data.
The correct approach is to use the symbol context's line entry, which
LLDB resolves by effectively checking PC-1 for non-leaf frames, properly
identifying the line information for the call instruction rather than
the return address.
Testing
-------
Manually tested with VSCode debugging sessions on production workloads.
Verified that non-leaf frames now correctly display source code instead
of disassembly view.
Before the change symptom:
<img width="1013" height="216" alt="image"
src="https://github.com/user-attachments/assets/9487fbc0-f438-4892-a8d2-1437dc25399b"
/>
And here is after the fix:
<img width="1068" height="198" alt="image"
src="https://github.com/user-attachments/assets/0d2ebaa7-cca6-4983-a1d1-1a26ae62c86f"
/>
---------
Co-authored-by: Jeffrey Tan <jeffreytan@fb.com>
Remove the unnecessary sleep in MachProcess::AttachForDebug. The
preceding comment makes it seem like it's necessary for synchronization,
though I don't believe that's the case (see below), and even if it were,
sleeping is not a reliable way to achieve that.
The reason I don't believe it's necessary is because after we return, we
synchronize with the exception thread on a state change. The latter will
call and update the process state, which is exactly what we synchronize
on. I was able to verify that this is the first time we change the
process state: i.e., `GetState` doesn't return a different value before
and after the sleep.
On top of that, there are 3 more places where we call ptrace attach
(`PosixSpawnChildForPTraceDebugging`, `SBLaunchForDebug`, and
`BoardServiceLaunchForDebug`) where we don't sleep.
rdar://163952037
I was looking at the calls to `usleep` in debugserver and noticed that
these default arguments are never overwritten. I converted them to
constants in the function, which makes it easier to reason about.
Users may have multiple devices and would like to resolve the homepath
based on the machine they are on.
expands the tilde `~` character at the front of the given file path.
Support launching a supported DAP client using the lldb-dap binary.
Currently, only the official LLDB-DAP Visual Studio Code extension is
supported. It uses the VS Code launch URL format.
Here's an example:
```
lldb-dap --client vscode -- /path/to/exe foo bar
```
This will open the following URL with `code --open-url`:
```
vscode://llvm-vs-code-extensions.lldb-dap/start?program=/path/to/exe&args=foo&arg=bar
```
Fixes#125777
This is a follow up to https://github.com/llvm/llvm-project/pull/162509.
Using the `SearchPathW` API, we can ensure that the correct version of
Python is installed before `liblldb` is loaded (and `python.dll`
subsequently). If it's not, we try to add it to the search path with the
methods introduced in https://github.com/llvm/llvm-project/pull/162509.
If that fails or if that method is `#ifdef`'d out, we print an error
which will appear before lldb crashes due to the missing dll.
Before https://github.com/llvm/llvm-project/pull/162509, when invoked
from Powershell, lldb would silently crash (no error message/crash
report). After https://github.com/llvm/llvm-project/pull/162509, it
crashes without any indications that the root cause is the missing
python.dll. With this patch, we print the error before crashing.
Noticed this while looking into test stability that the 'entry' stop
reason is not triggering correctly. This should ensure we correctly
trigger the 'entry' stop reason when launching a process with
`"stopOnEntry": true`. I've also updated the tests to ensure we receive
the 'entry' stop reason to catch this regression.
Turns out there's a bug in the current lldb sources that if you fork,
set the stdio file handles to close on exec and then exec lldb with some
commands and the `--batch` flag, lldb will stall on exit. The first
cause of the bug is that the Python session handler - and probably other
places in lldb - think 0, 1, and 2 HAVE TO BE the stdio file handles,
and open and close and dup them as needed. NB: I am NOT trying to fix
that bug. I'm not convinced running the lldb driver headless is worth a
lot of effort, it's just as easy to redirect them to /dev/null, which
does work.
But I would like to keep lldb from stalling on the way out when this
happens. The reason we stall is that we have a MainLoop waiting for
signals, and we try to Interrupt it, but because stdio was closed, the
interrupt pipe for the MainLoop gets the file descriptor 0, which gets
closed by the Python session handler if you run some script command. So
the Interrupt fails.
We were running the Write to the interrupt pipe wrapped in
`llvm::cantFail`, but in a no asserts build that just drops the error on
the floor. So then lldb went on to call std::thread::join on the still
active MainLoop, and that stalls
I made Interrupt (and AddCallback & AddPendingCallback) return a bool
for "interrupt success" instead. All the places where code was
requesting termination, I added checks for that failure, and skip the
std::thread::join call on the MainLoop thread, since that is almost
certainly going to stall at this point.
I didn't do the same for the Windows MainLoop, as I don't know if/when
the WSASetEvent call can fail, so I always return true here. I also
didn't turn the test off for Windows. According to the Python docs all
the API's I used should work on Windows... If that turns out not to be
true I'll make the test Darwin/Unix only.
The lldb-server platform help text is inconsistent with lldb-server
gdbserver help text. This PR modernizes the platform server to use
LLVM's [TableGen](https://llvm.org/docs/TableGen/)-based option parsing
(matching the existing gdbserver implementation), which auto-generates
option parsing code and help text.
The changes improve documentation quality by adding comprehensive option
descriptions,, adding support for `-h`/`--help` flags, and organizing
help output with DESCRIPTION and EXAMPLES sections. Internal-only
options (`--child-platform-fd`) and unused legacy options (`--debug`,
`--verbose`) are now hidden from help while maintaining backward
compatibility. All functional behavior remains unchanged—this is purely
a documentation and code modernization improvement.
## before
```
> /opt/llvm/bin/lldb-server p -h
p: unrecognized option '-h'
Usage:
/opt/llvm/bin/lldb-server p [--log-file log-file-name] [--log-channels log-channel-list] [--port-file port-file-path] --server --listen port
```
## after
```
lldb-server p -h
OVERVIEW: lldb-server platform
USAGE: lldb-server p [options] --listen <[host]:port> [[--] program args...]
CONNECTION OPTIONS:
--gdbserver-port <port> Port to use for spawned gdbserver instances. If 0 or unspecified, a port will be chosen automatically. Short form: -P
--listen <[host]:port> Host and port to listen on. Format: [host]:port or protocol://[host]:port (e.g., tcp://localhost:1234, unix:///path/to/socket). Short form: -L
--socket-file <path> Write listening socket information (port number for TCP or path for Unix domain sockets) to the specified file. Short form: -f
GENERAL OPTIONS:
--help Display this help message and exit.
--log-channels <channel1 categories...:channel2 categories...>
Channels to log. A colon-separated list of entries. Each entry starts with a channel followed by a space-separated list of categories. Common channels: lldb, gdb-remote, platform, process. Short form: -c
--log-file <file> Destination file to log to. If empty, log to stderr. Short form: -l
--server Run in server mode, accepting multiple client connections sequentially. Without this flag, the server exits after handling the first connection.
OPTIONS:
-- program args Arguments to pass to launched gdbserver instances.
DESCRIPTION
Acts as a platform server for remote debugging. When LLDB clients connect,
the platform server handles platform operations (file transfers, process
launching) and spawns debug server instances (lldb-server gdbserver) to
handle actual debugging sessions.
By default, the server exits after handling one connection. Use --server
to keep running and accept multiple connections sequentially.
EXAMPLES
# Listen on port 1234, exit after first connection
lldb-server platform --listen tcp://0.0.0.0:1234
# Listen on port 5555, accept multiple connections
lldb-server platform --server --listen tcp://localhost:5555
# Listen on Unix domain socket
lldb-server platform --listen unix:///tmp/lldb-server.sock
```
For comparison, here is the **gdbserver** help text:
```
lldb-server g -h
OVERVIEW: lldb-server
USAGE: lldb-server g[dbserver] [options] [[host]:port] [[--] program args...]
CONNECTION:
--fd <fd> Communicate over the given file descriptor.
--named-pipe <name> Write port lldb-server will listen on to the given named pipe.
--pipe <fd> Write port lldb-server will listen on to the given file descriptor.
--reverse-connect Connect to the client instead of passively waiting for a connection. In this case [host]:port denotes the remote address to connect to.
GENERAL OPTIONS:
--help Prints out the usage information for lldb-server.
--log-channels <channel1 categories...:channel2 categories...>
Channels to log. A colon-separated list of entries. Each entry starts with a channel followed by a space-separated list of categories.
--log-file <file> Destination file to log to. If empty, log to stderr.
--setsid Run lldb-server in a new session.
TARGET SELECTION:
--attach <pid-or-name> Attach to the process given by a (numeric) process id or a name.
-- program args Launch program for debugging.
DESCRIPTION
lldb-server connects to the LLDB client, which drives the debugging session.
If no connection options are given, the [host]:port argument must be present
and will denote the address that lldb-server will listen on. [host] defaults
to "localhost" if empty. Port can be zero, in which case the port number will
be chosen dynamically and written to destinations given by --named-pipe and
--pipe arguments.
If no target is selected at startup, lldb-server can be directed by the LLDB
client to launch or attach to a process.
```
The modules event requires the `APIMutex` to create the module load
address. this may happen at the same time the `module request` is
handled.
The modules request also requires the `APIMutex` and the `modules_mutex,
set the order to acquire the mutexes.
We've been treating the `seq` attribute incorrectly in lldb-dap.
Previously, we always had `seq=0` for events and responses. We only
filled in the `seq` field on reverse requests.
However, looking at the spec and other DAP implementations, we are
supposed to fill in the `seq` field for each request we send to the DAP
client.
I've updated our usage to fill in `seq` in `DAP::Send` so that all
events/requests/responses have a properly filled `seq` value.
When we restart a process, send an updated 'process' event describing
the newly launched process.
I also updated the `isLocalProcess` value based on if we're on the
'host' platform or not.
When the user send `thread return <expr>` command this changes the stack
length but the UI does not update.
Send stack invalidated event to the client to update the stack.
This updates lldb-dap to clear the screen when using `"console":
"integratedTerminal"` or `"console": "externalTerminal"`.
VSCode will reuse the same terminal for a given debug configuration.
After the process exits it will return to the shell but if the debug
session is launched again it will be invoked in the same terminal.
VSCode is sending the terminal the launch args as terminal input which
means the terminal would now have a string like `lldb-dap --comm-file
... --launch-target ...` and the scrollback buffer from any previous
output or shell commands used in the terminal.
To address this, I've updated LaunchRunInTerminalTarget to reset the
cursor, clear the screen and clear the scrollback buffer to soft 'reset'
the terminal prior to launching the process.
---------
Co-authored-by: Jonas Devlieghere <jonas@devlieghere.com>
This patch extends stdio redirection support to integrated and external
terminals. Currently, these cases are not covered by the standard logic
because `attach` is used instead of `launch`. To be honest,
`runInTerminal` in
[VSCode](https://github.com/microsoft/vscode/blob/main/src/vs/workbench/contrib/debug/node/terminals.ts#L188)
request supports `>` and `<` for redirection, but not `2>`. We could use
the `argsCanBeInterpretedByShell` option to use full power of shell,
however it requieres proper escaping of arguments on lldb-dap side. So,
I think it will be better to have the only one option to control stdio
redirection that works consistently across all console modes.
Also, fixed a small typo in a comparison that was leading to
out-of-bound access, and added `null` as a possible value for `stdio`
array in package.json.
This patch adds `readOnly`
[attribute](https://microsoft.github.io/debug-adapter-protocol/specification#Types_VariablePresentationHint)
for variables. When this attribute is returned for a variable, VS Code
prevents editing (and grays out the `Set Value` button). Without this,
users might be confused if the UI allows edits but the debug adapter
often returns an error. I checked `SetValueFromCString` function
implementation and found that it doesn't support aggregate data types,
so I added simple check for them. Also, I found that I can update value
of registers sets (but without any effects). So I also added checks for
those as well.
