Author: Michael Buch Date: 2025-09-22T16:51:20-07:00 New Revision: 54896838ca5e1f69f0f3040fe6847546db0463d4
URL: https://github.com/llvm/llvm-project/commit/54896838ca5e1f69f0f3040fe6847546db0463d4 DIFF: https://github.com/llvm/llvm-project/commit/54896838ca5e1f69f0f3040fe6847546db0463d4.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. (cherry picked from commit 39572f5e9168b1b44c2f9078494616fed8752086) 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 8a66296346f2d..8d455dc831df3 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; /// The number of concrete frames fetched while filling the frame list. This 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 2aa02fd58335e..a2fa88b569135 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -4257,6 +4257,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 16cd2548c2784..931b73b1e3633 100644 --- a/lldb/source/Target/StackFrameList.cpp +++ b/lldb/source/Target/StackFrameList.cpp @@ -936,3 +936,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(); } _______________________________________________ llvm-branch-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
