Merge branch 'debugger2' of https://github.com/unknownbrackets/ppsspp into unknownbrackets-debugger2

Conflicts:
	Core/Debugger/Breakpoints.cpp

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 <cstdio>
@@ -27,10 +28,17 @@ std::vector<BreakPoint> CBreakPoints::breakPoints_;
 u32 CBreakPoints::breakSkipFirstAt_ = 0;
 u64 CBreakPoints::breakSkipFirstTicks_ = 0;
 std::vector<MemCheck> CBreakPoints::memChecks_;
+std::vector<MemCheck *> 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;

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<MemCheck> memChecks_;
+	static std::vector<MemCheck *> cleanupMemChecks_;
 };
 
 

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()

Core/MIPS/MIPSAnalyst.cpp

@@ -55,6 +55,8 @@ static std::set<HashMapFunc> 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;
 

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 {

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;
 	}
 

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;
 	}
 }
 

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)
 	{

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");