llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) <details> <summary>Changes</summary> Completes the work started in https://github.com/llvm/llvm-project/pull/128262. This PR removes the old way of register request handlers with callbacks. Builds on top of https://github.com/llvm/llvm-project/pull/128551. --- Patch is 234.33 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/128553.diff 29 Files Affected: - (modified) lldb/tools/lldb-dap/CMakeLists.txt (+23) - (modified) lldb/tools/lldb-dap/DAP.cpp (+2-15) - (modified) lldb/tools/lldb-dap/DAP.h (+2-17) - (added) lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp (+80) - (added) lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp (+190) - (added) lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp (+222) - (added) lldb/tools/lldb-dap/Handler/LocationsRequestHandler.cpp (+160) - (added) lldb/tools/lldb-dap/Handler/ModulesRequestHandler.cpp (+58) - (added) lldb/tools/lldb-dap/Handler/NextRequestHandler.cpp (+79) - (added) lldb/tools/lldb-dap/Handler/PauseRequestHandler.cpp (+60) - (added) lldb/tools/lldb-dap/Handler/ReadMemoryRequestHandler.cpp (+139) - (modified) lldb/tools/lldb-dap/Handler/RequestHandler.cpp (+62) - (modified) lldb/tools/lldb-dap/Handler/RequestHandler.h (+176-1) - (added) lldb/tools/lldb-dap/Handler/ScopesRequestHandler.cpp (+106) - (added) lldb/tools/lldb-dap/Handler/SetBreakpointsRequestHandler.cpp (+182) - (added) lldb/tools/lldb-dap/Handler/SetDataBreakpointsRequestHandler.cpp (+114) - (added) lldb/tools/lldb-dap/Handler/SetExceptionBreakpointsRequestHandler.cpp (+93) - (added) lldb/tools/lldb-dap/Handler/SetFunctionBreakpointsRequestHandler.cpp (+139) - (added) lldb/tools/lldb-dap/Handler/SetInstructionBreakpointsRequestHandler.cpp (+249) - (added) lldb/tools/lldb-dap/Handler/SetVariableRequestHandler.cpp (+177) - (added) lldb/tools/lldb-dap/Handler/SourceRequestHandler.cpp (+82) - (added) lldb/tools/lldb-dap/Handler/StackTraceRequestHandler.cpp (+197) - (added) lldb/tools/lldb-dap/Handler/StepInRequestHandler.cpp (+96) - (added) lldb/tools/lldb-dap/Handler/StepInTargetsRequestHandler.cpp (+149) - (added) lldb/tools/lldb-dap/Handler/StepOutRequestHandler.cpp (+68) - (added) lldb/tools/lldb-dap/Handler/TestGetTargetBreakpointsRequestHandler.cpp (+31) - (added) lldb/tools/lldb-dap/Handler/ThreadsRequestHandler.cpp (+71) - (added) lldb/tools/lldb-dap/Handler/VariablesRequestHandler.cpp (+217) - (modified) lldb/tools/lldb-dap/lldb-dap.cpp (+25-2678) ``````````diff diff --git a/lldb/tools/lldb-dap/CMakeLists.txt b/lldb/tools/lldb-dap/CMakeLists.txt index 73762af5c2fd7..804dd8e4cc2a0 100644 --- a/lldb/tools/lldb-dap/CMakeLists.txt +++ b/lldb/tools/lldb-dap/CMakeLists.txt @@ -39,16 +39,39 @@ add_lldb_tool(lldb-dap Handler/AttachRequestHandler.cpp Handler/BreakpointLocationsHandler.cpp + Handler/CompileUnitsRequestHandler.cpp Handler/CompletionsHandler.cpp Handler/ConfigurationDoneRequestHandler.cpp Handler/ContinueRequestHandler.cpp + Handler/DataBreakpointInfoRequestHandler.cpp + Handler/DisassembleRequestHandler.cpp Handler/DisconnectRequestHandler.cpp Handler/EvaluateRequestHandler.cpp Handler/ExceptionInfoRequestHandler.cpp Handler/InitializeRequestHandler.cpp Handler/LaunchRequestHandler.cpp + Handler/LocationsRequestHandler.cpp + Handler/ModulesRequestHandler.cpp + Handler/NextRequestHandler.cpp + Handler/PauseRequestHandler.cpp + Handler/ReadMemoryRequestHandler.cpp Handler/RequestHandler.cpp Handler/RestartRequestHandler.cpp + Handler/ScopesRequestHandler.cpp + Handler/SetBreakpointsRequestHandler.cpp + Handler/SetDataBreakpointsRequestHandler.cpp + Handler/SetExceptionBreakpointsRequestHandler.cpp + Handler/SetFunctionBreakpointsRequestHandler.cpp + Handler/SetInstructionBreakpointsRequestHandler.cpp + Handler/SetVariableRequestHandler.cpp + Handler/SourceRequestHandler.cpp + Handler/StackTraceRequestHandler.cpp + Handler/StepInRequestHandler.cpp + Handler/StepInTargetsRequestHandler.cpp + Handler/StepOutRequestHandler.cpp + Handler/TestGetTargetBreakpointsRequestHandler.cpp + Handler/ThreadsRequestHandler.cpp + Handler/VariablesRequestHandler.cpp LINK_LIBS liblldb diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp index 9b22b60a68d94..d5dd0304f7221 100644 --- a/lldb/tools/lldb-dap/DAP.cpp +++ b/lldb/tools/lldb-dap/DAP.cpp @@ -758,20 +758,12 @@ bool DAP::HandleObject(const llvm::json::Object &object) { if (packet_type == "request") { const auto command = GetString(object, "command"); - // Try the new request handler first. - auto new_handler_pos = new_request_handlers.find(command); - if (new_handler_pos != new_request_handlers.end()) { + auto new_handler_pos = request_handlers.find(command); + if (new_handler_pos != request_handlers.end()) { (*new_handler_pos->second)(object); return true; // Success } - // FIXME: Remove request_handlers once everything has been migrated. - auto handler_pos = request_handlers.find(command); - if (handler_pos != request_handlers.end()) { - handler_pos->second(*this, object); - return true; // Success - } - if (log) *log << "error: unhandled command \"" << command.data() << "\"" << std::endl; @@ -900,11 +892,6 @@ void DAP::SendReverseRequest(llvm::StringRef command, }); } -void DAP::RegisterRequestCallback(std::string request, - RequestCallback callback) { - request_handlers[request] = callback; -} - lldb::SBError DAP::WaitForProcessToStop(uint32_t seconds) { lldb::SBError error; lldb::SBProcess process = target.GetProcess(); diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h index 18de39838f218..ddda8603c189f 100644 --- a/lldb/tools/lldb-dap/DAP.h +++ b/lldb/tools/lldb-dap/DAP.h @@ -68,7 +68,6 @@ enum DAPBroadcasterBits { eBroadcastBitStopProgressThread = 1u << 1 }; -typedef void (*RequestCallback)(DAP &dap, const llvm::json::Object &command); typedef void (*ResponseCallback)(llvm::Expected<llvm::json::Value> value); enum class PacketStatus { @@ -186,8 +185,7 @@ struct DAP { // the old process here so we can detect this case and keep running. lldb::pid_t restarting_process_id; bool configuration_done_sent; - std::map<std::string, RequestCallback, std::less<>> request_handlers; - llvm::StringMap<std::unique_ptr<RequestHandler>> new_request_handlers; + llvm::StringMap<std::unique_ptr<RequestHandler>> request_handlers; bool waiting_for_run_in_terminal; ProgressEventReporter progress_event_reporter; // Keep track of the last stop thread index IDs as threads won't go away @@ -305,8 +303,6 @@ struct DAP { /// listeing for its breakpoint events. void SetTarget(const lldb::SBTarget target); - const std::map<std::string, RequestCallback> &GetRequestHandlers(); - PacketStatus GetNextObject(llvm::json::Object &object); bool HandleObject(const llvm::json::Object &object); @@ -334,20 +330,9 @@ struct DAP { void SendReverseRequest(llvm::StringRef command, llvm::json::Value arguments, ResponseCallback callback); - /// Registers a callback handler for a Debug Adapter Protocol request - /// - /// \param[in] request - /// The name of the request following the Debug Adapter Protocol - /// specification. - /// - /// \param[in] callback - /// The callback to execute when the given request is triggered by the - /// IDE. - void RegisterRequestCallback(std::string request, RequestCallback callback); - /// Registers a request handler. template <typename Handler> void RegisterRequest() { - new_request_handlers[Handler::getCommand()] = + request_handlers[Handler::getCommand()] = std::make_unique<Handler>(*this); } diff --git a/lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp new file mode 100644 index 0000000000000..c541d1cd039c8 --- /dev/null +++ b/lldb/tools/lldb-dap/Handler/CompileUnitsRequestHandler.cpp @@ -0,0 +1,80 @@ +//===-- CompileUnitsRequestHandler.cpp ------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "DAP.h" +#include "EventHelper.h" +#include "JSONUtils.h" +#include "RequestHandler.h" + +namespace lldb_dap { + +// "compileUnitsRequest": { +// "allOf": [ { "$ref": "#/definitions/Request" }, { +// "type": "object", +// "description": "Compile Unit request; value of command field is +// 'compileUnits'.", +// "properties": { +// "command": { +// "type": "string", +// "enum": [ "compileUnits" ] +// }, +// "arguments": { +// "$ref": "#/definitions/compileUnitRequestArguments" +// } +// }, +// "required": [ "command", "arguments" ] +// }] +// }, +// "compileUnitsRequestArguments": { +// "type": "object", +// "description": "Arguments for 'compileUnits' request.", +// "properties": { +// "moduleId": { +// "type": "string", +// "description": "The ID of the module." +// } +// }, +// "required": [ "moduleId" ] +// }, +// "compileUnitsResponse": { +// "allOf": [ { "$ref": "#/definitions/Response" }, { +// "type": "object", +// "description": "Response to 'compileUnits' request.", +// "properties": { +// "body": { +// "description": "Response to 'compileUnits' request. Array of +// paths of compile units." +// } +// } +// }] +// } +void CompileUnitsRequestHandler::operator()(const llvm::json::Object &request) { + llvm::json::Object response; + FillResponse(request, response); + llvm::json::Object body; + llvm::json::Array units; + const auto *arguments = request.getObject("arguments"); + std::string module_id = std::string(GetString(arguments, "moduleId")); + int num_modules = dap.target.GetNumModules(); + for (int i = 0; i < num_modules; i++) { + auto curr_module = dap.target.GetModuleAtIndex(i); + if (module_id == curr_module.GetUUIDString()) { + int num_units = curr_module.GetNumCompileUnits(); + for (int j = 0; j < num_units; j++) { + auto curr_unit = curr_module.GetCompileUnitAtIndex(j); + units.emplace_back(CreateCompileUnit(curr_unit)); + } + body.try_emplace("compileUnits", std::move(units)); + break; + } + } + response.try_emplace("body", std::move(body)); + dap.SendJSON(llvm::json::Value(std::move(response))); +} + +} // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp new file mode 100644 index 0000000000000..519a9c728e4b3 --- /dev/null +++ b/lldb/tools/lldb-dap/Handler/DataBreakpointInfoRequestHandler.cpp @@ -0,0 +1,190 @@ +//===-- DataBreakpointInfoRequestHandler.cpp ------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "DAP.h" +#include "EventHelper.h" +#include "JSONUtils.h" +#include "RequestHandler.h" +#include "lldb/API/SBMemoryRegionInfo.h" +#include "llvm/ADT/StringExtras.h" + +namespace lldb_dap { + +// "DataBreakpointInfoRequest": { +// "allOf": [ { "$ref": "#/definitions/Request" }, { +// "type": "object", +// "description": "Obtains information on a possible data breakpoint that +// could be set on an expression or variable.\nClients should only call this +// request if the corresponding capability `supportsDataBreakpoints` is +// true.", "properties": { +// "command": { +// "type": "string", +// "enum": [ "dataBreakpointInfo" ] +// }, +// "arguments": { +// "$ref": "#/definitions/DataBreakpointInfoArguments" +// } +// }, +// "required": [ "command", "arguments" ] +// }] +// }, +// "DataBreakpointInfoArguments": { +// "type": "object", +// "description": "Arguments for `dataBreakpointInfo` request.", +// "properties": { +// "variablesReference": { +// "type": "integer", +// "description": "Reference to the variable container if the data +// breakpoint is requested for a child of the container. The +// `variablesReference` must have been obtained in the current suspended +// state. See 'Lifetime of Object References' in the Overview section for +// details." +// }, +// "name": { +// "type": "string", +// "description": "The name of the variable's child to obtain data +// breakpoint information for.\nIf `variablesReference` isn't specified, +// this can be an expression." +// }, +// "frameId": { +// "type": "integer", +// "description": "When `name` is an expression, evaluate it in the scope +// of this stack frame. If not specified, the expression is evaluated in +// the global scope. When `variablesReference` is specified, this property +// has no effect." +// } +// }, +// "required": [ "name" ] +// }, +// "DataBreakpointInfoResponse": { +// "allOf": [ { "$ref": "#/definitions/Response" }, { +// "type": "object", +// "description": "Response to `dataBreakpointInfo` request.", +// "properties": { +// "body": { +// "type": "object", +// "properties": { +// "dataId": { +// "type": [ "string", "null" ], +// "description": "An identifier for the data on which a data +// breakpoint can be registered with the `setDataBreakpoints` +// request or null if no data breakpoint is available. If a +// `variablesReference` or `frameId` is passed, the `dataId` is +// valid in the current suspended state, otherwise it's valid +// indefinitely. See 'Lifetime of Object References' in the Overview +// section for details. Breakpoints set using the `dataId` in the +// `setDataBreakpoints` request may outlive the lifetime of the +// associated `dataId`." +// }, +// "description": { +// "type": "string", +// "description": "UI string that describes on what data the +// breakpoint is set on or why a data breakpoint is not available." +// }, +// "accessTypes": { +// "type": "array", +// "items": { +// "$ref": "#/definitions/DataBreakpointAccessType" +// }, +// "description": "Attribute lists the available access types for a +// potential data breakpoint. A UI client could surface this +// information." +// }, +// "canPersist": { +// "type": "boolean", +// "description": "Attribute indicates that a potential data +// breakpoint could be persisted across sessions." +// } +// }, +// "required": [ "dataId", "description" ] +// } +// }, +// "required": [ "body" ] +// }] +// } +void DataBreakpointInfoRequestHandler::operator()( + const llvm::json::Object &request) { + llvm::json::Object response; + FillResponse(request, response); + llvm::json::Object body; + lldb::SBError error; + llvm::json::Array accessTypes{"read", "write", "readWrite"}; + const auto *arguments = request.getObject("arguments"); + const auto variablesReference = + GetUnsigned(arguments, "variablesReference", 0); + llvm::StringRef name = GetString(arguments, "name"); + lldb::SBFrame frame = dap.GetLLDBFrame(*arguments); + lldb::SBValue variable = FindVariable(variablesReference, name); + std::string addr, size; + + if (variable.IsValid()) { + lldb::addr_t load_addr = variable.GetLoadAddress(); + size_t byte_size = variable.GetByteSize(); + if (load_addr == LLDB_INVALID_ADDRESS) { + body.try_emplace("dataId", nullptr); + body.try_emplace("description", + "does not exist in memory, its location is " + + std::string(variable.GetLocation())); + } else if (byte_size == 0) { + body.try_emplace("dataId", nullptr); + body.try_emplace("description", "variable size is 0"); + } else { + addr = llvm::utohexstr(load_addr); + size = llvm::utostr(byte_size); + } + } else if (variablesReference == 0 && frame.IsValid()) { + lldb::SBValue value = frame.EvaluateExpression(name.data()); + if (value.GetError().Fail()) { + lldb::SBError error = value.GetError(); + const char *error_cstr = error.GetCString(); + body.try_emplace("dataId", nullptr); + body.try_emplace("description", error_cstr && error_cstr[0] + ? std::string(error_cstr) + : "evaluation failed"); + } else { + uint64_t load_addr = value.GetValueAsUnsigned(); + lldb::SBData data = value.GetPointeeData(); + if (data.IsValid()) { + size = llvm::utostr(data.GetByteSize()); + addr = llvm::utohexstr(load_addr); + lldb::SBMemoryRegionInfo region; + lldb::SBError err = + dap.target.GetProcess().GetMemoryRegionInfo(load_addr, region); + // Only lldb-server supports "qMemoryRegionInfo". So, don't fail this + // request if SBProcess::GetMemoryRegionInfo returns error. + if (err.Success()) { + if (!(region.IsReadable() || region.IsWritable())) { + body.try_emplace("dataId", nullptr); + body.try_emplace("description", + "memory region for address " + addr + + " has no read or write permissions"); + } + } + } else { + body.try_emplace("dataId", nullptr); + body.try_emplace("description", + "unable to get byte size for expression: " + + name.str()); + } + } + } else { + body.try_emplace("dataId", nullptr); + body.try_emplace("description", "variable not found: " + name.str()); + } + + if (!body.getObject("dataId")) { + body.try_emplace("dataId", addr + "/" + size); + body.try_emplace("accessTypes", std::move(accessTypes)); + body.try_emplace("description", + size + " bytes at " + addr + " " + name.str()); + } + response.try_emplace("body", std::move(body)); + dap.SendJSON(llvm::json::Value(std::move(response))); +} + +} // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp new file mode 100644 index 0000000000000..6d25ef0fc5d74 --- /dev/null +++ b/lldb/tools/lldb-dap/Handler/DisassembleRequestHandler.cpp @@ -0,0 +1,222 @@ +//===-- DisassembleRequestHandler.cpp -------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "DAP.h" +#include "EventHelper.h" +#include "JSONUtils.h" +#include "RequestHandler.h" +#include "lldb/API/SBInstruction.h" +#include "llvm/ADT/StringExtras.h" + +namespace lldb_dap { + +// "DisassembleRequest": { +// "allOf": [ { "$ref": "#/definitions/Request" }, { +// "type": "object", +// "description": "Disassembles code stored at the provided +// location.\nClients should only call this request if the corresponding +// capability `supportsDisassembleRequest` is true.", "properties": { +// "command": { +// "type": "string", +// "enum": [ "disassemble" ] +// }, +// "arguments": { +// "$ref": "#/definitions/DisassembleArguments" +// } +// }, +// "required": [ "command", "arguments" ] +// }] +// }, +// "DisassembleArguments": { +// "type": "object", +// "description": "Arguments for `disassemble` request.", +// "properties": { +// "memoryReference": { +// "type": "string", +// "description": "Memory reference to the base location containing the +// instructions to disassemble." +// }, +// "offset": { +// "type": "integer", +// "description": "Offset (in bytes) to be applied to the reference +// location before disassembling. Can be negative." +// }, +// "instructionOffset": { +// "type": "integer", +// "description": "Offset (in instructions) to be applied after the byte +// offset (if any) before disassembling. Can be negative." +// }, +// "instructionCount": { +// "type": "integer", +// "description": "Number of instructions to disassemble starting at the +// specified location and offset.\nAn adapter must return exactly this +// number of instructions - any unavailable instructions should be +// replaced with an implementation-defined 'invalid instruction' value." +// }, +// "resolveSymbols": { +// "type": "boolean", +// "description": "If true, the adapter should attempt to resolve memory +// addresses and other values to symbolic names." +// } +// }, +// "required": [ "memoryReference", "instructionCount" ] +// }, +// "DisassembleResponse": { +// "allOf": [ { "$ref": "#/definitions/Response" }, { +// "type": "object", +// "description": "Response to `disassemble` request.", +// "properties": { +// "body": { +// "type": "object", +// "properties": { +// "instructions": { +// "type": "array", +// "items": { +// "$ref": "#/definitions/DisassembledInstruction" +// }, +// "description": "The list of disassembled instructions." +// } +// }, +// "required": [ "instructions" ] +// } +// } +// }] +// } +void DisassembleRequestHandler::operator()(const llvm::json::Object &request) { + llvm::json::Object response; + FillResponse(request, response); + auto *arguments = request.getObject("arguments"); + + llvm::StringRef memoryReference = GetString(arguments, "memoryReference"); + auto addr_opt = DecodeMemoryReference(memoryReference); + if (!addr_opt.has_value()) { + response["success"] = false; + response["message"] = + "Malformed memory reference: " + memoryR... [truncated] `````````` </details> https://github.com/llvm/llvm-project/pull/128553 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits