Skip to content

Commit 24e8bbe

Browse files
committed
[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`
1 parent cbf781f commit 24e8bbe

File tree

7 files changed

+87
-7
lines changed

7 files changed

+87
-7
lines changed

lldb/packages/Python/lldbsuite/test/tools/lldb-dap/lldbdap_testcase.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,12 @@ def stepOver(
344344
granularity="statement",
345345
timeout=DEFAULT_TIMEOUT,
346346
):
347-
self.dap_server.request_next(threadId=threadId, granularity=granularity)
347+
response = self.dap_server.request_next(
348+
threadId=threadId, granularity=granularity
349+
)
350+
self.assertTrue(
351+
response["success"], f"next request failed: response {response}"
352+
)
348353
if waitForStop:
349354
return self.dap_server.wait_for_stopped(timeout)
350355
return None

lldb/test/API/tools/lldb-dap/step/TestDAP_step.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,3 +83,40 @@ def test_step(self):
8383

8484
# only step one thread that is at the breakpoint and stop
8585
break
86+
87+
def test_step_over(self):
88+
"""
89+
Test stepping over when the program counter is in another file.
90+
"""
91+
program = self.getBuildArtifact("a.out")
92+
self.build_and_launch(program)
93+
source = "main.cpp"
94+
breakpoint1_line = line_number(source, "// breakpoint 2")
95+
step_over_pos = line_number(source, "// position_after_step_over")
96+
lines = [breakpoint1_line]
97+
breakpoint_ids = self.set_source_breakpoints(source, lines)
98+
self.assertEqual(
99+
len(breakpoint_ids), len(lines), "expect correct number of breakpoints."
100+
)
101+
self.continue_to_breakpoints(breakpoint_ids)
102+
103+
thread_id = self.dap_server.get_thread_id()
104+
self.stepOver(thread_id)
105+
levels = 1
106+
frames = self.get_stackFrames(thread_id, 0, levels)
107+
self.assertEqual(len(frames), levels, "expect current number of frame levels.")
108+
top_frame = frames[0]
109+
self.assertEqual(
110+
top_frame["source"]["name"], source, "expect we are in the same file."
111+
)
112+
self.assertTrue(
113+
top_frame["source"]["path"].endswith(source),
114+
f"expect path ending with '{source}'.",
115+
)
116+
self.assertEqual(
117+
top_frame["line"],
118+
step_over_pos,
119+
f"expect step_over on line {step_over_pos}",
120+
)
121+
122+
self.continue_to_exit()
Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,20 @@
1+
#include "other.h"
2+
13
int function(int x) {
24
if ((x % 2) == 0)
35
return function(x - 1) + x; // breakpoint 1
46
else
57
return x;
68
}
79

8-
int main(int argc, char const *argv[]) { return function(2); }
10+
int function2() {
11+
int volatile value = 3; // breakpoint 2
12+
inlined_fn(); // position_after_step_over
13+
14+
return value;
15+
}
16+
17+
int main(int argc, char const *argv[]) {
18+
int func_result = function2();
19+
return function(2) - func_result; // returns 0
20+
}
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
#ifndef OTHER_H
2+
#define OTHER_H
3+
4+
__attribute__((noinline)) void not_inlined_fn() {};
5+
6+
__attribute__((always_inline)) inline void inlined_fn() { not_inlined_fn(); }
7+
#endif // OTHER_H

lldb/tools/lldb-dap/DAP.cpp

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -623,6 +623,17 @@ ReplMode DAP::DetectReplMode(lldb::SBFrame frame, std::string &expression,
623623
llvm_unreachable("enum cases exhausted.");
624624
}
625625

626+
std::optional<protocol::Source> DAP::ResolveSource(const lldb::SBFrame &frame) {
627+
if (!frame.IsValid())
628+
return std::nullopt;
629+
630+
const lldb::SBAddress frame_pc = frame.GetPCAddress();
631+
if (DisplayAssemblySource(debugger, frame_pc))
632+
return ResolveAssemblySource(frame_pc);
633+
634+
return CreateSource(frame.GetLineEntry().GetFileSpec());
635+
}
636+
626637
std::optional<protocol::Source> DAP::ResolveSource(lldb::SBAddress address) {
627638
if (DisplayAssemblySource(debugger, address))
628639
return ResolveAssemblySource(address);

lldb/tools/lldb-dap/DAP.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,17 @@ struct DAP {
254254
ReplMode DetectReplMode(lldb::SBFrame frame, std::string &expression,
255255
bool partial_expression);
256256

257+
/// Create a `protocol::Source` object as described in the debug adapter
258+
/// definition.
259+
///
260+
/// \param[in] frame
261+
/// The frame to use when populating the "Source" object.
262+
///
263+
/// \return
264+
/// A `protocol::Source` object that follows the formal JSON
265+
/// definition outlined by Microsoft.
266+
std::optional<protocol::Source> ResolveSource(const lldb::SBFrame &frame);
267+
257268
/// Create a "Source" JSON object as described in the debug adapter
258269
/// definition.
259270
///

lldb/tools/lldb-dap/JSONUtils.cpp

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -572,9 +572,7 @@ llvm::json::Value CreateStackFrame(DAP &dap, lldb::SBFrame &frame,
572572

573573
EmplaceSafeString(object, "name", frame_name);
574574

575-
auto target = frame.GetThread().GetProcess().GetTarget();
576-
std::optional<protocol::Source> source =
577-
dap.ResolveSource(frame.GetPCAddress());
575+
std::optional<protocol::Source> source = dap.ResolveSource(frame);
578576

579577
if (source && !IsAssemblySource(*source)) {
580578
// This is a normal source with a valid line entry.
@@ -586,8 +584,7 @@ llvm::json::Value CreateStackFrame(DAP &dap, lldb::SBFrame &frame,
586584
// This is a source where the disassembly is used, but there is a valid
587585
// symbol. Calculate the line of the current PC from the start of the
588586
// current symbol.
589-
lldb::SBTarget target = frame.GetThread().GetProcess().GetTarget();
590-
lldb::SBInstructionList inst_list = target.ReadInstructions(
587+
lldb::SBInstructionList inst_list = dap.target.ReadInstructions(
591588
frame.GetSymbol().GetStartAddress(), frame.GetPCAddress(), nullptr);
592589
size_t inst_line = inst_list.GetSize();
593590

0 commit comments

Comments
 (0)