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

da-viper · web-flow · commit 4b6c608615a2 · 2025-06-04T08:05:31.000+01:00
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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
diff --git a/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h b/lldb/tools/lldb-dap/Protocol/ProtocolTypes.h
diff --git a/lldb/unittests/DAP/ProtocolTypesTest.cpp b/lldb/unittests/DAP/ProtocolTypesTest.cpp
