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

[felipepiovezan]

The Process class is the one responsible for managing the state of a BreakpointSite inside the process. As such, it should be the one answering questions about the state of the site.

Future patches will make this even more important by introducing a
"logical" is enabled, by delaying the moment in which breakpoints are
actually updated in the process.

The following PRs are related to the MultiBreakpoint feature:

• [lldb] Propose MultiBreakpoint extension to GDB Remote #192910
• [debugserver] Implement MultiBreakpoint #192914
• [lldbremote][NFC] Factor out code handling breakpoint packets #192915
• [lldbremote] Implement support for MultiBreakpoint packet #192919
• [lldb][GDBRemote] Parse MultiBreakpoint+ capability #192962
• [lldb][NFC] Move BreakpointSite::IsEnabled/SetEnabled into Process #192964
• [lldb] Implement delayed breakpoints #192971
• [lldb] Override UpdateBreakpointSites in ProcessGDBRemote to use MultiBreakpoint #192988

[llvmbot]

@llvm/pr-subscribers-lldb

Author: Felipe de Azevedo Piovezan (felipepiovezan)

Changes

The Process class is the one responsible for managing the state of a BreakpointSite inside the process. As such, it should be the one answering questions about the state of the site.

The following PRs are related to the MultiBreakpoint feature:

• [lldb] Propose MultiBreakpoint extension to GDB Remote #192910
• [debugserver] Implement MultiBreakpoint #192914
• [lldbremote][NFC] Factor out code handling breakpoint packets #192915
• [lldbremote] Implement support for MultiBreakpoint packet #192919
• [lldb][GDBRemote] Parse MultiBreakpoint+ capability #192962
• [lldb][NFC] Move BreakpointSite::IsEnabled/SetEnabled into Process #192964
• [lldb] Implement delayed breakpoints #192971
• [lldb] Override UpdateBreakpointSites in ProcessGDBRemote to use MultiBreakpoint #192988

---

Patch is 25.95 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/192964.diff

14 Files Affected:

• (modified) lldb/include/lldb/Breakpoint/BreakpointSite.h (+6-14)
• (modified) lldb/include/lldb/Target/Process.h (+10)
• (modified) lldb/source/Breakpoint/BreakpointSite.cpp (-2)
• (modified) lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp (+5-5)
• (modified) lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp (+3-3)
• (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp (+10)
• (modified) lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h (+3)
• (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp (+23-29)
• (modified) lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h (+8)
• (modified) lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp (+1-1)
• (modified) lldb/source/Plugins/Process/scripted/ScriptedThread.cpp (+3-2)
• (modified) lldb/source/Target/Process.cpp (+16-7)
• (modified) lldb/source/Target/ThreadPlanStepOverBreakpoint.cpp (+1-1)
• (modified) lldb/unittests/Process/gdb-remote/GDBRemoteCommunicationClientTest.cpp (+22)

diff --git a/lldb/include/lldb/Breakpoint/BreakpointSite.h b/lldb/include/lldb/Breakpoint/BreakpointSite.h
index e189ed77e261b..e40eb8e3545c1 100644
--- a/lldb/include/lldb/Breakpoint/BreakpointSite.h
+++ b/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.
diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index e0ea6379e967e..f90c11813ba48 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -2284,6 +2284,9 @@ class Process : public std::enable_shared_from_this<Process>,
 
   Status EnableBreakpointSiteByID(lldb::user_id_t break_id);
 
+  bool IsBreakpointSiteEnabled(const lldb::BreakpointSiteSP &site);
+  bool IsBreakpointSiteEnabled(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 +3622,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);
 
diff --git a/lldb/source/Breakpoint/BreakpointSite.cpp b/lldb/source/Breakpoint/BreakpointSite.cpp
index 8639379afe1df..584ff1ff52c07 100644
--- a/lldb/source/Breakpoint/BreakpointSite.cpp
+++ b/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) {
diff --git a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp b/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
index e81a5dea193a2..6b3354aad09e6 100644
--- a/lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
+++ b/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");
         }
diff --git a/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp b/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
index d374742b5722c..9951fe1d9e293 100644
--- a/lldb/source/Plugins/Process/Utility/StopInfoMachException.cpp
+++ b/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.");
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
index 43c6490d5d889..5f397071c8be6 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
@@ -217,6 +217,12 @@ bool GDBRemoteCommunicationClient::GetMultiMemReadSupported() {
   return m_supports_multi_mem_read == eLazyBoolYes;
 }
 
+bool GDBRemoteCommunicationClient::GetMultiBreakpointSupported() {
+  if (m_supports_multi_breakpoint == eLazyBoolCalculate)
+    GetRemoteQSupported();
+  return m_supports_multi_breakpoint == eLazyBoolYes;
+}
+
 bool GDBRemoteCommunicationClient::QueryNoAckModeSupported() {
   if (m_supports_not_sending_acks == eLazyBoolCalculate) {
     m_send_acks = true;
@@ -346,6 +352,7 @@ void GDBRemoteCommunicationClient::ResetDiscoverableSettings(bool did_exec) {
     m_supported_async_json_packets_sp.reset();
     m_supports_jModulesInfo = true;
     m_supports_multi_mem_read = eLazyBoolCalculate;
+    m_supports_multi_breakpoint = eLazyBoolCalculate;
   }
 
   // These flags should be reset when we first connect to a GDB server and when
@@ -373,6 +380,7 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
   m_supports_reverse_continue = eLazyBoolNo;
   m_supports_reverse_step = eLazyBoolNo;
   m_supports_multi_mem_read = eLazyBoolNo;
+  m_supports_multi_breakpoint = eLazyBoolNo;
 
   m_max_packet_size = UINT64_MAX; // It's supposed to always be there, but if
                                   // not, we assume no limit
@@ -434,6 +442,8 @@ void GDBRemoteCommunicationClient::GetRemoteQSupported() {
         m_supports_reverse_step = eLazyBoolYes;
       else if (x == "MultiMemRead+")
         m_supports_multi_mem_read = eLazyBoolYes;
+      else if (x == "jMultiBreakpoint+")
+        m_supports_multi_breakpoint = eLazyBoolYes;
       // Look for a list of compressions in the features list e.g.
       // qXfer:features:read+;PacketSize=20000;qEcho+;SupportedCompressions=zlib-
       // deflate,lzma
diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
index 6b5ed99583439..79ca0bcd3ed22 100644
--- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
+++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.h
@@ -344,6 +344,8 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase {
 
   bool GetMultiMemReadSupported();
 
+  bool GetMultiBreakpointSupported();
+
   LazyBool SupportsAllocDeallocMemory() // const
   {
     // Uncomment this to have lldb pretend the debug server doesn't respond to
@@ -579,6 +581,7 @@ class GDBRemoteCommunicationClient : public GDBRemoteClientBase {
   LazyBool m_supports_reverse_continue = eLazyBoolCalculate;
   LazyBool m_supports_reverse_step = eLazyBoolCalculate;
   LazyBool m_supports_multi_mem_read = eLazyBoolCalculate;
+  LazyBool m_supports_multi_breakpoint = eLazyBoolCalculate;
 
   bool m_supports_qProcessInfoPID : 1, m_supports_qfProcessInfo : 1,
       m_supports_qUserName : 1, m_supports_qGroupName : 1,
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index 188f47dce1814..145d69d6b6d64 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/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,12 +3328,11 @@ 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
@@ -3341,10 +3340,9 @@ static llvm::Error DoEnableBreakpointSite(ProcessGDBRemote &proc,
   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,7 +3429,7 @@ 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)",
@@ -3443,7 +3437,7 @@ Status ProcessGDBRemote::EnableBreakpointSite(BreakpointSite *bp_site) {
     return Status();
   }
 
-  return Status::FromError(DoEnableBreakpointSite(*this, *bp_site));
+  return Status::FromError(DoEnableBreakpointSite(*bp_site));
 }
 
 Status ProcessGDBRemote::DisableBreakpointSite(BreakpointSite *bp_site) {
@@ -3456,7 +3450,7 @@ 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)",
@@ -3464,7 +3458,7 @@ Status ProcessGDBRemote::DisableBreakpointSite(BreakpointSite *bp_site) {
     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(),
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
index 1ef3254a1760c..7c2877fa71d49 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.h
+++ b/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,
diff --git a/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp b/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
index b8e2e4933bcc7..b0718b771295b 100644
--- a/lldb/source/Plugins/Process/scripted/ScriptedProcess.cpp
+++ b/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 {};
   }
 
diff --git a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
index f1d39a85cb18e..aacfcc29e5f97 100644
--- a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
+++ b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
@@ -313,9 +313,10 @@ bool ScriptedThread::CalculateStopInfo() {
   // if we CreateStopReasonWithBreakpointSiteID.
   if (RegisterContextSP re...
[truncated]

[DavidSpickett]

Commit 1 is a different PR, right? Commit 2 is the new changes.

[felipepiovezan]

Commit 1 is a different PR, right? Commit 2 is the new changes.

Yes, sorry, fixed it.
Can't wait to have native stacked PR support here..

[felipepiovezan]

2026-04-27T08:25:38.5083295Z ******************** TEST 'lldb-unit :: Host/./HostTests/12/18' FAILED ********************

Don't think this is related, but I'll force a re-run

[DavidSpickett]

The Process class is the one responsible for managing the state of a BreakpointSite inside the process. As such, it should be the one answering questions about the state of the site.

This might be true but for now of course all I see is a layer of indirection. I assume I'll see this being used in later PRs but please add a sentence to the PR description to give the practical reasons for doing this.

[slydiman]

The Windows buildbots are broken
https://lab.llvm.org/buildbot/#/builders/211/builds/7866
https://lab.llvm.org/buildbot/#/builders/197/builds/15641

FAILED: tools/lldb/source/Plugins/Process/Windows/Common/CMakeFiles/lldbPluginProcessWindowsCommon.dir/ProcessWindows.cpp.obj 
ccache C:\PROGRA~1\MICROS~1\2022\COMMUN~1\VC\Tools\MSVC\1444~1.352\bin\Hostx64\x64\cl.exe  /nologo /TP -DUNICODE -D_CRT_NONSTDC_NO_DEPRECATE -D_CRT_NONSTDC_NO_WARNINGS -D_CRT_SECURE_NO_DEPRECATE -D_CRT_SECURE_NO_WARNINGS -D_ENABLE_EXTENDED_ALIGNED_STORAGE -D_GLIBCXX_ASSERTIONS -D_HAS_EXCEPTIONS=0 -D_LIBCPP_HARDENING_MODE=_LIBCPP_HARDENING_MODE_EXTENSIVE -D_SCL_SECURE_NO_DEPRECATE -D_SCL_SECURE_NO_WARNINGS -D_UNICODE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -IC:\buildbot\as-builder-10\lldb-x86-64\build\tools\lldb\source\Plugins\Process\Windows\Common -IC:\buildbot\as-builder-10\lldb-x86-64\llvm-project\lldb\source\Plugins\Process\Windows\Common -IC:\buildbot\as-builder-10\lldb-x86-64\llvm-project\lldb\include -IC:\buildbot\as-builder-10\lldb-x86-64\build\tools\lldb\include -IC:\buildbot\as-builder-10\lldb-x86-64\build\include -IC:\buildbot\as-builder-10\lldb-x86-64\llvm-project\llvm\include -IC:\buildbot\as-builder-10\lldb-x86-64\llvm-project\llvm\..\clang\include -IC:\buildbot\as-builder-10\lldb-x86-64\build\tools\lldb\..\clang\include -IC:\buildbot\as-builder-10\lldb-x86-64\llvm-project\lldb\source -IC:\buildbot\as-builder-10\lldb-x86-64\build\tools\lldb\source -D__OPTIMIZE__ /Zc:inline /Zc:preprocessor /Zc:__cplusplus /Oi /bigobj /permissive- /W4 -wd4146 -wd4244 -wd4267 -wd4456 -wd4457 -wd4458 -wd4459 -wd4624 -wd4100 -wd4127 -wd4505 -wd4702 -wd4245 -wd4310 -wd4701 -wd4703 -wd4389 -wd4805 -wd4577 -wd4319 -wd4324 -wd4251 -wd4275 -w14062 -we4238 /Gw /O2 /Ob2  -MD   -wd4018 -wd4068 -wd4150 -wd4201 -wd4251 -wd4521 -wd4530 -wd4589 -UNDEBUG /EHs-c- /GR- -std:c++17 /YuC:/buildbot/as-builder-10/lldb-x86-64/build/lib/Support/CMakeFiles/LLVMSupport.dir/cmake_pch.hxx /FpC:/buildbot/as-builder-10/lldb-x86-64/build/lib/Support/CMakeFiles/LLVMSupport.dir/./cmake_pch.cxx.pch /FIC:/buildbot/as-builder-10/lldb-x86-64/build/lib/Support/CMakeFiles/LLVMSupport.dir/cmake_pch.hxx /showIncludes /Fotools\lldb\source\Plugins\Process\Windows\Common\CMakeFiles\lldbPluginProcessWindowsCommon.dir\ProcessWindows.cpp.obj /Fdlib\LLVMSupport.pdb /FS -c C:\buildbot\as-builder-10\lldb-x86-64\llvm-project\lldb\source\Plugins\Process\Windows\Common\ProcessWindows.cpp
C:\buildbot\as-builder-10\lldb-x86-64\llvm-project\lldb\source\Plugins\Process\Windows\Common\ProcessWindows.cpp(418): error C2039: 'IsEnabled': is not a member of 'lldb_private::BreakpointSite'
C:\buildbot\as-builder-10\lldb-x86-64\llvm-project\lldb\include\lldb/Breakpoint/BreakpointSite.h(35): note: see declaration of 'lldb_private::BreakpointSite'

[slydiman]

The Windows buildbots are broken

Please fix ASAP or revert.
Currently we already have multiple different errors on buildbots.
It is hard to monitor new issues while buildbots are already red.

[charles-zablit]

Opened a fix here:

• [lldb][windows] fix build issue #195089

[felipepiovezan]

Opened a fix here:

• [lldb][windows] fix build issue #195089

Thank you, Charles!