On macOS, breakpoints are briefly unresolved between process launch and
when the dynamic loader has informed us about the loaded libraries. This
information was being forwarded by lldb-dap, but only partially. In the
event handler, we were listening for the `LocationsAdded` and
`LocationsRemoved` breakpoint events. For the scenario described above,
the latter would trigger and we'd send an event reporting the breakpoint
as unresolved. The problem is that when the breakpoint location is
resolved again, you receive a `LocationsResolved` event, not a
`LocationsAdded` event. As a result, the breakpoint would continue to
show up as unresolved in the DAP client.
I found a test that tried to test part of this behavior, but the test
was broken and disabled. I revived the test and added coverage for the
situation described above.
Fixes#112629
rdar://137968318
This adds support for launching lldb-dap in server mode. The extension
will start lldb-dap in server mode on-demand and retain the server until
the VSCode window is closed (when the extension context is disposed).
While running in server mode, launch performance for binaries is greatly
improved by improving caching between debug sessions.
For example, on my local M1 Max laptop it takes ~5s to attach for the
first attach to an iOS Simulator process and ~0.5s to attach each time
after the first.
Both spellings are considered correct and acceptable, with adapter being
more common in American English. Given that DAP stands for Debug Adapter
Protocol (with an e) let's go with that as the canonical spelling.
This simplifies the IOStream.cpp implementation by building on top of
the existing lldb::IOObjectSP.
Additionally, this should help ensure clients connected of a
`--connection` specifier properly detect shutdown requests when the
Socket is closed. Previously, the StreamDescriptor was just accessing
the underlying native handle and was not aware of the `Close()` call to
the Socket itself.
This is both nice to have for simplifying the existing code and this
unblocks an upcoming refactor to support the cancel request.
---------
Co-authored-by: Jonas Devlieghere <jonas@devlieghere.com>
This refactors the response handlers for reverse request to follow the
same architecture as the request handlers. With only two implementation
that might be overkill, but it reduces code duplication and improves
error reporting by storing the sequence ID. This PR also fixes an
unchecked Expected in the old callback for unknown sequence IDs.
The TestDAP_ouput test is flaky due to the order of events during
shutdown. We were stopping the output and error handle redirection after
we finished the disconnect request, which can cause us to miss output
events due to timing. Moving when we stop the redirection to ensure we
have consistent output prior to disconnect responding.
Fixes#128567
Currently, all request handlers are implemented as free functions in
lldb-dap.cpp. That file has grown to over 5000 lines and is starting to
become hard to maintain. This PR moves the request handlers into their
own class (and file), together with their documentation.
This PR migrates about a third of the request handlers and the rest will
be migrated in subsequent commits. I'm merging this in an incomplete
state because almost any lldb-dap change is going to result in merge
conflicts and migrating request handlers one by one is easier to review.
This patch fixes:
lldb/tools/lldb-dap/lldb-dap.cpp:5184:9: error: 'scoped_lock' may
not intend to support class template argument deduction
[-Werror,-Wctad-maybe-unsupported]
lldb/tools/lldb-dap/lldb-dap.cpp:5200:7: error: 'unique_lock' may
not intend to support class template argument deduction
[-Werror,-Wctad-maybe-unsupported]
lldb/tools/lldb-dap/lldb-dap.cpp:5222:5: error: 'scoped_lock' may
not intend to support class template argument deduction
[-Werror,-Wctad-maybe-unsupported]
lldb/tools/lldb-dap/lldb-dap.cpp:5236:3: error: 'unique_lock' may
not intend to support class template argument deduction
[-Werror,-Wctad-maybe-unsupported]
This adjusts the lldb-dap listening mode to accept multiple clients.
Each client initializes a new instance of DAP and an associated
`lldb::SBDebugger` instance.
The listening mode is configured with the `--connection` option and
supports listening on a port or a unix socket on supported platforms.
When running in server mode launch and attach performance should
be improved by lldb sharing symbols for core libraries between debug
sessions.
If you have an lldb-dap log file you'll almost always see a final
message like:
```
<--
Content-Length: 94
{
"body": {
"category": "stdout",
"output": "\u0000\u0000"
},
"event": "output",
"seq": 0,
"type": "event"
}
<--
Content-Length: 94
{
"body": {
"category": "stderr",
"output": "\u0000\u0000"
},
"event": "output",
"seq": 0,
"type": "event"
}
```
The OutputRedirect is always writing the `"\0"` byte as a final stdout
message during shutdown. Instead, I adjusted this to detect the sentinel
value and break out of the read loop as if we detected EOF.
---------
Co-authored-by: Pavel Labath <pavel@labath.sk>
The Swift extension for VS Code requires that the `lldb-dap` executable
come from the Swift toolchain which may or may not be configured in
`PATH`. At the moment, this can be configured via LLDB DAP's extension
settings, but experience has shown that modifying other extensions'
settings on behalf of the user (especially those subject to change
whenever a new toolchain is selected) causes issues. Instead, it would
be easier to have this configurable in the launch configuration and let
the Swift extension (or any other extension that wanted to, really)
configure the path to `lldb-dap` that way. This allows the Swift
extension to have its own launch configuration type that delegates to
the LLDB DAP extension in order to provide a more seamless debugging
experience for Swift executables.
This PR adds a new property to the launch configuration object called
`debugAdapterExecutable` which allows overriding the `lldb-dap`
executable path for a specific debug session.
This commit upgrades our npm dependencies to the latest available
version.
I was prompted to this change because `npm run package` failed for me
with an error. The error disappeared after upgrading `@vscode/vsce`. I
also upgraded the other dependencies because I think it's generally
preferable to stay up-to-date.
I did not bump the `@types/vscode` and `@types/node` versions, since
this would effectively make older VS-Code versions unsupported. I also
changed `@types/vscode` to be a precise version match, since we are
claiming compatibility with that version via the `enginges.vscode`
property.
This commit adds support for column breakpoints to lldb-dap
To do so, support for the `breakpointLocations` request was
added. To find all available breakpoint positions, we iterate over
the line table.
The `setBreakpoints` request already forwarded the column correctly to
`SBTarget::BreakpointCreateByLocation`. However, `SourceBreakpointMap`
did not keep track of multiple breakpoints in the same line. To do so,
the `SourceBreakpointMap` is now indexed by line+column instead of by
line only.
This was previously submitted as #113787, but got reverted due to
failures on ARM and macOS. This second attempt has less strict test
case expectations. Also, I added a release note.
This reverts commit a774de807e56c1147d4630bfec3110c11d41776e.
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".
I just noticed with these changes lldb-dap was using 200% of my CPU and
root causing the issue it seems that lldb_private::Pipe::Read() (without
a timeout) is using a timeout of `std::chrono::microseconds::zero()`
which ends up setting the SelectHelper timeout to `now + 0` (see
7ceef1b182/lldb/source/Host/posix/PipePosix.cpp (L314)
and
7ceef1b182/lldb/source/Utility/SelectHelper.cpp (L46))
which causes SelectHelper to return immediately with a timeout error. As
a result the `lldb_dap::OutputRedirector::RedirectTo()` to turn into a
busy loop.
Additionally, the 'read' call is waiting until the output buffer is full
before returning, which prevents any partial output (see
7ceef1b182/lldb/source/Host/posix/PipePosix.cpp (L325C9-L326C17)).
This is not the desired behavior for lldb-dap. Instead we want a write
to the FD to result in a callback to send the DAP Output event, which
mean we want partial output.
To mitigate this, I'm reverting the reading operation to the previous
behavior before 873426bea3dd67d80dd10650e64e91c69796614f but keeping the
refactored structure and thread management.
The published VSIX for the LLDB DAP extension contains a bunch of
unnecessary files:
```
❯ tar tf llvm-vs-code-extensions.lldb-dap-0.2.8.vsix
extension.vsixmanifest
[Content_Types].xml
extension/.github/workflows/auto_publish.yml
extension/.github/workflows/integrate_llvmproject.yml
extension/.gitignore
extension/.vscode/launch.json
extension/.vscode/tasks.json
extension/LICENSE.TXT
extension/out/debug-adapter-factory.js
extension/out/debug-adapter-factory.js.map
extension/out/disposable-context.js
extension/out/disposable-context.js.map
extension/out/extension.js
extension/out/extension.js.map
extension/out/types.js
extension/out/types.js.map
extension/package.json
extension/README.md
extension/src-ts/debug-adapter-factory.ts
extension/src-ts/disposable-context.ts
extension/src-ts/extension.ts
extension/src-ts/types.ts
extension/syntaxes/arm.disasm
extension/syntaxes/arm64.disasm
extension/syntaxes/disassembly.json
extension/syntaxes/x86.disasm
extension/tsconfig.json
```
All that's really needed is the package.json, license, README, syntaxes
folder, and compiled sources. This PR adds a `.vscodeignore` file that
requires files and directories to be explicitly added to the VSIX.
Contents of the VSIX after applying this change and running `npm run
package`:
```
❯ tar tf out/lldb-dap.vsix
extension.vsixmanifest
[Content_Types].xml
extension/LICENSE.TXT
extension/out/debug-adapter-factory.js
extension/out/debug-adapter-factory.js.map
extension/out/disposable-context.js
extension/out/disposable-context.js.map
extension/out/extension.js
extension/out/extension.js.map
extension/package.json
extension/README.md
extension/syntaxes/arm.disasm
extension/syntaxes/arm64.disasm
extension/syntaxes/disassembly.json
extension/syntaxes/x86.disasm
```
I did a very basic sanity check of installing the packaged extension and
debugging a simple swift application in VS Code to make sure the
extension was still functional.
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 22561cfb443267905d4190f0e2a738e6b412457f and fixes
b7b9ccf44988edf49886743ae5c3cf4184db211f (#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>
This moves the ownership of the threads that forward stdout/stderr to
the DAP object itself to ensure that the threads are joined and that the
forwarding is cleaned up when the DAP connection is disconnected.
This is part of a larger refactor to allow lldb-dap to run in a
listening mode and accept multiple connections.
This reverts the previous revert and now that the underlying Windows
issue was fixed by 3ea2b546a8d17014d3ecf05356ecfaadf26ed846.
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 added SBProgress (#119052), and during that original commit I
tested if the progress event was sent over LLDB-DAP, and it was. However
upon the suggestion of @JDevlieghere and @labath we added an external
category (#120171), which I did not test.
This small patch wires up DAP to listen for external events by default,
and adds the external category to the SBDebugger enumeration.
This moves the ownership of the threads that forward stdout/stderr to
the DAP object itself to ensure that the threads are joined and that the
forwarding is cleaned up when the DAP connection is disconnected.
This is part of a larger refactor to allow lldb-dap to run in a
listening mode and accept multiple connections.
---------
Co-authored-by: Pavel Labath <pavel@labath.sk>
Apologies for the large change, I looked for ways to break this up and
all of the ones I saw added real complexity. This change focuses on the
option's prefixed names and the array of prefixes. These are present in
every option and the dominant source of dynamic relocations for PIE or
PIC users of LLVM and Clang tooling. In some cases, 100s or 1000s of
them for the Clang driver which has a huge number of options.
This PR addresses this by building a string table and a prefixes table
that can be referenced with indices rather than pointers that require
dynamic relocations. This removes almost 7k dynmaic relocations from the
`clang` binary, roughly 8% of the remaining dynmaic relocations outside
of vtables. For busy-boxing use cases where many different option tables
are linked into the same binary, the savings add up a bit more.
The string table is a straightforward mechanism, but the prefixes
required some subtlety. They are encoded in a Pascal-string fashion with
a size followed by a sequence of offsets. This works relatively well for
the small realistic prefixes arrays in use.
Lots of code has to change in order to land this though: both all the
option library code has to be updated to use the string table and
prefixes table, and all the users of the options library have to be
updated to correctly instantiate the objects.
Some follow-up patches in the works to provide an abstraction for this
style of code, and to start using the same technique for some of the
other strings here now that the infrastructure is in place.
I am a member of Microsoft vcpkg, due to there are new changes merged by
microsoft/STL#5105, which revealed a conformance issue in `llvm`. It
must add include `<chrono>` to fix this error.
Compiler error with this STL change:
```
D:\b\llvm\src\org-18.1.6-e754cb1d0b.clean\lldb\tools\lldb-dap\ProgressEvent.h(79): error C2039: 'system_clock': is not a member of 'std::chrono'
D:\b\llvm\src\org-18.1.6-e754cb1d0b.clean\lldb\tools\lldb-dap\ProgressEvent.cpp(134): error C3083: 'system_clock': the symbol to the left of a '::' must be a type
D:\b\llvm\src\org-18.1.6-e754cb1d0b.clean\lldb\tools\lldb-dap\ProgressEvent.cpp(134): error C2039: 'now': is not a member of 'std::chrono'
```
Currently, the link to the issue tracker takes you to the Github source
repository, rather than the Github issue tracker. This fixes the link
and includes the lldb-dap label in both the issue and PR URL.
Support finding the lldb-dap binary with `xcrun` on Darwin or in PATH on
all other platforms.
Unfortunately, this PR is larger than I would like because it removes
the `lldbDapOptions`. I believe these options are not necessary, and as
previously implemented, they caused a spurious warning with this change.
The problem was that the options were created before the custom factory.
By moving the creation logic into the factory, we make sure it's only
called after the factory has been registered. The upside is that this
simplifies the code and removes a level of indirection.
The markdown tables in the README aren't getting rendered correctly on
the LLDB-DAP page in the Visual Studio arketplace [1]. This is a
somewhat speculative fix as the table itself appears to be correct. Even
if this change doesn't fix it, the new formatting significantly improves
the readability.
[1] https://marketplace.visualstudio.com/items?itemName=llvm-vs-code-extensions.lldb-dap
