[Lldb-commits] [lldb] [lldb-dap] Prevent using an implicit `step-in`. (PR #143644)
Ebuka Ezike via lldb-commits
lldb-commits at lists.llvm.org
Sun Jun 29 04:34:58 PDT 2025
https://github.com/da-viper updated https://github.com/llvm/llvm-project/pull/143644
>From 6ff8149d35aa931c7f08373d92cf556ad0d9abc3 Mon Sep 17 00:00:00 2001
From: Ebuka Ezike <yerimyah1 at gmail.com>
Date: Wed, 11 Jun 2025 02:14:12 +0100
Subject: [PATCH] [lldb-dap] Prevent using an implicit `step-in`.
When there is a function that is inlined at the current program counter. If you get the current `line_entry` using the program counter's address it will point to the location of the inline function that may be in another file. (this is in implicit step-in and should not happen what step over is called).
Use the current frame to get the correct `line_entry`
---
.../test/tools/lldb-dap/lldbdap_testcase.py | 7 +++-
.../API/tools/lldb-dap/step/TestDAP_step.py | 37 +++++++++++++++++++
lldb/test/API/tools/lldb-dap/step/main.cpp | 14 ++++++-
lldb/test/API/tools/lldb-dap/step/other.h | 7 ++++
lldb/tools/lldb-dap/DAP.cpp | 11 ++++++
lldb/tools/lldb-dap/DAP.h | 11 ++++++
lldb/tools/lldb-dap/JSONUtils.cpp | 7 +---
7 files changed, 87 insertions(+), 7 deletions(-)
create mode 100644 lldb/test/API/tools/lldb-dap/step/other.h
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 3b54d598c3509..6299caf7631af 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
@@ -344,7 +344,12 @@ def stepOver(
granularity="statement",
timeout=DEFAULT_TIMEOUT,
):
- self.dap_server.request_next(threadId=threadId, granularity=granularity)
+ response = self.dap_server.request_next(
+ threadId=threadId, granularity=granularity
+ )
+ self.assertTrue(
+ response["success"], f"next request failed: response {response}"
+ )
if waitForStop:
return self.dap_server.wait_for_stopped(timeout)
return None
diff --git a/lldb/test/API/tools/lldb-dap/step/TestDAP_step.py b/lldb/test/API/tools/lldb-dap/step/TestDAP_step.py
index 42a39e3c8c080..5339e0bab1d5e 100644
--- a/lldb/test/API/tools/lldb-dap/step/TestDAP_step.py
+++ b/lldb/test/API/tools/lldb-dap/step/TestDAP_step.py
@@ -83,3 +83,40 @@ def test_step(self):
# only step one thread that is at the breakpoint and stop
break
+
+ def test_step_over(self):
+ """
+ Test stepping over when the program counter is in another file.
+ """
+ program = self.getBuildArtifact("a.out")
+ self.build_and_launch(program)
+ source = "main.cpp"
+ breakpoint1_line = line_number(source, "// breakpoint 2")
+ step_over_pos = line_number(source, "// position_after_step_over")
+ lines = [breakpoint1_line]
+ 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)
+
+ thread_id = self.dap_server.get_thread_id()
+ self.stepOver(thread_id)
+ levels = 1
+ frames = self.get_stackFrames(thread_id, 0, levels)
+ self.assertEqual(len(frames), levels, "expect current number of frame levels.")
+ top_frame = frames[0]
+ self.assertEqual(
+ top_frame["source"]["name"], source, "expect we are in the same file."
+ )
+ self.assertTrue(
+ top_frame["source"]["path"].endswith(source),
+ f"expect path ending with '{source}'.",
+ )
+ self.assertEqual(
+ top_frame["line"],
+ step_over_pos,
+ f"expect step_over on line {step_over_pos}",
+ )
+
+ self.continue_to_exit()
diff --git a/lldb/test/API/tools/lldb-dap/step/main.cpp b/lldb/test/API/tools/lldb-dap/step/main.cpp
index 8905beb5e7eff..9454fa7a7855a 100644
--- a/lldb/test/API/tools/lldb-dap/step/main.cpp
+++ b/lldb/test/API/tools/lldb-dap/step/main.cpp
@@ -1,3 +1,5 @@
+#include "other.h"
+
int function(int x) {
if ((x % 2) == 0)
return function(x - 1) + x; // breakpoint 1
@@ -5,4 +7,14 @@ int function(int x) {
return x;
}
-int main(int argc, char const *argv[]) { return function(2); }
+int function2() {
+ int volatile value = 3; // breakpoint 2
+ fn2(); // position_after_step_over
+
+ return value;
+}
+
+int main(int argc, char const *argv[]) {
+ int func_result = function2();
+ return function(2) - func_result; // returns 0
+}
diff --git a/lldb/test/API/tools/lldb-dap/step/other.h b/lldb/test/API/tools/lldb-dap/step/other.h
new file mode 100644
index 0000000000000..aad022bbc2615
--- /dev/null
+++ b/lldb/test/API/tools/lldb-dap/step/other.h
@@ -0,0 +1,7 @@
+#ifndef OTHER_H
+#define OTHER_H
+
+__attribute__((noinline)) void fn1() {};
+
+__attribute__((always_inline)) inline void fn2() { fn1(); }
+#endif // OTHER_H
\ No newline at end of file
diff --git a/lldb/tools/lldb-dap/DAP.cpp b/lldb/tools/lldb-dap/DAP.cpp
index cd97458bd4aa8..a26beed491e98 100644
--- a/lldb/tools/lldb-dap/DAP.cpp
+++ b/lldb/tools/lldb-dap/DAP.cpp
@@ -623,6 +623,17 @@ ReplMode DAP::DetectReplMode(lldb::SBFrame frame, std::string &expression,
llvm_unreachable("enum cases exhausted.");
}
+std::optional<protocol::Source> DAP::ResolveSource(const lldb::SBFrame &frame) {
+ if (!frame.IsValid())
+ return std::nullopt;
+
+ const lldb::SBAddress frame_pc = frame.GetPCAddress();
+ if (DisplayAssemblySource(debugger, frame_pc))
+ return ResolveAssemblySource(frame_pc);
+
+ return CreateSource(frame.GetLineEntry().GetFileSpec());
+}
+
std::optional<protocol::Source> DAP::ResolveSource(lldb::SBAddress address) {
if (DisplayAssemblySource(debugger, address))
return ResolveAssemblySource(address);
diff --git a/lldb/tools/lldb-dap/DAP.h b/lldb/tools/lldb-dap/DAP.h
index 0e9a9e0eb674c..af4aabaafaae8 100644
--- a/lldb/tools/lldb-dap/DAP.h
+++ b/lldb/tools/lldb-dap/DAP.h
@@ -254,6 +254,17 @@ struct DAP {
ReplMode DetectReplMode(lldb::SBFrame frame, std::string &expression,
bool partial_expression);
+ /// Create a `protocol::Source` object as described in the debug adapter
+ /// definition.
+ ///
+ /// \param[in] frame
+ /// The frame to use when populating the "Source" object.
+ ///
+ /// \return
+ /// A `protocol::Source` object that follows the formal JSON
+ /// definition outlined by Microsoft.
+ std::optional<protocol::Source> ResolveSource(const lldb::SBFrame &frame);
+
/// Create a "Source" JSON object as described in the debug adapter
/// definition.
///
diff --git a/lldb/tools/lldb-dap/JSONUtils.cpp b/lldb/tools/lldb-dap/JSONUtils.cpp
index 08e65ab835a57..e72d93ee34571 100644
--- a/lldb/tools/lldb-dap/JSONUtils.cpp
+++ b/lldb/tools/lldb-dap/JSONUtils.cpp
@@ -572,9 +572,7 @@ llvm::json::Value CreateStackFrame(DAP &dap, lldb::SBFrame &frame,
EmplaceSafeString(object, "name", frame_name);
- auto target = frame.GetThread().GetProcess().GetTarget();
- std::optional<protocol::Source> source =
- dap.ResolveSource(frame.GetPCAddress());
+ std::optional<protocol::Source> source = dap.ResolveSource(frame);
if (source && !IsAssemblySource(*source)) {
// This is a normal source with a valid line entry.
@@ -586,8 +584,7 @@ llvm::json::Value CreateStackFrame(DAP &dap, lldb::SBFrame &frame,
// This is a source where the disassembly is used, but there is a valid
// symbol. Calculate the line of the current PC from the start of the
// current symbol.
- lldb::SBTarget target = frame.GetThread().GetProcess().GetTarget();
- lldb::SBInstructionList inst_list = target.ReadInstructions(
+ lldb::SBInstructionList inst_list = dap.target.ReadInstructions(
frame.GetSymbol().GetStartAddress(), frame.GetPCAddress(), nullptr);
size_t inst_line = inst_list.GetSize();
More information about the lldb-commits
mailing list