[lldb-dap] Migrate attach to typed RequestHandler. #137911
Merged
+285
−325
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This updates the `attach` request to the typed `RequestHandler<protocol::AttachRequestArguments, protocol::AttachResponse>`. Added a few more overlapping configurations to `lldb_dap::protocol::Configuration` that are shared between launching and attaching. There may be some additional code we could clean-up that is no longer referenced now that this has migrated to use well defined types.
|
@llvm/pr-subscribers-lldb Author: John Harrison (ashgti) ChangesThis updates the Added a few more overlapping configurations to There may be some additional code we could clean-up that is no longer referenced now that this has migrated to use well defined types. Patch is 36.50 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/137911.diff 10 Files Affected:
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index b593353110787..abed9983118f9 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -675,12 +675,11 @@ lldb::SBTarget DAP::CreateTarget(lldb::SBError &error) {
// enough information to determine correct arch and platform (or ELF can be
// omitted at all), so it is good to leave the user an opportunity to specify
// those. Any of those three can be left empty.
- auto target = this->debugger.CreateTarget(
- configuration.program.value_or("").data(),
- configuration.targetTriple.value_or("").data(),
- configuration.platformName.value_or("").data(),
- true, // Add dependent modules.
- error);
+ auto target = this->debugger.CreateTarget(configuration.program.data(),
+ configuration.targetTriple.data(),
+ configuration.platformName.data(),
+ true, // Add dependent modules.
+ error);
return target;
}
@@ -1192,7 +1191,7 @@ bool SendEventRequestHandler::DoExecute(lldb::SBDebugger debugger,
}
void DAP::ConfigureSourceMaps() {
- if (configuration.sourceMap.empty() && !configuration.sourcePath)
+ if (configuration.sourceMap.empty() && configuration.sourcePath.empty())
return;
std::string sourceMapCommand;
@@ -1203,8 +1202,8 @@ void DAP::ConfigureSourceMaps() {
for (const auto &kv : configuration.sourceMap) {
strm << "\"" << kv.first << "\" \"" << kv.second << "\" ";
}
- } else if (configuration.sourcePath) {
- strm << "\".\" \"" << *configuration.sourcePath << "\"";
+ } else if (!configuration.sourcePath.empty()) {
+ strm << "\".\" \"" << configuration.sourcePath << "\"";
}
RunLLDBCommands("Setting source map:", {sourceMapCommand});
@@ -1213,12 +1212,13 @@ void DAP::ConfigureSourceMaps() {
void DAP::SetConfiguration(const protocol::Configuration &config,
bool is_attach) {
configuration = config;
+ stop_at_entry = config.stopOnEntry;
this->is_attach = is_attach;
- if (configuration.customFrameFormat)
- SetFrameFormat(*configuration.customFrameFormat);
- if (configuration.customThreadFormat)
- SetThreadFormat(*configuration.customThreadFormat);
+ if (!configuration.customFrameFormat.empty())
+ SetFrameFormat(configuration.customFrameFormat);
+ if (!configuration.customThreadFormat.empty())
+ SetThreadFormat(configuration.customThreadFormat);
}
void DAP::SetFrameFormat(llvm::StringRef format) {
diff --git a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
index 3ef87cbef873c..fd19a4f835686 100644
--- a/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp
@@ -9,203 +9,135 @@
#include "DAP.h"
#include "EventHelper.h"
#include "JSONUtils.h"
+#include "LLDBUtils.h"
+#include "Protocol/ProtocolRequests.h"
#include "RequestHandler.h"
#include "lldb/API/SBListener.h"
+#include "lldb/lldb-defines.h"
+#include "llvm/Support/Error.h"
#include "llvm/Support/FileSystem.h"
+using namespace llvm;
+using namespace lldb_dap::protocol;
+
namespace lldb_dap {
-// "AttachRequest": {
-// "allOf": [ { "$ref": "#/definitions/Request" }, {
-// "type": "object",
-// "description": "Attach request; value of command field is 'attach'.",
-// "properties": {
-// "command": {
-// "type": "string",
-// "enum": [ "attach" ]
-// },
-// "arguments": {
-// "$ref": "#/definitions/AttachRequestArguments"
-// }
-// },
-// "required": [ "command", "arguments" ]
-// }]
-// },
-// "AttachRequestArguments": {
-// "type": "object",
-// "description": "Arguments for 'attach' request.\nThe attach request has no
-// standardized attributes."
-// },
-// "AttachResponse": {
-// "allOf": [ { "$ref": "#/definitions/Response" }, {
-// "type": "object",
-// "description": "Response to 'attach' request. This is just an
-// acknowledgement, so no body field is required."
-// }]
-// }
-void AttachRequestHandler::operator()(const llvm::json::Object &request) const {
- dap.is_attach = true;
- llvm::json::Object response;
- lldb::SBError error;
- FillResponse(request, response);
- lldb::SBAttachInfo attach_info;
- const int invalid_port = 0;
- const auto *arguments = request.getObject("arguments");
- const lldb::pid_t pid =
- GetInteger<uint64_t>(arguments, "pid").value_or(LLDB_INVALID_PROCESS_ID);
- const auto gdb_remote_port =
- GetInteger<uint64_t>(arguments, "gdb-remote-port").value_or(invalid_port);
- const auto gdb_remote_hostname =
- GetString(arguments, "gdb-remote-hostname").value_or("localhost");
- if (pid != LLDB_INVALID_PROCESS_ID)
- attach_info.SetProcessID(pid);
- const auto wait_for = GetBoolean(arguments, "waitFor").value_or(false);
- attach_info.SetWaitForLaunch(wait_for, false /*async*/);
- dap.configuration.initCommands = GetStrings(arguments, "initCommands");
- dap.configuration.preRunCommands = GetStrings(arguments, "preRunCommands");
- dap.configuration.postRunCommands = GetStrings(arguments, "postRunCommands");
- dap.configuration.stopCommands = GetStrings(arguments, "stopCommands");
- dap.configuration.exitCommands = GetStrings(arguments, "exitCommands");
- dap.configuration.terminateCommands =
- GetStrings(arguments, "terminateCommands");
- auto attachCommands = GetStrings(arguments, "attachCommands");
- llvm::StringRef core_file = GetString(arguments, "coreFile").value_or("");
- const uint64_t timeout_seconds =
- GetInteger<uint64_t>(arguments, "timeout").value_or(30);
- dap.stop_at_entry = core_file.empty()
- ? GetBoolean(arguments, "stopOnEntry").value_or(false)
- : true;
- const llvm::StringRef debuggerRoot =
- GetString(arguments, "debuggerRoot").value_or("");
- dap.configuration.enableAutoVariableSummaries =
- GetBoolean(arguments, "enableAutoVariableSummaries").value_or(false);
- dap.configuration.enableSyntheticChildDebugging =
- GetBoolean(arguments, "enableSyntheticChildDebugging").value_or(false);
- dap.configuration.displayExtendedBacktrace =
- GetBoolean(arguments, "displayExtendedBacktrace").value_or(false);
- dap.configuration.commandEscapePrefix =
- GetString(arguments, "commandEscapePrefix").value_or("`");
- dap.configuration.program = GetString(arguments, "program");
- dap.configuration.targetTriple = GetString(arguments, "targetTriple");
- dap.configuration.platformName = GetString(arguments, "platformName");
- dap.SetFrameFormat(GetString(arguments, "customFrameFormat").value_or(""));
- dap.SetThreadFormat(GetString(arguments, "customThreadFormat").value_or(""));
+/// The `attach` request is sent from the client to the debug adapter to attach
+/// to a debuggee that is already running.
+///
+/// Since attaching is debugger/runtime specific, the arguments for this request
+/// are not part of this specification.
+Error AttachRequestHandler::Run(const AttachRequestArguments &args) const {
+ dap.SetConfiguration(args.configuration, true);
+ if (!args.coreFile.empty())
+ dap.stop_at_entry = true;
+
+ // If both pid and port numbers are specified.
+ 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");
PrintWelcomeMessage();
// This is a hack for loading DWARF in .o files on Mac where the .o files
- // in the debug map of the main executable have relative paths which require
- // the lldb-dap binary to have its working directory set to that relative
- // root for the .o files in order to be able to load debug info.
- if (!debuggerRoot.empty())
- llvm::sys::fs::set_current_path(debuggerRoot);
+ // in the debug map of the main executable have relative paths which
+ // require the lldb-dap binary to have its working directory set to that
+ // relative root for the .o files in order to be able to load debug info.
+ if (!dap.configuration.debuggerRoot.empty())
+ sys::fs::set_current_path(dap.configuration.debuggerRoot);
// Run any initialize LLDB commands the user specified in the launch.json
- if (llvm::Error err = dap.RunInitCommands()) {
- response["success"] = false;
- EmplaceSafeString(response, "message", llvm::toString(std::move(err)));
- dap.SendJSON(llvm::json::Value(std::move(response)));
- return;
- }
+ if (llvm::Error err = dap.RunInitCommands())
+ return err;
- SetSourceMapFromArguments(*arguments);
+ dap.ConfigureSourceMaps();
- lldb::SBError status;
- dap.SetTarget(dap.CreateTarget(status));
- if (status.Fail()) {
- response["success"] = llvm::json::Value(false);
- EmplaceSafeString(response, "message", status.GetCString());
- dap.SendJSON(llvm::json::Value(std::move(response)));
- return;
- }
+ lldb::SBError error;
+ lldb::SBTarget target = dap.CreateTarget(error);
+ if (error.Fail())
+ return ToError(error);
+
+ dap.SetTarget(target);
// Run any pre run LLDB commands the user specified in the launch.json
- if (llvm::Error err = dap.RunPreRunCommands()) {
- response["success"] = false;
- EmplaceSafeString(response, "message", llvm::toString(std::move(err)));
- dap.SendJSON(llvm::json::Value(std::move(response)));
- return;
- }
+ if (Error err = dap.RunPreRunCommands())
+ return err;
- if ((pid == LLDB_INVALID_PROCESS_ID || gdb_remote_port == invalid_port) &&
- wait_for) {
+ if ((args.pid == LLDB_INVALID_PROCESS_ID ||
+ args.gdbRemotePort == LLDB_DAP_INVALID_PORT) &&
+ args.waitFor) {
char attach_msg[256];
auto attach_msg_len = snprintf(attach_msg, sizeof(attach_msg),
"Waiting to attach to \"%s\"...",
dap.target.GetExecutable().GetFilename());
- dap.SendOutput(OutputType::Console,
- llvm::StringRef(attach_msg, attach_msg_len));
+ dap.SendOutput(OutputType::Console, StringRef(attach_msg, attach_msg_len));
}
- if (attachCommands.empty()) {
+
+ if (args.attachCommands.empty()) {
// No "attachCommands", just attach normally.
// Disable async events so the attach will be successful when we return from
// the launch call and the launch will happen synchronously
dap.debugger.SetAsync(false);
- if (core_file.empty()) {
- if ((pid != LLDB_INVALID_PROCESS_ID) &&
- (gdb_remote_port != invalid_port)) {
- // If both pid and port numbers are specified.
- error.SetErrorString("The user can't specify both pid and port");
- } else if (gdb_remote_port != invalid_port) {
+ if (args.coreFile.empty()) {
+ if (args.gdbRemotePort != LLDB_DAP_INVALID_PORT) {
// If port is specified and pid is not.
lldb::SBListener listener = dap.debugger.GetListener();
// If the user hasn't provided the hostname property, default localhost
// being used.
std::string connect_url =
- llvm::formatv("connect://{0}:", gdb_remote_hostname);
- connect_url += std::to_string(gdb_remote_port);
- dap.target.ConnectRemote(listener, connect_url.c_str(), "gdb-remote",
+ llvm::formatv("connect://{0}:", args.gdbRemoteHostname);
+ connect_url += std::to_string(args.gdbRemotePort);
+ dap.target.ConnectRemote(listener, connect_url.data(), "gdb-remote",
error);
} else {
// Attach by process name or id.
+ lldb::SBAttachInfo attach_info;
+ if (!args.configuration.program.empty())
+ attach_info.SetExecutable(args.configuration.program.data());
+ if (args.pid != LLDB_INVALID_PROCESS_ID)
+ attach_info.SetProcessID(args.pid);
+ attach_info.SetWaitForLaunch(args.waitFor, false /*async*/);
dap.target.Attach(attach_info, error);
}
} else
- dap.target.LoadCore(core_file.data(), error);
+ dap.target.LoadCore(args.coreFile.data(), error);
// Reenable async events
dap.debugger.SetAsync(true);
} else {
// We have "attachCommands" that are a set of commands that are expected
// to execute the commands after which a process should be created. If there
// is no valid process after running these commands, we have failed.
- if (llvm::Error err = dap.RunAttachCommands(attachCommands)) {
- response["success"] = false;
- EmplaceSafeString(response, "message", llvm::toString(std::move(err)));
- dap.SendJSON(llvm::json::Value(std::move(response)));
- return;
- }
+ if (llvm::Error err = dap.RunAttachCommands(args.attachCommands))
+ return err;
+
// The custom commands might have created a new target so we should use the
// selected target after these commands are run.
dap.target = dap.debugger.GetSelectedTarget();
// Make sure the process is attached and stopped before proceeding as the
// the launch commands are not run using the synchronous mode.
- error = dap.WaitForProcessToStop(std::chrono::seconds(timeout_seconds));
+ error = dap.WaitForProcessToStop(dap.configuration.timeout);
}
- if (error.Success() && core_file.empty()) {
- auto attached_pid = dap.target.GetProcess().GetProcessID();
- if (attached_pid == LLDB_INVALID_PROCESS_ID) {
- if (attachCommands.empty())
- error.SetErrorString("failed to attach to a process");
- else
- error.SetErrorString("attachCommands failed to attach to a process");
- }
- }
+ if (error.Fail())
+ return ToError(error);
- if (error.Fail()) {
- response["success"] = llvm::json::Value(false);
- EmplaceSafeString(response, "message", std::string(error.GetCString()));
- } else {
- dap.RunPostRunCommands();
- }
+ if (args.coreFile.empty() && !dap.target.GetProcess().IsValid())
+ return make_error<DAPError>("failed to attach to process");
- dap.SendJSON(llvm::json::Value(std::move(response)));
- if (error.Success()) {
+ dap.RunPostRunCommands();
+
+ return Error::success();
+}
+
+void AttachRequestHandler::PostRun() const {
+ if (dap.target.GetProcess().IsValid()) {
SendProcessEvent(dap, Attach);
- dap.SendJSON(CreateEventObject("initialized"));
}
+
+ dap.SendJSON(CreateEventObject("initialized"));
}
} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
index ce34c52bcc334..7a2adef4b2faf 100644
--- a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp
@@ -277,7 +277,7 @@ static void EventThreadFunction(DAP &dap) {
}
/// Initialize request; value of command field is 'initialize'.
-llvm::Expected<InitializeResponseBody> InitializeRequestHandler::Run(
+llvm::Expected<InitializeResponse> InitializeRequestHandler::Run(
const InitializeRequestArguments &arguments) const {
dap.clientFeatures = arguments.supportedFeatures;
diff --git a/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp
index 3e4532e754ec6..fd0846bd00f45 100644
--- a/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp
@@ -24,7 +24,6 @@ namespace lldb_dap {
Error LaunchRequestHandler::Run(const LaunchRequestArguments &arguments) const {
dap.SetConfiguration(arguments.configuration, /*is_attach=*/false);
dap.last_launch_request = arguments;
- dap.stop_at_entry = arguments.stopOnEntry;
if (!arguments.launchCommands.empty() && arguments.runInTerminal)
return make_error<DAPError>(
@@ -36,9 +35,8 @@ Error LaunchRequestHandler::Run(const LaunchRequestArguments &arguments) const {
// in the debug map of the main executable have relative paths which
// require the lldb-dap binary to have its working directory set to that
// relative root for the .o files in order to be able to load debug info.
- const std::string debugger_root = dap.configuration.debuggerRoot.value_or("");
- if (!debugger_root.empty())
- sys::fs::set_current_path(debugger_root);
+ if (!dap.configuration.debuggerRoot.empty())
+ sys::fs::set_current_path(dap.configuration.debuggerRoot);
// Run any initialize LLDB commands the user specified in the launch.json.
// This is run before target is created, so commands can't do anything with
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
index b7d3c8ced69f1..dbfb1da635cbb 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp
@@ -49,56 +49,6 @@ static uint32_t SetLaunchFlag(uint32_t flags, bool flag,
return flags;
}
-// Both attach and launch take either a sourcePath or a sourceMap
-// argument (or neither), from which we need to set the target.source-map.
-void BaseRequestHandler::SetSourceMapFromArguments(
- const llvm::json::Object &arguments) const {
- const char *sourceMapHelp =
- "source must be be an array of two-element arrays, "
- "each containing a source and replacement path string.\n";
-
- std::string sourceMapCommand;
- llvm::raw_string_ostream strm(sourceMapCommand);
- strm << "settings set target.source-map ";
- const auto sourcePath = GetString(arguments, "sourcePath").value_or("");
-
- // sourceMap is the new, more general form of sourcePath and overrides it.
- constexpr llvm::StringRef sourceMapKey = "sourceMap";
-
- if (const auto *sourceMapArray = arguments.getArray(sourceMapKey)) {
- for (const auto &value : *sourceMapArray) {
- const auto *mapping = value.getAsArray();
- if (mapping == nullptr || mapping->size() != 2 ||
- (*mapping)[0].kind() != llvm::json::Value::String ||
- (*mapping)[1].kind() != llvm::json::Value::String) {
- dap.SendOutput(OutputType::Console, llvm::StringRef(sourceMapHelp));
- return;
- }
- const auto mapFrom = GetAsString((*mapping)[0]);
- const auto mapTo = GetAsString((*mapping)[1]);
- strm << "\"" << mapFrom << "\" \"" << mapTo << "\" ";
- }
- } else if (const auto *sourceMapObj = arguments.getObject(sourceMapKey)) {
- for (const auto &[key, value] : *sourceMapObj) {
- if (value.kind() == llvm::json::Value::String) {
- strm << "\"" << key.str() << "\" \"" << GetAsString(value) << "\" ";
- }
- }
- } else {
- if (ObjectContainsKey(arguments, sourceMapKey)) {
- dap.SendOutput(OutputType::Console, llvm::StringRef(sourceMapHelp));
- return;
- }
- if (sourcePath.empty())
- return;
- // Do any source remapping needed before we create our targets
- strm << "\".\" \"" << sourcePath << "\"";
- }
- if (!sourceMapCommand.empty()) {
- dap.RunLLDBCommands("Setting source map:", {sourceMapCommand});
- }
-}
-
static llvm::Error
RunInTerminal(DAP &dap, const protocol::LaunchRequestArguments &arguments) {
if (!dap.clientFeatures.contains(
@@ -106,8 +56,7 @@ RunInTerminal(DAP &dap, const protocol::LaunchRequestArguments &arguments) {
return llvm::make_error<DAPError>("Cannot use runInTerminal, feature is "
"not supported by the connected client");
- if (!arguments.configuration.program ||
- arguments.configuration.program->empty())
+ if (arguments.configuration.program.empty())
return llvm::make_error<DAPError>(
"program must be set to when using runInTerminal");
@@ -128,8 +77,8 @@ RunInTerminal(DAP &dap, const protocol::LaunchRequestArguments &arguments) {
#endif
llvm::json::Object reverse_request = CreateRunInTerminalReverseRequest(
- *arguments.configuration.program, arguments.args, arguments.env,
- arguments.cwd.value_or(""), comm_file.m_path, debugger_pid);
+ arguments.configuration.program, arguments.args, arguments.env,
+ arguments.cwd, comm_file.m_path, debugger_pid);
dap.SendReverseRequest<LogFailureResponseHandler>("runInTerminal",
std::move(reverse_request));
@@ -209,9 +158,8 @@ llvm::Error BaseRequestHandler::...
[truncated]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
da-viper
reviewed
May 1, 2025
| dap.target.ConnectRemote(listener, connect_url.c_str(), "gdb-remote", | ||
| llvm::formatv("connect://{0}:", args.gdbRemoteHostname); | ||
| connect_url += std::to_string(args.gdbRemotePort); | ||
| dap.target.ConnectRemote(listener, connect_url.data(), "gdb-remote", |
|
It is also ignored in the llvm-project/lldb/tools/lldb-dap/DAP.cpp Lines 1224 to 1227 in c0917ab |
Updated to remove the |
da-viper
approved these changes
May 6, 2025
…nitial_thread_list and simplifying the number of if branches in the attach request handler.
|
@JDevlieghere I sync this with your change to the attach flow. After syncing I tried to simplify the AttachRequestHandler a bit since there were a lot of branches that were a bit hard to follow. LMKWYT |
…ests to simplify the flow later on.
…tachCommands. Applying reviewer suggestions.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This updates the
attachrequest to the typedRequestHandler<protocol::AttachRequestArguments, protocol::AttachResponse>.Added a few more overlapping configurations to
lldb_dap::protocol::Configurationthat are shared between launching and attaching.There may be some additional code we could clean-up that is no longer referenced now that this has migrated to use well defined types.