diff --git a/Core/Debugger/Breakpoints.cpp b/Core/Debugger/Breakpoints.cpp index 0223a2f6fcbf..c5515f4f342d 100644 --- a/Core/Debugger/Breakpoints.cpp +++ b/Core/Debugger/Breakpoints.cpp @@ -16,6 +16,7 @@ // https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/. #include +#include #include "Common/Log.h" #include "Core/Core.h" @@ -27,9 +28,11 @@ #include "Core/MIPS/JitCommon/JitCommon.h" #include "Core/CoreTiming.h" +static std::mutex breakPointsMutex_; std::vector CBreakPoints::breakPoints_; u32 CBreakPoints::breakSkipFirstAt_ = 0; u64 CBreakPoints::breakSkipFirstTicks_ = 0; +static std::mutex memCheckMutex_; std::vector CBreakPoints::memChecks_; std::vector CBreakPoints::cleanupMemChecks_; @@ -45,16 +48,21 @@ void MemCheck::Log(u32 addr, bool write, int size, u32 pc) { } } -BreakAction MemCheck::Action(u32 addr, bool write, int size, u32 pc) -{ +BreakAction MemCheck::Apply(u32 addr, bool write, int size, u32 pc) { int mask = write ? MEMCHECK_WRITE : MEMCHECK_READ; - if (cond & mask) - { + if (cond & mask) { ++numHits; + return result; + } + + return BREAK_ACTION_IGNORE; +} +BreakAction MemCheck::Action(u32 addr, bool write, int size, u32 pc) { + int mask = write ? MEMCHECK_WRITE : MEMCHECK_READ; + if (cond & mask) { Log(addr, write, size, pc); - if (result & BREAK_ACTION_PAUSE) - { + if (result & BREAK_ACTION_PAUSE) { Core_EnableStepping(true); host->SetDebugMode(true); } @@ -65,38 +73,46 @@ BreakAction MemCheck::Action(u32 addr, bool write, int size, u32 pc) return BREAK_ACTION_IGNORE; } -void MemCheck::JitBefore(u32 addr, bool write, int size, u32 pc) -{ +void MemCheck::JitBeforeApply(u32 addr, bool write, int size, u32 pc) { int mask = MEMCHECK_WRITE | MEMCHECK_WRITE_ONCHANGE; - if (write && (cond & mask) == mask) - { + if (write && (cond & mask) == mask) { lastAddr = addr; lastPC = pc; lastSize = size; + } else { + lastAddr = 0; + Apply(addr, write, size, pc); + } +} +void MemCheck::JitBeforeAction(u32 addr, bool write, int size, u32 pc) { + if (lastAddr) { // We have to break to find out if it changed. Core_EnableStepping(true); - } - else - { - lastAddr = 0; + } else { Action(addr, write, size, pc); } } -void MemCheck::JitCleanup() -{ +bool MemCheck::JitApplyChanged() { if (lastAddr == 0 || lastPC == 0) - return; + return false; // Here's the tricky part: would this have changed memory? // Note that it did not actually get written. bool changed = MIPSAnalyst::OpWouldChangeMemory(lastPC, lastAddr, lastSize); if (changed) - { ++numHits; + return changed; +} + +void MemCheck::JitCleanup(bool changed) +{ + if (lastAddr == 0 || lastPC == 0) + return; + + if (changed) Log(lastAddr, true, lastSize, lastPC); - } // Resume if it should not have gone to stepping, or if it did not change. if ((!(result & BREAK_ACTION_PAUSE) || !changed) && coreState == CORE_STEPPING) @@ -108,6 +124,7 @@ void MemCheck::JitCleanup() host->SetDebugMode(true); } +// Note: must lock while calling this. size_t CBreakPoints::FindBreakpoint(u32 addr, bool matchTemp, bool temp) { size_t found = INVALID_BREAKPOINT; @@ -140,12 +157,14 @@ size_t CBreakPoints::FindMemCheck(u32 start, u32 end) bool CBreakPoints::IsAddressBreakPoint(u32 addr) { + std::lock_guard guard(breakPointsMutex_); size_t bp = FindBreakpoint(addr); return bp != INVALID_BREAKPOINT && breakPoints_[bp].result != BREAK_ACTION_IGNORE; } bool CBreakPoints::IsAddressBreakPoint(u32 addr, bool* enabled) { + std::lock_guard guard(breakPointsMutex_); size_t bp = FindBreakpoint(addr); if (bp == INVALID_BREAKPOINT) return false; if (enabled != nullptr) @@ -155,12 +174,14 @@ bool CBreakPoints::IsAddressBreakPoint(u32 addr, bool* enabled) bool CBreakPoints::IsTempBreakPoint(u32 addr) { + std::lock_guard guard(breakPointsMutex_); size_t bp = FindBreakpoint(addr, true, true); return bp != INVALID_BREAKPOINT; } bool CBreakPoints::RangeContainsBreakPoint(u32 addr, u32 size) { + std::lock_guard guard(breakPointsMutex_); const u32 end = addr + size; for (const auto &bp : breakPoints_) { @@ -173,6 +194,7 @@ bool CBreakPoints::RangeContainsBreakPoint(u32 addr, u32 size) void CBreakPoints::AddBreakPoint(u32 addr, bool temp) { + std::unique_lock guard(breakPointsMutex_); size_t bp = FindBreakpoint(addr, true, temp); if (bp == INVALID_BREAKPOINT) { @@ -182,18 +204,21 @@ void CBreakPoints::AddBreakPoint(u32 addr, bool temp) pt.addr = addr; breakPoints_.push_back(pt); + guard.unlock(); Update(addr); } else if (!breakPoints_[bp].IsEnabled()) { breakPoints_[bp].result |= BREAK_ACTION_PAUSE; breakPoints_[bp].hasCond = false; + guard.unlock(); Update(addr); } } void CBreakPoints::RemoveBreakPoint(u32 addr) { + std::unique_lock guard(breakPointsMutex_); size_t bp = FindBreakpoint(addr); if (bp != INVALID_BREAKPOINT) { @@ -204,12 +229,14 @@ void CBreakPoints::RemoveBreakPoint(u32 addr) if (bp != INVALID_BREAKPOINT) breakPoints_.erase(breakPoints_.begin() + bp); + guard.unlock(); Update(addr); } } void CBreakPoints::ChangeBreakPoint(u32 addr, bool status) { + std::unique_lock guard(breakPointsMutex_); size_t bp = FindBreakpoint(addr); if (bp != INVALID_BREAKPOINT) { @@ -217,31 +244,38 @@ void CBreakPoints::ChangeBreakPoint(u32 addr, bool status) breakPoints_[bp].result |= BREAK_ACTION_PAUSE; else breakPoints_[bp].result = BreakAction(breakPoints_[bp].result & ~BREAK_ACTION_PAUSE); + + guard.unlock(); Update(addr); } } void CBreakPoints::ChangeBreakPoint(u32 addr, BreakAction result) { + std::unique_lock guard(breakPointsMutex_); size_t bp = FindBreakpoint(addr); if (bp != INVALID_BREAKPOINT) { breakPoints_[bp].result = result; + guard.unlock(); Update(addr); } } void CBreakPoints::ClearAllBreakPoints() { + std::unique_lock guard(breakPointsMutex_); if (!breakPoints_.empty()) { breakPoints_.clear(); + guard.unlock(); Update(); } } void CBreakPoints::ClearTemporaryBreakPoints() { + std::unique_lock guard(breakPointsMutex_); if (breakPoints_.empty()) return; @@ -254,34 +288,40 @@ void CBreakPoints::ClearTemporaryBreakPoints() update = true; } } - + + guard.unlock(); if (update) Update(); } void CBreakPoints::ChangeBreakPointAddCond(u32 addr, const BreakPointCond &cond) { + std::unique_lock guard(breakPointsMutex_); size_t bp = FindBreakpoint(addr, true, false); if (bp != INVALID_BREAKPOINT) { breakPoints_[bp].hasCond = true; breakPoints_[bp].cond = cond; + guard.unlock(); Update(addr); } } void CBreakPoints::ChangeBreakPointRemoveCond(u32 addr) { + std::unique_lock guard(breakPointsMutex_); size_t bp = FindBreakpoint(addr, true, false); if (bp != INVALID_BREAKPOINT) { breakPoints_[bp].hasCond = false; + guard.unlock(); Update(addr); } } BreakPointCond *CBreakPoints::GetBreakPointCondition(u32 addr) { + std::lock_guard guard(breakPointsMutex_); size_t bp = FindBreakpoint(addr, true, false); if (bp != INVALID_BREAKPOINT && breakPoints_[bp].hasCond) return &breakPoints_[bp].cond; @@ -289,38 +329,44 @@ BreakPointCond *CBreakPoints::GetBreakPointCondition(u32 addr) } void CBreakPoints::ChangeBreakPointLogFormat(u32 addr, const std::string &fmt) { + std::unique_lock guard(breakPointsMutex_); size_t bp = FindBreakpoint(addr, true, false); if (bp != INVALID_BREAKPOINT) { breakPoints_[bp].logFormat = fmt; + guard.unlock(); Update(addr); } } BreakAction CBreakPoints::ExecBreakPoint(u32 addr) { + std::unique_lock guard(breakPointsMutex_); size_t bp = FindBreakpoint(addr, false); if (bp != INVALID_BREAKPOINT) { - if (breakPoints_[bp].hasCond) { + BreakPoint info = breakPoints_[bp]; + guard.unlock(); + + if (info.hasCond) { // Evaluate the breakpoint and abort if necessary. auto cond = CBreakPoints::GetBreakPointCondition(currentMIPS->pc); if (cond && !cond->Evaluate()) return BREAK_ACTION_IGNORE; } - if (breakPoints_[bp].result & BREAK_ACTION_LOG) { - if (breakPoints_[bp].logFormat.empty()) { + if (info.result & BREAK_ACTION_LOG) { + if (info.logFormat.empty()) { NOTICE_LOG(JIT, "BKP PC=%08x (%s)", addr, g_symbolMap->GetDescription(addr).c_str()); } else { std::string formatted; - CBreakPoints::EvaluateLogFormat(currentDebugMIPS, breakPoints_[bp].logFormat, formatted); + CBreakPoints::EvaluateLogFormat(currentDebugMIPS, info.logFormat, formatted); NOTICE_LOG(JIT, "BKP PC=%08x: %s", addr, formatted.c_str()); } } - if (breakPoints_[bp].result & BREAK_ACTION_PAUSE) { + if (info.result & BREAK_ACTION_PAUSE) { Core_EnableStepping(true); host->SetDebugMode(true); } - return breakPoints_[bp].result; + return info.result; } return BREAK_ACTION_IGNORE; @@ -328,6 +374,7 @@ BreakAction CBreakPoints::ExecBreakPoint(u32 addr) { void CBreakPoints::AddMemCheck(u32 start, u32 end, MemCheckCondition cond, BreakAction result) { + std::unique_lock guard(memCheckMutex_); // This will ruin any pending memchecks. cleanupMemChecks_.clear(); @@ -341,18 +388,21 @@ void CBreakPoints::AddMemCheck(u32 start, u32 end, MemCheckCondition cond, Break check.result = result; memChecks_.push_back(check); + guard.unlock(); Update(); } else { memChecks_[mc].cond = (MemCheckCondition)(memChecks_[mc].cond | cond); memChecks_[mc].result = (BreakAction)(memChecks_[mc].result | result); + guard.unlock(); Update(); } } void CBreakPoints::RemoveMemCheck(u32 start, u32 end) { + std::unique_lock guard(memCheckMutex_); // This will ruin any pending memchecks. cleanupMemChecks_.clear(); @@ -360,37 +410,44 @@ void CBreakPoints::RemoveMemCheck(u32 start, u32 end) if (mc != INVALID_MEMCHECK) { memChecks_.erase(memChecks_.begin() + mc); + guard.unlock(); Update(); } } void CBreakPoints::ChangeMemCheck(u32 start, u32 end, MemCheckCondition cond, BreakAction result) { + std::unique_lock guard(memCheckMutex_); size_t mc = FindMemCheck(start, end); if (mc != INVALID_MEMCHECK) { memChecks_[mc].cond = cond; memChecks_[mc].result = result; + guard.unlock(); Update(); } } void CBreakPoints::ClearAllMemChecks() { + std::unique_lock guard(memCheckMutex_); // This will ruin any pending memchecks. cleanupMemChecks_.clear(); if (!memChecks_.empty()) { memChecks_.clear(); + guard.unlock(); Update(); } } void CBreakPoints::ChangeMemCheckLogFormat(u32 start, u32 end, const std::string &fmt) { + std::unique_lock guard(memCheckMutex_); size_t mc = FindMemCheck(start, end); if (mc != INVALID_MEMCHECK) { memChecks_[mc].logFormat = fmt; + guard.unlock(); Update(); } } @@ -401,8 +458,12 @@ static inline u32 NotCached(u32 val) return val & ~0x40000000; } -MemCheck *CBreakPoints::GetMemCheck(u32 address, int size) -{ +MemCheck *CBreakPoints::GetMemCheck(u32 address, int size) { + std::lock_guard guard(memCheckMutex_); + return GetMemCheckLocked(address, size); +} + +MemCheck *CBreakPoints::GetMemCheckLocked(u32 address, int size) { std::vector::iterator iter; for (iter = memChecks_.begin(); iter != memChecks_.end(); ++iter) { @@ -425,9 +486,14 @@ MemCheck *CBreakPoints::GetMemCheck(u32 address, int size) BreakAction CBreakPoints::ExecMemCheck(u32 address, bool write, int size, u32 pc) { - auto check = GetMemCheck(address, size); - if (check) - return check->Action(address, write, size, pc); + std::unique_lock guard(memCheckMutex_); + auto check = GetMemCheckLocked(address, size); + if (check) { + check->Apply(address, write, size, pc); + auto copy = *check; + guard.unlock(); + return copy.Action(address, write, size, pc); + } return BREAK_ACTION_IGNORE; } @@ -443,15 +509,23 @@ BreakAction CBreakPoints::ExecOpMemCheck(u32 address, u32 pc) } bool write = MIPSAnalyst::IsOpMemoryWrite(pc); - auto check = GetMemCheck(address, size); + std::unique_lock guard(memCheckMutex_); + auto check = GetMemCheckLocked(address, size); if (check) { int mask = MEMCHECK_WRITE | MEMCHECK_WRITE_ONCHANGE; + bool apply = false; if (write && (check->cond & mask) == mask) { if (MIPSAnalyst::OpWouldChangeMemory(pc, address, size)) { - return check->Action(address, write, size, pc); + apply = true; } } else { - return check->Action(address, write, size, pc); + apply = true; + } + if (apply) { + check->Apply(address, write, size, pc); + auto copy = *check; + guard.unlock(); + return copy.Action(address, write, size, pc); } } return BREAK_ACTION_IGNORE; @@ -459,18 +533,28 @@ BreakAction CBreakPoints::ExecOpMemCheck(u32 address, u32 pc) void CBreakPoints::ExecMemCheckJitBefore(u32 address, bool write, int size, u32 pc) { - auto check = GetMemCheck(address, size); + std::unique_lock guard(memCheckMutex_); + auto check = GetMemCheckLocked(address, size); if (check) { - check->JitBefore(address, write, size, pc); + check->JitBeforeApply(address, write, size, pc); + auto copy = *check; + guard.unlock(); + copy.JitBeforeAction(address, write, size, pc); + guard.lock(); cleanupMemChecks_.push_back(check); } } void CBreakPoints::ExecMemCheckJitCleanup() { + std::unique_lock guard(memCheckMutex_); for (auto it = cleanupMemChecks_.begin(), end = cleanupMemChecks_.end(); it != end; ++it) { auto check = *it; - check->JitCleanup(); + bool changed = check->JitApplyChanged(); + auto copy = *check; + guard.unlock(); + copy.JitCleanup(changed); + guard.lock(); } cleanupMemChecks_.clear(); } @@ -489,6 +573,7 @@ u32 CBreakPoints::CheckSkipFirst() } const std::vector CBreakPoints::GetMemCheckRanges(bool write) { + std::lock_guard guard(memCheckMutex_); std::vector ranges = memChecks_; for (const auto &check : memChecks_) { if (!(check.cond & MEMCHECK_READ) && !write) @@ -509,16 +594,19 @@ const std::vector CBreakPoints::GetMemCheckRanges(bool write) { const std::vector CBreakPoints::GetMemChecks() { + std::lock_guard guard(memCheckMutex_); return memChecks_; } const std::vector CBreakPoints::GetBreakpoints() { + std::lock_guard guard(breakPointsMutex_); return breakPoints_; } bool CBreakPoints::HasMemChecks() { + std::lock_guard guard(memCheckMutex_); return !memChecks_.empty(); } diff --git a/Core/Debugger/Breakpoints.h b/Core/Debugger/Breakpoints.h index 8cd7acdc4d5b..5c5804290427 100644 --- a/Core/Debugger/Breakpoints.h +++ b/Core/Debugger/Breakpoints.h @@ -93,9 +93,14 @@ struct MemCheck { u32 lastAddr = 0; int lastSize = 0; + // Called on the stored memcheck (affects numHits, etc.) + BreakAction Apply(u32 addr, bool write, int size, u32 pc); + // Called on a copy. BreakAction Action(u32 addr, bool write, int size, u32 pc); - void JitBefore(u32 addr, bool write, int size, u32 pc); - void JitCleanup(); + void JitBeforeApply(u32 addr, bool write, int size, u32 pc); + void JitBeforeAction(u32 addr, bool write, int size, u32 pc); + bool JitApplyChanged(); + void JitCleanup(bool changed); void Log(u32 addr, bool write, int size, u32 pc); @@ -172,6 +177,7 @@ class CBreakPoints static size_t FindBreakpoint(u32 addr, bool matchTemp = false, bool temp = false); // Finds exactly, not using a range check. static size_t FindMemCheck(u32 start, u32 end); + static MemCheck *GetMemCheckLocked(u32 address, int size); static std::vector breakPoints_; static u32 breakSkipFirstAt_;