[Lldb-commits] [lldb] 37ce62a - [lldb-dap] Migrate restart request to structured types (#172488)
via lldb-commits
lldb-commits at lists.llvm.org
Sat Dec 20 04:58:17 PST 2025
Author: Sergei Druzhkov
Date: 2025-12-20T15:58:13+03:00
New Revision: 37ce62a7063b8d852aaba10d7f8037ec618ebe10
URL: https://github.com/llvm/llvm-project/commit/37ce62a7063b8d852aaba10d7f8037ec618ebe10
DIFF: https://github.com/llvm/llvm-project/commit/37ce62a7063b8d852aaba10d7f8037ec618ebe10.diff
LOG: [lldb-dap] Migrate restart request to structured types (#172488)
This patch migrates `restart` request to structured types. Also, I added
some checks that at least one of the required fields was provided for
`launch` and `attach` requests. Maybe I missed some possible
configurations, so please double check.
Added:
Modified:
lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py
lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp
lldb/tools/lldb-dap/Handler/RequestHandler.h
lldb/tools/lldb-dap/Handler/RestartRequestHandler.cpp
lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
lldb/unittests/DAP/ProtocolRequestsTest.cpp
Removed:
################################################################################
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 d9acd0a4cae01..ce1b965d3cd0d 100644
--- a/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py
+++ b/lldb/test/API/tools/lldb-dap/attach/TestDAP_attach.py
@@ -88,7 +88,7 @@ def test_attach_with_missing_debuggerId_or_targetId(self):
resp = self.attach(targetId=99999, expectFailure=True)
self.assertFalse(resp["success"])
self.assertIn(
- "Both debuggerId and targetId must be specified together",
+ "Both 'debuggerId' and 'targetId' must be specified together",
resp["body"]["error"]["format"],
)
diff --git a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
index fe49cdc8414f2..647ce055a32f8 100644
--- a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
+++ b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
@@ -57,7 +57,7 @@ def test_failing_launch_commands_and_console(self):
)
self.assertFalse(response["success"])
self.assertTrue(self.get_dict_value(response, ["body", "error", "showUser"]))
- self.assertEqual(
+ self.assertIn(
"'launchCommands' and non-internal 'console' are mutually exclusive",
self.get_dict_value(response, ["body", "error", "format"]),
)
diff --git a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
index f0996eb3ff0f4..1c14dcd16e5e6 100644
--- a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
@@ -35,33 +35,11 @@ Error AttachRequestHandler::Run(const AttachRequestArguments &args) const {
std::optional<int> debugger_id = args.debuggerId;
std::optional<lldb::user_id_t> target_id = args.targetId;
- // Validate that both debugger_id and target_id are provided together.
- if (debugger_id.has_value() != target_id.has_value()) {
- return llvm::createStringError(
- "Both debuggerId and targetId must be specified together for debugger "
- "reuse, or both must be omitted to create a new debugger");
- }
-
if (Error err = debugger_id && target_id
? dap.InitializeDebugger(*debugger_id, *target_id)
: dap.InitializeDebugger())
return err;
- // Validate that we have a well formed attach request.
- if (args.attachCommands.empty() && args.coreFile.empty() &&
- args.configuration.program.empty() &&
- args.pid == LLDB_INVALID_PROCESS_ID &&
- args.gdbRemotePort == LLDB_DAP_INVALID_PORT && !target_id.has_value())
- return make_error<DAPError>(
- "expected one of 'pid', 'program', 'attachCommands', "
- "'coreFile', 'gdb-remote-port', or target_id to be specified");
-
- // Check if we have mutually exclusive arguments.
- if ((args.pid != LLDB_INVALID_PROCESS_ID) &&
- (args.gdbRemotePort != LLDB_DAP_INVALID_PORT))
- return make_error<DAPError>(
- "'pid' and 'gdb-remote-port' are mutually exclusive");
-
dap.SetConfiguration(args.configuration, /*is_attach=*/true);
if (!args.coreFile.empty())
dap.stop_at_entry = true;
diff --git a/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp
index 329f0a7bf6453..33563cc7a5cee 100644
--- a/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp
@@ -26,12 +26,6 @@ Error LaunchRequestHandler::Run(const LaunchRequestArguments &arguments) const {
if (Error err = dap.InitializeDebugger())
return err;
- // Validate that we have a well formed launch request.
- if (!arguments.launchCommands.empty() &&
- arguments.console != protocol::eConsoleInternal)
- return make_error<DAPError>(
- "'launchCommands' and non-internal 'console' are mutually exclusive");
-
dap.SetConfiguration(arguments.configuration, /*is_attach=*/false);
dap.last_launch_request = arguments;
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h
index 0669be50597e9..a18a8049c804d 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.h
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h
@@ -340,11 +340,14 @@ class LaunchRequestHandler
void PostRun() const override;
};
-class RestartRequestHandler : public LegacyRequestHandler {
+class RestartRequestHandler
+ : public RequestHandler<std::optional<protocol::RestartArguments>,
+ protocol::RestartResponse> {
public:
- using LegacyRequestHandler::LegacyRequestHandler;
+ using RequestHandler::RequestHandler;
static llvm::StringLiteral GetCommand() { return "restart"; }
- void operator()(const llvm::json::Object &request) const override;
+ llvm::Error
+ Run(const std::optional<protocol::RestartArguments> &args) const override;
};
class NextRequestHandler
diff --git a/lldb/tools/lldb-dap/Handler/RestartRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/RestartRequestHandler.cpp
index 100173bfc3082..72580d2224a18 100644
--- a/lldb/tools/lldb-dap/Handler/RestartRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/RestartRequestHandler.cpp
@@ -7,91 +7,37 @@
//===----------------------------------------------------------------------===//
#include "DAP.h"
+#include "DAPError.h"
#include "EventHelper.h"
-#include "JSONUtils.h"
#include "LLDBUtils.h"
#include "Protocol/ProtocolRequests.h"
#include "RequestHandler.h"
-#include "llvm/Support/JSON.h"
-#include "llvm/Support/raw_ostream.h"
-namespace lldb_dap {
+using namespace lldb_dap;
+using namespace lldb_dap::protocol;
-// "RestartRequest": {
-// "allOf": [ { "$ref": "#/definitions/Request" }, {
-// "type": "object",
-// "description": "Restarts a debug session. Clients should only call this
-// request if the corresponding capability `supportsRestartRequest` is
-// true.\nIf the capability is missing or has the value false, a typical
-// client emulates `restart` by terminating the debug adapter first and then
-// launching it anew.",
-// "properties": {
-// "command": {
-// "type": "string",
-// "enum": [ "restart" ]
-// },
-// "arguments": {
-// "$ref": "#/definitions/RestartArguments"
-// }
-// },
-// "required": [ "command" ]
-// }]
-// },
-// "RestartArguments": {
-// "type": "object",
-// "description": "Arguments for `restart` request.",
-// "properties": {
-// "arguments": {
-// "oneOf": [
-// { "$ref": "#/definitions/LaunchRequestArguments" },
-// { "$ref": "#/definitions/AttachRequestArguments" }
-// ],
-// "description": "The latest version of the `launch` or `attach`
-// configuration."
-// }
-// }
-// },
-// "RestartResponse": {
-// "allOf": [ { "$ref": "#/definitions/Response" }, {
-// "type": "object",
-// "description": "Response to `restart` request. This is just an
-// acknowledgement, so no body field is required."
-// }]
-// },
-void RestartRequestHandler::operator()(
- const llvm::json::Object &request) const {
- llvm::json::Object response;
- FillResponse(request, response);
- if (!dap.target.GetProcess().IsValid()) {
- response["success"] = llvm::json::Value(false);
- EmplaceSafeString(response, "message",
- "Restart request received but no process was launched.");
- dap.SendJSON(llvm::json::Value(std::move(response)));
- return;
- }
+/// Restarts a debug session. Clients should only call this request if the
+/// corresponding capability `supportsRestartRequest` is true.
+/// If the capability is missing or has the value false, a typical client
+/// emulates `restart` by terminating the debug adapter first and then launching
+/// it anew.
+llvm::Error
+RestartRequestHandler::Run(const std::optional<RestartArguments> &args) const {
+ if (!dap.target.GetProcess().IsValid())
+ return llvm::make_error<DAPError>(
+ "Restart request received but no process was launched.");
- const llvm::json::Object *arguments = request.getObject("arguments");
- if (arguments) {
- // The optional `arguments` field in RestartRequest can contain an updated
- // version of the launch arguments. If there's one, use it.
- if (const llvm::json::Value *restart_arguments =
- arguments->get("arguments")) {
- protocol::LaunchRequestArguments updated_arguments;
- llvm::json::Path::Root root;
- if (!fromJSON(*restart_arguments, updated_arguments, root)) {
- response["success"] = llvm::json::Value(false);
- EmplaceSafeString(
- response, "message",
- llvm::formatv("Failed to parse updated launch arguments: {0}",
- llvm::toString(root.getError()))
- .str());
- dap.SendJSON(llvm::json::Value(std::move(response)));
- return;
- }
- dap.last_launch_request = updated_arguments;
+ if (args) {
+ if (std::holds_alternative<AttachRequestArguments>(args->arguments))
+ return llvm::make_error<DAPError>(
+ "Restarting an AttachRequest is not supported.");
+ if (const auto *arguments =
+ std::get_if<LaunchRequestArguments>(&args->arguments);
+ arguments) {
+ dap.last_launch_request = *arguments;
// Update DAP configuration based on the latest copy of the launch
// arguments.
- dap.SetConfiguration(updated_arguments.configuration, false);
+ dap.SetConfiguration(arguments->configuration, false);
dap.ConfigureSourceMaps();
}
}
@@ -108,7 +54,8 @@ void RestartRequestHandler::operator()(
ScopeSyncMode scope_sync_mode(dap.debugger);
lldb::StateType state = process.GetState();
if (state != lldb::eStateConnected) {
- process.Kill();
+ if (lldb::SBError error = process.Kill(); error.Fail())
+ return ToError(error);
}
// Clear the list of thread ids to avoid sending "thread exited" events
// for threads of the process we are terminating.
@@ -117,29 +64,16 @@ void RestartRequestHandler::operator()(
// FIXME: Should we run 'preRunCommands'?
// FIXME: Should we add a 'preRestartCommands'?
- if (llvm::Error err = LaunchProcess(*dap.last_launch_request)) {
- response["success"] = llvm::json::Value(false);
- EmplaceSafeString(response, "message", llvm::toString(std::move(err)));
- dap.SendJSON(llvm::json::Value(std::move(response)));
- return;
- }
+ if (llvm::Error error = LaunchProcess(*dap.last_launch_request))
+ return error;
SendProcessEvent(dap, Launch);
// This is normally done after receiving a "configuration done" request.
// Because we're restarting, configuration has already happened so we can
// continue the process right away.
- if (dap.stop_at_entry) {
- if (llvm::Error err = SendThreadStoppedEvent(dap, /*on_entry=*/true)) {
- EmplaceSafeString(response, "message", llvm::toString(std::move(err)));
- dap.SendJSON(llvm::json::Value(std::move(response)));
- return;
- }
- } else {
- dap.target.GetProcess().Continue();
- }
+ if (dap.stop_at_entry)
+ return SendThreadStoppedEvent(dap, /*on_entry=*/true);
- dap.SendJSON(llvm::json::Value(std::move(response)));
+ return ToError(dap.target.GetProcess().Continue());
}
-
-} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
index bf470b78077df..c87111d8f1b78 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
@@ -296,31 +296,70 @@ bool fromJSON(const json::Value &Params, Console &C, json::Path P) {
bool fromJSON(const json::Value &Params, LaunchRequestArguments &LRA,
json::Path P) {
json::ObjectMapper O(Params, P);
- return O && fromJSON(Params, LRA.configuration, P) &&
- O.mapOptional("noDebug", LRA.noDebug) &&
- O.mapOptional("launchCommands", LRA.launchCommands) &&
- O.mapOptional("cwd", LRA.cwd) && O.mapOptional("args", LRA.args) &&
- O.mapOptional("detachOnError", LRA.detachOnError) &&
- O.mapOptional("disableASLR", LRA.disableASLR) &&
- O.mapOptional("disableSTDIO", LRA.disableSTDIO) &&
- O.mapOptional("shellExpandArguments", LRA.shellExpandArguments) &&
- O.mapOptional("runInTerminal", LRA.console) &&
- O.mapOptional("console", LRA.console) &&
- O.mapOptional("stdio", LRA.stdio) && parseEnv(Params, LRA.env, P);
+ bool success =
+ O && fromJSON(Params, LRA.configuration, P) &&
+ O.mapOptional("noDebug", LRA.noDebug) &&
+ O.mapOptional("launchCommands", LRA.launchCommands) &&
+ O.mapOptional("cwd", LRA.cwd) && O.mapOptional("args", LRA.args) &&
+ O.mapOptional("detachOnError", LRA.detachOnError) &&
+ O.mapOptional("disableASLR", LRA.disableASLR) &&
+ O.mapOptional("disableSTDIO", LRA.disableSTDIO) &&
+ O.mapOptional("shellExpandArguments", LRA.shellExpandArguments) &&
+ O.mapOptional("runInTerminal", LRA.console) &&
+ O.mapOptional("console", LRA.console) &&
+ O.mapOptional("stdio", LRA.stdio) && parseEnv(Params, LRA.env, P);
+ if (!success)
+ return false;
+ // Validate that we have a well formed launch request.
+ if (!LRA.launchCommands.empty() &&
+ LRA.console != protocol::eConsoleInternal) {
+ P.report(
+ "'launchCommands' and non-internal 'console' are mutually exclusive");
+ return false;
+ }
+ if (LRA.configuration.program.empty() && LRA.launchCommands.empty()) {
+ P.report("'program' or 'launchCommands' should be provided");
+ return false;
+ }
+ return true;
}
bool fromJSON(const json::Value &Params, AttachRequestArguments &ARA,
json::Path P) {
json::ObjectMapper O(Params, P);
- return O && fromJSON(Params, ARA.configuration, P) &&
- O.mapOptional("attachCommands", ARA.attachCommands) &&
- O.mapOptional("pid", ARA.pid) &&
- O.mapOptional("waitFor", ARA.waitFor) &&
- 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);
+ bool success = O && fromJSON(Params, ARA.configuration, P) &&
+ O.mapOptional("attachCommands", ARA.attachCommands) &&
+ O.mapOptional("pid", ARA.pid) &&
+ O.mapOptional("waitFor", ARA.waitFor) &&
+ 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);
+ 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()) {
+ P.report("expected one of 'pid', 'program', 'attachCommands', "
+ "'coreFile', 'gdb-remote-port', or 'targetId' to be specified");
+ return false;
+ }
+ // Check if we have mutually exclusive arguments.
+ if ((ARA.pid != LLDB_INVALID_PROCESS_ID) &&
+ (ARA.gdbRemotePort != LLDB_DAP_INVALID_PORT)) {
+ 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;
}
bool fromJSON(const json::Value &Params, ContinueArguments &CA, json::Path P) {
@@ -737,4 +776,30 @@ llvm::json::Value toJSON(const TestGetTargetBreakpointsResponseBody &Body) {
return result;
}
+bool fromJSON(const llvm::json::Value &Params, RestartArguments &Args,
+ llvm::json::Path Path) {
+ const json::Object *O = Params.getAsObject();
+ if (!O) {
+ Path.report("expected object");
+ return false;
+ }
+ const json::Value *arguments = O->get("arguments");
+ if (arguments == nullptr)
+ return true;
+ LaunchRequestArguments launchArguments;
+ llvm::json::Path::Root root;
+ if (fromJSON(*arguments, launchArguments, root)) {
+ Args.arguments = std::move(launchArguments);
+ return true;
+ }
+ AttachRequestArguments attachArguments;
+ if (fromJSON(*arguments, attachArguments, root)) {
+ Args.arguments = std::move(attachArguments);
+ return true;
+ }
+ Path.report(
+ "failed to parse arguments, expected `launch` or `attach` arguments");
+ return false;
+}
+
} // namespace lldb_dap::protocol
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
index cc123943ec0b6..33fcaae1710b5 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
@@ -31,6 +31,7 @@
#include <cstdint>
#include <optional>
#include <string>
+#include <variant>
#include <vector>
namespace lldb_dap::protocol {
@@ -1255,6 +1256,18 @@ struct TestGetTargetBreakpointsResponseBody {
};
llvm::json::Value toJSON(const TestGetTargetBreakpointsResponseBody &);
+/// Arguments for `restart` request.
+struct RestartArguments {
+ /// The latest version of the `launch` or `attach` configuration.
+ std::variant<std::monostate, LaunchRequestArguments, AttachRequestArguments>
+ arguments = std::monostate{};
+};
+bool fromJSON(const llvm::json::Value &, RestartArguments &, llvm::json::Path);
+
+/// Response to `restart` request. This is just an acknowledgement, so no body
+/// field is required.
+using RestartResponse = VoidResponse;
+
} // namespace lldb_dap::protocol
#endif
diff --git a/lldb/unittests/DAP/ProtocolRequestsTest.cpp b/lldb/unittests/DAP/ProtocolRequestsTest.cpp
index 710b78960ef09..c639e40453fb0 100644
--- a/lldb/unittests/DAP/ProtocolRequestsTest.cpp
+++ b/lldb/unittests/DAP/ProtocolRequestsTest.cpp
@@ -304,3 +304,43 @@ TEST(ProtocolRequestsTest, TestGetTargetBreakpointsResponseBody) {
ASSERT_THAT_EXPECTED(expected, llvm::Succeeded());
EXPECT_EQ(PrettyPrint(*expected), PrettyPrint(body));
}
+
+TEST(ProtocolRequestsTest, RestartArguments) {
+ llvm::Expected<RestartArguments> expected = parse<RestartArguments>(R"({})");
+ ASSERT_THAT_EXPECTED(expected, llvm::Succeeded());
+ EXPECT_TRUE(std::holds_alternative<std::monostate>(expected->arguments));
+
+ // Check missed keys.
+ expected = parse<RestartArguments>(R"({
+ "arguments": {}
+ })");
+ EXPECT_THAT_EXPECTED(expected,
+ FailedWithMessage("failed to parse arguments, expected "
+ "`launch` or `attach` arguments"));
+
+ // Check launch arguments.
+ expected = parse<RestartArguments>(R"({
+ "arguments": {
+ "program": "main.exe",
+ "cwd": "/home/root"
+ }
+ })");
+ ASSERT_THAT_EXPECTED(expected, llvm::Succeeded());
+ const LaunchRequestArguments *launch_args =
+ std::get_if<LaunchRequestArguments>(&expected->arguments);
+ EXPECT_NE(launch_args, nullptr);
+ EXPECT_EQ(launch_args->configuration.program, "main.exe");
+ EXPECT_EQ(launch_args->cwd, "/home/root");
+
+ // Check attach arguments.
+ expected = parse<RestartArguments>(R"({
+ "arguments": {
+ "pid": 123
+ }
+ })");
+ ASSERT_THAT_EXPECTED(expected, llvm::Succeeded());
+ const AttachRequestArguments *attach_args =
+ std::get_if<AttachRequestArguments>(&expected->arguments);
+ EXPECT_NE(attach_args, nullptr);
+ EXPECT_EQ(attach_args->pid, 123U);
+}
More information about the lldb-commits
mailing list