From 303366b1ce2e93ff33bac1afc42251f281073f9b Mon Sep 17 00:00:00 2001 From: JosJuice Date: Sat, 22 Mar 2025 12:13:48 +0100 Subject: [PATCH] PowerPC: Add RAII handling for breakpoint updates bbf72e7 made a change where you can pass `false` to certain MemChecks functions to get them to skip performing an "update" step. It was then up to the caller to call the Update function later. This commit changes the implementation so that, instead of the caller passing in a boolean that controls whether a function calls Update, the function now returns an object that on destruction will call Update. Callers that are fine with Update being called right away can skip storing the object in a variable and thereby call Update immediately, and callers that want to call Update later can keep the object around. This new design reduces the risk that someone will forget calling Update. --- Source/Core/Core/PowerPC/BreakPoints.cpp | 19 ++++----- Source/Core/Core/PowerPC/BreakPoints.h | 42 ++++++++++++++++++- Source/Core/Core/PowerPC/GDBStub.cpp | 4 +- .../DolphinQt/Debugger/MemoryViewWidget.cpp | 7 ++-- 4 files changed, 52 insertions(+), 20 deletions(-) diff --git a/Source/Core/Core/PowerPC/BreakPoints.cpp b/Source/Core/Core/PowerPC/BreakPoints.cpp index bb3c683fb4..10e037ca7b 100644 --- a/Source/Core/Core/PowerPC/BreakPoints.cpp +++ b/Source/Core/Core/PowerPC/BreakPoints.cpp @@ -256,6 +256,7 @@ MemChecks::TMemChecksStr MemChecks::GetStrings() const void MemChecks::AddFromStrings(const TMemChecksStr& mc_strings) { const Core::CPUThreadGuard guard(m_system); + DelayedMemCheckUpdate delayed_update(this); for (const std::string& mc_string : mc_strings) { @@ -283,13 +284,11 @@ void MemChecks::AddFromStrings(const TMemChecksStr& mc_strings) mc.condition = Expression::TryParse(condition); } - Add(std::move(mc), false); + delayed_update |= Add(std::move(mc)); } - - Update(); } -void MemChecks::Add(TMemCheck memory_check, bool update) +DelayedMemCheckUpdate MemChecks::Add(TMemCheck memory_check) { const Core::CPUThreadGuard guard(m_system); @@ -310,8 +309,7 @@ void MemChecks::Add(TMemCheck memory_check, bool update) m_mem_checks.emplace_back(std::move(memory_check)); } - if (update) - Update(); + return DelayedMemCheckUpdate(this, true); } bool MemChecks::ToggleEnable(u32 address) @@ -326,22 +324,19 @@ bool MemChecks::ToggleEnable(u32 address) return true; } -bool MemChecks::Remove(u32 address, bool update) +DelayedMemCheckUpdate 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 false; + return DelayedMemCheckUpdate(this, false); const Core::CPUThreadGuard guard(m_system); m_mem_checks.erase(iter); - if (update) - Update(); - - return true; + return DelayedMemCheckUpdate(this, true); } void MemChecks::Clear() diff --git a/Source/Core/Core/PowerPC/BreakPoints.h b/Source/Core/Core/PowerPC/BreakPoints.h index 6b08c25c8d..6408a54252 100644 --- a/Source/Core/Core/PowerPC/BreakPoints.h +++ b/Source/Core/Core/PowerPC/BreakPoints.h @@ -97,6 +97,8 @@ private: Core::System& m_system; }; +class DelayedMemCheckUpdate; + // Memory breakpoints class MemChecks { @@ -115,13 +117,13 @@ public: TMemChecksStr GetStrings() const; void AddFromStrings(const TMemChecksStr& mc_strings); - void Add(TMemCheck memory_check, bool update = true); + DelayedMemCheckUpdate Add(TMemCheck memory_check); bool ToggleEnable(u32 address); TMemCheck* GetMemCheck(u32 address, size_t size = 1); bool OverlapsMemcheck(u32 address, u32 length) const; - bool Remove(u32 address, bool update = true); + DelayedMemCheckUpdate Remove(u32 address); void Update(); void Clear(); @@ -132,3 +134,39 @@ private: Core::System& m_system; bool m_mem_breakpoints_set = false; }; + +class DelayedMemCheckUpdate final +{ +public: + DelayedMemCheckUpdate(MemChecks* memchecks, bool update_needed = false) + : m_memchecks(memchecks), m_update_needed(update_needed) + { + } + + DelayedMemCheckUpdate(const DelayedMemCheckUpdate&) = delete; + DelayedMemCheckUpdate(DelayedMemCheckUpdate&& other) = delete; + DelayedMemCheckUpdate& operator=(const DelayedMemCheckUpdate&) = delete; + DelayedMemCheckUpdate& operator=(DelayedMemCheckUpdate&& other) = delete; + + ~DelayedMemCheckUpdate() + { + if (m_update_needed) + m_memchecks->Update(); + } + + DelayedMemCheckUpdate& operator|=(DelayedMemCheckUpdate&& other) + { + if (m_memchecks == other.m_memchecks) + { + m_update_needed |= other.m_update_needed; + other.m_update_needed = false; + } + return *this; + } + + operator bool() const { return m_update_needed; } + +private: + MemChecks* m_memchecks; + bool m_update_needed; +}; diff --git a/Source/Core/Core/PowerPC/GDBStub.cpp b/Source/Core/Core/PowerPC/GDBStub.cpp index c9875a99db..08b9f09c20 100644 --- a/Source/Core/Core/PowerPC/GDBStub.cpp +++ b/Source/Core/Core/PowerPC/GDBStub.cpp @@ -172,12 +172,12 @@ static void RemoveBreakpoint(BreakpointType type, u32 addr, u32 len) else { auto& memchecks = Core::System::GetInstance().GetPowerPC().GetMemChecks(); + DelayedMemCheckUpdate delayed_update(&memchecks); while (memchecks.GetMemCheck(addr, len) != nullptr) { - memchecks.Remove(addr, false); + delayed_update |= memchecks.Remove(addr); INFO_LOG_FMT(GDB_STUB, "gdb: removed a memcheck: {:08x} bytes at {:08x}", len, addr); } - memchecks.Update(); } Host_PPCBreakpointsChanged(); } diff --git a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp index 75cafdd814..7d8c506ecc 100644 --- a/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp +++ b/Source/Core/DolphinQt/Debugger/MemoryViewWidget.cpp @@ -996,6 +996,7 @@ void MemoryViewWidget::ToggleBreakpoint(u32 addr, bool row) { const Core::CPUThreadGuard guard(m_system); + DelayedMemCheckUpdate delayed_update(&memchecks); for (int i = 0; i < breaks; i++) { @@ -1014,16 +1015,14 @@ void MemoryViewWidget::ToggleBreakpoint(u32 addr, bool row) check.log_on_hit = m_do_log; check.break_on_hit = true; - memchecks.Add(std::move(check), false); + delayed_update |= memchecks.Add(std::move(check)); } else if (check_ptr != nullptr) { // Using the pointer fixes misaligned breakpoints (0x11 breakpoint in 0x10 aligned view). - memchecks.Remove(check_ptr->start_address, false); + delayed_update |= memchecks.Remove(check_ptr->start_address); } } - - memchecks.Update(); } emit Host::GetInstance()->PPCBreakpointsChanged();