[debugserver] Unify the breakpoint/watchpoint interface (NFCI)

[JDevlieghere]

Summary

Unify the interface for enabling and disabling breakpoints with their watchpoint counterpart. This allows both to go through DoHardwareBreakpointAction.

[jasonmolenda]

Looks good.

lldb/tools/debugserver/source/DNBArch.h

public:
   virtual uint64_t GetPC(uint64_t failValue) = 0; // Get program counter
   virtual kern_return_t SetPC(uint64_t value) = 0;
   virtual uint64_t GetSP(uint64_t failValue) = 0; // Get stack pointer
   virtual void ThreadWillResume() = 0;
   virtual bool ThreadDidStop() = 0;
   virtual bool NotifyException(MachException::Data &exc) { return false; }
   virtual uint32_t NumSupportedHardwareBreakpoints() { return 0; }
   virtual uint32_t NumSupportedHardwareWatchpoints() { return 0; }
-  virtual uint32_t EnableHardwareBreakpoint(nub_addr_t addr, nub_size_t size) {
+  virtual uint32_t EnableHardwareBreakpoint(nub_addr_t addr, nub_size_t size,
+                                            bool also_set_on_task) {
     return INVALID_NUB_HW_INDEX;
   }
   virtual uint32_t EnableHardwareWatchpoint(nub_addr_t addr, nub_size_t size,
                                             bool read, bool write,
                                             bool also_set_on_task) {
     return INVALID_NUB_HW_INDEX;
   }
-  virtual bool DisableHardwareBreakpoint(uint32_t hw_index) { return false; }
+  virtual bool DisableHardwareBreakpoint(uint32_t hw_index,
+                                         bool also_set_on_task) {
+    return false;
+  }
   virtual bool DisableHardwareWatchpoint(uint32_t hw_index,
                                          bool also_set_on_task) {
     return false;
   }
   virtual uint32_t GetHardwareWatchpointHit(nub_addr_t &addr) {
     return INVALID_NUB_HW_INDEX;
   }
   virtual bool StepNotComplete() { return false; }

lldb/tools/debugserver/source/MacOSX/MachThread.h

public:
   bool GetRegisterState(int flavor, bool force);
   bool SetRegisterState(int flavor);
   uint64_t
   GetPC(uint64_t failValue = INVALID_NUB_ADDRESS); // Get program counter
   bool SetPC(uint64_t value);                      // Set program counter
   uint64_t GetSP(uint64_t failValue = INVALID_NUB_ADDRESS); // Get stack pointer
 
   DNBBreakpoint *CurrentBreakpoint();
-  uint32_t EnableHardwareBreakpoint(const DNBBreakpoint *breakpoint);
+  uint32_t EnableHardwareBreakpoint(const DNBBreakpoint *breakpoint,
+                                    bool also_set_on_task);
   uint32_t EnableHardwareWatchpoint(const DNBBreakpoint *watchpoint,
                                     bool also_set_on_task);
-  bool DisableHardwareBreakpoint(const DNBBreakpoint *breakpoint);
+  bool DisableHardwareBreakpoint(const DNBBreakpoint *breakpoint,
+                                 bool also_set_on_task);
   bool DisableHardwareWatchpoint(const DNBBreakpoint *watchpoint,
                                  bool also_set_on_task);
   uint32_t NumSupportedHardwareWatchpoints() const;
   bool RollbackTransForHWP();
   bool FinishTransForHWP();
 
   nub_state_t GetState();
   void SetState(nub_state_t state);

lldb/tools/debugserver/source/MacOSX/MachThread.cpp

 
 uint32_t MachThread::SaveRegisterState() {
   return m_arch_up->SaveRegisterState();
 }
 bool MachThread::RestoreRegisterState(uint32_t save_id) {
   return m_arch_up->RestoreRegisterState(save_id);
 }
 
-uint32_t MachThread::EnableHardwareBreakpoint(const DNBBreakpoint *bp) {
-  if (bp != NULL && bp->IsBreakpoint())
-    return m_arch_up->EnableHardwareBreakpoint(bp->Address(), bp->ByteSize());
+uint32_t MachThread::EnableHardwareBreakpoint(const DNBBreakpoint *bp,
+                                              bool also_set_on_task) {
+  if (bp != NULL && bp->IsBreakpoint()) {
+    return m_arch_up->EnableHardwareBreakpoint(bp->Address(), bp->ByteSize(),
+                                               also_set_on_task);
+  }
   return INVALID_NUB_HW_INDEX;
 }
 
 uint32_t MachThread::EnableHardwareWatchpoint(const DNBBreakpoint *wp,
                                               bool also_set_on_task) {
   if (wp != NULL && wp->IsWatchpoint())
     return m_arch_up->EnableHardwareWatchpoint(
         wp->Address(), wp->ByteSize(), wp->WatchpointRead(),
         wp->WatchpointWrite(), also_set_on_task);
   return INVALID_NUB_HW_INDEX;
 }
 
 bool MachThread::RollbackTransForHWP() {
   return m_arch_up->RollbackTransForHWP();
 }
 
 bool MachThread::FinishTransForHWP() { return m_arch_up->FinishTransForHWP(); }
 
-bool MachThread::DisableHardwareBreakpoint(const DNBBreakpoint *bp) {
-  if (bp != NULL && bp->IsHardware())
-    return m_arch_up->DisableHardwareBreakpoint(bp->GetHardwareIndex());
+bool MachThread::DisableHardwareBreakpoint(const DNBBreakpoint *bp,
+                                           bool also_set_on_task) {
+  if (bp != NULL && bp->IsHardware()) {
+    return m_arch_up->DisableHardwareBreakpoint(bp->GetHardwareIndex(),
+                                                also_set_on_task);
+  }
   return false;
 }
 
 bool MachThread::DisableHardwareWatchpoint(const DNBBreakpoint *wp,
                                            bool also_set_on_task) {
   if (wp != NULL && wp->IsHardware())
     return m_arch_up->DisableHardwareWatchpoint(wp->GetHardwareIndex(),
                                                 also_set_on_task);

lldb/tools/debugserver/source/MacOSX/MachThreadList.h

 protected:
   typedef std::vector<MachThreadSP> collection;
   typedef collection::iterator iterator;
   typedef collection::const_iterator const_iterator;
 
   enum class HardwareBreakpointAction {
     EnableWatchpoint,
     DisableWatchpoint,
+    EnableBreakpoint,
+    DisableBreakpoint,
   };
 
-  uint32_t DoHardwareBreakpointAction(const DNBBreakpoint *wp,
+  uint32_t DoHardwareBreakpointAction(const DNBBreakpoint *bp,
                                       HardwareBreakpointAction action) const;
 
   uint32_t UpdateThreadList(MachProcess *process, bool update,
                             collection *num_threads = NULL);
   //  const_iterator  FindThreadByID (thread_t tid) const;
 
   collection m_threads;
   mutable PThreadMutex m_threads_mutex;
   MachThreadSP m_current_thread;
   bool m_is_64_bit;
 };
 
 #endif // #ifndef __MachThreadList_h__

lldb/tools/debugserver/source/MacOSX/MachThreadList.cpp

 void MachThreadList::NotifyBreakpointChanged(const DNBBreakpoint *bp) {
   PTHREAD_MUTEX_LOCKER(locker, m_threads_mutex);
   const size_t num_threads = m_threads.size();
   for (uint32_t idx = 0; idx < num_threads; ++idx) {
     m_threads[idx]->NotifyBreakpointChanged(bp);
   }
 }
 
-uint32_t
-MachThreadList::EnableHardwareBreakpoint(const DNBBreakpoint *bp) const {
-  if (bp != NULL) {
-    const size_t num_threads = m_threads.size();
-    for (uint32_t idx = 0; idx < num_threads; ++idx)
-      m_threads[idx]->EnableHardwareBreakpoint(bp);
-  }
-  return INVALID_NUB_HW_INDEX;
-}
-
-bool MachThreadList::DisableHardwareBreakpoint(const DNBBreakpoint *bp) const {
-  if (bp != NULL) {
-    const size_t num_threads = m_threads.size();
-    for (uint32_t idx = 0; idx < num_threads; ++idx)
-      m_threads[idx]->DisableHardwareBreakpoint(bp);
-  }
-  return false;
-}
-
 uint32_t MachThreadList::DoHardwareBreakpointAction(
-    const DNBBreakpoint *wp, HardwareBreakpointAction action) const {
-  if (wp == NULL)
+    const DNBBreakpoint *bp, HardwareBreakpointAction action) const {
+  if (bp == NULL)
     return INVALID_NUB_HW_INDEX;
 
   uint32_t hw_index = INVALID_NUB_HW_INDEX;
   PTHREAD_MUTEX_LOCKER(locker, m_threads_mutex);
   const size_t num_threads = m_threads.size();
   // On Mac OS X we have to prime the control registers for new threads.  We do
   // this using the control register data for the first thread, for lack of a
   // better way of choosing.
   bool also_set_on_task = true;
   for (uint32_t idx = 0; idx < num_threads; ++idx) {
     switch (action) {
     case HardwareBreakpointAction::EnableWatchpoint:
-      hw_index = m_threads[idx]->EnableHardwareWatchpoint(wp, also_set_on_task);
+      hw_index = m_threads[idx]->EnableHardwareWatchpoint(bp, also_set_on_task);
       break;
     case HardwareBreakpointAction::DisableWatchpoint:
       hw_index =
-          m_threads[idx]->DisableHardwareWatchpoint(wp, also_set_on_task);
+          m_threads[idx]->DisableHardwareWatchpoint(bp, also_set_on_task);
+      break;
+    case HardwareBreakpointAction::EnableBreakpoint:
+      hw_index = m_threads[idx]->EnableHardwareBreakpoint(bp, also_set_on_task);
+      break;
+    case HardwareBreakpointAction::DisableBreakpoint:
+      hw_index =
+          m_threads[idx]->DisableHardwareBreakpoint(bp, also_set_on_task);
       break;
     }
     if (hw_index == INVALID_NUB_HW_INDEX) {
       // We know that idx failed for some reason.  Let's rollback the
       // transaction for [0, idx).
       for (uint32_t i = 0; i < idx; ++i)
         m_threads[i]->RollbackTransForHWP();
       return INVALID_NUB_HW_INDEX;

 }
 
 bool MachThreadList::DisableHardwareWatchpoint(const DNBBreakpoint *wp) const {
   return DoHardwareBreakpointAction(
              wp, HardwareBreakpointAction::DisableWatchpoint) !=
          INVALID_NUB_HW_INDEX;
 }
 
+uint32_t
+MachThreadList::EnableHardwareBreakpoint(const DNBBreakpoint *bp) const {
+  return DoHardwareBreakpointAction(bp,
+                                    HardwareBreakpointAction::EnableBreakpoint);
+}
+
+bool MachThreadList::DisableHardwareBreakpoint(const DNBBreakpoint *bp) const {
+  return DoHardwareBreakpointAction(
+             bp, HardwareBreakpointAction::DisableBreakpoint) !=
+         INVALID_NUB_HW_INDEX;
+}
+
 uint32_t MachThreadList::NumSupportedHardwareWatchpoints() const {
   PTHREAD_MUTEX_LOCKER(locker, m_threads_mutex);
   const size_t num_threads = m_threads.size();
   // Use an arbitrary thread to retrieve the number of supported hardware
   // watchpoints.
   if (num_threads)
     return m_threads[0]->NumSupportedHardwareWatchpoints();
   return 0;