diff --git a/Core/Debugger/Breakpoints.cpp b/Core/Debugger/Breakpoints.cpp index 65dd5109a5..fe77fb9250 100644 --- a/Core/Debugger/Breakpoints.cpp +++ b/Core/Debugger/Breakpoints.cpp @@ -19,6 +19,7 @@ #include "Core/Debugger/Breakpoints.h" #include "Core/Debugger/SymbolMap.h" #include "Core/Host.h" +#include "Core/MIPS/MIPSAnalyst.h" #include "Core/MIPS/JitCommon/JitCommon.h" #include "Core/CoreTiming.h" #include @@ -27,10 +28,17 @@ std::vector CBreakPoints::breakPoints_; u32 CBreakPoints::breakSkipFirstAt_ = 0; u64 CBreakPoints::breakSkipFirstTicks_ = 0; std::vector CBreakPoints::memChecks_; +std::vector CBreakPoints::cleanupMemChecks_; -MemCheck::MemCheck(void) +MemCheck::MemCheck() { - numHits=0; + numHits = 0; +} + +void MemCheck::Log(u32 addr, bool write, int size, u32 pc) +{ + if (result & MEMCHECK_LOG) + NOTICE_LOG(MEMMAP, "CHK %s%i at %08x (%s), PC=%08x (%s)", write ? "Write" : "Read", size * 8, addr, symbolMap.GetDescription(addr), pc, symbolMap.GetDescription(pc)); } void MemCheck::Action(u32 addr, bool write, int size, u32 pc) @@ -40,8 +48,7 @@ void MemCheck::Action(u32 addr, bool write, int size, u32 pc) { ++numHits; - if (result & MEMCHECK_LOG) - NOTICE_LOG(MEMMAP, "CHK %s%i at %08x (%s), PC=%08x (%s)", write ? "Write" : "Read", size * 8, addr, symbolMap.GetDescription(addr).c_str(), pc, symbolMap.GetDescription(pc).c_str()); + Log(addr, write, size, pc); if (result & MEMCHECK_BREAK) { Core_EnableStepping(true); @@ -50,6 +57,49 @@ void MemCheck::Action(u32 addr, bool write, int size, u32 pc) } } +void MemCheck::JitBefore(u32 addr, bool write, int size, u32 pc) +{ + int mask = MEMCHECK_WRITE | MEMCHECK_WRITE_ONCHANGE; + if (write && (cond & mask) == mask) + { + lastAddr = addr; + lastPC = pc; + lastSize = size; + + // We have to break to find out if it changed. + Core_EnableStepping(true); + } + else + { + lastAddr = 0; + Action(addr, write, size, pc); + } +} + +void MemCheck::JitCleanup() +{ + if (lastAddr == 0 || lastPC == 0) + return; + + // Here's the tricky part: would this have changed memory? + // Note that it did not actually get written. + bool changed = MIPSAnalyst::OpWouldChangeMemory(lastPC, lastAddr); + if (changed) + { + ++numHits; + Log(lastAddr, true, lastSize, lastPC); + } + + // Resume if it should not have gone to stepping, or if it did not change. + if ((!(result & MEMCHECK_BREAK) || !changed) && coreState == CORE_STEPPING) + { + CBreakPoints::SetSkipFirst(lastPC); + Core_EnableStepping(false); + } + else + host->SetDebugMode(true); +} + size_t CBreakPoints::FindBreakpoint(u32 addr, bool matchTemp, bool temp) { for (size_t i = 0; i < breakPoints_.size(); ++i) @@ -198,6 +248,9 @@ BreakPointCond *CBreakPoints::GetBreakPointCondition(u32 addr) void CBreakPoints::AddMemCheck(u32 start, u32 end, MemCheckCondition cond, MemCheckResult result) { + // This will ruin any pending memchecks. + cleanupMemChecks_.clear(); + size_t mc = FindMemCheck(start, end); if (mc == INVALID_MEMCHECK) { @@ -220,6 +273,9 @@ void CBreakPoints::AddMemCheck(u32 start, u32 end, MemCheckCondition cond, MemCh void CBreakPoints::RemoveMemCheck(u32 start, u32 end) { + // This will ruin any pending memchecks. + cleanupMemChecks_.clear(); + size_t mc = FindMemCheck(start, end); if (mc != INVALID_MEMCHECK) { @@ -241,6 +297,9 @@ void CBreakPoints::ChangeMemCheck(u32 start, u32 end, MemCheckCondition cond, Me void CBreakPoints::ClearAllMemChecks() { + // This will ruin any pending memchecks. + cleanupMemChecks_.clear(); + if (!memChecks_.empty()) { memChecks_.clear(); @@ -283,6 +342,24 @@ void CBreakPoints::ExecMemCheck(u32 address, bool write, int size, u32 pc) check->Action(address, write, size, pc); } +void CBreakPoints::ExecMemCheckJitBefore(u32 address, bool write, int size, u32 pc) +{ + auto check = GetMemCheck(address, size); + if (check) { + check->JitBefore(address, write, size, pc); + cleanupMemChecks_.push_back(check); + } +} + +void CBreakPoints::ExecMemCheckJitCleanup() +{ + for (auto it = cleanupMemChecks_.begin(), end = cleanupMemChecks_.end(); it != end; ++it) { + auto check = *it; + check->JitCleanup(); + } + cleanupMemChecks_.clear(); +} + void CBreakPoints::SetSkipFirst(u32 pc) { breakSkipFirstAt_ = pc; diff --git a/Core/Debugger/Breakpoints.h b/Core/Debugger/Breakpoints.h index 76be4000d7..e371ab4475 100644 --- a/Core/Debugger/Breakpoints.h +++ b/Core/Debugger/Breakpoints.h @@ -63,6 +63,7 @@ enum MemCheckCondition { MEMCHECK_READ = 0x01, MEMCHECK_WRITE = 0x02, + MEMCHECK_WRITE_ONCHANGE = 0x04, MEMCHECK_READWRITE = 0x03, }; @@ -87,7 +88,15 @@ struct MemCheck u32 numHits; + u32 lastPC; + u32 lastAddr; + int lastSize; + void Action(u32 addr, bool write, int size, u32 pc); + void JitBefore(u32 addr, bool write, int size, u32 pc); + void JitCleanup(); + + void Log(u32 addr, bool write, int size, u32 pc); bool operator == (const MemCheck &other) const { return start == other.start && end == other.end; @@ -125,6 +134,10 @@ public: static MemCheck *GetMemCheck(u32 address, int size); static void ExecMemCheck(u32 address, bool write, int size, u32 pc); + // Executes memchecks but used by the jit. Cleanup finalizes after jit is done. + static void ExecMemCheckJitBefore(u32 address, bool write, int size, u32 pc); + static void ExecMemCheckJitCleanup(); + static void SetSkipFirst(u32 pc); static u32 CheckSkipFirst(); @@ -146,6 +159,7 @@ private: static u64 breakSkipFirstTicks_; static std::vector memChecks_; + static std::vector cleanupMemChecks_; }; diff --git a/Core/MIPS/JitCommon/JitState.h b/Core/MIPS/JitCommon/JitState.h index 6003ae0720..7dc1563042 100644 --- a/Core/MIPS/JitCommon/JitState.h +++ b/Core/MIPS/JitCommon/JitState.h @@ -49,6 +49,7 @@ namespace MIPSComp { AFTER_NONE = 0x00, AFTER_CORE_STATE = 0x01, AFTER_REWIND_PC_BAD_STATE = 0x02, + AFTER_MEMCHECK_CLEANUP = 0x04, }; JitState() diff --git a/Core/MIPS/MIPSAnalyst.cpp b/Core/MIPS/MIPSAnalyst.cpp index 66b1aca7b0..42220971af 100644 --- a/Core/MIPS/MIPSAnalyst.cpp +++ b/Core/MIPS/MIPSAnalyst.cpp @@ -55,6 +55,8 @@ static std::set hashMap; static std::string hashmapFileName; +#define MIPSTABLE_IMM_MASK 0xFC000000 + namespace MIPSAnalyst { // Only can ever output a single reg. MIPSGPReg GetOutGPReg(MIPSOpcode op) { @@ -118,6 +120,48 @@ namespace MIPSAnalyst { return (op >> 26) == 0 && (op & 0x3f) == 12; } + static bool IsSWInstr(MIPSOpcode op) { + return (op & MIPSTABLE_IMM_MASK) == 0xAC000000; + } + static bool IsSBInstr(MIPSOpcode op) { + return (op & MIPSTABLE_IMM_MASK) == 0xA0000000; + } + static bool IsSHInstr(MIPSOpcode op) { + return (op & MIPSTABLE_IMM_MASK) == 0xA4000000; + } + + static bool IsSWLInstr(MIPSOpcode op) { + return (op & MIPSTABLE_IMM_MASK) == 0xA8000000; + } + static bool IsSWRInstr(MIPSOpcode op) { + return (op & MIPSTABLE_IMM_MASK) == 0xB8000000; + } + + bool OpWouldChangeMemory(u32 pc, u32 addr) { + auto op = Memory::Read_Instruction(pc); + int gprMask = 0; + // TODO: swl/swr are annoying, not handled yet. + if (IsSWInstr(op)) + gprMask = 0xFFFFFFFF; + if (IsSHInstr(op)) + gprMask = 0x0000FFFF; + if (IsSBInstr(op)) + gprMask = 0x000000FF; + + if (gprMask != 0) + { + MIPSGPReg reg = MIPS_GET_RT(op); + u32 writeVal = currentMIPS->r[reg] & gprMask; + u32 prevVal = Memory::Read_U32(addr) & gprMask; + + // TODO: Technically, the break might be for 1 byte in the middle of a sw. + return writeVal != prevVal; + } + + // TODO: Not handled yet. + return true; + } + AnalysisResults Analyze(u32 address) { const int MAX_ANALYZE = 10000; diff --git a/Core/MIPS/MIPSAnalyst.h b/Core/MIPS/MIPSAnalyst.h index 7cb3642b0f..8f11c681f8 100644 --- a/Core/MIPS/MIPSAnalyst.h +++ b/Core/MIPS/MIPSAnalyst.h @@ -125,6 +125,8 @@ namespace MIPSAnalyst bool IsDelaySlotNiceFPU(MIPSOpcode branchOp, MIPSOpcode op); bool IsSyscall(MIPSOpcode op); + bool OpWouldChangeMemory(u32 pc, u32 addr); + void Shutdown(); typedef struct { diff --git a/Core/MIPS/MIPSInt.cpp b/Core/MIPS/MIPSInt.cpp index 74a1590dad..ec7f297507 100644 --- a/Core/MIPS/MIPSInt.cpp +++ b/Core/MIPS/MIPSInt.cpp @@ -22,13 +22,14 @@ #include "math/math_util.h" #include "Common/Common.h" +#include "Core/Config.h" #include "Core/Core.h" +#include "Core/Host.h" #include "Core/MIPS/MIPS.h" #include "Core/MIPS/MIPSInt.h" #include "Core/MIPS/MIPSTables.h" #include "Core/MIPS/JitCommon/JitCommon.h" #include "Core/Reporting.h" -#include "Core/Config.h" #include "Core/HLE/HLE.h" #include "Core/HLE/HLETables.h" #include "Core/HLE/ReplaceTables.h" @@ -182,8 +183,10 @@ namespace MIPSInt { Reporting::ReportMessage("BREAK instruction hit"); ERROR_LOG(CPU, "BREAK!"); - if (!g_Config.bIgnoreBadMemAccess) - Core_UpdateState(CORE_STEPPING); + if (!g_Config.bIgnoreBadMemAccess) { + Core_EnableStepping(true); + host->SetDebugMode(true); + } PC += 4; } diff --git a/Core/MIPS/x86/Jit.cpp b/Core/MIPS/x86/Jit.cpp index 07a984de53..79c89170b1 100644 --- a/Core/MIPS/x86/Jit.cpp +++ b/Core/MIPS/x86/Jit.cpp @@ -101,6 +101,24 @@ u32 JitBreakpoint() return 1; } +void JitMemCheck(u32 addr, int size, int isWrite) +{ + // Should we skip this breakpoint? + if (CBreakPoints::CheckSkipFirst() == currentMIPS->pc) + return; + + // Did we already hit one? + if (coreState != CORE_RUNNING && coreState != CORE_NEXTFRAME) + return; + + CBreakPoints::ExecMemCheckJitBefore(addr, isWrite == 1, size, currentMIPS->pc); +} + +void JitMemCheckCleanup() +{ + CBreakPoints::ExecMemCheckJitCleanup(); +} + static void JitLogMiss(MIPSOpcode op) { if (USE_JIT_MISSMAP) @@ -602,6 +620,10 @@ void Jit::WriteExitDestInReg(X64Reg reg) void Jit::WriteSyscallExit() { WriteDowncount(); + if ((js.afterOp & JitState::AFTER_MEMCHECK_CLEANUP) != 0) { + ABI_CallFunction(&JitMemCheckCleanup); + js.afterOp &= ~JitState::AFTER_MEMCHECK_CLEANUP; + } JMP(asm_.dispatcherCheckCoreState, true); } @@ -892,19 +914,6 @@ void Jit::JitSafeMem::Finish() jit_->SetJumpTarget(*it); } -void JitMemCheck(u32 addr, int size, int isWrite) -{ - // Should we skip this breakpoint? - if (CBreakPoints::CheckSkipFirst() == currentMIPS->pc) - return; - - // Did we already hit one? - if (coreState != CORE_RUNNING && coreState != CORE_NEXTFRAME) - return; - - CBreakPoints::ExecMemCheck(addr, isWrite == 1, size, currentMIPS->pc); -} - void Jit::JitSafeMem::MemCheckImm(ReadType type) { MemCheck *check = CBreakPoints::GetMemCheck(iaddr_, size_); @@ -921,7 +930,7 @@ void Jit::JitSafeMem::MemCheckImm(ReadType type) // CORE_RUNNING is <= CORE_NEXTFRAME. jit_->CMP(32, M(&coreState), Imm32(CORE_NEXTFRAME)); skipChecks_.push_back(jit_->J_CC(CC_G, true)); - jit_->js.afterOp |= JitState::AFTER_CORE_STATE | JitState::AFTER_REWIND_PC_BAD_STATE; + jit_->js.afterOp |= JitState::AFTER_CORE_STATE | JitState::AFTER_REWIND_PC_BAD_STATE | JitState::AFTER_MEMCHECK_CLEANUP; } } @@ -971,7 +980,7 @@ void Jit::JitSafeMem::MemCheckAsm(ReadType type) // CORE_RUNNING is <= CORE_NEXTFRAME. jit_->CMP(32, M(&coreState), Imm32(CORE_NEXTFRAME)); skipChecks_.push_back(jit_->J_CC(CC_G, true)); - jit_->js.afterOp |= JitState::AFTER_CORE_STATE | JitState::AFTER_REWIND_PC_BAD_STATE; + jit_->js.afterOp |= JitState::AFTER_CORE_STATE | JitState::AFTER_REWIND_PC_BAD_STATE | JitState::AFTER_MEMCHECK_CLEANUP; } } diff --git a/Windows/Debugger/BreakpointWindow.cpp b/Windows/Debugger/BreakpointWindow.cpp index b019318bdb..20e1fc80ca 100644 --- a/Windows/Debugger/BreakpointWindow.cpp +++ b/Windows/Debugger/BreakpointWindow.cpp @@ -182,10 +182,13 @@ void BreakpointWindow::addBreakpoint() if (memory) { // add memcheck - MemCheckCondition cond; - if (read && write) cond = MEMCHECK_READWRITE; - else if (read) cond = MEMCHECK_READ; - else cond = MEMCHECK_WRITE; + int cond = 0; + if (read) + cond |= MEMCHECK_READ; + if (write) + cond |= MEMCHECK_WRITE; + if (onChange) + cond |= MEMCHECK_WRITE_ONCHANGE; MemCheckResult result; if (log && enabled) result = MEMCHECK_BOTH; @@ -193,7 +196,7 @@ void BreakpointWindow::addBreakpoint() else if (enabled) result = MEMCHECK_BREAK; else result = MEMCHECK_IGNORE; - CBreakPoints::AddMemCheck(address,address+size,cond,result); + CBreakPoints::AddMemCheck(address, address + size, (MemCheckCondition)cond, result); } else { // add breakpoint CBreakPoints::AddBreakPoint(address,false); @@ -218,23 +221,9 @@ void BreakpointWindow::loadFromMemcheck(MemCheck& memcheck) { memory = true; - switch (memcheck.cond) - { - case MEMCHECK_READWRITE: - read = write = true; - break; - case MEMCHECK_READ: - read = true; - write = false; - break; - case MEMCHECK_WRITE: - read = false; - write = true; - break; - default: - read = write = false; - break; - } + read = (memcheck.cond & MEMCHECK_READ) != 0; + write = (memcheck.cond & MEMCHECK_WRITE) != 0; + onChange = (memcheck.cond & MEMCHECK_WRITE_ONCHANGE) != 0; switch (memcheck.result) { diff --git a/Windows/Debugger/Debugger_Lists.cpp b/Windows/Debugger/Debugger_Lists.cpp index b5f1de0115..f20279220b 100644 --- a/Windows/Debugger/Debugger_Lists.cpp +++ b/Windows/Debugger/Debugger_Lists.cpp @@ -429,7 +429,7 @@ void CtrlBreakpointList::GetColumnText(wchar_t* dest, int row, int col) case BPL_TYPE: { if (isMemory) { - switch (displayedMemChecks_[index].cond) { + switch ((int)displayedMemChecks_[index].cond) { case MEMCHECK_READ: wcscpy(dest,L"Read"); break; @@ -439,6 +439,12 @@ void CtrlBreakpointList::GetColumnText(wchar_t* dest, int row, int col) case MEMCHECK_READWRITE: wcscpy(dest,L"Read/Write"); break; + case MEMCHECK_WRITE | MEMCHECK_WRITE_ONCHANGE: + wcscpy(dest,L"Write Change"); + break; + case MEMCHECK_READWRITE | MEMCHECK_WRITE_ONCHANGE: + wcscpy(dest,L"Read/Write Change"); + break; } } else { wcscpy(dest,L"Execute");