Merge pull request #2567 from unknownbrackets/debugger

Rewrite CBreakPoints, fix MemCheck bugs

Core/Debugger/Breakpoints.cpp

@@ -15,20 +15,18 @@
 // Official git repository and contact information can be found at
 // https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/.
 
-#include "../Core.h"
-#include "Breakpoints.h"
-#include "SymbolMap.h"
-#include "FixedSizeUnorderedSet.h"
+#include "Core/Core.h"
+#include "Core/Debugger/Breakpoints.h"
+#include "Core/Debugger/SymbolMap.h"
 #include "Core/Host.h"
-#include "../MIPS/JitCommon/JitCommon.h"
+#include "Core/MIPS/JitCommon/JitCommon.h"
+#include "Core/CoreTiming.h"
 #include <cstdio>
 
-#define MAX_BREAKPOINTS 16
-
-static FixedSizeUnorderedSet<BreakPoint, MAX_BREAKPOINTS> m_iBreakPoints;
-
-std::vector<MemCheck>		CBreakPoints::MemChecks;
+std::vector<BreakPoint> CBreakPoints::breakPoints_;
 u32 CBreakPoints::breakSkipFirstAt_ = 0;
+u64 CBreakPoints::breakSkipFirstTicks_ = 0;
+std::vector<MemCheck> CBreakPoints::memChecks_;
 
 MemCheck::MemCheck(void)
 {
@@ -37,13 +35,14 @@ MemCheck::MemCheck(void)
 
 void MemCheck::Action(u32 addr, bool write, int size, u32 pc)
 {
-	if ((write && bOnWrite) || (!write && bOnRead))
+	int mask = write ? MEMCHECK_WRITE : MEMCHECK_READ;
+	if (cond & mask)
 	{
 		++numHits;
 
-		if (bLog)
+		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));
-		if (bBreak)
+		if (result & MEMCHECK_BREAK)
 		{
 			Core_EnableStepping(true);
 			host->SetDebugMode(true);
@@ -51,76 +50,209 @@ void MemCheck::Action(u32 addr, bool write, int size, u32 pc)
 	}
 }
 
-bool CBreakPoints::IsAddressBreakPoint(u32 _iAddress)
+size_t CBreakPoints::FindBreakpoint(u32 addr, bool matchTemp, bool temp)
 {
-	for (size_t i = 0; i < m_iBreakPoints.size(); i++)
-		if (m_iBreakPoints[i].iAddress == _iAddress)
-			return true;
-
-	return false;
-}
-
-bool CBreakPoints::IsTempBreakPoint(u32 _iAddress)
-{
-	for (size_t i = 0; i < m_iBreakPoints.size(); i++)
-		if (m_iBreakPoints[i].iAddress == _iAddress && m_iBreakPoints[i].bTemporary)
-			return true;
-
-	return false;
-}
-
-void CBreakPoints::RemoveBreakPoint(u32 _iAddress)
-{
-	for (size_t i = 0; i < m_iBreakPoints.size(); i++)
+	for (size_t i = 0; i < breakPoints_.size(); ++i)
 	{
-		if (m_iBreakPoints[i].iAddress == _iAddress)
-		{
-			m_iBreakPoints.remove(m_iBreakPoints[i]);
-			InvalidateJit(_iAddress);
-			host->UpdateDisassembly();	// redraw in order to not show the breakpoint anymore
-			break;
-		}
+		if (breakPoints_[i].addr == addr && (!matchTemp || breakPoints_[i].temporary == temp))
+			return i;
+	}
+
+	return INVALID_BREAKPOINT;
+}
+
+size_t CBreakPoints::FindMemCheck(u32 start, u32 end)
+{
+	for (size_t i = 0; i < memChecks_.size(); ++i)
+	{
+		if (memChecks_[i].start == start && memChecks_[i].end == end)
+			return i;
+	}
+
+	return INVALID_MEMCHECK;
+}
+
+bool CBreakPoints::IsAddressBreakPoint(u32 addr)
+{
+	size_t bp = FindBreakpoint(addr);
+	return bp != INVALID_BREAKPOINT && breakPoints_[bp].enabled;
+}
+
+bool CBreakPoints::IsTempBreakPoint(u32 addr)
+{
+	size_t bp = FindBreakpoint(addr, true, true);
+	return bp != INVALID_BREAKPOINT;
+}
+
+void CBreakPoints::AddBreakPoint(u32 addr, bool temp)
+{
+	size_t bp = FindBreakpoint(addr, true, temp);
+	if (bp == INVALID_BREAKPOINT)
+	{
+		BreakPoint pt;
+		pt.enabled = true;
+		pt.temporary = temp;
+		pt.addr = addr;
+
+		breakPoints_.push_back(pt);
+		Update(addr);
+	}
+	else if (!breakPoints_[bp].enabled)
+	{
+		breakPoints_[bp].enabled = true;
+		Update(addr);
+	}
+}
+
+void CBreakPoints::RemoveBreakPoint(u32 addr)
+{
+	size_t bp = FindBreakpoint(addr);
+	if (bp != INVALID_BREAKPOINT)
+	{
+		breakPoints_.erase(breakPoints_.begin() + bp);
+
+		// Check again, there might've been an overlapping temp breakpoint.
+		bp = FindBreakpoint(addr);
+		if (bp != INVALID_BREAKPOINT)
+			breakPoints_.erase(breakPoints_.begin() + bp);
+
+		Update(addr);
+	}
+}
+
+void CBreakPoints::ChangeBreakPoint(u32 addr, bool status)
+{
+	size_t bp = FindBreakpoint(addr);
+	if (bp != INVALID_BREAKPOINT)
+	{
+		breakPoints_[bp].enabled = status;
+		Update(addr);
 	}
 }
 
 void CBreakPoints::ClearAllBreakPoints()
 {
-	m_iBreakPoints.clear();
-	InvalidateJit();
+	if (!breakPoints_.empty())
+	{
+		breakPoints_.clear();
+		Update();
+	}
 }
 
 void CBreakPoints::ClearTemporaryBreakPoints()
 {
-	if (m_iBreakPoints.size() == 0) return;
+	if (breakPoints_.empty())
+		return;
 
 	bool update = false;
-	for (int i = (int)m_iBreakPoints.size()-1; i >= 0; --i)
+	for (int i = (int)breakPoints_.size()-1; i >= 0; --i)
 	{
-		if (m_iBreakPoints[i].bTemporary)
+		if (breakPoints_[i].temporary)
 		{
-			InvalidateJit(m_iBreakPoints[i].iAddress);
-			m_iBreakPoints.remove(m_iBreakPoints[i]);
+			breakPoints_.erase(breakPoints_.begin() + i);
 			update = true;
 		}
 	}
 	
-	if (update) host->UpdateDisassembly();	// redraw in order to not show the breakpoint anymore
+	if (update)
+		Update();
+}
+
+void CBreakPoints::ChangeBreakPointAddCond(u32 addr, const BreakPointCond &cond)
+{
+	size_t bp = FindBreakpoint(addr, true, false);
+	if (bp != INVALID_BREAKPOINT)
+	{
+		breakPoints_[bp].hasCond = true;
+		breakPoints_[bp].cond = cond;
+		Update();
+	}
+}
+
+void CBreakPoints::ChangeBreakPointRemoveCond(u32 addr)
+{
+	size_t bp = FindBreakpoint(addr, true, false);
+	if (bp != INVALID_BREAKPOINT)
+	{
+		breakPoints_[bp].hasCond = false;
+		Update();
+	}
+}
+
+BreakPointCond *CBreakPoints::GetBreakPointCondition(u32 addr)
+{
+	size_t bp = FindBreakpoint(addr, true, false);
+	if (bp != INVALID_BREAKPOINT && breakPoints_[bp].hasCond)
+		return &breakPoints_[bp].cond;
+	return NULL;
+}
+
+void CBreakPoints::AddMemCheck(u32 start, u32 end, MemCheckCondition cond, MemCheckResult result)
+{
+	size_t mc = FindMemCheck(start, end);
+	if (mc == INVALID_MEMCHECK)
+	{
+		MemCheck check;
+		check.start = start;
+		check.end = end;
+		check.cond = cond;
+		check.result = result;
+
+		memChecks_.push_back(check);
+		Update();
+	}
+	else
+	{
+		memChecks_[mc].cond = (MemCheckCondition)(memChecks_[mc].cond | cond);
+		memChecks_[mc].result = (MemCheckResult)(memChecks_[mc].result | result);
+		Update();
+	}
+}
+
+void CBreakPoints::RemoveMemCheck(u32 start, u32 end)
+{
+	size_t mc = FindMemCheck(start, end);
+	if (mc != INVALID_MEMCHECK)
+	{
+		memChecks_.erase(memChecks_.begin() + mc);
+		Update();
+	}
+}
+
+void CBreakPoints::ChangeMemCheck(u32 start, u32 end, MemCheckCondition cond, MemCheckResult result)
+{
+	size_t mc = FindMemCheck(start, end);
+	if (mc != INVALID_MEMCHECK)
+	{
+		memChecks_[mc].cond = cond;
+		memChecks_[mc].result = result;
+		Update();
+	}
+}
+
+void CBreakPoints::ClearAllMemChecks()
+{
+	if (!memChecks_.empty())
+	{
+		memChecks_.clear();
+		Update();
+	}
 }
 
 MemCheck *CBreakPoints::GetMemCheck(u32 address, int size)
 {
 	std::vector<MemCheck>::iterator iter;
-	for (iter = MemChecks.begin(); iter != MemChecks.end(); ++iter)
+	for (iter = memChecks_.begin(); iter != memChecks_.end(); ++iter)
 	{
 		MemCheck &check = *iter;
-		if (check.bRange)
+		if (check.end != 0)
 		{
-			if (address >= check.iStartAddress && address + size < check.iEndAddress)
+			if (address >= check.start && address + size < check.end)
 				return &check;
 		}
 		else
 		{
-			if (check.iStartAddress==address)
+			if (check.start==address)
 				return &check;
 		}
 	}
@@ -129,46 +261,40 @@ MemCheck *CBreakPoints::GetMemCheck(u32 address, int size)
 	return 0;
 }
 
-void CBreakPoints::AddBreakPoint(u32 _iAddress, bool temp)
+void CBreakPoints::SetSkipFirst(u32 pc)
 {
-	if (!IsAddressBreakPoint(_iAddress))
+	breakSkipFirstAt_ = pc;
+	breakSkipFirstTicks_ = CoreTiming::GetTicks();
+}
+u32 CBreakPoints::CheckSkipFirst()
+{
+	u32 pc = breakSkipFirstAt_;
+	breakSkipFirstAt_ = 0;
+	if (breakSkipFirstTicks_ == CoreTiming::GetTicks())
+		return pc;
+	return 0;
+}
+
+const std::vector<MemCheck> CBreakPoints::GetMemChecks()
+{
+	return memChecks_;
+}
+
+const std::vector<BreakPoint> CBreakPoints::GetBreakpoints()
+{
+	return breakPoints_;
+}
+
+void CBreakPoints::Update(u32 addr)
+{
+	if (MIPSComp::jit && Core_IsInactive())
 	{
-		BreakPoint pt;
-		pt.bOn=true;
-		pt.bTemporary=temp;
-		pt.iAddress = _iAddress;
-
-		m_iBreakPoints.insert(pt);
-		InvalidateJit(_iAddress);
-		host->UpdateDisassembly();	// redraw in order to show the breakpoint
+		if (addr != 0)
+			MIPSComp::jit->ClearCacheAt(addr);
+		else
+			MIPSComp::jit->ClearCache();
 	}
-}
 
-void CBreakPoints::InvalidateJit(u32 _iAddress)
-{
-	// Don't want to clear cache while running, I think?
-	if (MIPSComp::jit && Core_IsInactive())
-		MIPSComp::jit->ClearCacheAt(_iAddress);
-}
-
-void CBreakPoints::InvalidateJit()
-{
-	// Don't want to clear cache while running, I think?
-	if (MIPSComp::jit && Core_IsInactive())
-		MIPSComp::jit->ClearCache();
-}
-
-int CBreakPoints::GetNumBreakpoints()
-{
-	return (int)m_iBreakPoints.size();
-}
-
-BreakPoint CBreakPoints::GetBreakpoint(int i)
-{
-	return m_iBreakPoints[i];
-}
-
-int CBreakPoints::GetBreakpointAddress(int i)
-{
-	return m_iBreakPoints[i].iAddress;
+	// Redraw in order to show the breakpoint.
+	host->UpdateDisassembly();
 }

Core/Debugger/Breakpoints.h

@@ -17,81 +17,121 @@
 
 #pragma once
 
-
 #include "../../Globals.h"
+#include <vector>
+
+// TODO: Replace with expression or something.
+struct BreakPointCond
+{
+	u32 todo;
+
+	u32 Evaluate() const
+	{
+		return 1;
+	}
+};
 
 struct BreakPoint
 {
-	u32	iAddress;
-	bool bOn;
-	bool bTemporary;
+	BreakPoint() : hasCond(false) {}
 
-	bool operator == (const BreakPoint &other) const	{
-		return iAddress == other.iAddress && bOn == other.bOn && bTemporary == other.bTemporary;
+	u32	addr;
+	bool enabled;
+	bool temporary;
+
+	bool hasCond;
+	BreakPointCond cond;
+
+	bool operator == (const BreakPoint &other) const {
+		return addr == other.addr;
 	}
+	bool operator < (const BreakPoint &other) const {
+		return addr < other.addr;
+	}
+};
+
+enum MemCheckCondition
+{
+	MEMCHECK_READ = 0x01,
+	MEMCHECK_WRITE = 0x02,
+
+	MEMCHECK_READWRITE = 0x03,
+};
+
+enum MemCheckResult
+{
+	MEMCHECK_IGNORE = 0x00,
+	MEMCHECK_LOG = 0x01,
+	MEMCHECK_BREAK = 0x02,
+
+	MEMCHECK_BOTH = 0x03,
 };
 
 struct MemCheck
 {
 	MemCheck();
-	u32 iStartAddress;
-	u32 iEndAddress;
+	u32 start;
+	u32 end;
 
-	bool	bRange;
-
-	bool	bOnRead;
-	bool	bOnWrite;
-
-	bool	bLog;
-	bool	bBreak;
+	MemCheckCondition cond;
+	MemCheckResult result;
 
 	u32 numHits;
 
 	void Action(u32 addr, bool write, int size, u32 pc);
+
+	bool operator == (const MemCheck &other) const {
+		return start == other.start && end == other.end;
+	}
 };
 
+// BreakPoints cannot overlap, only one is allowed per address.
+// MemChecks can overlap, as long as their ends are different.
+// WARNING: MemChecks are not used in the interpreter or HLE currently.
 class CBreakPoints
 {
-private:
-
-	enum { MAX_NUMBER_OF_CALLSTACK_ENTRIES = 16384};
-	enum { MAX_NUMBER_OF_BREAKPOINTS = 16};
-
-	static u32 breakSkipFirstAt_;
-
 public:
-	// WARNING: Not used in interpreter or HLE, only jit CPU memory access.
-	static std::vector<MemCheck> MemChecks;
-
-	// is address breakpoint
-	static bool IsAddressBreakPoint(u32 _iAddress);
-
-	// WARNING: Not used in interpreter or HLE, only jit CPU memory access.
-	static MemCheck *GetMemCheck(u32 address, int size);
-
-	// is break on count
-	static bool IsBreakOnCount(u32 _iAddress);
-
-	static bool IsTempBreakPoint(u32 _iAddress);
-
-	// AddBreakPoint
-	static void AddBreakPoint(u32 _iAddress, bool temp=false);
-
-	// Remove Breakpoint
-	static void RemoveBreakPoint(u32 _iAddress);
+	static const size_t INVALID_BREAKPOINT = -1;
+	static const size_t INVALID_MEMCHECK = -1;
 
+	static bool IsAddressBreakPoint(u32 addr);
+	static bool IsTempBreakPoint(u32 addr);
+	static void AddBreakPoint(u32 addr, bool temp = false);
+	static void RemoveBreakPoint(u32 addr);
+	static void ChangeBreakPoint(u32 addr, bool enable);
 	static void ClearAllBreakPoints();
 	static void ClearTemporaryBreakPoints();
 
-	static void InvalidateJit(u32 _iAddress);
-	static void InvalidateJit();
+	// Makes a copy.  Temporary breakpoints can't have conditions.
+	static void ChangeBreakPointAddCond(u32 addr, const BreakPointCond &cond);
+	static void ChangeBreakPointRemoveCond(u32 addr);
+	static BreakPointCond *GetBreakPointCondition(u32 addr);
 
-	static int GetNumBreakpoints();
-	static BreakPoint GetBreakpoint(int i);
-	static int GetBreakpointAddress(int i);
+	static void AddMemCheck(u32 start, u32 end, MemCheckCondition cond, MemCheckResult result);
+	static void RemoveMemCheck(u32 start, u32 end);
+	static void ChangeMemCheck(u32 start, u32 end, MemCheckCondition cond, MemCheckResult result);
+	static void ClearAllMemChecks();
 
-	static void SetSkipFirst(u32 pc) { breakSkipFirstAt_ = pc; }
-	static u32 CheckSkipFirst() { u32 pc = breakSkipFirstAt_; breakSkipFirstAt_ = 0; return pc; }
+	static MemCheck *GetMemCheck(u32 address, int size);
+
+	static void SetSkipFirst(u32 pc);
+	static u32 CheckSkipFirst();
+
+	static const std::vector<MemCheck> GetMemChecks();
+	static const std::vector<BreakPoint> GetBreakpoints();
+
+	static void Update(u32 addr = 0);
+
+private:
+	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 std::vector<BreakPoint> breakPoints_;
+	static u32 breakSkipFirstAt_;
+	static u64 breakSkipFirstTicks_;
+
+	static std::vector<MemCheck> memChecks_;
 };
 
 

Core/Debugger/SymbolMap.cpp

@@ -151,6 +151,13 @@ bool SymbolMap::LoadSymbolMap(const char *filename)
 		if(p == NULL)
 			break;
 		
+		// Chop any newlines off.
+		for (size_t i = strlen(line) - 1; i > 0; i--) {
+			if (line[i] == '\r' || line[i] == '\n') {
+				line[i] = '\0';
+			}
+		}
+
 		if (strlen(line) < 4 || sscanf(line, "%s", temp) != 1)
 			continue;
 
@@ -180,8 +187,7 @@ bool SymbolMap::LoadSymbolMap(const char *filename)
 			e.size=4;
 
 		//e.vaddress|=0x80000000;
-		if (strcmp(e.name,".text")==0 || strcmp(e.name,".init")==0 || strlen(e.name)<=1)
-		{ 
+		if (strcmp(e.name,".text")==0 || strcmp(e.name,".init")==0 || strlen(e.name)<=1) { 
 			;
 		} else {
 			e.UndecorateName();

Core/MIPS/x86/Jit.cpp

@@ -65,6 +65,10 @@ u32 JitBreakpoint()
 	if (CBreakPoints::CheckSkipFirst() == currentMIPS->pc)
 		return 0;
 
+	auto cond = CBreakPoints::GetBreakPointCondition(currentMIPS->pc);
+	if (cond && !cond->Evaluate())
+		return 0;
+
 	Core_EnableStepping(true);
 	host->SetDebugMode(true);
 
@@ -469,7 +473,7 @@ Jit::JitSafeMem::JitSafeMem(Jit *jit, int raddr, s32 offset)
 	: jit_(jit), raddr_(raddr), offset_(offset), needsCheck_(false), needsSkip_(false)
 {
 	// This makes it more instructions, so let's play it safe and say we need a far jump.
-	far_ = !g_Config.bIgnoreBadMemAccess || !CBreakPoints::MemChecks.empty();
+	far_ = !g_Config.bIgnoreBadMemAccess || !CBreakPoints::GetMemChecks().empty();
 	if (jit_->gpr.IsImmediate(raddr_))
 		iaddr_ = jit_->gpr.GetImmediate32(raddr_) + offset_;
 	else
@@ -695,7 +699,7 @@ void Jit::JitSafeMem::Finish()
 {
 	// Memory::Read_U32/etc. may have tripped coreState.
 	if (needsCheck_ && !g_Config.bIgnoreBadMemAccess)
-		jit_->js.afterOp = JitState::AFTER_CORE_STATE;
+		jit_->js.afterOp |= JitState::AFTER_CORE_STATE;
 	if (needsSkip_)
 		jit_->SetJumpTarget(skip_);
 	for (auto it = skipChecks_.begin(), end = skipChecks_.end(); it != end; ++it)
@@ -704,6 +708,10 @@ void Jit::JitSafeMem::Finish()
 
 void JitMemCheck(u32 addr, int size, int isWrite)
 {
+	// Should we skip this breakpoint?
+	if (CBreakPoints::CheckSkipFirst() == currentMIPS->pc)
+		return;
+
 	MemCheck *check = CBreakPoints::GetMemCheck(addr, size);
 	if (check)
 		check->Action(addr, isWrite == 1, size, currentMIPS->pc);
@@ -714,9 +722,9 @@ void Jit::JitSafeMem::MemCheckImm(ReadType type)
 	MemCheck *check = CBreakPoints::GetMemCheck(iaddr_, size_);
 	if (check)
 	{
-		if (!check->bOnRead && type == MEM_READ)
+		if (!(check->cond & MEMCHECK_READ) && type == MEM_READ)
 			return;
-		if (!check->bOnWrite && type == MEM_WRITE)
+		if (!(check->cond & MEMCHECK_WRITE) && type == MEM_WRITE)
 			return;
 
 		jit_->MOV(32, M(&jit_->mips_->pc), Imm32(jit_->js.compilerPC));
@@ -730,24 +738,25 @@ void Jit::JitSafeMem::MemCheckImm(ReadType type)
 
 void Jit::JitSafeMem::MemCheckAsm(ReadType type)
 {
-	for (auto it = CBreakPoints::MemChecks.begin(), end = CBreakPoints::MemChecks.end(); it != end; ++it)
+	auto memchecks = CBreakPoints::GetMemChecks();
+	for (auto it = memchecks.begin(), end = memchecks.end(); it != end; ++it)
 	{
-		if (!it->bOnRead && type == MEM_READ)
-			continue;
-		if (!it->bOnWrite && type == MEM_WRITE)
-			continue;
+		if (!(it->cond & MEMCHECK_READ) && type == MEM_READ)
+			return;
+		if (!(it->cond & MEMCHECK_WRITE) && type == MEM_WRITE)
+			return;
 
 		FixupBranch skipNext, skipNextRange;
-		if (it->bRange)
+		if (it->end != 0)
 		{
-			jit_->CMP(32, R(xaddr_), Imm32(it->iStartAddress - offset_));
+			jit_->CMP(32, R(xaddr_), Imm32(it->start - offset_));
 			skipNext = jit_->J_CC(CC_B);
-			jit_->CMP(32, R(xaddr_), Imm32(it->iEndAddress - offset_ - size_));
+			jit_->CMP(32, R(xaddr_), Imm32(it->end - offset_ - size_));
 			skipNextRange = jit_->J_CC(CC_AE);
 		}
 		else
 		{
-			jit_->CMP(32, R(xaddr_), Imm32(it->iStartAddress - offset_));
+			jit_->CMP(32, R(xaddr_), Imm32(it->start - offset_));
 			skipNext = jit_->J_CC(CC_NE);
 		}
 
@@ -762,7 +771,7 @@ void Jit::JitSafeMem::MemCheckAsm(ReadType type)
 		jit_->js.afterOp = JitState::AFTER_CORE_STATE | JitState::AFTER_REWIND_PC_BAD_STATE;
 
 		jit_->SetJumpTarget(skipNext);
-		if (it->bRange)
+		if (it->end != 0)
 			jit_->SetJumpTarget(skipNextRange);
 	}
 }

Windows/Debugger/Debugger_Disasm.cpp

@@ -211,12 +211,12 @@ CDisasm::~CDisasm()
 {
 }
 
-int getTotalBreakpointCount()
+int CDisasm::getTotalBreakpointCount()
 {
-	int count = (int)CBreakPoints::MemChecks.size();
-	for (int i = 0; i < CBreakPoints::GetNumBreakpoints(); i++)
+	int count = (int)CBreakPoints::GetMemChecks().size();
+	for (size_t i = 0; i < CBreakPoints::GetBreakpoints().size(); i++)
 	{
-		if (!CBreakPoints::GetBreakpoint(i).bTemporary) count++;
+		if (!displayedBreakPoints_[i].temporary) count++;
 	}
 
 	return count;
@@ -224,6 +224,10 @@ int getTotalBreakpointCount()
 
 void CDisasm::updateBreakpointList()
 {
+	// Update the items we're displaying from the debugger.
+	displayedBreakPoints_ = CBreakPoints::GetBreakpoints();
+	displayedMemChecks_= CBreakPoints::GetMemChecks();
+
 	HWND breakpointHwnd = GetDlgItem(m_hDlg, IDC_BREAKPOINTLIST);
 	int breakpointCount = getTotalBreakpointCount();
 	int items = ListView_GetItemCount(breakpointHwnd);
@@ -252,21 +256,21 @@ void CDisasm::updateBreakpointList()
 	UpdateWindow(breakpointHwnd);
 }
 
-int getBreakpointIndex(int itemIndex, bool& isMemory)
+int CDisasm::getBreakpointIndex(int itemIndex, bool& isMemory)
 {
 	// memory breakpoints first
-	if (itemIndex < CBreakPoints::MemChecks.size())
+	if (itemIndex < (int)displayedMemChecks_.size())
 	{
 		isMemory = true;
 		return itemIndex;
 	}
 
-	itemIndex -= (int)CBreakPoints::MemChecks.size();
+	itemIndex -= (int)displayedMemChecks_.size();
 
-	int i = 0;
-	while (i < CBreakPoints::GetNumBreakpoints())
+	size_t i = 0;
+	while (i < displayedBreakPoints_.size())
 	{
-		if (CBreakPoints::GetBreakpoint(i).bTemporary)
+		if (displayedBreakPoints_[i].temporary)
 		{
 			i++;
 			continue;
@@ -276,7 +280,7 @@ int getBreakpointIndex(int itemIndex, bool& isMemory)
 		if (itemIndex == 0)
 		{
 			isMemory = false;
-			return i;
+			return (int)i;
 		}
 
 		i++;
@@ -294,11 +298,10 @@ void CDisasm::removeBreakpoint(int itemIndex)
 
 	if (isMemory)
 	{
-		CBreakPoints::MemChecks.erase(CBreakPoints::MemChecks.begin()+index);
-		CBreakPoints::InvalidateJit();
-		updateBreakpointList();
+		auto mc = displayedMemChecks_[index];
+		CBreakPoints::RemoveMemCheck(mc.start, mc.end);
 	} else {
-		u32 address = CBreakPoints::GetBreakpointAddress(index);
+		u32 address = displayedBreakPoints_[index].addr;
 		CBreakPoints::RemoveBreakPoint(address);
 	}
 }
@@ -311,13 +314,13 @@ void CDisasm::gotoBreakpointAddress(int itemIndex)
 
 	if (isMemory)
 	{
-		u32 address = CBreakPoints::MemChecks[index].iStartAddress;
+		u32 address = displayedMemChecks_[index].start;
 			
 		for (int i=0; i<numCPUs; i++)
 			if (memoryWindow[i])
 				memoryWindow[i]->Goto(address);
 	} else {
-		u32 address = CBreakPoints::GetBreakpointAddress(index);
+		u32 address = displayedBreakPoints_[index].addr;
 		Goto(address);
 		SetFocus(GetDlgItem(m_hDlg, IDC_DISASMVIEW));
 	}
@@ -337,7 +340,6 @@ void CDisasm::handleBreakpointNotify(LPARAM lParam)
 	if (((LPNMHDR)lParam)->code == LVN_GETDISPINFO)
 	{
 		NMLVDISPINFO* dispInfo = (NMLVDISPINFO*)lParam;
-		std::vector<MemCheck>& mem = CBreakPoints::MemChecks;
 		
 		bool isMemory;
 		int index = getBreakpointIndex(dispInfo->item.iItem,isMemory);
@@ -350,12 +352,18 @@ void CDisasm::handleBreakpointNotify(LPARAM lParam)
 			{
 				if (isMemory)
 				{
-					if (mem[index].bOnRead)
+					switch (displayedMemChecks_[index].cond)
 					{
-						strcpy(breakpointText,"Read");
-						if (mem[index].bOnWrite) strcat(breakpointText,"/");
+					case MEMCHECK_READ:
+						strcpy(breakpointText, "Read");
+						break;
+					case MEMCHECK_WRITE:
+						strcpy(breakpointText, "Write");
+						break;
+					case MEMCHECK_READWRITE:
+						strcpy(breakpointText, "Read/Write");
+						break;
 					}
-					if (mem[index].bOnWrite) strcat(breakpointText,"Write");
 				} else {
 					strcpy(breakpointText,"Execute");
 				}
@@ -365,9 +373,9 @@ void CDisasm::handleBreakpointNotify(LPARAM lParam)
 			{
 				if (isMemory)
 				{
-					sprintf(breakpointText,"0x%08X",mem[index].iStartAddress);
+					sprintf(breakpointText,"0x%08X",displayedMemChecks_[index].start);
 				} else {
-					sprintf(breakpointText,"0x%08X",CBreakPoints::GetBreakpointAddress(index));
+					sprintf(breakpointText,"0x%08X",displayedBreakPoints_[index].addr);
 				}
 			}
 			break;
@@ -375,10 +383,11 @@ void CDisasm::handleBreakpointNotify(LPARAM lParam)
 			{
 				if (isMemory)
 				{
-					if (mem[index].bRange == false) sprintf(breakpointText,"0x%08X",1);
-					else sprintf(breakpointText,"0x%08X",mem[index].iEndAddress-mem[index].iStartAddress);
+					auto mc = displayedMemChecks_[index];
+					if (mc.end == 0) sprintf(breakpointText,"0x%08X",1);
+					else sprintf(breakpointText,"0x%08X",mc.end-mc.start);
 				} else {
-					const char* sym = cpu->findSymbolForAddress(CBreakPoints::GetBreakpointAddress(index));
+					const char* sym = cpu->findSymbolForAddress(displayedBreakPoints_[index].addr);
 					if (sym != NULL)
 					{
 						strcpy(breakpointText,sym);
@@ -395,7 +404,7 @@ void CDisasm::handleBreakpointNotify(LPARAM lParam)
 					strcpy(breakpointText,"-");
 				} else {
 					CtrlDisAsmView *ptr = CtrlDisAsmView::getFrom(GetDlgItem(m_hDlg,IDC_DISASMVIEW));
-					ptr->getOpcodeText(CBreakPoints::GetBreakpointAddress(index),breakpointText);
+					ptr->getOpcodeText(displayedBreakPoints_[index].addr,breakpointText);
 				}
 			}
 			break;
@@ -403,7 +412,7 @@ void CDisasm::handleBreakpointNotify(LPARAM lParam)
 			{
 				if (isMemory)
 				{
-					sprintf(breakpointText,"%d",mem[index].numHits);
+					sprintf(breakpointText,"%d",displayedMemChecks_[index].numHits);
 				} else {
 					strcpy(breakpointText,"-");
 				}
@@ -413,9 +422,9 @@ void CDisasm::handleBreakpointNotify(LPARAM lParam)
 			{
 				if (isMemory)
 				{
-					strcpy(breakpointText,mem[index].bBreak == true ? "True" : "False");
+					strcpy(breakpointText,displayedMemChecks_[index].result & MEMCHECK_BREAK ? "True" : "False");
 				} else {
-					strcpy(breakpointText,CBreakPoints::GetBreakpoint(index).bOn ? "True" : "False");
+					strcpy(breakpointText,displayedBreakPoints_[index].enabled ? "True" : "False");
 				}
 			}
 			break;
@@ -647,19 +656,9 @@ BOOL CDisasm::DlgProc(UINT message, WPARAM wParam, LPARAM lParam)
 						Core_WaitInactive(200);
 					}
 
-					MemCheck check;
-
-					if (executeExpressionWindow(m_hDlg,cpu,check.iStartAddress))
-					{
-						check.bBreak = true;
-						check.bLog = true;
-						check.bOnRead = true;
-						check.bOnWrite = true;
-						check.bRange = false;
-						CBreakPoints::MemChecks.push_back(check);
-						CBreakPoints::InvalidateJit();
-						updateBreakpointList();
-					}
+					u32 start;
+					if (executeExpressionWindow(m_hDlg,cpu,start))
+						CBreakPoints::AddMemCheck(start, 0, MEMCHECK_READWRITE, MEMCHECK_BOTH);
 
 					if (isRunning)
 					{

Windows/Debugger/Debugger_Disasm.h

@@ -5,10 +5,12 @@
 
 #include "../W32Util/DialogManager.h"
 #include "CtrlDisasmView.h"
-#include "../../Core/MIPS/MIPSDebugInterface.h"
 #include "CPURegsInterface.h"
-#include "../../Globals.h"
-#include "../../Core/CPU.h"
+#include "Globals.h"
+#include "Core/CPU.h"
+#include "Core/MIPS/MIPSDebugInterface.h"
+#include "Core/Debugger/Breakpoints.h"
+#include <vector>
 
 #include <windows.h>
 
@@ -27,6 +29,9 @@ private:
 	DebugInterface *cpu;
 	u64 lastTicks;
 
+	std::vector<BreakPoint> displayedBreakPoints_;
+	std::vector<MemCheck> displayedMemChecks_;
+
 	BOOL DlgProc(UINT message, WPARAM wParam, LPARAM lParam);
 	void UpdateSize(WORD width, WORD height);
 	void SavePosition();
@@ -34,6 +39,8 @@ private:
 	void handleBreakpointNotify(LPARAM lParam);
 	void gotoBreakpointAddress(int itemIndex);
 	void removeBreakpoint(int itemIndex);
+	int getTotalBreakpointCount();
+	int getBreakpointIndex(int itemIndex, bool& isMemory);
 public:
 	int index; //helper