Skip to content

Commit c5f47c6

Browse files
authored
[lldb-dap] Prevent using an implicit step-in. (#143644)
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 `line_entry`
1 parent 8763ac3 commit c5f47c6

File tree

7 files changed

+88
-7
lines changed

7 files changed

+88
-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: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,3 +83,41 @@ 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_inlined_function(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+
breakpoint_lines = [line_number(source, "// breakpoint 2")]
95+
step_over_pos = line_number(source, "// position_after_step_over")
96+
breakpoint_ids = self.set_source_breakpoints(source, breakpoint_lines)
97+
self.assertEqual(
98+
len(breakpoint_ids),
99+
len(breakpoint_lines),
100+
"expect correct number of breakpoints.",
101+
)
102+
self.continue_to_breakpoints(breakpoint_ids)
103+
104+
thread_id = self.dap_server.get_thread_id()
105+
self.stepOver(thread_id)
106+
levels = 1
107+
frames = self.get_stackFrames(thread_id, 0, levels)
108+
self.assertEqual(len(frames), levels, "expect current number of frame levels.")
109+
top_frame = frames[0]
110+
self.assertEqual(
111+
top_frame["source"]["name"], source, "expect we are in the same file."
112+
)
113+
self.assertTrue(
114+
top_frame["source"]["path"].endswith(source),
115+
f"expect path ending with '{source}'.",
116+
)
117+
self.assertEqual(
118+
top_frame["line"],
119+
step_over_pos,
120+
f"expect step_over on line {step_over_pos}",
121+
)
122+
123+
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)