https://github.com/ashgti created https://github.com/llvm/llvm-project/pull/137071
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. >From f4f2fea5cebbce550de0e0c3facaac894b9f40b8 Mon Sep 17 00:00:00 2001 From: John Harrison <harj...@google.com> Date: Wed, 23 Apr 2025 15:01:53 -0700 Subject: [PATCH] [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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits