From 9aeeea37620cf7c554d1462188ad06c2f6d32617 Mon Sep 17 00:00:00 2001 From: Martino Fontana Date: Sat, 15 Jun 2024 11:02:15 +0200 Subject: [PATCH] Debugger: Small Breakpoint cleanup Reuse more code, change misleading names, remove useless documentation, add useful documentation --- Source/Core/Core/Debugger/DebugInterface.h | 4 +-- .../Core/Core/Debugger/PPCDebugInterface.cpp | 10 ++---- Source/Core/Core/Debugger/PPCDebugInterface.h | 4 +-- Source/Core/Core/PowerPC/BreakPoints.cpp | 33 ++++++++++++------- Source/Core/Core/PowerPC/BreakPoints.h | 15 ++++----- Source/Core/Core/PowerPC/GDBStub.cpp | 3 +- .../DolphinQt/Debugger/BreakpointWidget.cpp | 4 +-- .../DolphinQt/Debugger/CodeViewWidget.cpp | 8 ++--- 8 files changed, 41 insertions(+), 40 deletions(-) diff --git a/Source/Core/Core/Debugger/DebugInterface.h b/Source/Core/Core/Debugger/DebugInterface.h index 9802c31f3a..67ff54fcdb 100644 --- a/Source/Core/Core/Debugger/DebugInterface.h +++ b/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 ClearBreakpoint(u32 /*address*/) {} + virtual void AddBreakpoint(u32 /*address*/) {} + virtual void RemoveBreakpoint(u32 /*address*/) {} virtual void ClearAllBreakpoints() {} virtual void ToggleBreakpoint(u32 /*address*/) {} virtual void ClearAllMemChecks() {} diff --git a/Source/Core/Core/Debugger/PPCDebugInterface.cpp b/Source/Core/Core/Debugger/PPCDebugInterface.cpp index 9389fe76d8..3819f62493 100644 --- a/Source/Core/Core/Debugger/PPCDebugInterface.cpp +++ b/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(); - if (breakpoints.IsAddressBreakPoint(address)) - breakpoints.Remove(address); - else - breakpoints.Add(address); + m_system.GetPowerPC().GetBreakPoints().ToggleBreakPoint(address); } void PPCDebugInterface::ClearAllMemChecks() diff --git a/Source/Core/Core/Debugger/PPCDebugInterface.h b/Source/Core/Core/Debugger/PPCDebugInterface.h index 86207bc1da..5f8f8b04de 100644 --- a/Source/Core/Core/Debugger/PPCDebugInterface.h +++ b/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 ClearBreakpoint(u32 address) override; + void AddBreakpoint(u32 address) override; + void RemoveBreakpoint(u32 address) override; void ClearAllBreakpoints() override; void ToggleBreakpoint(u32 address) override; void ClearAllMemChecks() override; diff --git a/Source/Core/Core/PowerPC/BreakPoints.cpp b/Source/Core/Core/PowerPC/BreakPoints.cpp index 1e8689dd82..24b15750e0 100644 --- a/Source/Core/Core/PowerPC/BreakPoints.cpp +++ b/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(), - [address](const auto& bp) { return bp.address == address; }); + return GetBreakpoint(address) != nullptr; } bool BreakPoints::IsBreakPointEnable(u32 address) const { - return std::any_of(m_breakpoints.begin(), m_breakpoints.end(), - [address](const auto& bp) { return bp.is_enabled && bp.address == address; }); + const TBreakPoint* bp = GetBreakpoint(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() diff --git a/Source/Core/Core/PowerPC/BreakPoints.h b/Source/Core/Core/PowerPC/BreakPoints.h index ce509e5d97..6c60b8a24e 100644 --- a/Source/Core/Core/PowerPC/BreakPoints.h +++ b/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 condition); void Add(u32 address, bool temp = false); void Add(TBreakPoint bp); - // Modify Breakpoint bool ToggleBreakPoint(u32 address); + bool ToggleEnable(u32 address); - // Remove Breakpoint - void Remove(u32 address); + // Remove Breakpoint. Returns whether it was removed. + 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(); } diff --git a/Source/Core/Core/PowerPC/GDBStub.cpp b/Source/Core/Core/PowerPC/GDBStub.cpp index df87a6c75a..0f6617a5b4 100644 --- a/Source/Core/Core/PowerPC/GDBStub.cpp +++ b/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); } } diff --git a/Source/Core/DolphinQt/Debugger/BreakpointWidget.cpp b/Source/Core/DolphinQt/Debugger/BreakpointWidget.cpp index 130e68e860..3bed35c576 100644 --- a/Source/Core/DolphinQt/Debugger/BreakpointWidget.cpp +++ b/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(); diff --git a/Source/Core/DolphinQt/Debugger/CodeViewWidget.cpp b/Source/Core/DolphinQt/Debugger/CodeViewWidget.cpp index 51f2814d9b..e2df3420a6 100644 --- a/Source/Core/DolphinQt/Debugger/CodeViewWidget.cpp +++ b/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(); - if (power_pc.GetDebugInterface().IsBreakpoint(GetContextAddress())) - power_pc.GetBreakPoints().Remove(GetContextAddress()); - else - power_pc.GetBreakPoints().Add(GetContextAddress()); + m_system.GetPowerPC().GetBreakPoints().ToggleBreakPoint(GetContextAddress()); emit BreakpointsChanged(); Update();