[Lldb-commits] [lldb] More refinement of call site handling in stepping. (PR #114628)
via lldb-commits
lldb-commits at lists.llvm.org
Mon Nov 4 17:06:20 PST 2024
https://github.com/jimingham updated https://github.com/llvm/llvm-project/pull/114628
>From 042ac07ed67a5465aaf5c2dc8c4396adf5da2948 Mon Sep 17 00:00:00 2001
From: Jim Ingham <jingham at apple.com>
Date: Fri, 1 Nov 2024 17:23:12 -0700
Subject: [PATCH 1/4] More refinement of call site handling in stepping.
When you set a "next branch breakpoint" and run to it while
stepping, you have to claim the stop at that breakpoint to be
the top of the inlined call stack, or you will seem to "step in"
and then plans might try to step back out again.
This records the PrefferedLineEntry for next branch breakpoints and
adds a test to make sure this works.
---
lldb/source/Target/ThreadPlanStepRange.cpp | 42 +++++++++++++++++--
.../inline-stepping/TestInlineStepping.py | 20 ++++++++-
2 files changed, 58 insertions(+), 4 deletions(-)
diff --git a/lldb/source/Target/ThreadPlanStepRange.cpp b/lldb/source/Target/ThreadPlanStepRange.cpp
index 3c825058b8c375..e53b189d49be44 100644
--- a/lldb/source/Target/ThreadPlanStepRange.cpp
+++ b/lldb/source/Target/ThreadPlanStepRange.cpp
@@ -379,10 +379,9 @@ bool ThreadPlanStepRange::SetNextBranchBreakpoint() {
!m_next_branch_bp_sp->HasResolvedLocations())
m_could_not_resolve_hw_bp = true;
+ BreakpointLocationSP bp_loc = m_next_branch_bp_sp->GetLocationAtIndex(0);
if (log) {
lldb::break_id_t bp_site_id = LLDB_INVALID_BREAK_ID;
- BreakpointLocationSP bp_loc =
- m_next_branch_bp_sp->GetLocationAtIndex(0);
if (bp_loc) {
BreakpointSiteSP bp_site = bp_loc->GetBreakpointSite();
if (bp_site) {
@@ -395,7 +394,44 @@ bool ThreadPlanStepRange::SetNextBranchBreakpoint() {
m_next_branch_bp_sp->GetID(), bp_site_id,
run_to_address.GetLoadAddress(&m_process.GetTarget()));
}
-
+ // The "next branch breakpoint might land on an virtual inlined call
+ // stack. If that's true, we should always stop at the top of the
+ // inlined call stack. Only virtual steps should walk deeper into the
+ // inlined call stack.
+ Block *block = run_to_address.CalculateSymbolContextBlock();
+ if (bp_loc && block) {
+ LineEntry top_most_line_entry;
+ lldb::addr_t run_to_addr = run_to_address.GetFileAddress();
+ for (Block *inlined_parent = block->GetContainingInlinedBlock(); inlined_parent;
+ inlined_parent = inlined_parent->GetInlinedParent()) {
+ AddressRange range;
+ if (!inlined_parent->GetRangeContainingAddress(run_to_address, range))
+ break;
+ Address range_start_address = range.GetBaseAddress();
+ // Only compare addresses here, we may have different symbol
+ // contexts (for virtual inlined stacks), but we just want to know
+ // that they are all at the same address.
+ if (range_start_address.GetFileAddress() != run_to_addr)
+ break;
+ const InlineFunctionInfo *inline_info = inlined_parent->GetInlinedFunctionInfo();
+ if (!inline_info)
+ break;
+ const Declaration &call_site = inline_info->GetCallSite();
+ top_most_line_entry.line = call_site.GetLine();
+ top_most_line_entry.column = call_site.GetColumn();
+ FileSpec call_site_file_spec = call_site.GetFile();
+ top_most_line_entry.original_file_sp.reset(new SupportFile(call_site_file_spec));
+ top_most_line_entry.range = range;
+ top_most_line_entry.file_sp.reset();
+ top_most_line_entry.ApplyFileMappings(GetThread().CalculateTarget());
+ if (!top_most_line_entry.file_sp)
+ top_most_line_entry.file_sp = top_most_line_entry.original_file_sp;
+ }
+ if (top_most_line_entry.IsValid()) {
+ LLDB_LOG(log, "Setting preferred line entry: {0}:{1}", top_most_line_entry.GetFile(), top_most_line_entry.line);
+ bp_loc->SetPreferredLineEntry(top_most_line_entry);
+ }
+ }
m_next_branch_bp_sp->SetThreadID(m_tid);
m_next_branch_bp_sp->SetBreakpointKind("next-branch-location");
diff --git a/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py b/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
index 4e2d908e63b81c..b43bc71243f259 100644
--- a/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
+++ b/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
@@ -291,7 +291,7 @@ def inline_stepping_step_over(self):
break_1_in_main = target.BreakpointCreateBySourceRegex(
"// At second call of caller_ref_1 in main.", self.main_source_spec
)
- self.assertTrue(break_1_in_main, VALID_BREAKPOINT)
+ self.assertGreater(break_1_in_main.GetNumLocations(), 0, VALID_BREAKPOINT)
# Now launch the process, and do not stop at entry point.
self.process = target.LaunchSimple(
@@ -317,6 +317,24 @@ def inline_stepping_step_over(self):
]
self.run_step_sequence(step_sequence)
+ # Now make sure that next to a virtual inlined call stack
+ # gets the call stack depth correct.
+ break_2_in_main = target.BreakpointCreateBySourceRegex(
+ "// Call max_value specialized", self.main_source_spec
+ )
+ self.assertGreater(break_2_in_main.GetNumLocations(), 0, VALID_BREAKPOINT)
+ threads = lldbutil.continue_to_breakpoint(self.process, break_2_in_main)
+ self.assertEqual(len(threads), 1, "Hit our second breakpoint")
+ self.assertEqual(threads[0].id, self.thread.id, "Stopped at right thread")
+ self.thread.StepOver()
+ frame_0 = self.thread.frames[0]
+ line_entry = frame_0.line_entry
+ self.assertEqual(line_entry.file.basename, self.main_source_spec.basename, "File matches")
+ target_line = line_number("calling.cpp", "// At caller_trivial_inline_1")
+ self.assertEqual(line_entry.line, target_line, "Lines match as well.")
+
+
+
def step_in_template(self):
"""Use Python APIs to test stepping in to templated functions."""
exe = self.getBuildArtifact("a.out")
>From db6badf11dc4cb4a026b93a39e4671ff29e8fdc9 Mon Sep 17 00:00:00 2001
From: Jim Ingham <jingham at apple.com>
Date: Mon, 4 Nov 2024 17:04:10 -0800
Subject: [PATCH 2/4] clang-format
---
lldb/source/Target/ThreadPlanStepRange.cpp | 30 ++++++++++++++--------
1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/lldb/source/Target/ThreadPlanStepRange.cpp b/lldb/source/Target/ThreadPlanStepRange.cpp
index e53b189d49be44..6a0d8257fec271 100644
--- a/lldb/source/Target/ThreadPlanStepRange.cpp
+++ b/lldb/source/Target/ThreadPlanStepRange.cpp
@@ -379,7 +379,8 @@ bool ThreadPlanStepRange::SetNextBranchBreakpoint() {
!m_next_branch_bp_sp->HasResolvedLocations())
m_could_not_resolve_hw_bp = true;
- BreakpointLocationSP bp_loc = m_next_branch_bp_sp->GetLocationAtIndex(0);
+ BreakpointLocationSP bp_loc =
+ m_next_branch_bp_sp->GetLocationAtIndex(0);
if (log) {
lldb::break_id_t bp_site_id = LLDB_INVALID_BREAK_ID;
if (bp_loc) {
@@ -402,33 +403,40 @@ bool ThreadPlanStepRange::SetNextBranchBreakpoint() {
if (bp_loc && block) {
LineEntry top_most_line_entry;
lldb::addr_t run_to_addr = run_to_address.GetFileAddress();
- for (Block *inlined_parent = block->GetContainingInlinedBlock(); inlined_parent;
- inlined_parent = inlined_parent->GetInlinedParent()) {
+ for (Block *inlined_parent = block->GetContainingInlinedBlock();
+ inlined_parent;
+ inlined_parent = inlined_parent->GetInlinedParent()) {
AddressRange range;
- if (!inlined_parent->GetRangeContainingAddress(run_to_address, range))
- break;
+ if (!inlined_parent->GetRangeContainingAddress(run_to_address,
+ range))
+ break;
Address range_start_address = range.GetBaseAddress();
// Only compare addresses here, we may have different symbol
// contexts (for virtual inlined stacks), but we just want to know
// that they are all at the same address.
- if (range_start_address.GetFileAddress() != run_to_addr)
+ if (range_start_address.GetFileAddress() != run_to_addr)
break;
- const InlineFunctionInfo *inline_info = inlined_parent->GetInlinedFunctionInfo();
+ const InlineFunctionInfo *inline_info =
+ inlined_parent->GetInlinedFunctionInfo();
if (!inline_info)
break;
const Declaration &call_site = inline_info->GetCallSite();
top_most_line_entry.line = call_site.GetLine();
top_most_line_entry.column = call_site.GetColumn();
FileSpec call_site_file_spec = call_site.GetFile();
- top_most_line_entry.original_file_sp.reset(new SupportFile(call_site_file_spec));
+ top_most_line_entry.original_file_sp.reset(
+ new SupportFile(call_site_file_spec));
top_most_line_entry.range = range;
top_most_line_entry.file_sp.reset();
- top_most_line_entry.ApplyFileMappings(GetThread().CalculateTarget());
+ top_most_line_entry.ApplyFileMappings(
+ GetThread().CalculateTarget());
if (!top_most_line_entry.file_sp)
- top_most_line_entry.file_sp = top_most_line_entry.original_file_sp;
+ top_most_line_entry.file_sp =
+ top_most_line_entry.original_file_sp;
}
if (top_most_line_entry.IsValid()) {
- LLDB_LOG(log, "Setting preferred line entry: {0}:{1}", top_most_line_entry.GetFile(), top_most_line_entry.line);
+ LLDB_LOG(log, "Setting preferred line entry: {0}:{1}",
+ top_most_line_entry.GetFile(), top_most_line_entry.line);
bp_loc->SetPreferredLineEntry(top_most_line_entry);
}
}
>From 830fe44742710c01ffa07174b3be12d0b766a2f8 Mon Sep 17 00:00:00 2001
From: Jim Ingham <jingham at apple.com>
Date: Mon, 4 Nov 2024 17:05:00 -0800
Subject: [PATCH 3/4] uglification
---
.../functionalities/inline-stepping/TestInlineStepping.py | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py b/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
index b43bc71243f259..a8de6b7e7a5e05 100644
--- a/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
+++ b/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
@@ -329,11 +329,11 @@ def inline_stepping_step_over(self):
self.thread.StepOver()
frame_0 = self.thread.frames[0]
line_entry = frame_0.line_entry
- self.assertEqual(line_entry.file.basename, self.main_source_spec.basename, "File matches")
+ self.assertEqual(
+ line_entry.file.basename, self.main_source_spec.basename, "File matches"
+ )
target_line = line_number("calling.cpp", "// At caller_trivial_inline_1")
self.assertEqual(line_entry.line, target_line, "Lines match as well.")
-
-
def step_in_template(self):
"""Use Python APIs to test stepping in to templated functions."""
>From 5f9fe734c1d74c5c70704e3af104bdd24d8c7afa Mon Sep 17 00:00:00 2001
From: Jim Ingham <jingham at apple.com>
Date: Mon, 4 Nov 2024 17:05:55 -0800
Subject: [PATCH 4/4] Address review comments
---
lldb/source/Target/ThreadPlanStepRange.cpp | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lldb/source/Target/ThreadPlanStepRange.cpp b/lldb/source/Target/ThreadPlanStepRange.cpp
index 6a0d8257fec271..b64ce6f0dc9c28 100644
--- a/lldb/source/Target/ThreadPlanStepRange.cpp
+++ b/lldb/source/Target/ThreadPlanStepRange.cpp
@@ -395,7 +395,7 @@ bool ThreadPlanStepRange::SetNextBranchBreakpoint() {
m_next_branch_bp_sp->GetID(), bp_site_id,
run_to_address.GetLoadAddress(&m_process.GetTarget()));
}
- // The "next branch breakpoint might land on an virtual inlined call
+ // The "next branch breakpoint might land on a virtual inlined call
// stack. If that's true, we should always stop at the top of the
// inlined call stack. Only virtual steps should walk deeper into the
// inlined call stack.
More information about the lldb-commits
mailing list