https://github.com/da-viper updated https://github.com/llvm/llvm-project/pull/175930
>From b696426f9577059791921fde5ae57f783fe45685 Mon Sep 17 00:00:00 2001 From: Ebuka Ezike <[email protected]> Date: Wed, 14 Jan 2026 11:34:57 +0000 Subject: [PATCH 1/2] [lldb-dap] Move targetId and debuggerId into a session property This makes it clear the fields required for attaching to an existing debug session. It also makes it easier to check mutually exclusive fields required to attach. --- .../test/tools/lldb-dap/dap_server.py | 9 +++---- .../tools/lldb-dap/attach/TestDAP_attach.py | 13 +++++----- lldb/tools/lldb-dap/DAP.cpp | 7 +++--- lldb/tools/lldb-dap/DAP.h | 9 +++---- .../lldb-dap/Handler/AttachRequestHandler.cpp | 17 +++++++------ .../lldb-dap/Protocol/ProtocolRequests.cpp | 24 ++++++++++--------- .../lldb-dap/Protocol/ProtocolRequests.h | 16 +++++++++---- lldb/tools/lldb-dap/extension/package.json | 18 +++++++++++--- lldb/unittests/DAP/ProtocolTypesTest.cpp | 9 +++++++ 9 files changed, 72 insertions(+), 50 deletions(-) diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py index 875cfb6c6b197..f3f7a168db028 100644 --- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py +++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py @@ -874,8 +874,7 @@ def request_attach( *, program: Optional[str] = None, pid: Optional[int] = None, - debuggerId: Optional[int] = None, - targetId: Optional[int] = None, + session: Optional[dict[str, int]] = None, waitFor=False, initCommands: Optional[list[str]] = None, preRunCommands: Optional[list[str]] = None, @@ -895,10 +894,8 @@ def request_attach( args_dict["pid"] = pid if program is not None: args_dict["program"] = program - if debuggerId is not None: - args_dict["debuggerId"] = debuggerId - if targetId is not None: - args_dict["targetId"] = targetId + if session is not None: + args_dict["session"] = session if waitFor: args_dict["waitFor"] = waitFor args_dict["initCommands"] = self.init_commands diff --git a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py index 5810be67a2bf9..629eb657b38f6 100644 --- a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py +++ b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py @@ -94,7 +94,7 @@ def test_by_name_waitFor(self): if self.spawn_thread.is_alive(): self.spawn_thread.join(timeout=10) - def test_attach_with_missing_debuggerId_or_targetId(self): + def test_attach_with_missing_session_debugger(self): """ Test that attaching with only one of debuggerId/targetId specified fails with the expected error message. @@ -102,14 +102,14 @@ def test_attach_with_missing_debuggerId_or_targetId(self): self.build_and_create_debug_adapter() # Test with only targetId specified (no debuggerId) - resp = self.attach(targetId=99999, waitForResponse=True) + session = {"targetId": 99999} + resp = self.attach(session=session, waitForResponse=True) self.assertFalse(resp["success"]) - self.assertIn( - "Both 'debuggerId' and 'targetId' must be specified together", + self.assertIn("missing value at arguments.session.debuggerId", resp["body"]["error"]["format"], ) - def test_attach_with_invalid_debuggerId_and_targetId(self): + def test_attach_with_invalid_session(self): """ Test that attaching with both debuggerId and targetId specified but invalid fails with an appropriate error message. @@ -119,7 +119,8 @@ def test_attach_with_invalid_debuggerId_and_targetId(self): # Attach with both debuggerId=9999 and targetId=99999 (both invalid). # Since debugger ID 9999 likely doesn't exist in the global registry, # we expect a validation error. - resp = self.attach(debuggerId=9999, targetId=99999, waitForResponse=True) + session = {"debuggerId": 9999, "targetId": 9999} + resp = self.attach(session=session, waitForResponse=True) self.assertFalse(resp["success"]) error_msg = resp["body"]["error"]["format"] # Either error is acceptable - both indicate the debugger reuse diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 47468d517d333..c8ba2f73a871a 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -1310,17 +1310,16 @@ void DAP::StartEventThreads() { StartEventThread(); } -llvm::Error DAP::InitializeDebugger(int debugger_id, - lldb::user_id_t target_id) { +llvm::Error DAP::InitializeDebugger(const DAPSession &session) { // Find the existing debugger by ID - debugger = lldb::SBDebugger::FindDebuggerWithID(debugger_id); + debugger = lldb::SBDebugger::FindDebuggerWithID(session.debuggerId); if (!debugger.IsValid()) { return llvm::createStringError( "Unable to find existing debugger for debugger ID"); } // Find the target within the debugger by its globally unique ID - lldb::SBTarget target = debugger.FindTargetByGloballyUniqueID(target_id); + lldb::SBTarget target = debugger.FindTargetByGloballyUniqueID(session.targetId); if (!target.IsValid()) { return llvm::createStringError( "Unable to find existing target for target ID"); diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index e1cd5c29a3907..8d56a226dbb28 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -432,12 +432,9 @@ struct DAP final : public DAPTransport::MessageHandler { /// Perform complete DAP initialization by reusing an existing debugger and /// target. /// - /// \param[in] debugger_id - /// The ID of the existing debugger to reuse. - /// - /// \param[in] target_id - /// The globally unique ID of the existing target to reuse. - llvm::Error InitializeDebugger(int debugger_id, lldb::user_id_t target_id); + /// \param[in] session + /// A session consisting of an existing debugger and target. + llvm::Error InitializeDebugger(const protocol::DAPSession &session); /// Start event handling threads based on client capabilities. void StartEventThreads(); diff --git a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp index 2261924af9ccb..41e0551373aa7 100644 --- a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp @@ -30,12 +30,10 @@ namespace lldb_dap { Error AttachRequestHandler::Run(const AttachRequestArguments &args) const { // Initialize DAP debugger and related components if not sharing previously // launched debugger. - std::optional<int> debugger_id = args.debuggerId; - std::optional<lldb::user_id_t> target_id = args.targetId; + std::optional<DAPSession> session = args.session; - if (Error err = debugger_id && target_id - ? dap.InitializeDebugger(*debugger_id, *target_id) - : dap.InitializeDebugger()) + if (Error err = + session ? dap.InitializeDebugger(*session) : dap.InitializeDebugger()) return err; dap.SetConfiguration(args.configuration, /*is_attach=*/true); @@ -59,12 +57,13 @@ Error AttachRequestHandler::Run(const AttachRequestArguments &args) const { lldb::SBError error; lldb::SBTarget target; - if (target_id) { + if (session) { // Use the unique target ID to get the target. - target = dap.debugger.FindTargetByGloballyUniqueID(*target_id); + target = dap.debugger.FindTargetByGloballyUniqueID(session->targetId); if (!target.IsValid()) { error.SetErrorString( - llvm::formatv("invalid target_id {0} in attach config", *target_id) + llvm::formatv("invalid targetId {0} in attach config", + session->targetId) .str() .c_str()); } @@ -120,7 +119,7 @@ Error AttachRequestHandler::Run(const AttachRequestArguments &args) const { connect_url += std::to_string(args.gdbRemotePort); dap.target.ConnectRemote(listener, connect_url.c_str(), "gdb-remote", error); - } else if (!target_id.has_value()) { + } else if (!session) { // Attach by pid or process name. lldb::SBAttachInfo attach_info; if (args.pid != LLDB_INVALID_PROCESS_ID) diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp index 1f8283eba86fa..23789f6d46f90 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp @@ -331,6 +331,15 @@ bool fromJSON(const json::Value &Params, LaunchRequestArguments &LRA, return true; } +bool fromJSON(const llvm::json::Value &Params, DAPSession &Ses, + llvm::json::Path P) { + + json::ObjectMapper O(Params, P); + // Validate that both debuggerID and targetId are provided. + return O && O.map("targetId", Ses.targetId) && + O.map("debuggerId", Ses.debuggerId); +} + bool fromJSON(const json::Value &Params, AttachRequestArguments &ARA, json::Path P) { json::ObjectMapper O(Params, P); @@ -341,16 +350,15 @@ bool fromJSON(const json::Value &Params, AttachRequestArguments &ARA, O.mapOptional("gdb-remote-port", ARA.gdbRemotePort) && O.mapOptional("gdb-remote-hostname", ARA.gdbRemoteHostname) && O.mapOptional("coreFile", ARA.coreFile) && - O.mapOptional("targetId", ARA.targetId) && - O.mapOptional("debuggerId", ARA.debuggerId); + O.mapOptional("session", ARA.session); if (!success) return false; // Validate that we have a well formed attach request. if (ARA.attachCommands.empty() && ARA.coreFile.empty() && ARA.configuration.program.empty() && ARA.pid == LLDB_INVALID_PROCESS_ID && - ARA.gdbRemotePort == LLDB_DAP_INVALID_PORT && !ARA.targetId.has_value()) { + ARA.gdbRemotePort == LLDB_DAP_INVALID_PORT && !ARA.session.has_value()) { P.report("expected one of 'pid', 'program', 'attachCommands', " - "'coreFile', 'gdb-remote-port', or 'targetId' to be specified"); + "'coreFile', 'gdb-remote-port', or 'session' to be specified"); return false; } // Check if we have mutually exclusive arguments. @@ -359,13 +367,7 @@ bool fromJSON(const json::Value &Params, AttachRequestArguments &ARA, P.report("'pid' and 'gdb-remote-port' are mutually exclusive"); return false; } - // Validate that both debugger_id and target_id are provided together. - if (ARA.debuggerId.has_value() != ARA.targetId.has_value()) { - P.report( - "Both 'debuggerId' and 'targetId' must be specified together for " - "debugger reuse, or both must be omitted to create a new debugger"); - return false; - } + return true; } diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h index 9a99af9068ef1..fa826563963a5 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h @@ -317,6 +317,15 @@ using LaunchResponse = VoidResponse; /// An invalid 'frameId' default value. #define LLDB_DAP_INVALID_FRAME_ID UINT64_MAX +struct DAPSession { + /// Unique ID of an existing target to attach to. + lldb::user_id_t targetId; + + /// ID of an existing debugger instance to use. + int debuggerId; +}; +bool fromJSON(const llvm::json::Value &, DAPSession &, llvm::json::Path); + /// lldb-dap specific attach arguments. struct AttachRequestArguments { /// Common lldb-dap configuration values for launching/attaching operations. @@ -351,11 +360,8 @@ struct AttachRequestArguments { /// Path to the core file to debug. std::string coreFile; - /// Unique ID of an existing target to attach to. - std::optional<lldb::user_id_t> targetId; - - /// ID of an existing debugger instance to use. - std::optional<int> debuggerId; + /// An Existing session that consist of a target and debugger. + std::optional<DAPSession> session; /// @} }; diff --git a/lldb/tools/lldb-dap/extension/package.json b/lldb/tools/lldb-dap/extension/package.json index 2a5104fc61dbe..c90609cf7f760 100644 --- a/lldb/tools/lldb-dap/extension/package.json +++ b/lldb/tools/lldb-dap/extension/package.json @@ -795,9 +795,21 @@ "description": "Custom commands that are executed instead of attaching to a process ID or to a process by name. These commands may optionally create a new target and must perform an attach. A valid process must exist after these commands complete or the \"attach\" will fail.", "default": [] }, - "targetId": { - "type": "number", - "description": "The globally unique target id to attach to. Used when a target is dynamically created." + "session": { + "type": "object", + "properties": { + "targetId": { + "type": "number", + "description": "The globally unique target id to attach to. Use when a target is dynamically created." + }, + "debuggerId": { + "type": "number", + "description": "ID of an existing debugger instance to use." + }, + "additionalProperties": false, + "required": ["targetId", "debuggerId"] + }, + "description": "This property is Experimental.\nAttach to an existing debugging session.\nAdded in version 22.0." }, "initCommands": { "type": "array", diff --git a/lldb/unittests/DAP/ProtocolTypesTest.cpp b/lldb/unittests/DAP/ProtocolTypesTest.cpp index 2dd795d28172a..6476492813455 100644 --- a/lldb/unittests/DAP/ProtocolTypesTest.cpp +++ b/lldb/unittests/DAP/ProtocolTypesTest.cpp @@ -1262,3 +1262,12 @@ TEST(ProtocolTypesTest, StackFrame) { ASSERT_THAT_EXPECTED(expected_frame, llvm::Succeeded()); EXPECT_EQ(pp(*expected_frame), pp(frame)); } + +TEST(ProtocolTypesTest, DAPSession) { + const DAPSession session{/*targetId*/ 1000, /*debuggerId*/ 300}; + + auto expected = parse<DAPSession>(R"({"targetId": 1000, "debuggerId": 300})"); + ASSERT_THAT_EXPECTED(expected, llvm::Succeeded()); + EXPECT_EQ(expected->debuggerId, session.debuggerId); + EXPECT_EQ(expected->targetId, session.targetId); +} >From 62d0fdd93e0b062d28490e2db3963351dea25bd8 Mon Sep 17 00:00:00 2001 From: Ebuka Ezike <[email protected]> Date: Wed, 14 Jan 2026 11:46:44 +0000 Subject: [PATCH 2/2] format files --- lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py | 3 ++- lldb/tools/lldb-dap/DAP.cpp | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py index 629eb657b38f6..becf793d2dafb 100644 --- a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py +++ b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py @@ -105,7 +105,8 @@ def test_attach_with_missing_session_debugger(self): session = {"targetId": 99999} resp = self.attach(session=session, waitForResponse=True) self.assertFalse(resp["success"]) - self.assertIn("missing value at arguments.session.debuggerId", + self.assertIn( + "missing value at arguments.session.debuggerId", resp["body"]["error"]["format"], ) diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index c8ba2f73a871a..f09575ce8ba42 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -1319,7 +1319,8 @@ llvm::Error DAP::InitializeDebugger(const DAPSession &session) { } // Find the target within the debugger by its globally unique ID - lldb::SBTarget target = debugger.FindTargetByGloballyUniqueID(session.targetId); + lldb::SBTarget target = + debugger.FindTargetByGloballyUniqueID(session.targetId); if (!target.IsValid()) { return llvm::createStringError( "Unable to find existing target for target ID"); _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
