Make sure SelectMostRelevantFrame happens only when returning to the user

[jingham]

Summary

This is a user facing action, it is meant to focus the user's attention on
something other than the 0th frame when you stop somewhere where that's
helpful. For instance, stopping in pthread_kill after an assert will select
the assert frame.

This is not something you want to have happen internally in lldb, both
because internally you really don't want the selected frame changing out
from under you, and because the recognizers can do arbitrary work, and that
can cause deadlocks or other unexpected behavior.

However, it's not something that the current code does
explicitly after a stop has been delivered, it's expected to happen implicitly
as part of stopping. I changing this to call SMRF explicitly after a user
stop, but that got pretty ugly quickly.

So I added a bool to control whether to run this and audited all the current
uses to determine whether we're returning to the user or not.

This is currently a little ad hoc because there isn't a formal way to
determine why we're fetching events. It would certainly be cleaner to
pass around a "process control baton" that would hold the history of
who is currently running the target, and on whose behalf, so we know:
"we're stopping because we completed a user request", or "we're continuing
because we're running an expression for the user" vrs. "we're continuing
because we're running an expression to allocate memory for another expression",
etc.

But that's a much bigger project...

[JDevlieghere]

I looked at all the call sites and they all seemed reasonable in terms of doing work on behalf of the user or not and selecting the most relevant frame respectively. My only concern is that we now have a bunch of places where we're passing a blind true or false. While it's pretty obvious in this patch, it won't be so obvious when you're reading the same code in the future, let alone when someone inevitably copy-pastes it. I can think of two easy ways to improve readability:

1. Add an inline comment:

StackFrameSP frame_sp = GetSelectedFrame(/*select_most_relevant=*/false);

2. Add an enum to Frame:

enum SelectMostRelevant : bool {
  SelectMostRelevantFrame = true,
  DoNoSelectMostRelevantFrame = false,
};

StackFrameSP frame_sp = GetSelectedFrame(SelectMostRelevantFrame);

This is a trick that @dexonsmith thought me a long time ago. I personally like it the best because it makes things very explicit without the hassle of having to actually treat the value like an enum.

[jingham]

In D148863#4285797, @JDevlieghere wrote:

I looked at all the call sites and they all seemed reasonable in terms of doing work on behalf of the user or not and selecting the most relevant frame respectively. My only concern is that we now have a bunch of places where we're passing a blind true or false. While it's pretty obvious in this patch, it won't be so obvious when you're reading the same code in the future, let alone when someone inevitably copy-pastes it. I can think of two easy ways to improve readability:

1. Add an inline comment:

StackFrameSP frame_sp = GetSelectedFrame(/*select_most_relevant=*/false);

2. Add an enum to Frame:

enum SelectMostRelevant : bool {
  SelectMostRelevantFrame = true,
  DoNoSelectMostRelevantFrame = false,
};

StackFrameSP frame_sp = GetSelectedFrame(SelectMostRelevantFrame);

This is a trick that @dexonsmith thought me a long time ago. I personally like it the best because it makes things very explicit without the hassle of having to actually treat the value like an enum.

Oh, that's cute, I'll try that.

[mib]

It's unfortunate that we're not able to determine easily if the stop is public or not, so I guess this is the best we could do for now.

We could however greatly reduce the amount of changes in the patch by having select_most_relevant default to false (or the true either way). Then, to improve readability, we would only need to add an inline comment before the argument (GetSelectedFrame(/*select_most_relevant=*/true)) like Jonas suggested.

Other than that, this looks good to me :)

lldb/include/lldb/Target/Process.h

2171

typo I guess

[mib]

lldb/include/lldb/Target/StackFrameList.h

56

I'd suggested making select_most_relevant_frame = false by default.

lldb/include/lldb/Target/Thread.h

433

ditto

437–438

ditto

[jingham]

In D148863#4288186, @mib wrote:

It's unfortunate that we're not able to determine easily if the stop is public or not, so I guess this is the best we could do for now.

We could however greatly reduce the amount of changes in the patch by having select_most_relevant default to false (or the true either way). Then, to improve readability, we would only need to add an inline comment before the argument (GetSelectedFrame(/*select_most_relevant=*/true)) like Jonas suggested.

Other than that, this looks good to me :)

I thought about that (it would have made my job easier) but I don't want to have a default argument. I want people to have to think about who they are returning this too, and if it had a default argument it would be too easy to add it somewhere w/o thinking.

[jingham]

Adopted the "enum SelectMostRelevant" as Jonas suggested.

[JDevlieghere]

LGTM

lldb/include/lldb/Target/Process.h

/// \see Process::Notifications
   /// \return
   ///     Returns an error object.
   ///
   /// \see Thread:Resume()
   /// \see Thread:Step()
   /// \see Thread:Suspend()
   Status Resume();
 
+  /// Resume a process, and wait for it to stop.
   Status ResumeSynchronous(Stream *stream);
 
   /// Halts a running process.
   ///
   /// This function is not meant to be overridden by Process subclasses. If
   /// the process is successfully halted, a eStateStopped process event with
   /// GetInterrupted will be broadcast.  If false, we will halt the process
   /// with no events generated by the halt.

   lldb::StateType GetNextEvent(lldb::EventSP &event_sp);
 
   // Returns the process state when it is stopped. If specified, event_sp_ptr
   // is set to the event which triggered the stop. If wait_always = false, and
   // the process is already stopped, this function returns immediately. If the
   // process is hijacked and use_run_lock is true (the default), then this
   // function releases the run lock after the stop. Setting use_run_lock to
   // false will avoid this behavior.
+  // If we are waiting to stop that will return control to the user,
+  // then we also want to run SelectMostRelevantFrame, which is controlled
+  // by "select_most_relevant".
   lldb::StateType
   WaitForProcessToStop(const Timeout<std::micro> &timeout,
                        lldb::EventSP *event_sp_ptr = nullptr,
                        bool wait_always = true,
                        lldb::ListenerSP hijack_listener = lldb::ListenerSP(),
-                       Stream *stream = nullptr, bool use_run_lock = true);
+                       Stream *stream = nullptr, bool use_run_lock = true,
+                       SelectMostRelevant select_most_relevant =
+                           DoNoSelectMostRelevantFrame);
 
   uint32_t GetIOHandlerID() const { return m_iohandler_sync.GetValue(); }
 
   /// Waits for the process state to be running within a given msec timeout.
   ///
   /// The main purpose of this is to implement an interlock waiting for
   /// HandlePrivateEvent to push an IOHandler.
   ///

   ///     if needed.
   ///     Else this variable will be set to \b true or \b false to indicate if
   ///     the process
   ///     needs to have its process IOHandler popped.
   ///
   /// \return
   ///     \b true if the event describes a process state changed event, \b false
   ///     otherwise.
-  static bool HandleProcessStateChangedEvent(const lldb::EventSP &event_sp,
-                                             Stream *stream,
-                                             bool &pop_process_io_handler);
+  static bool
+  HandleProcessStateChangedEvent(const lldb::EventSP &event_sp, Stream *stream,
+                                 SelectMostRelevant select_most_relevant,
 
   Event *PeekAtStateChangedEvents();
 
   class ProcessEventHijacker {
   public:
     ProcessEventHijacker(Process &process, lldb::ListenerSP listener_sp)
         : m_process(process) {
       m_process.HijackProcessEvents(std::move(listener_sp));

lldb/include/lldb/Target/StackFrameList.h

public:
 
   /// Retrieve the stack frame with the given ID \p stack_id.
   lldb::StackFrameSP GetFrameWithStackID(const StackID &stack_id);
 
   /// Mark a stack frame as the currently selected frame and return its index.
   uint32_t SetSelectedFrame(lldb_private::StackFrame *frame);
 
   /// Get the currently selected frame index.
-  uint32_t GetSelectedFrameIndex();
+  /// We should only call SelectMostRelevantFrame if (a) the user hasn't already
+  /// selected a frame, and (b) if this really is a user facing
+  /// "GetSelectedFrame".  SMRF runs the frame recognizers which can do
+  /// arbitrary work that ends up being dangerous to do internally.  Also,
+  /// for most internal uses we don't actually want the frame changed by the
+  /// SMRF logic.  So unless this is in a command or SB API, you should
+  /// pass false here.
+  uint32_t
+  GetSelectedFrameIndex(SelectMostRelevant select_most_relevant_frame);
 
   /// Mark a stack frame as the currently selected frame using the frame index
   /// \p idx. Like \ref GetFrameAtIndex, invisible frames cannot be selected.
   bool SetSelectedFrameByIndex(uint32_t idx);
 
   /// If the current inline depth (i.e the number of invisible frames) is valid,
   /// subtract it from \p idx. Otherwise simply return \p idx.
   uint32_t GetVisibleStackFrameIndex(uint32_t idx) {

lldb/include/lldb/Target/Thread.h

Status JumpToLine(const FileSpec &file, uint32_t line,
                     bool can_leave_function, std::string *warnings = nullptr);
 
   virtual lldb::StackFrameSP GetFrameWithStackID(const StackID &stack_id) {
     if (stack_id.IsValid())
       return GetStackFrameList()->GetFrameWithStackID(stack_id);
     return lldb::StackFrameSP();
   }
 
-  uint32_t GetSelectedFrameIndex() {
-    return GetStackFrameList()->GetSelectedFrameIndex();
+  // Only pass true to select_most_relevant if you are fulfilling an explicit
+  // user request for GetSelectedFrameIndex.  The most relevant frame is only
+  // for showing to the user, and can do arbitrary work, so we don't want to
+  // call it internally.
+  uint32_t GetSelectedFrameIndex(SelectMostRelevant select_most_relevant) {
+    return GetStackFrameList()->GetSelectedFrameIndex(select_most_relevant);
   }
 
-  lldb::StackFrameSP GetSelectedFrame();
+  lldb::StackFrameSP
+  GetSelectedFrame(SelectMostRelevant select_most_relevant);
 
   uint32_t SetSelectedFrame(lldb_private::StackFrame *frame,
                             bool broadcast = false);
 
   bool SetSelectedFrameByIndex(uint32_t frame_idx, bool broadcast = false);
 
   bool SetSelectedFrameByIndexNoisily(uint32_t frame_idx,
                                       Stream &output_stream);

lldb/include/lldb/lldb-private-enumerations.h

case lldb_private::eVoteYes:
       Stream << "yes";
       return;
     }
     Stream << "invalid";
   }
 };
 }
 
+enum SelectMostRelevant : bool {
+  SelectMostRelevantFrame = true,
+  DoNoSelectMostRelevantFrame = false,
+};
+
 #endif // LLDB_LLDB_PRIVATE_ENUMERATIONS_H

lldb/source/API/SBThread.cpp

if (exe_ctx.HasThreadScope()) {
     Thread *thread = exe_ctx.GetThreadPtr();
 
     if (line == 0) {
       sb_error.SetErrorString("invalid line argument");
       return sb_error;
     }
 
     if (!frame_sp) {
-      frame_sp = thread->GetSelectedFrame();
+      // We don't want to run SelectMostRelevantFrame here, for instance if
+      // you called a sequence of StepOverUntil's you wouldn't want the
+      // frame changed out from under you because you stepped into a
+      // recognized frame.
+      frame_sp = thread->GetSelectedFrame(DoNoSelectMostRelevantFrame);
       if (!frame_sp)
         frame_sp = thread->GetStackFrameAtIndex(0);
     }
 
     SymbolContext frame_sc;
     if (!frame_sp) {
       sb_error.SetErrorString("no valid frames in thread to step");
       return sb_error;
lldb::SBFrame SBThread::GetSelectedFrame() {
   SBFrame sb_frame;
   StackFrameSP frame_sp;
   std::unique_lock<std::recursive_mutex> lock;
   ExecutionContext exe_ctx(m_opaque_sp.get(), lock);
 
   if (exe_ctx.HasThreadScope()) {
     Process::StopLocker stop_locker;
     if (stop_locker.TryLock(&exe_ctx.GetProcessPtr()->GetRunLock())) {
-      frame_sp = exe_ctx.GetThreadPtr()->GetSelectedFrame();
+      frame_sp =
+          exe_ctx.GetThreadPtr()->GetSelectedFrame(SelectMostRelevantFrame);
       sb_frame.SetFrameSP(frame_sp);
     }
   }
 
   return sb_frame;
 }
 
 lldb::SBFrame SBThread::SetSelectedFrame(uint32_t idx) {

lldb/source/Commands/CommandObjectFrame.cpp

public:
 
   ~CommandObjectFrameDiagnose() override = default;
 
   Options *GetOptions() override { return &m_options; }
 
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
     Thread *thread = m_exe_ctx.GetThreadPtr();
-    StackFrameSP frame_sp = thread->GetSelectedFrame();
+    StackFrameSP frame_sp = thread->GetSelectedFrame(SelectMostRelevantFrame);
 
     ValueObjectSP valobj_sp;
 
     if (m_options.address) {
       if (m_options.reg || m_options.offset) {
         result.AppendError(
             "`frame diagnose --address` is incompatible with other arguments.");
         return false;
protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
     // No need to check "thread" for validity as eCommandRequiresThread ensures
     // it is valid
     Thread *thread = m_exe_ctx.GetThreadPtr();
 
     uint32_t frame_idx = UINT32_MAX;
     if (m_options.relative_frame_offset) {
       // The one and only argument is a signed relative frame index
-      frame_idx = thread->GetSelectedFrameIndex();
+      frame_idx = thread->GetSelectedFrameIndex(SelectMostRelevantFrame);
       if (frame_idx == UINT32_MAX)
         frame_idx = 0;
 
       if (*m_options.relative_frame_offset < 0) {
         if (static_cast<int32_t>(frame_idx) >=
             -*m_options.relative_frame_offset)
           frame_idx += *m_options.relative_frame_offset;
         else {
if (m_options.relative_frame_offset) {
 
       if (command.GetArgumentCount() == 1) {
         if (command[0].ref().getAsInteger(0, frame_idx)) {
           result.AppendErrorWithFormat("invalid frame index argument '%s'.",
                                        command[0].c_str());
           return false;
         }
       } else if (command.GetArgumentCount() == 0) {
-        frame_idx = thread->GetSelectedFrameIndex();
+        frame_idx = thread->GetSelectedFrameIndex(SelectMostRelevantFrame);
         if (frame_idx == UINT32_MAX) {
           frame_idx = 0;
         }
       }
     }
 
     bool success = thread->SetSelectedFrameByIndexNoisily(
         frame_idx, result.GetOutputStream());
     if (success) {
-      m_exe_ctx.SetFrameSP(thread->GetSelectedFrame());
+      m_exe_ctx.SetFrameSP(thread->GetSelectedFrame(SelectMostRelevantFrame));
       result.SetStatus(eReturnStatusSuccessFinishResult);
     } else {
       result.AppendErrorWithFormat("Frame index (%u) out of range.\n",
                                    frame_idx);
     }
 
     return result.Succeeded();
   }

lldb/source/Commands/CommandObjectThread.cpp

if (m_step_type == eStepTypeInto) {
       new_plan_sp = thread->QueueThreadPlanForStepSingleInstruction(
           false, abort_other_plans, bool_stop_other_threads, new_plan_status);
     } else if (m_step_type == eStepTypeTraceOver) {
       new_plan_sp = thread->QueueThreadPlanForStepSingleInstruction(
           true, abort_other_plans, bool_stop_other_threads, new_plan_status);
     } else if (m_step_type == eStepTypeOut) {
       new_plan_sp = thread->QueueThreadPlanForStepOut(
           abort_other_plans, nullptr, false, bool_stop_other_threads, eVoteYes,
-          eVoteNoOpinion, thread->GetSelectedFrameIndex(), new_plan_status,
-          m_options.m_step_out_avoid_no_debug);
+          eVoteNoOpinion,
+          thread->GetSelectedFrameIndex(DoNoSelectMostRelevantFrame),
+          new_plan_status, m_options.m_step_out_avoid_no_debug);
     } else if (m_step_type == eStepTypeScripted) {
       new_plan_sp = thread->QueueThreadPlanForStepScripted(
           abort_other_plans, m_class_options.GetName().c_str(),
           m_class_options.GetStructuredData(), bool_stop_other_threads,
           new_plan_status);
     } else {
       result.AppendError("step type is not supported");
       return false;
if (command.startswith("-x")) {
       error = thread->UnwindInnermostExpression();
       if (!error.Success()) {
         result.AppendErrorWithFormat("Unwinding expression failed - %s.",
                                      error.AsCString());
       } else {
         bool success =
             thread->SetSelectedFrameByIndexNoisily(0, result.GetOutputStream());
         if (success) {
-          m_exe_ctx.SetFrameSP(thread->GetSelectedFrame());
+          m_exe_ctx.SetFrameSP(
+              thread->GetSelectedFrame(DoNoSelectMostRelevantFrame));
           result.SetStatus(eReturnStatusSuccessFinishResult);
         } else {
           result.AppendErrorWithFormat(
               "Could not select 0th frame after unwinding expression.");
         }
       }
       return result.Succeeded();
     }

lldb/source/Commands/CommandObjectType.cpp

bool DoExecute(llvm::StringRef command,
                  CommandReturnObject &result) override {
     TargetSP target_sp = GetDebugger().GetSelectedTarget();
     Thread *thread = GetDefaultThread();
     if (!thread) {
       result.AppendError("no default thread");
       return false;
     }
 
-    StackFrameSP frame_sp = thread->GetSelectedFrame();
+    StackFrameSP frame_sp =
+        thread->GetSelectedFrame(DoNoSelectMostRelevantFrame);
     ValueObjectSP result_valobj_sp;
     EvaluateExpressionOptions options;
     lldb::ExpressionResults expr_result = target_sp->EvaluateExpression(
         command, frame_sp.get(), result_valobj_sp, options);
     if (expr_result == eExpressionCompleted && result_valobj_sp) {
       result_valobj_sp =
           result_valobj_sp->GetQualifiedRepresentationIfAvailable(
               target_sp->GetPreferDynamicValue(),

lldb/source/Core/Debugger.cpp

if (got_state_changed) {
       StateType event_state =
           Process::ProcessEventData::GetStateFromEvent(event_sp.get());
       state_is_stopped = StateIsStoppedState(event_state, false);
     }
 
     // Display running state changes first before any STDIO
     if (got_state_changed && !state_is_stopped) {
       Process::HandleProcessStateChangedEvent(event_sp, output_stream_sp.get(),
+                                              DoNoSelectMostRelevantFrame,
                                               pop_process_io_handler);
     }
 
     // Now display STDOUT and STDERR
     FlushProcessOutput(*process_sp, got_stdout || got_state_changed,
                        got_stderr || got_state_changed);
 
     // Give structured data events an opportunity to display.
if (got_structured_data) {
           }
         }
       }
     }
 
     // Now display any stopped state changes after any STDIO
     if (got_state_changed && state_is_stopped) {
       Process::HandleProcessStateChangedEvent(event_sp, output_stream_sp.get(),
+                                              DoNoSelectMostRelevantFrame,
                                               pop_process_io_handler);
     }
 
     output_stream_sp->Flush();
     error_stream_sp->Flush();
 
     if (pop_process_io_handler)
       process_sp->PopProcessIOHandler();

lldb/source/Core/IOHandlerCursesGUI.cpp

void TreeDelegateUpdateSelection(TreeItem &root, int &selection_index,
 
     ThreadList &threads = process_sp->GetThreadList();
     std::lock_guard<std::recursive_mutex> guard(threads.GetMutex());
     ThreadSP selected_thread = threads.GetSelectedThread();
     size_t num_threads = threads.GetSize();
     for (size_t i = 0; i < num_threads; ++i) {
       ThreadSP thread = threads.GetThreadAtIndex(i);
       if (selected_thread->GetID() == thread->GetID()) {
-        selected_item = &root[i][thread->GetSelectedFrameIndex()];
+        selected_item =
+            &root[i][thread->GetSelectedFrameIndex(SelectMostRelevantFrame)];
         selection_index = selected_item->GetRowIndex();
         return;
       }
     }
   }
 
   bool TreeDelegateItemSelected(TreeItem &item) override { return false; }
 
MenuActionResult MenuDelegateAction(Menu &menu) override {
     case eMenuID_ThreadStepOut: {
       ExecutionContext exe_ctx =
           m_debugger.GetCommandInterpreter().GetExecutionContext();
       if (exe_ctx.HasThreadScope()) {
         Process *process = exe_ctx.GetProcessPtr();
         if (process && process->IsAlive() &&
             StateIsStoppedState(process->GetState(), true)) {
           Thread *thread = exe_ctx.GetThreadPtr();
-          uint32_t frame_idx = thread->GetSelectedFrameIndex();
+          uint32_t frame_idx =
+              thread->GetSelectedFrameIndex(SelectMostRelevantFrame);
           exe_ctx.GetThreadRef().StepOut(frame_idx);
         }
       }
     }
       return MenuActionResult::Handled;
 
     case eMenuID_ThreadStepOver: {
       ExecutionContext exe_ctx =
bool WindowDelegateDraw(Window &window, bool force) override {
     bool set_selected_line_to_pc = false;
 
     if (update_location) {
       const bool process_alive = process->IsAlive();
       bool thread_changed = false;
       if (process_alive) {
         thread = exe_ctx.GetThreadPtr();
         if (thread) {
-          frame_sp = thread->GetSelectedFrame();
+          frame_sp = thread->GetSelectedFrame(SelectMostRelevantFrame);
           auto tid = thread->GetID();
           thread_changed = tid != m_tid;
           m_tid = tid;
         } else {
           if (m_tid != LLDB_INVALID_THREAD_ID) {
             thread_changed = true;
             m_tid = LLDB_INVALID_THREAD_ID;
           }
HandleCharResult WindowDelegateHandleChar(Window &window, int c) override {
     case 'f':
       // 'f' == step out (finish)
       {
         ExecutionContext exe_ctx =
             m_debugger.GetCommandInterpreter().GetExecutionContext();
         if (exe_ctx.HasThreadScope() &&
             StateIsStoppedState(exe_ctx.GetProcessRef().GetState(), true)) {
           Thread *thread = exe_ctx.GetThreadPtr();
-          uint32_t frame_idx = thread->GetSelectedFrameIndex();
+          uint32_t frame_idx =
+              thread->GetSelectedFrameIndex(SelectMostRelevantFrame);
           exe_ctx.GetThreadRef().StepOut(frame_idx);
         }
       }
       return eKeyHandled;
 
     case 'n': // 'n' == step over
     case 'N': // 'N' == step over instruction
     {
HandleCharResult WindowDelegateHandleChar(Window &window, int c) override {
 
     case 'u': // 'u' == frame up
     case 'd': // 'd' == frame down
     {
       ExecutionContext exe_ctx =
           m_debugger.GetCommandInterpreter().GetExecutionContext();
       if (exe_ctx.HasThreadScope()) {
         Thread *thread = exe_ctx.GetThreadPtr();
-        uint32_t frame_idx = thread->GetSelectedFrameIndex();
+        uint32_t frame_idx =
+            thread->GetSelectedFrameIndex(SelectMostRelevantFrame);
         if (frame_idx == UINT32_MAX)
           frame_idx = 0;
         if (c == 'u' && frame_idx + 1 < thread->GetStackFrameCount())
           ++frame_idx;
         else if (c == 'd' && frame_idx > 0)
           --frame_idx;
         if (thread->SetSelectedFrameByIndex(frame_idx, true))
-          exe_ctx.SetFrameSP(thread->GetSelectedFrame());
+          exe_ctx.SetFrameSP(thread->GetSelectedFrame(SelectMostRelevantFrame));
       }
     }
       return eKeyHandled;
 
     case 'h':
       window.CreateHelpSubwindow();
       return eKeyHandled;
 

lldb/source/Core/ValueObject.cpp

if (process_sp) {
       }
 
       if (thread_sp) {
         m_exe_ctx_ref.SetThreadSP(thread_sp);
 
         StackFrameSP frame_sp(exe_ctx.GetFrameSP());
         if (!frame_sp) {
           if (use_selected)
-            frame_sp = thread_sp->GetSelectedFrame();
+            frame_sp = thread_sp->GetSelectedFrame(DoNoSelectMostRelevantFrame);
         }
         if (frame_sp)
           m_exe_ctx_ref.SetFrameSP(frame_sp);
       }
     }
   }
 }
 

lldb/source/Expression/REPL.cpp

void REPL::IOHandlerInputComplete(IOHandler &io_handler, std::string &code) {
   } else {
     Debugger &debugger = m_target.GetDebugger();
     CommandInterpreter &ci = debugger.GetCommandInterpreter();
     extra_line = ci.GetSpaceReplPrompts();
 
     ExecutionContext exe_ctx(m_target.GetProcessSP()
                                  ->GetThreadList()
                                  .GetSelectedThread()
-                                 ->GetSelectedFrame()
+                                 ->GetSelectedFrame(DoNoSelectMostRelevantFrame)
                                  .get());
 
     lldb::ProcessSP process_sp(exe_ctx.GetProcessSP());
 
     if (code[0] == ':') {
       // Meta command
       // Strip the ':'
       code.erase(0, 1);
if (code[0] == ':') {
       }
 
       // Unwind any expression we might have been running in case our REPL
       // expression crashed and the user was looking around
       if (m_dedicated_repl_mode) {
         Thread *thread = exe_ctx.GetThreadPtr();
         if (thread && thread->UnwindInnermostExpression().Success()) {
           thread->SetSelectedFrameByIndex(0, false);
-          exe_ctx.SetFrameSP(thread->GetSelectedFrame());
+          exe_ctx.SetFrameSP(
+              thread->GetSelectedFrame(DoNoSelectMostRelevantFrame));
         }
       }
 
       const bool colorize_err = error_sp->GetFile().GetIsTerminalWithColors();
 
       EvaluateExpressionOptions expr_options = m_expr_options;
       expr_options.SetCoerceToId(m_varobj_options.use_objc);
       expr_options.SetKeepInMemory(true);

lldb/source/Plugins/InstrumentationRuntime/ASan/InstrumentationRuntimeASan.cpp

 
 StructuredData::ObjectSP InstrumentationRuntimeASan::RetrieveReportData() {
   ProcessSP process_sp = GetProcessSP();
   if (!process_sp)
     return StructuredData::ObjectSP();
 
   ThreadSP thread_sp =
       process_sp->GetThreadList().GetExpressionExecutionThread();
-  StackFrameSP frame_sp = thread_sp->GetSelectedFrame();
+  StackFrameSP frame_sp =
+      thread_sp->GetSelectedFrame(DoNoSelectMostRelevantFrame);
 
   if (!frame_sp)
     return StructuredData::ObjectSP();
 
   EvaluateExpressionOptions options;
   options.SetUnwindOnError(true);
   options.SetTryAllThreads(true);
   options.SetStopOthers(true);

lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp

 StructuredData::ObjectSP
 InstrumentationRuntimeMainThreadChecker::RetrieveReportData(
     ExecutionContextRef exe_ctx_ref) {
   ProcessSP process_sp = GetProcessSP();
   if (!process_sp)
     return StructuredData::ObjectSP();
 
   ThreadSP thread_sp = exe_ctx_ref.GetThreadSP();
-  StackFrameSP frame_sp = thread_sp->GetSelectedFrame();
+  StackFrameSP frame_sp =
+      thread_sp->GetSelectedFrame(DoNoSelectMostRelevantFrame);
   ModuleSP runtime_module_sp = GetRuntimeModuleSP();
   Target &target = process_sp->GetTarget();
 
   if (!frame_sp)
     return StructuredData::ObjectSP();
 
   RegisterContextSP regctx_sp = frame_sp->GetRegisterContext();
   if (!regctx_sp)

lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp

 
 StructuredData::ObjectSP InstrumentationRuntimeTSan::RetrieveReportData(
     ExecutionContextRef exe_ctx_ref) {
   ProcessSP process_sp = GetProcessSP();
   if (!process_sp)
     return StructuredData::ObjectSP();
 
   ThreadSP thread_sp = exe_ctx_ref.GetThreadSP();
-  StackFrameSP frame_sp = thread_sp->GetSelectedFrame();
+  StackFrameSP frame_sp =
+      thread_sp->GetSelectedFrame(DoNoSelectMostRelevantFrame);
 
   if (!frame_sp)
     return StructuredData::ObjectSP();
 
   EvaluateExpressionOptions options;
   options.SetUnwindOnError(true);
   options.SetTryAllThreads(true);
   options.SetStopOthers(true);

lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp

 
 StructuredData::ObjectSP InstrumentationRuntimeUBSan::RetrieveReportData(
     ExecutionContextRef exe_ctx_ref) {
   ProcessSP process_sp = GetProcessSP();
   if (!process_sp)
     return StructuredData::ObjectSP();
 
   ThreadSP thread_sp = exe_ctx_ref.GetThreadSP();
-  StackFrameSP frame_sp = thread_sp->GetSelectedFrame();
+  StackFrameSP frame_sp =
+      thread_sp->GetSelectedFrame(DoNoSelectMostRelevantFrame);
   ModuleSP runtime_module_sp = GetRuntimeModuleSP();
   Target &target = process_sp->GetTarget();
 
   if (!frame_sp)
     return StructuredData::ObjectSP();
 
   StreamFileSP Stream = target.GetDebugger().GetOutputStreamSP();
 

lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp

bool AppleObjCRuntime::GetObjectDescription(Stream &strm, Value &value,
 
   if (exe_ctx.GetFramePtr() == nullptr) {
     Thread *thread = exe_ctx.GetThreadPtr();
     if (thread == nullptr) {
       exe_ctx.SetThreadSP(process->GetThreadList().GetSelectedThread());
       thread = exe_ctx.GetThreadPtr();
     }
     if (thread) {
-      exe_ctx.SetFrameSP(thread->GetSelectedFrame());
+      exe_ctx.SetFrameSP(thread->GetSelectedFrame(DoNoSelectMostRelevantFrame));
     }
   }
 
   // Now we're ready to call the function:
 
   DiagnosticManager diagnostics;
   lldb::addr_t wrapper_struct_addr = LLDB_INVALID_ADDRESS;
 

lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp

HistoryThreads MemoryHistoryASan::GetHistoryThreads(lldb::addr_t address) {
   if (!process_sp)
     return result;
 
   ThreadSP thread_sp =
       process_sp->GetThreadList().GetExpressionExecutionThread();
   if (!thread_sp)
     return result;
 
-  StackFrameSP frame_sp = thread_sp->GetSelectedFrame();
+  StackFrameSP frame_sp =
+      thread_sp->GetSelectedFrame(DoNoSelectMostRelevantFrame);
   if (!frame_sp)
     return result;
 
   ExecutionContext exe_ctx(frame_sp);
   ValueObjectSP return_value_sp;
   StreamString expr;
   Status eval_error;
   expr.Printf(memory_history_asan_command_format, address, address);

lldb/source/Target/ExecutionContext.cpp

ExecutionContext::ExecutionContext(Target *t,
     : m_target_sp(), m_process_sp(), m_thread_sp(), m_frame_sp() {
   if (t) {
     m_target_sp = t->shared_from_this();
     if (fill_current_process_thread_frame) {
       m_process_sp = t->GetProcessSP();
       if (m_process_sp) {
         m_thread_sp = m_process_sp->GetThreadList().GetSelectedThread();
         if (m_thread_sp)
-          m_frame_sp = m_thread_sp->GetSelectedFrame();
+          m_frame_sp =
+              m_thread_sp->GetSelectedFrame(DoNoSelectMostRelevantFrame);
       }
     }
   }
 }
 
 ExecutionContext::ExecutionContext(Process *process, Thread *thread,
                                    StackFrame *frame)
     : m_target_sp(), m_process_sp(), m_thread_sp(), m_frame_sp() {
if (target_sp) {
                 StateIsStoppedState(process_sp->GetState(), true)) {
               lldb::ThreadSP thread_sp(
                   process_sp->GetThreadList().GetSelectedThread());
               if (!thread_sp)
                 thread_sp = process_sp->GetThreadList().GetThreadAtIndex(0);
 
               if (thread_sp) {
                 SetThreadSP(thread_sp);
-                lldb::StackFrameSP frame_sp(thread_sp->GetSelectedFrame());
+                lldb::StackFrameSP frame_sp(
+                    thread_sp->GetSelectedFrame(DoNoSelectMostRelevantFrame));
                 if (!frame_sp)
                   frame_sp = thread_sp->GetStackFrameAtIndex(0);
                 if (frame_sp)
                   SetFrameSP(frame_sp);
               }
             }
           }
         }

lldb/source/Target/Platform.cpp

lldb::ProcessSP Platform::DoConnectProcess(llvm::StringRef connect_url,
   }
 
   if (synchronous) {
     EventSP event_sp;
     process_sp->WaitForProcessToStop(std::nullopt, &event_sp, true, listener_sp,
                                      nullptr);
     process_sp->RestoreProcessEvents();
     bool pop_process_io_handler = false;
-    Process::HandleProcessStateChangedEvent(event_sp, stream,
-                                            pop_process_io_handler);
+    // This is a user-level stop, so we allow recognizers to select frames.
+    Process::HandleProcessStateChangedEvent(
+        event_sp, stream, SelectMostRelevantFrame, pop_process_io_handler);
   }
 
   return process_sp;
 }
 
 size_t Platform::ConnectToWaitingProcesses(lldb_private::Debugger &debugger,
                                            lldb_private::Status &error) {
   error.Clear();

lldb/source/Target/Process.cpp

LLDB_LOG(
         "waited from m_iohandler_sync to change from {0}. New value is {1}.",
         iohandler_id, *Result);
   } else {
     LLDB_LOG(log, "timed out waiting for m_iohandler_sync to change from {0}.",
              iohandler_id);
   }
 }
 
-StateType Process::WaitForProcessToStop(const Timeout<std::micro> &timeout,
-                                        EventSP *event_sp_ptr, bool wait_always,
-                                        ListenerSP hijack_listener_sp,
-                                        Stream *stream, bool use_run_lock) {
+StateType Process::WaitForProcessToStop(
+    const Timeout<std::micro> &timeout, EventSP *event_sp_ptr, bool wait_always,
+    ListenerSP hijack_listener_sp, Stream *stream, bool use_run_lock,
+    SelectMostRelevant select_most_relevant) {
   // We can't just wait for a "stopped" event, because the stopped event may
   // have restarted the target. We have to actually check each event, and in
   // the case of a stopped event check the restarted flag on the event.
   if (event_sp_ptr)
     event_sp_ptr->reset();
   StateType state = GetState();
   // If we are exited or detached, we won't ever get back to any other valid
   // state...
StateType Process::WaitForProcessToStop(
 
   while (state != eStateInvalid) {
     EventSP event_sp;
     state = GetStateChangedEvents(event_sp, timeout, hijack_listener_sp);
     if (event_sp_ptr && event_sp)
       *event_sp_ptr = event_sp;
 
     bool pop_process_io_handler = (hijack_listener_sp.get() != nullptr);
-    Process::HandleProcessStateChangedEvent(event_sp, stream,
-                                            pop_process_io_handler);
+    Process::HandleProcessStateChangedEvent(
+        event_sp, stream, select_most_relevant, pop_process_io_handler);
 
     switch (state) {
     case eStateCrashed:
     case eStateDetached:
     case eStateExited:
     case eStateUnloaded:
       // We need to toggle the run lock as this won't get done in
       // SetPublicState() if the process is hijacked.
case eStateStopped:
       }
     default:
       continue;
     }
   }
   return state;
 }
 
-bool Process::HandleProcessStateChangedEvent(const EventSP &event_sp,
-                                             Stream *stream,
-                                             bool &pop_process_io_handler) {
+bool Process::HandleProcessStateChangedEvent(
+    const EventSP &event_sp, Stream *stream,
+    SelectMostRelevant select_most_relevant,
   const bool handle_pop = pop_process_io_handler;
 
   pop_process_io_handler = false;
   ProcessSP process_sp =
       Process::ProcessEventData::GetProcessFromEvent(event_sp.get());
 
   if (!process_sp)
     return false;
if (Process::ProcessEventData::GetRestartedFromEvent(event_sp.get())) {
         if (debugger.GetTargetList().GetSelectedTarget().get() ==
             &process_sp->GetTarget()) {
           ThreadSP thread_sp = process_sp->GetThreadList().GetSelectedThread();
 
           if (!thread_sp || !thread_sp->IsValid())
             return false;
 
           const bool only_threads_with_stop_reason = true;
-          const uint32_t start_frame = thread_sp->GetSelectedFrameIndex();
+          const uint32_t start_frame =
+              thread_sp->GetSelectedFrameIndex(select_most_relevant);
           const uint32_t num_frames = 1;
           const uint32_t num_frames_with_source = 1;
           const bool stop_format = true;
 
           process_sp->GetStatus(*stream);
           process_sp->GetThreadStatus(*stream, only_threads_with_stop_reason,
                                       start_frame, num_frames,
                                       num_frames_with_source,
Status Process::ResumeSynchronous(Stream *stream) {
 
   ListenerSP listener_sp(
       Listener::MakeListener(ResumeSynchronousHijackListenerName.data()));
   HijackProcessEvents(listener_sp);
 
   Status error = PrivateResume();
   if (error.Success()) {
     StateType state =
-        WaitForProcessToStop(std::nullopt, nullptr, true, listener_sp, stream);
+        WaitForProcessToStop(std::nullopt, nullptr, true, listener_sp, stream,
+                             true /* use_run_lock */, SelectMostRelevantFrame);
     const bool must_be_alive =
         false; // eStateExited is ok, so this must be false
     if (!StateIsStoppedState(state, must_be_alive))
       error.SetErrorStringWithFormat(
           "process not in stopped state after synchronous resume: %s",
           StateAsCString(state));
   } else {
     // Undo running state change
if (error.Success()) {
 
     // We successfully loaded a core file, now pretend we stopped so we can
     // show all of the threads in the core file and explore the crashed state.
     SetPrivateState(eStateStopped);
 
     // Wait for a stopped event since we just posted one above...
     lldb::EventSP event_sp;
     StateType state =
-        WaitForProcessToStop(std::nullopt, &event_sp, true, listener_sp);
+        WaitForProcessToStop(std::nullopt, &event_sp, true, listener_sp,
+                             nullptr, true, SelectMostRelevantFrame);
 
     if (!StateIsStoppedState(state, false)) {
       Log *log = GetLog(LLDBLog::Process);
       LLDB_LOGF(log, "Process::Halt() failed to stop, state is: %s",
                 StateAsCString(state));
       error.SetErrorString(
           "Did not get stopped event after loading the core file.");
     }
if (m_public_state.GetValue() == eStateAttaching) {
     // attach will be waiting for this event...
     RestoreProcessEvents();
     SetExitStatus(SIGKILL, "Cancelled async attach.");
     Destroy(false);
     return Status();
   }
 
   // Wait for the process halt timeout seconds for the process to stop.
-  StateType state =
-      WaitForProcessToStop(GetInterruptTimeout(), &event_sp, true,
-                           halt_listener_sp, nullptr, use_run_lock);
+  // If we are going to use the run lock, that means we're stopping out to the
+  // user, so we should also select the most relevant frame.
+  SelectMostRelevant select_most_relevant =
+      use_run_lock ? SelectMostRelevantFrame : DoNoSelectMostRelevantFrame;
+  StateType state = WaitForProcessToStop(GetInterruptTimeout(), &event_sp, true,
+                                         halt_listener_sp, nullptr,
+                                         use_run_lock, select_most_relevant);
   RestoreProcessEvents();
 
   if (state == eStateInvalid || !event_sp) {
     // We timed out and didn't get a stop event...
     return Status("Halt timed out. State = %s", StateAsCString(GetState()));
   }
 
   BroadcastEvent(event_sp);
if (m_private_state.GetValue() != eStateStopped) {
     diagnostic_manager.PutString(
         eDiagnosticSeverityError,
         "RunThreadPlan called while the private state was not stopped.");
     return eExpressionSetupError;
   }
 
   // Save the thread & frame from the exe_ctx for restoration after we run
   const uint32_t thread_idx_id = thread->GetIndexID();
-  StackFrameSP selected_frame_sp = thread->GetSelectedFrame();
+  StackFrameSP selected_frame_sp =
+      thread->GetSelectedFrame(DoNoSelectMostRelevantFrame);
   if (!selected_frame_sp) {
     thread->SetSelectedFrame(nullptr);
-    selected_frame_sp = thread->GetSelectedFrame();
+    selected_frame_sp = thread->GetSelectedFrame(DoNoSelectMostRelevantFrame);
     if (!selected_frame_sp) {
       diagnostic_manager.Printf(
           eDiagnosticSeverityError,
           "RunThreadPlan called without a selected frame on thread %d",
           thread_idx_id);
       return eExpressionSetupError;
     }
   }
Process::RunThreadPlan(ExecutionContext &exe_ctx,
   // well.
 
   lldb::ThreadSP selected_thread_sp = GetThreadList().GetSelectedThread();
 
   uint32_t selected_tid;
   StackID selected_stack_id;
   if (selected_thread_sp) {
     selected_tid = selected_thread_sp->GetIndexID();
-    selected_stack_id = selected_thread_sp->GetSelectedFrame()->GetStackID();
+    selected_stack_id =
+        selected_thread_sp->GetSelectedFrame(DoNoSelectMostRelevantFrame)
+            ->GetStackID();
   } else {
     selected_tid = LLDB_INVALID_THREAD_ID;
   }
 
   HostThread backup_private_state_thread;
   lldb::StateType old_state = eStateInvalid;
   lldb::ThreadPlanSP stopper_base_plan_sp;
 

lldb/source/Target/StackFrameList.cpp

if (StackFrameSP most_relevant_frame_sp =
     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!");
   }
 }
 
-uint32_t StackFrameList::GetSelectedFrameIndex() {
+uint32_t StackFrameList::GetSelectedFrameIndex(
+    SelectMostRelevant select_most_relevant) {
   std::lock_guard<std::recursive_mutex> guard(m_mutex);
-  if (!m_selected_frame_idx)
+  if (!m_selected_frame_idx && select_most_relevant)
     SelectMostRelevantFrame();
-  if (!m_selected_frame_idx)
+  if (!m_selected_frame_idx) {
+    // If we aren't selecting the most relevant frame, and the selected frame
+    // isn't set, then don't force a selection here, just return 0.
+    if (!select_most_relevant)
+      return 0;
     m_selected_frame_idx = 0;
+  }
   return *m_selected_frame_idx;
 }
 
 uint32_t StackFrameList::SetSelectedFrame(lldb_private::StackFrame *frame) {
   std::lock_guard<std::recursive_mutex> guard(m_mutex);
   const_iterator pos;
   const_iterator begin = m_frames.begin();
   const_iterator end = m_frames.end();
if (frame_sp) {
     return true;
   } else
     return false;
 }
 
 void StackFrameList::SetDefaultFileAndLineToSelectedFrame() {
   if (m_thread.GetID() ==
       m_thread.GetProcess()->GetThreadList().GetSelectedThread()->GetID()) {
-    StackFrameSP frame_sp(GetFrameAtIndex(GetSelectedFrameIndex()));
+    StackFrameSP frame_sp(
+        GetFrameAtIndex(GetSelectedFrameIndex(DoNoSelectMostRelevantFrame)));
     if (frame_sp) {
       SymbolContext sc = frame_sp->GetSymbolContext(eSymbolContextLineEntry);
       if (sc.line_entry.file)
         m_thread.CalculateTarget()->GetSourceManager().SetDefaultFileAndLine(
             sc.line_entry.file, sc.line_entry.line);
     }
   }
 }
size_t StackFrameList::GetStatus(Stream &strm, uint32_t first_frame,
   uint32_t last_frame;
 
   // Don't let the last frame wrap around...
   if (num_frames == UINT32_MAX)
     last_frame = UINT32_MAX;
   else
     last_frame = first_frame + num_frames;
 
-  StackFrameSP selected_frame_sp = m_thread.GetSelectedFrame();
+  StackFrameSP selected_frame_sp =
+      m_thread.GetSelectedFrame(DoNoSelectMostRelevantFrame);
   const char *unselected_marker = nullptr;
   std::string buffer;
   if (selected_frame_marker) {
     size_t len = strlen(selected_frame_marker);
     buffer.insert(buffer.begin(), len, ' ');
     unselected_marker = buffer.c_str();
   }
   const char *marker = nullptr;

lldb/source/Target/StopInfo.cpp

if (!description) {
     return ValueObjectSP();
   }
 
   ThreadSP thread_sp = stop_info_sp->GetThread();
   if (!thread_sp) {
     return ValueObjectSP();
   }
 
-  StackFrameSP frame_sp = thread_sp->GetSelectedFrame();
+  StackFrameSP frame_sp =
+      thread_sp->GetSelectedFrame(DoNoSelectMostRelevantFrame);
 
   if (!frame_sp) {
     return ValueObjectSP();
   }
 
   const char address_string[] = "address=";
 
   const char *address_loc = strstr(description, address_string);

lldb/source/Target/Target.cpp

if (hijack_listener_sp)
       process_sp->HijackProcessEvents(hijack_listener_sp);
     error = process_sp->Attach(attach_info);
   }
 
   if (error.Success() && process_sp) {
     if (async) {
       process_sp->RestoreProcessEvents();
     } else {
-      state = process_sp->WaitForProcessToStop(std::nullopt, nullptr, false,
-                                               attach_info.GetHijackListener(),
-                                               stream);
+      // We are stopping all the way out to the user, so update selected frames.
+      state = process_sp->WaitForProcessToStop(
+          std::nullopt, nullptr, false, attach_info.GetHijackListener(), stream,
+          true, SelectMostRelevantFrame);
       process_sp->RestoreProcessEvents();
 
       if (state != eStateStopped) {
         const char *exit_desc = process_sp->GetExitDescription();
         if (exit_desc)
           error.SetErrorStringWithFormat("%s", exit_desc);
         else
           error.SetErrorString(

lldb/source/Target/Thread.cpp

 }
 
 void Thread::BroadcastSelectedFrameChange(StackID &new_frame_id) {
   if (EventTypeHasListeners(eBroadcastBitSelectedFrameChanged))
     BroadcastEvent(eBroadcastBitSelectedFrameChanged,
                    new ThreadEventData(this->shared_from_this(), new_frame_id));
 }
 
-lldb::StackFrameSP Thread::GetSelectedFrame() {
+lldb::StackFrameSP
+Thread::GetSelectedFrame(SelectMostRelevant select_most_relevant) {
   StackFrameListSP stack_frame_list_sp(GetStackFrameList());
   StackFrameSP frame_sp = stack_frame_list_sp->GetFrameAtIndex(
-      stack_frame_list_sp->GetSelectedFrameIndex());
+      stack_frame_list_sp->GetSelectedFrameIndex(select_most_relevant));
   FrameSelectedCallback(frame_sp.get());
   return frame_sp;
 }
 
 uint32_t Thread::SetSelectedFrame(lldb_private::StackFrame *frame,
                                   bool broadcast) {
   uint32_t ret_value = GetStackFrameList()->SetSelectedFrame(frame);
   if (broadcast)
if (frame_sp) {
     return false;
 }
 
 bool Thread::SetSelectedFrameByIndexNoisily(uint32_t frame_idx,
                                             Stream &output_stream) {
   const bool broadcast = true;
   bool success = SetSelectedFrameByIndex(frame_idx, broadcast);
   if (success) {
-    StackFrameSP frame_sp = GetSelectedFrame();
+    StackFrameSP frame_sp = GetSelectedFrame(DoNoSelectMostRelevantFrame);
     if (frame_sp) {
       bool already_shown = false;
       SymbolContext frame_sc(
           frame_sp->GetSymbolContext(eSymbolContextLineEntry));
       if (GetProcess()->GetTarget().GetDebugger().GetUseExternalEditor() &&
           frame_sc.line_entry.file && frame_sc.line_entry.line != 0) {
         already_shown = Host::OpenFileInExternalEditor(
             frame_sc.line_entry.file, frame_sc.line_entry.line);