Debugger: Small other cleanup

Change misleading names.
Fix function usage: Intepreter and Step Out will not check breakpoints in their own wrong way anymore (e.g. breaking on log-only breakpoints).

Source/Core/Core/Core.cpp

@@ -704,13 +704,13 @@ void SetState(Core::System& system, State state, bool report_state_change)
   case State::Paused:
     // NOTE: GetState() will return State::Paused immediately, even before anything has
     //   stopped (including the CPU).
-    system.GetCPU().EnableStepping(true);  // Break
+    system.GetCPU().SetStepping(true);  // Break
     Wiimote::Pause();
     ResetRumble();
     break;
   case State::Running:
   {
-    system.GetCPU().EnableStepping(false);
+    system.GetCPU().SetStepping(false);
     Wiimote::Resume();
     break;
   }
@@ -813,7 +813,7 @@ static bool PauseAndLock(Core::System& system, bool do_lock, bool unpause_on_unl
     // The CPU is responsible for managing the Audio and FIFO state so we use its
     // mechanism to unpause them. If we unpaused the systems above when releasing
     // the locks then they could call CPU::Break which would require detecting it
-    // and re-pausing with CPU::EnableStepping.
+    // and re-pausing with CPU::SetStepping.
     was_unpaused = system.GetCPU().PauseAndLock(false, unpause_on_unlock, true);
   }
 

Source/Core/Core/FifoPlayer/FifoPlayer.cpp

@@ -228,7 +228,7 @@ public:
     IsPlayingBackFifologWithBrokenEFBCopies = m_parent->m_File->HasBrokenEFBCopies();
     // Without this call, we deadlock in initialization in dual core, as the FIFO is disabled and
     // thus ClearEfb()'s call to WaitForGPUInactive() never returns
-    m_parent->m_system.GetCPU().EnableStepping(false);
+    m_parent->m_system.GetCPU().SetStepping(false);
 
     m_parent->m_CurrentFrame = m_parent->m_FrameRangeStart;
     m_parent->LoadMemory();
@@ -243,7 +243,7 @@ public:
   void SingleStep() override
   {
     // NOTE: AdvanceFrame() will get stuck forever in Dual Core because the FIFO
-    //   is disabled by CPU::EnableStepping(true) so the frame never gets displayed.
+    //   is disabled by CPU::SetStepping(true) so the frame never gets displayed.
     PanicAlertFmtT("Cannot SingleStep the FIFO. Use Frame Advance instead.");
   }
 

Source/Core/Core/HW/CPU.cpp

@@ -88,6 +88,7 @@ void CPUManager::Run()
       // Adjust PC for JIT when debugging
       // SingleStep so that the "continue", "step over" and "step out" debugger functions
       // work when the PC is at a breakpoint at the beginning of the block
+      // Don't use PowerPCManager::CheckBreakPoints, otherwise you get double logging
       // If watchpoints are enabled, any instruction could be a breakpoint.
       if (power_pc.GetMode() != PowerPC::CoreMode::Interpreter)
       {
@@ -174,7 +175,7 @@ void CPUManager::Run()
 // Requires holding m_state_change_lock
 void CPUManager::RunAdjacentSystems(bool running)
 {
-  // NOTE: We're assuming these will not try to call Break or EnableStepping.
+  // NOTE: We're assuming these will not try to call Break or SetStepping.
   m_system.GetFifo().EmulatorState(running);
   // Core is responsible for shutting down the sound stream.
   if (m_state != State::PowerDown)
@@ -247,7 +248,7 @@ bool CPUManager::SetStateLocked(State s)
   return true;
 }
 
-void CPUManager::EnableStepping(bool stepping)
+void CPUManager::SetStepping(bool stepping)
 {
   std::lock_guard stepping_lock(m_stepping_lock);
   std::unique_lock state_lock(m_state_change_lock);
@@ -290,7 +291,7 @@ void CPUManager::Break()
 
 void CPUManager::Continue()
 {
-  EnableStepping(false);
+  SetStepping(false);
   Core::CallOnStateChangedCallbacks(Core::State::Running);
 }
 

Source/Core/Core/HW/CPU.h

@@ -62,10 +62,10 @@ public:
   void StepOpcode(Common::Event* event = nullptr);
 
   // Enable or Disable Stepping. [Will deadlock if called from a system thread]
-  void EnableStepping(bool stepping);
+  void SetStepping(bool stepping);
 
-  // Breakpoint activation for system threads. Similar to EnableStepping(true).
-  // NOTE: Unlike EnableStepping, this does NOT synchronize with the CPU Thread
+  // Breakpoint activation for system threads. Similar to SetStepping(true).
+  // NOTE: Unlike SetStepping, this does NOT synchronize with the CPU Thread
   //   which enables it to avoid deadlocks but also makes it less safe so it
   //   should not be used by the Host.
   void Break();
@@ -91,7 +91,7 @@ public:
   // Return value for do_lock == true is whether the state was State::Running or not.
   // Return value for do_lock == false is whether the state was changed *to* State::Running or not.
   // Cannot be used by System threads as it will deadlock. It is threadsafe otherwise.
-  // "control_adjacent" causes PauseAndLock to behave like EnableStepping by modifying the
+  // "control_adjacent" causes PauseAndLock to behave like SetStepping by modifying the
   //   state of the Audio and FIFO subsystems as well.
   bool PauseAndLock(bool do_lock, bool unpause_on_unlock = true, bool control_adjacent = false);
 
@@ -110,9 +110,9 @@ private:
   // Read access is unsynchronized.
   State m_state = State::PowerDown;
 
-  // Synchronizes EnableStepping and PauseAndLock so only one instance can be
+  // Synchronizes SetStepping and PauseAndLock so only one instance can be
   // active at a time. Simplifies code by eliminating several edge cases where
-  // the EnableStepping(true)/PauseAndLock(true) case must release the state lock
+  // the SetStepping(true)/PauseAndLock(true) case must release the state lock
   // and wait for the CPU Thread which would otherwise require additional flags.
   // NOTE: When using the stepping lock, it must always be acquired first. If
   //   the lock is acquired after the state lock then that is guaranteed to

Source/Core/Core/PowerPC/CachedInterpreter/CachedInterpreter.cpp

@@ -249,8 +249,7 @@ bool CachedInterpreter::CheckProgramException(CachedInterpreter& cached_interpre
 
 bool CachedInterpreter::CheckBreakpoint(CachedInterpreter& cached_interpreter, u32 data)
 {
-  cached_interpreter.m_system.GetPowerPC().CheckBreakPoints();
-  if (cached_interpreter.m_system.GetCPU().GetState() != CPU::State::Running)
+  if (cached_interpreter.m_system.GetPowerPC().CheckAndHandleBreakPoints())
   {
     cached_interpreter.m_ppc_state.downcount -= data;
     return true;

Source/Core/Core/PowerPC/GDBStub.cpp

@@ -865,7 +865,7 @@ static void WriteMemory(const Core::CPUThreadGuard& guard)
 static void Step()
 {
   auto& system = Core::System::GetInstance();
-  system.GetCPU().EnableStepping(true);
+  system.GetCPU().SetStepping(true);
   Core::CallOnStateChangedCallbacks(Core::State::Paused);
 }
 

Source/Core/Core/PowerPC/Interpreter/Interpreter.cpp

@@ -261,38 +261,8 @@ void Interpreter::Run()
             s_pc_vec.erase(s_pc_vec.begin());
 #endif
 
-          // 2: check for breakpoint
-          if (power_pc.GetBreakPoints().IsAddressBreakPoint(m_ppc_state.pc))
-          {
-#ifdef SHOW_HISTORY
-            NOTICE_LOG_FMT(POWERPC, "----------------------------");
-            NOTICE_LOG_FMT(POWERPC, "Blocks:");
-            for (const u32 entry : s_pc_block_vec)
-              NOTICE_LOG_FMT(POWERPC, "PC: {:#010x}", entry);
-            NOTICE_LOG_FMT(POWERPC, "----------------------------");
-            NOTICE_LOG_FMT(POWERPC, "Steps:");
-            for (size_t j = 0; j < s_pc_vec.size(); j++)
-            {
-              // Write space
-              if (j > 0)
-              {
-                if (s_pc_vec[j] != s_pc_vec[(j - 1) + 4]
-                  NOTICE_LOG_FMT(POWERPC, "");
-              }
-
-              NOTICE_LOG_FMT(POWERPC, "PC: {:#010x}", s_pc_vec[j]);
-            }
-#endif
-            INFO_LOG_FMT(POWERPC, "Hit Breakpoint - {:08x}", m_ppc_state.pc);
-            cpu.Break();
-            if (GDBStub::IsActive())
-              GDBStub::TakeControl();
-            if (power_pc.GetBreakPoints().IsTempBreakPoint(m_ppc_state.pc))
-              power_pc.GetBreakPoints().Remove(m_ppc_state.pc);
-
-            Host_UpdateDisasmDialog();
+          if (power_pc.CheckAndHandleBreakPoints())
             return;
-          }
           cycles += SingleStepInner();
         }
         m_ppc_state.downcount -= cycles;

Source/Core/Core/PowerPC/Jit64/Jit.cpp

@@ -1043,7 +1043,7 @@ bool Jit64::DoJit(u32 em_address, JitBlock* b, u32 nextPC)
 
         MOV(32, PPCSTATE(pc), Imm32(op.address));
         ABI_PushRegistersAndAdjustStack({}, 0);
-        ABI_CallFunctionP(PowerPC::CheckBreakPointsFromJIT, &power_pc);
+        ABI_CallFunctionP(PowerPC::CheckAndHandleBreakPointsFromJIT, &power_pc);
         ABI_PopRegistersAndAdjustStack({}, 0);
         MOV(64, R(RSCRATCH), ImmPtr(cpu.GetStatePtr()));
         CMP(32, MatR(RSCRATCH), Imm32(Common::ToUnderlying(CPU::State::Running)));

Source/Core/Core/PowerPC/JitArm64/Jit.cpp

@@ -1249,7 +1249,7 @@ bool JitArm64::DoJit(u32 em_address, JitBlock* b, u32 nextPC)
 
         MOVI2R(DISPATCHER_PC, op.address);
         STP(IndexType::Signed, DISPATCHER_PC, DISPATCHER_PC, PPC_REG, PPCSTATE_OFF(pc));
-        ABI_CallFunction(&PowerPC::CheckBreakPointsFromJIT, &m_system.GetPowerPC());
+        ABI_CallFunction(&PowerPC::CheckAndHandleBreakPointsFromJIT, &m_system.GetPowerPC());
 
         LDR(IndexType::Unsigned, ARM64Reg::W0, ARM64Reg::X0,
             MOVPage2R(ARM64Reg::X0, cpu.GetStatePtr()));

Source/Core/Core/PowerPC/PowerPC.cpp

@@ -629,19 +629,13 @@ void PowerPCManager::CheckExternalExceptions()
   m_system.GetJitInterface().UpdateMembase();
 }
 
-void PowerPCManager::CheckBreakPoints()
+bool PowerPCManager::CheckBreakPoints()
 {
   const TBreakPoint* bp = m_breakpoints.GetBreakpoint(m_ppc_state.pc);
 
   if (!bp || !bp->is_enabled || !EvaluateCondition(m_system, bp->condition))
-    return;
+    return false;
 
-  if (bp->break_on_hit)
-  {
-    m_system.GetCPU().Break();
-    if (GDBStub::IsActive())
-      GDBStub::TakeControl();
-  }
   if (bp->log_on_hit)
   {
     NOTICE_LOG_FMT(MEMMAP,
@@ -652,8 +646,21 @@ void PowerPCManager::CheckBreakPoints()
                    m_ppc_state.gpr[8], m_ppc_state.gpr[9], m_ppc_state.gpr[10], m_ppc_state.gpr[11],
                    m_ppc_state.gpr[12], LR(m_ppc_state));
   }
-  if (m_breakpoints.IsTempBreakPoint(m_ppc_state.pc))
-    m_breakpoints.Remove(m_ppc_state.pc);
+  if (bp->break_on_hit)
+    return true;
+  return false;
+}
+
+bool PowerPCManager::CheckAndHandleBreakPoints()
+{
+  if (CheckBreakPoints())
+  {
+    m_system.GetCPU().Break();
+    if (GDBStub::IsActive())
+      GDBStub::TakeControl();
+    return true;
+  }
+  return false;
 }
 
 void PowerPCState::SetSR(u32 index, u32 value)
@@ -722,8 +729,8 @@ void CheckExternalExceptionsFromJIT(PowerPCManager& power_pc)
   power_pc.CheckExternalExceptions();
 }
 
-void CheckBreakPointsFromJIT(PowerPCManager& power_pc)
+void CheckAndHandleBreakPointsFromJIT(PowerPCManager& power_pc)
 {
-  power_pc.CheckBreakPoints();
+  power_pc.CheckAndHandleBreakPoints();
 }
 }  // namespace PowerPC

Source/Core/Core/PowerPC/PowerPC.h

@@ -281,7 +281,10 @@ public:
   void SingleStep();
   void CheckExceptions();
   void CheckExternalExceptions();
-  void CheckBreakPoints();
+  // Evaluate the breakpoints in order to log. Returns whether it would break.
+  bool CheckBreakPoints();
+  // Evaluate the breakpoints in order to log and/or break. Returns whether it breaks.
+  bool CheckAndHandleBreakPoints();
   void RunLoop();
 
   u64 ReadFullTimeBaseValue() const;
@@ -330,7 +333,7 @@ void UpdatePerformanceMonitor(u32 cycles, u32 num_load_stores, u32 num_fp_inst,
 
 void CheckExceptionsFromJIT(PowerPCManager& power_pc);
 void CheckExternalExceptionsFromJIT(PowerPCManager& power_pc);
-void CheckBreakPointsFromJIT(PowerPCManager& power_pc);
+void CheckAndHandleBreakPointsFromJIT(PowerPCManager& power_pc);
 
 // Easy register access macros.
 #define HID0(ppc_state) ((UReg_HID0&)(ppc_state).spr[SPR_HID0])

Source/Core/DolphinQt/Debugger/CodeWidget.cpp

@@ -484,7 +484,7 @@ void CodeWidget::StepOver()
     auto& breakpoints = m_system.GetPowerPC().GetBreakPoints();
     breakpoints.ClearAllTemporary();
     breakpoints.Add(m_system.GetPPCState().pc + 4, true);
-    cpu.EnableStepping(false);
+    cpu.SetStepping(false);
     Core::DisplayMessage(tr("Step over in progress...").toStdString(), 2000);
   }
   else
@@ -547,8 +547,7 @@ void CodeWidget::StepOut()
         do
         {
           power_pc.SingleStep();
-        } while (ppc_state.pc != next_pc && clock::now() < timeout &&
-                 !breakpoints.IsAddressBreakPoint(ppc_state.pc));
+        } while (ppc_state.pc != next_pc && clock::now() < timeout && !power_pc.CheckBreakPoints());
       }
       else
       {
@@ -556,14 +555,14 @@ void CodeWidget::StepOut()
       }
 
       inst = PowerPC::MMU::HostRead_Instruction(guard, ppc_state.pc);
-    } while (clock::now() < timeout && !breakpoints.IsAddressBreakPoint(ppc_state.pc));
+    } while (clock::now() < timeout && !power_pc.CheckBreakPoints());
 
     power_pc.SetMode(old_mode);
   }
 
   emit Host::GetInstance()->UpdateDisasmDialog();
 
-  if (breakpoints.IsAddressBreakPoint(ppc_state.pc))
+  if (power_pc.CheckBreakPoints())
     Core::DisplayMessage(tr("Breakpoint encountered! Step out aborted.").toStdString(), 2000);
   else if (clock::now() >= timeout)
     Core::DisplayMessage(tr("Step out timed out!").toStdString(), 2000);