[Lldb-commits] [lldb] More refinement of call site handling in stepping. (PR #114628)

[via lldb-commits]

https://github.com/jimingham created https://github.com/llvm/llvm-project/pull/114628

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.

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