llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: John Harrison (ashgti) <details> <summary>Changes</summary> This adds new types and helpers to support the 'initialize' request with the new typed RequestHandler. While working on this I found there were a few cases where we incorrectly treated initialize arguments as capabilities. The new `lldb_dap::protocol::InitializeRequestArguments` and `lldb_dap::protocol::Capabilities` uncovered the inconsistencies. --- Patch is 56.73 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/133007.diff 13 Files Affected: - (modified) lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py (+3-2) - (modified) lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py (+1-2) - (modified) lldb/tools/lldb-dap/DAP.cpp (+126-19) - (modified) lldb/tools/lldb-dap/DAP.h (+4-2) - (modified) lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp (+25-137) - (modified) lldb/tools/lldb-dap/Handler/RequestHandler.cpp (+1) - (modified) lldb/tools/lldb-dap/Handler/RequestHandler.h (+57-32) - (modified) lldb/tools/lldb-dap/JSONUtils.cpp (+6-28) - (modified) lldb/tools/lldb-dap/JSONUtils.h (+2-1) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp (+52) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.h (+69) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp (+199) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolTypes.h (+255) ``````````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 359ac718138b2..01ef4b68f2653 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 @@ -776,7 +776,8 @@ def request_initialize(self, sourceInitFile): "supportsVariablePaging": True, "supportsVariableType": True, "supportsStartDebuggingRequest": True, - "sourceInitFile": sourceInitFile, + "supportsProgressReporting": True, + "$__lldb_sourceInitFile": sourceInitFile, }, } response = self.send_recv(command_dict) @@ -1261,7 +1262,7 @@ def launch(cls, /, executable, env=None, log_file=None, connection=None): expected_prefix = "Listening for: " out = process.stdout.readline().decode() if not out.startswith(expected_prefix): - self.process.kill() + process.kill() raise ValueError( "lldb-dap failed to print listening address, expected '{}', got '{}'".format( expected_prefix, out 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 0c92e5bff07c6..64c99019a1c9b 100644 --- a/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py +++ b/lldb/test/API/tools/lldb-dap/launch/TestDAP_launch.py @@ -524,8 +524,7 @@ def test_version(self): # The first line is the prompt line like "(lldb) version", so we skip it. version_eval_output_without_prompt_line = version_eval_output.splitlines()[1:] - lldb_json = self.dap_server.get_initialize_value("__lldb") - version_string = lldb_json["version"] + version_string = self.dap_server.get_initialize_value("$__lldb_version") self.assertEqual( version_eval_output_without_prompt_line, version_string.splitlines(), diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 65de0488729e5..0da8ce43f73c4 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/ProtocolTypes.h" #include "Transport.h" #include "lldb/API/SBBreakpoint.h" #include "lldb/API/SBCommandInterpreter.h" @@ -1144,31 +1145,137 @@ lldb::SBValue Variables::FindVariable(uint64_t variablesReference, return variable; } -llvm::StringMap<bool> DAP::GetCapabilities() { - llvm::StringMap<bool> capabilities; +static void mergeCapabilities(protocol::Capabilities &into, + const protocol::Capabilities &from) { + if (from.supportsConfigurationDoneRequest) + into.supportsConfigurationDoneRequest = + *from.supportsConfigurationDoneRequest; + if (from.supportsFunctionBreakpoints) + into.supportsFunctionBreakpoints = *from.supportsFunctionBreakpoints; + if (from.supportsConditionalBreakpoints) + into.supportsConditionalBreakpoints = *from.supportsConditionalBreakpoints; + if (from.supportsHitConditionalBreakpoints) + into.supportsHitConditionalBreakpoints = + *from.supportsHitConditionalBreakpoints; + if (from.supportsEvaluateForHovers) + into.supportsEvaluateForHovers = *from.supportsEvaluateForHovers; + if (from.exceptionBreakpointFilters) + into.exceptionBreakpointFilters = *from.exceptionBreakpointFilters; + if (from.supportsStepBack) + into.supportsStepBack = *from.supportsStepBack; + if (from.supportsSetVariable) + into.supportsSetVariable = *from.supportsSetVariable; + if (from.supportsRestartFrame) + into.supportsRestartFrame = *from.supportsRestartFrame; + if (from.supportsGotoTargetsRequest) + into.supportsGotoTargetsRequest = *from.supportsGotoTargetsRequest; + if (from.supportsStepInTargetsRequest) + into.supportsStepInTargetsRequest = *from.supportsStepInTargetsRequest; + if (from.supportsCompletionsRequest) + into.supportsCompletionsRequest = *from.supportsCompletionsRequest; + if (from.completionTriggerCharacters) + into.completionTriggerCharacters = *from.completionTriggerCharacters; + if (from.supportsModulesRequest) + into.supportsModulesRequest = *from.supportsModulesRequest; + if (from.additionalModuleColumns) + into.additionalModuleColumns = *from.additionalModuleColumns; + if (from.supportedChecksumAlgorithms) + into.supportedChecksumAlgorithms = *from.supportedChecksumAlgorithms; + if (from.supportsRestartRequest) + into.supportsRestartRequest = *from.supportsRestartRequest; + if (from.supportsExceptionOptions) + into.supportsExceptionOptions = *from.supportsExceptionOptions; + if (from.supportsValueFormattingOptions) + into.supportsValueFormattingOptions = *from.supportsValueFormattingOptions; + if (from.supportsExceptionInfoRequest) + into.supportsExceptionInfoRequest = *from.supportsExceptionInfoRequest; + if (from.supportTerminateDebuggee) + into.supportTerminateDebuggee = *from.supportTerminateDebuggee; + if (from.supportSuspendDebuggee) + into.supportSuspendDebuggee = *from.supportSuspendDebuggee; + if (from.supportsDelayedStackTraceLoading) + into.supportsDelayedStackTraceLoading = + *from.supportsDelayedStackTraceLoading; + if (from.supportsLoadedSourcesRequest) + into.supportsLoadedSourcesRequest = *from.supportsLoadedSourcesRequest; + if (from.supportsLogPoints) + into.supportsLogPoints = *from.supportsLogPoints; + if (from.supportsTerminateThreadsRequest) + into.supportsTerminateThreadsRequest = + *from.supportsTerminateThreadsRequest; + if (from.supportsSetExpression) + into.supportsSetExpression = *from.supportsSetExpression; + if (from.supportsTerminateRequest) + into.supportsTerminateRequest = *from.supportsTerminateRequest; + if (from.supportsDataBreakpoints) + into.supportsDataBreakpoints = *from.supportsDataBreakpoints; + if (from.supportsReadMemoryRequest) + into.supportsReadMemoryRequest = *from.supportsReadMemoryRequest; + if (from.supportsWriteMemoryRequest) + into.supportsWriteMemoryRequest = *from.supportsWriteMemoryRequest; + if (from.supportsDisassembleRequest) + into.supportsDisassembleRequest = *from.supportsDisassembleRequest; + if (from.supportsCancelRequest) + into.supportsCancelRequest = *from.supportsCancelRequest; + if (from.supportsBreakpointLocationsRequest) + into.supportsBreakpointLocationsRequest = + *from.supportsBreakpointLocationsRequest; + if (from.supportsClipboardContext) + into.supportsClipboardContext = *from.supportsClipboardContext; + if (from.supportsSteppingGranularity) + into.supportsSteppingGranularity = *from.supportsSteppingGranularity; + if (from.supportsInstructionBreakpoints) + into.supportsInstructionBreakpoints = *from.supportsInstructionBreakpoints; + if (from.supportsExceptionFilterOptions) + into.supportsExceptionFilterOptions = *from.supportsExceptionFilterOptions; + if (from.supportsSingleThreadExecutionRequests) + into.supportsSingleThreadExecutionRequests = + *from.supportsSingleThreadExecutionRequests; + if (from.supportsDataBreakpointBytes) + into.supportsDataBreakpointBytes = *from.supportsDataBreakpointBytes; + if (from.breakpointModes) + into.breakpointModes = *from.breakpointModes; + if (from.supportsANSIStyling) + into.supportsANSIStyling = *from.supportsANSIStyling; +} + +protocol::Capabilities DAP::GetCapabilities() { + protocol::Capabilities capabilities; // Supported capabilities. - capabilities["supportTerminateDebuggee"] = true; - capabilities["supportsDataBreakpoints"] = true; - capabilities["supportsDelayedStackTraceLoading"] = true; - capabilities["supportsEvaluateForHovers"] = true; - capabilities["supportsExceptionOptions"] = true; - capabilities["supportsLogPoints"] = true; - capabilities["supportsProgressReporting"] = true; - capabilities["supportsSteppingGranularity"] = true; - capabilities["supportsValueFormattingOptions"] = true; + capabilities.supportTerminateDebuggee = true; + capabilities.supportsDataBreakpoints = true; + capabilities.supportsDelayedStackTraceLoading = true; + capabilities.supportsEvaluateForHovers = true; + capabilities.supportsExceptionOptions = true; + capabilities.supportsLogPoints = true; + capabilities.supportsSteppingGranularity = true; + capabilities.supportsValueFormattingOptions = true; // Unsupported capabilities. - capabilities["supportsGotoTargetsRequest"] = false; - capabilities["supportsLoadedSourcesRequest"] = false; - capabilities["supportsRestartFrame"] = false; - capabilities["supportsStepBack"] = false; + capabilities.supportsGotoTargetsRequest = false; + capabilities.supportsLoadedSourcesRequest = false; + capabilities.supportsRestartFrame = false; + capabilities.supportsStepBack = false; // Capabilities associated with specific requests. - for (auto &kv : request_handlers) { - for (auto &request_kv : kv.second->GetCapabilities()) - capabilities[request_kv.getKey()] = request_kv.getValue(); - } + for (auto &kv : request_handlers) + mergeCapabilities(capabilities, kv.second->GetCapabilities()); + + // Available filters or options for the setExceptionBreakpoints request. + std::vector<protocol::ExceptionBreakpointsFilter> filters; + for (const auto &exc_bp : *exception_breakpoints) + filters.emplace_back(CreateExceptionBreakpointFilter(exc_bp)); + capabilities.exceptionBreakpointFilters = std::move(filters); + + std::vector<std::string> completion_characters; + completion_characters.emplace_back("."); + completion_characters.emplace_back(" "); + completion_characters.emplace_back("\t"); + capabilities.completionTriggerCharacters = std::move(completion_characters); + + // Put in non-DAP specification lldb specific information. + capabilities.lldbVersion = debugger.GetVersionString(); return capabilities; } diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 4b4d471161137..2f23e5a116bc3 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -16,6 +16,7 @@ #include "OutputRedirector.h" #include "ProgressEvent.h" #include "Protocol/ProtocolBase.h" +#include "Protocol/ProtocolTypes.h" #include "SourceBreakpoint.h" #include "Transport.h" #include "lldb/API/SBBroadcaster.h" @@ -180,6 +181,7 @@ struct DAP { bool enable_auto_variable_summaries; bool enable_synthetic_child_debugging; bool display_extended_backtrace; + bool supports_run_in_terminal_request; // 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. @@ -363,8 +365,8 @@ struct DAP { request_handlers[Handler::GetCommand()] = std::make_unique<Handler>(*this); } - /// Return a key-value list of capabilities. - llvm::StringMap<bool> GetCapabilities(); + /// The set of capablities supported by this adapter. + protocol::Capabilities GetCapabilities(); /// Debuggee will continue from stopped state. void WillContinue() { variables.Clear(); } diff --git a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp index a8fe0d6ffce8b..b85424975a0fc 100644 --- a/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/InitializeRequestHandler.cpp @@ -9,6 +9,7 @@ #include "DAP.h" #include "EventHelper.h" #include "JSONUtils.h" +#include "Protocol/ProtocolRequests.h" #include "RequestHandler.h" #include "lldb/API/SBEvent.h" #include "lldb/API/SBListener.h" @@ -229,118 +230,29 @@ static void EventThreadFunction(DAP &dap) { } } -// "InitializeRequest": { -// "allOf": [ { "$ref": "#/definitions/Request" }, { -// "type": "object", -// "description": "Initialize request; value of command field is -// 'initialize'.", -// "properties": { -// "command": { -// "type": "string", -// "enum": [ "initialize" ] -// }, -// "arguments": { -// "$ref": "#/definitions/InitializeRequestArguments" -// } -// }, -// "required": [ "command", "arguments" ] -// }] -// }, -// "InitializeRequestArguments": { -// "type": "object", -// "description": "Arguments for 'initialize' request.", -// "properties": { -// "clientID": { -// "type": "string", -// "description": "The ID of the (frontend) client using this adapter." -// }, -// "adapterID": { -// "type": "string", -// "description": "The ID of the debug adapter." -// }, -// "locale": { -// "type": "string", -// "description": "The ISO-639 locale of the (frontend) client using -// this adapter, e.g. en-US or de-CH." -// }, -// "linesStartAt1": { -// "type": "boolean", -// "description": "If true all line numbers are 1-based (default)." -// }, -// "columnsStartAt1": { -// "type": "boolean", -// "description": "If true all column numbers are 1-based (default)." -// }, -// "pathFormat": { -// "type": "string", -// "_enum": [ "path", "uri" ], -// "description": "Determines in what format paths are specified. The -// default is 'path', which is the native format." -// }, -// "supportsVariableType": { -// "type": "boolean", -// "description": "Client supports the optional type attribute for -// variables." -// }, -// "supportsVariablePaging": { -// "type": "boolean", -// "description": "Client supports the paging of variables." -// }, -// "supportsRunInTerminalRequest": { -// "type": "boolean", -// "description": "Client supports the runInTerminal request." -// } -// }, -// "required": [ "adapterID" ] -// }, -// "InitializeResponse": { -// "allOf": [ { "$ref": "#/definitions/Response" }, { -// "type": "object", -// "description": "Response to 'initialize' request.", -// "properties": { -// "body": { -// "$ref": "#/definitions/Capabilities", -// "description": "The capabilities of this debug adapter." -// } -// } -// }] -// } -void InitializeRequestHandler::operator()( - const llvm::json::Object &request) const { - llvm::json::Object response; - FillResponse(request, response); - llvm::json::Object body; - - const auto *arguments = request.getObject("arguments"); - // sourceInitFile option is not from formal DAP specification. It is only - // used by unit tests to prevent sourcing .lldbinit files from environment - // which may affect the outcome of tests. - bool source_init_file = - GetBoolean(arguments, "sourceInitFile").value_or(true); - +/// Initialize request; value of command field is 'initialize'. +llvm::Expected<protocol::InitializeResponseBody> InitializeRequestHandler::Run( + const protocol::InitializeRequestArguments &arguments) const { // Do not source init files until in/out/err are configured. dap.debugger = lldb::SBDebugger::Create(false); dap.debugger.SetInputFile(dap.in); - auto out_fd = dap.out.GetWriteFileDescriptor(); - if (llvm::Error err = out_fd.takeError()) { - response["success"] = false; - EmplaceSafeString(response, "message", llvm::toString(std::move(err))); - dap.SendJSON(llvm::json::Value(std::move(response))); - return; - } + + llvm::Expected<int> out_fd = dap.out.GetWriteFileDescriptor(); + if (!out_fd) + return out_fd.takeError(); dap.debugger.SetOutputFile(lldb::SBFile(*out_fd, "w", false)); - auto err_fd = dap.err.GetWriteFileDescriptor(); - if (llvm::Error err = err_fd.takeError()) { - response["success"] = false; - EmplaceSafeString(response, "message", llvm::toString(std::move(err))); - dap.SendJSON(llvm::json::Value(std::move(response))); - return; - } + + llvm::Expected<int> err_fd = dap.err.GetWriteFileDescriptor(); + if (!err_fd) + return err_fd.takeError(); dap.debugger.SetErrorFile(lldb::SBFile(*err_fd, "w", false)); auto interp = dap.debugger.GetCommandInterpreter(); - if (source_init_file) { + // sourceInitFile option is not from formal DAP specification. It is only + // used by unit tests to prevent sourcing .lldbinit files from environment + // which may affect the outcome of tests. + if (arguments.sourceInitFile.value_or(true)) { dap.debugger.SkipLLDBInitFiles(false); dap.debugger.SkipAppInitFiles(false); lldb::SBCommandReturnObject init; @@ -348,59 +260,35 @@ void InitializeRequestHandler::operator()( interp.SourceInitFileInHomeDirectory(init); } - if (llvm::Error err = dap.RunPreInitCommands()) { - 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.RunPreInitCommands()) + return err; dap.PopulateExceptionBreakpoints(); auto cmd = dap.debugger.GetCommandInterpreter().AddMultiwordCommand( "lldb-dap", "Commands for managing lldb-dap."); - if (GetBoolean(arguments, "supportsStartDebuggingRequest").value_or(false)) { + if (arguments.supportsStartDebuggingRequest.value_or(false)) { cmd.AddCommand( "start-debugging", new StartDebuggingRequestHandler(dap), "Sends a startDebugging request from the debug adapter to the client " "to start a child debug session of the same type as the caller."); } + dap.supports_run_in_terminal_request = + arguments.supportsRunInTerminalRequest.value_or(false); cmd.AddCommand( "repl-mode", new ReplModeRequestHandler(dap), "Get or set the repl behavior of lldb-dap evaluation requests."); cmd.AddCommand("send-event", new SendEventRequestHandler(dap), "Sends an DAP event to the client."); - dap.progress_event_thread = - std::thread(ProgressEventThreadFunction, std::ref(dap)); + if (arguments.supportsProgressReporting) + dap.progress_event_thread = + std::thread(ProgressEventThreadFunction, std::ref(dap)); // Start our event thread so we can receive events from the debugger, target, // process and more. dap.event_thread = std::thread(EventThreadFunction, std::ref(dap)); - llvm::StringMap<bool> capabilities = dap.GetCapabilities(); - for (auto &kv : capabilities) - body.try_emplace(kv.getKey(), kv.getValue()); - - // Available filters or options for the setExceptionBreakpoints request. - llvm::json::Array filters; - for (const auto &exc_bp : *dap.exception_breakpoints) - filters.emplace_back(CreateExceptionBreakpointFilter(exc_bp)); - body.try_emplace("exceptionBreakpointFilters", std::move(filters)); - - llvm::json::Array completion_characters; - completion_characters.emplace_back("."); - completion_characters.emplace_back(" "); - completion_characters.emplace_back("\t"); - body.try_emplace("completionTriggerCharacters", - std::move(completion_characters)); - - // Put in non-DAP specification lldb specific information. - llvm::json::Object lldb_json; - lldb_json.try_emplace("version", dap.debugger.GetVersionString()); - body.try_emplace("__lldb", std::move(lldb_json)); - - response.try_emplace("body", std::move(body)); - dap.SendJSON(llvm::json::Value(std::move(response))); + return dap.GetCapabilities(); } } // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp index 60c82649938d6..d2a0da420739e 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.cpp @@ -201,6 +201,7 @@ BaseRequestHandler::LaunchProcess(const llvm::json::Object &request) const { const auto timeout_seconds = GetInteger<uint64_t>(arguments, "timeout").value_or(30); + // FIXME: Check dap.supports_run_in_terminal_request. if (GetBoolean(arguments, "runInTerminal").value_or(false)) { if (llvm::Error err = RunInTerminal(dap, request, timeout_seconds)) error.SetErrorString(llvm::toString(std::move(err)).c_str()); diff --git a/lldb/tools/lldb-dap/Handler/... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/133007 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
