llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: John Harrison (ashgti) <details> <summary>Changes</summary> 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. --- 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: - (modified) lldb/tools/lldb-dap/DAP.cpp (+13-13) - (modified) lldb/tools/lldb-dap/Handler/AttachRequestHandler.cpp (+73-141) - (modified) lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp (+1-1) - (modified) lldb/tools/lldb-dap/Handler/LaunchRequestHandler.cpp (+2-4) - (modified) lldb/tools/lldb-dap/Handler/RequestHandler.cpp (+6-58) - (modified) lldb/tools/lldb-dap/Handler/RequestHandler.h (+10-11) - (modified) lldb/tools/lldb-dap/Handler/RestartRequestHandler.cpp (-1) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp (+25-10) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.h (+70-17) - (modified) lldb/tools/lldb-dap/package.json (+4-9) ``````````diff 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] `````````` </details> https://github.com/llvm/llvm-project/pull/137911 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
