Fix a race condition between "ephemeral watchpoint disable/enable" and continue in commands.

Also, watchpoint commands, like breakpoint commands, need to run in async mode.

This was causing intermittent failures in TestWatchpointCommandPython.py, which is now solid.

llvm-svn: 284795

lldb/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_commands/command/TestWatchpointCommandPython.py

@@ -54,9 +54,6 @@ class WatchpointPythonCommandTestCase(TestBase):
         # Add a breakpoint to set a watchpoint when stopped on the breakpoint.
         lldbutil.run_break_set_by_file_and_line(
             self, None, self.line, num_expected_locations=1)
-#        self.expect("breakpoint set -l %d" % self.line, BREAKPOINT_CREATED,
-#            startstr = "Breakpoint created: 1: file ='%s', line = %d, locations = 1" %
-#                       (self.source, self.line))#
 
         # Run the program.
         self.runCmd("run", RUN_SUCCEEDED)
@@ -131,9 +128,6 @@ class WatchpointPythonCommandTestCase(TestBase):
         # Add a breakpoint to set a watchpoint when stopped on the breakpoint.
         lldbutil.run_break_set_by_file_and_line(
             self, None, self.line, num_expected_locations=1)
-#        self.expect("breakpoint set -l %d" % self.line, BREAKPOINT_CREATED,
-#            startstr = "Breakpoint created: 1: file ='%s', line = %d, locations = 1" %
-#                       (self.source, self.line))#
 
         # Run the program.
         self.runCmd("run", RUN_SUCCEEDED)
@@ -177,3 +171,4 @@ class WatchpointPythonCommandTestCase(TestBase):
         # second hit and set it to 999
         self.expect("frame variable --show-globals cookie",
                     substrs=['(int32_t)', 'cookie = 999'])
+

lldb/source/Breakpoint/Watchpoint.cpp

@@ -232,7 +232,7 @@ void Watchpoint::TurnOffEphemeralMode() {
 }
 
 bool Watchpoint::IsDisabledDuringEphemeralMode() {
-  return m_disabled_count > 1;
+  return m_disabled_count > 1 && m_is_ephemeral;
 }
 
 void Watchpoint::SetEnabled(bool enabled, bool notify) {

lldb/source/Target/StopInfo.cpp

@@ -557,27 +557,45 @@ public:
   // performing watchpoint actions.
   class WatchpointSentry {
   public:
-    WatchpointSentry(Process *p, Watchpoint *w) : process(p), watchpoint(w) {
-      if (process && watchpoint) {
+    WatchpointSentry(ProcessSP p_sp, WatchpointSP w_sp) : process_sp(p_sp), 
+                     watchpoint_sp(w_sp) {
+      if (process_sp && watchpoint_sp) {
         const bool notify = false;
-        watchpoint->TurnOnEphemeralMode();
-        process->DisableWatchpoint(watchpoint, notify);
+        watchpoint_sp->TurnOnEphemeralMode();
+        process_sp->DisableWatchpoint(watchpoint_sp.get(), notify);
+        process_sp->AddPreResumeAction(SentryPreResumeAction, this);
       }
     }
-
-    ~WatchpointSentry() {
-      if (process && watchpoint) {
-        if (!watchpoint->IsDisabledDuringEphemeralMode()) {
-          const bool notify = false;
-          process->EnableWatchpoint(watchpoint, notify);
+    
+    void DoReenable() {
+      if (process_sp && watchpoint_sp) {
+        bool was_disabled = watchpoint_sp->IsDisabledDuringEphemeralMode();
+        watchpoint_sp->TurnOffEphemeralMode();
+        const bool notify = false;
+        if (was_disabled) {
+          process_sp->DisableWatchpoint(watchpoint_sp.get(), notify);
+        } else {
+          process_sp->EnableWatchpoint(watchpoint_sp.get(), notify);
         }
-        watchpoint->TurnOffEphemeralMode();
       }
     }
+    
+    ~WatchpointSentry() {
+        DoReenable();
+        if (process_sp)
+            process_sp->ClearPreResumeAction(SentryPreResumeAction, this);
+    }
+    
+    static bool SentryPreResumeAction(void *sentry_void) {
+        WatchpointSentry *sentry = (WatchpointSentry *) sentry_void;
+        sentry->DoReenable();
+        return true;
+    }
 
   private:
-    Process *process;
-    Watchpoint *watchpoint;
+    ProcessSP process_sp;
+    WatchpointSP watchpoint_sp;
+    bool sentry_triggered = false;
   };
 
   StopInfoWatchpoint(Thread &thread, break_id_t watch_id,
@@ -659,12 +677,12 @@ protected:
               GetValue()));
       if (wp_sp) {
         ExecutionContext exe_ctx(thread_sp->GetStackFrameAtIndex(0));
-        Process *process = exe_ctx.GetProcessPtr();
+        ProcessSP process_sp = exe_ctx.GetProcessSP();
 
         // This sentry object makes sure the current watchpoint is disabled
         // while performing watchpoint actions,
         // and it is then enabled after we are finished.
-        WatchpointSentry sentry(process, wp_sp.get());
+        WatchpointSentry sentry(process_sp, wp_sp);
 
         {
           // check if this process is running on an architecture where
@@ -672,10 +690,10 @@ protected:
           // before the associated instruction runs. if so, disable the WP,
           // single-step and then
           // re-enable the watchpoint
-          if (process) {
+          if (process_sp) {
             uint32_t num;
             bool wp_triggers_after;
-            if (process->GetWatchpointSupportInfo(num, wp_triggers_after)
+            if (process_sp->GetWatchpointSupportInfo(num, wp_triggers_after)
                     .Success()) {
               if (!wp_triggers_after) {
                 StopInfoSP stored_stop_info_sp = thread_sp->GetStopInfo();
@@ -689,10 +707,10 @@ protected:
                 new_plan_sp->SetIsMasterPlan(true);
                 new_plan_sp->SetOkayToDiscard(false);
                 new_plan_sp->SetPrivate(true);
-                process->GetThreadList().SetSelectedThreadByID(
+                process_sp->GetThreadList().SetSelectedThreadByID(
                     thread_sp->GetID());
-                process->ResumeSynchronous(nullptr);
-                process->GetThreadList().SetSelectedThreadByID(
+                process_sp->ResumeSynchronous(nullptr);
+                process_sp->GetThreadList().SetSelectedThreadByID(
                     thread_sp->GetID());
                 thread_sp->SetStopInfo(stored_stop_info_sp);
               }
@@ -739,6 +757,8 @@ protected:
         if (wp_sp->GetHitCount() <= wp_sp->GetIgnoreCount())
           m_should_stop = false;
 
+        Debugger &debugger = exe_ctx.GetTargetRef().GetDebugger();
+
         if (m_should_stop && wp_sp->GetConditionText() != nullptr) {
           // We need to make sure the user sees any parse errors in their
           // condition, so we'll hook the
@@ -778,7 +798,6 @@ protected:
               }
             }
           } else {
-            Debugger &debugger = exe_ctx.GetTargetRef().GetDebugger();
             StreamSP error_sp = debugger.GetAsyncErrorStream();
             error_sp->Printf(
                 "Stopped due to an error evaluating condition of watchpoint ");
@@ -800,8 +819,20 @@ protected:
         // If the condition says to stop, we run the callback to further decide
         // whether to stop.
         if (m_should_stop) {
+            // FIXME: For now the callbacks have to run in async mode - the
+            // first time we restart we need
+            // to get out of there.  So set it here.
+            // When we figure out how to nest watchpoint hits then this will
+            // change.
+
+          bool old_async = debugger.GetAsyncExecution();
+          debugger.SetAsyncExecution(true);
+          
           StoppointCallbackContext context(event_ptr, exe_ctx, false);
           bool stop_requested = wp_sp->InvokeCallback(&context);
+          
+          debugger.SetAsyncExecution(old_async);
+          
           // Also make sure that the callback hasn't continued the target.
           // If it did, when we'll set m_should_stop to false and get out of
           // here.