llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: Ebuka Ezike (da-viper) <details> <summary>Changes</summary> --- Full diff: https://github.com/llvm/llvm-project/pull/142439.diff 7 Files Affected: - (modified) lldb/tools/lldb-dap/Handler/RequestHandler.h (+15-1) - (modified) lldb/tools/lldb-dap/Handler/StepInTargetsRequestHandler.cpp (+71-129) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.cpp (+9) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolRequests.h (+15) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp (+27) - (modified) lldb/tools/lldb-dap/Protocol/ProtocolTypes.h (+29) - (modified) lldb/unittests/DAP/ProtocolTypesTest.cpp (+20) ``````````diff 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..440613b201c42 100644 --- a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp +++ b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.cpp @@ -582,6 +582,33 @@ llvm::json::Value toJSON(const SteppingGranularity &SG) { llvm_unreachable("unhandled stepping granularity."); } +bool fromJSON(const llvm::json::Value &Params, StepInTarget &SIT, + llvm::json::Path P) { + json::ObjectMapper O(Params, P); + return O && O.map("id", SIT.id) && O.map("label", SIT.label) && + O.mapOptional("line", SIT.line) && + O.mapOptional("column", SIT.column) && + O.mapOptional("endLine", SIT.endLine) && + O.mapOptional("endColumn", SIT.endColumn); +} + +llvm::json::Value toJSON(const StepInTarget &SIT) { + json::Object target; + + target.insert({"id", SIT.id}); + target.insert({"label", SIT.label}); + if (SIT.line) + target.insert({"line", SIT.line}); + if (SIT.column) + target.insert({"column", SIT.column}); + if (SIT.endLine) + target.insert({"endLine", SIT.endLine}); + if (SIT.endColumn) + 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..edaca049a6ffa 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. + uint64_t id = std::numeric_limits<uint64_t>::max(); + + /// The name of the step-in target (shown in the UI). + std::string label; + + /// The line of the step-in target. + std::optional<uint64_t> line; + + /// 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. + std::optional<uint64_t> column; + + /// The end line of the range covered by the step-in target. + std::optional<uint64_t> endLine; + + /// 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. + std::optional<uint64_t> endColumn; +}; +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 `````````` </details> https://github.com/llvm/llvm-project/pull/142439 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
