[Lldb-commits] [lldb] [lldb-dap] Migrate 'stepIn' request to well structured types. (PR #137071)
John Harrison via lldb-commits
lldb-commits at lists.llvm.org
Wed Apr 23 15:10:25 PDT 2025
https://github.com/ashgti updated https://github.com/llvm/llvm-project/pull/137071
>From f4f2fea5cebbce550de0e0c3facaac894b9f40b8 Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Wed, 23 Apr 2025 15:01:53 -0700
Subject: [PATCH 1/2] [lldb-dap] Migrate 'stepIn' request to well structured
types.
Migrates the 'stepIn' request handler to have well structured types instead of raw json values.
I also noticed in the 'next' request handler we were not passing the 'RunMode' flag. Updated the 'next' request handler as well.
---
.../lldb-dap/Handler/NextRequestHandler.cpp | 3 +-
lldb/tools/lldb-dap/Handler/RequestHandler.h | 7 +-
.../lldb-dap/Handler/StepInRequestHandler.cpp | 107 ++++++------------
.../lldb-dap/Protocol/ProtocolRequests.cpp | 9 ++
.../lldb-dap/Protocol/ProtocolRequests.h | 22 ++++
5 files changed, 69 insertions(+), 79 deletions(-)
diff --git a/lldb/tools/lldb-dap/Handler/NextRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/NextRequestHandler.cpp
index 1603563841005..3fa167686d2f9 100644
--- a/lldb/tools/lldb-dap/Handler/NextRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/NextRequestHandler.cpp
@@ -13,6 +13,7 @@
#include "llvm/Support/Error.h"
using namespace llvm;
+using namespace lldb;
using namespace lldb_dap::protocol;
namespace lldb_dap {
@@ -35,7 +36,7 @@ Error NextRequestHandler::Run(const NextArguments &args) const {
if (args.granularity == eSteppingGranularityInstruction) {
thread.StepInstruction(/*step_over=*/true);
} else {
- thread.StepOver();
+ thread.StepOver(args.singleThread ? eOnlyThisThread : eOnlyDuringStepping);
}
return Error::success();
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h
index edb9de7d0dc20..e13f7a3749e00 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.h
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h
@@ -298,11 +298,12 @@ class NextRequestHandler
llvm::Error Run(const protocol::NextArguments &args) const override;
};
-class StepInRequestHandler : public LegacyRequestHandler {
+class StepInRequestHandler : public RequestHandler<protocol::StepInArguments,
+ protocol::StepInResponse> {
public:
- using LegacyRequestHandler::LegacyRequestHandler;
+ using RequestHandler::RequestHandler;
static llvm::StringLiteral GetCommand() { return "stepIn"; }
- void operator()(const llvm::json::Object &request) const override;
+ llvm::Error Run(const protocol::StepInArguments &args) const override;
};
class StepInTargetsRequestHandler : public LegacyRequestHandler {
diff --git a/lldb/tools/lldb-dap/Handler/StepInRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/StepInRequestHandler.cpp
index 9d8d75b359447..00b68abb48677 100644
--- a/lldb/tools/lldb-dap/Handler/StepInRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/StepInRequestHandler.cpp
@@ -8,91 +8,48 @@
#include "DAP.h"
#include "EventHelper.h"
-#include "JSONUtils.h"
+#include "Protocol/ProtocolRequests.h"
+#include "Protocol/ProtocolTypes.h"
#include "RequestHandler.h"
-namespace lldb_dap {
+using namespace llvm;
+using namespace lldb;
+using namespace lldb_dap::protocol;
-// "StepInRequest": {
-// "allOf": [ { "$ref": "#/definitions/Request" }, {
-// "type": "object",
-// "description": "StepIn request; value of command field is 'stepIn'. The
-// request starts the debuggee to step into a function/method if possible.
-// If it cannot step into a target, 'stepIn' behaves like 'next'. The debug
-// adapter first sends the StepInResponse and then a StoppedEvent (event
-// type 'step') after the step has completed. If there are multiple
-// function/method calls (or other targets) on the source line, the optional
-// argument 'targetId' can be used to control into which target the 'stepIn'
-// should occur. The list of possible targets for a given source line can be
-// retrieved via the 'stepInTargets' request.", "properties": {
-// "command": {
-// "type": "string",
-// "enum": [ "stepIn" ]
-// },
-// "arguments": {
-// "$ref": "#/definitions/StepInArguments"
-// }
-// },
-// "required": [ "command", "arguments" ]
-// }]
-// },
-// "StepInArguments": {
-// "type": "object",
-// "description": "Arguments for 'stepIn' request.",
-// "properties": {
-// "threadId": {
-// "type": "integer",
-// "description": "Execute 'stepIn' for this thread."
-// },
-// "targetId": {
-// "type": "integer",
-// "description": "Optional id of the target to step into."
-// },
-// "granularity": {
-// "$ref": "#/definitions/SteppingGranularity",
-// "description": "Stepping granularity. If no granularity is specified, a
-// granularity of `statement` is assumed."
-// }
-// },
-// "required": [ "threadId" ]
-// },
-// "StepInResponse": {
-// "allOf": [ { "$ref": "#/definitions/Response" }, {
-// "type": "object",
-// "description": "Response to 'stepIn' request. This is just an
-// acknowledgement, so no body field is required."
-// }]
-// }
-void StepInRequestHandler::operator()(const llvm::json::Object &request) const {
- llvm::json::Object response;
- FillResponse(request, response);
- const auto *arguments = request.getObject("arguments");
+namespace lldb_dap {
+// The request resumes the given thread to step into a function/method and
+// allows all other threads to run freely by resuming them. If the debug adapter
+// supports single thread execution (see capability
+// `supportsSingleThreadExecutionRequests`), setting the `singleThread` argument
+// to true prevents other suspended threads from resuming. If the request cannot
+// step into a target, `stepIn` behaves like the `next` request. The debug
+// adapter first sends the response and then a `stopped` event (with reason
+// `step`) after the step has completed. If there are multiple function/method
+// calls (or other targets) on the source line, the argument `targetId` can be
+// used to control into which target the `stepIn` should occur. The list of
+// possible targets for a given source line can be retrieved via the
+// `stepInTargets` request.
+Error StepInRequestHandler::Run(const StepInArguments &args) const {
std::string step_in_target;
- const auto target_id =
- GetInteger<uint64_t>(arguments, "targetId").value_or(0);
- auto it = dap.step_in_targets.find(target_id);
+ auto it = dap.step_in_targets.find(args.targetId.value_or(0));
if (it != dap.step_in_targets.end())
step_in_target = it->second;
- const bool single_thread =
- GetBoolean(arguments, "singleThread").value_or(false);
- lldb::RunMode run_mode =
- single_thread ? lldb::eOnlyThisThread : lldb::eOnlyDuringStepping;
- lldb::SBThread thread = dap.GetLLDBThread(*arguments);
- if (thread.IsValid()) {
- // Remember the thread ID that caused the resume so we can set the
- // "threadCausedFocus" boolean value in the "stopped" events.
- dap.focus_tid = thread.GetThreadID();
- if (HasInstructionGranularity(*arguments)) {
- thread.StepInstruction(/*step_over=*/false);
- } else {
- thread.StepInto(step_in_target.c_str(), run_mode);
- }
+ RunMode run_mode = args.singleThread ? eOnlyThisThread : eOnlyDuringStepping;
+ SBThread thread = dap.GetLLDBThread(args.threadId);
+ if (!thread.IsValid())
+ return make_error<DAPError>("invalid thread");
+
+ // Remember the thread ID that caused the resume so we can set the
+ // "threadCausedFocus" boolean value in the "stopped" events.
+ dap.focus_tid = thread.GetThreadID();
+ if (args.granularity == eSteppingGranularityInstruction) {
+ thread.StepInstruction(/*step_over=*/false);
} else {
- response["success"] = llvm::json::Value(false);
+ thread.StepInto(step_in_target.c_str(), run_mode);
}
- dap.SendJSON(llvm::json::Value(std::move(response)));
+ return Error::success();
}
} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
index b113299affb0f..ee7c653ee9f1b 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
@@ -121,4 +121,13 @@ bool fromJSON(const llvm::json::Value &Params, NextArguments &NA,
OM.mapOptional("granularity", NA.granularity);
}
+bool fromJSON(const llvm::json::Value &Params, StepInArguments &SIA,
+ llvm::json::Path P) {
+ json::ObjectMapper OM(Params, P);
+ return OM && OM.map("threadId", SIA.threadId) &&
+ OM.map("targetId", SIA.targetId) &&
+ OM.mapOptional("singleThread", SIA.singleThread) &&
+ OM.mapOptional("granularity", SIA.granularity);
+}
+
} // namespace lldb_dap::protocol
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
index 6e3e2c6a9e2c8..50c16c15cef32 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
@@ -256,6 +256,28 @@ bool fromJSON(const llvm::json::Value &, NextArguments &, llvm::json::Path);
/// body field is required.
using NextResponse = VoidResponse;
+/// Arguments for `stepIn` request.
+struct StepInArguments {
+ /// Specifies the thread for which to resume execution for one step-into (of
+ /// the given granularity).
+ uint64_t threadId = LLDB_INVALID_THREAD_ID;
+
+ /// If this flag is true, all other suspended threads are not resumed.
+ bool singleThread = false;
+
+ /// Id of the target to step into.
+ std::optional<uint64_t> targetId;
+
+ /// Stepping granularity. If no granularity is specified, a granularity of
+ /// `statement` is assumed.
+ SteppingGranularity granularity = eSteppingGranularityStatement;
+};
+bool fromJSON(const llvm::json::Value &, StepInArguments &, llvm::json::Path);
+
+/// Response to `stepIn` request. This is just an acknowledgement, so no
+/// body field is required.
+using StepInResponse = VoidResponse;
+
} // namespace lldb_dap::protocol
#endif
>From 3b3382e055ba293237fed719c5d5753318ec7a2f Mon Sep 17 00:00:00 2001
From: John Harrison <harjohn at google.com>
Date: Wed, 23 Apr 2025 15:10:04 -0700
Subject: [PATCH 2/2] Restructure StepInRequestHandler::Run.
---
.../lldb-dap/Handler/StepInRequestHandler.cpp | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/lldb/tools/lldb-dap/Handler/StepInRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/StepInRequestHandler.cpp
index 00b68abb48677..15f242a9e18ff 100644
--- a/lldb/tools/lldb-dap/Handler/StepInRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/StepInRequestHandler.cpp
@@ -31,12 +31,6 @@ namespace lldb_dap {
// possible targets for a given source line can be retrieved via the
// `stepInTargets` request.
Error StepInRequestHandler::Run(const StepInArguments &args) const {
- std::string step_in_target;
- auto it = dap.step_in_targets.find(args.targetId.value_or(0));
- if (it != dap.step_in_targets.end())
- step_in_target = it->second;
-
- RunMode run_mode = args.singleThread ? eOnlyThisThread : eOnlyDuringStepping;
SBThread thread = dap.GetLLDBThread(args.threadId);
if (!thread.IsValid())
return make_error<DAPError>("invalid thread");
@@ -44,11 +38,19 @@ Error StepInRequestHandler::Run(const StepInArguments &args) const {
// Remember the thread ID that caused the resume so we can set the
// "threadCausedFocus" boolean value in the "stopped" events.
dap.focus_tid = thread.GetThreadID();
+
if (args.granularity == eSteppingGranularityInstruction) {
thread.StepInstruction(/*step_over=*/false);
- } else {
- thread.StepInto(step_in_target.c_str(), run_mode);
+ return Error::success();
}
+
+ std::string step_in_target;
+ auto it = dap.step_in_targets.find(args.targetId.value_or(0));
+ if (it != dap.step_in_targets.end())
+ step_in_target = it->second;
+
+ RunMode run_mode = args.singleThread ? eOnlyThisThread : eOnlyDuringStepping;
+ thread.StepInto(step_in_target.c_str(), run_mode);
return Error::success();
}
More information about the lldb-commits
mailing list