https://github.com/da-viper created
https://github.com/llvm/llvm-project/pull/144072
Take 2.
uses the `SendTargetCapabilityes` from #142831
>From c4d909a9bb25983a955254956d3f0dab2ecc284f Mon Sep 17 00:00:00 2001
From: Ebuka Ezike <yerimy...@gmail.com>
Date: Thu, 12 Jun 2025 12:48:24 +0100
Subject: [PATCH] [lldb-dap] Use structured types for stepInTargets request
---
.../test/tools/lldb-dap/dap_server.py | 2 +-
.../stepInTargets/TestDAP_stepInTargets.py | 44 ++++
lldb/tools/lldb-dap/EventHelper.cpp | 5 +
lldb/tools/lldb-dap/Handler/RequestHandler.h | 13 +-
.../Handler/StepInTargetsRequestHandler.cpp | 200 +++++++-----------
.../lldb-dap/Protocol/ProtocolRequests.cpp | 10 +
.../lldb-dap/Protocol/ProtocolRequests.h | 15 ++
.../tools/lldb-dap/Protocol/ProtocolTypes.cpp | 22 ++
lldb/tools/lldb-dap/Protocol/ProtocolTypes.h | 28 +++
lldb/unittests/DAP/ProtocolTypesTest.cpp | 20 ++
10 files changed, 223 insertions(+), 136 deletions(-)
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
index 9786678aa53f9..baf2d4ae542ba 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py
@@ -494,7 +494,7 @@ def wait_for_terminated(self, timeout: Optional[float] =
None):
raise ValueError("didn't get terminated event")
return event_dict
- def get_capability(self, key):
+ def get_capability(self, key: str):
"""Get a value for the given key if it there is a key/value pair in
the capabilities reported by the adapter.
"""
diff --git
a/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py
b/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py
index 07acfe07c9ffc..51ccf2ccbdcad 100644
--- a/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py
+++ b/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py
@@ -78,3 +78,47 @@ def test_basic(self):
leaf_frame = self.dap_server.get_stackFrame()
self.assertIsNotNone(leaf_frame, "expect a leaf frame")
self.assertEqual(step_in_targets[1]["label"], leaf_frame["name"])
+
+ @skipIf(archs=no_match(["x86", "x86_64"]))
+ def test_supported_capability_x86_arch(self):
+ program = self.getBuildArtifact("a.out")
+ self.build_and_launch(program)
+ source = "main.cpp"
+ bp_lines = [line_number(source, "// set breakpoint here")]
+ breakpoint_ids = self.set_source_breakpoints(source, bp_lines)
+ self.assertEqual(
+ len(breakpoint_ids), len(bp_lines), "expect correct number of
breakpoints"
+ )
+ self.continue_to_breakpoints(breakpoint_ids)
+ is_supported =
self.dap_server.get_capability("supportsStepInTargetsRequest")
+
+ self.assertEqual(
+ is_supported,
+ True,
+ f"expect capability `stepInTarget` is supported with architecture
{self.getArchitecture()}",
+ )
+ # clear breakpoints.
+ self.set_source_breakpoints(source, [])
+ self.continue_to_exit()
+
+ @skipIf(archs=["x86", "x86_64"])
+ def test_supported_capability_other_archs(self):
+ program = self.getBuildArtifact("a.out")
+ self.build_and_launch(program)
+ source = "main.cpp"
+ bp_lines = [line_number(source, "// set breakpoint here")]
+ breakpoint_ids = self.set_source_breakpoints(source, bp_lines)
+ self.assertEqual(
+ len(breakpoint_ids), len(bp_lines), "expect correct number of
breakpoints"
+ )
+ self.continue_to_breakpoints(breakpoint_ids)
+ is_supported =
self.dap_server.get_capability("supportsStepInTargetsRequest")
+
+ self.assertEqual(
+ is_supported,
+ False,
+ f"expect capability `stepInTarget` is not supported with
architecture {self.getArchitecture()}",
+ )
+ # clear breakpoints.
+ self.set_source_breakpoints(source, [])
+ self.continue_to_exit()
diff --git a/lldb/tools/lldb-dap/EventHelper.cpp
b/lldb/tools/lldb-dap/EventHelper.cpp
index 9641f29698b10..364cc7ab4ef8c 100644
--- a/lldb/tools/lldb-dap/EventHelper.cpp
+++ b/lldb/tools/lldb-dap/EventHelper.cpp
@@ -44,6 +44,11 @@ void SendTargetBasedCapabilities(DAP &dap) {
protocol::CapabilitiesEventBody body;
+ const llvm::StringRef target_triple = dap.target.GetTriple();
+ if (target_triple.starts_with("x86"))
+ body.capabilities.supportedFeatures.insert(
+ protocol::eAdapterFeatureStepInTargetsRequest);
+
// We only support restarting launch requests not attach requests.
if (dap.last_launch_request)
body.capabilities.supportedFeatures.insert(
diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h
b/lldb/tools/lldb-dap/Handler/RequestHandler.h
index d3f231589b54c..0ac8ca7c9a49e 100644
--- a/lldb/tools/lldb-dap/Handler/RequestHandler.h
+++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h
@@ -353,14 +353,15 @@ class StepInRequestHandler : public
RequestHandler<protocol::StepInArguments,
llvm::Error Run(const protocol::StepInArguments &args) const override;
};
-class StepInTargetsRequestHandler : public LegacyRequestHandler {
+class StepInTargetsRequestHandler
+ : public RequestHandler<
+ protocol::StepInTargetsArguments,
+ llvm::Expected<protocol::StepInTargetsResponseBody>> {
public:
- using LegacyRequestHandler::LegacyRequestHandler;
+ using RequestHandler::RequestHandler;
static llvm::StringLiteral GetCommand() { return "stepInTargets"; }
- FeatureSet GetSupportedFeatures() const override {
- return {protocol::eAdapterFeatureStepInTargetsRequest};
- }
- void operator()(const llvm::json::Object &request) const override;
+ llvm::Expected<protocol::StepInTargetsResponseBody>
+ Run(const protocol::StepInTargetsArguments &args) const override;
};
class StepOutRequestHandler : public RequestHandler<protocol::StepOutArguments,
diff --git a/lldb/tools/lldb-dap/Handler/StepInTargetsRequestHandler.cpp
b/lldb/tools/lldb-dap/Handler/StepInTargetsRequestHandler.cpp
index 9b99791599f82..1a76371be2d58 100644
--- a/lldb/tools/lldb-dap/Handler/StepInTargetsRequestHandler.cpp
+++ b/lldb/tools/lldb-dap/Handler/StepInTargetsRequestHandler.cpp
@@ -7,143 +7,85 @@
//===----------------------------------------------------------------------===//
#include "DAP.h"
-#include "EventHelper.h"
-#include "JSONUtils.h"
+#include "Protocol/ProtocolRequests.h"
#include "RequestHandler.h"
#include "lldb/API/SBInstruction.h"
+#include "lldb/lldb-defines.h"
+using namespace lldb_dap::protocol;
namespace lldb_dap {
-// "StepInTargetsRequest": {
-// "allOf": [ { "$ref": "#/definitions/Request" }, {
-// "type": "object",
-// "description": "This request retrieves the possible step-in targets for
-// the specified stack frame.\nThese targets can be used in the `stepIn`
-// request.\nClients should only call this request if the corresponding
-// capability `supportsStepInTargetsRequest` is true.", "properties": {
-// "command": {
-// "type": "string",
-// "enum": [ "stepInTargets" ]
-// },
-// "arguments": {
-// "$ref": "#/definitions/StepInTargetsArguments"
-// }
-// },
-// "required": [ "command", "arguments" ]
-// }]
-// },
-// "StepInTargetsArguments": {
-// "type": "object",
-// "description": "Arguments for `stepInTargets` request.",
-// "properties": {
-// "frameId": {
-// "type": "integer",
-// "description": "The stack frame for which to retrieve the possible
-// step-in targets."
-// }
-// },
-// "required": [ "frameId" ]
-// },
-// "StepInTargetsResponse": {
-// "allOf": [ { "$ref": "#/definitions/Response" }, {
-// "type": "object",
-// "description": "Response to `stepInTargets` request.",
-// "properties": {
-// "body": {
-// "type": "object",
-// "properties": {
-// "targets": {
-// "type": "array",
-// "items": {
-// "$ref": "#/definitions/StepInTarget"
-// },
-// "description": "The possible step-in targets of the specified
-// source location."
-// }
-// },
-// "required": [ "targets" ]
-// }
-// },
-// "required": [ "body" ]
-// }]
-// }
-void StepInTargetsRequestHandler::operator()(
- const llvm::json::Object &request) const {
- llvm::json::Object response;
- FillResponse(request, response);
- const auto *arguments = request.getObject("arguments");
-
+// This request retrieves the possible step-in targets for the specified stack
+// frame.
+// These targets can be used in the `stepIn` request.
+// Clients should only call this request if the corresponding capability
+// `supportsStepInTargetsRequest` is true.
+llvm::Expected<StepInTargetsResponseBody>
+StepInTargetsRequestHandler::Run(const StepInTargetsArguments &args) const {
dap.step_in_targets.clear();
- lldb::SBFrame frame = dap.GetLLDBFrame(*arguments);
- if (frame.IsValid()) {
- lldb::SBAddress pc_addr = frame.GetPCAddress();
- lldb::SBAddress line_end_addr =
- pc_addr.GetLineEntry().GetSameLineContiguousAddressRangeEnd(true);
- lldb::SBInstructionList insts = dap.target.ReadInstructions(
- pc_addr, line_end_addr, /*flavor_string=*/nullptr);
-
- if (!insts.IsValid()) {
- response["success"] = false;
- response["message"] = "Failed to get instructions for frame.";
- dap.SendJSON(llvm::json::Value(std::move(response)));
- return;
- }
+ const lldb::SBFrame frame = dap.GetLLDBFrame(args.frameId);
+ if (!frame.IsValid())
+ return llvm::make_error<DAPError>("Failed to get frame for input
frameId.");
+
+ lldb::SBAddress pc_addr = frame.GetPCAddress();
+ lldb::SBAddress line_end_addr =
+ pc_addr.GetLineEntry().GetSameLineContiguousAddressRangeEnd(true);
+ lldb::SBInstructionList insts = dap.target.ReadInstructions(
+ pc_addr, line_end_addr, /*flavor_string=*/nullptr);
+
+ if (!insts.IsValid())
+ return llvm::make_error<DAPError>("Failed to get instructions for frame.");
+
+ StepInTargetsResponseBody body;
+ const size_t num_insts = insts.GetSize();
+ for (size_t i = 0; i < num_insts; ++i) {
+ lldb::SBInstruction inst = insts.GetInstructionAtIndex(i);
+ if (!inst.IsValid())
+ break;
+
+ const lldb::addr_t inst_addr =
inst.GetAddress().GetLoadAddress(dap.target);
+ if (inst_addr == LLDB_INVALID_ADDRESS)
+ break;
+
+ // Note: currently only x86/x64 supports flow kind.
+ const lldb::InstructionControlFlowKind flow_kind =
+ inst.GetControlFlowKind(dap.target);
+
+ if (flow_kind == lldb::eInstructionControlFlowKindCall) {
+
+ const llvm::StringRef call_operand_name = inst.GetOperands(dap.target);
+ lldb::addr_t call_target_addr = LLDB_INVALID_ADDRESS;
+ if (call_operand_name.getAsInteger(0, call_target_addr))
+ continue;
+
+ const lldb::SBAddress call_target_load_addr =
+ dap.target.ResolveLoadAddress(call_target_addr);
+ if (!call_target_load_addr.IsValid())
+ continue;
+
+ // The existing ThreadPlanStepInRange only accept step in target
+ // function with debug info.
+ lldb::SBSymbolContext sc = dap.target.ResolveSymbolContextForAddress(
+ call_target_load_addr, lldb::eSymbolContextFunction);
+
+ // The existing ThreadPlanStepInRange only accept step in target
+ // function with debug info.
+ llvm::StringRef step_in_target_name;
+ if (sc.IsValid() && sc.GetFunction().IsValid())
+ step_in_target_name = sc.GetFunction().GetDisplayName();
+
+ // Skip call sites if we fail to resolve its symbol name.
+ if (step_in_target_name.empty())
+ continue;
- llvm::json::Array step_in_targets;
- const auto num_insts = insts.GetSize();
- for (size_t i = 0; i < num_insts; ++i) {
- lldb::SBInstruction inst = insts.GetInstructionAtIndex(i);
- if (!inst.IsValid())
- break;
-
- lldb::addr_t inst_addr = inst.GetAddress().GetLoadAddress(dap.target);
-
- // Note: currently only x86/x64 supports flow kind.
- lldb::InstructionControlFlowKind flow_kind =
- inst.GetControlFlowKind(dap.target);
- if (flow_kind == lldb::eInstructionControlFlowKindCall) {
- // Use call site instruction address as id which is easy to debug.
- llvm::json::Object step_in_target;
- step_in_target["id"] = inst_addr;
-
- llvm::StringRef call_operand_name = inst.GetOperands(dap.target);
- lldb::addr_t call_target_addr;
- if (call_operand_name.getAsInteger(0, call_target_addr))
- continue;
-
- lldb::SBAddress call_target_load_addr =
- dap.target.ResolveLoadAddress(call_target_addr);
- if (!call_target_load_addr.IsValid())
- continue;
-
- // The existing ThreadPlanStepInRange only accept step in target
- // function with debug info.
- lldb::SBSymbolContext sc = dap.target.ResolveSymbolContextForAddress(
- call_target_load_addr, lldb::eSymbolContextFunction);
-
- // The existing ThreadPlanStepInRange only accept step in target
- // function with debug info.
- std::string step_in_target_name;
- if (sc.IsValid() && sc.GetFunction().IsValid())
- step_in_target_name = sc.GetFunction().GetDisplayName();
-
- // Skip call sites if we fail to resolve its symbol name.
- if (step_in_target_name.empty())
- continue;
-
- dap.step_in_targets.try_emplace(inst_addr, step_in_target_name);
- step_in_target.try_emplace("label", step_in_target_name);
- step_in_targets.emplace_back(std::move(step_in_target));
- }
+ StepInTarget target;
+ target.id = inst_addr;
+ target.label = step_in_target_name;
+ dap.step_in_targets.try_emplace(inst_addr, step_in_target_name);
+ body.targets.emplace_back(std::move(target));
}
- llvm::json::Object body;
- body.try_emplace("targets", std::move(step_in_targets));
- response.try_emplace("body", std::move(body));
- } else {
- response["success"] = llvm::json::Value(false);
- response["message"] = "Failed to get frame for input frameId.";
}
- dap.SendJSON(llvm::json::Value(std::move(response)));
-}
+ return body;
+};
} // namespace lldb_dap
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
index 2cb7c47d60203..1b1891ba59e61 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp
@@ -368,6 +368,16 @@ bool fromJSON(const json::Value &Params, StepInArguments
&SIA, json::Path P) {
OM.mapOptional("granularity", SIA.granularity);
}
+bool fromJSON(const llvm::json::Value &Params, StepInTargetsArguments &SITA,
+ llvm::json::Path P) {
+ json::ObjectMapper OM(Params, P);
+ return OM && OM.map("frameId", SITA.frameId);
+}
+
+llvm::json::Value toJSON(const StepInTargetsResponseBody &SITR) {
+ return llvm::json::Object{{"targets", SITR.targets}};
+}
+
bool fromJSON(const json::Value &Params, StepOutArguments &SOA, json::Path P) {
json::ObjectMapper OM(Params, P);
return OM && OM.map("threadId", SOA.threadId) &&
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
index d199cc886b11c..583c203be8e1a 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h
@@ -533,6 +533,21 @@ bool fromJSON(const llvm::json::Value &, StepInArguments
&, llvm::json::Path);
/// body field is required.
using StepInResponse = VoidResponse;
+/// Arguments for `stepInTargets` request.
+struct StepInTargetsArguments {
+ /// The stack frame for which to retrieve the possible step-in targets.
+ uint64_t frameId = LLDB_INVALID_FRAME_ID;
+};
+bool fromJSON(const llvm::json::Value &, StepInTargetsArguments &,
+ llvm::json::Path);
+
+/// Response to `stepInTargets` request.
+struct StepInTargetsResponseBody {
+ /// The possible step-in targets of the specified source location.
+ std::vector<StepInTarget> targets;
+};
+llvm::json::Value toJSON(const StepInTargetsResponseBody &);
+
/// Arguments for `stepOut` request.
struct StepOutArguments {
/// Specifies the thread for which to resume execution for one step-out (of
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp
b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp
index 085d53bb006ef..c1c1ec509fb3b 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp
@@ -582,6 +582,28 @@ llvm::json::Value toJSON(const SteppingGranularity &SG) {
llvm_unreachable("unhandled stepping granularity.");
}
+bool fromJSON(const json::Value &Params, StepInTarget &SIT, json::Path P) {
+ json::ObjectMapper O(Params, P);
+ return O && O.map("id", SIT.id) && O.map("label", SIT.label) &&
+ O.map("line", SIT.line) && O.map("column", SIT.column) &&
+ O.map("endLine", SIT.endLine) && O.map("endColumn", SIT.endColumn);
+}
+
+llvm::json::Value toJSON(const StepInTarget &SIT) {
+ json::Object target{{"id", SIT.id}, {"label", SIT.label}};
+
+ if (SIT.line != LLDB_INVALID_LINE_NUMBER)
+ target.insert({"line", SIT.line});
+ if (SIT.column != LLDB_INVALID_COLUMN_NUMBER)
+ target.insert({"column", SIT.column});
+ if (SIT.endLine != LLDB_INVALID_LINE_NUMBER)
+ target.insert({"endLine", SIT.endLine});
+ if (SIT.endLine != LLDB_INVALID_COLUMN_NUMBER)
+ target.insert({"endColumn", SIT.endColumn});
+
+ return target;
+}
+
bool fromJSON(const json::Value &Params, Thread &T, json::Path P) {
json::ObjectMapper O(Params, P);
return O && O.map("id", T.id) && O.map("name", T.name);
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h
b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h
index c7acfc482987b..d7094fbab9e59 100644
--- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h
+++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h
@@ -414,6 +414,34 @@ bool fromJSON(const llvm::json::Value &,
SteppingGranularity &,
llvm::json::Path);
llvm::json::Value toJSON(const SteppingGranularity &);
+/// A `StepInTarget` can be used in the `stepIn` request and determines into
+/// which single target the `stepIn` request should step.
+struct StepInTarget {
+ /// Unique identifier for a step-in target.
+ lldb::addr_t id = LLDB_INVALID_ADDRESS;
+
+ /// The name of the step-in target (shown in the UI).
+ std::string label;
+
+ /// The line of the step-in target.
+ uint32_t line = LLDB_INVALID_LINE_NUMBER;
+
+ /// Start position of the range covered by the step in target. It is measured
+ /// in UTF-16 code units and the client capability `columnsStartAt1`
+ /// determines whether it is 0- or 1-based.
+ uint32_t column = LLDB_INVALID_COLUMN_NUMBER;
+
+ /// The end line of the range covered by the step-in target.
+ uint32_t endLine = LLDB_INVALID_LINE_NUMBER;
+
+ /// End position of the range covered by the step in target. It is measured
in
+ /// UTF-16 code units and the client capability `columnsStartAt1` determines
+ /// whether it is 0- or 1-based.
+ uint32_t endColumn = LLDB_INVALID_COLUMN_NUMBER;
+};
+bool fromJSON(const llvm::json::Value &, StepInTarget &, llvm::json::Path);
+llvm::json::Value toJSON(const StepInTarget &);
+
/// A Thread.
struct Thread {
/// Unique identifier for the thread.
diff --git a/lldb/unittests/DAP/ProtocolTypesTest.cpp
b/lldb/unittests/DAP/ProtocolTypesTest.cpp
index adf43c9ac2046..f2a23db346565 100644
--- a/lldb/unittests/DAP/ProtocolTypesTest.cpp
+++ b/lldb/unittests/DAP/ProtocolTypesTest.cpp
@@ -686,3 +686,23 @@ TEST(ProtocolTypesTest, CapabilitiesEventBody) {
// Validate toJSON
EXPECT_EQ(json, pp(body));
}
+
+TEST(ProtocolTypesTest, StepInTarget) {
+ StepInTarget target;
+ target.id = 230;
+ target.label = "the_function_name";
+ target.line = 2;
+ target.column = 320;
+ target.endLine = 32;
+ target.endColumn = 23;
+
+ llvm::Expected<StepInTarget> deserialized_target = roundtrip(target);
+ ASSERT_THAT_EXPECTED(deserialized_target, llvm::Succeeded());
+
+ EXPECT_EQ(target.id, deserialized_target->id);
+ EXPECT_EQ(target.label, deserialized_target->label);
+ EXPECT_EQ(target.line, deserialized_target->line);
+ EXPECT_EQ(target.column, deserialized_target->column);
+ EXPECT_EQ(target.endLine, deserialized_target->endLine);
+ EXPECT_EQ(target.endColumn, deserialized_target->endColumn);
+}
\ No newline at end of file
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
