[Lldb-commits] [lldb] r285742 - Switch SBWatchpoint::SetEnabled over to using Process::{Enable, Disable}Watchpoint.

[Jim Ingham via lldb-commits]

Author: jingham
Date: Tue Nov  1 15:37:02 2016
New Revision: 285742

URL: http://llvm.org/viewvc/llvm-project?rev=285742&view=rev
Log:
Switch SBWatchpoint::SetEnabled over to using Process::{Enable,Disable}Watchpoint.

We don't have a good story for what happens to watchpoints when you don't
have a process, or if your process exits.  Clearing that up will instruct 
how to fix this for real.

Also added a test to make sure disable->enable works as well.
This resolves llvm.org/pr30789.

Modified:
    lldb/trunk/include/lldb/Breakpoint/Watchpoint.h
    lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_disable/TestWatchpointDisable.py
    lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_disable/main.c
    lldb/trunk/source/API/SBWatchpoint.cpp

Modified: lldb/trunk/include/lldb/Breakpoint/Watchpoint.h
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/include/lldb/Breakpoint/Watchpoint.h?rev=285742&r1=285741&r2=285742&view=diff
==============================================================================
--- lldb/trunk/include/lldb/Breakpoint/Watchpoint.h (original)
+++ lldb/trunk/include/lldb/Breakpoint/Watchpoint.h Tue Nov  1 15:37:02 2016
@@ -71,6 +71,10 @@ public:
 
   bool IsEnabled() const;
 
+  // This doesn't really enable/disable the watchpoint.  
+  // It is currently just for use in the Process plugin's
+  // {Enable,Disable}Watchpoint, which should be used instead.
+  
   void SetEnabled(bool enabled, bool notify = true);
 
   bool IsHardware() const override;

Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_disable/TestWatchpointDisable.py
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_disable/TestWatchpointDisable.py?rev=285742&r1=285741&r2=285742&view=diff
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_disable/TestWatchpointDisable.py (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_disable/TestWatchpointDisable.py Tue Nov  1 15:37:02 2016
@@ -21,13 +21,21 @@ class TestWatchpointSetEnable(TestBase):
     @expectedFailureAll(
         oslist=["windows"],
         bugnumber="llvm.org/pr24446: WINDOWS XFAIL TRIAGE - Watchpoints not supported on Windows")
-    @expectedFailureAll(bugnumber="llvm.org/pr30789, <rdar://problem/28944061>")
     def test_disable_works (self):
         """Set a watchpoint, disable it, and make sure it doesn't get hit."""
         self.build()
-        self.disable_works()
+        self.do_test(False)
 
-    def disable_works(self):
+    @expectedFailureAndroid(archs=['arm', 'aarch64'])
+    @expectedFailureAll(
+        oslist=["windows"],
+        bugnumber="llvm.org/pr24446: WINDOWS XFAIL TRIAGE - Watchpoints not supported on Windows")
+    def test_disable_enable_works (self):
+        """Set a watchpoint, disable it, and make sure it doesn't get hit."""
+        self.build()
+        self.do_test(True)
+
+    def do_test(self, test_enable):
         """Set a watchpoint, disable it and make sure it doesn't get hit."""
 
         exe = 'a.out'
@@ -69,7 +77,12 @@ class TestWatchpointSetEnable(TestBase):
         
         stop_reason = thread.GetStopReason()
 
-        self.assertEqual(stop_reason, lldb.eStopReasonWatchpoint, "We didn't stop at our watchpoint.")
+        self.assertEqual(stop_reason, lldb.eStopReasonBreakpoint, "We didn't stop at our breakpoint.")
 
-        
+        if test_enable:
+            wp.SetEnabled(True)
+            self.assertTrue(wp.IsEnabled(), "The watchpoint thinks it is still disabled.")
+            process.Continue()
+            stop_reason = thread.GetStopReason()
+            self.assertEqual(stop_reason, lldb.eStopReasonWatchpoint, "We didn't stop at our watchpoint")
         

Modified: lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_disable/main.c
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_disable/main.c?rev=285742&r1=285741&r2=285742&view=diff
==============================================================================
--- lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_disable/main.c (original)
+++ lldb/trunk/packages/Python/lldbsuite/test/functionalities/watchpoint/watchpoint_disable/main.c Tue Nov  1 15:37:02 2016
@@ -8,5 +8,6 @@ main()
   printf("Set a breakpoint here: %d.\n", global_var);
   global_var = 20;
   printf("We should have stopped on the previous line: %d.\n", global_var);
+  global_var = 30;
   return 0;
 }

Modified: lldb/trunk/source/API/SBWatchpoint.cpp
URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/API/SBWatchpoint.cpp?rev=285742&r1=285741&r2=285742&view=diff
==============================================================================
--- lldb/trunk/source/API/SBWatchpoint.cpp (original)
+++ lldb/trunk/source/API/SBWatchpoint.cpp Tue Nov  1 15:37:02 2016
@@ -19,6 +19,7 @@
 #include "lldb/Core/Log.h"
 #include "lldb/Core/Stream.h"
 #include "lldb/Core/StreamFile.h"
+#include "lldb/Target/Process.h"
 #include "lldb/Target/Target.h"
 #include "lldb/lldb-defines.h"
 #include "lldb/lldb-types.h"
@@ -126,9 +127,17 @@ size_t SBWatchpoint::GetWatchSize() {
 void SBWatchpoint::SetEnabled(bool enabled) {
   lldb::WatchpointSP watchpoint_sp(GetSP());
   if (watchpoint_sp) {
-    std::lock_guard<std::recursive_mutex> guard(
-        watchpoint_sp->GetTarget().GetAPIMutex());
-    watchpoint_sp->SetEnabled(enabled);
+    Target &target = watchpoint_sp->GetTarget();
+    std::lock_guard<std::recursive_mutex> guard(target.GetAPIMutex());
+    ProcessSP process_sp = target.GetProcessSP();
+    if (process_sp) {
+      if (enabled)
+        process_sp->EnableWatchpoint(watchpoint_sp.get(), false);
+      else
+        process_sp->DisableWatchpoint(watchpoint_sp.get(), false);
+    } else {
+      watchpoint_sp->SetEnabled(enabled);
+    }
   }
 }