Revert "[lldb-dap] Use structured types for stepInTargets request (#142439)" (#142891)

This reverts commit 4b6c608615a285d81132acf8e33b81b2ec2c9bf9 and
follow-up commits 159de3633640a5cb2d322ebe8cc4ec0c1c9a896d and
c9e1c52e2e75a91a44a98df818cc9bd11655e51d because this breaks
TestDAP_stepInTargets.py on Darwin.

https://green.lab.llvm.org/job/llvm.org/view/LLDB/job/as-lldb-cmake
This commit is contained in:
Jonas Devlieghere 2025-06-04 20:19:56 -07:00 committed by GitHub
parent e4c8ff94e7
commit d8ba707b0c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 131 additions and 252 deletions

View File

@ -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:

View File

@ -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()

View File

@ -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" },

View File

@ -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);

View File

@ -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() ||

View File

@ -356,21 +356,7 @@ public:
llvm::Error Run(const protocol::StepInArguments &args) const override;
};
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 {
class StepInTargetsRequestHandler : public LegacyRequestHandler {
public:
using LegacyRequestHandler::LegacyRequestHandler;
static llvm::StringLiteral GetCommand() { return "stepInTargets"; }

View File

@ -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<StepInTargetsResponseBody>
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<DAPError>("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<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;
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

View File

@ -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);

View File

@ -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<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

View File

@ -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);

View File

@ -24,7 +24,6 @@
#include "llvm/ADT/DenseSet.h"
#include "llvm/Support/JSON.h"
#include <cstdint>
#include <limits>
#include <optional>
#include <string>
@ -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.

View File

@ -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<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);
}