Skip to content

Commit 5489683

Browse files
Michael137tstellar
authored andcommitted
[lldb][Target] Clear selected frame index after a StopInfo::PerformAction (llvm#133078)
The motivation for this patch is that `StopInfo::GetSuggestedStackFrameIndex` would not take effect for `InstrumentationRuntimeStopInfo` (which we plan to implement in llvm#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 39572f5)
1 parent f490704 commit 5489683

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
/// The number of concrete frames fetched while filling the frame list. This

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
@@ -4257,6 +4257,14 @@ bool Process::ProcessEventData::ShouldStop(Event *event_ptr,
42574257
// appropriately. We also need to stop processing actions, since they
42584258
// aren't expecting the target to be running.
42594259

4260+
// Clear the selected frame which may have been set as part of utility
4261+
// expressions that have been run as part of this stop. If we didn't
4262+
// clear this, then StopInfo::GetSuggestedStackFrameIndex would not
4263+
// take affect when we next called SelectMostRelevantFrame.
4264+
// PerformAction should not be the one setting a selected frame, instead
4265+
// this should be done via GetSuggestedStackFrameIndex.
4266+
thread_sp->ClearSelectedFrameIndex();
4267+
42604268
// FIXME: we might have run.
42614269
if (stop_info_sp->HasTargetRunSinceMe()) {
42624270
SetRestarted(true);

lldb/source/Target/StackFrameList.cpp

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -936,3 +936,5 @@ size_t StackFrameList::GetStatus(Stream &strm, uint32_t first_frame,
936936
strm.IndentLess();
937937
return num_frames_displayed;
938938
}
939+
940+
void StackFrameList::ClearSelectedFrameIndex() { m_selected_frame_idx.reset(); }

0 commit comments

Comments
 (0)