[lldb-dap] Finish refactoring the request handlers (NFC) #128553
Merged
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
Member
|
@llvm/pr-subscribers-lldb Author: Jonas Devlieghere (JDevlieghere) ChangesCompletes the work started in #128262. This PR removes the old way of register request handlers with callbacks. Builds on top of #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:
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]
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
ashgti
approved these changes
Feb 24, 2025
|
|
||
| virtual ~RequestHandler() = default; | ||
|
|
||
| virtual void operator()(const llvm::json::Object &request) = 0; |
Contributor
There was a problem hiding this comment.
Should we also mark this const? I don't think any of the request handlers are using any state at the moment.
Member
Author
There was a problem hiding this comment.
Yup, I was going to it in a separate commit, but I can include it here.
This removes the old way of register request handlers with callbacks.
JDevlieghere
force-pushed
the
finish-request-handlers
branch
from
76c1fd8 to
611d0c9
Compare
SquallATF
pushed a commit
to SquallATF/llvm-project
that referenced
this pull request
Jun 13, 2025
Completes the work started in llvm#128262. This PR removes the old way of register request handlers with callbacks and makes the operator const.
SquallATF
pushed a commit
to SquallATF/llvm-project
that referenced
this pull request
Jul 14, 2025
Completes the work started in llvm#128262. This PR removes the old way of register request handlers with callbacks and makes the operator const.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Completes the work started in #128262. This PR removes the old way of register request handlers with callbacks. Builds on top of #128551.