More refinement of call site handling in stepping.

[jimingham]

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.

[llvmbot]

@llvm/pr-subscribers-lldb

Author: None (jimingham)

Changes

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.

---

Full diff: https://github.com/llvm/llvm-project/pull/114628.diff

2 Files Affected:

• (modified) lldb/source/Target/ThreadPlanStepRange.cpp (+39-3)
• (modified) lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py (+19-1)

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")

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[adrian-prantl]

Suggested change

	// The "next branch breakpoint might land on an virtual inlined call
	// The "next branch breakpoint might land on a virtual inlined call

[DavidSpickett]

Intentionally or not, this also fixes TestInlineStepping.py test_with_python_api on Arm 32 bit.

[jimingham]

Intentionally or not, this also fixes TestInlineStepping.py test_with_python_api on Arm 32 bit.

That's plausible. I removed the skip along with this patch, let's see if that holds on the bots.