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 f1e3cab06ccd..6b41aef2bb5b 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 @@ class DebugCommunication(object): self.recv_thread = threading.Thread(target=self._read_packet_thread) self.process_event_body = None self.exit_status: Optional[int] = None - self.initialize_body: dict[str, Any] = {} + self.initialize_body = None self.progress_events: list[Event] = [] self.reverse_requests = [] self.sequence = 1 @@ -300,9 +300,6 @@ class DebugCommunication(object): 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": @@ -497,7 +494,7 @@ class DebugCommunication(object): """ if self.initialize_body and key in self.initialize_body: return self.initialize_body[key] - raise ValueError(f"no value for key: {key} in {self.initialize_body}") + return None 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 af698074f347..07acfe07c9ff 100644 --- a/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py +++ b/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py @@ -78,49 +78,3 @@ class TestDAP_stepInTargets(lldbdap_testcase.DAPTestCaseBase): 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 33bc7c2cbef1..c698084836e2 100644 --- a/lldb/tools/lldb-dap/EventHelper.cpp +++ b/lldb/tools/lldb-dap/EventHelper.cpp @@ -33,24 +33,6 @@ 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{ - {"capabilities", - 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 e648afbf67e5..90b009c73089 100644 --- a/lldb/tools/lldb-dap/EventHelper.h +++ b/lldb/tools/lldb-dap/EventHelper.h @@ -16,8 +16,6 @@ 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 7cbbbd798246..1281857ef4b6 100644 --- a/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/ConfigurationDoneRequestHandler.cpp @@ -30,7 +30,6 @@ 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 559929ffb21e..3a965bcc87a5 100644 --- a/lldb/tools/lldb-dap/Handler/RequestHandler.h +++ b/lldb/tools/lldb-dap/Handler/RequestHandler.h @@ -356,21 +356,7 @@ public: llvm::Error Run(const protocol::StepInArguments &args) const override; }; -class StepInTargetsRequestHandler - : public RequestHandler< - protocol::StepInTargetsArguments, - llvm::Expected> { -public: - using RequestHandler::RequestHandler; - static llvm::StringLiteral GetCommand() { return "stepInTargets"; } - FeatureSet GetSupportedFeatures() const override { - return {protocol::eAdapterFeatureStepInTargetsRequest}; - } - llvm::Expected - Run(const protocol::StepInTargetsArguments &args) const override; -}; - -class StepInTargetsRequestHandler2 : public LegacyRequestHandler { +class StepInTargetsRequestHandler : 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 9295b6ceae36..9b99791599f8 100644 --- a/lldb/tools/lldb-dap/Handler/StepInTargetsRequestHandler.cpp +++ b/lldb/tools/lldb-dap/Handler/StepInTargetsRequestHandler.cpp @@ -7,85 +7,143 @@ //===----------------------------------------------------------------------===// #include "DAP.h" -#include "Protocol/ProtocolRequests.h" +#include "EventHelper.h" +#include "JSONUtils.h" #include "RequestHandler.h" #include "lldb/API/SBInstruction.h" -#include "lldb/lldb-defines.h" -using namespace lldb_dap::protocol; namespace lldb_dap { -// 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 -StepInTargetsRequestHandler::Run(const StepInTargetsArguments &args) const { +// "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"); + dap.step_in_targets.clear(); - const lldb::SBFrame frame = dap.GetLLDBFrame(args.frameId); - if (!frame.IsValid()) - return llvm::make_error("Failed to get frame for input frameId."); + 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); - 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("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; - - 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)); + if (!insts.IsValid()) { + response["success"] = false; + response["message"] = "Failed to get instructions for frame."; + dap.SendJSON(llvm::json::Value(std::move(response))); + return; } + + 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)); + } + } + 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."; } - return body; + dap.SendJSON(llvm::json::Value(std::move(response))); } } // namespace lldb_dap diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp index a7f28fb566d3..4160077d419e 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp @@ -382,15 +382,6 @@ 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 570a0b6d3968..7c774e50d6e5 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolRequests.h @@ -523,21 +523,6 @@ 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 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 33db5f0f4b89..3b297a0bd431 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp +++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp @@ -582,28 +582,6 @@ 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 a9f67c280c92..f5e21c96fe17 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h +++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h @@ -24,7 +24,6 @@ #include "llvm/ADT/DenseSet.h" #include "llvm/Support/JSON.h" #include -#include #include #include @@ -415,34 +414,6 @@ 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 5949abcb717d..41703f4a071f 100644 --- a/lldb/unittests/DAP/ProtocolTypesTest.cpp +++ b/lldb/unittests/DAP/ProtocolTypesTest.cpp @@ -602,23 +602,3 @@ 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 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