Skip to content

[lldb-dap] Prevent using an implicit step-in. #143644

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
37 changes: 37 additions & 0 deletions lldb/test/API/tools/lldb-dap/step/TestDAP_step.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
14 changes: 13 additions & 1 deletion lldb/test/API/tools/lldb-dap/step/main.cpp
Original file line number Diff line number Diff line change
@@ -1,8 +1,20 @@
#include "other.h"

int function(int x) {
if ((x % 2) == 0)
return function(x - 1) + x; // breakpoint 1
else
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
}
7 changes: 7 additions & 0 deletions lldb/test/API/tools/lldb-dap/step/other.h
Original file line number Diff line number Diff line change
@@ -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
11 changes: 11 additions & 0 deletions lldb/tools/lldb-dap/DAP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
11 changes: 11 additions & 0 deletions lldb/tools/lldb-dap/DAP.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand Down
7 changes: 2 additions & 5 deletions lldb/tools/lldb-dap/JSONUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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();

Expand Down
Loading