[Lldb-commits] [lldb] [lldb-dap] Refactoring lldb-dap 'launch' request to use typed RequestHandler<>. (PR #133624)
via lldb-commits
lldb-commits at lists.llvm.org
Sat Mar 29 23:45:23 PDT 2025
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-lldb
Author: John Harrison (ashgti)
<details>
<summary>Changes</summary>
This converts a number of json::Value's into well defined types that are used throughout lldb-dap and updates the 'launch' command to use the new well defined types.
---
Patch is 68.84 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/133624.diff
22 Files Affected:
- (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py (+2-1)
- (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py (+1-1)
- (modified) lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py (+4-2)
- (modified) lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_runInTerminal.py (+2-2)
- (modified) lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py (+8-8)
- (modified) lldb/tools/lldb-dap/DAP.cpp (+54-28)
- (modified) lldb/tools/lldb-dap/DAP.h (+24-20)
- (modified) lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp (+19-14)
- (modified) lldb/tools/lldb-dap/Handler/CompletionsHandler.cpp (+4-3)
- (modified) lldb/tools/lldb-dap/Handler/EvaluateRequestHandler.cpp (+2-1)
- (modified) lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp (+29-89)
- (modified) lldb/tools/lldb-dap/Handler/RequestHandler.cpp (+51-45)
- (modified) lldb/tools/lldb-dap/Handler/RequestHandler.h (+15-4)
- (modified) lldb/tools/lldb-dap/Handler/RestartRequestHandler.cpp (+40-14)
- (modified) lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp (+2-1)
- (modified) lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp (+1-1)
- (modified) lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp (+10-10)
- (modified) lldb/tools/lldb-dap/JSONUtils.cpp (+20-28)
- (modified) lldb/tools/lldb-dap/JSONUtils.h (+6-5)
- (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp (+169-6)
- (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.h (+150)
- (modified) lldb/tools/lldb-dap/SourceBreakpoint.cpp (+3-3)
``````````diff
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 01ef4b68f2653..6e13fcddcc933 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
@@ -862,7 +862,8 @@ def request_launch(
args_dict["enableAutoVariableSummaries"] = enableAutoVariableSummaries
args_dict["enableSyntheticChildDebugging"] = enableSyntheticChildDebugging
args_dict["displayExtendedBacktrace"] = displayExtendedBacktrace
- args_dict["commandEscapePrefix"] = commandEscapePrefix
+ if commandEscapePrefix:
+ args_dict["commandEscapePrefix"] = commandEscapePrefix
command_dict = {"command": "launch", "type": "request", "arguments": args_dict}
response = self.send_recv(command_dict)
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
index 70b04b051e0ec..9ab8a905a79dd 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
@@ -443,7 +443,7 @@ def cleanup():
if not (response and response["success"]):
self.assertTrue(
- response["success"], "launch failed (%s)" % (response["message"])
+ response["success"], "launch failed (%s)" % (response["body"]["error"]["format"])
)
return response
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 64c99019a1c9b..c6a3e9cc879a4 100644
--- a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
+++ b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py
@@ -41,7 +41,9 @@ def test_termination(self):
self.dap_server.request_disconnect()
# Wait until the underlying lldb-dap process dies.
- self.dap_server.process.wait(timeout=lldbdap_testcase.DAPTestCaseBase.timeoutval)
+ self.dap_server.process.wait(
+ timeout=lldbdap_testcase.DAPTestCaseBase.timeoutval
+ )
# Check the return code
self.assertEqual(self.dap_server.process.poll(), 0)
@@ -459,7 +461,7 @@ def test_failing_launch_commands(self):
self.assertFalse(response["success"])
self.assertRegex(
- response["message"],
+ response["body"]["error"]["format"],
r"Failed to run launch commands\. See the Debug Console for more details",
)
diff --git a/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_runInTerminal.py b/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_runInTerminal.py
index 5a9938c25c2c8..a94c9860c1508 100644
--- a/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_runInTerminal.py
+++ b/lldb/test/API/tools/lldb-dap/restart/TestDAP_restart_runInTerminal.py
@@ -21,7 +21,7 @@ def isTestSupported(self):
return False
@skipIfWindows
- @skipIf(archs=["arm"]) # Always times out on buildbot
+ @skipIf(oslist=["linux"], archs=["arm"]) # Always times out on buildbot
def test_basic_functionality(self):
"""
Test basic restarting functionality when the process is running in
@@ -61,7 +61,7 @@ def test_basic_functionality(self):
)
@skipIfWindows
- @skipIf(archs=["arm"]) # Always times out on buildbot
+ @skipIf(oslist=["linux"], archs=["arm"]) # Always times out on buildbot
def test_stopOnEntry(self):
"""
Check that stopOnEntry works correctly when using runInTerminal.
diff --git a/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py b/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py
index 9141565ac1b9b..9aab7ca3293db 100644
--- a/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py
+++ b/lldb/test/API/tools/lldb-dap/runInTerminal/TestDAP_runInTerminal.py
@@ -44,7 +44,7 @@ def isTestSupported(self):
return False
@skipIfWindows
- @skipIf(archs=no_match(["x86_64"]))
+ @skipIf(oslist=["linux"], archs=no_match(["x86_64"]))
def test_runInTerminal(self):
if not self.isTestSupported():
return
@@ -90,7 +90,7 @@ def test_runInTerminal(self):
env = self.dap_server.request_evaluate("foo")["body"]["result"]
self.assertIn("bar", env)
- @skipIf(archs=no_match(["x86_64"]))
+ @skipIf(oslist=["linux"], archs=no_match(["x86_64"]))
def test_runInTerminalWithObjectEnv(self):
if not self.isTestSupported():
return
@@ -114,7 +114,7 @@ def test_runInTerminalWithObjectEnv(self):
self.assertEqual("BAR", request_envs["FOO"])
@skipIfWindows
- @skipIf(archs=no_match(["x86_64"]))
+ @skipIf(oslist=["linux"], archs=no_match(["x86_64"]))
def test_runInTerminalInvalidTarget(self):
if not self.isTestSupported():
return
@@ -133,7 +133,7 @@ def test_runInTerminalInvalidTarget(self):
)
@skipIfWindows
- @skipIf(archs=no_match(["x86_64"]))
+ @skipIf(oslist=["linux"], archs=no_match(["x86_64"]))
def test_missingArgInRunInTerminalLauncher(self):
if not self.isTestSupported():
return
@@ -148,7 +148,7 @@ def test_missingArgInRunInTerminalLauncher(self):
)
@skipIfWindows
- @skipIf(archs=no_match(["x86_64"]))
+ @skipIf(oslist=["linux"], archs=no_match(["x86_64"]))
def test_FakeAttachedRunInTerminalLauncherWithInvalidProgram(self):
if not self.isTestSupported():
return
@@ -175,7 +175,7 @@ def test_FakeAttachedRunInTerminalLauncherWithInvalidProgram(self):
self.assertIn("No such file or directory", stderr)
@skipIfWindows
- @skipIf(archs=no_match(["x86_64"]))
+ @skipIf(oslist=["linux"], archs=no_match(["x86_64"]))
def test_FakeAttachedRunInTerminalLauncherWithValidProgram(self):
if not self.isTestSupported():
return
@@ -202,7 +202,7 @@ def test_FakeAttachedRunInTerminalLauncherWithValidProgram(self):
self.assertIn("foo", stdout)
@skipIfWindows
- @skipIf(archs=no_match(["x86_64"]))
+ @skipIf(oslist=["linux"], archs=no_match(["x86_64"]))
def test_FakeAttachedRunInTerminalLauncherAndCheckEnvironment(self):
if not self.isTestSupported():
return
@@ -223,7 +223,7 @@ def test_FakeAttachedRunInTerminalLauncherAndCheckEnvironment(self):
self.assertIn("FOO=BAR", stdout)
@skipIfWindows
- @skipIf(archs=no_match(["x86_64"]))
+ @skipIf(oslist=["linux"], archs=no_match(["x86_64"]))
def test_NonAttachedRunInTerminalLauncher(self):
if not self.isTestSupported():
return
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index 23f0400c8bd4d..52dd377bc40fb 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -14,6 +14,7 @@
#include "LLDBUtils.h"
#include "OutputRedirector.h"
#include "Protocol/ProtocolBase.h"
+#include "Protocol/ProtocolRequests.h"
#include "Protocol/ProtocolTypes.h"
#include "Transport.h"
#include "lldb/API/SBBreakpoint.h"
@@ -73,11 +74,8 @@ DAP::DAP(llvm::StringRef path, Log *log, const ReplMode default_repl_mode,
std::vector<std::string> pre_init_commands, Transport &transport)
: debug_adapter_path(path), log(log), transport(transport),
broadcaster("lldb-dap"), exception_breakpoints(),
+ focus_tid(LLDB_INVALID_THREAD_ID), is_attach(false),
pre_init_commands(std::move(pre_init_commands)),
- focus_tid(LLDB_INVALID_THREAD_ID), stop_at_entry(false), is_attach(false),
- enable_auto_variable_summaries(false),
- enable_synthetic_child_debugging(false),
- display_extended_backtrace(false),
restarting_process_id(LLDB_INVALID_PROCESS_ID),
configuration_done_sent(false), waiting_for_run_in_terminal(false),
progress_event_reporter(
@@ -505,8 +503,9 @@ ReplMode DAP::DetectReplMode(lldb::SBFrame frame, std::string &expression,
bool partial_expression) {
// Check for the escape hatch prefix.
if (!expression.empty() &&
- llvm::StringRef(expression).starts_with(command_escape_prefix)) {
- expression = expression.substr(command_escape_prefix.size());
+ llvm::StringRef(expression)
+ .starts_with(configuration.commandEscapePrefix)) {
+ expression = expression.substr(configuration.commandEscapePrefix.size());
return ReplMode::Command;
}
@@ -546,7 +545,7 @@ ReplMode DAP::DetectReplMode(lldb::SBFrame frame, std::string &expression,
<< "Warning: Expression '" << term
<< "' is both an LLDB command and variable. It will be evaluated as "
"a variable. To evaluate the expression as an LLDB command, use '"
- << command_escape_prefix << "' as a prefix.\n";
+ << configuration.commandEscapePrefix << "' as a prefix.\n";
}
// Variables take preference to commands in auto, since commands can always
@@ -593,7 +592,7 @@ DAP::RunLaunchCommands(llvm::ArrayRef<std::string> launch_commands) {
}
llvm::Error DAP::RunInitCommands() {
- if (!RunLLDBCommands("Running initCommands:", init_commands))
+ if (!RunLLDBCommands("Running initCommands:", configuration.initCommands))
return createRunLLDBCommandsErrorMessage("initCommands");
return llvm::Error::success();
}
@@ -605,29 +604,31 @@ llvm::Error DAP::RunPreInitCommands() {
}
llvm::Error DAP::RunPreRunCommands() {
- if (!RunLLDBCommands("Running preRunCommands:", pre_run_commands))
+ if (!RunLLDBCommands("Running preRunCommands:", configuration.preRunCommands))
return createRunLLDBCommandsErrorMessage("preRunCommands");
return llvm::Error::success();
}
void DAP::RunPostRunCommands() {
- RunLLDBCommands("Running postRunCommands:", post_run_commands);
+ RunLLDBCommands("Running postRunCommands:", configuration.postRunCommands);
}
void DAP::RunStopCommands() {
- RunLLDBCommands("Running stopCommands:", stop_commands);
+ RunLLDBCommands("Running stopCommands:", configuration.stopCommands);
}
void DAP::RunExitCommands() {
- RunLLDBCommands("Running exitCommands:", exit_commands);
+ RunLLDBCommands("Running exitCommands:", configuration.exitCommands);
}
void DAP::RunTerminateCommands() {
- RunLLDBCommands("Running terminateCommands:", terminate_commands);
+ RunLLDBCommands("Running terminateCommands:",
+ configuration.terminateCommands);
}
-lldb::SBTarget
-DAP::CreateTargetFromArguments(const llvm::json::Object &arguments,
- lldb::SBError &error) {
+lldb::SBTarget DAP::CreateTargetFromArguments(llvm::StringRef program,
+ llvm::StringRef targetTriple,
+ llvm::StringRef platformName,
+ lldb::SBError &error) {
// Grab the name of the program we need to debug and create a target using
// the given program as an argument. Executable file can be a source of target
// architecture and platform, if they differ from the host. Setting exe path
@@ -638,15 +639,10 @@ DAP::CreateTargetFromArguments(const llvm::json::Object &arguments,
// enough information to determine correct arch and platform (or ELF can be
// omitted at all), so it is good to leave the user an apportunity to specify
// those. Any of those three can be left empty.
- const llvm::StringRef target_triple =
- GetString(arguments, "targetTriple").value_or("");
- const llvm::StringRef platform_name =
- GetString(arguments, "platformName").value_or("");
- const llvm::StringRef program = GetString(arguments, "program").value_or("");
- auto target = this->debugger.CreateTarget(
- program.data(), target_triple.data(), platform_name.data(),
- true, // Add dependent modules.
- error);
+ auto target = this->debugger.CreateTarget(program.data(), targetTriple.data(),
+ platformName.data(),
+ true, // Add dependent modules.
+ error);
if (error.Fail()) {
// Update message if there was an error.
@@ -802,7 +798,7 @@ llvm::Error DAP::Loop() {
return llvm::Error::success();
}
-lldb::SBError DAP::WaitForProcessToStop(uint32_t seconds) {
+lldb::SBError DAP::WaitForProcessToStop(std::chrono::seconds seconds) {
lldb::SBError error;
lldb::SBProcess process = target.GetProcess();
if (!process.IsValid()) {
@@ -837,8 +833,8 @@ lldb::SBError DAP::WaitForProcessToStop(uint32_t seconds) {
}
std::this_thread::sleep_for(std::chrono::microseconds(250));
}
- error.SetErrorStringWithFormat("process failed to stop within %u seconds",
- seconds);
+ error.SetErrorStringWithFormat("process failed to stop within %lld seconds",
+ seconds.count());
return error;
}
@@ -1035,6 +1031,36 @@ bool SendEventRequestHandler::DoExecute(lldb::SBDebugger debugger,
return true;
}
+void DAP::ConfigureSourceMaps() {
+ if (configuration.sourceMap.empty() && !configuration.sourcePath)
+ return;
+
+ std::string sourceMapCommand;
+ llvm::raw_string_ostream strm(sourceMapCommand);
+ strm << "settings set target.source-map ";
+
+ if (!configuration.sourceMap.empty()) {
+ for (const auto &kv : configuration.sourceMap) {
+ strm << "\"" << kv.first << "\" \"" << kv.second << "\" ";
+ }
+ } else if (configuration.sourcePath) {
+ strm << "\".\" \"" << *configuration.sourcePath << "\"";
+ }
+
+ RunLLDBCommands("Setting source map:", {sourceMapCommand});
+}
+
+void DAP::SetConfiguration(const protocol::DAPConfiguration &config,
+ bool is_attach) {
+ configuration = config;
+ this->is_attach = is_attach;
+
+ if (configuration.customFrameFormat)
+ SetFrameFormat(*configuration.customFrameFormat);
+ if (configuration.customThreadFormat)
+ SetThreadFormat(*configuration.customThreadFormat);
+}
+
void DAP::SetFrameFormat(llvm::StringRef format) {
if (format.empty())
return;
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 6689980806047..f3c4a7bf22798 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -35,6 +35,7 @@
#include "lldb/lldb-types.h"
#include "llvm/ADT/DenseMap.h"
#include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/FunctionExtras.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Error.h"
@@ -128,21 +129,21 @@ struct Variables {
struct StartDebuggingRequestHandler : public lldb::SBCommandPluginInterface {
DAP &dap;
- explicit StartDebuggingRequestHandler(DAP &d) : dap(d) {};
+ explicit StartDebuggingRequestHandler(DAP &d) : dap(d){};
bool DoExecute(lldb::SBDebugger debugger, char **command,
lldb::SBCommandReturnObject &result) override;
};
struct ReplModeRequestHandler : public lldb::SBCommandPluginInterface {
DAP &dap;
- explicit ReplModeRequestHandler(DAP &d) : dap(d) {};
+ explicit ReplModeRequestHandler(DAP &d) : dap(d){};
bool DoExecute(lldb::SBDebugger debugger, char **command,
lldb::SBCommandReturnObject &result) override;
};
struct SendEventRequestHandler : public lldb::SBCommandPluginInterface {
DAP &dap;
- explicit SendEventRequestHandler(DAP &d) : dap(d) {};
+ explicit SendEventRequestHandler(DAP &d) : dap(d){};
bool DoExecute(lldb::SBDebugger debugger, char **command,
lldb::SBCommandReturnObject &result) override;
};
@@ -165,26 +166,21 @@ struct DAP {
InstructionBreakpointMap instruction_breakpoints;
std::optional<std::vector<ExceptionBreakpoint>> exception_breakpoints;
llvm::once_flag init_exception_breakpoints_flag;
- std::vector<std::string> pre_init_commands;
- std::vector<std::string> init_commands;
- std::vector<std::string> pre_run_commands;
- std::vector<std::string> post_run_commands;
- std::vector<std::string> exit_commands;
- std::vector<std::string> stop_commands;
- std::vector<std::string> terminate_commands;
+
// Map step in target id to list of function targets that user can choose.
llvm::DenseMap<lldb::addr_t, std::string> step_in_targets;
- // A copy of the last LaunchRequest or AttachRequest so we can reuse its
- // arguments if we get a RestartRequest.
- std::optional<llvm::json::Object> last_launch_or_attach_request;
+ // A copy of the last LaunchRequest so we can reuse its arguments if we get a
+ // RestartRequest. Restarting an AttachRequest is not supported.
+ std::optional<protocol::LaunchRequestArguments> last_launch_request;
lldb::tid_t focus_tid;
bool disconnecting = false;
llvm::once_flag terminated_event_flag;
- bool stop_at_entry;
bool is_attach;
- bool enable_auto_variable_summaries;
- bool enable_synthetic_child_debugging;
- bool display_extended_backtrace;
+ bool stop_at_entry;
+ /// pre_init_commands are configured by a CLI flag and are not part of the
+ /// common launching/attaching definition.
+ std::vector<std::string> pre_init_commands;
+ protocol::DAPConfiguration configuration;
// The process event thread normally responds to process exited events by
// shutting down the entire adapter. When we're restarting, we keep the id of
// the old process here so we can detect this case and keep running.
@@ -201,7 +197,6 @@ struct DAP {
llvm::SmallDenseMap<int64_t, std::unique_ptr<ResponseHandler>>
inflight_reverse_requests;
ReplMode repl_mode;
- std::string command_escape_prefix = "`";
lldb::SBFormat frame_format;
lldb::SBFormat thread_format;
// This is used to allow request_evaluate to handle empty expressions
@@ -249,6 +244,13 @@ struct DAP {
/// Stop event handler threads.
void StopEventHandlers();
+ /// Configures the debug adapter for launching/attaching.
+ void SetConfiguration(const protocol::DAPConfiguration &confing,
+ bool is_attach);
+
+ /// Configure source maps based on the current `DAPConfiguration`.
+ void ConfigureSourceMaps();
+
/// Serialize the JSON value into a string and send the JSON packet to the
/// "out" stream.
void SendJSON(const llvm::json::Value &json);
@@ -320,7 +322,9 @@ struct DAP {
///
/// \return
/// An SBTarget object.
- lldb::SBTarget CreateTargetFromArguments(const llvm::json::Object &arguments,
+ lldb::SBTarget CreateTargetFromArguments(llvm::StringRef program,
+ llvm::StringRef targetTriple,
+ llvm::StringRef platformName,
lldb::SBError &error);
/// Set given target object as a current target for lldb-dap and start
@@ -395,7 +399,7 @@ struct DAP {
/// The number of seconds to poll the process to wait until it is stopped.
///
/// \return Error if waiting for the process fails, no error if succeeds.
- lldb::SBError WaitForProcessToStop(uint32_t seconds);
+ lldb::SBError WaitForProcessToStop(std::chrono::seconds seconds);
void SetFrameFormat(llvm::StringRef format);
diff --git a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
index 20f7c80a1ed90..cd7fbf4f7be9f 100644
--- a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
@@ -43,10 +43,8 @@ namespace lldb_dap {
// acknowledgement, so no body field is required."
// }]
// }
-
void AttachRequestHandler::operator()(const llvm::json::Object &request) const {
dap.is_attach = true;
- dap.last_launch_or_attach_request = request;
llvm::json::Object response;
lldb::SBError error;
FillResponse(request, response);
@@ -63,11 +61,13 @@ void AttachRequestHandler::operator()(const llvm::json::Object &request) const {
attach_info.SetProcessID(pid);
const auto wait_for = GetBoolean(arguments, "waitFor").value_or(false);
attach_info.SetWaitForLaunch(wait_for, false /*async*/);
- dap.init_commands = GetStrings(arguments, "initCommands");
- dap.pre_run_commands = GetStrings(arguments, "preRunCommands");
- dap.stop_commands = GetStrings(argu...
[truncated]
``````````
</details>
https://github.com/llvm/llvm-project/pull/133624
More information about the lldb-commits
mailing list