[lldb][NFC] Move BreakpointSite::IsEnabled/SetEnabled into Process

lldb/include/lldb/Breakpoint/BreakpointSite.h

@@ -77,20 +77,6 @@ class BreakpointSite : public std::enable_shared_from_this<BreakpointSite>,
                        lldb::addr_t *intersect_addr, size_t *intersect_size,
                        size_t *opcode_offset) const;
 
-  /// Tells whether the current breakpoint site is enabled or not
-  ///
-  /// This is a low-level enable bit for the breakpoint sites.  If a
-  /// breakpoint site has no enabled constituents, it should just get removed.
-  /// This enable/disable is for the low-level target code to enable and disable
-  /// breakpoint sites when single stepping, etc.
-  bool IsEnabled() const;
-
-  /// Sets whether the current breakpoint site is enabled or not
-  ///
-  /// \param[in] enabled
-  ///    \b true if the breakpoint is enabled, \b false otherwise.
-  void SetEnabled(bool enabled);
-
   /// Enquires of the breakpoint locations that produced this breakpoint site
   /// whether we should stop at this location.
   ///
@@ -223,6 +209,12 @@ class BreakpointSite : public std::enable_shared_from_this<BreakpointSite>,
   size_t RemoveConstituent(lldb::break_id_t break_id,
                            lldb::break_id_t break_loc_id);
 
+  /// Sets whether the current breakpoint site is enabled or not.
+  ///
+  /// \param[in] enabled
+  ///    \b true if the breakpoint is enabled, \b false otherwise.
+  void SetEnabled(bool enabled);
+
   BreakpointSite::Type m_type; ///< The type of this breakpoint site.
   uint8_t m_saved_opcode[8]; ///< The saved opcode bytes if this breakpoint site
                              ///uses trap opcodes.

lldb/include/lldb/Target/Process.h

@@ -2284,6 +2284,8 @@ class Process : public std::enable_shared_from_this<Process>,
 
   Status EnableBreakpointSiteByID(lldb::user_id_t break_id);
 
+  bool IsBreakpointSiteEnabled(const BreakpointSite &site);
+
   // BreakpointLocations use RemoveConstituentFromBreakpointSite to remove
   // themselves from the constituent's list of this breakpoint sites.
   void RemoveConstituentFromBreakpointSite(lldb::user_id_t site_id,
@@ -3619,6 +3621,13 @@ void PruneThreadPlans();
 
   void SetAddressableBitMasks(AddressableBits bit_masks);
 
+  // Updates the state of site.
+  // This should be used by derived Process classes after they have changed the
+  // state of a site.
+  void SetBreakpointSiteEnabled(BreakpointSite &site, bool is_enabled = true) {
+    site.SetEnabled(is_enabled);
+  }
+
 private:
   Status DestroyImpl(bool force_kill);
 

lldb/source/Breakpoint/BreakpointSite.cpp

@@ -135,8 +135,6 @@ const uint8_t *BreakpointSite::GetSavedOpcodeBytes() const {
   return &m_saved_opcode[0];
 }
 
-bool BreakpointSite::IsEnabled() const { return m_enabled; }
-
 void BreakpointSite::SetEnabled(bool enabled) { m_enabled = enabled; }
 
 void BreakpointSite::AddConstituent(const BreakpointLocationSP &constituent) {

lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp

@@ -633,9 +633,9 @@ Status ProcessKDP::EnableBreakpointSite(BreakpointSite *bp_site) {
 
   if (m_comm.LocalBreakpointsAreSupported()) {
     Status error;
-    if (!bp_site->IsEnabled()) {
+    if (!IsBreakpointSiteEnabled(*bp_site)) {
       if (m_comm.SendRequestBreakpoint(true, bp_site->GetLoadAddress())) {
-        bp_site->SetEnabled(true);
+        SetBreakpointSiteEnabled(*bp_site);
         bp_site->SetType(BreakpointSite::eExternal);
       } else {
         return Status::FromErrorString("KDP set breakpoint failed");
@@ -649,15 +649,15 @@ Status ProcessKDP::EnableBreakpointSite(BreakpointSite *bp_site) {
 Status ProcessKDP::DisableBreakpointSite(BreakpointSite *bp_site) {
   if (m_comm.LocalBreakpointsAreSupported()) {
     Status error;
-    if (bp_site->IsEnabled()) {
+    if (IsBreakpointSiteEnabled(*bp_site)) {
       BreakpointSite::Type bp_type = bp_site->GetType();
       if (bp_type == BreakpointSite::eExternal) {
         if (m_destroy_in_process && m_comm.IsRunning()) {
           // We are trying to destroy our connection and we are running
-          bp_site->SetEnabled(false);
+          SetBreakpointSiteEnabled(*bp_site, false);
         } else {
           if (m_comm.SendRequestBreakpoint(false, bp_site->GetLoadAddress()))
-            bp_site->SetEnabled(false);
+            SetBreakpointSiteEnabled(*bp_site, false);
           else
             return Status::FromErrorString("KDP remove breakpoint failed");
         }

lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp

@@ -636,7 +636,7 @@ StopInfoSP StopInfoMachException::CreateStopReasonWithMachException(
   addr_t pc = reg_ctx_sp->GetPC();
   BreakpointSiteSP bp_site_sp =
       process_sp->GetBreakpointSiteList().FindByAddress(pc);
-  if (bp_site_sp && bp_site_sp->IsEnabled())
+  if (bp_site_sp && process_sp->IsBreakpointSiteEnabled(*bp_site_sp))
     thread.SetThreadStoppedAtUnexecutedBP(pc);
 
   switch (exc_type) {
@@ -771,7 +771,7 @@ StopInfoSP StopInfoMachException::CreateStopReasonWithMachException(
       if (!bp_site_sp && reg_ctx_sp) {
         bp_site_sp = process_sp->GetBreakpointSiteList().FindByAddress(pc);
       }
-      if (bp_site_sp && bp_site_sp->IsEnabled()) {
+      if (bp_site_sp && process_sp->IsBreakpointSiteEnabled(*bp_site_sp)) {
         // We've hit this breakpoint, whether it was intended for this thread
         // or not.  Clear this in the Tread object so we step past it on resume.
         thread.SetThreadHitBreakpointSite();
@@ -865,7 +865,7 @@ bool StopInfoMachException::WasContinueInterrupted(Thread &thread) {
   // We have a hardware breakpoint -- this is the kernel bug.
   auto &bp_site_list = process_sp->GetBreakpointSiteList();
   for (auto &site : bp_site_list.Sites()) {
-    if (site->IsHardware() && site->IsEnabled()) {
+    if (site->IsHardware() && process_sp->IsBreakpointSiteEnabled(*site)) {
       LLDB_LOGF(log,
                 "Thread stopped with insn-step completed mach exception but "
                 "thread was not stepping; there is a hardware breakpoint set.");

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp

@@ -1848,7 +1848,7 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo(
     addr_t pc = thread_sp->GetRegisterContext()->GetPC();
     BreakpointSiteSP bp_site_sp =
         thread_sp->GetProcess()->GetBreakpointSiteList().FindByAddress(pc);
-    if (bp_site_sp && bp_site_sp->IsEnabled())
+    if (bp_site_sp && IsBreakpointSiteEnabled(*bp_site_sp))
       thread_sp->SetThreadStoppedAtUnexecutedBP(pc);
 
     if (exc_type != 0) {
@@ -2032,7 +2032,7 @@ ThreadSP ProcessGDBRemote::SetThreadStopInfo(
           // BreakpointSites in any other location, but we can't know for
           // sure what happened so it's a reasonable default.
           if (bp_site_sp) {
-            if (bp_site_sp->IsEnabled())
+            if (IsBreakpointSiteEnabled(*bp_site_sp))
               thread_sp->SetThreadHitBreakpointSite();
 
             if (bp_site_sp->ValidForThisThread(*thread_sp)) {
@@ -3328,23 +3328,21 @@ size_t ProcessGDBRemote::PutSTDIN(const char *src, size_t src_len,
 
 /// Enable a single breakpoint site by trying Z0 (software), then Z1
 /// (hardware), then manual memory write as a last resort.
-static llvm::Error DoEnableBreakpointSite(ProcessGDBRemote &proc,
-                                          BreakpointSite &bp_site) {
+llvm::Error ProcessGDBRemote::DoEnableBreakpointSite(BreakpointSite &bp_site) {
   Log *log = GetLog(GDBRLog::Breakpoints);
   const addr_t addr = bp_site.GetLoadAddress();
-  const size_t bp_op_size = proc.GetSoftwareBreakpointTrapOpcode(&bp_site);
-  auto &gdb_comm = proc.GetGDBRemote();
+  const size_t bp_op_size = GetSoftwareBreakpointTrapOpcode(&bp_site);
+  auto &gdb_comm = GetGDBRemote();
 
   // SupportsGDBStoppointPacket always returns true unless a previously sent
   // packet failed. As such, query the function before AND after sending the
   // packet.
   if (gdb_comm.SupportsGDBStoppointPacket(eBreakpointSoftware) &&
       !bp_site.HardwareRequired()) {
     uint8_t error_no = gdb_comm.SendGDBStoppointTypePacket(
-        eBreakpointSoftware, true, addr, bp_op_size,
-        proc.GetInterruptTimeout());
+        eBreakpointSoftware, true, addr, bp_op_size, GetInterruptTimeout());
     if (error_no == 0) {
-      bp_site.SetEnabled(true);
+      SetBreakpointSiteEnabled(bp_site);
       bp_site.SetType(BreakpointSite::eExternal);
       return llvm::Error::success();
     }
@@ -3360,10 +3358,9 @@ static llvm::Error DoEnableBreakpointSite(ProcessGDBRemote &proc,
   // Like above, this is also queried twice.
   if (gdb_comm.SupportsGDBStoppointPacket(eBreakpointHardware)) {
     uint8_t error_no = gdb_comm.SendGDBStoppointTypePacket(
-        eBreakpointHardware, true, addr, bp_op_size,
-        proc.GetInterruptTimeout());
+        eBreakpointHardware, true, addr, bp_op_size, GetInterruptTimeout());
     if (error_no == 0) {
-      bp_site.SetEnabled(true);
+      SetBreakpointSiteEnabled(bp_site);
       bp_site.SetType(BreakpointSite::eHardware);
       return llvm::Error::success();
     }
@@ -3383,38 +3380,35 @@ static llvm::Error DoEnableBreakpointSite(ProcessGDBRemote &proc,
   if (bp_site.HardwareRequired())
     return llvm::createStringError("hardware breakpoints are not supported");
 
-  return proc.EnableSoftwareBreakpoint(&bp_site).takeError();
+  return EnableSoftwareBreakpoint(&bp_site).takeError();
 }
 
 /// Disable a single breakpoint site directly by sending the appropriate
 /// z packet or restoring the original instruction.
-static llvm::Error DoDisableBreakpointSite(ProcessGDBRemote &proc,
-                                           BreakpointSite &bp_site) {
+llvm::Error ProcessGDBRemote::DoDisableBreakpointSite(BreakpointSite &bp_site) {
   const addr_t addr = bp_site.GetLoadAddress();
-  const size_t bp_op_size = proc.GetSoftwareBreakpointTrapOpcode(&bp_site);
-  auto &gdb_comm = proc.GetGDBRemote();
+  const size_t bp_op_size = GetSoftwareBreakpointTrapOpcode(&bp_site);
+  auto &gdb_comm = GetGDBRemote();
 
   switch (bp_site.GetType()) {
   case BreakpointSite::eSoftware: {
-    Status error = proc.DisableSoftwareBreakpoint(&bp_site);
+    Status error = DisableSoftwareBreakpoint(&bp_site);
     if (error.Fail())
       return error.takeError();
     break;
   }
   case BreakpointSite::eHardware:
     if (gdb_comm.SendGDBStoppointTypePacket(eBreakpointHardware, false, addr,
-                                            bp_op_size,
-                                            proc.GetInterruptTimeout()))
+                                            bp_op_size, GetInterruptTimeout()))
       return llvm::createStringError("unknown error");
     break;
   case BreakpointSite::eExternal:
     if (gdb_comm.SendGDBStoppointTypePacket(eBreakpointSoftware, false, addr,
-                                            bp_op_size,
-                                            proc.GetInterruptTimeout()))
+                                            bp_op_size, GetInterruptTimeout()))
       return llvm::createStringError("unknown error");
     break;
   }
-  bp_site.SetEnabled(false);
+  SetBreakpointSiteEnabled(bp_site, false);
   return llvm::Error::success();
 }
 
@@ -3435,15 +3429,15 @@ Status ProcessGDBRemote::EnableBreakpointSite(BreakpointSite *bp_site) {
             site_id, (uint64_t)addr);
 
   // Breakpoint already exists and is enabled
-  if (bp_site->IsEnabled()) {
+  if (IsBreakpointSiteEnabled(*bp_site)) {
     LLDB_LOGF(log,
               "ProcessGDBRemote::EnableBreakpointSite (size_id = %" PRIu64
               ") address = 0x%" PRIx64 " -- SUCCESS (already enabled)",
               site_id, (uint64_t)addr);
     return Status();
   }
 
-  return Status::FromError(DoEnableBreakpointSite(*this, *bp_site));
+  return Status::FromError(DoEnableBreakpointSite(*bp_site));
 }
 
 Status ProcessGDBRemote::DisableBreakpointSite(BreakpointSite *bp_site) {
@@ -3456,15 +3450,15 @@ Status ProcessGDBRemote::DisableBreakpointSite(BreakpointSite *bp_site) {
             ") addr = 0x%8.8" PRIx64,
             site_id, (uint64_t)addr);
 
-  if (!bp_site->IsEnabled()) {
+  if (!IsBreakpointSiteEnabled(*bp_site)) {
     LLDB_LOGF(log,
               "ProcessGDBRemote::DisableBreakpointSite (site_id = %" PRIu64
               ") addr = 0x%8.8" PRIx64 " -- SUCCESS (already disabled)",
               site_id, (uint64_t)addr);
     return Status();
   }
 
-  return Status::FromError(DoDisableBreakpointSite(*this, *bp_site));
+  return Status::FromError(DoDisableBreakpointSite(*bp_site));
 }
 
 // Pre-requisite: wp != NULL.
@@ -6118,7 +6112,7 @@ void ProcessGDBRemote::DidForkSwitchSoftwareBreakpoints(
 
   GetBreakpointSiteList().ForEach([this, enable, entry_addr,
                                    log](BreakpointSite *bp_site) {
-    if (bp_site->IsEnabled() &&
+    if (IsBreakpointSiteEnabled(*bp_site) &&
         (bp_site->GetType() == BreakpointSite::eSoftware ||
          bp_site->GetType() == BreakpointSite::eExternal)) {
       // During expression evaluation, retain the expression-return trap
@@ -6142,7 +6136,7 @@ void ProcessGDBRemote::DidForkSwitchSoftwareBreakpoints(
 void ProcessGDBRemote::DidForkSwitchHardwareTraps(bool enable) {
   if (m_gdb_comm.SupportsGDBStoppointPacket(eBreakpointHardware)) {
     GetBreakpointSiteList().ForEach([this, enable](BreakpointSite *bp_site) {
-      if (bp_site->IsEnabled() &&
+      if (IsBreakpointSiteEnabled(*bp_site) &&
           bp_site->GetType() == BreakpointSite::eHardware) {
         m_gdb_comm.SendGDBStoppointTypePacket(
             eBreakpointHardware, enable, bp_site->GetLoadAddress(),

lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h

@@ -454,6 +454,14 @@ class ProcessGDBRemote : public Process,
   std::map<uint64_t, uint32_t> m_thread_id_to_used_usec_map;
   uint64_t m_last_signals_version = 0;
 
+  /// Enable a single breakpoint site by trying Z0 (software), then Z1
+  /// (hardware), then manual memory write as a last resort.
+  llvm::Error DoEnableBreakpointSite(BreakpointSite &bp_site);
+
+  /// Disable a single breakpoint site directly by sending the appropriate
+  /// z packet or restoring the original instruction.
+  llvm::Error DoDisableBreakpointSite(BreakpointSite &bp_site);
+
   static bool NewThreadNotifyBreakpointHit(void *baton,
                                            StoppointCallbackContext *context,
                                            lldb::user_id_t break_id,

lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp

@@ -265,7 +265,7 @@ size_t ScriptedProcess::DoWriteMemory(lldb::addr_t vm_addr, const void *buf,
 Status ScriptedProcess::EnableBreakpointSite(BreakpointSite *bp_site) {
   assert(bp_site != nullptr);
 
-  if (bp_site->IsEnabled()) {
+  if (IsBreakpointSiteEnabled(*bp_site)) {
     return {};
   }
 

lldb/source/Plugins/Process/scripted/ScriptedThread.cpp

@@ -313,9 +313,10 @@ bool ScriptedThread::CalculateStopInfo() {
   // if we CreateStopReasonWithBreakpointSiteID.
   if (RegisterContextSP reg_ctx_sp = GetRegisterContext()) {
     addr_t pc = reg_ctx_sp->GetPC();
+    ProcessSP proc = GetProcess();
     if (BreakpointSiteSP bp_site_sp =
-            GetProcess()->GetBreakpointSiteList().FindByAddress(pc))
-      if (bp_site_sp->IsEnabled())
+            proc->GetBreakpointSiteList().FindByAddress(pc))
+      if (proc->IsBreakpointSiteEnabled(*bp_site_sp))
         SetThreadStoppedAtUnexecutedBP(pc);
   }
 

lldb/source/Target/Process.cpp

@@ -1546,7 +1546,6 @@ Process::GetBreakpointSiteList() const {
 
 void Process::DisableAllBreakpointSites() {
   m_breakpoint_site_list.ForEach([this](BreakpointSite *bp_site) -> void {
-    //        bp_site->SetEnabled(true);
     DisableBreakpointSite(bp_site);
   });
 }
@@ -1564,7 +1563,7 @@ Status Process::DisableBreakpointSiteByID(lldb::user_id_t break_id) {
   Status error;
   BreakpointSiteSP bp_site_sp = m_breakpoint_site_list.FindByID(break_id);
   if (bp_site_sp) {
-    if (bp_site_sp->IsEnabled())
+    if (IsBreakpointSiteEnabled(*bp_site_sp))
       error = DisableBreakpointSite(bp_site_sp.get());
   } else {
     error = Status::FromErrorStringWithFormat(
@@ -1578,7 +1577,7 @@ Status Process::EnableBreakpointSiteByID(lldb::user_id_t break_id) {
   Status error;
   BreakpointSiteSP bp_site_sp = m_breakpoint_site_list.FindByID(break_id);
   if (bp_site_sp) {
-    if (!bp_site_sp->IsEnabled())
+    if (!IsBreakpointSiteEnabled(*bp_site_sp))
       error = EnableBreakpointSite(bp_site_sp.get());
   } else {
     error = Status::FromErrorStringWithFormat(
@@ -1587,6 +1586,10 @@ Status Process::EnableBreakpointSiteByID(lldb::user_id_t break_id) {
   return error;
 }
 
+bool Process::IsBreakpointSiteEnabled(const BreakpointSite &site) {
+  return site.m_enabled;
+}
+
 static bool ShouldShowError(Process &process) {
   switch (process.GetState()) {
   case eStateInvalid:
@@ -1739,7 +1742,7 @@ Status Process::EnableSoftwareBreakpoint(BreakpointSite *bp_site) {
   LLDB_LOGF(
       log, "Process::EnableSoftwareBreakpoint (site_id = %d) addr = 0x%" PRIx64,
       bp_site->GetID(), (uint64_t)bp_addr);
-  if (bp_site->IsEnabled()) {
+  if (IsBreakpointSiteEnabled(*bp_site)) {
     LLDB_LOGF(
         log,
         "Process::EnableSoftwareBreakpoint (site_id = %d) addr = 0x%" PRIx64
@@ -1783,7 +1786,7 @@ Status Process::EnableSoftwareBreakpoint(BreakpointSite *bp_site) {
                          error) == bp_opcode_size) {
           if (::memcmp(bp_opcode_bytes, verify_bp_opcode_bytes,
                        bp_opcode_size) == 0) {
-            bp_site->SetEnabled(true);
+            SetBreakpointSiteEnabled(*bp_site);
             bp_site->SetType(BreakpointSite::eSoftware);
             LLDB_LOGF(log,
                       "Process::EnableSoftwareBreakpoint (site_id = %d) "
@@ -1825,7 +1828,7 @@ Status Process::DisableSoftwareBreakpoint(BreakpointSite *bp_site) {
   if (bp_site->IsHardware()) {
     error =
         Status::FromErrorString("Breakpoint site is a hardware breakpoint.");
-  } else if (bp_site->IsEnabled()) {
+  } else if (IsBreakpointSiteEnabled(*bp_site)) {
     const size_t break_op_size = bp_site->GetByteSize();
     const uint8_t *const break_op = bp_site->GetTrapOpcodeBytes();
     if (break_op_size > 0) {
@@ -1867,7 +1870,7 @@ Status Process::DisableSoftwareBreakpoint(BreakpointSite *bp_site) {
             if (::memcmp(bp_site->GetSavedOpcodeBytes(), verify_opcode,
                          break_op_size) == 0) {
               // SUCCESS
-              bp_site->SetEnabled(false);
+              SetBreakpointSiteEnabled(*bp_site, false);
               LLDB_LOGF(log,
                         "Process::DisableSoftwareBreakpoint (site_id = %d) "
                         "addr = 0x%" PRIx64 " -- SUCCESS",