Skip to content

🍒 Backport lldb-dap changes before JSONTransport change #11598

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 18 commits into from
Oct 10, 2025
Merged

🍒 Backport lldb-dap changes before JSONTransport change #11598

merged 18 commits into from
Oct 10, 2025

Conversation

JDevlieghere
Copy link

This PR backports the DAP changes leading up to the big JSONTransport refactor.

royitaqi and others added 16 commits October 8, 2025 16:10
@royitaqi @JDevlieghere
[lldb] Fix a crash in lldb-server during RemoveSoftwareBreakpoint() (l…
e4156c5 
…lvm#148738)

# Lldb-server crash

We have seen stacks like the following in lldb-server core dumps:
```
[
  "__GI___pthread_kill at pthread_kill.c:46",
  "__GI_raise at raise.c:26",
  "__GI_abort at abort.c:100",
  "__assert_fail_base at assert.c:92",
  "__GI___assert_fail at assert.c:101",
  "lldb_private::NativeProcessProtocol::RemoveSoftwareBreakpoint(unsigned long) at /redacted/lldb-server:0"
]
```

# Hypothesis of root cause

In `NativeProcessProtocol::RemoveSoftwareBreakpoint()`
([code](https://github.com/llvm/llvm-project/blob/19b2dd9d798c124406b0124a1b8debb711675281/lldb/source/Host/common/NativeProcessProtocol.cpp#L359-L423)),
a `ref_count` is asserted and reduced. If it becomes zero, the code
first go through a series of memory reads and writes to remove the
breakpoint trap opcode and to restore the original process code, then,
if everything goes fine, removes the entry from the map
`m_software_breakpoints` at the end of the function.

However, if any of the validations for the above reads and writes goes
wrong, the code returns an error early, skipping the removal of the
entry. This leaves the entry behind with a `ref_count` of zero.

The next call to `NativeProcessProtocol::RemoveSoftwareBreakpoint()` for
the same breakpoint[*] would violate the assertion about `ref_count > 0`
([here](https://github.com/llvm/llvm-project/blob/19b2dd9d798c124406b0124a1b8debb711675281/lldb/source/Host/common/NativeProcessProtocol.cpp#L365)),
which would cause a crash.

[*] We haven't found a *regular* way to repro such a next call in lldb
or lldb-dap. This is because both of them remove the breakpoint from
their internal list when they get any response from the lldb-server (OK
or error). Asking the client to delete the breakpoint a second time
doesn't trigger the client to send the `$z` gdb packet to lldb-server.
We are able to trigger the crash by sending the `$z` packet directly,
see "Manual test" below.

# Fix

Lift the removal of the map entry to be immediately after the decrement
of `ref_count`, before the early returns. This ensures that the asserted
case will never happen. The validation errors can still happen, and
whether they happen or not, the breakpoint has been removed from the
perspective of the lldb-server (same as that of lldb and lldb-dap).

# Manual test & unit test

See PR.

(cherry picked from commit a13712e)
@kusmour @JDevlieghere
[lldb-dap] Fix type req->arguments == "disconnect" (llvm#149446)
af47436 
This typo was introduced in PR llvm#140331. This branch will never get
executed. We also set the `disconnecting = true` in the
`DAP::Disconnect()` so I am not sure if we need it in both places.

(cherry picked from commit de453e8)
@SLTozer @JDevlieghere
[lldb-dap] Allow returning multiple breakpoints in "stopped" event (l…
96a3aaf 
…lvm#149133)

Currently, the "stopped" event returned when a breakpoint is hit will
always return only the ID of first breakpoint returned from
`GetStopReasonDataAtIndex`. This is slightly different from the
behaviour in `lldb`, where multiple breakpoints can exist at a single
instruction address and all are returned as part of the stop reason when
that address is hit.

This patch allows all multiple hit breakpoints to be returned in the
"stopped" event, both in the hitBreakpointIds field and in the
description, using the same formatting as lldb e.g. "breakpoint 1.1
2.1". I'm not aware of any effect this will have on debugger plugins; as
far as I can tell, it makes no difference within the VS Code UI - this
just fixes a minor issue encountered while writing an `lldb-dap` backend
for Dexter.

(cherry picked from commit d544005)
@woruyu @da-viper @JDevlieghere
[lldb-dap] support moduleId in the stackTrace response (llvm#149774)
f984e48 
This PR resolves llvm#149316

---------

Co-authored-by: Ebuka Ezike <yerimyah1@gmail.com>
(cherry picked from commit 0b9470b)
@royitaqi @JDevlieghere
[vscode-lldb] Fix format (llvm#151829)
aa1f117 
Generated by running the formatter:
```
royshi-mac-home ~/public_llvm/llvm-project/lldb/tools/lldb-dap % yarn format
yarn run v1.22.22
warning package.json: License should be a valid SPDX license expression
warning lldb-dap@0.2.15: The engine "vscode" appears to be invalid.
$ npx prettier './src-ts/' --write
src-ts/debug-adapter-factory.ts 65ms
src-ts/debug-configuration-provider.ts 17ms (unchanged)
src-ts/debug-session-tracker.ts 11ms (unchanged)
src-ts/disposable-context.ts 2ms (unchanged)
src-ts/extension.ts 3ms (unchanged)
src-ts/lldb-dap-server.ts 7ms
src-ts/ui/error-with-notification.ts 4ms (unchanged)
src-ts/ui/modules-data-provider.ts 5ms (unchanged)
src-ts/ui/show-error-message.ts 6ms (unchanged)
src-ts/uri-launch-handler.ts 5ms (unchanged)
✨  Done in 0.99s.
```

(cherry picked from commit cd83444)
@award999 @JDevlieghere
Logging setup for lldb-dap extension (llvm#146884)
22ea1d5 
- ~Add `winston` dependency (MIT license) to handle logging setup~
- Have an `LogOutputChannel` to log user facing information, errors,
warnings
- Write a debug session logs under the provided `logUri` to capture
further diagnostics when the `lldb-dap.captureSessionLogs` setting is
enabled. *Note* the `lldb-dap.log-path` setting takes precedence when
set
Issue: llvm#146880

---------

Co-authored-by: Jonas Devlieghere <jonas@devlieghere.com>
(cherry picked from commit ae7be39)
@JDevlieghere
[lldb-dap] Bump the version to 0.2.16
0890afc 
(cherry picked from commit dd0bb2c)
@royitaqi @JDevlieghere
[vscode-lldb] Fix yarn package (llvm#152002)
8a8ead6 
# Problem
`yarn package` cannot be run twice in a row - the second time will error
out:
> Error: ENOENT: no such file or directory, stat
'<path>/llvm-project/lldb/tools/lldb-dap/out/lldb-dap.vsix'

This error is also weird, because the file actually exists. See the end
of this [full
output](https://gist.github.com/royitaqi/f3f4838ed30d7ade846f53f0fb7d68f4).

# Fix

Delete the `lldb-dap.vsix` file at the start of each run. See
consecutive runs [being
successful](https://gist.github.com/royitaqi/9609181b4fe6a8a4e71880c36cd0c7c9).

(cherry picked from commit 4882874)
@DavidSpickett @JDevlieghere
[lldb] Treat address found via function name as a callable address (l…
cdadfab 
…lvm#151973)

I discovered this building the lldb test suite with `-mthumb` set, so
that all test programs are purely Arm Thumb code.

When C++ expressions did a function lookup, they took a different path
from C programs. That path happened to land on the line that I've
changed. Where we try to look something up as a function, but don't then
resolve the address as if it's a callable.

With Thumb, if you do the non-callable lookup, the bottom bit won't be
set. This means when lldb's expression wrapper function branches into
the found function, it'll be in Arm mode trying to execute Thumb code.

Thumb is the only instance where you'd notice this. Aside from maybe
MicroMIPS or MIPS16 perhaps but I expect that there are 0 users of that
and lldb.

I have added a new test case that will simulate this situation in
"normal" Arm builds so that it will get run on Linaro's buildbot.

This change also fixes the following existing tests when `-mthumb` is
used:
```
  lldb-api :: commands/expression/anonymous-struct/TestCallUserAnonTypedef.py
  lldb-api :: commands/expression/argument_passing_restrictions/TestArgumentPassingRestrictions.py
  lldb-api :: commands/expression/call-function/TestCallStopAndContinue.py
  lldb-api :: commands/expression/call-function/TestCallUserDefinedFunction.py
  lldb-api :: commands/expression/char/TestExprsChar.py
  lldb-api :: commands/expression/class_template_specialization_empty_pack/TestClassTemplateSpecializationParametersHandling.py
  lldb-api :: commands/expression/context-object/TestContextObject.py
  lldb-api :: commands/expression/formatters/TestFormatters.py
  lldb-api :: commands/expression/import_base_class_when_class_has_derived_member/TestImportBaseClassWhenClassHasDerivedMember.py
  lldb-api :: commands/expression/inline-namespace/TestInlineNamespace.py
  lldb-api :: commands/expression/namespace-alias/TestInlineNamespaceAlias.py
  lldb-api :: commands/expression/no-deadlock/TestExprDoesntBlock.py
  lldb-api :: commands/expression/pr35310/TestExprsBug35310.py
  lldb-api :: commands/expression/static-initializers/TestStaticInitializers.py
  lldb-api :: commands/expression/test/TestExprs.py
  lldb-api :: commands/expression/timeout/TestCallWithTimeout.py
  lldb-api :: commands/expression/top-level/TestTopLevelExprs.py
  lldb-api :: commands/expression/unwind_expression/TestUnwindExpression.py
  lldb-api :: commands/expression/xvalue/TestXValuePrinting.py
  lldb-api :: functionalities/thread/main_thread_exit/TestMainThreadExit.py
  lldb-api :: lang/cpp/call-function/TestCallCPPFunction.py
  lldb-api :: lang/cpp/chained-calls/TestCppChainedCalls.py
  lldb-api :: lang/cpp/class-template-parameter-pack/TestClassTemplateParameterPack.py
  lldb-api :: lang/cpp/constructors/TestCppConstructors.py
  lldb-api :: lang/cpp/function-qualifiers/TestCppFunctionQualifiers.py
  lldb-api :: lang/cpp/function-ref-qualifiers/TestCppFunctionRefQualifiers.py
  lldb-api :: lang/cpp/global_operators/TestCppGlobalOperators.py
  lldb-api :: lang/cpp/llvm-style/TestLLVMStyle.py
  lldb-api :: lang/cpp/multiple-inheritance/TestCppMultipleInheritance.py
  lldb-api :: lang/cpp/namespace/TestNamespace.py
  lldb-api :: lang/cpp/namespace/TestNamespaceLookup.py
  lldb-api :: lang/cpp/namespace_conflicts/TestNamespaceConflicts.py
  lldb-api :: lang/cpp/operators/TestCppOperators.py
  lldb-api :: lang/cpp/overloaded-functions/TestOverloadedFunctions.py
  lldb-api :: lang/cpp/rvalue-references/TestRvalueReferences.py
  lldb-api :: lang/cpp/static_methods/TestCPPStaticMethods.py
  lldb-api :: lang/cpp/template/TestTemplateArgs.py
  lldb-api :: python_api/thread/TestThreadAPI.py
```

There are other failures that are due to different problems, and this
change does not make those worse.

(I have no plans to run the test suite with `-mthumb` regularly, I just
did it to test some other refactoring)

(cherry picked from commit 4077e66)
@royitaqi @JDevlieghere
[vscode-lldb] Fix race condition when changing lldb-dap arguments (ll…
b3e2b85 
…vm#151828)

# Problem

When the user changes lldb-dap's arguments (e.g. path), there is a race
condition, where the new lldb-dap process could be started first and
have set the extension's `serverProcess` and `serverInfo` according to
the new process, while the old lldb-dap process exits later and wipes
out these two fields.

Consequences:
1. This causes `getServerProcess()` to return `undefined` when it should
return the new process.
2. This also causes wrong behavior when starting the next debug session
that a new lldb-dap process will be started and the old not reused nor
killed.

# Fix

When wiping the two fields, check if `serverProcess` equals to the
process captured by the handler. If they equal, wipe the fields. If not,
then the fields have already been updated (either new process has
started, or the fields were already wiped out by another handler), and
so the wiping should be skipped.

(cherry picked from commit bb2642f)
@royitaqi @JDevlieghere
[vscode-lldb] Add VS Code commands for high level debug workflow (llv…
dd49810 
…m#151827)

This allows other debugger extensions to leverage the `lldb-dap`
extension's settings and logic (e.g. "Server Mode").

Other debugger extensions can invoke these commands to resolve
configuration, create adapter descriptor, and get the `lldb-dap` process
for state tracking, additional interaction, and telemetry.

VS Code commands added:
* `lldb-dap.resolveDebugConfiguration`
* `lldb-dap.resolveDebugConfigurationWithSubstitutedVariables`
* `lldb-dap.createDebugAdapterDescriptor`
* `lldb-dap.getServerProcess`

(cherry picked from commit 0e0ea71)
@DavidSpickett @JDevlieghere
[lldb][test] Disable TestDAP_memory.py on 32-bit Arm Linux
3c6c6b2 
This has been very flakey lately:
======================================================================
ERROR: test_writeMemory (TestDAP_memory.TestDAP_memory)
   Tests the 'writeMemory' request
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/test/API/tools/lldb-dap/memory/TestDAP_memory.py", line 202, in test_writeMemory
    mem_response = self.writeMemory(
  File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py", line 545, in writeMemory
    response = self.dap_server.request_writeMemory(
  File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py", line 850, in request_writeMemory
    return self.send_recv(command_dict)
  File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py", line 403, in send_recv
    raise ValueError(desc)
ValueError: no response for "writeMemory"
Config=arm-/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/bin/clang
======================================================================
ERROR: test_writeMemory (TestDAP_memory.TestDAP_memory)
   Tests the 'writeMemory' request
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 2067, in tearDown
    Base.tearDown(self)
  File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/lldbtest.py", line 1105, in tearDown
    hook()  # try the plain call and hope it works
  File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py", line 486, in cleanup
    self.dap_server.request_disconnect(terminateDebuggee=True)
  File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py", line 799, in request_disconnect
    return self.send_recv(command_dict)
  File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py", line 397, in send_recv
    self.send_packet(command)
  File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py", line 349, in send_packet
    self.send.flush()
BrokenPipeError: [Errno 32] Broken pipe
Config=arm-/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/bin/clang
----------------------------------------------------------------------

General tracking issue - llvm#137660

(cherry picked from commit b800930)
@eronnen @JDevlieghere
[lldb-dap] persistent assembly breakpoints (llvm#148061)
2612df6 
Resolves llvm#141955

- Adds data to breakpoints `Source` object, in order for assembly
breakpoints, which rely on a temporary `sourceReference` value, to be
able to resolve in future sessions like normal path+line breakpoints
- Adds optional `instructions_offset` parameter to `BreakpointResolver`

(cherry picked from commit 4d3feae)
@kazutakahirata @JDevlieghere
[lldb] Fix warnings
a2d0c7b 
This patch fixes:

  lldb/unittests/DAP/ProtocolTypesTest.cpp:112:67: error: missing
  field 'adapterData' initializer
  [-Werror,-Wmissing-field-initializers]

  lldb/unittests/DAP/ProtocolTypesTest.cpp:571:70: error: missing
  field 'adapterData' initializer
  [-Werror,-Wmissing-field-initializers]

(cherry picked from commit f091b40)
@DavidSpickett @JDevlieghere
[lldb][lldb-dap][test] Disable part of TestDAP_launch on Arm 32-bit
5534767 
This test has been flakey on our bot:
https://lab.llvm.org/buildbot/#/builders/18/builds/20410

```
======================================================================
FAIL: test_extra_launch_commands (TestDAP_launch.TestDAP_launch)
    Tests the "launchCommands" with extra launching settings
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py", line 482, in test_extra_launch_commands
    self.verify_commands("stopCommands", output, stopCommands)
  File "/home/tcwg-buildbot/worker/lldb-arm-ubuntu/llvm-project/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py", line 228, in verify_commands
    self.assertTrue(
AssertionError: False is not true : verify 'frame variable' found in console output for 'stopCommands'
Config=arm-/home/tcwg-buildbot/worker/lldb-arm-ubuntu/build/bin/clang
----------------------------------------------------------------------
```

Likely a timing issue waiting for the command output on a slower
machine.

General tracking issue - llvm#137660

(cherry picked from commit 4f65345)
@DavidSpickett @JDevlieghere
[lldb][lldb-dap][test] Correct skip in TestDAP_launch
592682a 
Fixes 4f65345

Yet again I forgot it's skip[I]f.

(cherry picked from commit 0fca1e4)
@JDevlieghere
Copy link
Author

@swift-ci test

@JDevlieghere JDevlieghere mentioned this pull request Oct 8, 2025
Merged
@JDevlieghere
Copy link
Author

Related: #11599

@JDevlieghere
Copy link
Author

@swift-ci test

@DavidSpickett @JDevlieghere
[lldb][test] Fix typo in decorator
4078c20 
(cherry picked from commit 61d569f)
@JDevlieghere
Copy link
Author

@swift-ci test

@JDevlieghere JDevlieghere merged commit f744935 into stable/21.x Oct 10, 2025
3 checks passed
@JDevlieghere JDevlieghere deleted the lldb-dap-backport-before-json branch October 10, 2025 03:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@JDevlieghere @royitaqi @kusmour @SLTozer @woruyu @award999 @DavidSpickett @eronnen @kazutakahirata