jingham created this revision.
jingham added reviewers: JDevlieghere, bulbazord, mib.
Herald added a project: All.
jingham requested review of this revision.
Herald added a project: LLDB.
Herald added a subscriber: lldb-commits.

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...


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148863

Files:
  lldb/include/lldb/Target/Process.h
  lldb/include/lldb/Target/StackFrameList.h
  lldb/include/lldb/Target/Thread.h
  lldb/source/API/SBThread.cpp
  lldb/source/Commands/CommandObjectFrame.cpp
  lldb/source/Commands/CommandObjectThread.cpp
  lldb/source/Commands/CommandObjectType.cpp
  lldb/source/Core/Debugger.cpp
  lldb/source/Core/IOHandlerCursesGUI.cpp
  lldb/source/Core/ValueObject.cpp
  lldb/source/Expression/REPL.cpp
  lldb/source/Plugins/InstrumentationRuntime/ASan/InstrumentationRuntimeASan.cpp
  
lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
  lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
  
lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
  lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
  lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
  lldb/source/Target/ExecutionContext.cpp
  lldb/source/Target/Platform.cpp
  lldb/source/Target/Process.cpp
  lldb/source/Target/StackFrameList.cpp
  lldb/source/Target/StopInfo.cpp
  lldb/source/Target/Target.cpp
  lldb/source/Target/Thread.cpp

Index: lldb/source/Target/Thread.cpp
===================================================================
--- lldb/source/Target/Thread.cpp
+++ lldb/source/Target/Thread.cpp
@@ -263,10 +263,10 @@
                    new ThreadEventData(this->shared_from_this(), new_frame_id));
 }
 
-lldb::StackFrameSP Thread::GetSelectedFrame() {
+lldb::StackFrameSP Thread::GetSelectedFrame(bool 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;
 }
@@ -297,7 +297,7 @@
   const bool broadcast = true;
   bool success = SetSelectedFrameByIndex(frame_idx, broadcast);
   if (success) {
-    StackFrameSP frame_sp = GetSelectedFrame();
+    StackFrameSP frame_sp = GetSelectedFrame(false);
     if (frame_sp) {
       bool already_shown = false;
       SymbolContext frame_sc(
Index: lldb/source/Target/Target.cpp
===================================================================
--- lldb/source/Target/Target.cpp
+++ lldb/source/Target/Target.cpp
@@ -3358,9 +3358,10 @@
     if (async) {
       process_sp->RestoreProcessEvents();
     } else {
+      // 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);
+                                               stream, true, true);
       process_sp->RestoreProcessEvents();
 
       if (state != eStateStopped) {
Index: lldb/source/Target/StopInfo.cpp
===================================================================
--- lldb/source/Target/StopInfo.cpp
+++ lldb/source/Target/StopInfo.cpp
@@ -1464,7 +1464,7 @@
     return ValueObjectSP();
   }
 
-  StackFrameSP frame_sp = thread_sp->GetSelectedFrame();
+  StackFrameSP frame_sp = thread_sp->GetSelectedFrame(false);
 
   if (!frame_sp) {
     return ValueObjectSP();
Index: lldb/source/Target/StackFrameList.cpp
===================================================================
--- lldb/source/Target/StackFrameList.cpp
+++ lldb/source/Target/StackFrameList.cpp
@@ -815,12 +815,17 @@
   }
 }
 
-uint32_t StackFrameList::GetSelectedFrameIndex() {
+uint32_t StackFrameList::GetSelectedFrameIndex(bool 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;
 }
 
@@ -858,7 +863,7 @@
 void StackFrameList::SetDefaultFileAndLineToSelectedFrame() {
   if (m_thread.GetID() ==
       m_thread.GetProcess()->GetThreadList().GetSelectedThread()->GetID()) {
-    StackFrameSP frame_sp(GetFrameAtIndex(GetSelectedFrameIndex()));
+    StackFrameSP frame_sp(GetFrameAtIndex(GetSelectedFrameIndex(false)));
     if (frame_sp) {
       SymbolContext sc = frame_sp->GetSymbolContext(eSymbolContextLineEntry);
       if (sc.line_entry.file)
@@ -918,7 +923,7 @@
   else
     last_frame = first_frame + num_frames;
 
-  StackFrameSP selected_frame_sp = m_thread.GetSelectedFrame();
+  StackFrameSP selected_frame_sp = m_thread.GetSelectedFrame(false);
   const char *unselected_marker = nullptr;
   std::string buffer;
   if (selected_frame_marker) {
Index: lldb/source/Target/Process.cpp
===================================================================
--- lldb/source/Target/Process.cpp
+++ lldb/source/Target/Process.cpp
@@ -647,7 +647,8 @@
 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) {
+                                        Stream *stream, bool use_run_lock,
+                                        bool update_selected_frames) {
   // 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.
@@ -682,7 +683,8 @@
       *event_sp_ptr = event_sp;
 
     bool pop_process_io_handler = (hijack_listener_sp.get() != nullptr);
-    Process::HandleProcessStateChangedEvent(event_sp, stream,
+    Process::HandleProcessStateChangedEvent(event_sp, stream, 
+                                            update_selected_frames,
                                             pop_process_io_handler);
 
     switch (state) {
@@ -714,6 +716,7 @@
 
 bool Process::HandleProcessStateChangedEvent(const EventSP &event_sp,
                                              Stream *stream,
+                                             bool select_most_relevant,
                                              bool &pop_process_io_handler) {
   const bool handle_pop = pop_process_io_handler;
 
@@ -896,7 +899,8 @@
             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;
@@ -1373,7 +1377,8 @@
   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 */, true /* select_most_relevant */);
     const bool must_be_alive =
         false; // eStateExited is ok, so this must be false
     if (!StateIsStoppedState(state, must_be_alive))
@@ -2653,7 +2658,8 @@
     // 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, true);
 
     if (!StateIsStoppedState(state, false)) {
       Log *log = GetLog(LLDBLog::Process);
@@ -3158,9 +3164,11 @@
   }
 
   // Wait for the process halt timeout seconds for the process to stop.
+  // 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.
   StateType state =
       WaitForProcessToStop(GetInterruptTimeout(), &event_sp, true,
-                           halt_listener_sp, nullptr, use_run_lock);
+                           halt_listener_sp, nullptr, use_run_lock, use_run_lock);
   RestoreProcessEvents();
 
   if (state == eStateInvalid || !event_sp) {
@@ -4760,10 +4768,10 @@
 
   // 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(false);
   if (!selected_frame_sp) {
     thread->SetSelectedFrame(nullptr);
-    selected_frame_sp = thread->GetSelectedFrame();
+    selected_frame_sp = thread->GetSelectedFrame(false);
     if (!selected_frame_sp) {
       diagnostic_manager.Printf(
           eDiagnosticSeverityError,
@@ -4795,7 +4803,7 @@
   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(false)->GetStackID();
   } else {
     selected_tid = LLDB_INVALID_THREAD_ID;
   }
Index: lldb/source/Target/Platform.cpp
===================================================================
--- lldb/source/Target/Platform.cpp
+++ lldb/source/Target/Platform.cpp
@@ -1815,7 +1815,10 @@
                                      nullptr);
     process_sp->RestoreProcessEvents();
     bool pop_process_io_handler = false;
-    Process::HandleProcessStateChangedEvent(event_sp, stream,
+    bool select_most_relevant = true; // This is a user-level stop, allow
+                                        // recognizers to update frames.
+    Process::HandleProcessStateChangedEvent(event_sp, stream, 
+                                            select_most_relevant,
                                             pop_process_io_handler);
   }
 
Index: lldb/source/Target/ExecutionContext.cpp
===================================================================
--- lldb/source/Target/ExecutionContext.cpp
+++ lldb/source/Target/ExecutionContext.cpp
@@ -85,7 +85,7 @@
       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(false);
       }
     }
   }
@@ -517,7 +517,7 @@
 
               if (thread_sp) {
                 SetThreadSP(thread_sp);
-                lldb::StackFrameSP frame_sp(thread_sp->GetSelectedFrame());
+                lldb::StackFrameSP frame_sp(thread_sp->GetSelectedFrame(false));
                 if (!frame_sp)
                   frame_sp = thread_sp->GetStackFrameAtIndex(0);
                 if (frame_sp)
Index: lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
===================================================================
--- lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
+++ lldb/source/Plugins/MemoryHistory/asan/MemoryHistoryASan.cpp
@@ -154,7 +154,7 @@
   if (!thread_sp)
     return result;
 
-  StackFrameSP frame_sp = thread_sp->GetSelectedFrame();
+  StackFrameSP frame_sp = thread_sp->GetSelectedFrame(false);
   if (!frame_sp)
     return result;
 
Index: lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
===================================================================
--- lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
+++ lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCRuntime.cpp
@@ -156,7 +156,7 @@
       thread = exe_ctx.GetThreadPtr();
     }
     if (thread) {
-      exe_ctx.SetFrameSP(thread->GetSelectedFrame());
+      exe_ctx.SetFrameSP(thread->GetSelectedFrame(false));
     }
   }
 
Index: lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
===================================================================
--- lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
+++ lldb/source/Plugins/InstrumentationRuntime/UBSan/InstrumentationRuntimeUBSan.cpp
@@ -108,7 +108,7 @@
     return StructuredData::ObjectSP();
 
   ThreadSP thread_sp = exe_ctx_ref.GetThreadSP();
-  StackFrameSP frame_sp = thread_sp->GetSelectedFrame();
+  StackFrameSP frame_sp = thread_sp->GetSelectedFrame(false);
   ModuleSP runtime_module_sp = GetRuntimeModuleSP();
   Target &target = process_sp->GetTarget();
 
Index: lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
===================================================================
--- lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
+++ lldb/source/Plugins/InstrumentationRuntime/TSan/InstrumentationRuntimeTSan.cpp
@@ -303,7 +303,7 @@
     return StructuredData::ObjectSP();
 
   ThreadSP thread_sp = exe_ctx_ref.GetThreadSP();
-  StackFrameSP frame_sp = thread_sp->GetSelectedFrame();
+  StackFrameSP frame_sp = thread_sp->GetSelectedFrame(false);
 
   if (!frame_sp)
     return StructuredData::ObjectSP();
Index: lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
===================================================================
--- lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
+++ lldb/source/Plugins/InstrumentationRuntime/MainThreadChecker/InstrumentationRuntimeMainThreadChecker.cpp
@@ -81,7 +81,7 @@
     return StructuredData::ObjectSP();
 
   ThreadSP thread_sp = exe_ctx_ref.GetThreadSP();
-  StackFrameSP frame_sp = thread_sp->GetSelectedFrame();
+  StackFrameSP frame_sp = thread_sp->GetSelectedFrame(false);
   ModuleSP runtime_module_sp = GetRuntimeModuleSP();
   Target &target = process_sp->GetTarget();
 
Index: lldb/source/Plugins/InstrumentationRuntime/ASan/InstrumentationRuntimeASan.cpp
===================================================================
--- lldb/source/Plugins/InstrumentationRuntime/ASan/InstrumentationRuntimeASan.cpp
+++ lldb/source/Plugins/InstrumentationRuntime/ASan/InstrumentationRuntimeASan.cpp
@@ -113,7 +113,7 @@
 
   ThreadSP thread_sp =
       process_sp->GetThreadList().GetExpressionExecutionThread();
-  StackFrameSP frame_sp = thread_sp->GetSelectedFrame();
+  StackFrameSP frame_sp = thread_sp->GetSelectedFrame(false);
 
   if (!frame_sp)
     return StructuredData::ObjectSP();
Index: lldb/source/Expression/REPL.cpp
===================================================================
--- lldb/source/Expression/REPL.cpp
+++ lldb/source/Expression/REPL.cpp
@@ -233,7 +233,7 @@
     ExecutionContext exe_ctx(m_target.GetProcessSP()
                                  ->GetThreadList()
                                  .GetSelectedThread()
-                                 ->GetSelectedFrame()
+                                 ->GetSelectedFrame(false)
                                  .get());
 
     lldb::ProcessSP process_sp(exe_ctx.GetProcessSP());
@@ -308,7 +308,7 @@
         Thread *thread = exe_ctx.GetThreadPtr();
         if (thread && thread->UnwindInnermostExpression().Success()) {
           thread->SetSelectedFrameByIndex(0, false);
-          exe_ctx.SetFrameSP(thread->GetSelectedFrame());
+          exe_ctx.SetFrameSP(thread->GetSelectedFrame(false));
         }
       }
 
Index: lldb/source/Core/ValueObject.cpp
===================================================================
--- lldb/source/Core/ValueObject.cpp
+++ lldb/source/Core/ValueObject.cpp
@@ -2852,7 +2852,7 @@
         StackFrameSP frame_sp(exe_ctx.GetFrameSP());
         if (!frame_sp) {
           if (use_selected)
-            frame_sp = thread_sp->GetSelectedFrame();
+            frame_sp = thread_sp->GetSelectedFrame(false);
         }
         if (frame_sp)
           m_exe_ctx_ref.SetFrameSP(frame_sp);
Index: lldb/source/Core/IOHandlerCursesGUI.cpp
===================================================================
--- lldb/source/Core/IOHandlerCursesGUI.cpp
+++ lldb/source/Core/IOHandlerCursesGUI.cpp
@@ -5259,7 +5259,7 @@
     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(true)];
         selection_index = selected_item->GetRowIndex();
         return;
       }
@@ -6411,7 +6411,7 @@
         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(true);
           exe_ctx.GetThreadRef().StepOut(frame_idx);
         }
       }
@@ -6827,7 +6827,7 @@
       if (process_alive) {
         thread = exe_ctx.GetThreadPtr();
         if (thread) {
-          frame_sp = thread->GetSelectedFrame();
+          frame_sp = thread->GetSelectedFrame(true);
           auto tid = thread->GetID();
           thread_changed = tid != m_tid;
           m_tid = tid;
@@ -7374,7 +7374,7 @@
         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(true);
           exe_ctx.GetThreadRef().StepOut(frame_idx);
         }
       }
@@ -7413,7 +7413,7 @@
           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(true);
         if (frame_idx == UINT32_MAX)
           frame_idx = 0;
         if (c == 'u' && frame_idx + 1 < thread->GetStackFrameCount())
@@ -7421,7 +7421,7 @@
         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(true));
       }
     }
       return eKeyHandled;
Index: lldb/source/Core/Debugger.cpp
===================================================================
--- lldb/source/Core/Debugger.cpp
+++ lldb/source/Core/Debugger.cpp
@@ -1680,7 +1680,7 @@
     // Display running state changes first before any STDIO
     if (got_state_changed && !state_is_stopped) {
       Process::HandleProcessStateChangedEvent(event_sp, output_stream_sp.get(),
-                                              pop_process_io_handler);
+                                              false, pop_process_io_handler);
     }
 
     // Now display STDOUT and STDERR
@@ -1719,7 +1719,7 @@
     // Now display any stopped state changes after any STDIO
     if (got_state_changed && state_is_stopped) {
       Process::HandleProcessStateChangedEvent(event_sp, output_stream_sp.get(),
-                                              pop_process_io_handler);
+                                              true, pop_process_io_handler);
     }
 
     output_stream_sp->Flush();
Index: lldb/source/Commands/CommandObjectType.cpp
===================================================================
--- lldb/source/Commands/CommandObjectType.cpp
+++ lldb/source/Commands/CommandObjectType.cpp
@@ -2845,7 +2845,7 @@
       return false;
     }
 
-    StackFrameSP frame_sp = thread->GetSelectedFrame();
+    StackFrameSP frame_sp = thread->GetSelectedFrame(false);
     ValueObjectSP result_valobj_sp;
     EvaluateExpressionOptions options;
     lldb::ExpressionResults expr_result = target_sp->EvaluateExpression(
Index: lldb/source/Commands/CommandObjectThread.cpp
===================================================================
--- lldb/source/Commands/CommandObjectThread.cpp
+++ lldb/source/Commands/CommandObjectThread.cpp
@@ -556,7 +556,7 @@
     } 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,
+          eVoteNoOpinion, thread->GetSelectedFrameIndex(false), new_plan_status,
           m_options.m_step_out_avoid_no_debug);
     } else if (m_step_type == eStepTypeScripted) {
       new_plan_sp = thread->QueueThreadPlanForStepScripted(
@@ -1527,7 +1527,7 @@
         bool success =
             thread->SetSelectedFrameByIndexNoisily(0, result.GetOutputStream());
         if (success) {
-          m_exe_ctx.SetFrameSP(thread->GetSelectedFrame());
+          m_exe_ctx.SetFrameSP(thread->GetSelectedFrame(false));
           result.SetStatus(eReturnStatusSuccessFinishResult);
         } else {
           result.AppendErrorWithFormat(
Index: lldb/source/Commands/CommandObjectFrame.cpp
===================================================================
--- lldb/source/Commands/CommandObjectFrame.cpp
+++ lldb/source/Commands/CommandObjectFrame.cpp
@@ -135,7 +135,7 @@
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
     Thread *thread = m_exe_ctx.GetThreadPtr();
-    StackFrameSP frame_sp = thread->GetSelectedFrame();
+    StackFrameSP frame_sp = thread->GetSelectedFrame(true);
 
     ValueObjectSP valobj_sp;
 
@@ -308,7 +308,7 @@
     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(true);
       if (frame_idx == UINT32_MAX)
         frame_idx = 0;
 
@@ -362,7 +362,7 @@
           return false;
         }
       } else if (command.GetArgumentCount() == 0) {
-        frame_idx = thread->GetSelectedFrameIndex();
+        frame_idx = thread->GetSelectedFrameIndex(true);
         if (frame_idx == UINT32_MAX) {
           frame_idx = 0;
         }
@@ -372,7 +372,7 @@
     bool success = thread->SetSelectedFrameByIndexNoisily(
         frame_idx, result.GetOutputStream());
     if (success) {
-      m_exe_ctx.SetFrameSP(thread->GetSelectedFrame());
+      m_exe_ctx.SetFrameSP(thread->GetSelectedFrame(true));
       result.SetStatus(eReturnStatusSuccessFinishResult);
     } else {
       result.AppendErrorWithFormat("Frame index (%u) out of range.\n",
Index: lldb/source/API/SBThread.cpp
===================================================================
--- lldb/source/API/SBThread.cpp
+++ lldb/source/API/SBThread.cpp
@@ -789,7 +789,11 @@
     }
 
     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(false);
       if (!frame_sp)
         frame_sp = thread->GetStackFrameAtIndex(0);
     }
@@ -1133,7 +1137,7 @@
   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(true);
       sb_frame.SetFrameSP(frame_sp);
     }
   }
Index: lldb/include/lldb/Target/Thread.h
===================================================================
--- lldb/include/lldb/Target/Thread.h
+++ lldb/include/lldb/Target/Thread.h
@@ -426,11 +426,16 @@
     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(bool select_most_relevant) {
+    return GetStackFrameList()
+        ->GetSelectedFrameIndex(select_most_relevant);
   }
 
-  lldb::StackFrameSP GetSelectedFrame();
+  lldb::StackFrameSP GetSelectedFrame(bool select_most_relevant);
 
   uint32_t SetSelectedFrame(lldb_private::StackFrame *frame,
                             bool broadcast = false);
Index: lldb/include/lldb/Target/StackFrameList.h
===================================================================
--- lldb/include/lldb/Target/StackFrameList.h
+++ lldb/include/lldb/Target/StackFrameList.h
@@ -46,7 +46,14 @@
   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(bool 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.
Index: lldb/include/lldb/Target/Process.h
===================================================================
--- lldb/include/lldb/Target/Process.h
+++ lldb/include/lldb/Target/Process.h
@@ -825,6 +825,7 @@
   /// \see Thread:Suspend()
   Status Resume();
 
+  /// Resume a process, and wait for it to stop.
   Status ResumeSynchronous(Stream *stream);
 
   /// Halts a running process.
@@ -2160,12 +2161,16 @@
   // 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,
+                       bool select_most_relevant = false);
 
   uint32_t GetIOHandlerID() const { return m_iohandler_sync.GetValue(); }
 
@@ -2205,6 +2210,7 @@
   ///     otherwise.
   static bool HandleProcessStateChangedEvent(const lldb::EventSP &event_sp,
                                              Stream *stream,
+                                             bool select_most_relevant,
                                              bool &pop_process_io_handler);
 
   Event *PeekAtStateChangedEvents();
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to