Fix a problem with "watchpoint triggers before" watchpoint handling.

We need to step the watchpoint instruction in these cases, but the
when we queued the ThreadPlanStepOverWatchpoint to do this, we didn't
make it a Controlling plan.  So if you are stepping, this plan returns as
though it were a utility plan, and the stepping plan keeps going.

This only partially fixes the problem on Darwin; there's another bug
with reporting a watchpoint when we're instruction single stepping over
an instruction that triggers a watchpoint.  The kernel reports the
"single step completed" but not the watchpoint hit.  So this commit
also refactors the test into a part that works (at least on Darwin) and
a part that still fails.

We may have to adjust the test result expectations for other systems after
this fix.

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

diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index 3ffacb5..6ce38f6 100644 (file)
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -311,6 +311,14 @@ public:
       return m_last_natural_stop_event;
     return lldb::EventSP();
   }
+  
+  void SetSafeToCallFunctions(bool safe) {
+    m_safe = safe;
+  }
+  
+  bool GetSafeToCallFunctions() {
+    return m_safe;
+  }
 
 private:
   uint32_t m_stop_id = 0;
@@ -321,6 +329,7 @@ private:
   uint32_t m_running_user_expression = false;
   uint32_t m_running_utility_function = 0;
   lldb::EventSP m_last_natural_stop_event;
+  std::atomic<bool> m_safe = true;
 };
 
 inline bool operator==(const ProcessModID &lhs, const ProcessModID &rhs) {
@@ -459,7 +468,7 @@ public:
     void SetRestarted(bool new_value) { m_restarted = new_value; }
 
     void SetInterrupted(bool new_value) { m_interrupted = new_value; }
-
+    
     void AddRestartedReason(const char *reason) {
       m_restarted_reasons.push_back(reason);
     }
@@ -1250,6 +1259,14 @@ public:
                 DiagnosticManager &diagnostic_manager);
 
   static const char *ExecutionResultAsCString(lldb::ExpressionResults result);
+  
+  void SetSafeToCallFunctions(bool safe) {
+    GetModID().SetSafeToCallFunctions(safe);
+  }
+  
+  bool GetSafeToCallFunctions() {
+    return GetModID().GetSafeToCallFunctions();
+  }
 
   void GetStatus(Stream &ostrm);
 

diff --git a/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp b/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
index aae15b2..458d44f 100644 (file)
--- a/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
+++ b/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
@@ -795,6 +795,19 @@ StopInfoSP StopInfoMachException::CreateStopReasonWithMachException(
   case 9:  // EXC_RPC_ALERT
   case 10: // EXC_CRASH
     break;
+  case 12: // EXC_GUARD
+    {
+      // Some EXC_GUARD exceptions are fatal, and the process will go away
+      // the next time you allow it to run.  When we get one of those 
+      // exceptions we have to make sure SafeToCallFunctions returns false to
+      // prevent us or other agents running the process.  This has to be set
+      // on the process because even the threads that didn't get the exception
+      // can't run.
+      ProcessSP process_sp(thread.GetProcess());
+      if (process_sp)
+        process_sp->SetSafeToCallFunctions(false);
+      
+    }
   }
 
   return StopInfoSP(new StopInfoMachException(thread, exc_type, exc_data_count,

diff --git a/lldb/source/Target/StopInfo.cpp b/lldb/source/Target/StopInfo.cpp
index 9fdb29f..ebc355c 100644 (file)
--- a/lldb/source/Target/StopInfo.cpp
+++ b/lldb/source/Target/StopInfo.cpp
@@ -831,6 +831,11 @@ protected:
           = std::static_pointer_cast<StopInfoWatchpoint>(shared_from_this());
       ThreadPlanSP step_over_wp_sp(new ThreadPlanStepOverWatchpoint(
           *(thread_sp.get()), me_as_siwp_sp, wp_sp));
+      // When this plan is done we want to stop, so set this as a Controlling
+      // plan.    
+      step_over_wp_sp->SetIsControllingPlan(true);
+      step_over_wp_sp->SetOkayToDiscard(false);
+
       Status error;
       error = thread_sp->QueueThreadPlan(step_over_wp_sp, false);
       // If we couldn't push the thread plan, just stop here:

diff --git a/lldb/source/Target/Thread.cpp b/lldb/source/Target/Thread.cpp
index d620f74..df8bff5 100644 (file)
--- a/lldb/source/Target/Thread.cpp
+++ b/lldb/source/Target/Thread.cpp
@@ -1664,6 +1664,9 @@ addr_t Thread::GetThreadLocalData(const ModuleSP module,
 bool Thread::SafeToCallFunctions() {
   Process *process = GetProcess().get();
   if (process) {
+    if (!process->SafeToCallFunctions())
+      return false;
+
     DynamicLoader *loader = GetProcess()->GetDynamicLoader();
     if (loader && loader->IsFullyInitialized() == false)
       return false;

diff --git a/lldb/test/API/commands/watchpoints/step_over_watchpoint/TestStepOverWatchpoint.py b/lldb/test/API/commands/watchpoints/step_over_watchpoint/TestStepOverWatchpoint.py
index 7d54156..52fc899 100644 (file)
--- a/lldb/test/API/commands/watchpoints/step_over_watchpoint/TestStepOverWatchpoint.py
+++ b/lldb/test/API/commands/watchpoints/step_over_watchpoint/TestStepOverWatchpoint.py
@@ -11,36 +11,11 @@ from lldbsuite.test import lldbutil
 class TestStepOverWatchpoint(TestBase):
     NO_DEBUG_INFO_TESTCASE = True
 
-    @expectedFailureAll(
-        oslist=["freebsd", "linux"],
-        archs=[
-            'aarch64',
-            'arm'],
-        bugnumber="llvm.org/pr26031")
-    # Read-write watchpoints not supported on SystemZ
-    @expectedFailureAll(archs=['s390x'])
-    @expectedFailureAll(
-        oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
-        archs=['aarch64', 'arm'],
-        bugnumber="<rdar://problem/34027183>")
-    @add_test_categories(["basic_process"])
-    def test(self):
+    def get_to_start(self, bkpt_text):
         """Test stepping over watchpoints."""
         self.build()
-        target = self.createTestTarget()
-
-        lldbutil.run_break_set_by_symbol(self, 'main')
-
-        process = target.LaunchSimple(None, None,
-                                      self.get_process_working_directory())
-        self.assertTrue(process.IsValid(), PROCESS_IS_VALID)
-        self.assertState(process.GetState(), lldb.eStateStopped,
-                         PROCESS_STOPPED)
-
-        thread = lldbutil.get_stopped_thread(process,
-                                             lldb.eStopReasonBreakpoint)
-        self.assertTrue(thread.IsValid(), "Failed to get thread.")
-
+        target, process, thread, bkpt = lldbutil.run_to_source_breakpoint(self, bkpt_text,
+                                                                       lldb.SBFileSpec("main.c"))
         frame = thread.GetFrameAtIndex(0)
         self.assertTrue(frame.IsValid(), "Failed to get frame.")
 
@@ -55,14 +30,45 @@ class TestStepOverWatchpoint(TestBase):
         self.assertSuccess(error, "Error while setting watchpoint")
         self.assertTrue(read_watchpoint, "Failed to set read watchpoint.")
 
+        # Disable the breakpoint we hit so we don't muddy the waters with
+        # stepping off from the breakpoint:
+        bkpt.SetEnabled(False)
+        
+        return (target, process, thread, read_watchpoint)
+    
+    @expectedFailureAll(
+        oslist=["freebsd", "linux"],
+        archs=[
+            'aarch64',
+            'arm'],
+        bugnumber="llvm.org/pr26031")
+    # Read-write watchpoints not supported on SystemZ
+    @expectedFailureAll(archs=['s390x'])
+    @add_test_categories(["basic_process"])
+    def test_step_over(self):
+        target, process, thread, wp = self.get_to_start("Set a breakpoint here")
+    
         thread.StepOver()
         self.assertStopReason(thread.GetStopReason(), lldb.eStopReasonWatchpoint,
                         STOPPED_DUE_TO_WATCHPOINT)
         self.assertEquals(thread.GetStopDescription(20), 'watchpoint 1')
 
-        process.Continue()
-        self.assertState(process.GetState(), lldb.eStateStopped,
-                         PROCESS_STOPPED)
+    @expectedFailureAll(
+        oslist=["freebsd", "linux"],
+        archs=[
+            'aarch64',
+            'arm'],
+        bugnumber="llvm.org/pr26031")
+    # Read-write watchpoints not supported on SystemZ
+    @expectedFailureAll(archs=['s390x'])
+    @expectedFailureAll(
+        oslist=["ios", "watchos", "tvos", "bridgeos", "macosx"],
+        archs=['aarch64', 'arm'],
+        bugnumber="<rdar://problem/34027183>")
+    @add_test_categories(["basic_process"])
+    def test_step_instruction(self):
+        target, process, thread, wp = self.get_to_start("Set breakpoint after call")
+
         self.assertEquals(thread.GetStopDescription(20), 'step over')
 
         self.step_inst_for_watchpoint(1)

diff --git a/lldb/test/API/commands/watchpoints/step_over_watchpoint/main.c b/lldb/test/API/commands/watchpoints/step_over_watchpoint/main.c
index 2d87d9a..e48d43c 100644 (file)
--- a/lldb/test/API/commands/watchpoints/step_over_watchpoint/main.c
+++ b/lldb/test/API/commands/watchpoints/step_over_watchpoint/main.c
@@ -11,8 +11,8 @@ void watch_write() {
 }
 
 int main() {
-    watch_read();
-    g_temp = g_watch_me_read;
+    watch_read(); // Set a breakpoint here
+    g_temp = g_watch_me_read; // Set breakpoint after call
     watch_write();
     g_watch_me_write = g_temp;
     return 0;