Merge pull request #17244 from unknownbrackets/debugger-memcheck

Debugger: Trigger mem breakpoints for mirrors

Core/Debugger/Breakpoints.cpp

@@ -41,6 +41,8 @@ u64 CBreakPoints::breakSkipFirstTicks_ = 0;
 static std::mutex memCheckMutex_;
 std::vector<MemCheck> CBreakPoints::memChecks_;
 std::vector<MemCheck *> CBreakPoints::cleanupMemChecks_;
+std::vector<MemCheck> CBreakPoints::memCheckRangesRead_;
+std::vector<MemCheck> CBreakPoints::memCheckRangesWrite_;
 
 void MemCheck::Log(u32 addr, bool write, int size, u32 pc, const char *reason) {
 	if (result & BREAK_ACTION_LOG) {
@@ -489,9 +491,10 @@ bool CBreakPoints::GetMemCheck(u32 start, u32 end, MemCheck *check) {
 	return false;
 }
 
-static inline u32 NotCached(u32 val)
+static inline u32 NotCached(u32 val) {
-{
+	// Remove the cached part of the address as well as any mirror.
-	// Remove the cached part of the address.
+	if ((val & 0x3F800000) == 0x04000000)
+		return val & ~0x40600000;
 	return val & ~0x40000000;
 }
 
@@ -614,24 +617,60 @@ u32 CBreakPoints::CheckSkipFirst()
 	return 0;
 }
 
+static MemCheck NotCached(MemCheck mc) {
+	// Toggle the cached part of the address.
+	mc.start ^= 0x40000000;
+	if (mc.end != 0)
+		mc.end ^= 0x40000000;
+	return mc;
+}
+
+static MemCheck VRAMMirror(uint8_t mirror, MemCheck mc) {
+	mc.start &= ~0x00600000;
+	mc.start += 0x00200000 * mirror;
+	if (mc.end != 0) {
+		mc.end &= ~0x00600000;
+		mc.end += 0x00200000 * mirror;
+		if (mc.end < mc.start)
+			mc.end += 0x00200000;
+	}
+	return mc;
+}
+
+void CBreakPoints::UpdateCachedMemCheckRanges() {
+	std::lock_guard<std::mutex> guard(memCheckMutex_);
+	memCheckRangesRead_.clear();
+	memCheckRangesWrite_.clear();
+
+	auto add = [&](bool read, bool write, const MemCheck &mc) {
+		if (read)
+			memCheckRangesRead_.push_back(mc);
+		if (write)
+			memCheckRangesWrite_.push_back(mc);
+	};
+
+	for (const auto &check : memChecks_) {
+		bool read = (check.cond & MEMCHECK_READ) != 0;
+		bool write = (check.cond & MEMCHECK_WRITE) != 0;
+
+		if (Memory::IsVRAMAddress(check.start) && (check.end == 0 || Memory::IsVRAMAddress(check.end))) {
+			for (uint8_t mirror = 0; mirror < 4; ++mirror) {
+				MemCheck copy = VRAMMirror(mirror, check);
+				add(read, write, copy);
+				add(read, write, NotCached(copy));
+			}
+		} else {
+			add(read, write, check);
+			add(read, write, NotCached(check));
+		}
+	}
+}
+
 const std::vector<MemCheck> CBreakPoints::GetMemCheckRanges(bool write) {
 	std::lock_guard<std::mutex> guard(memCheckMutex_);
-	std::vector<MemCheck> ranges = memChecks_;
+	if (write)
-	for (const auto &check : memChecks_) {
+		return memCheckRangesWrite_;
-		if (!(check.cond & MEMCHECK_READ) && !write)
+	return memCheckRangesRead_;
-			continue;
-		if (!(check.cond & MEMCHECK_WRITE) && write)
-			continue;
-
-		MemCheck copy = check;
-		// Toggle the cached part of the address.
-		copy.start ^= 0x40000000;
-		if (copy.end != 0)
-			copy.end ^= 0x40000000;
-		ranges.push_back(copy);
-	}
-
-	return ranges;
 }
 
 const std::vector<MemCheck> CBreakPoints::GetMemChecks()
@@ -673,6 +712,9 @@ void CBreakPoints::Update(u32 addr) {
 			Core_EnableStepping(false);
 	}
 
+	if (anyMemChecks_)
+		UpdateCachedMemCheckRanges();
+
 	// Redraw in order to show the breakpoint.
 	System_Notify(SystemNotification::DISASSEMBLY);
 }

Core/Debugger/Breakpoints.h

@@ -180,6 +180,7 @@ private:
 	// Finds exactly, not using a range check.
 	static size_t FindMemCheck(u32 start, u32 end);
 	static MemCheck *GetMemCheckLocked(u32 address, int size);
+	static void UpdateCachedMemCheckRanges();
 
 	static std::vector<BreakPoint> breakPoints_;
 	static u32 breakSkipFirstAt_;
@@ -187,6 +188,8 @@ private:
 
 	static std::vector<MemCheck> memChecks_;
 	static std::vector<MemCheck *> cleanupMemChecks_;
+	static std::vector<MemCheck> memCheckRangesRead_;
+	static std::vector<MemCheck> memCheckRangesWrite_;
 };
 
 

Core/MIPS/ARM/ArmCompFPU.cpp

@@ -95,7 +95,7 @@ void ArmJit::Comp_FPULS(MIPSOpcode op)
 	CONDITIONAL_DISABLE(LSU_FPU);
 	CheckMemoryBreakpoint();
 
-	s32 offset = (s16)(op & 0xFFFF);
+	s32 offset = SignExtend16ToS32(op & 0xFFFF);
 	int ft = _FT;
 	MIPSGPReg rs = _RS;
 	// u32 addr = R(rs) + offset;

Core/MIPS/ARM/ArmCompLoadStore.cpp

@@ -113,7 +113,7 @@ namespace MIPSComp
 	void ArmJit::Comp_ITypeMemLR(MIPSOpcode op, bool load) {
 		CONDITIONAL_DISABLE(LSU);
 		CheckMemoryBreakpoint();
-		int offset = (signed short)(op & 0xFFFF);
+		int offset = SignExtend16ToS32(op & 0xFFFF);
 		MIPSGPReg rt = _RT;
 		MIPSGPReg rs = _RS;
 		int o = op >> 26;

Core/MIPS/ARM/ArmJit.cpp

@@ -91,7 +91,7 @@ static u32 JitMemCheck(u32 pc) {
 
 	// Note: pc may be the delay slot.
 	const auto op = Memory::Read_Instruction(pc, true);
-	s32 offset = (s16)(op & 0xFFFF);
+	s32 offset = SignExtend16ToS32(op & 0xFFFF);
 	if (MIPSGetInfo(op) & IS_VFPU)
 		offset &= 0xFFFC;
 	u32 addr = currentMIPS->r[MIPS_GET_RS(op)] + offset;

Core/MIPS/ARM64/Arm64CompFPU.cpp

@@ -83,9 +83,7 @@ void Arm64Jit::Comp_FPULS(MIPSOpcode op)
 	CONDITIONAL_DISABLE(LSU_FPU);
 	CheckMemoryBreakpoint();
 
-	// Surprisingly, these work fine alraedy.
+	s32 offset = SignExtend16ToS32(op & 0xFFFF);
-
-	s32 offset = (s16)(op & 0xFFFF);
 	int ft = _FT;
 	MIPSGPReg rs = _RS;
 	// u32 addr = R(rs) + offset;

Core/MIPS/ARM64/Arm64CompLoadStore.cpp

@@ -115,7 +115,7 @@ namespace MIPSComp {
 	void Arm64Jit::Comp_ITypeMemLR(MIPSOpcode op, bool load) {
 		CONDITIONAL_DISABLE(LSU);
 		CheckMemoryBreakpoint();
-		int offset = (signed short)(op & 0xFFFF);
+		int offset = SignExtend16ToS32(op & 0xFFFF);
 		MIPSGPReg rt = _RT;
 		MIPSGPReg rs = _RS;
 		int o = op >> 26;
@@ -267,7 +267,7 @@ namespace MIPSComp {
 		CONDITIONAL_DISABLE(LSU);
 		CheckMemoryBreakpoint();
 
-		int offset = (signed short)(op & 0xFFFF);
+		int offset = SignExtend16ToS32(op & 0xFFFF);
 		bool load = false;
 		MIPSGPReg rt = _RT;
 		MIPSGPReg rs = _RS;

Core/MIPS/ARM64/Arm64Jit.cpp

@@ -81,7 +81,7 @@ static u32 JitMemCheck(u32 pc) {
 
 	// Note: pc may be the delay slot.
 	const auto op = Memory::Read_Instruction(pc, true);
-	s32 offset = (s16)(op & 0xFFFF);
+	s32 offset = SignExtend16ToS32(op & 0xFFFF);
 	if (MIPSGetInfo(op) & IS_VFPU)
 		offset &= 0xFFFC;
 	u32 addr = currentMIPS->r[MIPS_GET_RS(op)] + offset;

Core/MIPS/MIPSAnalyst.cpp

@@ -1449,7 +1449,7 @@ skip:
 		case 0x08:	// addi
 		case 0x09:	// addiu
 			info.hasRelevantAddress = true;
-			info.relevantAddress = cpu->GetRegValue(0,MIPS_GET_RS(op))+((s16)(op & 0xFFFF));
+			info.relevantAddress = cpu->GetRegValue(0, MIPS_GET_RS(op)) + SignExtend16ToS32(op & 0xFFFF);
 			break;
 		}
 

Core/MIPS/MIPSDis.cpp

@@ -64,7 +64,7 @@ namespace MIPSDis
 
 	void Dis_Cache(MIPSOpcode op, char *out)
 	{
-		int imm = (s16)(op & 0xFFFF);
+		int imm = SignExtend16ToS32(op & 0xFFFF);
 		int rs = _RS;
 		int func = (op >> 16) & 0x1F;
 		sprintf(out, "%s\tfunc=%i, %s(%s)", MIPSGetName(op), func, RN(rs), SignedHex(imm));

Core/MIPS/MIPSInt.cpp

@@ -94,7 +94,7 @@ namespace MIPSInt
 {
 	void Int_Cache(MIPSOpcode op)
 	{
-		int imm = (s16)(op & 0xFFFF);
+		int imm = SignExtend16ToS32(op & 0xFFFF);
 		int rs = _RS;
 		uint32_t addr = R(rs) + imm;
 		int func = (op >> 16) & 0x1F;

Core/MIPS/x86/JitSafeMem.cpp

@@ -403,42 +403,42 @@ void JitSafeMem::MemCheckImm(MemoryOpType type) {
 	}
 }
 
-void JitSafeMem::MemCheckAsm(MemoryOpType type)
+void JitSafeMem::MemCheckAsm(MemoryOpType type) {
-{
 	const auto memchecks = CBreakPoints::GetMemCheckRanges(type == MEM_WRITE);
 	bool possible = !memchecks.empty();
-	for (auto it = memchecks.begin(), end = memchecks.end(); it != end; ++it)
+	std::vector<FixupBranch> hitChecks;
-	{
+	for (auto it = memchecks.begin(), end = memchecks.end(); it != end; ++it) {
-		FixupBranch skipNext, skipNextRange;
+		if (it->end != 0) {
-		if (it->end != 0)
-		{
 			jit_->CMP(32, R(xaddr_), Imm32(it->start - offset_ - size_));
-			skipNext = jit_->J_CC(CC_BE);
+			FixupBranch skipNext = jit_->J_CC(CC_BE);
-			jit_->CMP(32, R(xaddr_), Imm32(it->end - offset_));
-			skipNextRange = jit_->J_CC(CC_AE);
-		}
-		else
-		{
-			jit_->CMP(32, R(xaddr_), Imm32(it->start - offset_));
-			skipNext = jit_->J_CC(CC_NE);
-		}
 
-		// Keep the stack 16-byte aligned, just PUSH/POP 4 times.
+			jit_->CMP(32, R(xaddr_), Imm32(it->end - offset_));
-		for (int i = 0; i < 4; ++i)
+			hitChecks.push_back(jit_->J_CC(CC_B, true));
-			jit_->PUSH(xaddr_);
+
+			jit_->SetJumpTarget(skipNext);
+		} else {
+			jit_->CMP(32, R(xaddr_), Imm32(it->start - offset_));
+			hitChecks.push_back(jit_->J_CC(CC_E, true));
+		}
+	}
+
+	if (possible) {
+		FixupBranch noHits = jit_->J(true);
+
+		// Okay, now land any hit here.
+		for (auto &fixup : hitChecks)
+			jit_->SetJumpTarget(fixup);
+		hitChecks.clear();
+
+		jit_->PUSH(xaddr_);
+		// Keep the stack 16-byte aligned.
+		jit_->SUB(PTRBITS, R(SP), Imm32(16 - PTRBITS / 8));
 		jit_->MOV(32, MIPSSTATE_VAR(pc), Imm32(jit_->GetCompilerPC()));
 		jit_->ADD(32, R(xaddr_), Imm32(offset_));
 		jit_->CallProtectedFunction(&JitMemCheck, R(xaddr_), size_, type == MEM_WRITE ? 1 : 0);
-		for (int i = 0; i < 4; ++i)
+		jit_->ADD(PTRBITS, R(SP), Imm32(16 - PTRBITS / 8));
-			jit_->POP(xaddr_);
+		jit_->POP(xaddr_);
 
-		jit_->SetJumpTarget(skipNext);
-		if (it->end != 0)
-			jit_->SetJumpTarget(skipNextRange);
-	}
-
-	if (possible)
-	{
 		// CORE_RUNNING is <= CORE_NEXTFRAME.
 		if (jit_->RipAccessible((const void *)&coreState)) {
 			jit_->CMP(32, M(&coreState), Imm32(CORE_NEXTFRAME));  // rip accessible
@@ -451,6 +451,8 @@ void JitSafeMem::MemCheckAsm(MemoryOpType type)
 		}
 		skipChecks_.push_back(jit_->J_CC(CC_G, true));
 		jit_->js.afterOp |= JitState::AFTER_CORE_STATE | JitState::AFTER_REWIND_PC_BAD_STATE | JitState::AFTER_MEMCHECK_CLEANUP;
+
+		jit_->SetJumpTarget(noHits);
 	}
 }