Skip to content

Commit 777e29d

Browse files
da-viperc-rhodes
da-viper
authored and
c-rhodes
committed
[lldb-dap] Move targetId and debuggerId into a session property (#175930)
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. (cherry picked from commit 6977e68)
1 parent cc8df4d commit 777e29d

File tree

11 files changed

+86
-60
lines changed

11 files changed

+86
-60
lines changed

‎lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -865,8 +865,7 @@ def request_attach(
865865
*,
866866
program: Optional[str] = None,
867867
pid: Optional[int] = None,
868-
debuggerId: Optional[int] = None,
869-
targetId: Optional[int] = None,
868+
session: Optional[dict[str, int]] = None,
870869
waitFor=False,
871870
initCommands: Optional[list[str]] = None,
872871
preRunCommands: Optional[list[str]] = None,
@@ -886,10 +885,8 @@ def request_attach(
886885
args_dict["pid"] = pid
887886
if program is not None:
888887
args_dict["program"] = program
889-
if debuggerId is not None:
890-
args_dict["debuggerId"] = debuggerId
891-
if targetId is not None:
892-
args_dict["targetId"] = targetId
888+
if session is not None:
889+
args_dict["session"] = session
893890
if waitFor:
894891
args_dict["waitFor"] = waitFor
895892
args_dict["initCommands"] = self.init_commands

‎lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -94,22 +94,23 @@ def test_by_name_waitFor(self):
9494
if self.spawn_thread.is_alive():
9595
self.spawn_thread.join(timeout=10)
9696

97-
def test_attach_with_missing_debuggerId_or_targetId(self):
97+
def test_attach_with_missing_session_debugger(self):
9898
"""
9999
Test that attaching with only one of debuggerId/targetId specified
100100
fails with the expected error message.
101101
"""
102102
self.build_and_create_debug_adapter()
103103

104104
# Test with only targetId specified (no debuggerId)
105-
resp = self.attach(targetId=99999, waitForResponse=True)
105+
session = {"targetId": 99999}
106+
resp = self.attach(session=session, waitForResponse=True)
106107
self.assertFalse(resp["success"])
107108
self.assertIn(
108-
"Both 'debuggerId' and 'targetId' must be specified together",
109+
"missing value at arguments.session.debuggerId",
109110
resp["body"]["error"]["format"],
110111
)
111112

112-
def test_attach_with_invalid_debuggerId_and_targetId(self):
113+
def test_attach_with_invalid_session(self):
113114
"""
114115
Test that attaching with both debuggerId and targetId specified but
115116
invalid fails with an appropriate error message.
@@ -119,7 +120,8 @@ def test_attach_with_invalid_debuggerId_and_targetId(self):
119120
# Attach with both debuggerId=9999 and targetId=99999 (both invalid).
120121
# Since debugger ID 9999 likely doesn't exist in the global registry,
121122
# we expect a validation error.
122-
resp = self.attach(debuggerId=9999, targetId=99999, waitForResponse=True)
123+
session = {"debuggerId": 9999, "targetId": 9999}
124+
resp = self.attach(session=session, waitForResponse=True)
123125
self.assertFalse(resp["success"])
124126
error_msg = resp["body"]["error"]["format"]
125127
# Either error is acceptable - both indicate the debugger reuse

‎lldb/test/API/tools/lldb-dap/startDebugging/TestDAP_startDebugging.py‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -53,13 +53,14 @@ def test_startDebugging_debugger_reuse(self):
5353

5454
# Use mock IDs to test the infrastructure
5555
# In a real scenario, these would come from the parent session
56-
test_debugger_id = 1
57-
test_target_id = 100
56+
debugger_id = 1
57+
target_id = 100
5858

5959
# Send a startDebugging request with debuggerId and targetId
6060
# This simulates creating a child DAP session that reuses the debugger
61+
session = {"session": {"debuggerId": debugger_id, "targetId": target_id}}
6162
self.dap_server.request_evaluate(
62-
f'`lldb-dap start-debugging attach \'{{"debuggerId":{test_debugger_id},"targetId":{test_target_id}}}\'',
63+
f"`lldb-dap start-debugging attach '{json.dumps(session)}'",
6364
context="repl",
6465
)
6566

@@ -76,14 +77,14 @@ def test_startDebugging_debugger_reuse(self):
7677
self.assertEqual(request["command"], "startDebugging")
7778
self.assertEqual(request["arguments"]["request"], "attach")
7879

79-
config = request["arguments"]["configuration"]
80+
session = request["arguments"]["configuration"]["session"]
8081
self.assertEqual(
81-
config["debuggerId"],
82-
test_debugger_id,
82+
session["debuggerId"],
83+
debugger_id,
8384
"Reverse request should include debugger ID",
8485
)
8586
self.assertEqual(
86-
config["targetId"],
87-
test_target_id,
87+
session["targetId"],
88+
target_id,
8889
"Reverse request should include target ID",
8990
)

‎lldb/tools/lldb-dap/DAP.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1310,17 +1310,17 @@ void DAP::StartEventThreads() {
13101310
StartEventThread();
13111311
}
13121312

1313-
llvm::Error DAP::InitializeDebugger(int debugger_id,
1314-
lldb::user_id_t target_id) {
1313+
llvm::Error DAP::InitializeDebugger(const DAPSession &session) {
13151314
// Find the existing debugger by ID
1316-
debugger = lldb::SBDebugger::FindDebuggerWithID(debugger_id);
1315+
debugger = lldb::SBDebugger::FindDebuggerWithID(session.debuggerId);
13171316
if (!debugger.IsValid()) {
13181317
return llvm::createStringError(
13191318
"Unable to find existing debugger for debugger ID");
13201319
}
13211320

13221321
// Find the target within the debugger by its globally unique ID
1323-
lldb::SBTarget target = debugger.FindTargetByGloballyUniqueID(target_id);
1322+
lldb::SBTarget target =
1323+
debugger.FindTargetByGloballyUniqueID(session.targetId);
13241324
if (!target.IsValid()) {
13251325
return llvm::createStringError(
13261326
"Unable to find existing target for target ID");

‎lldb/tools/lldb-dap/DAP.h‎

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -432,12 +432,9 @@ struct DAP final : public DAPTransport::MessageHandler {
432432
/// Perform complete DAP initialization by reusing an existing debugger and
433433
/// target.
434434
///
435-
/// \param[in] debugger_id
436-
/// The ID of the existing debugger to reuse.
437-
///
438-
/// \param[in] target_id
439-
/// The globally unique ID of the existing target to reuse.
440-
llvm::Error InitializeDebugger(int debugger_id, lldb::user_id_t target_id);
435+
/// \param[in] session
436+
/// A session consisting of an existing debugger and target.
437+
llvm::Error InitializeDebugger(const protocol::DAPSession &session);
441438

442439
/// Start event handling threads based on client capabilities.
443440
void StartEventThreads();

‎lldb/tools/lldb-dap/EventHelper.cpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -466,11 +466,12 @@ static void HandleTargetEvent(const lldb::SBEvent &event, Log &log) {
466466
// FindTargetByGloballyUniqueID.
467467
llvm::json::Object configuration;
468468
configuration.try_emplace("type", "lldb");
469-
configuration.try_emplace("debuggerId",
470-
created_target.GetDebugger().GetID());
471-
configuration.try_emplace("targetId", created_target.GetGloballyUniqueID());
472469
configuration.try_emplace("name", created_target.GetTargetSessionName());
473470

471+
json::Object session{{"targetId", created_target.GetGloballyUniqueID()},
472+
{"debuggerId", created_target.GetDebugger().GetID()}};
473+
configuration.try_emplace("session", std::move(session));
474+
474475
llvm::json::Object request;
475476
request.try_emplace("request", "attach");
476477
request.try_emplace("configuration", std::move(configuration));

‎lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,12 +30,10 @@ namespace lldb_dap {
3030
Error AttachRequestHandler::Run(const AttachRequestArguments &args) const {
3131
// Initialize DAP debugger and related components if not sharing previously
3232
// launched debugger.
33-
std::optional<int> debugger_id = args.debuggerId;
34-
std::optional<lldb::user_id_t> target_id = args.targetId;
33+
std::optional<DAPSession> session = args.session;
3534

36-
if (Error err = debugger_id && target_id
37-
? dap.InitializeDebugger(*debugger_id, *target_id)
38-
: dap.InitializeDebugger())
35+
if (Error err =
36+
session ? dap.InitializeDebugger(*session) : dap.InitializeDebugger())
3937
return err;
4038

4139
dap.SetConfiguration(args.configuration, /*is_attach=*/true);
@@ -59,12 +57,13 @@ Error AttachRequestHandler::Run(const AttachRequestArguments &args) const {
5957

6058
lldb::SBError error;
6159
lldb::SBTarget target;
62-
if (target_id) {
60+
if (session) {
6361
// Use the unique target ID to get the target.
64-
target = dap.debugger.FindTargetByGloballyUniqueID(*target_id);
62+
target = dap.debugger.FindTargetByGloballyUniqueID(session->targetId);
6563
if (!target.IsValid()) {
6664
error.SetErrorString(
67-
llvm::formatv("invalid target_id {0} in attach config", *target_id)
65+
llvm::formatv("invalid targetId {0} in attach config",
66+
session->targetId)
6867
.str()
6968
.c_str());
7069
}
@@ -120,7 +119,7 @@ Error AttachRequestHandler::Run(const AttachRequestArguments &args) const {
120119
connect_url += std::to_string(args.gdbRemotePort);
121120
dap.target.ConnectRemote(listener, connect_url.c_str(), "gdb-remote",
122121
error);
123-
} else if (!target_id.has_value()) {
122+
} else if (!session) {
124123
// Attach by pid or process name.
125124
lldb::SBAttachInfo attach_info;
126125
if (args.pid != LLDB_INVALID_PROCESS_ID)

‎lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -324,6 +324,15 @@ bool fromJSON(const json::Value &Params, LaunchRequestArguments &LRA,
324324
return true;
325325
}
326326

327+
bool fromJSON(const llvm::json::Value &Params, DAPSession &Ses,
328+
llvm::json::Path P) {
329+
330+
json::ObjectMapper O(Params, P);
331+
// Validate that both debuggerID and targetId are provided.
332+
return O && O.map("targetId", Ses.targetId) &&
333+
O.map("debuggerId", Ses.debuggerId);
334+
}
335+
327336
bool fromJSON(const json::Value &Params, AttachRequestArguments &ARA,
328337
json::Path P) {
329338
json::ObjectMapper O(Params, P);
@@ -334,16 +343,15 @@ bool fromJSON(const json::Value &Params, AttachRequestArguments &ARA,
334343
O.mapOptional("gdb-remote-port", ARA.gdbRemotePort) &&
335344
O.mapOptional("gdb-remote-hostname", ARA.gdbRemoteHostname) &&
336345
O.mapOptional("coreFile", ARA.coreFile) &&
337-
O.mapOptional("targetId", ARA.targetId) &&
338-
O.mapOptional("debuggerId", ARA.debuggerId);
346+
O.mapOptional("session", ARA.session);
339347
if (!success)
340348
return false;
341349
// Validate that we have a well formed attach request.
342350
if (ARA.attachCommands.empty() && ARA.coreFile.empty() &&
343351
ARA.configuration.program.empty() && ARA.pid == LLDB_INVALID_PROCESS_ID &&
344-
ARA.gdbRemotePort == LLDB_DAP_INVALID_PORT && !ARA.targetId.has_value()) {
352+
ARA.gdbRemotePort == LLDB_DAP_INVALID_PORT && !ARA.session.has_value()) {
345353
P.report("expected one of 'pid', 'program', 'attachCommands', "
346-
"'coreFile', 'gdb-remote-port', or 'targetId' to be specified");
354+
"'coreFile', 'gdb-remote-port', or 'session' to be specified");
347355
return false;
348356
}
349357
// Check if we have mutually exclusive arguments.
@@ -352,13 +360,7 @@ bool fromJSON(const json::Value &Params, AttachRequestArguments &ARA,
352360
P.report("'pid' and 'gdb-remote-port' are mutually exclusive");
353361
return false;
354362
}
355-
// Validate that both debugger_id and target_id are provided together.
356-
if (ARA.debuggerId.has_value() != ARA.targetId.has_value()) {
357-
P.report(
358-
"Both 'debuggerId' and 'targetId' must be specified together for "
359-
"debugger reuse, or both must be omitted to create a new debugger");
360-
return false;
361-
}
363+
362364
return true;
363365
}
364366

‎lldb/tools/lldb-dap/Protocol/ProtocolRequests.h‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,15 @@ using LaunchResponse = VoidResponse;
317317
/// An invalid 'frameId' default value.
318318
#define LLDB_DAP_INVALID_FRAME_ID UINT64_MAX
319319

320+
struct DAPSession {
321+
/// Unique ID of an existing target to attach to.
322+
lldb::user_id_t targetId;
323+
324+
/// ID of an existing debugger instance to use.
325+
lldb::user_id_t debuggerId;
326+
};
327+
bool fromJSON(const llvm::json::Value &, DAPSession &, llvm::json::Path);
328+
320329
/// lldb-dap specific attach arguments.
321330
struct AttachRequestArguments {
322331
/// Common lldb-dap configuration values for launching/attaching operations.
@@ -351,11 +360,8 @@ struct AttachRequestArguments {
351360
/// Path to the core file to debug.
352361
std::string coreFile;
353362

354-
/// Unique ID of an existing target to attach to.
355-
std::optional<lldb::user_id_t> targetId;
356-
357-
/// ID of an existing debugger instance to use.
358-
std::optional<int> debuggerId;
363+
/// An Existing session that consist of a target and debugger.
364+
std::optional<DAPSession> session;
359365

360366
/// @}
361367
};

‎lldb/tools/lldb-dap/extension/package.json‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -795,9 +795,21 @@
795795
"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.",
796796
"default": []
797797
},
798-
"targetId": {
799-
"type": "number",
800-
"description": "The globally unique target id to attach to. Used when a target is dynamically created."
798+
"session": {
799+
"type": "object",
800+
"properties": {
801+
"targetId": {
802+
"type": "number",
803+
"description": "The globally unique target id to attach to. Use when a target is dynamically created."
804+
},
805+
"debuggerId": {
806+
"type": "number",
807+
"description": "ID of an existing debugger instance to use."
808+
},
809+
"additionalProperties": false,
810+
"required": ["targetId", "debuggerId"]
811+
},
812+
"description": "This property is Experimental.\nAttach to an existing debugging session.\nAdded in version 22.0."
801813
},
802814
"initCommands": {
803815
"type": "array",

0 commit comments

Comments
 (0)