Author: Ebuka Ezike Date: 2025-06-04T08:05:31+01:00 New Revision: 4b6c608615a285d81132acf8e33b81b2ec2c9bf9
URL: https://github.com/llvm/llvm-project/commit/4b6c608615a285d81132acf8e33b81b2ec2c9bf9 DIFF: https://github.com/llvm/llvm-project/commit/4b6c608615a285d81132acf8e33b81b2ec2c9bf9.diff LOG: [lldb-dap] Use structured types for stepInTargets request (#142439) Added: Modified: lldb/packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py lldb/tools/lldb-dap/EventHelper.cpp lldb/tools/lldb-dap/EventHelper.h lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp lldb/tools/lldb-dap/Handler/RequestHandler.h lldb/tools/lldb-dap/Handler/StepInTargetsRequestHandler.cpp lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp lldb/tools/lldb-dap/Protocol/ProtocolRequests.h lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp lldb/tools/lldb-dap/Protocol/ProtocolTypes.h lldb/unittests/DAP/ProtocolTypesTest.cpp Removed: ################################################################################ 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 6b41aef2bb5b8..f1e3cab06ccde 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 @@ -153,7 +153,7 @@ def __init__( self.recv_thread = threading.Thread(target=self._read_packet_thread) self.process_event_body = None self.exit_status: Optional[int] = None - self.initialize_body = None + self.initialize_body: dict[str, Any] = {} self.progress_events: list[Event] = [] self.reverse_requests = [] self.sequence = 1 @@ -300,6 +300,9 @@ def _handle_recv_packet(self, packet: Optional[ProtocolMessage]) -> bool: elif event == "breakpoint": # Breakpoint events are sent when a breakpoint is resolved self._update_verified_breakpoints([body["breakpoint"]]) + elif event == "capabilities": + # update the capabilities with new ones from the event. + self.initialize_body.update(body["capabilities"]) elif packet_type == "response": if packet["command"] == "disconnect": @@ -494,7 +497,7 @@ def get_initialize_value(self, key): """ if self.initialize_body and key in self.initialize_body: return self.initialize_body[key] - return None + raise ValueError(f"no value for key: {key} in {self.initialize_body}") def get_threads(self): if self.threads is None: 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..af698074f3479 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,49 @@ 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" + ) + is_supported = self.dap_server.get_initialize_value( + "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" + ) + is_supported = self.dap_server.get_initialize_value( + "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 c698084836e2f..0dd0b61b1e2ae 100644 --- a/lldb/tools/lldb-dap/EventHelper.cpp +++ b/lldb/tools/lldb-dap/EventHelper.cpp @@ -33,6 +33,22 @@ static void SendThreadExitedEvent(DAP &dap, lldb::tid_t tid) { dap.SendJSON(llvm::json::Value(std::move(event))); } +void SendTargetBasedCapabilities(DAP &dap) { + if (!dap.target.IsValid()) + return; + + // FIXME: stepInTargets request is only supported by the x86 + // architecture remove when `lldb::InstructionControlFlowKind` is + // supported by other architectures + const llvm::StringRef target_triple = dap.target.GetTriple(); + if (target_triple.starts_with("x86")) + return; + + protocol::Event event; + event.event = "capabilities"; + event.body = llvm::json::Object{{"supportsStepInTargetsRequest", false}}; + dap.Send(event); +} // "ProcessEvent": { // "allOf": [ // { "$ref": "#/definitions/Event" }, diff --git a/lldb/tools/lldb-dap/EventHelper.h b/lldb/tools/lldb-dap/EventHelper.h index 90b009c73089e..e648afbf67e59 100644 --- a/lldb/tools/lldb-dap/EventHelper.h +++ b/lldb/tools/lldb-dap/EventHelper.h @@ -16,6 +16,8 @@ struct DAP; enum LaunchMethod { Launch, Attach, AttachForSuspendedLaunch }; +void SendTargetBasedCapabilities(DAP &dap); + void SendProcessEvent(DAP &dap, LaunchMethod launch_method); void SendThreadStoppedEvent(DAP &dap); diff --git a/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp b/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp index 1281857ef4b60..7cbbbd7982462 100644 --- a/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp @@ -30,6 +30,7 @@ llvm::Error ConfigurationDoneRequestHandler::Run(const ConfigurationDoneArguments &) const { dap.configuration_done = true; + SendTargetBasedCapabilities(dap); // Ensure any command scripts did not leave us in an unexpected state. lldb::SBProcess process = dap.target.GetProcess(); if (!process.IsValid() || diff --git a/lldb/tools/lldb-dap/Handler/RequestHandler.h b/lldb/tools/lldb-dap/Handler/RequestHandler.h index 3a965bcc87a5e..559929ffb21e8 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.h +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h @@ -356,7 +356,21 @@ 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 RequestHandler::RequestHandler; + static llvm::StringLiteral GetCommand() { return "stepInTargets"; } + FeatureSet GetSupportedFeatures() const override { + return {protocol::eAdapterFeatureStepInTargetsRequest}; + } + llvm::Expected<protocol::StepInTargetsResponseBody> + Run(const protocol::StepInTargetsArguments &args) const override; +}; + +class StepInTargetsRequestHandler2 : public LegacyRequestHandler { public: using LegacyRequestHandler::LegacyRequestHandler; static llvm::StringLiteral GetCommand() { return "stepInTargets"; } 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 4160077d419e1..a7f28fb566d3d 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp @@ -382,6 +382,15 @@ bool fromJSON(const llvm::json::Value &Params, StepInArguments &SIA, 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 llvm::json::Value &Params, StepOutArguments &SOA, llvm::json::Path P) { json::ObjectMapper OM(Params, P); diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h index 7c774e50d6e56..570a0b6d39682 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h @@ -523,6 +523,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 3b297a0bd431f..33db5f0f4b89b 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 llvm::json::Value &Params, ValueFormat &VF, llvm::json::Path P) { json::ObjectMapper O(Params, P); diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h index f5e21c96fe17f..a9f67c280c921 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h @@ -24,6 +24,7 @@ #include "llvm/ADT/DenseSet.h" #include "llvm/Support/JSON.h" #include <cstdint> +#include <limits> #include <optional> #include <string> @@ -414,6 +415,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 &); + /// Provides formatting information for a value. struct ValueFormat { /// Display the value in hex. diff --git a/lldb/unittests/DAP/ProtocolTypesTest.cpp b/lldb/unittests/DAP/ProtocolTypesTest.cpp index 41703f4a071fb..5949abcb717d6 100644 --- a/lldb/unittests/DAP/ProtocolTypesTest.cpp +++ b/lldb/unittests/DAP/ProtocolTypesTest.cpp @@ -602,3 +602,23 @@ TEST(ProtocolTypesTest, DisassembledInstruction) { EXPECT_EQ(instruction.presentationHint, deserialized_instruction->presentationHint); } + +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
