preserve-emulation/ppsspp
1
0
Fork 0
mirror of https://github.com/hrydgard/ppsspp.git synced 2026-08-31 09:45:24 +02:00
Code Issues Packages Projects Releases Wiki Activity

BreakpointManager: remove internal locking, now redundant

Browse Source 
Every caller either runs on the CPU thread already, routes mutations
through Core_RunOnCPUThread, or holds g_frameMutex for reads - audited
across WebSocket subscribers, the legacy Win32 debugger, ImDebugger, and
the JIT/interpreter backends. Also renames GetMemCheckLocked to
FindMemCheckInRange since it no longer implies a lock is held.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Hqm11k99viLfbJm2MkH4BH
This commit is contained in:
Henrik Rydgård
2026-08-08 17:22:12 +02:00
co-authored by Claude Sonnet 5
parent a39efbdaec
commit 180becf883
2 changed files with 8 additions and 55 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Core/Debugger/Breakpoints.cpp
+6 -49
View File
@@ -16,7 +16,6 @@
// https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/.
#include <atomic>
#include <mutex>
#include "Common/System/System.h"
#include "Common/Log.h"
@@ -69,7 +68,6 @@ BreakAction MemCheck::Action(u32 addr, bool write, int size, u32 pc, const char
return result;
}
// Note: must lock while calling this.
size_t BreakpointManager::FindBreakpoint(u32 addr, bool matchTemp, bool temp) {
size_t found = INVALID_BREAKPOINT;
for (size_t i = 0; i < breakPoints_.size(); ++i) {
@@ -100,7 +98,6 @@ bool BreakpointManager::IsAddressBreakPoint(u32 addr)
{
if (!anyBreakPoints_)
return false;
std::lock_guard<std::mutex> guard(breakPointsMutex_);
size_t bp = FindBreakpoint(addr);
return bp != INVALID_BREAKPOINT && breakPoints_[bp].result != BREAK_ACTION_IGNORE;
}
@@ -109,7 +106,6 @@ bool BreakpointManager::IsAddressBreakPoint(u32 addr, bool* enabled)
{
if (!anyBreakPoints_)
return false;
std::lock_guard<std::mutex> guard(breakPointsMutex_);
size_t bp = FindBreakpoint(addr);
if (bp == INVALID_BREAKPOINT) return false;
if (enabled != nullptr)
@@ -119,7 +115,6 @@ bool BreakpointManager::IsAddressBreakPoint(u32 addr, bool* enabled)
bool BreakpointManager::IsTempBreakPoint(u32 addr)
{
std::lock_guard<std::mutex> guard(breakPointsMutex_);
size_t bp = FindBreakpoint(addr, true, true);
return bp != INVALID_BREAKPOINT;
}
@@ -128,7 +123,6 @@ bool BreakpointManager::RangeContainsBreakPoint(u32 addr, u32 size)
{
if (!anyBreakPoints_)
return false;
std::lock_guard<std::mutex> guard(breakPointsMutex_);
const u32 end = addr + size;
for (const auto &bp : breakPoints_)
{
@@ -140,7 +134,6 @@ bool BreakpointManager::RangeContainsBreakPoint(u32 addr, u32 size)
}
int BreakpointManager::AddBreakPoint(u32 addr, bool temp) {
std::unique_lock<std::mutex> guard(breakPointsMutex_);
size_t bp = FindBreakpoint(addr, true, temp);
if (bp == INVALID_BREAKPOINT) {
BreakPoint pt;
@@ -164,7 +157,6 @@ int BreakpointManager::AddBreakPoint(u32 addr, bool temp) {
}
void BreakpointManager::RemoveBreakPoint(u32 addr) {
std::unique_lock<std::mutex> guard(breakPointsMutex_);
size_t bp = FindBreakpoint(addr);
if (bp != INVALID_BREAKPOINT) {
breakPoints_.erase(breakPoints_.begin() + bp);
@@ -180,7 +172,6 @@ void BreakpointManager::RemoveBreakPoint(u32 addr) {
}
void BreakpointManager::ChangeBreakPoint(u32 addr, bool status) {
std::unique_lock<std::mutex> guard(breakPointsMutex_);
size_t bp = FindBreakpoint(addr);
if (bp != INVALID_BREAKPOINT) {
if (status)
@@ -192,7 +183,6 @@ void BreakpointManager::ChangeBreakPoint(u32 addr, bool status) {
}
void BreakpointManager::ChangeBreakPoint(u32 addr, BreakAction result) {
std::unique_lock<std::mutex> guard(breakPointsMutex_);
size_t bp = FindBreakpoint(addr);
if (bp != INVALID_BREAKPOINT) {
breakPoints_[bp].result = result;
@@ -204,7 +194,6 @@ void BreakpointManager::ChangeBreakPoint(u32 addr, BreakAction result) {
void BreakpointManager::ClearAllBreakPoints() {
if (!anyBreakPoints_)
return;
std::unique_lock<std::mutex> guard(breakPointsMutex_);
if (!breakPoints_.empty()) {
// Same strategy as ClearTemporaryBreakPoints - if there's only one, we can update just that one.
if (breakPoints_.size() == 1) {
@@ -221,8 +210,6 @@ void BreakpointManager::ClearTemporaryBreakPoints()
if (!anyBreakPoints_)
return;
std::unique_lock<std::mutex> guard(breakPointsMutex_);
std::vector<u32> addrsToUpdate;
for (auto it = breakPoints_.begin(); it != breakPoints_.end(); ) {
@@ -248,7 +235,6 @@ void BreakpointManager::ClearTemporaryBreakPoints()
void BreakpointManager::ChangeBreakPointAddCond(u32 addr, const BreakPointCond &cond)
{
std::unique_lock<std::mutex> guard(breakPointsMutex_);
size_t bp = FindBreakpoint(addr);
if (bp != INVALID_BREAKPOINT)
{
@@ -259,7 +245,6 @@ void BreakpointManager::ChangeBreakPointAddCond(u32 addr, const BreakPointCond &
}
void BreakpointManager::ChangeBreakPointRemoveCond(u32 addr) {
std::unique_lock<std::mutex> guard(breakPointsMutex_);
size_t bp = FindBreakpoint(addr);
if (bp != INVALID_BREAKPOINT) {
breakPoints_[bp].hasCond = false;
@@ -268,7 +253,6 @@ void BreakpointManager::ChangeBreakPointRemoveCond(u32 addr) {
}
BreakPointCond *BreakpointManager::GetBreakPointCondition(u32 addr) {
std::lock_guard<std::mutex> guard(breakPointsMutex_);
size_t bp = FindBreakpoint(addr);
if (bp != INVALID_BREAKPOINT && breakPoints_[bp].hasCond)
return &breakPoints_[bp].cond;
@@ -276,7 +260,6 @@ BreakPointCond *BreakpointManager::GetBreakPointCondition(u32 addr) {
}
void BreakpointManager::ChangeBreakPointLogFormat(u32 addr, const std::string &fmt) {
std::unique_lock<std::mutex> guard(breakPointsMutex_);
size_t bp = FindBreakpoint(addr, true, false);
if (bp != INVALID_BREAKPOINT) {
breakPoints_[bp].logFormat = fmt;
@@ -287,11 +270,9 @@ void BreakpointManager::ChangeBreakPointLogFormat(u32 addr, const std::string &f
BreakAction BreakpointManager::ExecBreakPoint(u32 addr) {
if (!anyBreakPoints_)
return BREAK_ACTION_IGNORE;
std::unique_lock<std::mutex> guard(breakPointsMutex_);
size_t bp = FindBreakpoint(addr, false);
if (bp != INVALID_BREAKPOINT) {
const BreakPoint &info = breakPoints_[bp];
guard.unlock();
if (info.hasCond) {
// Evaluate the breakpoint and abort if necessary.
@@ -320,8 +301,6 @@ BreakAction BreakpointManager::ExecBreakPoint(u32 addr) {
}
int BreakpointManager::AddMemCheck(u32 start, u32 end, MemCheckCondition cond, BreakAction result) {
std::unique_lock<std::mutex> guard(memCheckMutex_);
size_t mc = FindMemCheck(start, end);
if (mc == INVALID_MEMCHECK) {
MemCheck check;
@@ -351,8 +330,6 @@ int BreakpointManager::AddMemCheck(u32 start, u32 end, MemCheckCondition cond, B
void BreakpointManager::RemoveMemCheck(u32 start, u32 end)
{
std::unique_lock<std::mutex> guard(memCheckMutex_);
size_t mc = FindMemCheck(start, end);
if (mc != INVALID_MEMCHECK)
{
@@ -366,7 +343,6 @@ void BreakpointManager::RemoveMemCheck(u32 start, u32 end)
void BreakpointManager::ChangeMemCheck(u32 start, u32 end, MemCheckCondition cond, BreakAction result)
{
std::unique_lock<std::mutex> guard(memCheckMutex_);
size_t mc = FindMemCheck(start, end);
if (mc != INVALID_MEMCHECK)
{
@@ -378,8 +354,6 @@ void BreakpointManager::ChangeMemCheck(u32 start, u32 end, MemCheckCondition con
void BreakpointManager::ClearAllMemChecks()
{
std::unique_lock<std::mutex> guard(memCheckMutex_);
if (!memChecks_.empty())
{
memChecks_.clear();
@@ -392,7 +366,6 @@ void BreakpointManager::ClearAllMemChecks()
void BreakpointManager::ChangeMemCheckAddCond(u32 start, u32 end, const BreakPointCond &cond) {
std::unique_lock<std::mutex> guard(memCheckMutex_);
size_t mc = FindMemCheck(start, end);
if (mc != INVALID_MEMCHECK) {
memChecks_[mc].hasCondition = true;
@@ -403,7 +376,6 @@ void BreakpointManager::ChangeMemCheckAddCond(u32 start, u32 end, const BreakPoi
}
void BreakpointManager::ChangeMemCheckRemoveCond(u32 start, u32 end) {
std::unique_lock<std::mutex> guard(memCheckMutex_);
size_t mc = FindMemCheck(start, end);
if (mc != INVALID_MEMCHECK) {
memChecks_[mc].hasCondition = false;
@@ -413,7 +385,6 @@ void BreakpointManager::ChangeMemCheckRemoveCond(u32 start, u32 end) {
}
BreakPointCond *BreakpointManager::GetMemCheckCondition(u32 start, u32 end) {
std::unique_lock<std::mutex> guard(memCheckMutex_);
size_t mc = FindMemCheck(start, end);
if (mc != INVALID_MEMCHECK && memChecks_[mc].hasCondition)
return &memChecks_[mc].condition;
@@ -421,7 +392,6 @@ BreakPointCond *BreakpointManager::GetMemCheckCondition(u32 start, u32 end) {
}
void BreakpointManager::ChangeMemCheckLogFormat(u32 start, u32 end, const std::string &fmt) {
std::unique_lock<std::mutex> guard(memCheckMutex_);
size_t mc = FindMemCheck(start, end);
if (mc != INVALID_MEMCHECK) {
memChecks_[mc].logFormat = fmt;
@@ -430,7 +400,6 @@ void BreakpointManager::ChangeMemCheckLogFormat(u32 start, u32 end, const std::s
}
bool BreakpointManager::GetMemCheck(u32 start, u32 end, MemCheck *check) {
std::lock_guard<std::mutex> guard(memCheckMutex_);
size_t mc = FindMemCheck(start, end);
if (mc != INVALID_MEMCHECK) {
*check = memChecks_[mc];
@@ -447,14 +416,13 @@ static inline u32 NotCached(u32 val) {
}
bool BreakpointManager::GetMemCheckInRange(u32 address, int size, MemCheck *check) {
std::lock_guard<std::mutex> guard(memCheckMutex_);
auto result = GetMemCheckLocked(address, size);
auto result = FindMemCheckInRange(address, size);
if (result)
*check = *result;
return result != nullptr;
}
MemCheck *BreakpointManager::GetMemCheckLocked(u32 address, int size) {
MemCheck *BreakpointManager::FindMemCheckInRange(u32 address, int size) {
std::vector<MemCheck>::iterator iter;
for (iter = memChecks_.begin(); iter != memChecks_.end(); ++iter)
{
@@ -479,15 +447,13 @@ BreakAction BreakpointManager::ExecMemCheck(u32 address, bool write, int size, u
{
if (!anyMemChecks_)
return BREAK_ACTION_IGNORE;
std::unique_lock<std::mutex> guard(memCheckMutex_);
auto check = GetMemCheckLocked(address, size);
MemCheck *check = FindMemCheckInRange(address, size);
if (check) {
BreakAction applyAction = check->Apply(address, write, size, pc);
if (applyAction == BREAK_ACTION_IGNORE)
return applyAction;
auto copy = *check;
guard.unlock();
MemCheck copy = *check;
return copy.Action(address, write, size, pc, reason);
}
return BREAK_ACTION_IGNORE;
@@ -504,8 +470,7 @@ BreakAction BreakpointManager::ExecOpMemCheck(u32 address, u32 pc) {
}
bool write = MIPSAnalyst::IsOpMemoryWrite(pc);
std::unique_lock<std::mutex> guard(memCheckMutex_);
auto check = GetMemCheckLocked(address, size);
MemCheck *check = FindMemCheckInRange(address, size);
if (check) {
int mask = MEMCHECK_WRITE | MEMCHECK_WRITE_ONCHANGE;
bool apply = false;
@@ -521,9 +486,7 @@ BreakAction BreakpointManager::ExecOpMemCheck(u32 address, u32 pc) {
if (applyAction == BREAK_ACTION_IGNORE)
return applyAction;
// Make a copy so we can safely unlock.
auto copy = *check;
guard.unlock();
MemCheck copy = *check;
return copy.Action(address, write, size, pc, "CPU");
}
}
@@ -563,7 +526,6 @@ static MemCheck VRAMMirror(uint8_t mirror, MemCheck mc) {
}
void BreakpointManager::UpdateCachedMemCheckRanges() {
std::lock_guard<std::mutex> guard(memCheckMutex_);
memCheckRangesRead_.clear();
memCheckRangesWrite_.clear();
@@ -592,29 +554,24 @@ void BreakpointManager::UpdateCachedMemCheckRanges() {
}
std::vector<MemCheck> BreakpointManager::GetMemCheckRanges(bool write) {
std::lock_guard<std::mutex> guard(memCheckMutex_);
if (write)
return memCheckRangesWrite_;
return memCheckRangesRead_;
}
std::vector<MemCheck> BreakpointManager::GetMemChecks() {
std::lock_guard<std::mutex> guard(memCheckMutex_);
return memChecks_;
}
std::vector<BreakPoint> BreakpointManager::GetBreakpoints() {
std::lock_guard<std::mutex> guard(breakPointsMutex_);
return breakPoints_;
}
void BreakpointManager::Frame() {
// outside the lock here, should be ok.
if (!needsUpdate_) {
return;
}
std::lock_guard<std::mutex> guard(breakPointsMutex_);
if (MIPSComp::jit && updateAddr_ != INVALID_ADDRESS) {
// In case this is a delay slot, clear the previous instruction too.
if (updateAddr_ != 0)