Reapply the commits to enable accurate hit-count detection for watchpoints.

This commit combines the initial commit (7c240de609af), a fix for x86_64 Linux
(3a0581501e76) and a fix for thinko in a last minute rewrite that I really
should have run the testsuite on.

Also, make sure that all the "I need to step over watchpoint" plans execute
before we call a public stop.  Otherwise, e.g. if you have N watchpoints and
a Signal, the signal stop info will get us to stop with the watchpoints in a
half-done state.

Differential Revision: https://reviews.llvm.org/D130674

20 files changed, 294 insertions, 157 deletions

diff --git a/lldb/include/lldb/Breakpoint/Watchpoint.h b/lldb/include/lldb/Breakpoint/Watchpoint.h
index 41b723a66b6a..a5a72e3ad5a1 100644
--- a/lldb/include/lldb/Breakpoint/Watchpoint.h
+++ b/lldb/include/lldb/Breakpoint/Watchpoint.h
@@ -75,7 +75,7 @@ public:
   bool IsHardware() const override;
 
   bool ShouldStop(StoppointCallbackContext *context) override;
-
+  
   bool WatchpointRead() const;
   bool WatchpointWrite() const;
   uint32_t GetIgnoreCount() const;
@@ -157,12 +157,15 @@ public:
 private:
   friend class Target;
   friend class WatchpointList;
+  friend class StopInfoWatchpoint; // This needs to call UndoHitCount()
 
   void ResetHistoricValues() {
     m_old_value_sp.reset();
     m_new_value_sp.reset();
   }
 
+  void UndoHitCount() { m_hit_counter.Decrement(); }
+
   Target &m_target;
   bool m_enabled;           // Is this watchpoint enabled
   bool m_is_hardware;       // Is this a hardware watchpoint
diff --git a/lldb/include/lldb/Target/StopInfo.h b/lldb/include/lldb/Target/StopInfo.h
index cdb906dcd7ed..9527a6ea553e 100644
--- a/lldb/include/lldb/Target/StopInfo.h
+++ b/lldb/include/lldb/Target/StopInfo.h
@@ -17,7 +17,7 @@
 
 namespace lldb_private {
 
-class StopInfo {
+class StopInfo : public std::enable_shared_from_this<StopInfo> {
   friend class Process::ProcessEventData;
   friend class ThreadPlanBase;
 
diff --git a/lldb/include/lldb/Target/Thread.h b/lldb/include/lldb/Target/Thread.h
index f5f024434c8e..efe82721a04e 100644
--- a/lldb/include/lldb/Target/Thread.h
+++ b/lldb/include/lldb/Target/Thread.h
@@ -240,6 +240,7 @@ public:
   // this just calls through to the ThreadSpec's ThreadPassesBasicTests method.
   virtual bool MatchesSpec(const ThreadSpec *spec);
 
+  // Get the current public stop info, calculating it if necessary.
   lldb::StopInfoSP GetStopInfo();
 
   lldb::StopReason GetStopReason();
@@ -1121,7 +1122,7 @@ public:
   // "checkpointed and restored" stop info, so if it is still around it is
   // right even if you have not calculated this yourself, or if it disagrees
   // with what you might have calculated.
-  virtual lldb::StopInfoSP GetPrivateStopInfo();
+  virtual lldb::StopInfoSP GetPrivateStopInfo(bool calculate = true);
 
   // Calculate the stop info that will be shown to lldb clients.  For instance,
   // a "step out" is implemented by running to a breakpoint on the function
@@ -1165,6 +1166,14 @@ public:
   void ResetStopInfo();
 
   void SetShouldReportStop(Vote vote);
+  
+  void SetShouldRunBeforePublicStop(bool newval) { 
+      m_should_run_before_public_stop = newval; 
+  }
+  
+  bool ShouldRunBeforePublicStop() {
+      return m_should_run_before_public_stop;
+  }
 
   /// Sets the extended backtrace token for this thread
   ///
@@ -1254,6 +1263,9 @@ protected:
   uint32_t m_stop_info_override_stop_id; // The stop ID containing the last time
                                          // the stop info was checked against
                                          // the stop info override
+  bool m_should_run_before_public_stop;  // If this thread has "stop others" 
+                                         // private work to do, then it will
+                                         // set this.
   const uint32_t m_index_id; ///< A unique 1 based index assigned to each thread
                              /// for easy UI/command line access.
   lldb::RegisterContextSP m_reg_context_sp; ///< The register context for this
diff --git a/lldb/include/lldb/Target/ThreadPlan.h b/lldb/include/lldb/Target/ThreadPlan.h
index 616939f89fc8..bf68a42e54d1 100644
--- a/lldb/include/lldb/Target/ThreadPlan.h
+++ b/lldb/include/lldb/Target/ThreadPlan.h
@@ -384,6 +384,8 @@ public:
   virtual void SetStopOthers(bool new_value);
 
   virtual bool StopOthers();
+  
+  virtual bool ShouldRunBeforePublicStop() { return false; }
 
   // This is the wrapper for DoWillResume that does generic ThreadPlan logic,
   // then calls DoWillResume.
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index e3dada1864da..233a665f6590 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -1675,7 +1675,15 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo(
       if (dispatch_queue_t != LLDB_INVALID_ADDRESS)
         gdb_thread->SetQueueLibdispatchQueueAddress(dispatch_queue_t);
 
-      // Make sure we update our thread stop reason just once
+      // Make sure we update our thread stop reason just once, but don't 
+      // overwrite the stop info for threads that haven't moved:
+      StopInfoSP current_stop_info_sp = thread_sp->GetPrivateStopInfo(false);
+      if (thread_sp->GetTemporaryResumeState() == eStateSuspended &&
+          current_stop_info_sp) {
+          thread_sp->SetStopInfo(current_stop_info_sp);
+          return thread_sp;
+      }
+
       if (!thread_sp->StopInfoIsUpToDate()) {
         thread_sp->SetStopInfo(StopInfoSP());
         // If there's a memory thread backed by this thread, we need to use it
diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index 00d30070c8c9..d8fb10441d89 100644
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -20,6 +20,7 @@
 #include "lldb/Target/Target.h"
 #include "lldb/Target/Thread.h"
 #include "lldb/Target/ThreadPlan.h"
+#include "lldb/Target/ThreadPlanStepInstruction.h"
 #include "lldb/Target/UnixSignals.h"
 #include "lldb/Utility/LLDBLog.h"
 #include "lldb/Utility/Log.h"
@@ -690,39 +691,184 @@ public:
   }
 
 protected:
+  using StopInfoWatchpointSP = std::shared_ptr<StopInfoWatchpoint>;
+  // This plan is used to orchestrate stepping over the watchpoint for
+  // architectures (e.g. ARM) that report the watch before running the watched
+  // access.  This is the sort of job you have to defer to the thread plans,
+  // if you try to do it directly in the stop info and there are other threads
+  // that needed to process this stop you will have yanked control away from
+  // them and they won't behave correctly.
+  class ThreadPlanStepOverWatchpoint : public ThreadPlanStepInstruction {
+  public:
+    ThreadPlanStepOverWatchpoint(Thread &thread, 
+                                 StopInfoWatchpointSP stop_info_sp,
+                                 WatchpointSP watch_sp)
+        : ThreadPlanStepInstruction(thread, false, true, eVoteNoOpinion,
+                                    eVoteNoOpinion),
+          m_stop_info_sp(stop_info_sp), m_watch_sp(watch_sp) {
+      assert(watch_sp);
+      m_watch_index = watch_sp->GetHardwareIndex();
+    }
+
+    bool DoWillResume(lldb::StateType resume_state,
+                      bool current_plan) override {
+      if (resume_state == eStateSuspended)
+        return true;
+
+      if (!m_did_disable_wp) {
+        GetThread().GetProcess()->DisableWatchpoint(m_watch_sp.get(), false);
+        m_did_disable_wp = true;
+      }
+      return true;
+    }
+    
+    bool DoPlanExplainsStop(Event *event_ptr) override {
+      if (ThreadPlanStepInstruction::DoPlanExplainsStop(event_ptr))
+        return true;
+      StopInfoSP stop_info_sp = GetThread().GetPrivateStopInfo();
+      // lldb-server resets the stop info for threads that didn't get to run,
+      // so we might have not gotten to run, but still have a watchpoint stop
+      // reason, in which case this will indeed be for us.
+      if (stop_info_sp 
+          && stop_info_sp->GetStopReason() == eStopReasonWatchpoint)
+        return true;
+      return false;
+    }
+
+    void DidPop() override {
+      // Don't artifically keep the watchpoint alive.
+      m_watch_sp.reset();
+    }
+    
+    bool ShouldStop(Event *event_ptr) override {
+      bool should_stop = ThreadPlanStepInstruction::ShouldStop(event_ptr);
+      bool plan_done = MischiefManaged();
+      if (plan_done) {
+        m_stop_info_sp->SetStepOverPlanComplete();
+        GetThread().SetStopInfo(m_stop_info_sp);
+        ResetWatchpoint();
+      }
+      return should_stop;
+    }
+    
+    bool ShouldRunBeforePublicStop() override {
+        return true;
+    }
+
+  protected:
+    void ResetWatchpoint() {
+      if (!m_did_disable_wp)
+        return;
+      m_did_disable_wp = true;
+      GetThread().GetProcess()->EnableWatchpoint(m_watch_sp.get(), true);
+      m_watch_sp->SetHardwareIndex(m_watch_index);
+    }
+
+  private:
+    StopInfoWatchpointSP m_stop_info_sp;
+    WatchpointSP m_watch_sp;
+    uint32_t m_watch_index = LLDB_INVALID_INDEX32;
+    bool m_did_disable_wp = false;
+  };
+
   bool ShouldStopSynchronous(Event *event_ptr) override {
-    // ShouldStop() method is idempotent and should not affect hit count. See
-    // Process::RunPrivateStateThread()->Process()->HandlePrivateEvent()
-    // -->Process()::ShouldBroadcastEvent()->ThreadList::ShouldStop()->
-    // Thread::ShouldStop()->ThreadPlanBase::ShouldStop()->
-    // StopInfoWatchpoint::ShouldStop() and
-    // Event::DoOnRemoval()->Process::ProcessEventData::DoOnRemoval()->
-    // StopInfoWatchpoint::PerformAction().
+    // If we are running our step-over the watchpoint plan, stop if it's done
+    // and continue if it's not:
     if (m_should_stop_is_valid)
       return m_should_stop;
 
+    // If we are running our step over plan, then stop here and let the regular
+    // ShouldStop figure out what we should do:  Otherwise, give our plan
+    // more time to get run:
+    if (m_using_step_over_plan)
+      return m_step_over_plan_complete;
+
+    Log *log = GetLog(LLDBLog::Process);
     ThreadSP thread_sp(m_thread_wp.lock());
-    if (thread_sp) {
-      WatchpointSP wp_sp(
-          thread_sp->CalculateTarget()->GetWatchpointList().FindByID(
-              GetValue()));
-      if (wp_sp) {
-        // Check if we should stop at a watchpoint.
-        ExecutionContext exe_ctx(thread_sp->GetStackFrameAtIndex(0));
-        StoppointCallbackContext context(event_ptr, exe_ctx, true);
-        m_should_stop = wp_sp->ShouldStop(&context);
-      } else {
-        Log *log = GetLog(LLDBLog::Process);
+    assert(thread_sp);
+    
+    if (thread_sp->GetTemporaryResumeState() == eStateSuspended) {
+      // This is the second firing of a watchpoint so don't process it again.
+      LLDB_LOG(log, "We didn't run but stopped with a StopInfoWatchpoint, we "
+               "have already handled this one, don't do it again.");
+      m_should_stop = false;
+      m_should_stop_is_valid = true;
+      return m_should_stop;
+    }
+    
+    WatchpointSP wp_sp(
+        thread_sp->CalculateTarget()->GetWatchpointList().FindByID(GetValue()));
+    // If we can no longer find the watchpoint, we just have to stop:
+    if (!wp_sp) {
 
-        LLDB_LOGF(log,
-                  "Process::%s could not find watchpoint location id: %" PRId64
-                  "...",
-                  __FUNCTION__, GetValue());
+      LLDB_LOGF(log,
+                "Process::%s could not find watchpoint location id: %" PRId64
+                "...",
+                __FUNCTION__, GetValue());
+
+      m_should_stop = true;
+      m_should_stop_is_valid = true;
+      return true;
+    }
+
+    ExecutionContext exe_ctx(thread_sp->GetStackFrameAtIndex(0));
+    StoppointCallbackContext context(event_ptr, exe_ctx, true);
+    m_should_stop = wp_sp->ShouldStop(&context);
+    if (!m_should_stop) {
+      // This won't happen at present because we only allow one watchpoint per
+      // watched range.  So we won't stop at a watched address with a disabled
+      // watchpoint.  If we start allowing overlapping watchpoints, then we
+      // will have to make watchpoints be real "WatchpointSite" and delegate to
+      // all the watchpoints sharing the site.  In that case, the code below
+      // would be the right thing to do.
+      m_should_stop_is_valid = true;
+      return m_should_stop;
+    }
+    // If this is a system where we need to execute the watchpoint by hand
+    // after the hit, queue a thread plan to do that, and then say not to stop.
+    // Otherwise, let the async action figure out whether the watchpoint should
+    // stop
+
+    ProcessSP process_sp = exe_ctx.GetProcessSP();
+    uint32_t num;
+    bool wp_triggers_after;
+
+    if (!process_sp->GetWatchpointSupportInfo(num, wp_triggers_after)
+            .Success()) {
+      m_should_stop_is_valid = true;
+      m_should_stop = true;
+      return m_should_stop;
+    }
+            
+    if (!wp_triggers_after) {
+      // We have to step over the watchpoint before we know what to do:   
+      StopInfoWatchpointSP me_as_siwp_sp 
+          = std::static_pointer_cast<StopInfoWatchpoint>(shared_from_this());
+      ThreadPlanSP step_over_wp_sp(new ThreadPlanStepOverWatchpoint(
+          *(thread_sp.get()), me_as_siwp_sp, wp_sp));
+      Status error;
+      error = thread_sp->QueueThreadPlan(step_over_wp_sp, false);
+      // If we couldn't push the thread plan, just stop here:
+      if (!error.Success()) {
+        LLDB_LOGF(log, "Could not push our step over watchpoint plan: %s", 
+            error.AsCString());
 
         m_should_stop = true;
+        m_should_stop_is_valid = true;
+        return true;
+      } else {
+      // Otherwise, don't set m_should_stop, we don't know that yet.  Just 
+      // say we should continue, and tell the thread we really should do so:
+        thread_sp->SetShouldRunBeforePublicStop(true);
+        m_using_step_over_plan = true;
+        return false;
       }
+    } else {
+      // We didn't have to do anything special
+      m_should_stop_is_valid = true;
+      return m_should_stop;
     }
-    m_should_stop_is_valid = true;
+    
     return m_should_stop;
   }
 
@@ -749,57 +895,12 @@ protected:
           thread_sp->CalculateTarget()->GetWatchpointList().FindByID(
               GetValue()));
       if (wp_sp) {
-        ExecutionContext exe_ctx(thread_sp->GetStackFrameAtIndex(0));
-        ProcessSP process_sp = exe_ctx.GetProcessSP();
-
-        {
-          // check if this process is running on an architecture where
-          // watchpoints trigger before the associated instruction runs. if so,
-          // disable the WP, single-step and then re-enable the watchpoint
-          if (process_sp) {
-            uint32_t num;
-            bool wp_triggers_after;
-
-            if (process_sp->GetWatchpointSupportInfo(num, wp_triggers_after)
-                    .Success()) {
-              if (!wp_triggers_after) {
-                // We need to preserve the watch_index before watchpoint  is
-                // disable. Since Watchpoint::SetEnabled will clear the watch
-                // index. This will fix TestWatchpointIter failure
-                Watchpoint *wp = wp_sp.get();
-                uint32_t watch_index = wp->GetHardwareIndex();
-                process_sp->DisableWatchpoint(wp, false);
-                StopInfoSP stored_stop_info_sp = thread_sp->GetStopInfo();
-                assert(stored_stop_info_sp.get() == this);
-
-                Status new_plan_status;
-                ThreadPlanSP new_plan_sp(
-                    thread_sp->QueueThreadPlanForStepSingleInstruction(
-                        false, // step-over
-                        false, // abort_other_plans
-                        true,  // stop_other_threads
-                        new_plan_status));
-                if (new_plan_sp && new_plan_status.Success()) {
-                  new_plan_sp->SetIsControllingPlan(true);
-                  new_plan_sp->SetOkayToDiscard(false);
-                  new_plan_sp->SetPrivate(true);
-                }
-                process_sp->GetThreadList().SetSelectedThreadByID(
-                    thread_sp->GetID());
-                process_sp->ResumeSynchronous(nullptr);
-                process_sp->GetThreadList().SetSelectedThreadByID(
-                    thread_sp->GetID());
-                thread_sp->SetStopInfo(stored_stop_info_sp);
-                process_sp->EnableWatchpoint(wp, false);
-                wp->SetHardwareIndex(watch_index);
-              }
-            }
-          }
-        }
-
         // This sentry object makes sure the current watchpoint is disabled
         // while performing watchpoint actions, and it is then enabled after we
         // are finished.
+        ExecutionContext exe_ctx(thread_sp->GetStackFrameAtIndex(0));
+        ProcessSP process_sp = exe_ctx.GetProcessSP();
+
         WatchpointSentry sentry(process_sp, wp_sp);
 
         /*
@@ -825,18 +926,10 @@ protected:
           }
         }
 
-        // TODO: This condition should be checked in the synchronous part of the
-        // watchpoint code
-        // (Watchpoint::ShouldStop), so that we avoid pulling an event even if
-        // the watchpoint fails the ignore count condition. It is moved here
-        // temporarily, because for archs with
-        // watchpoint_exceptions_received=before, the code in the previous
-        // lines takes care of moving the inferior to next PC. We have to check
-        // the ignore count condition after this is done, otherwise we will hit
-        // same watchpoint multiple times until we pass ignore condition, but
-        // we won't actually be ignoring them.
-        if (wp_sp->GetHitCount() <= wp_sp->GetIgnoreCount())
+        if (wp_sp->GetHitCount() <= wp_sp->GetIgnoreCount()) {
           m_should_stop = false;
+          m_should_stop_is_valid = true;
+        }
 
         Debugger &debugger = exe_ctx.GetTargetRef().GetDebugger();
 
@@ -859,10 +952,9 @@ protected:
               Scalar scalar_value;
               if (result_value_sp->ResolveValue(scalar_value)) {
                 if (scalar_value.ULongLong(1) == 0) {
-                  // We have been vetoed.  This takes precedence over querying
-                  // the watchpoint whether it should stop (aka ignore count
-                  // and friends).  See also StopInfoWatchpoint::ShouldStop()
-                  // as well as Process::ProcessEventData::DoOnRemoval().
+                  // The condition failed, which we consider "not having hit
+                  // the watchpoint" so undo the hit count here.
+                  wp_sp->UndoHitCount();
                   m_should_stop = false;
                 } else
                   m_should_stop = true;
@@ -946,9 +1038,16 @@ protected:
   }
 
 private:
+  void SetStepOverPlanComplete() {
+    assert(m_using_step_over_plan);
+    m_step_over_plan_complete = true;
+  }
+  
   bool m_should_stop = false;
   bool m_should_stop_is_valid = false;
   lldb::addr_t m_watch_hit_addr;
+  bool m_step_over_plan_complete = false;
+  bool m_using_step_over_plan = false;
 };
 
 // StopInfoUnixSignal
diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index f63b57fd4e62..6c3381cf6e9e 100644
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -221,6 +221,7 @@ Thread::Thread(Process &process, lldb::tid_t tid, bool use_invalid_index_id)
                   Thread::GetStaticBroadcasterClass().AsCString()),
       m_process_wp(process.shared_from_this()), m_stop_info_sp(),
       m_stop_info_stop_id(0), m_stop_info_override_stop_id(0),
+      m_should_run_before_public_stop(false),
       m_index_id(use_invalid_index_id ? LLDB_INVALID_INDEX32
                                       : process.GetNextThreadIndexID(tid)),
       m_reg_context_sp(), m_state(eStateUnloaded), m_state_mutex(),
@@ -369,7 +370,10 @@ void Thread::CalculatePublicStopInfo() {
   SetStopInfo(GetStopInfo());
 }
 
-lldb::StopInfoSP Thread::GetPrivateStopInfo() {
+lldb::StopInfoSP Thread::GetPrivateStopInfo(bool calculate) {
+  if (!calculate)
+    return m_stop_info_sp;
+
   if (m_destroy_called)
     return m_stop_info_sp;
 
@@ -377,9 +381,15 @@ lldb::StopInfoSP Thread::GetPrivateStopInfo() {
   if (process_sp) {
     const uint32_t process_stop_id = process_sp->GetStopID();
     if (m_stop_info_stop_id != process_stop_id) {
+      // We preserve the old stop info for a variety of reasons:
+      // 1) Someone has already updated it by the time we get here
+      // 2) We didn't get to execute the breakpoint instruction we stopped at
+      // 3) This is a virtual step so we didn't actually run
+      // 4) If this thread wasn't allowed to run the last time round.
       if (m_stop_info_sp) {
         if (m_stop_info_sp->IsValid() || IsStillAtLastBreakpointHit() ||
-            GetCurrentPlan()->IsVirtualStep())
+            GetCurrentPlan()->IsVirtualStep()
+            || GetTemporaryResumeState() == eStateSuspended)
           SetStopInfo(m_stop_info_sp);
         else
           m_stop_info_sp.reset();
@@ -723,7 +733,11 @@ bool Thread::ShouldResume(StateType resume_state) {
   return need_to_resume;
 }
 
-void Thread::DidResume() { SetResumeSignal(LLDB_INVALID_SIGNAL_NUMBER); }
+void Thread::DidResume() { 
+  SetResumeSignal(LLDB_INVALID_SIGNAL_NUMBER);
+  // This will get recomputed each time when we stop.
+  SetShouldRunBeforePublicStop(false);
+}
 
 void Thread::DidStop() { SetState(eStateStopped); }
 
@@ -763,6 +777,9 @@ bool Thread::ShouldStop(Event *event_ptr) {
                                    : LLDB_INVALID_ADDRESS);
     return false;
   }
+  
+  // Clear the "must run me before stop" if it was set:
+  SetShouldRunBeforePublicStop(false);
 
   if (log) {
     LLDB_LOGF(log,
@@ -845,9 +862,14 @@ bool Thread::ShouldStop(Event *event_ptr) {
             // stack below.
             done_processing_current_plan =
                 (plan_ptr->IsControllingPlan() && !plan_ptr->OkayToDiscard());
-          } else
+          } else {
+            bool should_force_run = plan_ptr->ShouldRunBeforePublicStop();
+            if (should_force_run) {
+              SetShouldRunBeforePublicStop(true);
+              should_stop = false;
+            }
             done_processing_current_plan = true;
-
+          }
           break;
         }
       }
diff --git a/lldb/source/Target/ThreadList.cpp b/lldb/source/Target/ThreadList.cpp
index 7359bfcd3cfc..006c8648be52 100644
--- a/lldb/source/Target/ThreadList.cpp
+++ b/lldb/source/Target/ThreadList.cpp
@@ -245,14 +245,17 @@ bool ThreadList::ShouldStop(Event *event_ptr) {
     for (lldb::ThreadSP thread_sp : m_threads) {
       // This is an optimization...  If we didn't let a thread run in between
       // the previous stop and this one, we shouldn't have to consult it for
-      // ShouldStop.  So just leave it off the list we are going to inspect. On
-      // Linux, if a thread-specific conditional breakpoint was hit, it won't
+      // ShouldStop.  So just leave it off the list we are going to inspect.
+      // If the thread didn't run but had work to do before declaring a public
+      // stop, then also include it.
+      // On Linux, if a thread-specific conditional breakpoint was hit, it won't
       // necessarily be the thread that hit the breakpoint itself that
       // evaluates the conditional expression, so the thread that hit the
       // breakpoint could still be asked to stop, even though it hasn't been
       // allowed to run since the previous stop.
       if (thread_sp->GetTemporaryResumeState() != eStateSuspended ||
-          thread_sp->IsStillAtLastBreakpointHit())
+          thread_sp->IsStillAtLastBreakpointHit()
+          || thread_sp->ShouldRunBeforePublicStop())
         threads_copy.push_back(thread_sp);
     }
 
@@ -300,6 +303,10 @@ bool ThreadList::ShouldStop(Event *event_ptr) {
     thread_sp->GetStopInfo();
   }
 
+  // If a thread needs to finish some job that can be done just on this thread
+  // before broadcastion the stop, it will signal that by returning true for
+  // ShouldRunBeforePublicStop.  This variable gathers the results from that.
+  bool a_thread_needs_to_run = false;
   for (pos = threads_copy.begin(); pos != end; ++pos) {
     ThreadSP thread_sp(*pos);
 
@@ -329,11 +336,23 @@ bool ThreadList::ShouldStop(Event *event_ptr) {
       did_anybody_stop_for_a_reason |= thread_sp->ThreadStoppedForAReason();
 
     const bool thread_should_stop = thread_sp->ShouldStop(event_ptr);
+
     if (thread_should_stop)
       should_stop |= true;
+    else {
+      bool this_thread_forces_run = thread_sp->ShouldRunBeforePublicStop();
+      a_thread_needs_to_run |= this_thread_forces_run;
+      if (this_thread_forces_run) 
+        LLDB_LOG(log,
+                 "ThreadList::{0} thread: {1:x}, "
+                 "says it needs to run before public stop.",
+                 __FUNCTION__, thread_sp->GetID());
+    }
   }
 
-  if (!should_stop && !did_anybody_stop_for_a_reason) {
+  if (a_thread_needs_to_run) {
+    should_stop = false;
+  } else if (!should_stop && !did_anybody_stop_for_a_reason) {
     should_stop = true;
     LLDB_LOGF(log,
               "ThreadList::%s we stopped but no threads had a stop reason, "
@@ -368,9 +387,17 @@ Vote ThreadList::ShouldReportStop(Event *event_ptr) {
 
   // Run through the threads and ask whether we should report this event. For
   // stopping, a YES vote wins over everything.  A NO vote wins over NO
-  // opinion.
+  // opinion.  The exception is if a thread has work it needs to force before
+  // a public stop, which overrides everyone else's opinion:
   for (pos = m_threads.begin(); pos != end; ++pos) {
     ThreadSP thread_sp(*pos);
+    if (thread_sp->ShouldRunBeforePublicStop()) {
+      LLDB_LOG(log, "Thread {0:x} has private business to complete, overrode "
+               "the should report stop.", thread_sp->GetID());
+      result = eVoteNo;
+      break;
+    }
+
     const Vote vote = thread_sp->ShouldReportStop(event_ptr);
     switch (vote) {
     case eVoteNoOpinion:
@@ -550,7 +577,13 @@ bool ThreadList::WillResume() {
 
   run_me_only_list.SetStopID(m_process->GetStopID());
 
-  bool run_only_current_thread = false;
+  // One or more threads might want to "Stop Others".  We want to handle all
+  // those requests first.  And if there is a thread that wanted to "resume
+  // before a public stop", let it get the first crack:
+  // There are two special kinds of thread that have priority for "StopOthers":
+  // a "ShouldRunBeforePublicStop thread, or the currently selected thread.  If
+  // we find one satisfying that critereon, put it here.
+  ThreadSP stop_others_thread_sp;
 
   for (pos = m_threads.begin(); pos != end; ++pos) {
     ThreadSP thread_sp(*pos);
@@ -562,17 +595,16 @@ bool ThreadList::WillResume() {
 
       // You can't say "stop others" and also want yourself to be suspended.
       assert(thread_sp->GetCurrentPlan()->RunState() != eStateSuspended);
+      run_me_only_list.AddThread(thread_sp);
 
-      if (thread_sp == GetSelectedThread()) {
-        // If the currently selected thread wants to run on its own, always let
-        // it.
-        run_only_current_thread = true;
-        run_me_only_list.Clear();
-        run_me_only_list.AddThread(thread_sp);
+      if (thread_sp == GetSelectedThread())
+        stop_others_thread_sp = thread_sp;
+        
+      if (thread_sp->ShouldRunBeforePublicStop()) {
+        // This takes precedence, so if we find one of these, service it:
+        stop_others_thread_sp = thread_sp;
         break;
       }
-
-      run_me_only_list.AddThread(thread_sp);
     }
   }
 
@@ -593,8 +625,8 @@ bool ThreadList::WillResume() {
   } else {
     ThreadSP thread_to_run;
 
-    if (run_only_current_thread) {
-      thread_to_run = GetSelectedThread();
+    if (stop_others_thread_sp) {
+      thread_to_run = stop_others_thread_sp;
     } else if (run_me_only_list.GetSize(false) == 1) {
       thread_to_run = run_me_only_list.GetThreadAtIndex(0);
     } else {
@@ -607,6 +639,9 @@ bool ThreadList::WillResume() {
     for (pos = m_threads.begin(); pos != end; ++pos) {
       ThreadSP thread_sp(*pos);
       if (thread_sp == thread_to_run) {
+        // Note, a thread might be able to fulfil it's plan w/o actually
+        // resuming.  An example of this is a step that changes the current
+        // inlined function depth w/o moving the PC.  Check that here:
         if (!thread_sp->ShouldResume(thread_sp->GetCurrentPlan()->RunState()))
           need_to_resume = false;
       } else
@@ -624,7 +659,7 @@ void ThreadList::DidResume() {
     // Don't clear out threads that aren't going to get a chance to run, rather
     // leave their state for the next time around.
     ThreadSP thread_sp(*pos);
-    if (thread_sp->GetResumeState() != eStateSuspended)
+    if (thread_sp->GetTemporaryResumeState() != eStateSuspended)
       thread_sp->DidResume();
   }
 }
diff --git a/lldb/test/API/commands/watchpoints/watchpoint_commands/condition/TestWatchpointConditionCmd.py b/lldb/test/API/commands/watchpoints/watchpoint_commands/condition/TestWatchpointConditionCmd.py
index b3b0e102e6c3..fee6b8b582c2 100644
--- a/lldb/test/API/commands/watchpoints/watchpoint_commands/condition/TestWatchpointConditionCmd.py
+++ b/lldb/test/API/commands/watchpoints/watchpoint_commands/condition/TestWatchpointConditionCmd.py
@@ -82,4 +82,4 @@ class WatchpointConditionCmdTestCase(TestBase):
         # Use the '-v' option to do verbose listing of the watchpoint.
         # The hit count should now be 2.
         self.expect("watchpoint list -v",
-                    substrs=['hit_count = 5'])
+                    substrs=['hit_count = 1'])
diff --git a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentDelayWatchBreak.py b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentDelayWatchBreak.py
index ba648b87ea8d..0a958c0c38c7 100644
--- a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentDelayWatchBreak.py
+++ b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentDelayWatchBreak.py
@@ -11,10 +11,6 @@ class ConcurrentDelayWatchBreak(ConcurrentEventsBase):
 
     # Atomic sequences are not supported yet for MIPS in LLDB.
     @skipIf(triple='^mips')
-    @skipIf(
-        oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
-        archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
-        bugnumber="rdar://81811539")
     @add_test_categories(["watchpoint"])
     def test(self):
         """Test (1-second delay) watchpoint and a breakpoint in multiple threads."""
diff --git a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py
index cd82dab82756..b74d0c7097d6 100644
--- a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py
+++ b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentManyWatchpoints.py
@@ -10,10 +10,6 @@ class ConcurrentManyWatchpoints(ConcurrentEventsBase):
 
     # Atomic sequences are not supported yet for MIPS in LLDB.
     @skipIf(triple='^mips')
-    @skipIf(
-        oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
-        archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
-        bugnumber="rdar://93863107")
     @add_test_categories(["watchpoint"])
     @skipIfOutOfTreeDebugserver
     def test(self):
diff --git a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentNWatchNBreak.py b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentNWatchNBreak.py
index 0dab66598eac..40b548e708e4 100644
--- a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentNWatchNBreak.py
+++ b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentNWatchNBreak.py
@@ -13,10 +13,6 @@ class ConcurrentNWatchNBreak(ConcurrentEventsBase):
     @skipIf(triple='^mips')
     @expectedFailureAll(archs=["aarch64"], oslist=["freebsd"],
                         bugnumber="llvm.org/pr49433")
-    @skipIf(
-        oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
-        archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
-        bugnumber="rdar://93863107")
     @add_test_categories(["watchpoint"])
     def test(self):
         """Test with 5 watchpoint and breakpoint threads."""
diff --git a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalNWatchNBreak.py b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalNWatchNBreak.py
index 2478a49de4e3..bcf1c6409b92 100644
--- a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalNWatchNBreak.py
+++ b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalNWatchNBreak.py
@@ -14,10 +14,6 @@ class ConcurrentSignalNWatchNBreak(ConcurrentEventsBase):
     @expectedFailureNetBSD
     @expectedFailureAll(archs=["aarch64"], oslist=["freebsd"],
                         bugnumber="llvm.org/pr49433")
-    @skipIf(
-        oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
-        archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
-        bugnumber="rdar://93863107")
     @add_test_categories(["watchpoint"])
     def test(self):
         """Test one signal thread with 5 watchpoint and breakpoint threads."""
diff --git a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalWatch.py b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalWatch.py
index 2922070f9da6..421db5a31132 100644
--- a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalWatch.py
+++ b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalWatch.py
@@ -11,10 +11,6 @@ class ConcurrentSignalWatch(ConcurrentEventsBase):
 
     # Atomic sequences are not supported yet for MIPS in LLDB.
     @skipIf(triple='^mips')
-    @skipIf(
-        oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
-        archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
-        bugnumber="rdar://93863107")
     @add_test_categories(["watchpoint"])
     def test(self):
         """Test a watchpoint and a signal in multiple threads."""
diff --git a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalWatchBreak.py b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalWatchBreak.py
index 3ca3a8661c01..50fead6f499f 100644
--- a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalWatchBreak.py
+++ b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentSignalWatchBreak.py
@@ -12,10 +12,6 @@ class ConcurrentSignalWatchBreak(ConcurrentEventsBase):
     # Atomic sequences are not supported yet for MIPS in LLDB.
     @skipIf(triple='^mips')
     @expectedFailureNetBSD
-    @skipIf(
-        oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
-        archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
-        bugnumber="rdar://93863107")
     @add_test_categories(["watchpoint"])
     def test(self):
         """Test a signal/watchpoint/breakpoint in multiple threads."""
diff --git a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointThreads.py b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointThreads.py
index 5cddeee58748..1e8a2d063286 100644
--- a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointThreads.py
+++ b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointThreads.py
@@ -11,10 +11,6 @@ class ConcurrentTwoWatchpointThreads(ConcurrentEventsBase):
 
     # Atomic sequences are not supported yet for MIPS in LLDB.
     @skipIf(triple='^mips')
-    @skipIf(
-        oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
-        archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
-        bugnumber="rdar://93863107")
     @add_test_categories(["watchpoint"])
     def test(self):
         """Test two threads that trigger a watchpoint. """
diff --git a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneBreakpoint.py b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneBreakpoint.py
index c83862d25d50..88eae0accdf4 100644
--- a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneBreakpoint.py
+++ b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneBreakpoint.py
@@ -11,10 +11,6 @@ class ConcurrentTwoWatchpointsOneBreakpoint(ConcurrentEventsBase):
 
     # Atomic sequences are not supported yet for MIPS in LLDB.
     @skipIf(triple='^mips')
-    @skipIf(
-        oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
-        archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
-        bugnumber="rdar://93863107")
     @add_test_categories(["watchpoint"])
     def test(self):
         """Test two threads that trigger a watchpoint and one breakpoint thread. """
diff --git a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneDelayBreakpoint.py b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneDelayBreakpoint.py
index f1b65220a091..8cad1213221a 100644
--- a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneDelayBreakpoint.py
+++ b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneDelayBreakpoint.py
@@ -11,10 +11,6 @@ class ConcurrentTwoWatchpointsOneDelayBreakpoint(ConcurrentEventsBase):
 
     # Atomic sequences are not supported yet for MIPS in LLDB.
     @skipIf(triple='^mips')
-    @skipIf(
-        oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
-        archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
-        bugnumber="rdar://93863107")
     @add_test_categories(["watchpoint"])
     def test(self):
         """Test two threads that trigger a watchpoint and one (1 second delay) breakpoint thread. """
diff --git a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneSignal.py b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneSignal.py
index acc08e1dd077..4cca7ecc64e7 100644
--- a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneSignal.py
+++ b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentTwoWatchpointsOneSignal.py
@@ -12,10 +12,6 @@ class ConcurrentTwoWatchpointsOneSignal(ConcurrentEventsBase):
     # Atomic sequences are not supported yet for MIPS in LLDB.
     @skipIf(triple='^mips')
     @expectedFailureNetBSD
-    @skipIf(
-        oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
-        archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
-        bugnumber="rdar://93863107")
     @add_test_categories(["watchpoint"])
     def test(self):
         """Test two threads that trigger a watchpoint and one signal thread. """
diff --git a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentWatchBreak.py b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentWatchBreak.py
index 7d3a759e09ce..1e95aa2c19f2 100644
--- a/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentWatchBreak.py
+++ b/lldb/test/API/functionalities/thread/concurrent_events/TestConcurrentWatchBreak.py
@@ -12,10 +12,6 @@ class ConcurrentWatchBreak(ConcurrentEventsBase):
     # Atomic sequences are not supported yet for MIPS in LLDB.
     @skipIf(triple='^mips')
     @add_test_categories(["watchpoint"])
-    @skipIf(
-    oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
-    archs=['arm64', 'arm64e', 'arm64_32', 'arm'],
-    bugnumber="rdar://93863107")
 
     def test(self):
         """Test watchpoint and a breakpoint in multiple threads."""