Skip to content

Conversation

JDevlieghere
Copy link

No description provided.

ashgti and others added 29 commits October 9, 2025 20:35
Reapply "[lldb] Update JSONTransport to use MainLoop for reading."
(llvm#152155)

This reverts commit cd40281.

This also includes some updates to try to address the platforms with
failing tests.

I updated the JSONTransport and tests to use std::function instead of
llvm:unique_function. I think the tests were failing due to the
unique_function not being moved correctly in the loop on some platforms.

(cherry picked from commit 45d4e84)
This is to unblock CI. Disabling the tests while I investigate the
timeouts.

(cherry picked from commit 3a36070)
This should reduce flakes observed in the Fuchsia AArch64 Linux LLDB CI
builders.

(cherry picked from commit 986d7aa)
… and transport layer. (llvm#153121)

This abstracts the base Transport handler to have a MessageHandler
component and allows us to generalize both JSON-RPC 2.0 for MCP (or an
LSP) and DAP format.

This should allow us to create clearly defined clients and servers for
protocols, both for testing and for RPC between the lldb instances and
an lldb-mcp multiplexer.

This basic model is inspiried by the clangd/Transport.h file and the
mlir/lsp-server-support/Transport.h that are both used for LSP servers
within the llvm project.

Additionally, this helps with testing by subclassing `Transport` to
allow us to simplify sending/receiving messages without needing to use a
toJSON/fromJSON and a pair of pipes, see `TestTransport` in
DAP/TestBase.h.

(cherry picked from commit 538bd83)
…40626 (llvm#153836)

- VS Code extension:
- Add module symbol table viewer using
[Tabulator](https://tabulator.info/) for sorting and formatting rows.
  - Add context menu action to the modules tree.
 - lldb-dap
   -  Add `DAPGetModuleSymbolsRequest` to get symbols from a module.

Fixes llvm#140626

[Screencast From 2025-08-15
19-12-33.webm](https://github.com/user-attachments/assets/75e2f229-ac82-487c-812e-3ea33a575b70)

(cherry picked from commit 8b64cd8)
Originally commited in 362b9d7 and then
reverted in cb63b75.

This re-lands a subset of the changes to
dap_server.py/DebugCommunication and addresses the python3.10
compatibility issue.

This includes less type annotations since those were the reason for the
failures on that specific version of python.

I've done additional testing on python3.8, python3.10 and python3.13 to
further validate these changes.

(cherry picked from commit 13eca52)
…)"

This reverts commit 13eca52.

This change broke greendragon lldb test:

lldb-api.tools/lldb-dap/moduleSymbols.TestDAP_moduleSymbols.py

(cherry picked from commit 0f33b90)
)" (llvm#154832)

This reverts commit 0f33b90 and
includes a fix for the added test that was submitted between my last
update and pull.

(cherry picked from commit 36d07ad)
…llvm#140626 (llvm#153836)"

This reverts commit 8b64cd8.

This breaks lldb-aarch64-* bots causing a crash in lldb-dap while
running test TestDAP_moduleSymbols.py

https://lab.llvm.org/buildbot/#/builders/59/builds/22959
https://lab.llvm.org/buildbot/#/builders/141/builds/10975
(cherry picked from commit 2b8e806)
This migrates the CompletionHandler to structured types and adds a new
CompletionItem and CompletionItemType to the general types.

---------

Co-authored-by: Ebuka Ezike <yerimyah1@gmail.com>
(cherry picked from commit 63f1008)
Re-land the symbol table feature in lldb-dap after it was
[reverted](llvm@2b8e806)
because of a crash in the `aarch64` tests, which was caused by
dereferencing `SBSymbol::GetName` which might return `nullptr` for an
invalid symbol.

This patch reapplies the original commits and adds the missing null
check.

Also adding `skipIfWindows` for the module symbols tests, since LLDB
doesn't recognize the symbols from a.out there.

(cherry picked from commit 428ffbd)
This PR adopts JSONTransport in the MCP server implementation. It
required a slight change in design in the relationship between the two
server classes. Previously, these two had an "is-a" connection, while
now they have a "has-a" connection.

The "generic" protocol server in Protocol/MCP now operates using a
single connection (Transport). This matches the design in DAP where each
DAP instance has its own connection. The protocol server in Plugins
still supports multiple clients and creates a new server instance for
each connection.

I believe the new design makes sense in the long term (as proved by DAP)
and allows us to make the server stateful if we choose to do so. There's
no reason that multiple client support can't live in the generic
protocol library, but for now I kept it in ProtocolServerMCP to avoid
creating unnecessary abstractions.

(cherry picked from commit a49df8e)
Reverts llvm#155034 because the unit tests are flakey on
the Debian bot: https://lab.llvm.org/buildbot/#/builders/162.

(cherry picked from commit aa1dd4b)
* apply odd table rows color from vscode theme
* apply hover color from vscode theme

[Screencast From 2025-08-23
14-48-44.webm](https://github.com/user-attachments/assets/a738ac3c-3e56-4a57-b713-7430c614c415)

(cherry picked from commit 3cbbc07)
This PR adopts JSONTransport in the MCP server implementation. It
required a slight change in design in the relationship between the two
server classes. Previously, these two had an "is-a" connection, while
now they have a "has-a" connection.

The "generic" protocol server in Protocol/MCP now operates using a
single connection (Transport). This matches the design in DAP where each
DAP instance has its own connection. The protocol server in Plugins
still supports multiple clients and creates a new server instance for
each connection.

I believe the new design makes sense in the long term (as proved by DAP)
and allows us to make the server stateful if we choose to do so. There's
no reason that multiple client support can't live in the generic
protocol library, but for now I kept it in ProtocolServerMCP to avoid
creating unnecessary abstractions.

This is a reland of llvm#155034 but with significant changes to the tests.
The unit tests now test the generic server implementation, which matches
the original intent. This also means the test are now single threaded
and therefore fully deterministic using the MainLoop.

(cherry picked from commit 1ba8b36)
This patch fixes:

  lldb/unittests/Protocol/ProtocolMCPServerTest.cpp:285:14: error:
  unused variable 'mutex' [-Werror,-Wunused-variable]

(cherry picked from commit acf9611)
pathFormat is an optional field in `initializeAruguments`.

(cherry picked from commit 749537f)
This adds or renames existing types to match the names of the types on
https://modelcontextprotocol.io/specification/2025-06-18/schema for the
existing calls.

The new types are used in the unit tests and server implementation to
remove the need for crafting various `llvm::json::Object` values by
hand.

(cherry picked from commit a67257b)
These have been flakey in the last few days on Windows on Arm.

llvm#137660
(cherry picked from commit f3b542e)
Moving `lldb_protocol::mcp::MCPTransport` out of Server.h and into its
own file and simplifying the name to `Transport`.

(cherry picked from commit 71a065e)
Add the scaffolding for a new tool called lldb-mcp. This utility is meant to
replace netcat and acts a proxy between the LLM and one or more LLDB
instances. In its current form, the utility is a trivial MCP server
without any tools or resources.

(cherry picked from commit aa71d95)
Fixes lldb-mcp, aa71d95 was merged after 71a065e.

(cherry picked from commit de1baa5)
This reverts commit de1baa5.

This is needed as the other renaming commit was reverted in
1bb8db5.

(cherry picked from commit 5f3e2b6)
This fixes build errors like these:

    lldb/tools/lldb-mcp/lldb-mcp.cpp:45:41: error: use of undeclared identifier '_O_BINARY'
       45 |   int result = _setmode(fileno(stdout), _O_BINARY);
          |                                         ^
    lldb/tools/lldb-mcp/lldb-mcp.cpp:47:36: error: use of undeclared identifier '_O_BINARY'
       47 |   result = _setmode(fileno(stdin), _O_BINARY);
          |                                    ^

(cherry picked from commit 1ec0688)
Fixes linker error:

```
/usr/bin/ld: /tmp/lto-llvm-013f16.o: in function `lldb_private::JSONTransport<lldb_protocol::mcp::Request, lldb_protocol::mcp::Response, lldb_protocol::mcp::Notification>::OnRead(lldb_private::MainLoopBase&, lldb_private::Transport<lldb_protocol::mcp::Request, lldb_protocol::mcp::Response, lldb_protocol::mcp::Notification>::MessageHandler&)':
/usr/bin/../lib/gcc/x86_64-redhat-linux/15/../../../../include/c++/15/bits/unique_ptr.h:1085:(.text._ZN12lldb_private13JSONTransportIN13lldb_protocol3mcp7RequestENS2_8ResponseENS2_12NotificationEE6OnReadERNS_12MainLoopBaseERNS_9TransportIS3_S4_S5_E14MessageHandlerE+0x7fc): undefined reference to `lldb_private::TransportUnhandledContentsError::TransportUnhandledContentsError(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >)'
```

(cherry picked from commit a4ac04d)
…vm#153536)

# Changes
1. Add a new debug configuration field called `debugAdapterEnv`. It accepts a string-valued dictionary or array.
    1. This input format is consistent with the (program's) `env` debug configuration.
2. In the adapter descriptor factory, honor the said field before looking at the VS Code settings `Lldb-dap: Environment `.
    1. This order is consistent with how things are done for other debug configuration fields, e.g. lldb-dap path and args.
3. In the lldb-dap server, note down the environment entries as a part of the adapter spawn info (now it becomes "path + args + env"), and prompt the user to restart server if such info has changed.

# Motivation
1. Adapter environment can be set in `launch.json`.
2. Other debugger extensions can invoke the lldb-dap extension with adapter environment (via debug configuration).

# Tests
See PR llvm#153536.

(cherry picked from commit 20b8664)
It is flakey lately on our Windows on Arm bot:
https://lab.llvm.org/buildbot/#/builders/141/builds/11169

======================================================================

ERROR: test_startDebugging (TestDAP_startDebugging.TestDAP_startDebugging.test_startDebugging)

   Tests the "startDebugging" reverse request. It makes sure that the IDE can

----------------------------------------------------------------------

Traceback (most recent call last):

  File "C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\packages\Python\lldbsuite\test\lldbtest.py", line 2067, in tearDown

    Base.tearDown(self)

  File "C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\packages\Python\lldbsuite\test\lldbtest.py", line 1105, in tearDown

    hook()  # try the plain call and hope it works

    ^^^^^^

  File "C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\packages\Python\lldbsuite\test\tools\lldb-dap\lldbdap_testcase.py", line 518, in cleanup

    self.dap_server.terminate()

  File "C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\llvm-project\lldb\packages\Python\lldbsuite\test\tools\lldb-dap\dap_server.py", line 1593, in terminate

    raise DebugAdapterProcessError(process.returncode)

dap_server.DebugAdapterProcessError: lldb-dap returned non-zero exit status 1.

Config=aarch64-C:\Users\tcwg\llvm-worker\lldb-aarch64-windows\build\bin\clang.exe

----------------------------------------------------------------------

See llvm#137660.

I am aware that I am disabling these tests with little thought, and I would
like to be more careful with it but I don't have the knowledge to really
debug these issues.

Having buildbot results we can properly triage is more important to
me personally than finding whatever the underlying issue is. I'm sure
DAP experts will figure it out eventually.

(cherry picked from commit 7dd879b)
@JDevlieghere
Copy link
Author

@swift-ci test

@JDevlieghere
Copy link
Author

@swift-ci test windows
@swift-ci test macos

@JDevlieghere
Copy link
Author

@swift-ci test windows

1 similar comment
@JDevlieghere
Copy link
Author

@swift-ci test windows

@JDevlieghere
Copy link
Author

Failed were TestDAP_io.py & TestDAP_step.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.