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.
Recognize the visionOS Triple::OSType::XROS os type. Some of these have
already been landed on main, but I reviewed the downstream sources and
there were a few that still needed to be landed upstream.
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>
**Note:** The register reading and writing depends on new register
flavor support in thread_get_state/thread_set_state in the kernel, which
will be first available in macOS 15.4.
The Apple M4 line of cores includes the Scalable Matrix Extension (SME)
feature. The M4s do not implement Scalable Vector Extension (SVE),
although the processor is in Streaming SVE Mode when the SME is being
used. The most obvious side effects of being in SSVE Mode are that (on
the M4 cores) NEON instructions cannot be used, and watchpoints may get
false positives, the address comparisons are done at a lowered
granularity.
When SSVE mode is enabled, the kernel will provide the Streaming Vector
Length register, which is a maximum of 64 bytes with the M4. Also
provided are SVCR (with bits indicating if SSVE mode and SME mode are
enabled), TPIDR2, SVL. Then the SVE registers Z0..31 (SVL bytes long),
P0..15 (SVL/8 bytes), the ZA matrix register (SVL*SVL bytes), and the M4
supports SME2, so the ZT0 register (64 bytes).
When SSVE/SME are disabled, none of these registers are provided by the
kernel - reads and writes of them will fail.
Unlike Linux, lldb cannot modify the SVL through a thread_set_state
call, or change the processor state's SSVE/SME status. There is also no
way for a process to request a lowered SVL size today, so the work that
David did to handle VL/SVL changing while stepping through a process is
not an issue on Darwin today. But debugserver should be providing
everything necessary so we can reuse all of David's work on resizing the
register contexts in lldb if it happens in the future. debugbserver
sends svl, svcr, and tpidr2 in the expedited registers when a thread
stops, if SSVE|SME mode are enabled (if the kernel allows it to read the
ARM_SME_STATE register set).
While the maximum SVL is 64 bytes on M4, the AArch64 maximum possible
SVL is 256; this would give us a 64k ZA register. If debugserver sized
all of its register contexts assuming the largest possible SVL, we could
easily use 2MB more memory for the register contexts of all threads in a
process -- and on iOS et al, processes must run within a small memory
allotment and this would push us over that.
Much of the work in debugserver was changing the arm64 register context
from being a static compile-time array of register sets, to being
initialized at runtime if debugserver is running on a machine with SME.
The ZA is only created to the machine's actual maximum SVL. The size of
the 32 SVE Z registers is less significant so I am statically allocating
those to the architecturally largest possible SVL value today.
Also, debugserver includes information about registers that share the
same part of the register file. e.g. S0 and D0 are the lower parts of
the NEON 128-bit V0 register. And when running on an SME machine, v0 is
the lower 128 bits of the SVE Z0 register. So the register maps used
when defining the VFP registers must differ depending on the
capabilities of the cpu at runtime.
I also changed register reading in debugserver, where formerly when
debugserver was asked to read a register, and the thread_get_state read
of that register failed, it would return all zero's. This is necessary
when constructing a `g` packet that gets all registers - because there
is no separation between register bytes, the offsets are fixed. But when
we are asking for a single register (e.g. Z0) when not in SSVE/SME mode,
this should return an error.
This does mean that when you're running on an SME capabable machine, but
not in SME mode, and do `register read -a`, lldb will report that 48 SVE
registers were unavailable and 5 SME registers were unavailable. But
that's only when `-a` is used.
The register reading and writing depends on new register flavor support
in thread_get_state/thread_set_state in the kernel, which is not yet in
a release. The test case I wrote is skipped on current OSes. I pilfered
the SME register setup from some of David's existing SME test files;
there were a few Linux specific details in those tests that they weren't
easy to reuse on Darwin.
rdar://121608074
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.
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
