From 24e8bbe12758773ef9ca55c3c4022a610db434e9 Mon Sep 17 00:00:00 2001 From: Ebuka Ezike 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..7320e83154f5b 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 + inlined_fn(); // 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..c71cc373fbdff --- /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 not_inlined_fn() {}; + +__attribute__((always_inline)) inline void inlined_fn() { not_inlined_fn(); } +#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 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 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 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 source = - dap.ResolveSource(frame.GetPCAddress()); + std::optional 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();