[Lldb-commits] [lldb] [lldb-dap] Finish refactoring the request handlers (NFC) (PR #128553)
via lldb-commits
lldb-commits at lists.llvm.org
Mon Feb 24 11:12:26 PST 2025
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
More information about the lldb-commits
mailing list