Skip to content

Commit 39572f5

Browse files
authored
[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 #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.
1 parent 3ea39a5 commit 39572f5

File tree

4 files changed

+27
-0
lines changed

4 files changed

+27
-0
lines changed

lldb/include/lldb/Target/StackFrameList.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,9 @@ class StackFrameList {
4646
/// Mark a stack frame as the currently selected frame and return its index.
4747
uint32_t SetSelectedFrame(lldb_private::StackFrame *frame);
4848

49+
/// Resets the selected frame index of this object.
50+
void ClearSelectedFrameIndex();
51+
4952
/// Get the currently selected frame index.
5053
/// We should only call SelectMostRelevantFrame if (a) the user hasn't already
5154
/// selected a frame, and (b) if this really is a user facing
@@ -172,6 +175,15 @@ class StackFrameList {
172175
/// The currently selected frame. An optional is used to record whether anyone
173176
/// has set the selected frame on this stack yet. We only let recognizers
174177
/// change the frame if this is the first time GetSelectedFrame is called.
178+
///
179+
/// Thread-safety:
180+
/// This member is not protected by a mutex.
181+
/// LLDB really only should have an opinion about the selected frame index
182+
/// when a process stops, before control gets handed back to the user.
183+
/// After that, it's up to them to change it whenever they feel like it.
184+
/// If two parts of lldb decided they wanted to be in control of the selected
185+
/// frame index on stop the right way to fix it would need to be some explicit
186+
/// negotiation for who gets to control this.
175187
std::optional<uint32_t> m_selected_frame_idx;
176188

177189
/// Protect access to m_selected_frame_idx. Always acquire after m_list_mutex

lldb/include/lldb/Target/Thread.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -479,6 +479,11 @@ class Thread : public std::enable_shared_from_this<Thread>,
479479
bool SetSelectedFrameByIndexNoisily(uint32_t frame_idx,
480480
Stream &output_stream);
481481

482+
/// Resets the selected frame index of this object.
483+
void ClearSelectedFrameIndex() {
484+
return GetStackFrameList()->ClearSelectedFrameIndex();
485+
}
486+
482487
void SetDefaultFileAndLineToSelectedFrame() {
483488
GetStackFrameList()->SetDefaultFileAndLineToSelectedFrame();
484489
}

lldb/source/Target/Process.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4269,6 +4269,14 @@ bool Process::ProcessEventData::ShouldStop(Event *event_ptr,
42694269
// appropriately. We also need to stop processing actions, since they
42704270
// aren't expecting the target to be running.
42714271

4272+
// Clear the selected frame which may have been set as part of utility
4273+
// expressions that have been run as part of this stop. If we didn't
4274+
// clear this, then StopInfo::GetSuggestedStackFrameIndex would not
4275+
// take affect when we next called SelectMostRelevantFrame.
4276+
// PerformAction should not be the one setting a selected frame, instead
4277+
// this should be done via GetSuggestedStackFrameIndex.
4278+
thread_sp->ClearSelectedFrameIndex();
4279+
42724280
// FIXME: we might have run.
42734281
if (stop_info_sp->HasTargetRunSinceMe()) {
42744282
SetRestarted(true);

lldb/source/Target/StackFrameList.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -944,3 +944,5 @@ size_t StackFrameList::GetStatus(Stream &strm, uint32_t first_frame,
944944
strm.IndentLess();
945945
return num_frames_displayed;
946946
}
947+
948+
void StackFrameList::ClearSelectedFrameIndex() { m_selected_frame_idx.reset(); }

0 commit comments

Comments
 (0)