[Lldb-commits] [lldb] Add the ability to break on call-site locations, improve inline stepping (PR #112939)

via lldb-commits lldb-commits at lists.llvm.org
Fri Oct 18 10:57:27 PDT 2024


https://github.com/jimingham updated https://github.com/llvm/llvm-project/pull/112939

>From 9c6705b21df14dc911665e1082c9b31ce00d7e7c Mon Sep 17 00:00:00 2001
From: Jim Ingham <jingham at apple.com>
Date: Thu, 3 Oct 2024 18:24:46 -0700
Subject: [PATCH 1/3] Add the ability to break on call-site locations, report
 the correct position in the virtual inlined call stack when they are hit, and
 step through the inlined stack from there.

---
 .../lldb/Breakpoint/BreakpointLocation.h      |  31 ++++
 lldb/include/lldb/Breakpoint/BreakpointSite.h |   5 +
 lldb/include/lldb/Target/StopInfo.h           |  11 ++
 .../lldb/Target/ThreadPlanStepInRange.h       |   4 +-
 lldb/source/Breakpoint/BreakpointLocation.cpp |  61 ++++++-
 lldb/source/Breakpoint/BreakpointResolver.cpp |  12 ++
 lldb/source/Breakpoint/BreakpointSite.cpp     |  16 ++
 lldb/source/Core/Declaration.cpp              |   2 +-
 lldb/source/Symbol/CompileUnit.cpp            | 104 ++++++++++-
 lldb/source/Target/StackFrameList.cpp         | 170 ++++++------------
 lldb/source/Target/StopInfo.cpp               |  55 ++++++
 lldb/source/Target/Thread.cpp                 |   8 +
 lldb/source/Target/ThreadPlanStepInRange.cpp  |  24 ++-
 .../source/Target/ThreadPlanStepOverRange.cpp |   2 +-
 .../inline-stepping/TestInlineStepping.py     |  54 ++++++
 .../inline-stepping/calling.cpp               |  31 ++++
 16 files changed, 462 insertions(+), 128 deletions(-)

diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocation.h b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
index cca00335bc3c67..f9c258daf137f7 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
@@ -11,10 +11,12 @@
 
 #include <memory>
 #include <mutex>
+#include <optional>
 
 #include "lldb/Breakpoint/BreakpointOptions.h"
 #include "lldb/Breakpoint/StoppointHitCounter.h"
 #include "lldb/Core/Address.h"
+#include "lldb/Symbol/LineEntry.h"
 #include "lldb/Utility/UserID.h"
 #include "lldb/lldb-private.h"
 
@@ -281,6 +283,18 @@ class BreakpointLocation
 
   /// Returns the breakpoint location ID.
   lldb::break_id_t GetID() const { return m_loc_id; }
+  
+  // Set the line entry that should be shown to users for this location.
+  // It is up to the caller to verify that this is a valid entry to show.
+  // The current use of this is to distinguish among line entries from a
+  // virtual inlined call stack that all share the same address.
+  void SetPreferredLineEntry(const LineEntry &line_entry) {
+    m_preferred_line_entry = line_entry;
+  }
+  
+  const std::optional<LineEntry> GetPreferredLineEntry() {
+    return m_preferred_line_entry;
+  }
 
 protected:
   friend class BreakpointSite;
@@ -305,6 +319,16 @@ class BreakpointLocation
   /// It also takes care of decrementing the ignore counters.
   /// If it returns false we should continue, otherwise stop.
   bool IgnoreCountShouldStop();
+  
+  // If this location knows that the virtual stack frame it represents is
+  // not frame 0, return the suggested stack frame instead.  This will happen
+  // when the location's address contains a "virtual inlined call stack" and the
+  // breakpoint was set on a file & line that are not at the bottom of that
+  // stack.  For now we key off the "preferred line entry" - looking for that
+  // in the blocks that start with the stop PC.
+  // This version of the API doesn't take an "inlined" parameter because it
+  // only changes frames in the inline stack.
+  std::optional<uint32_t> GetSuggestedStackFrameIndex();
 
 private:
   void SwapLocation(lldb::BreakpointLocationSP swap_from);
@@ -369,6 +393,13 @@ class BreakpointLocation
   lldb::break_id_t m_loc_id; ///< Breakpoint location ID.
   StoppointHitCounter m_hit_counter; ///< Number of times this breakpoint
                                      /// location has been hit.
+  std::optional<LineEntry> m_preferred_line_entry; // If this exists, use it to print the stop
+                                    // description rather than the LineEntry 
+                                    // m_address resolves to directly.  Use this
+                                    // for instance when the location was given
+                                    // somewhere in the virtual inlined call
+                                    // stack since the Address always resolves 
+                                    // to the lowest entry in the stack.
 
   void SetShouldResolveIndirectFunctions(bool do_resolve) {
     m_should_resolve_indirect_functions = do_resolve;
diff --git a/lldb/include/lldb/Breakpoint/BreakpointSite.h b/lldb/include/lldb/Breakpoint/BreakpointSite.h
index 17b76d51c1ae53..30cb5a80b908e0 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointSite.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointSite.h
@@ -169,6 +169,11 @@ class BreakpointSite : public std::enable_shared_from_this<BreakpointSite>,
   ///
   /// \see lldb::DescriptionLevel
   void GetDescription(Stream *s, lldb::DescriptionLevel level);
+  
+  // This runs through all the breakpoint locations owning this site and returns
+  // the greatest of their suggested stack frame indexes.  This only handles
+  // inlined stack changes.
+  std::optional<uint32_t> GetSuggestedStackFrameIndex();
 
   /// Tell whether a breakpoint has a location at this site.
   ///
diff --git a/lldb/include/lldb/Target/StopInfo.h b/lldb/include/lldb/Target/StopInfo.h
index fae90364deaf0a..d997e0fd6fc550 100644
--- a/lldb/include/lldb/Target/StopInfo.h
+++ b/lldb/include/lldb/Target/StopInfo.h
@@ -77,6 +77,17 @@ class StopInfo : public std::enable_shared_from_this<StopInfo> {
       m_description.clear();
   }
 
+  /// This gives the StopInfo a chance to suggest a stack frame to select.
+  /// Passing true for inlined_stack will request changes to the inlined
+  /// call stack.  Passing false will request changes to the real stack 
+  /// frame.  The inlined stack gets adjusted before we call into the thread
+  /// plans so they can reason based on the correct values.  The real stack
+  /// adjustment is handled after the frame recognizers get a chance to adjust
+  /// the frame.
+  virtual std::optional<uint32_t> GetSuggestedStackFrameIndex(bool inlined_stack) { 
+     return {};
+  }
+
   virtual bool IsValidForOperatingSystemThread(Thread &thread) { return true; }
 
   /// A Continue operation can result in a false stop event
diff --git a/lldb/include/lldb/Target/ThreadPlanStepInRange.h b/lldb/include/lldb/Target/ThreadPlanStepInRange.h
index f9ef87942a7c03..6bd79e1d287d35 100644
--- a/lldb/include/lldb/Target/ThreadPlanStepInRange.h
+++ b/lldb/include/lldb/Target/ThreadPlanStepInRange.h
@@ -80,8 +80,8 @@ class ThreadPlanStepInRange : public ThreadPlanStepRange,
   bool m_step_past_prologue; // FIXME: For now hard-coded to true, we could put
                              // a switch in for this if there's
                              // demand for that.
-  bool m_virtual_step; // true if we've just done a "virtual step", i.e. just
-                       // moved the inline stack depth.
+  LazyBool m_virtual_step; // true if we've just done a "virtual step", i.e. just
+                           // moved the inline stack depth.
   ConstString m_step_into_target;
   ThreadPlanStepInRange(const ThreadPlanStepInRange &) = delete;
   const ThreadPlanStepInRange &
diff --git a/lldb/source/Breakpoint/BreakpointLocation.cpp b/lldb/source/Breakpoint/BreakpointLocation.cpp
index 35058a713aef86..61fe467987228b 100644
--- a/lldb/source/Breakpoint/BreakpointLocation.cpp
+++ b/lldb/source/Breakpoint/BreakpointLocation.cpp
@@ -508,8 +508,19 @@ void BreakpointLocation::GetDescription(Stream *s,
         s->PutCString("re-exported target = ");
       else
         s->PutCString("where = ");
+        
+      // If there's a preferred line entry for printing, use that.
+      bool show_function_info = true;
+      if (GetPreferredLineEntry()) {
+        sc.line_entry = *GetPreferredLineEntry();
+        // FIXME: We're going to get the function name wrong when the preferred
+        // line entry is not the lowest one.  For now, just leave the function
+        // out in this case, but we really should also figure out how to easily
+        // fake the function name here.
+        show_function_info = false;
+      }
       sc.DumpStopContext(s, m_owner.GetTarget().GetProcessSP().get(), m_address,
-                         false, true, false, true, true, true);
+                         false, true, false, show_function_info, show_function_info, show_function_info);
     } else {
       if (sc.module_sp) {
         s->EOL();
@@ -537,7 +548,10 @@ void BreakpointLocation::GetDescription(Stream *s,
         if (sc.line_entry.line > 0) {
           s->EOL();
           s->Indent("location = ");
-          sc.line_entry.DumpStopContext(s, true);
+          if (GetPreferredLineEntry())
+            GetPreferredLineEntry()->DumpStopContext(s, true);
+          else
+            sc.line_entry.DumpStopContext(s, true);
         }
 
       } else {
@@ -656,6 +670,49 @@ void BreakpointLocation::SendBreakpointLocationChangedEvent(
   }
 }
 
+std::optional<uint32_t>  BreakpointLocation::GetSuggestedStackFrameIndex() {
+  if (!GetPreferredLineEntry())
+    return {};
+  LineEntry preferred = *GetPreferredLineEntry();
+  SymbolContext sc;
+  if (!m_address.CalculateSymbolContext(&sc))
+    return {};
+  // Don't return anything special if frame 0 is the preferred line entry.
+  // We not really telling the stack frame list to do anything special in that
+  // case.
+  if (!LineEntry::Compare(sc.line_entry, preferred))
+    return {};
+  
+  if (!sc.block)
+    return {};
+
+  // Blocks have their line info in Declaration form, so make one here:
+  Declaration preferred_decl(preferred.GetFile(), preferred.line, preferred.column);
+
+  uint32_t depth = 0;
+  Block *inlined_block = sc.block->GetContainingInlinedBlock();
+  while (inlined_block) {
+    // If we've moved to a block that this isn't the start of, that's not
+    // our inlining info or call site, so we can stop here.
+    Address start_address;
+    if (!inlined_block->GetStartAddress(start_address) 
+        || start_address != m_address)
+      return {};
+
+    const InlineFunctionInfo *info = inlined_block->GetInlinedFunctionInfo();
+    if (info) {
+      if (preferred_decl == info->GetDeclaration())
+        return depth;
+      if (preferred_decl == info->GetCallSite())
+        return depth + 1;
+    }
+    inlined_block = inlined_block->GetInlinedParent();
+    depth++;
+  }
+  return {};
+  
+}
+
 void BreakpointLocation::SwapLocation(BreakpointLocationSP swap_from) {
   m_address = swap_from->m_address;
   m_should_resolve_indirect_functions =
diff --git a/lldb/source/Breakpoint/BreakpointResolver.cpp b/lldb/source/Breakpoint/BreakpointResolver.cpp
index 8307689c7640cf..465fc387aa43e5 100644
--- a/lldb/source/Breakpoint/BreakpointResolver.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolver.cpp
@@ -340,6 +340,18 @@ void BreakpointResolver::AddLocation(SearchFilter &filter,
   }
 
   BreakpointLocationSP bp_loc_sp(AddLocation(line_start));
+  // If the address that we resolved the location to returns a different 
+  // LineEntry from the one in the incoming SC, we're probably dealing with an
+  // inlined call site, so set that as the preferred LineEntry:
+  LineEntry resolved_entry;
+  if (!skipped_prologue && bp_loc_sp && 
+      line_start.CalculateSymbolContextLineEntry(resolved_entry) &&
+      LineEntry::Compare(resolved_entry, sc.line_entry)) {
+      // FIXME: The function name will also be wrong here.  Do we need to record
+      // that as well, or can we figure that out again when we report this
+      // breakpoint location.
+      bp_loc_sp->SetPreferredLineEntry(sc.line_entry);
+  }
   if (log && bp_loc_sp && !GetBreakpoint()->IsInternal()) {
     StreamString s;
     bp_loc_sp->GetDescription(&s, lldb::eDescriptionLevelVerbose);
diff --git a/lldb/source/Breakpoint/BreakpointSite.cpp b/lldb/source/Breakpoint/BreakpointSite.cpp
index 3ca93f908e30b8..b283952d028305 100644
--- a/lldb/source/Breakpoint/BreakpointSite.cpp
+++ b/lldb/source/Breakpoint/BreakpointSite.cpp
@@ -87,6 +87,22 @@ void BreakpointSite::GetDescription(Stream *s, lldb::DescriptionLevel level) {
   m_constituents.GetDescription(s, level);
 }
 
+std::optional<uint32_t> BreakpointSite::GetSuggestedStackFrameIndex() {
+
+  std::optional<uint32_t> result;
+  std::lock_guard<std::recursive_mutex> guard(m_constituents_mutex);
+  for (BreakpointLocationSP loc_sp : m_constituents.BreakpointLocations()) {
+    std::optional<uint32_t> this_result = loc_sp->GetSuggestedStackFrameIndex();
+    if (this_result) {
+      if (!result)
+        result = this_result;
+      else
+        result = std::max(*this_result, *result);
+    }
+  }
+  return result;
+}
+
 bool BreakpointSite::IsInternal() const { return m_constituents.IsInternal(); }
 
 uint8_t *BreakpointSite::GetTrapOpcodeBytes() { return &m_trap_opcode[0]; }
diff --git a/lldb/source/Core/Declaration.cpp b/lldb/source/Core/Declaration.cpp
index 579a3999d14ea0..e7855f3e364376 100644
--- a/lldb/source/Core/Declaration.cpp
+++ b/lldb/source/Core/Declaration.cpp
@@ -71,7 +71,7 @@ int Declaration::Compare(const Declaration &a, const Declaration &b) {
 }
 
 bool Declaration::FileAndLineEqual(const Declaration &declaration) const {
-  int file_compare = FileSpec::Compare(this->m_file, declaration.m_file, true);
+  int file_compare = FileSpec::Compare(this->m_file, declaration.m_file, false);
   return file_compare == 0 && this->m_line == declaration.m_line;
 }
 
diff --git a/lldb/source/Symbol/CompileUnit.cpp b/lldb/source/Symbol/CompileUnit.cpp
index db8f8ce6bcbc92..f4c61d6ec1b196 100644
--- a/lldb/source/Symbol/CompileUnit.cpp
+++ b/lldb/source/Symbol/CompileUnit.cpp
@@ -251,7 +251,8 @@ void CompileUnit::ResolveSymbolContext(
     SymbolContextItem resolve_scope, SymbolContextList &sc_list,
     RealpathPrefixes *realpath_prefixes) {
   const FileSpec file_spec = src_location_spec.GetFileSpec();
-  const uint32_t line = src_location_spec.GetLine().value_or(0);
+  const uint32_t line = src_location_spec.GetLine().value_or(LLDB_INVALID_LINE_NUMBER);
+  const uint32_t column_num = src_location_spec.GetColumn().value_or(LLDB_INVALID_COLUMN_NUMBER);
   const bool check_inlines = src_location_spec.GetCheckInlines();
 
   // First find all of the file indexes that match our "file_spec". If
@@ -311,6 +312,107 @@ void CompileUnit::ResolveSymbolContext(
     line_idx = line_table->FindLineEntryIndexByFileIndex(
         0, file_indexes, src_location_spec, &line_entry);
   }
+  
+  // If we didn't manage to find a breakpoint that matched the line number
+  // requested, that might be because it is only an inline call site, and 
+  // doesn't have a line entry in the line table.  Scan for that here.
+  //
+  // We are making the assumption that if there was an inlined function it will
+  // contribute at least 1 non-call-site entry to the line table.  That's handy 
+  // because we don't move line breakpoints over function boundaries, so if we 
+  // found a hit, and there were also a call site entry, it would have to be in
+  // the function containing the PC of the line table match.  That way we can
+  // limit the call site search to that function.
+  // We will miss functions that ONLY exist as a call site entry.
+
+  if (line_entry.IsValid() && (line_entry.line != line || line_entry.column != column_num)                                
+      && resolve_scope & eSymbolContextLineEntry && check_inlines) {
+    // We don't move lines over function boundaries, so the address in the
+    // line entry will be the in function that contained the line that might
+    // be a CallSite, and we can just iterate over that function to find any
+    // inline records, and dig up their call sites.
+    Address start_addr = line_entry.range.GetBaseAddress();
+    Function *function = start_addr.CalculateSymbolContextFunction();
+    
+    Declaration sought_decl(file_spec, line, column_num);
+    // We use this recursive function to descend the block structure looking
+    // for a block that has this Declaration as in it's CallSite info.
+    // This function recursively scans the sibling blocks of the incoming
+    // block parameter.
+    std::function<void(Block&)> examine_block = 
+        [&sought_decl, &sc_list, &src_location_spec, resolve_scope, &examine_block] (Block &block) -> void {
+      // Iterate over the sibling child blocks of the incoming block.
+      Block *sibling_block = block.GetFirstChild();
+      while (sibling_block) {
+        // We only have to descend through the regular blocks, looking for
+        // immediate inlines, since those are the only ones that will have this
+        // callsite.
+        const InlineFunctionInfo *inline_info = sibling_block->GetInlinedFunctionInfo();
+        if (inline_info) {
+          // If this is the call-site we are looking for, record that:
+          // We need to be careful because the call site from the debug info
+          // will generally have a column, but the user might not have specified
+          // it.
+          Declaration found_decl = inline_info->GetCallSite();
+          uint32_t sought_column = sought_decl.GetColumn();
+          if (found_decl.FileAndLineEqual(sought_decl)
+              && (sought_column == LLDB_INVALID_COLUMN_NUMBER 
+                  || sought_column == found_decl.GetColumn())) {
+            // If we found a call site, it belongs not in this inlined block,
+            // but in the parent block that inlined it.
+            Address parent_start_addr;
+            if (sibling_block->GetParent()->GetStartAddress(parent_start_addr)) {
+              SymbolContext sc;
+              parent_start_addr.CalculateSymbolContext(&sc, resolve_scope);
+              // Now swap out the line entry for the one we found.
+              LineEntry call_site_line = sc.line_entry;
+              call_site_line.line = found_decl.GetLine();
+              call_site_line.column = found_decl.GetColumn();
+              bool matches_spec = true;
+              // If the user asked for an exact match, we need to make sure the
+              // call site we found actually matches the location.
+              if (src_location_spec.GetExactMatch()) {
+                matches_spec = false;
+                if ((src_location_spec.GetFileSpec() == sc.line_entry.GetFile())
+                    && (src_location_spec.GetLine() &&
+                      *src_location_spec.GetLine() == call_site_line.line)
+                    && (src_location_spec.GetColumn() &&
+                        *src_location_spec.GetColumn() == call_site_line.column))
+                        matches_spec = true;
+              }
+              if (matches_spec && 
+                  sibling_block->GetRangeAtIndex(0, call_site_line.range)) {
+                SymbolContext call_site_sc(sc.target_sp, sc.module_sp, sc.comp_unit,
+                                           sc.function, sc.block, &call_site_line,
+                                           sc.symbol);
+                sc_list.Append(call_site_sc);
+              }
+            }
+          }
+        }
+        
+        // Descend into the child blocks:
+        examine_block(*sibling_block);
+        // Now go to the next sibling:
+        sibling_block = sibling_block->GetSibling();  
+      }
+    };
+    
+    if (function) {
+      // We don't need to examine the function block, it can't be inlined.
+      Block &func_block = function->GetBlock(true);
+      examine_block(func_block);
+    }
+    // If we found entries here, we are done.  We only get here because we
+    // didn't find an exact line entry for this line & column, but if we found
+    // an exact match from the call site info that's strictly better than 
+    // continuing to look for matches further on in the file.
+    // FIXME: Should I also do this for "call site line exists between the
+    // given line number and the later line we found in the line table"?  That's
+    // a closer approximation to our general sliding algorithm.
+    if (sc_list.GetSize())
+      return;
+  }
 
   // If "exact == true", then "found_line" will be the same as "line". If
   // "exact == false", the "found_line" will be the closest line entry
diff --git a/lldb/source/Target/StackFrameList.cpp b/lldb/source/Target/StackFrameList.cpp
index 3849ec5ed178d9..cda360baed6491 100644
--- a/lldb/source/Target/StackFrameList.cpp
+++ b/lldb/source/Target/StackFrameList.cpp
@@ -86,120 +86,31 @@ void StackFrameList::ResetCurrentInlinedDepth() {
 
   std::lock_guard<std::recursive_mutex> guard(m_mutex);
   
-  GetFramesUpTo(0, DoNotAllowInterruption);
-  if (m_frames.empty())
-    return;
-  if (!m_frames[0]->IsInlined()) {
-    m_current_inlined_depth = UINT32_MAX;
-    m_current_inlined_pc = LLDB_INVALID_ADDRESS;
-    Log *log = GetLog(LLDBLog::Step);
-    if (log && log->GetVerbose())
-      LLDB_LOGF(
-          log,
-          "ResetCurrentInlinedDepth: Invalidating current inlined depth.\n");
-    return;
-  }
+  m_current_inlined_pc = LLDB_INVALID_ADDRESS;
+  m_current_inlined_depth = UINT32_MAX;
 
-  // We only need to do something special about inlined blocks when we are
-  // at the beginning of an inlined function:
-  // FIXME: We probably also have to do something special if the PC is at
-  // the END of an inlined function, which coincides with the end of either
-  // its containing function or another inlined function.
-
-  Block *block_ptr = m_frames[0]->GetFrameBlock();
-  if (!block_ptr)
-    return;
-
-  Address pc_as_address;
-  lldb::addr_t curr_pc = m_thread.GetRegisterContext()->GetPC();
-  pc_as_address.SetLoadAddress(curr_pc, &(m_thread.GetProcess()->GetTarget()));
-  AddressRange containing_range;
-  if (!block_ptr->GetRangeContainingAddress(pc_as_address, containing_range) ||
-      pc_as_address != containing_range.GetBaseAddress())
-    return;
-
-  // If we got here because of a breakpoint hit, then set the inlined depth
-  // depending on where the breakpoint was set. If we got here because of a
-  // crash, then set the inlined depth to the deepest most block.  Otherwise,
-  // we stopped here naturally as the result of a step, so set ourselves in the
-  // containing frame of the whole set of nested inlines, so the user can then
-  // "virtually" step into the frames one by one, or next over the whole mess.
-  // Note: We don't have to handle being somewhere in the middle of the stack
-  // here, since ResetCurrentInlinedDepth doesn't get called if there is a
-  // valid inlined depth set.
   StopInfoSP stop_info_sp = m_thread.GetStopInfo();
   if (!stop_info_sp)
     return;
-  switch (stop_info_sp->GetStopReason()) {
-  case eStopReasonWatchpoint:
-  case eStopReasonException:
-  case eStopReasonExec:
-  case eStopReasonFork:
-  case eStopReasonVFork:
-  case eStopReasonVForkDone:
-  case eStopReasonSignal:
-    // In all these cases we want to stop in the deepest frame.
-    m_current_inlined_pc = curr_pc;
-    m_current_inlined_depth = 0;
-    break;
-  case eStopReasonBreakpoint: {
-    // FIXME: Figure out what this break point is doing, and set the inline
-    // depth appropriately.  Be careful to take into account breakpoints that
-    // implement step over prologue, since that should do the default
-    // calculation. For now, if the breakpoints corresponding to this hit are
-    // all internal, I set the stop location to the top of the inlined stack,
-    // since that will make things like stepping over prologues work right.
-    // But if there are any non-internal breakpoints I do to the bottom of the
-    // stack, since that was the old behavior.
-    uint32_t bp_site_id = stop_info_sp->GetValue();
-    BreakpointSiteSP bp_site_sp(
-        m_thread.GetProcess()->GetBreakpointSiteList().FindByID(bp_site_id));
-    bool all_internal = true;
-    if (bp_site_sp) {
-      uint32_t num_owners = bp_site_sp->GetNumberOfConstituents();
-      for (uint32_t i = 0; i < num_owners; i++) {
-        Breakpoint &bp_ref =
-            bp_site_sp->GetConstituentAtIndex(i)->GetBreakpoint();
-        if (!bp_ref.IsInternal()) {
-          all_internal = false;
-        }
-      }
-    }
-    if (!all_internal) {
-      m_current_inlined_pc = curr_pc;
-      m_current_inlined_depth = 0;
-      break;
-    }
-  }
-    [[fallthrough]];
-  default: {
-    // Otherwise, we should set ourselves at the container of the inlining, so
-    // that the user can descend into them. So first we check whether we have
-    // more than one inlined block sharing this PC:
-    int num_inlined_functions = 0;
-
-    for (Block *container_ptr = block_ptr->GetInlinedParent();
-         container_ptr != nullptr;
-         container_ptr = container_ptr->GetInlinedParent()) {
-      if (!container_ptr->GetRangeContainingAddress(pc_as_address,
-                                                    containing_range))
-        break;
-      if (pc_as_address != containing_range.GetBaseAddress())
-        break;
+    
+  bool inlined = true;
+  auto inline_depth = stop_info_sp->GetSuggestedStackFrameIndex(inlined);
+  // We're only adjusting the inlined stack here.
+  Log *log = GetLog(LLDBLog::Step);
+  if (inline_depth) {
+    m_current_inlined_depth = *inline_depth;
+    m_current_inlined_pc = m_thread.GetRegisterContext()->GetPC();
 
-      num_inlined_functions++;
-    }
-    m_current_inlined_pc = curr_pc;
-    m_current_inlined_depth = num_inlined_functions + 1;
-    Log *log = GetLog(LLDBLog::Step);
     if (log && log->GetVerbose())
       LLDB_LOGF(log,
                 "ResetCurrentInlinedDepth: setting inlined "
                 "depth: %d 0x%" PRIx64 ".\n",
-                m_current_inlined_depth, curr_pc);
-
-    break;
-  }
+                m_current_inlined_depth, m_current_inlined_pc);
+  } else {
+    if (log && log->GetVerbose())
+      LLDB_LOGF(
+          log,
+          "ResetCurrentInlinedDepth: Invalidating current inlined depth.\n");
   }
 }
 
@@ -816,19 +727,47 @@ void StackFrameList::SelectMostRelevantFrame() {
 
   RecognizedStackFrameSP recognized_frame_sp = frame_sp->GetRecognizedFrame();
 
-  if (!recognized_frame_sp) {
-    LLDB_LOG(log, "Frame #0 not recognized");
-    return;
+  if (recognized_frame_sp) {
+    if (StackFrameSP most_relevant_frame_sp =
+            recognized_frame_sp->GetMostRelevantFrame()) {
+      LLDB_LOG(log, "Found most relevant frame at index {0}",
+               most_relevant_frame_sp->GetFrameIndex());
+      SetSelectedFrame(most_relevant_frame_sp.get());
+      return;
+    }
   }
+  LLDB_LOG(log, "Frame #0 not recognized");
 
-  if (StackFrameSP most_relevant_frame_sp =
-          recognized_frame_sp->GetMostRelevantFrame()) {
-    LLDB_LOG(log, "Found most relevant frame at index {0}",
-             most_relevant_frame_sp->GetFrameIndex());
-    SetSelectedFrame(most_relevant_frame_sp.get());
-  } else {
-    LLDB_LOG(log, "No relevant frame!");
+  // If this thread has a non-trivial StopInof, then let it suggest
+  // a most relevant frame:
+  StopInfoSP stop_info_sp = m_thread.GetStopInfo();
+  uint32_t stack_idx = 0;
+  bool found_relevant = false;
+  if (stop_info_sp) {
+    // Here we're only asking the stop info if it wants to adjust the real stack
+    // index.  We have to ask about the m_inlined_stack_depth in 
+    // Thread::ShouldStop since the plans need to reason with that info.
+    bool inlined = false;
+    std::optional<uint32_t> stack_opt = stop_info_sp->GetSuggestedStackFrameIndex(inlined);
+    if (stack_opt) {
+      stack_idx = *stack_opt;
+      found_relevant = true;
+    }
   }
+  
+  frame_sp = GetFrameAtIndex(stack_idx);
+  if (!frame_sp)
+    LLDB_LOG(log, "Stop info suggested relevant frame {0} but it didn't exist", 
+             stack_idx);
+  else if (found_relevant)
+    LLDB_LOG(log, "Setting selected frame from stop info to {0}", stack_idx);
+  // Note, we don't have to worry about "inlined" frames here, because we've
+  // already calculated the inlined frame in Thread::ShouldStop, and 
+  // SetSelectedFrame will take care of that adjustment for us.
+  SetSelectedFrame(frame_sp.get());
+  
+  if (!found_relevant)
+    LLDB_LOG(log, "No relevant frame!");
 }
 
 uint32_t StackFrameList::GetSelectedFrameIndex(
@@ -841,6 +780,7 @@ uint32_t StackFrameList::GetSelectedFrameIndex(
     // isn't set, then don't force a selection here, just return 0.
     if (!select_most_relevant)
       return 0;
+    // If the inlined stack frame is set, then use that:
     m_selected_frame_idx = 0;
   }
   return *m_selected_frame_idx;
diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index bd7032b803df90..8ba46366c8b9d9 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -16,6 +16,7 @@
 #include "lldb/Core/Debugger.h"
 #include "lldb/Core/ValueObject.h"
 #include "lldb/Expression/UserExpression.h"
+#include "lldb/Symbol/Block.h"
 #include "lldb/Target/Process.h"
 #include "lldb/Target/StopInfo.h"
 #include "lldb/Target/Target.h"
@@ -246,6 +247,21 @@ class StopInfoBreakpoint : public StopInfo {
     return m_description.c_str();
   }
 
+  std::optional<uint32_t> GetSuggestedStackFrameIndex(bool inlined_stack) override {
+    if (!inlined_stack)
+      return {};
+
+    ThreadSP thread_sp(m_thread_wp.lock());
+    if (!thread_sp)
+      return {};
+    BreakpointSiteSP bp_site_sp(
+        thread_sp->GetProcess()->GetBreakpointSiteList().FindByID(m_value));
+    if (!bp_site_sp)
+      return {};
+      
+    return bp_site_sp->GetSuggestedStackFrameIndex();
+  }
+  
 protected:
   bool ShouldStop(Event *event_ptr) override {
     // This just reports the work done by PerformAction or the synchronous
@@ -1164,6 +1180,45 @@ class StopInfoTrace : public StopInfo {
     else
       return m_description.c_str();
   }
+  
+  std::optional<uint32_t> GetSuggestedStackFrameIndex(bool inlined_stack) override
+  {
+    // Trace only knows how to adjust inlined stacks:
+    if (!inlined_stack)
+      return {};
+
+    ThreadSP thread_sp = GetThread();
+    StackFrameSP frame_0_sp = thread_sp->GetStackFrameAtIndex(0);
+    if (!frame_0_sp)
+      return {};
+    if (!frame_0_sp->IsInlined())
+      return {};
+    Block *block_ptr = frame_0_sp->GetFrameBlock();
+    if (!block_ptr)
+      return {};
+    Address pc_address = frame_0_sp->GetFrameCodeAddress();
+    AddressRange containing_range;
+    if (!block_ptr->GetRangeContainingAddress(pc_address, containing_range) ||
+        pc_address != containing_range.GetBaseAddress())
+      return {};
+      
+    int num_inlined_functions = 0;
+
+    for (Block *container_ptr = block_ptr->GetInlinedParent();
+         container_ptr != nullptr;
+         container_ptr = container_ptr->GetInlinedParent()) {
+      if (!container_ptr->GetRangeContainingAddress(pc_address,
+                                                    containing_range))
+        break;
+      if (pc_address != containing_range.GetBaseAddress())
+        break;
+
+      num_inlined_functions++;
+    }
+    inlined_stack = true;
+    return num_inlined_functions + 1;
+  }
+
 };
 
 // StopInfoException
diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index 902fbb2b519ef7..d13013c18565c9 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -619,6 +619,14 @@ void Thread::WillStop() {
 
 void Thread::SetupForResume() {
   if (GetResumeState() != eStateSuspended) {
+    // First check whether this thread is going to "actually" resume at all.
+    // For instance, if we're stepping from one level to the next of an 
+    // virtual inlined call stack, we just change the inlined call stack index
+    // without actually running this thread.  In that case, for this thread we
+    // shouldn't push a step over breakpoint plan or do that work.
+    if (GetCurrentPlan()->IsVirtualStep())
+      return;
+
     // If we're at a breakpoint push the step-over breakpoint plan.  Do this
     // before telling the current plan it will resume, since we might change
     // what the current plan is.
diff --git a/lldb/source/Target/ThreadPlanStepInRange.cpp b/lldb/source/Target/ThreadPlanStepInRange.cpp
index 567dcc26d0d372..5319afb4edc418 100644
--- a/lldb/source/Target/ThreadPlanStepInRange.cpp
+++ b/lldb/source/Target/ThreadPlanStepInRange.cpp
@@ -41,7 +41,7 @@ ThreadPlanStepInRange::ThreadPlanStepInRange(
                           "Step Range stepping in", thread, range, addr_context,
                           stop_others),
       ThreadPlanShouldStopHere(this), m_step_past_prologue(true),
-      m_virtual_step(false), m_step_into_target(step_into_target) {
+      m_virtual_step(eLazyBoolCalculate), m_step_into_target(step_into_target) {
   SetCallbacks();
   SetFlagsToDefault();
   SetupAvoidNoDebug(step_in_avoids_code_without_debug_info,
@@ -149,7 +149,7 @@ bool ThreadPlanStepInRange::ShouldStop(Event *event_ptr) {
       m_sub_plan_sp.reset();
   }
 
-  if (m_virtual_step) {
+  if (m_virtual_step == eLazyBoolYes) {
     // If we've just completed a virtual step, all we need to do is check for a
     // ShouldStopHere plan, and otherwise we're done.
     // FIXME - This can be both a step in and a step out.  Probably should
@@ -431,7 +431,7 @@ bool ThreadPlanStepInRange::DoPlanExplainsStop(Event *event_ptr) {
 
   bool return_value = false;
 
-  if (m_virtual_step) {
+  if (m_virtual_step == eLazyBoolYes) {
     return_value = true;
   } else {
     StopInfoSP stop_info_sp = GetPrivateStopInfo();
@@ -460,10 +460,13 @@ bool ThreadPlanStepInRange::DoPlanExplainsStop(Event *event_ptr) {
 
 bool ThreadPlanStepInRange::DoWillResume(lldb::StateType resume_state,
                                          bool current_plan) {
-  m_virtual_step = false;
+  m_virtual_step = eLazyBoolCalculate;
   if (resume_state == eStateStepping && current_plan) {
     Thread &thread = GetThread();
     // See if we are about to step over a virtual inlined call.
+    // But if we already know we're virtual stepping, don't decrement the
+    // inlined depth again...
+
     bool step_without_resume = thread.DecrementCurrentInlinedDepth();
     if (step_without_resume) {
       Log *log = GetLog(LLDBLog::Step);
@@ -476,11 +479,20 @@ bool ThreadPlanStepInRange::DoWillResume(lldb::StateType resume_state,
       // FIXME: Maybe it would be better to create a InlineStep stop reason, but
       // then
       // the whole rest of the world would have to handle that stop reason.
-      m_virtual_step = true;
+      m_virtual_step = eLazyBoolYes;
     }
     return !step_without_resume;
   }
   return true;
 }
 
-bool ThreadPlanStepInRange::IsVirtualStep() { return m_virtual_step; }
+bool ThreadPlanStepInRange::IsVirtualStep() { 
+  if (m_virtual_step == eLazyBoolCalculate) {
+    Thread &thread = GetThread();
+    if (thread.GetCurrentInlinedDepth() == UINT32_MAX)
+      m_virtual_step = eLazyBoolNo;
+    else
+      m_virtual_step = eLazyBoolYes;
+  }
+  return m_virtual_step == eLazyBoolYes; 
+}
diff --git a/lldb/source/Target/ThreadPlanStepOverRange.cpp b/lldb/source/Target/ThreadPlanStepOverRange.cpp
index 934f23b3b21a28..3741f51e756870 100644
--- a/lldb/source/Target/ThreadPlanStepOverRange.cpp
+++ b/lldb/source/Target/ThreadPlanStepOverRange.cpp
@@ -397,7 +397,7 @@ bool ThreadPlanStepOverRange::DoWillResume(lldb::StateType resume_state,
       if (in_inlined_stack) {
         Log *log = GetLog(LLDBLog::Step);
         LLDB_LOGF(log,
-                  "ThreadPlanStepInRange::DoWillResume: adjusting range to "
+                  "ThreadPlanStepOverRange::DoWillResume: adjusting range to "
                   "the frame at inlined depth %d.",
                   thread.GetCurrentInlinedDepth());
         StackFrameSP stack_sp = thread.GetStackFrameAtIndex(0);
diff --git a/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py b/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
index 752c3a9cbd286a..92389d5ebfa645 100644
--- a/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
+++ b/lldb/test/API/functionalities/inline-stepping/TestInlineStepping.py
@@ -32,6 +32,12 @@ def test_step_in_template_with_python_api(self):
         self.build()
         self.step_in_template()
 
+    @add_test_categories(["pyapi"])
+    def test_virtual_inline_stepping(self):
+        """Test stepping through a virtual inlined call stack"""
+        self.build()
+        self.virtual_inline_stepping()
+
     def setUp(self):
         # Call super's setUp().
         TestBase.setUp(self)
@@ -357,3 +363,51 @@ def step_in_template(self):
 
         step_sequence = [["// In max_value specialized", "into"]]
         self.run_step_sequence(step_sequence)
+
+    def run_to_call_site_and_step(self, source_regex, func_name, start_pos):
+        main_spec = lldb.SBFileSpec("calling.cpp")
+        # Set the breakpoint by file and line, not sourced regex because
+        # we want to make sure we can set breakpoints on call sites:
+        call_site_line_num = line_number(self.main_source, source_regex)
+        target, process, thread, bkpt = lldbutil.run_to_line_breakpoint(self, main_spec, call_site_line_num)
+
+        # Make sure that the location is at the call site (run_to_line_breakpoint already asserted
+        # that there's one location.):
+        bkpt_loc = bkpt.location[0]
+        strm = lldb.SBStream()
+        result = bkpt_loc.GetDescription(strm, lldb.eDescriptionLevelFull)
+
+        self.assertTrue(result, "Got a location description")
+        desc = strm.GetData()
+        print(f"Description:\n{desc}\n")
+        self.assertIn(f"calling.cpp:{call_site_line_num}", desc, "Right line listed")
+        # We don't get the function name right yet - so we omit it in printing.
+        # Turn on this test when that is working.
+        #self.assertIn(func_name, desc, "Right function listed")
+
+        pc = thread.frame[0].pc
+        for i in range(start_pos, 3):
+            thread.StepInto()
+            frame_0 = thread.frame[0]
+            
+            trivial_line_num = line_number(self.main_source, f"In caller_trivial_inline_{i}.")
+            self.assertEqual(frame_0.line_entry.line, trivial_line_num, f"Stepped into the caller_trivial_inline_{i}")
+            if pc != frame_0.pc:
+                # If we get here, we stepped to the expected line number, but
+                # the compiler on this system has decided to insert an instruction
+                # between the call site of an inlined function with no arguments,
+                # returning void, and its immediate call to another void inlined function
+                # with no arguments.  We aren't going to be testing virtual inline
+                # stepping for this function...
+                break
+
+        process.Kill()
+        target.Clear()
+
+    def virtual_inline_stepping(self):
+        """Use the Python API's to step through a virtual inlined stack"""
+        self.run_to_call_site_and_step("At caller_trivial_inline_1", "main", 1)
+        self.run_to_call_site_and_step("In caller_trivial_inline_1", "caller_trivial_inline_1", 2)
+        self.run_to_call_site_and_step("In caller_trivial_inline_2", "caller_trivial_inline_2", 3)
+
+        
diff --git a/lldb/test/API/functionalities/inline-stepping/calling.cpp b/lldb/test/API/functionalities/inline-stepping/calling.cpp
index 49179ce7c97883..0009388bd80633 100644
--- a/lldb/test/API/functionalities/inline-stepping/calling.cpp
+++ b/lldb/test/API/functionalities/inline-stepping/calling.cpp
@@ -13,6 +13,12 @@ int called_by_inline_ref (int &value);
 inline void inline_trivial_1 () __attribute__((always_inline));
 inline void inline_trivial_2 () __attribute__((always_inline));
 
+// These three should share the same initial pc so we can test
+// virtual inline stepping.
+inline void caller_trivial_inline_1 () __attribute__((always_inline));
+inline void caller_trivial_inline_2 () __attribute__((always_inline));
+inline void caller_trivial_inline_3 () __attribute__((always_inline));
+
 void caller_trivial_1 ();
 void caller_trivial_2 ();
 
@@ -79,6 +85,29 @@ caller_trivial_2 ()
     inline_value += 1;  // At increment in caller_trivial_2.
 }
 
+// When you call caller_trivial_inline_1, the inlined call-site
+// should share a PC with all three of the following inlined
+// functions, so we can exercise "virtual inline stepping".
+void
+caller_trivial_inline_1 ()
+{
+    caller_trivial_inline_2(); // In caller_trivial_inline_1.
+    inline_value += 1;
+}
+
+void
+caller_trivial_inline_2 ()
+{
+    caller_trivial_inline_3(); // In caller_trivial_inline_2.
+    inline_value += 1;
+}
+
+void
+caller_trivial_inline_3 ()
+{
+    inline_value += 1;  // In caller_trivial_inline_3.
+}
+
 void
 called_by_inline_trivial ()
 {
@@ -132,5 +161,7 @@ main (int argc, char **argv)
     max_value(123, 456);                                // Call max_value template
     max_value(std::string("abc"), std::string("0022")); // Call max_value specialized
 
+    caller_trivial_inline_1(); // At caller_trivial_inline_1.
+ 
     return 0;            // About to return from main.
 }

>From 7a60871262fe9c807930f20833b46c6353860269 Mon Sep 17 00:00:00 2001
From: Jim Ingham <jingham at apple.com>
Date: Fri, 18 Oct 2024 10:55:19 -0700
Subject: [PATCH 2/3] clang-format

---
 .../lldb/Breakpoint/BreakpointLocation.h      | 21 +++---
 lldb/include/lldb/Breakpoint/BreakpointSite.h |  2 +-
 lldb/include/lldb/Target/StopInfo.h           |  7 +-
 .../lldb/Target/ThreadPlanStepInRange.h       |  4 +-
 lldb/source/Breakpoint/BreakpointLocation.cpp | 17 ++---
 lldb/source/Breakpoint/BreakpointResolver.cpp | 12 ++--
 lldb/source/Symbol/CompileUnit.cpp            | 67 ++++++++++---------
 lldb/source/Target/StackFrameList.cpp         | 17 ++---
 lldb/source/Target/StopInfo.cpp               | 16 ++---
 lldb/source/Target/Thread.cpp                 |  2 +-
 lldb/source/Target/ThreadPlanStepInRange.cpp  |  4 +-
 .../inline-stepping/calling.cpp               | 30 ++++-----
 12 files changed, 102 insertions(+), 97 deletions(-)

diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocation.h b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
index f9c258daf137f7..552fafab97cb9a 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
@@ -283,7 +283,7 @@ class BreakpointLocation
 
   /// Returns the breakpoint location ID.
   lldb::break_id_t GetID() const { return m_loc_id; }
-  
+
   // Set the line entry that should be shown to users for this location.
   // It is up to the caller to verify that this is a valid entry to show.
   // The current use of this is to distinguish among line entries from a
@@ -291,7 +291,7 @@ class BreakpointLocation
   void SetPreferredLineEntry(const LineEntry &line_entry) {
     m_preferred_line_entry = line_entry;
   }
-  
+
   const std::optional<LineEntry> GetPreferredLineEntry() {
     return m_preferred_line_entry;
   }
@@ -319,7 +319,7 @@ class BreakpointLocation
   /// It also takes care of decrementing the ignore counters.
   /// If it returns false we should continue, otherwise stop.
   bool IgnoreCountShouldStop();
-  
+
   // If this location knows that the virtual stack frame it represents is
   // not frame 0, return the suggested stack frame instead.  This will happen
   // when the location's address contains a "virtual inlined call stack" and the
@@ -393,13 +393,14 @@ class BreakpointLocation
   lldb::break_id_t m_loc_id; ///< Breakpoint location ID.
   StoppointHitCounter m_hit_counter; ///< Number of times this breakpoint
                                      /// location has been hit.
-  std::optional<LineEntry> m_preferred_line_entry; // If this exists, use it to print the stop
-                                    // description rather than the LineEntry 
-                                    // m_address resolves to directly.  Use this
-                                    // for instance when the location was given
-                                    // somewhere in the virtual inlined call
-                                    // stack since the Address always resolves 
-                                    // to the lowest entry in the stack.
+  std::optional<LineEntry>
+      m_preferred_line_entry; // If this exists, use it to print the stop
+                              // description rather than the LineEntry
+                              // m_address resolves to directly.  Use this
+                              // for instance when the location was given
+                              // somewhere in the virtual inlined call
+                              // stack since the Address always resolves
+                              // to the lowest entry in the stack.
 
   void SetShouldResolveIndirectFunctions(bool do_resolve) {
     m_should_resolve_indirect_functions = do_resolve;
diff --git a/lldb/include/lldb/Breakpoint/BreakpointSite.h b/lldb/include/lldb/Breakpoint/BreakpointSite.h
index 30cb5a80b908e0..7b3f7be23639f2 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointSite.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointSite.h
@@ -169,7 +169,7 @@ class BreakpointSite : public std::enable_shared_from_this<BreakpointSite>,
   ///
   /// \see lldb::DescriptionLevel
   void GetDescription(Stream *s, lldb::DescriptionLevel level);
-  
+
   // This runs through all the breakpoint locations owning this site and returns
   // the greatest of their suggested stack frame indexes.  This only handles
   // inlined stack changes.
diff --git a/lldb/include/lldb/Target/StopInfo.h b/lldb/include/lldb/Target/StopInfo.h
index d997e0fd6fc550..45beac129e86f7 100644
--- a/lldb/include/lldb/Target/StopInfo.h
+++ b/lldb/include/lldb/Target/StopInfo.h
@@ -79,13 +79,14 @@ class StopInfo : public std::enable_shared_from_this<StopInfo> {
 
   /// This gives the StopInfo a chance to suggest a stack frame to select.
   /// Passing true for inlined_stack will request changes to the inlined
-  /// call stack.  Passing false will request changes to the real stack 
+  /// call stack.  Passing false will request changes to the real stack
   /// frame.  The inlined stack gets adjusted before we call into the thread
   /// plans so they can reason based on the correct values.  The real stack
   /// adjustment is handled after the frame recognizers get a chance to adjust
   /// the frame.
-  virtual std::optional<uint32_t> GetSuggestedStackFrameIndex(bool inlined_stack) { 
-     return {};
+  virtual std::optional<uint32_t>
+  GetSuggestedStackFrameIndex(bool inlined_stack) {
+    return {};
   }
 
   virtual bool IsValidForOperatingSystemThread(Thread &thread) { return true; }
diff --git a/lldb/include/lldb/Target/ThreadPlanStepInRange.h b/lldb/include/lldb/Target/ThreadPlanStepInRange.h
index 6bd79e1d287d35..f2cb1085302318 100644
--- a/lldb/include/lldb/Target/ThreadPlanStepInRange.h
+++ b/lldb/include/lldb/Target/ThreadPlanStepInRange.h
@@ -80,8 +80,8 @@ class ThreadPlanStepInRange : public ThreadPlanStepRange,
   bool m_step_past_prologue; // FIXME: For now hard-coded to true, we could put
                              // a switch in for this if there's
                              // demand for that.
-  LazyBool m_virtual_step; // true if we've just done a "virtual step", i.e. just
-                           // moved the inline stack depth.
+  LazyBool m_virtual_step;   // true if we've just done a "virtual step", i.e.
+                           // just moved the inline stack depth.
   ConstString m_step_into_target;
   ThreadPlanStepInRange(const ThreadPlanStepInRange &) = delete;
   const ThreadPlanStepInRange &
diff --git a/lldb/source/Breakpoint/BreakpointLocation.cpp b/lldb/source/Breakpoint/BreakpointLocation.cpp
index 61fe467987228b..7e002b9ba4c2e2 100644
--- a/lldb/source/Breakpoint/BreakpointLocation.cpp
+++ b/lldb/source/Breakpoint/BreakpointLocation.cpp
@@ -508,7 +508,7 @@ void BreakpointLocation::GetDescription(Stream *s,
         s->PutCString("re-exported target = ");
       else
         s->PutCString("where = ");
-        
+
       // If there's a preferred line entry for printing, use that.
       bool show_function_info = true;
       if (GetPreferredLineEntry()) {
@@ -520,7 +520,8 @@ void BreakpointLocation::GetDescription(Stream *s,
         show_function_info = false;
       }
       sc.DumpStopContext(s, m_owner.GetTarget().GetProcessSP().get(), m_address,
-                         false, true, false, show_function_info, show_function_info, show_function_info);
+                         false, true, false, show_function_info,
+                         show_function_info, show_function_info);
     } else {
       if (sc.module_sp) {
         s->EOL();
@@ -670,7 +671,7 @@ void BreakpointLocation::SendBreakpointLocationChangedEvent(
   }
 }
 
-std::optional<uint32_t>  BreakpointLocation::GetSuggestedStackFrameIndex() {
+std::optional<uint32_t> BreakpointLocation::GetSuggestedStackFrameIndex() {
   if (!GetPreferredLineEntry())
     return {};
   LineEntry preferred = *GetPreferredLineEntry();
@@ -682,12 +683,13 @@ std::optional<uint32_t>  BreakpointLocation::GetSuggestedStackFrameIndex() {
   // case.
   if (!LineEntry::Compare(sc.line_entry, preferred))
     return {};
-  
+
   if (!sc.block)
     return {};
 
   // Blocks have their line info in Declaration form, so make one here:
-  Declaration preferred_decl(preferred.GetFile(), preferred.line, preferred.column);
+  Declaration preferred_decl(preferred.GetFile(), preferred.line,
+                             preferred.column);
 
   uint32_t depth = 0;
   Block *inlined_block = sc.block->GetContainingInlinedBlock();
@@ -695,8 +697,8 @@ std::optional<uint32_t>  BreakpointLocation::GetSuggestedStackFrameIndex() {
     // If we've moved to a block that this isn't the start of, that's not
     // our inlining info or call site, so we can stop here.
     Address start_address;
-    if (!inlined_block->GetStartAddress(start_address) 
-        || start_address != m_address)
+    if (!inlined_block->GetStartAddress(start_address) ||
+        start_address != m_address)
       return {};
 
     const InlineFunctionInfo *info = inlined_block->GetInlinedFunctionInfo();
@@ -710,7 +712,6 @@ std::optional<uint32_t>  BreakpointLocation::GetSuggestedStackFrameIndex() {
     depth++;
   }
   return {};
-  
 }
 
 void BreakpointLocation::SwapLocation(BreakpointLocationSP swap_from) {
diff --git a/lldb/source/Breakpoint/BreakpointResolver.cpp b/lldb/source/Breakpoint/BreakpointResolver.cpp
index 465fc387aa43e5..688dffa02eb162 100644
--- a/lldb/source/Breakpoint/BreakpointResolver.cpp
+++ b/lldb/source/Breakpoint/BreakpointResolver.cpp
@@ -340,17 +340,17 @@ void BreakpointResolver::AddLocation(SearchFilter &filter,
   }
 
   BreakpointLocationSP bp_loc_sp(AddLocation(line_start));
-  // If the address that we resolved the location to returns a different 
+  // If the address that we resolved the location to returns a different
   // LineEntry from the one in the incoming SC, we're probably dealing with an
   // inlined call site, so set that as the preferred LineEntry:
   LineEntry resolved_entry;
-  if (!skipped_prologue && bp_loc_sp && 
+  if (!skipped_prologue && bp_loc_sp &&
       line_start.CalculateSymbolContextLineEntry(resolved_entry) &&
       LineEntry::Compare(resolved_entry, sc.line_entry)) {
-      // FIXME: The function name will also be wrong here.  Do we need to record
-      // that as well, or can we figure that out again when we report this
-      // breakpoint location.
-      bp_loc_sp->SetPreferredLineEntry(sc.line_entry);
+    // FIXME: The function name will also be wrong here.  Do we need to record
+    // that as well, or can we figure that out again when we report this
+    // breakpoint location.
+    bp_loc_sp->SetPreferredLineEntry(sc.line_entry);
   }
   if (log && bp_loc_sp && !GetBreakpoint()->IsInternal()) {
     StreamString s;
diff --git a/lldb/source/Symbol/CompileUnit.cpp b/lldb/source/Symbol/CompileUnit.cpp
index f4c61d6ec1b196..666e682ff537c4 100644
--- a/lldb/source/Symbol/CompileUnit.cpp
+++ b/lldb/source/Symbol/CompileUnit.cpp
@@ -251,8 +251,10 @@ void CompileUnit::ResolveSymbolContext(
     SymbolContextItem resolve_scope, SymbolContextList &sc_list,
     RealpathPrefixes *realpath_prefixes) {
   const FileSpec file_spec = src_location_spec.GetFileSpec();
-  const uint32_t line = src_location_spec.GetLine().value_or(LLDB_INVALID_LINE_NUMBER);
-  const uint32_t column_num = src_location_spec.GetColumn().value_or(LLDB_INVALID_COLUMN_NUMBER);
+  const uint32_t line =
+      src_location_spec.GetLine().value_or(LLDB_INVALID_LINE_NUMBER);
+  const uint32_t column_num =
+      src_location_spec.GetColumn().value_or(LLDB_INVALID_COLUMN_NUMBER);
   const bool check_inlines = src_location_spec.GetCheckInlines();
 
   // First find all of the file indexes that match our "file_spec". If
@@ -312,42 +314,45 @@ void CompileUnit::ResolveSymbolContext(
     line_idx = line_table->FindLineEntryIndexByFileIndex(
         0, file_indexes, src_location_spec, &line_entry);
   }
-  
+
   // If we didn't manage to find a breakpoint that matched the line number
-  // requested, that might be because it is only an inline call site, and 
+  // requested, that might be because it is only an inline call site, and
   // doesn't have a line entry in the line table.  Scan for that here.
   //
   // We are making the assumption that if there was an inlined function it will
-  // contribute at least 1 non-call-site entry to the line table.  That's handy 
-  // because we don't move line breakpoints over function boundaries, so if we 
+  // contribute at least 1 non-call-site entry to the line table.  That's handy
+  // because we don't move line breakpoints over function boundaries, so if we
   // found a hit, and there were also a call site entry, it would have to be in
   // the function containing the PC of the line table match.  That way we can
   // limit the call site search to that function.
   // We will miss functions that ONLY exist as a call site entry.
 
-  if (line_entry.IsValid() && (line_entry.line != line || line_entry.column != column_num)                                
-      && resolve_scope & eSymbolContextLineEntry && check_inlines) {
+  if (line_entry.IsValid() &&
+      (line_entry.line != line || line_entry.column != column_num) &&
+      resolve_scope & eSymbolContextLineEntry && check_inlines) {
     // We don't move lines over function boundaries, so the address in the
     // line entry will be the in function that contained the line that might
     // be a CallSite, and we can just iterate over that function to find any
     // inline records, and dig up their call sites.
     Address start_addr = line_entry.range.GetBaseAddress();
     Function *function = start_addr.CalculateSymbolContextFunction();
-    
+
     Declaration sought_decl(file_spec, line, column_num);
     // We use this recursive function to descend the block structure looking
     // for a block that has this Declaration as in it's CallSite info.
     // This function recursively scans the sibling blocks of the incoming
     // block parameter.
-    std::function<void(Block&)> examine_block = 
-        [&sought_decl, &sc_list, &src_location_spec, resolve_scope, &examine_block] (Block &block) -> void {
+    std::function<void(Block &)> examine_block =
+        [&sought_decl, &sc_list, &src_location_spec, resolve_scope,
+         &examine_block](Block &block) -> void {
       // Iterate over the sibling child blocks of the incoming block.
       Block *sibling_block = block.GetFirstChild();
       while (sibling_block) {
         // We only have to descend through the regular blocks, looking for
         // immediate inlines, since those are the only ones that will have this
         // callsite.
-        const InlineFunctionInfo *inline_info = sibling_block->GetInlinedFunctionInfo();
+        const InlineFunctionInfo *inline_info =
+            sibling_block->GetInlinedFunctionInfo();
         if (inline_info) {
           // If this is the call-site we are looking for, record that:
           // We need to be careful because the call site from the debug info
@@ -355,13 +360,14 @@ void CompileUnit::ResolveSymbolContext(
           // it.
           Declaration found_decl = inline_info->GetCallSite();
           uint32_t sought_column = sought_decl.GetColumn();
-          if (found_decl.FileAndLineEqual(sought_decl)
-              && (sought_column == LLDB_INVALID_COLUMN_NUMBER 
-                  || sought_column == found_decl.GetColumn())) {
+          if (found_decl.FileAndLineEqual(sought_decl) &&
+              (sought_column == LLDB_INVALID_COLUMN_NUMBER ||
+               sought_column == found_decl.GetColumn())) {
             // If we found a call site, it belongs not in this inlined block,
             // but in the parent block that inlined it.
             Address parent_start_addr;
-            if (sibling_block->GetParent()->GetStartAddress(parent_start_addr)) {
+            if (sibling_block->GetParent()->GetStartAddress(
+                    parent_start_addr)) {
               SymbolContext sc;
               parent_start_addr.CalculateSymbolContext(&sc, resolve_scope);
               // Now swap out the line entry for the one we found.
@@ -373,31 +379,32 @@ void CompileUnit::ResolveSymbolContext(
               // call site we found actually matches the location.
               if (src_location_spec.GetExactMatch()) {
                 matches_spec = false;
-                if ((src_location_spec.GetFileSpec() == sc.line_entry.GetFile())
-                    && (src_location_spec.GetLine() &&
-                      *src_location_spec.GetLine() == call_site_line.line)
-                    && (src_location_spec.GetColumn() &&
-                        *src_location_spec.GetColumn() == call_site_line.column))
-                        matches_spec = true;
+                if ((src_location_spec.GetFileSpec() ==
+                     sc.line_entry.GetFile()) &&
+                    (src_location_spec.GetLine() &&
+                     *src_location_spec.GetLine() == call_site_line.line) &&
+                    (src_location_spec.GetColumn() &&
+                     *src_location_spec.GetColumn() == call_site_line.column))
+                  matches_spec = true;
               }
-              if (matches_spec && 
+              if (matches_spec &&
                   sibling_block->GetRangeAtIndex(0, call_site_line.range)) {
-                SymbolContext call_site_sc(sc.target_sp, sc.module_sp, sc.comp_unit,
-                                           sc.function, sc.block, &call_site_line,
-                                           sc.symbol);
+                SymbolContext call_site_sc(sc.target_sp, sc.module_sp,
+                                           sc.comp_unit, sc.function, sc.block,
+                                           &call_site_line, sc.symbol);
                 sc_list.Append(call_site_sc);
               }
             }
           }
         }
-        
+
         // Descend into the child blocks:
         examine_block(*sibling_block);
         // Now go to the next sibling:
-        sibling_block = sibling_block->GetSibling();  
+        sibling_block = sibling_block->GetSibling();
       }
     };
-    
+
     if (function) {
       // We don't need to examine the function block, it can't be inlined.
       Block &func_block = function->GetBlock(true);
@@ -405,7 +412,7 @@ void CompileUnit::ResolveSymbolContext(
     }
     // If we found entries here, we are done.  We only get here because we
     // didn't find an exact line entry for this line & column, but if we found
-    // an exact match from the call site info that's strictly better than 
+    // an exact match from the call site info that's strictly better than
     // continuing to look for matches further on in the file.
     // FIXME: Should I also do this for "call site line exists between the
     // given line number and the later line we found in the line table"?  That's
diff --git a/lldb/source/Target/StackFrameList.cpp b/lldb/source/Target/StackFrameList.cpp
index cda360baed6491..94a381edd5e202 100644
--- a/lldb/source/Target/StackFrameList.cpp
+++ b/lldb/source/Target/StackFrameList.cpp
@@ -85,14 +85,14 @@ void StackFrameList::ResetCurrentInlinedDepth() {
     return;
 
   std::lock_guard<std::recursive_mutex> guard(m_mutex);
-  
+
   m_current_inlined_pc = LLDB_INVALID_ADDRESS;
   m_current_inlined_depth = UINT32_MAX;
 
   StopInfoSP stop_info_sp = m_thread.GetStopInfo();
   if (!stop_info_sp)
     return;
-    
+
   bool inlined = true;
   auto inline_depth = stop_info_sp->GetSuggestedStackFrameIndex(inlined);
   // We're only adjusting the inlined stack here.
@@ -745,27 +745,28 @@ void StackFrameList::SelectMostRelevantFrame() {
   bool found_relevant = false;
   if (stop_info_sp) {
     // Here we're only asking the stop info if it wants to adjust the real stack
-    // index.  We have to ask about the m_inlined_stack_depth in 
+    // index.  We have to ask about the m_inlined_stack_depth in
     // Thread::ShouldStop since the plans need to reason with that info.
     bool inlined = false;
-    std::optional<uint32_t> stack_opt = stop_info_sp->GetSuggestedStackFrameIndex(inlined);
+    std::optional<uint32_t> stack_opt =
+        stop_info_sp->GetSuggestedStackFrameIndex(inlined);
     if (stack_opt) {
       stack_idx = *stack_opt;
       found_relevant = true;
     }
   }
-  
+
   frame_sp = GetFrameAtIndex(stack_idx);
   if (!frame_sp)
-    LLDB_LOG(log, "Stop info suggested relevant frame {0} but it didn't exist", 
+    LLDB_LOG(log, "Stop info suggested relevant frame {0} but it didn't exist",
              stack_idx);
   else if (found_relevant)
     LLDB_LOG(log, "Setting selected frame from stop info to {0}", stack_idx);
   // Note, we don't have to worry about "inlined" frames here, because we've
-  // already calculated the inlined frame in Thread::ShouldStop, and 
+  // already calculated the inlined frame in Thread::ShouldStop, and
   // SetSelectedFrame will take care of that adjustment for us.
   SetSelectedFrame(frame_sp.get());
-  
+
   if (!found_relevant)
     LLDB_LOG(log, "No relevant frame!");
 }
diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index 8ba46366c8b9d9..aa9c97a615b25e 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -247,7 +247,8 @@ class StopInfoBreakpoint : public StopInfo {
     return m_description.c_str();
   }
 
-  std::optional<uint32_t> GetSuggestedStackFrameIndex(bool inlined_stack) override {
+  std::optional<uint32_t>
+  GetSuggestedStackFrameIndex(bool inlined_stack) override {
     if (!inlined_stack)
       return {};
 
@@ -258,10 +259,10 @@ class StopInfoBreakpoint : public StopInfo {
         thread_sp->GetProcess()->GetBreakpointSiteList().FindByID(m_value));
     if (!bp_site_sp)
       return {};
-      
+
     return bp_site_sp->GetSuggestedStackFrameIndex();
   }
-  
+
 protected:
   bool ShouldStop(Event *event_ptr) override {
     // This just reports the work done by PerformAction or the synchronous
@@ -1180,9 +1181,9 @@ class StopInfoTrace : public StopInfo {
     else
       return m_description.c_str();
   }
-  
-  std::optional<uint32_t> GetSuggestedStackFrameIndex(bool inlined_stack) override
-  {
+
+  std::optional<uint32_t>
+  GetSuggestedStackFrameIndex(bool inlined_stack) override {
     // Trace only knows how to adjust inlined stacks:
     if (!inlined_stack)
       return {};
@@ -1201,7 +1202,7 @@ class StopInfoTrace : public StopInfo {
     if (!block_ptr->GetRangeContainingAddress(pc_address, containing_range) ||
         pc_address != containing_range.GetBaseAddress())
       return {};
-      
+
     int num_inlined_functions = 0;
 
     for (Block *container_ptr = block_ptr->GetInlinedParent();
@@ -1218,7 +1219,6 @@ class StopInfoTrace : public StopInfo {
     inlined_stack = true;
     return num_inlined_functions + 1;
   }
-
 };
 
 // StopInfoException
diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index d13013c18565c9..6c22b50d3b6068 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -620,7 +620,7 @@ void Thread::WillStop() {
 void Thread::SetupForResume() {
   if (GetResumeState() != eStateSuspended) {
     // First check whether this thread is going to "actually" resume at all.
-    // For instance, if we're stepping from one level to the next of an 
+    // For instance, if we're stepping from one level to the next of an
     // virtual inlined call stack, we just change the inlined call stack index
     // without actually running this thread.  In that case, for this thread we
     // shouldn't push a step over breakpoint plan or do that work.
diff --git a/lldb/source/Target/ThreadPlanStepInRange.cpp b/lldb/source/Target/ThreadPlanStepInRange.cpp
index 5319afb4edc418..325a70619908b6 100644
--- a/lldb/source/Target/ThreadPlanStepInRange.cpp
+++ b/lldb/source/Target/ThreadPlanStepInRange.cpp
@@ -486,7 +486,7 @@ bool ThreadPlanStepInRange::DoWillResume(lldb::StateType resume_state,
   return true;
 }
 
-bool ThreadPlanStepInRange::IsVirtualStep() { 
+bool ThreadPlanStepInRange::IsVirtualStep() {
   if (m_virtual_step == eLazyBoolCalculate) {
     Thread &thread = GetThread();
     if (thread.GetCurrentInlinedDepth() == UINT32_MAX)
@@ -494,5 +494,5 @@ bool ThreadPlanStepInRange::IsVirtualStep() {
     else
       m_virtual_step = eLazyBoolYes;
   }
-  return m_virtual_step == eLazyBoolYes; 
+  return m_virtual_step == eLazyBoolYes;
 }
diff --git a/lldb/test/API/functionalities/inline-stepping/calling.cpp b/lldb/test/API/functionalities/inline-stepping/calling.cpp
index 0009388bd80633..d7ee56b3c07909 100644
--- a/lldb/test/API/functionalities/inline-stepping/calling.cpp
+++ b/lldb/test/API/functionalities/inline-stepping/calling.cpp
@@ -15,9 +15,9 @@ inline void inline_trivial_2 () __attribute__((always_inline));
 
 // These three should share the same initial pc so we can test
 // virtual inline stepping.
-inline void caller_trivial_inline_1 () __attribute__((always_inline));
-inline void caller_trivial_inline_2 () __attribute__((always_inline));
-inline void caller_trivial_inline_3 () __attribute__((always_inline));
+inline void caller_trivial_inline_1() __attribute__((always_inline));
+inline void caller_trivial_inline_2() __attribute__((always_inline));
+inline void caller_trivial_inline_3() __attribute__((always_inline));
 
 void caller_trivial_1 ();
 void caller_trivial_2 ();
@@ -88,24 +88,18 @@ caller_trivial_2 ()
 // When you call caller_trivial_inline_1, the inlined call-site
 // should share a PC with all three of the following inlined
 // functions, so we can exercise "virtual inline stepping".
-void
-caller_trivial_inline_1 ()
-{
-    caller_trivial_inline_2(); // In caller_trivial_inline_1.
-    inline_value += 1;
+void caller_trivial_inline_1() {
+  caller_trivial_inline_2(); // In caller_trivial_inline_1.
+  inline_value += 1;
 }
 
-void
-caller_trivial_inline_2 ()
-{
-    caller_trivial_inline_3(); // In caller_trivial_inline_2.
-    inline_value += 1;
+void caller_trivial_inline_2() {
+  caller_trivial_inline_3(); // In caller_trivial_inline_2.
+  inline_value += 1;
 }
 
-void
-caller_trivial_inline_3 ()
-{
-    inline_value += 1;  // In caller_trivial_inline_3.
+void caller_trivial_inline_3() {
+  inline_value += 1; // In caller_trivial_inline_3.
 }
 
 void
@@ -162,6 +156,6 @@ main (int argc, char **argv)
     max_value(std::string("abc"), std::string("0022")); // Call max_value specialized
 
     caller_trivial_inline_1(); // At caller_trivial_inline_1.
- 
+
     return 0;            // About to return from main.
 }

>From a19b3dce64dadccc11559fa1d626ea67c5a9bff7 Mon Sep 17 00:00:00 2001
From: Jim Ingham <jingham at apple.com>
Date: Fri, 18 Oct 2024 10:56:59 -0700
Subject: [PATCH 3/3] Comment -> Doxygen comment.

---
 .../lldb/Breakpoint/BreakpointLocation.h      | 24 +++++++++----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/lldb/include/lldb/Breakpoint/BreakpointLocation.h b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
index 552fafab97cb9a..fc1de12ae27f92 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointLocation.h
+++ b/lldb/include/lldb/Breakpoint/BreakpointLocation.h
@@ -284,10 +284,10 @@ class BreakpointLocation
   /// Returns the breakpoint location ID.
   lldb::break_id_t GetID() const { return m_loc_id; }
 
-  // Set the line entry that should be shown to users for this location.
-  // It is up to the caller to verify that this is a valid entry to show.
-  // The current use of this is to distinguish among line entries from a
-  // virtual inlined call stack that all share the same address.
+  /// Set the line entry that should be shown to users for this location.
+  /// It is up to the caller to verify that this is a valid entry to show.
+  /// The current use of this is to distinguish among line entries from a
+  /// virtual inlined call stack that all share the same address.
   void SetPreferredLineEntry(const LineEntry &line_entry) {
     m_preferred_line_entry = line_entry;
   }
@@ -320,14 +320,14 @@ class BreakpointLocation
   /// If it returns false we should continue, otherwise stop.
   bool IgnoreCountShouldStop();
 
-  // If this location knows that the virtual stack frame it represents is
-  // not frame 0, return the suggested stack frame instead.  This will happen
-  // when the location's address contains a "virtual inlined call stack" and the
-  // breakpoint was set on a file & line that are not at the bottom of that
-  // stack.  For now we key off the "preferred line entry" - looking for that
-  // in the blocks that start with the stop PC.
-  // This version of the API doesn't take an "inlined" parameter because it
-  // only changes frames in the inline stack.
+  /// If this location knows that the virtual stack frame it represents is
+  /// not frame 0, return the suggested stack frame instead.  This will happen
+  /// when the location's address contains a "virtual inlined call stack" and the
+  /// breakpoint was set on a file & line that are not at the bottom of that
+  /// stack.  For now we key off the "preferred line entry" - looking for that
+  /// in the blocks that start with the stop PC.
+  /// This version of the API doesn't take an "inlined" parameter because it
+  /// only changes frames in the inline stack.
   std::optional<uint32_t> GetSuggestedStackFrameIndex();
 
 private:



More information about the lldb-commits mailing list