[Lldb-commits] [lldb] Initial step in targets DAP support (PR #86623)

via lldb-commits lldb-commits at lists.llvm.org
Tue Apr 2 08:18:38 PDT 2024


https://github.com/jeffreytan81 updated https://github.com/llvm/llvm-project/pull/86623

>From 6cccde22723157260e7c0b19bf8372aae8d1afaa Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at fb.com>
Date: Wed, 6 Mar 2024 12:07:03 -0800
Subject: [PATCH 1/7] Fix strcmp build error on buildbot

---
 lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp b/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp
index b0a4446ba01581..40cb63755ee8a5 100644
--- a/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp
+++ b/lldb/test/API/functionalities/fork/concurrent_vfork/main.cpp
@@ -1,6 +1,7 @@
 #include <assert.h>
 #include <iostream>
 #include <mutex>
+#include <string.h>
 #include <sys/wait.h>
 #include <thread>
 #include <unistd.h>

>From 06ec5e2a045e6ab4d97029fa0de5cc52dc4f0769 Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at fb.com>
Date: Mon, 25 Mar 2024 20:51:56 -0700
Subject: [PATCH 2/7] Initial support for stepInTargets feature

---
 lldb/include/lldb/API/SBCompileUnit.h         |   5 +
 lldb/include/lldb/API/SBLineEntry.h           |   3 +
 lldb/include/lldb/API/SBSymbolContextList.h   |   1 +
 .../test/tools/lldb-dap/dap_server.py         |  17 +-
 .../test/tools/lldb-dap/lldbdap_testcase.py   |   4 +-
 lldb/source/API/SBCompileUnit.cpp             |  17 ++
 lldb/source/API/SBLineEntry.cpp               |  15 ++
 .../API/tools/lldb-dap/stepInTargets/Makefile |   6 +
 .../stepInTargets/TestDAP_stepInTargets.py    |  66 ++++++++
 .../API/tools/lldb-dap/stepInTargets/main.cpp |  17 ++
 lldb/tools/lldb-dap/DAP.h                     |   2 +
 lldb/tools/lldb-dap/lldb-dap.cpp              | 149 +++++++++++++++++-
 12 files changed, 293 insertions(+), 9 deletions(-)
 create mode 100644 lldb/test/API/tools/lldb-dap/stepInTargets/Makefile
 create mode 100644 lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py
 create mode 100644 lldb/test/API/tools/lldb-dap/stepInTargets/main.cpp

diff --git a/lldb/include/lldb/API/SBCompileUnit.h b/lldb/include/lldb/API/SBCompileUnit.h
index 36492d9398ce9e..4fc9ad43386f45 100644
--- a/lldb/include/lldb/API/SBCompileUnit.h
+++ b/lldb/include/lldb/API/SBCompileUnit.h
@@ -11,6 +11,7 @@
 
 #include "lldb/API/SBDefines.h"
 #include "lldb/API/SBFileSpec.h"
+#include <optional>
 
 namespace lldb {
 
@@ -30,6 +31,10 @@ class LLDB_API SBCompileUnit {
 
   lldb::SBFileSpec GetFileSpec() const;
 
+  lldb::SBSymbolContextList
+  ResolveSymbolContext(const char *file, uint32_t line,
+                       std::optional<uint16_t> column = std::nullopt) const;
+
   uint32_t GetNumLineEntries() const;
 
   lldb::SBLineEntry GetLineEntryAtIndex(uint32_t idx) const;
diff --git a/lldb/include/lldb/API/SBLineEntry.h b/lldb/include/lldb/API/SBLineEntry.h
index 7c2431ba3c8a51..d70c4fac6ec717 100644
--- a/lldb/include/lldb/API/SBLineEntry.h
+++ b/lldb/include/lldb/API/SBLineEntry.h
@@ -29,6 +29,9 @@ class LLDB_API SBLineEntry {
 
   lldb::SBAddress GetEndAddress() const;
 
+  lldb::SBAddress
+  GetSameLineContiguousAddressRangeEnd(bool include_inlined_functions) const;
+
   explicit operator bool() const;
 
   bool IsValid() const;
diff --git a/lldb/include/lldb/API/SBSymbolContextList.h b/lldb/include/lldb/API/SBSymbolContextList.h
index 4026afc213571c..95100d219df20f 100644
--- a/lldb/include/lldb/API/SBSymbolContextList.h
+++ b/lldb/include/lldb/API/SBSymbolContextList.h
@@ -44,6 +44,7 @@ class LLDB_API SBSymbolContextList {
 protected:
   friend class SBModule;
   friend class SBTarget;
+  friend class SBCompileUnit;
 
   lldb_private::SymbolContextList *operator->() const;
 
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 27a76a652f4063..f2ee6bd8212415 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
@@ -811,23 +811,30 @@ def request_next(self, threadId):
         command_dict = {"command": "next", "type": "request", "arguments": args_dict}
         return self.send_recv(command_dict)
 
-    def request_stepIn(self, threadId):
+    def request_stepInTargets(self, frameId):
         if self.exit_status is not None:
-            raise ValueError("request_continue called after process exited")
-        args_dict = {"threadId": threadId}
+            raise ValueError("request_stepInTargets called after process exited")
+        args_dict = {"frameId": frameId}
+        command_dict = {"command": "stepInTargets", "type": "request", "arguments": args_dict}
+        return self.send_recv(command_dict)
+    
+    def request_stepIn(self, threadId, targetId):
+        if self.exit_status is not None:
+            raise ValueError("request_stepIn called after process exited")
+        args_dict = {"threadId": threadId, "targetId": targetId}
         command_dict = {"command": "stepIn", "type": "request", "arguments": args_dict}
         return self.send_recv(command_dict)
 
     def request_stepOut(self, threadId):
         if self.exit_status is not None:
-            raise ValueError("request_continue called after process exited")
+            raise ValueError("request_stepOut called after process exited")
         args_dict = {"threadId": threadId}
         command_dict = {"command": "stepOut", "type": "request", "arguments": args_dict}
         return self.send_recv(command_dict)
 
     def request_pause(self, threadId=None):
         if self.exit_status is not None:
-            raise ValueError("request_continue called after process exited")
+            raise ValueError("request_pause called after process exited")
         if threadId is None:
             threadId = self.get_thread_id()
         args_dict = {"threadId": threadId}
diff --git a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
index 23f650d2d36fdd..d56ea5dca14beb 100644
--- a/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
+++ b/lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py
@@ -218,8 +218,8 @@ def set_global(self, name, value, id=None):
         """Set a top level global variable only."""
         return self.dap_server.request_setVariable(2, name, str(value), id=id)
 
-    def stepIn(self, threadId=None, waitForStop=True):
-        self.dap_server.request_stepIn(threadId=threadId)
+    def stepIn(self, threadId=None, targetId=None, waitForStop=True):
+        self.dap_server.request_stepIn(threadId=threadId, targetId=targetId)
         if waitForStop:
             return self.dap_server.wait_for_stopped()
         return None
diff --git a/lldb/source/API/SBCompileUnit.cpp b/lldb/source/API/SBCompileUnit.cpp
index 65fdb11032b9c0..8d99bd5cbb0d5d 100644
--- a/lldb/source/API/SBCompileUnit.cpp
+++ b/lldb/source/API/SBCompileUnit.cpp
@@ -49,6 +49,23 @@ SBFileSpec SBCompileUnit::GetFileSpec() const {
   return file_spec;
 }
 
+lldb::SBSymbolContextList
+SBCompileUnit::ResolveSymbolContext(const char *file, uint32_t line,
+                                    std::optional<uint16_t> column) const {
+  LLDB_INSTRUMENT_VA(this, file, line);
+
+  lldb::SBSymbolContextList sc_list;
+  FileSpec file_spec(file);
+  SourceLocationSpec location_spec(file_spec, line, /*column=*/column,
+                                   /*check_inlines=*/true,
+                                   /*exact_match=*/true);
+
+  if (m_opaque_ptr)
+    m_opaque_ptr->ResolveSymbolContext(location_spec, eSymbolContextLineEntry,
+                                       *sc_list);
+  return sc_list;
+}
+
 uint32_t SBCompileUnit::GetNumLineEntries() const {
   LLDB_INSTRUMENT_VA(this);
 
diff --git a/lldb/source/API/SBLineEntry.cpp b/lldb/source/API/SBLineEntry.cpp
index 99a7b8fe644cb5..216ea6d18eab89 100644
--- a/lldb/source/API/SBLineEntry.cpp
+++ b/lldb/source/API/SBLineEntry.cpp
@@ -67,6 +67,21 @@ SBAddress SBLineEntry::GetEndAddress() const {
   return sb_address;
 }
 
+SBAddress SBLineEntry::GetSameLineContiguousAddressRangeEnd(
+    bool include_inlined_functions) const {
+  LLDB_INSTRUMENT_VA(this);
+
+  SBAddress sb_address;
+  if (m_opaque_up) {
+    AddressRange line_range = m_opaque_up->GetSameLineContiguousAddressRange(
+        include_inlined_functions);
+
+    sb_address.SetAddress(line_range.GetBaseAddress());
+    sb_address.OffsetAddress(line_range.GetByteSize());
+  }
+  return sb_address;
+}
+
 bool SBLineEntry::IsValid() const {
   LLDB_INSTRUMENT_VA(this);
   return this->operator bool();
diff --git a/lldb/test/API/tools/lldb-dap/stepInTargets/Makefile b/lldb/test/API/tools/lldb-dap/stepInTargets/Makefile
new file mode 100644
index 00000000000000..f772575cd5613b
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/stepInTargets/Makefile
@@ -0,0 +1,6 @@
+	
+ENABLE_THREADS := YES
+
+CXX_SOURCES := main.cpp
+
+include Makefile.rules
diff --git a/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py b/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py
new file mode 100644
index 00000000000000..3fa955305037cf
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py
@@ -0,0 +1,66 @@
+"""
+Test lldb-dap stepInTargets request
+"""
+
+import dap_server
+from lldbsuite.test.decorators import *
+from lldbsuite.test.lldbtest import *
+import lldbdap_testcase
+from lldbsuite.test import lldbutil
+
+
+class TestDAP_stepInTargets(lldbdap_testcase.DAPTestCaseBase):
+    @skipIf(archs=no_match(["x86_64"]))  # ARM flow kind is not supported yet.
+    def test_basic(self):
+        """
+        Tests the basic stepping in targets with directly calls.
+        """
+        program = self.getBuildArtifact("a.out")
+        self.build_and_launch(program)
+        source = "main.cpp"
+
+        breakpoint_line = line_number(source, "// set breakpoint here")
+        lines = [breakpoint_line]
+        # Set breakpoint in the thread function so we can step the threads
+        breakpoint_ids = self.set_source_breakpoints(source, lines)
+        self.assertEqual(
+            len(breakpoint_ids), len(lines), "expect correct number of breakpoints"
+        )
+        self.continue_to_breakpoints(breakpoint_ids)
+
+        threads = self.dap_server.get_threads()
+        self.assertEqual(len(threads), 1, "expect one thread")
+        tid = threads[0]["id"]
+
+        leaf_frame = self.dap_server.get_stackFrame()
+        self.assertIsNotNone(leaf_frame, "expect a leaf frame")
+
+        # Request all step in targets list and verify the response.
+        step_in_targets_response = self.dap_server.request_stepInTargets(
+            leaf_frame["id"]
+        )
+        self.assertEqual(step_in_targets_response["success"], True, "expect success")
+        self.assertIn(
+            "body", step_in_targets_response, "expect body field in response body"
+        )
+        self.assertIn(
+            "targets",
+            step_in_targets_response["body"],
+            "expect targets field in response body",
+        )
+
+        step_in_targets = step_in_targets_response["body"]["targets"]
+        self.assertEqual(len(step_in_targets), 3, "expect 3 step in targets")
+
+        # Verify the target names are correct.
+        self.assertEqual(step_in_targets[0]["label"], "bar()", "expect bar()")
+        self.assertEqual(step_in_targets[1]["label"], "bar2()", "expect bar2()")
+        self.assertEqual(
+            step_in_targets[2]["label"], "foo(int, int)", "expect foo(int, int)"
+        )
+
+        # Choose to step into second target and verify that we are in bar2()
+        self.stepIn(threadId=tid, targetId=step_in_targets[1]["id"], waitForStop=True)
+        leaf_frame = self.dap_server.get_stackFrame()
+        self.assertIsNotNone(leaf_frame, "expect a leaf frame")
+        self.assertEqual(leaf_frame["name"], "bar2()")
diff --git a/lldb/test/API/tools/lldb-dap/stepInTargets/main.cpp b/lldb/test/API/tools/lldb-dap/stepInTargets/main.cpp
new file mode 100644
index 00000000000000..cd4419e8a4b047
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/stepInTargets/main.cpp
@@ -0,0 +1,17 @@
+
+int foo(int val, int extra) {
+  return val + extra;
+}
+
+int bar() {
+  return 22;
+}
+
+int bar2() {
+  return 54;
+}
+
+int main(int argc, char const *argv[]) { 
+  foo(bar(), bar2()); // set breakpoint here
+  return 0;
+}
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 8015dec9ba8fe6..5c70a056fea4bf 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -162,6 +162,8 @@ struct DAP {
   std::vector<std::string> exit_commands;
   std::vector<std::string> stop_commands;
   std::vector<std::string> terminate_commands;
+  // Map step in target id to list of function targets that user can choose.
+  llvm::DenseMap<lldb::addr_t, std::string> step_in_targets;
   // A copy of the last LaunchRequest or AttachRequest so we can reuse its
   // arguments if we get a RestartRequest.
   std::optional<llvm::json::Object> last_launch_or_attach_request;
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 55f8c920e60016..1331bf2d03d20f 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -1645,7 +1645,7 @@ void request_initialize(const llvm::json::Object &request) {
   // The debug adapter supports the gotoTargetsRequest.
   body.try_emplace("supportsGotoTargetsRequest", false);
   // The debug adapter supports the stepInTargetsRequest.
-  body.try_emplace("supportsStepInTargetsRequest", false);
+  body.try_emplace("supportsStepInTargetsRequest", true);
   // The debug adapter supports the completions request.
   body.try_emplace("supportsCompletionsRequest", true);
   // The debug adapter supports the disassembly request.
@@ -3180,14 +3180,158 @@ void request_stepIn(const llvm::json::Object &request) {
   llvm::json::Object response;
   FillResponse(request, response);
   auto arguments = request.getObject("arguments");
+
+  std::string step_in_target;
+  uint64_t target_id = GetUnsigned(arguments, "targetId", 0);
+  auto it = g_dap.step_in_targets.find(target_id);
+  if (it != g_dap.step_in_targets.end())
+    step_in_target = it->second;
+  
+  const bool single_thread = GetBoolean(arguments, "singleThread", false);
+  lldb::RunMode run_mode =
+      single_thread ? lldb::eOnlyThisThread : lldb::eOnlyDuringStepping;
   lldb::SBThread thread = g_dap.GetLLDBThread(*arguments);
   if (thread.IsValid()) {
     // Remember the thread ID that caused the resume so we can set the
     // "threadCausedFocus" boolean value in the "stopped" events.
     g_dap.focus_tid = thread.GetThreadID();
-    thread.StepInto();
+    thread.StepInto(step_in_target.c_str(), run_mode);
+  } else {
+    response["success"] = llvm::json::Value(false);
+  }
+  g_dap.SendJSON(llvm::json::Value(std::move(response)));
+}
+
+// "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 request_stepInTargets(const llvm::json::Object &request) {
+  llvm::json::Object response;
+  FillResponse(request, response);
+  auto arguments = request.getObject("arguments");
+
+  g_dap.step_in_targets.clear();
+  lldb::SBFrame frame = g_dap.GetLLDBFrame(*arguments);
+  if (frame.IsValid()) {
+    lldb::SBAddress pc_addr = frame.GetPCAddress();
+    lldb::addr_t line_end_addr = pc_addr.GetLineEntry()
+                                     .GetSameLineContiguousAddressRangeEnd(true)
+                                     .GetLoadAddress(g_dap.target);
+
+    int max_inst_count = line_end_addr - pc_addr.GetLoadAddress(g_dap.target);
+    lldb::SBInstructionList insts =
+        g_dap.target.ReadInstructions(pc_addr, max_inst_count);
+
+    if (!insts.IsValid()) {
+      response["success"] = false;
+      response["message"] = "Failed to get instructions for frame.";
+      g_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(g_dap.target);
+      // line_end_addr is exclusive of the line range.
+      if (inst_addr >= line_end_addr)
+        break;
+
+      // Note: currently only x86/x64 supports flow kind.
+      lldb::InstructionControlFlowKind flow_kind =
+          inst.GetControlFlowKind(g_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(g_dap.target);
+        lldb::addr_t call_target_addr;
+        if (call_operand_name.getAsInteger(0, call_target_addr))
+          continue;
+
+        lldb::SBAddress call_target_load_addr =
+            g_dap.target.ResolveLoadAddress(call_target_addr);
+        if (!call_target_load_addr.IsValid())
+          continue;
+
+        lldb::SBSymbolContext sc = g_dap.target.ResolveSymbolContextForAddress(
+            call_target_load_addr, lldb::eSymbolContextFunction);
+
+        // Step in targets only supports functions 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;
+
+        g_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.";
   }
   g_dap.SendJSON(llvm::json::Value(std::move(response)));
 }
@@ -3904,6 +4048,7 @@ void RegisterRequestCallbacks() {
   g_dap.RegisterRequestCallback("source", request_source);
   g_dap.RegisterRequestCallback("stackTrace", request_stackTrace);
   g_dap.RegisterRequestCallback("stepIn", request_stepIn);
+  g_dap.RegisterRequestCallback("stepInTargets", request_stepInTargets);
   g_dap.RegisterRequestCallback("stepOut", request_stepOut);
   g_dap.RegisterRequestCallback("threads", request_threads);
   g_dap.RegisterRequestCallback("variables", request_variables);

>From b2dfdb546808c495779e5781c6619fcadb752b00 Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at fb.com>
Date: Mon, 25 Mar 2024 20:57:55 -0700
Subject: [PATCH 3/7] Remove unnecessary changes

---
 lldb/include/lldb/API/SBCompileUnit.h |  5 -----
 lldb/source/API/SBCompileUnit.cpp     | 17 -----------------
 lldb/tools/lldb-dap/lldb-dap.cpp      |  3 ++-
 3 files changed, 2 insertions(+), 23 deletions(-)

diff --git a/lldb/include/lldb/API/SBCompileUnit.h b/lldb/include/lldb/API/SBCompileUnit.h
index 4fc9ad43386f45..36492d9398ce9e 100644
--- a/lldb/include/lldb/API/SBCompileUnit.h
+++ b/lldb/include/lldb/API/SBCompileUnit.h
@@ -11,7 +11,6 @@
 
 #include "lldb/API/SBDefines.h"
 #include "lldb/API/SBFileSpec.h"
-#include <optional>
 
 namespace lldb {
 
@@ -31,10 +30,6 @@ class LLDB_API SBCompileUnit {
 
   lldb::SBFileSpec GetFileSpec() const;
 
-  lldb::SBSymbolContextList
-  ResolveSymbolContext(const char *file, uint32_t line,
-                       std::optional<uint16_t> column = std::nullopt) const;
-
   uint32_t GetNumLineEntries() const;
 
   lldb::SBLineEntry GetLineEntryAtIndex(uint32_t idx) const;
diff --git a/lldb/source/API/SBCompileUnit.cpp b/lldb/source/API/SBCompileUnit.cpp
index 8d99bd5cbb0d5d..65fdb11032b9c0 100644
--- a/lldb/source/API/SBCompileUnit.cpp
+++ b/lldb/source/API/SBCompileUnit.cpp
@@ -49,23 +49,6 @@ SBFileSpec SBCompileUnit::GetFileSpec() const {
   return file_spec;
 }
 
-lldb::SBSymbolContextList
-SBCompileUnit::ResolveSymbolContext(const char *file, uint32_t line,
-                                    std::optional<uint16_t> column) const {
-  LLDB_INSTRUMENT_VA(this, file, line);
-
-  lldb::SBSymbolContextList sc_list;
-  FileSpec file_spec(file);
-  SourceLocationSpec location_spec(file_spec, line, /*column=*/column,
-                                   /*check_inlines=*/true,
-                                   /*exact_match=*/true);
-
-  if (m_opaque_ptr)
-    m_opaque_ptr->ResolveSymbolContext(location_spec, eSymbolContextLineEntry,
-                                       *sc_list);
-  return sc_list;
-}
-
 uint32_t SBCompileUnit::GetNumLineEntries() const {
   LLDB_INSTRUMENT_VA(this);
 
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 1331bf2d03d20f..e27c078d14979d 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -3265,7 +3265,8 @@ void request_stepInTargets(const llvm::json::Object &request) {
   if (frame.IsValid()) {
     lldb::SBAddress pc_addr = frame.GetPCAddress();
     lldb::addr_t line_end_addr = pc_addr.GetLineEntry()
-                                     .GetSameLineContiguousAddressRangeEnd(true)
+                                     .GetSameLineContiguousAddressRangeEnd(
+                                         /*include_inlined_functions=*/true)
                                      .GetLoadAddress(g_dap.target);
 
     int max_inst_count = line_end_addr - pc_addr.GetLoadAddress(g_dap.target);

>From f7f8a69122506469687c42a5b9bfe020c150e34d Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at fb.com>
Date: Tue, 26 Mar 2024 09:20:27 -0700
Subject: [PATCH 4/7] Fix formatter

---
 .../lldbsuite/test/tools/lldb-dap/dap_server.py    |  6 +++++-
 .../test/API/tools/lldb-dap/stepInTargets/main.cpp | 14 ++++----------
 lldb/tools/lldb-dap/lldb-dap.cpp                   |  2 +-
 3 files changed, 10 insertions(+), 12 deletions(-)

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 f2ee6bd8212415..45e48a10c7611d 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
@@ -815,7 +815,11 @@ def request_stepInTargets(self, frameId):
         if self.exit_status is not None:
             raise ValueError("request_stepInTargets called after process exited")
         args_dict = {"frameId": frameId}
-        command_dict = {"command": "stepInTargets", "type": "request", "arguments": args_dict}
+        command_dict = {
+            "command": "stepInTargets",
+            "type": "request",
+            "arguments": args_dict,
+        }
         return self.send_recv(command_dict)
     
     def request_stepIn(self, threadId, targetId):
diff --git a/lldb/test/API/tools/lldb-dap/stepInTargets/main.cpp b/lldb/test/API/tools/lldb-dap/stepInTargets/main.cpp
index cd4419e8a4b047..d3c3dbcc139ef0 100644
--- a/lldb/test/API/tools/lldb-dap/stepInTargets/main.cpp
+++ b/lldb/test/API/tools/lldb-dap/stepInTargets/main.cpp
@@ -1,17 +1,11 @@
 
-int foo(int val, int extra) {
-  return val + extra;
-}
+int foo(int val, int extra) { return val + extra; }
 
-int bar() {
-  return 22;
-}
+int bar() { return 22; }
 
-int bar2() {
-  return 54;
-}
+int bar2() { return 54; }
 
-int main(int argc, char const *argv[]) { 
+int main(int argc, char const *argv[]) {
   foo(bar(), bar2()); // set breakpoint here
   return 0;
 }
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index e27c078d14979d..30cba6d23f542d 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -3186,7 +3186,7 @@ void request_stepIn(const llvm::json::Object &request) {
   auto it = g_dap.step_in_targets.find(target_id);
   if (it != g_dap.step_in_targets.end())
     step_in_target = it->second;
-  
+
   const bool single_thread = GetBoolean(arguments, "singleThread", false);
   lldb::RunMode run_mode =
       single_thread ? lldb::eOnlyThisThread : lldb::eOnlyDuringStepping;

>From 55a9f706251eea2be925980ead93cb1b170000a2 Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at fb.com>
Date: Tue, 26 Mar 2024 09:29:14 -0700
Subject: [PATCH 5/7] Remove empty space

---
 .../packages/Python/lldbsuite/test/tools/lldb-dap/dap_server.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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 45e48a10c7611d..78f7c22dd600ca 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
@@ -821,7 +821,7 @@ def request_stepInTargets(self, frameId):
             "arguments": args_dict,
         }
         return self.send_recv(command_dict)
-    
+
     def request_stepIn(self, threadId, targetId):
         if self.exit_status is not None:
             raise ValueError("request_stepIn called after process exited")

>From d51bf466ea9d9b24782a36ee89fb053fb1b874a1 Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at fb.com>
Date: Tue, 26 Mar 2024 15:44:52 -0700
Subject: [PATCH 6/7] Address review feedback

---
 lldb/include/lldb/API/SBTarget.h              |  4 ++++
 lldb/source/API/SBTarget.cpp                  | 24 +++++++++++++++++++
 .../stepInTargets/TestDAP_stepInTargets.py    |  4 +++-
 lldb/tools/lldb-dap/lldb-dap.cpp              | 16 ++++---------
 4 files changed, 36 insertions(+), 12 deletions(-)

diff --git a/lldb/include/lldb/API/SBTarget.h b/lldb/include/lldb/API/SBTarget.h
index 3644ac056da3dc..f707ebc658f7b0 100644
--- a/lldb/include/lldb/API/SBTarget.h
+++ b/lldb/include/lldb/API/SBTarget.h
@@ -879,6 +879,10 @@ class LLDB_API SBTarget {
                                            uint32_t count,
                                            const char *flavor_string);
 
+  lldb::SBInstructionList ReadInstructions(lldb::SBAddress start_addr,
+                                           lldb::SBAddress end_addr,
+                                           const char *flavor_string);
+
   lldb::SBInstructionList GetInstructions(lldb::SBAddress base_addr,
                                           const void *buf, size_t size);
 
diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index cc9f1fdd76afaa..b49ed19aeba990 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -2011,6 +2011,30 @@ lldb::SBInstructionList SBTarget::ReadInstructions(lldb::SBAddress base_addr,
   return sb_instructions;
 }
 
+lldb::SBInstructionList SBTarget::ReadInstructions(lldb::SBAddress start_addr,
+                                                   lldb::SBAddress end_addr,
+                                                   const char *flavor_string) {
+  LLDB_INSTRUMENT_VA(this, start_addr, end_addr, flavor_string);
+
+  SBInstructionList sb_instructions;
+
+  TargetSP target_sp(GetSP());
+  if (target_sp) {
+    lldb::addr_t start_load_addr = start_addr.GetLoadAddress(*this);
+    lldb::addr_t end_load_addr = end_addr.GetLoadAddress(*this);
+    if (end_load_addr > start_load_addr) {
+      lldb::addr_t size = end_load_addr - start_load_addr;
+
+      AddressRange range(start_load_addr, size);
+      const bool force_live_memory = true;
+      sb_instructions.SetDisassembler(Disassembler::DisassembleRange(
+          target_sp->GetArchitecture(), nullptr, flavor_string, *target_sp,
+          range, force_live_memory));
+    }
+  }
+  return sb_instructions;
+}
+
 lldb::SBInstructionList SBTarget::GetInstructions(lldb::SBAddress base_addr,
                                                   const void *buf,
                                                   size_t size) {
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 3fa955305037cf..6296f6554d07e5 100644
--- a/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py
+++ b/lldb/test/API/tools/lldb-dap/stepInTargets/TestDAP_stepInTargets.py
@@ -10,7 +10,9 @@
 
 
 class TestDAP_stepInTargets(lldbdap_testcase.DAPTestCaseBase):
-    @skipIf(archs=no_match(["x86_64"]))  # ARM flow kind is not supported yet.
+    @skipIf(
+        archs=no_match(["x86_64"])
+    )  # InstructionControlFlowKind for ARM is not supported yet.
     def test_basic(self):
         """
         Tests the basic stepping in targets with directly calls.
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 30cba6d23f542d..8fb009b5ecf3ec 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -3264,14 +3264,10 @@ void request_stepInTargets(const llvm::json::Object &request) {
   lldb::SBFrame frame = g_dap.GetLLDBFrame(*arguments);
   if (frame.IsValid()) {
     lldb::SBAddress pc_addr = frame.GetPCAddress();
-    lldb::addr_t line_end_addr = pc_addr.GetLineEntry()
-                                     .GetSameLineContiguousAddressRangeEnd(
-                                         /*include_inlined_functions=*/true)
-                                     .GetLoadAddress(g_dap.target);
-
-    int max_inst_count = line_end_addr - pc_addr.GetLoadAddress(g_dap.target);
-    lldb::SBInstructionList insts =
-        g_dap.target.ReadInstructions(pc_addr, max_inst_count);
+    lldb::SBAddress line_end_addr =
+        pc_addr.GetLineEntry().GetSameLineContiguousAddressRangeEnd(true);
+    lldb::SBInstructionList insts = g_dap.target.ReadInstructions(
+        pc_addr, line_end_addr, /*flavor_string=*/nullptr);
 
     if (!insts.IsValid()) {
       response["success"] = false;
@@ -3288,9 +3284,6 @@ void request_stepInTargets(const llvm::json::Object &request) {
         break;
 
       lldb::addr_t inst_addr = inst.GetAddress().GetLoadAddress(g_dap.target);
-      // line_end_addr is exclusive of the line range.
-      if (inst_addr >= line_end_addr)
-        break;
 
       // Note: currently only x86/x64 supports flow kind.
       lldb::InstructionControlFlowKind flow_kind =
@@ -3310,6 +3303,7 @@ void request_stepInTargets(const llvm::json::Object &request) {
         if (!call_target_load_addr.IsValid())
           continue;
 
+        // Step in targets only supports functions with debug info.
         lldb::SBSymbolContext sc = g_dap.target.ResolveSymbolContextForAddress(
             call_target_load_addr, lldb::eSymbolContextFunction);
 

>From 13ffb1cb3acec85969fa2df1684142715af66267 Mon Sep 17 00:00:00 2001
From: jeffreytan81 <jeffreytan at fb.com>
Date: Tue, 2 Apr 2024 08:18:09 -0700
Subject: [PATCH 7/7] Address review comments

---
 .../lldbsuite/test/tools/lldb-dap/dap_server.py    | 14 +++++++-------
 lldb/tools/lldb-dap/lldb-dap.cpp                   |  6 ++++--
 2 files changed, 11 insertions(+), 9 deletions(-)

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 78f7c22dd600ca..5838281bcb1a10 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
@@ -811,6 +811,13 @@ def request_next(self, threadId):
         command_dict = {"command": "next", "type": "request", "arguments": args_dict}
         return self.send_recv(command_dict)
 
+    def request_stepIn(self, threadId, targetId):
+        if self.exit_status is not None:
+            raise ValueError("request_stepIn called after process exited")
+        args_dict = {"threadId": threadId, "targetId": targetId}
+        command_dict = {"command": "stepIn", "type": "request", "arguments": args_dict}
+        return self.send_recv(command_dict)
+
     def request_stepInTargets(self, frameId):
         if self.exit_status is not None:
             raise ValueError("request_stepInTargets called after process exited")
@@ -822,13 +829,6 @@ def request_stepInTargets(self, frameId):
         }
         return self.send_recv(command_dict)
 
-    def request_stepIn(self, threadId, targetId):
-        if self.exit_status is not None:
-            raise ValueError("request_stepIn called after process exited")
-        args_dict = {"threadId": threadId, "targetId": targetId}
-        command_dict = {"command": "stepIn", "type": "request", "arguments": args_dict}
-        return self.send_recv(command_dict)
-
     def request_stepOut(self, threadId):
         if self.exit_status is not None:
             raise ValueError("request_stepOut called after process exited")
diff --git a/lldb/tools/lldb-dap/lldb-dap.cpp b/lldb/tools/lldb-dap/lldb-dap.cpp
index 8fb009b5ecf3ec..e129bd5c7e7679 100644
--- a/lldb/tools/lldb-dap/lldb-dap.cpp
+++ b/lldb/tools/lldb-dap/lldb-dap.cpp
@@ -3303,11 +3303,13 @@ void request_stepInTargets(const llvm::json::Object &request) {
         if (!call_target_load_addr.IsValid())
           continue;
 
-        // Step in targets only supports functions with debug info.
+        // The existing ThreadPlanStepInRange only accept step in target
+        // function with debug info.
         lldb::SBSymbolContext sc = g_dap.target.ResolveSymbolContextForAddress(
             call_target_load_addr, lldb::eSymbolContextFunction);
 
-        // Step in targets only supports functions with debug info.
+        // 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();



More information about the lldb-commits mailing list