Debugger: Small Breakpoint cleanup

Reuse more code, change misleading names, remove useless documentation, add useful documentation

Source/Core/Core/Debugger/DebugInterface.h

@@ -73,8 +73,8 @@ public:
   }
   virtual bool IsAlive() const { return true; }
   virtual bool IsBreakpoint(u32 /*address*/) const { return false; }
-  virtual void SetBreakpoint(u32 /*address*/) {}
+  virtual void AddBreakpoint(u32 /*address*/) {}
-  virtual void ClearBreakpoint(u32 /*address*/) {}
+  virtual void RemoveBreakpoint(u32 /*address*/) {}
   virtual void ClearAllBreakpoints() {}
   virtual void ToggleBreakpoint(u32 /*address*/) {}
   virtual void ClearAllMemChecks() {}

Source/Core/Core/Debugger/PPCDebugInterface.cpp

@@ -357,12 +357,12 @@ bool PPCDebugInterface::IsBreakpoint(u32 address) const
   return m_system.GetPowerPC().GetBreakPoints().IsAddressBreakPoint(address);
 }
 
-void PPCDebugInterface::SetBreakpoint(u32 address)
+void PPCDebugInterface::AddBreakpoint(u32 address)
 {
   m_system.GetPowerPC().GetBreakPoints().Add(address);
 }
 
-void PPCDebugInterface::ClearBreakpoint(u32 address)
+void PPCDebugInterface::RemoveBreakpoint(u32 address)
 {
   m_system.GetPowerPC().GetBreakPoints().Remove(address);
 }
@@ -374,11 +374,7 @@ void PPCDebugInterface::ClearAllBreakpoints()
 
 void PPCDebugInterface::ToggleBreakpoint(u32 address)
 {
-  auto& breakpoints = m_system.GetPowerPC().GetBreakPoints();
+  m_system.GetPowerPC().GetBreakPoints().ToggleBreakPoint(address);
-  if (breakpoints.IsAddressBreakPoint(address))
-    breakpoints.Remove(address);
-  else
-    breakpoints.Add(address);
 }
 
 void PPCDebugInterface::ClearAllMemChecks()

Source/Core/Core/Debugger/PPCDebugInterface.h

@@ -80,8 +80,8 @@ public:
                                  u32 address) const override;
   bool IsAlive() const override;
   bool IsBreakpoint(u32 address) const override;
-  void SetBreakpoint(u32 address) override;
+  void AddBreakpoint(u32 address) override;
-  void ClearBreakpoint(u32 address) override;
+  void RemoveBreakpoint(u32 address) override;
   void ClearAllBreakpoints() override;
   void ToggleBreakpoint(u32 address) override;
   void ClearAllMemChecks() override;

Source/Core/Core/PowerPC/BreakPoints.cpp

@@ -27,14 +27,13 @@ BreakPoints::~BreakPoints() = default;
 
 bool BreakPoints::IsAddressBreakPoint(u32 address) const
 {
-  return std::any_of(m_breakpoints.begin(), m_breakpoints.end(),
+  return GetBreakpoint(address) != nullptr;
-                     [address](const auto& bp) { return bp.address == address; });
 }
 
 bool BreakPoints::IsBreakPointEnable(u32 address) const
 {
-  return std::any_of(m_breakpoints.begin(), m_breakpoints.end(),
+  const TBreakPoint* bp = GetBreakpoint(address);
-                     [address](const auto& bp) { return bp.is_enabled && bp.address == address; });
+  return bp != nullptr && bp->is_enabled;
 }
 
 bool BreakPoints::IsTempBreakPoint(u32 address) const
@@ -153,6 +152,16 @@ void BreakPoints::Add(u32 address, bool temp, bool break_on_hit, bool log_on_hit
 }
 
 bool BreakPoints::ToggleBreakPoint(u32 address)
+{
+  if (!Remove(address))
+  {
+    Add(address);
+    return true;
+  }
+  return false;
+}
+
+bool BreakPoints::ToggleEnable(u32 address)
 {
   auto iter = std::find_if(m_breakpoints.begin(), m_breakpoints.end(),
                            [address](const auto& bp) { return bp.address == address; });
@@ -164,16 +173,18 @@ bool BreakPoints::ToggleBreakPoint(u32 address)
   return true;
 }
 
-void BreakPoints::Remove(u32 address)
+bool BreakPoints::Remove(u32 address)
 {
   const auto iter = std::find_if(m_breakpoints.begin(), m_breakpoints.end(),
                                  [address](const auto& bp) { return bp.address == address; });
 
   if (iter == m_breakpoints.cend())
-    return;
+    return false;
 
   m_breakpoints.erase(iter);
   m_system.GetJitInterface().InvalidateICache(address, 4, true);
+
+  return true;
 }
 
 void BreakPoints::Clear()
@@ -281,9 +292,8 @@ void MemChecks::Add(TMemCheck memory_check)
                    [address](const auto& check) { return check.start_address == address; });
   if (old_mem_check != m_mem_checks.end())
   {
-    const bool is_enabled = old_mem_check->is_enabled;  // Preserve enabled status
+    memory_check.is_enabled = old_mem_check->is_enabled;  // Preserve enabled status
     *old_mem_check = std::move(memory_check);
-    old_mem_check->is_enabled = is_enabled;
     old_mem_check->num_hits = 0;
   }
   else
@@ -297,7 +307,7 @@ void MemChecks::Add(TMemCheck memory_check)
   m_system.GetMMU().DBATUpdated();
 }
 
-bool MemChecks::ToggleBreakPoint(u32 address)
+bool MemChecks::ToggleEnable(u32 address)
 {
   auto iter = std::find_if(m_mem_checks.begin(), m_mem_checks.end(),
                            [address](const auto& bp) { return bp.start_address == address; });
@@ -309,20 +319,21 @@ bool MemChecks::ToggleBreakPoint(u32 address)
   return true;
 }
 
-void MemChecks::Remove(u32 address)
+bool MemChecks::Remove(u32 address)
 {
   const auto iter =
       std::find_if(m_mem_checks.cbegin(), m_mem_checks.cend(),
                    [address](const auto& check) { return check.start_address == address; });
 
   if (iter == m_mem_checks.cend())
-    return;
+    return false;
 
   const Core::CPUThreadGuard guard(m_system);
   m_mem_checks.erase(iter);
   if (!HasAny())
     m_system.GetJitInterface().ClearCache(guard);
   m_system.GetMMU().DBATUpdated();
+  return true;
 }
 
 void MemChecks::Clear()

Source/Core/Core/PowerPC/BreakPoints.h

@@ -66,23 +66,22 @@ public:
   TBreakPointsStr GetStrings() const;
   void AddFromStrings(const TBreakPointsStr& bp_strings);
 
-  // is address breakpoint
   bool IsAddressBreakPoint(u32 address) const;
   bool IsBreakPointEnable(u32 adresss) const;
   bool IsTempBreakPoint(u32 address) const;
   const TBreakPoint* GetBreakpoint(u32 address) const;
 
-  // Add BreakPoint
+  // Add BreakPoint. If one already exists on the same address, replace it.
   void Add(u32 address, bool temp, bool break_on_hit, bool log_on_hit,
            std::optional<Expression> condition);
   void Add(u32 address, bool temp = false);
   void Add(TBreakPoint bp);
 
-  // Modify Breakpoint
   bool ToggleBreakPoint(u32 address);
+  bool ToggleEnable(u32 address);
 
-  // Remove Breakpoint
+  // Remove Breakpoint. Returns whether it was removed.
-  void Remove(u32 address);
+  bool Remove(u32 address);
   void Clear();
   void ClearAllTemporary();
 
@@ -111,12 +110,12 @@ public:
 
   void Add(TMemCheck memory_check);
 
-  bool ToggleBreakPoint(u32 address);
+  bool ToggleEnable(u32 address);
 
-  // memory breakpoint
   TMemCheck* GetMemCheck(u32 address, size_t size = 1);
   bool OverlapsMemcheck(u32 address, u32 length) const;
-  void Remove(u32 address);
+  // Remove Breakpoint. Returns whether it was removed.
+  bool Remove(u32 address);
 
   void Clear();
   bool HasAny() const { return !m_mem_checks.empty(); }

Source/Core/Core/PowerPC/GDBStub.cpp

@@ -164,9 +164,8 @@ static void RemoveBreakpoint(BreakpointType type, u32 addr, u32 len)
   if (type == BreakpointType::ExecuteHard || type == BreakpointType::ExecuteSoft)
   {
     auto& breakpoints = Core::System::GetInstance().GetPowerPC().GetBreakPoints();
-    while (breakpoints.IsAddressBreakPoint(addr))
+    if (breakpoints.Remove(addr))
     {
-      breakpoints.Remove(addr);
       INFO_LOG_FMT(GDB_STUB, "gdb: removed a breakpoint: {:08x} bytes at {:08x}", len, addr);
     }
   }

Source/Core/DolphinQt/Debugger/BreakpointWidget.cpp

@@ -215,9 +215,9 @@ void BreakpointWidget::OnClicked(QTableWidgetItem* item)
   if (item->column() == ENABLED_COLUMN)
   {
     if (item->data(IS_MEMCHECK_ROLE).toBool())
-      m_system.GetPowerPC().GetMemChecks().ToggleBreakPoint(address);
+      m_system.GetPowerPC().GetMemChecks().ToggleEnable(address);
     else
-      m_system.GetPowerPC().GetBreakPoints().ToggleBreakPoint(address);
+      m_system.GetPowerPC().GetBreakPoints().ToggleEnable(address);
 
     emit BreakpointsChanged();
     Update();

Source/Core/DolphinQt/Debugger/CodeViewWidget.cpp

@@ -869,7 +869,7 @@ void CodeViewWidget::OnRunToHere()
 {
   const u32 addr = GetContextAddress();
 
-  m_system.GetPowerPC().GetDebugInterface().SetBreakpoint(addr);
+  m_system.GetPowerPC().GetDebugInterface().AddBreakpoint(addr);
   m_system.GetPowerPC().GetDebugInterface().RunToBreakpoint();
   Update();
 }
@@ -1137,11 +1137,7 @@ void CodeViewWidget::showEvent(QShowEvent* event)
 
 void CodeViewWidget::ToggleBreakpoint()
 {
-  auto& power_pc = m_system.GetPowerPC();
+  m_system.GetPowerPC().GetBreakPoints().ToggleBreakPoint(GetContextAddress());
-  if (power_pc.GetDebugInterface().IsBreakpoint(GetContextAddress()))
-    power_pc.GetBreakPoints().Remove(GetContextAddress());
-  else
-    power_pc.GetBreakPoints().Add(GetContextAddress());
 
   emit BreakpointsChanged();
   Update();