[Lldb-commits] [lldb] 39572f5 - [lldb][Target] Clear selected frame index after a StopInfo::PerformAction (#133078)

[via lldb-commits]

Author: Michael Buch
Date: 2025-09-08T17:18:25+01:00
New Revision: 39572f5e9168b1b44c2f9078494616fed8752086

URL: https://github.com/llvm/llvm-project/commit/39572f5e9168b1b44c2f9078494616fed8752086
DIFF: https://github.com/llvm/llvm-project/commit/39572f5e9168b1b44c2f9078494616fed8752086.diff

LOG: [lldb][Target] Clear selected frame index after a StopInfo::PerformAction (#133078)

The motivation for this patch is that
`StopInfo::GetSuggestedStackFrameIndex` would not take effect for
`InstrumentationRuntimeStopInfo` (which we plan to implement in
https://github.com/llvm/llvm-project/pull/133079). This was happening
because the instrumentation runtime plugins would run utility
expressions as part of the stop that would set the
`m_selected_frame_idx`. This means `SelectMostRelevantFrame` was never
called, and we would not be able to report the selected frame via the
`StopInfo` object.

This patch makes sure we clear the `m_selected_frame_idx` to allow
`GetSuggestedStackFrameIndex` to take effect, regardless of what the
frame recognizers choose to do.

Added: 
    

Modified: 
    lldb/include/lldb/Target/StackFrameList.h
    lldb/include/lldb/Target/Thread.h
    lldb/source/Target/Process.cpp
    lldb/source/Target/StackFrameList.cpp

Removed: 
    


################################################################################
diff  --git a/lldb/include/lldb/Target/StackFrameList.h b/lldb/include/lldb/Target/StackFrameList.h
index e5a6e942d7431..ea9aab86b8ea1 100644
--- a/lldb/include/lldb/Target/StackFrameList.h
+++ b/lldb/include/lldb/Target/StackFrameList.h
@@ -46,6 +46,9 @@ class StackFrameList {
   /// Mark a stack frame as the currently selected frame and return its index.
   uint32_t SetSelectedFrame(lldb_private::StackFrame *frame);
 
+  /// Resets the selected frame index of this object.
+  void ClearSelectedFrameIndex();
+
   /// Get the currently selected frame index.
   /// We should only call SelectMostRelevantFrame if (a) the user hasn't already
   /// selected a frame, and (b) if this really is a user facing
@@ -172,6 +175,15 @@ class StackFrameList {
   /// The currently selected frame. An optional is used to record whether anyone
   /// has set the selected frame on this stack yet. We only let recognizers
   /// change the frame if this is the first time GetSelectedFrame is called.
+  ///
+  /// Thread-safety:
+  /// This member is not protected by a mutex.
+  /// LLDB really only should have an opinion about the selected frame index
+  /// when a process stops, before control gets handed back to the user.
+  /// After that, it's up to them to change it whenever they feel like it.
+  /// If two parts of lldb decided they wanted to be in control of the selected
+  /// frame index on stop the right way to fix it would need to be some explicit
+  /// negotiation for who gets to control this.
   std::optional<uint32_t> m_selected_frame_idx;
 
   /// Protect access to m_selected_frame_idx. Always acquire after m_list_mutex

diff  --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h
index 6ede7fa301a82..688c056da2633 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -479,6 +479,11 @@ class Thread : public std::enable_shared_from_this<Thread>,
   bool SetSelectedFrameByIndexNoisily(uint32_t frame_idx,
                                       Stream &output_stream);
 
+  /// Resets the selected frame index of this object.
+  void ClearSelectedFrameIndex() {
+    return GetStackFrameList()->ClearSelectedFrameIndex();
+  }
+
   void SetDefaultFileAndLineToSelectedFrame() {
     GetStackFrameList()->SetDefaultFileAndLineToSelectedFrame();
   }

diff  --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index ff9e5fc12059e..3176852f0b724 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -4269,6 +4269,14 @@ bool Process::ProcessEventData::ShouldStop(Event *event_ptr,
         // appropriately. We also need to stop processing actions, since they
         // aren't expecting the target to be running.
 
+        // Clear the selected frame which may have been set as part of utility
+        // expressions that have been run as part of this stop. If we didn't
+        // clear this, then StopInfo::GetSuggestedStackFrameIndex would not
+        // take affect when we next called SelectMostRelevantFrame.
+        // PerformAction should not be the one setting a selected frame, instead
+        // this should be done via GetSuggestedStackFrameIndex.
+        thread_sp->ClearSelectedFrameIndex();
+
         // FIXME: we might have run.
         if (stop_info_sp->HasTargetRunSinceMe()) {
           SetRestarted(true);

diff  --git a/lldb/source/Target/StackFrameList.cpp b/lldb/source/Target/StackFrameList.cpp
index d83c90582ba9c..fa5d159c0c91a 100644
--- a/lldb/source/Target/StackFrameList.cpp
+++ b/lldb/source/Target/StackFrameList.cpp
@@ -944,3 +944,5 @@ size_t StackFrameList::GetStatus(Stream &strm, uint32_t first_frame,
   strm.IndentLess();
   return num_frames_displayed;
 }
+
+void StackFrameList::ClearSelectedFrameIndex() { m_selected_frame_idx.reset(); }