[lldb-dap] Use structured types for stepInTargets request #144072
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
da-viper
requested review from
ashgti and
JDevlieghere
and removed request for
JDevlieghere
|
@llvm/pr-subscribers-lldb Author: Ebuka Ezike (da-viper) ChangesTake 2. uses the Full diff: https://github.com/llvm/llvm-project/pull/144072.diff 10 Files Affected:
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
|
ashgti
approved these changes
Jun 16, 2025
da-viper
added a commit
that referenced
this pull request
Jun 16, 2025
Fixes #144072 buildbot error.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Take 2.
uses the
SendTargetCapabilitiesfrom #142831