From 60e0bd4e0191b7f6d2b7b3dc3dc1c8e27c004b2f Mon Sep 17 00:00:00 2001 From: Sour Date: Mon, 24 Dec 2018 21:22:08 -0500 Subject: [PATCH] Debugger: Fixed issues & improved performance with new breakpoint logic --- Core/BaseMapper.cpp | 23 ++++++ Core/BaseMapper.h | 2 + Core/Breakpoint.cpp | 18 ++-- Core/Breakpoint.h | 2 +- Core/CPU.cpp | 41 ++++------ Core/CPU.h | 4 +- Core/Debugger.cpp | 113 ++++++++++++++------------ Core/Debugger.h | 5 +- Core/DebuggerTypes.h | 2 + GUI.NET/Debugger/frmDebugger.cs | 8 +- GUI.NET/Dependencies/resources.en.xml | 2 + GUI.NET/InteropEmu.cs | 2 + PGOHelper/PGOHelper.cpp | 2 + 13 files changed, 128 insertions(+), 96 deletions(-) diff --git a/Core/BaseMapper.cpp b/Core/BaseMapper.cpp index 9cf6c318..91e563b4 100644 --- a/Core/BaseMapper.cpp +++ b/Core/BaseMapper.cpp @@ -1058,6 +1058,29 @@ void BaseMapper::SetMemoryValue(DebugMemoryType memoryType, uint32_t address, ui } } +void BaseMapper::GetAbsoluteAddressAndType(uint32_t relativeAddr, AddressTypeInfo* info) +{ + if(relativeAddr < 0x2000) { + info->Address = relativeAddr; + info->Type = AddressType::InternalRam; + } else { + uint8_t *prgAddr = _prgPages[relativeAddr >> 8] + (uint8_t)relativeAddr; + if(prgAddr >= _prgRom && prgAddr < _prgRom + _prgSize) { + info->Address = (uint32_t)(prgAddr - _prgRom); + info->Type = AddressType::PrgRom; + } else if(prgAddr >= _workRam && prgAddr < _workRam + _workRamSize) { + info->Address = (uint32_t)(prgAddr - _workRam); + info->Type = AddressType::WorkRam; + } else if(prgAddr >= _saveRam && prgAddr < _saveRam + _saveRamSize) { + info->Address = (uint32_t)(prgAddr - _saveRam); + info->Type = AddressType::SaveRam; + } else { + info->Address = -1; + info->Type = AddressType::InternalRam; + } + } +} + int32_t BaseMapper::ToAbsoluteAddress(uint16_t addr) { uint8_t *prgAddr = _prgPages[addr >> 8] + (uint8_t)addr; diff --git a/Core/BaseMapper.h b/Core/BaseMapper.h index b452d783..da77e69d 100644 --- a/Core/BaseMapper.h +++ b/Core/BaseMapper.h @@ -215,6 +215,8 @@ public: uint32_t CopyMemory(DebugMemoryType type, uint8_t* buffer); void WriteMemory(DebugMemoryType type, uint8_t* buffer); + + void GetAbsoluteAddressAndType(uint32_t relativeAddr, AddressTypeInfo *info); int32_t ToAbsoluteAddress(uint16_t addr); int32_t ToAbsoluteSaveRamAddress(uint16_t addr); int32_t ToAbsoluteWorkRamAddress(uint16_t addr); diff --git a/Core/Breakpoint.cpp b/Core/Breakpoint.cpp index 240bc05f..51bd7117 100644 --- a/Core/Breakpoint.cpp +++ b/Core/Breakpoint.cpp @@ -10,12 +10,8 @@ Breakpoint::~Breakpoint() { } -bool Breakpoint::Matches(uint32_t memoryAddr, AddressTypeInfo &info, MemoryOperationType opType) +bool Breakpoint::Matches(uint32_t memoryAddr, AddressTypeInfo &info) { - if(!_processDummyReadWrites && (opType == MemoryOperationType::DummyRead || opType == MemoryOperationType::DummyWrite)) { - return false; - } - if(_startAddr == -1) { return true; } @@ -72,11 +68,13 @@ bool Breakpoint::HasBreakpointType(BreakpointType type) { switch(type) { case BreakpointType::Global: return (_type == BreakpointTypeFlags::Global); - case BreakpointType::Execute: return (_type & BreakpointTypeFlags::Execute) == BreakpointTypeFlags::Execute; - case BreakpointType::ReadRam: return (_type & BreakpointTypeFlags::ReadRam) == BreakpointTypeFlags::ReadRam; - case BreakpointType::WriteRam: return (_type & BreakpointTypeFlags::WriteRam) == BreakpointTypeFlags::WriteRam; - case BreakpointType::ReadVram: return (_type & BreakpointTypeFlags::ReadVram) == BreakpointTypeFlags::ReadVram; - case BreakpointType::WriteVram: return (_type & BreakpointTypeFlags::WriteVram) == BreakpointTypeFlags::WriteVram; + case BreakpointType::Execute: return (_type & BreakpointTypeFlags::Execute) != 0; + case BreakpointType::ReadRam: return (_type & BreakpointTypeFlags::ReadRam) != 0; + case BreakpointType::WriteRam: return (_type & BreakpointTypeFlags::WriteRam) != 0; + case BreakpointType::ReadVram: return (_type & BreakpointTypeFlags::ReadVram) != 0; + case BreakpointType::WriteVram: return (_type & BreakpointTypeFlags::WriteVram) != 0; + case BreakpointType::DummyReadRam: return (_type & BreakpointTypeFlags::ReadRam) != 0 && _processDummyReadWrites; + case BreakpointType::DummyWriteRam: return (_type & BreakpointTypeFlags::WriteRam) != 0 && _processDummyReadWrites; } return false; } diff --git a/Core/Breakpoint.h b/Core/Breakpoint.h index 14c6b69b..a5a69403 100644 --- a/Core/Breakpoint.h +++ b/Core/Breakpoint.h @@ -19,7 +19,7 @@ public: Breakpoint(); ~Breakpoint(); - bool Matches(uint32_t memoryAddr, AddressTypeInfo &info, MemoryOperationType opType); + bool Matches(uint32_t memoryAddr, AddressTypeInfo &info); bool Matches(uint32_t memoryAddr, PpuAddressTypeInfo &info); bool HasBreakpointType(BreakpointType type); string GetCondition(); diff --git a/Core/CPU.cpp b/Core/CPU.cpp index 5db653ff..6b6b2db8 100644 --- a/Core/CPU.cpp +++ b/Core/CPU.cpp @@ -186,13 +186,6 @@ void CPU::BRK() { void CPU::MemoryWrite(uint16_t addr, uint8_t value, MemoryOperationType operationType) { - _cpuWrite = true;; - _writeAddr = addr; - IncCycleCount(); - while(_dmcDmaRunning) { - IncCycleCount(); - } - #ifdef DUMMYCPU if(operationType == MemoryOperationType::Write || operationType == MemoryOperationType::DummyWrite) { _writeAddresses[_writeCounter] = addr; @@ -201,17 +194,34 @@ void CPU::MemoryWrite(uint16_t addr, uint8_t value, MemoryOperationType operatio _writeCounter++; } #else + _cpuWrite = true;; + _writeAddr = addr; + IncCycleCount(); + while(_dmcDmaRunning) { + IncCycleCount(); + } + _memoryManager->Write(addr, value, operationType); -#endif //DMA DMC might have started after a write to $4015, stall CPU if needed while(_dmcDmaRunning) { IncCycleCount(); } _cpuWrite = false; +#endif } uint8_t CPU::MemoryRead(uint16_t addr, MemoryOperationType operationType) { +#ifdef DUMMYCPU + uint8_t value = _memoryManager->DebugRead(addr); + if(operationType == MemoryOperationType::Read || operationType == MemoryOperationType::DummyRead) { + _readAddresses[_readCounter] = addr; + _readValue[_readCounter] = value; + _isDummyRead[_readCounter] = operationType == MemoryOperationType::DummyRead; + _readCounter++; + } + return value; +#else IncCycleCount(); while(_dmcDmaRunning) { //Stall CPU until we can process a DMC read @@ -220,24 +230,11 @@ uint8_t CPU::MemoryRead(uint16_t addr, MemoryOperationType operationType) { //Reads are only performed every other cycle? This fixes "dma_2007_read" test //This behavior causes the $4016/7 data corruption when a DMC is running. //When reading $4016/7, only the last read counts (because this only occurs to low-to-high transitions, i.e once in this case) - #ifdef DUMMYCPU - _memoryManager->DebugRead(addr); - #else _memoryManager->Read(addr); - #endif } IncCycleCount(); } -#ifdef DUMMYCPU - if(operationType == MemoryOperationType::Read || operationType == MemoryOperationType::DummyRead) { - _readAddresses[_readCounter] = addr; - _isDummyRead[_readCounter] = operationType == MemoryOperationType::DummyRead; - _readCounter++; - } - - return _memoryManager->DebugRead(addr); -#else if(operationType == MemoryOperationType::ExecOpCode) { _state.DebugPC = _state.PC; } @@ -310,9 +307,7 @@ void CPU::IncCycleCount() } } - #ifndef DUMMYCPU _console->ProcessCpuClock(); - #endif if(!_spriteDmaTransfer && !_dmcDmaRunning) { //IRQ flags are ignored during Sprite DMA - fixes irq_and_dma diff --git a/Core/CPU.h b/Core/CPU.h index b6aa3f38..f4ebac8f 100644 --- a/Core/CPU.h +++ b/Core/CPU.h @@ -63,6 +63,7 @@ private: uint32_t _readCounter = 0; uint16_t _readAddresses[10]; + uint8_t _readValue[10]; bool _isDummyRead[10]; #endif @@ -863,9 +864,10 @@ public: isDummyWrite = _isDummyWrite[index]; } - void GetReadAddr(uint32_t index, uint16_t &addr, bool &isDummyRead) + void GetReadAddr(uint32_t index, uint16_t &addr, uint8_t &value, bool &isDummyRead) { addr = _readAddresses[index]; + value = _readValue[index]; isDummyRead = _isDummyRead[index]; } #endif diff --git a/Core/Debugger.cpp b/Core/Debugger.cpp index 87b5f8ab..8199a8d7 100644 --- a/Core/Debugger.cpp +++ b/Core/Debugger.cpp @@ -283,6 +283,8 @@ void Debugger::SetBreakpoints(Breakpoint breakpoints[], uint32_t length) _hasBreakpoint[i] = false; } + _bpDummyCpuRequired = false; + _bpExpEval.reset(new ExpressionEvaluator(this)); for(uint32_t j = 0; j < length; j++) { Breakpoint &bp = breakpoints[j]; @@ -297,6 +299,11 @@ void Debugger::SetBreakpoints(Breakpoint breakpoints[], uint32_t length) _breakpointRpnList[i].push_back(success ? data : ExpressionData()); } + if(bp.IsEnabled()) { + bool isReadWriteBp = i == BreakpointType::ReadVram || i == BreakpointType::ReadRam || i == BreakpointType::WriteVram || i == BreakpointType::WriteRam || i == BreakpointType::DummyReadRam || i == BreakpointType::DummyWriteRam; + _bpDummyCpuRequired |= isReadWriteBp; + } + _hasBreakpoint[i] = true; } } @@ -320,6 +327,8 @@ bool Debugger::ProcessBreakpoints(BreakpointType type, OperationInfo &operationI case BreakpointType::Execute: case BreakpointType::ReadRam: case BreakpointType::WriteRam: + case BreakpointType::DummyReadRam: + case BreakpointType::DummyWriteRam: GetAbsoluteAddressAndType(operationInfo.Address, &info); break; @@ -359,7 +368,7 @@ bool Debugger::ProcessBreakpoints(BreakpointType type, OperationInfo &operationI if( type == BreakpointType::Global || - (!isPpuBreakpoint && breakpoint.Matches(operationInfo.Address, info, operationInfo.OperationType)) || + (!isPpuBreakpoint && breakpoint.Matches(operationInfo.Address, info)) || (isPpuBreakpoint && breakpoint.Matches(operationInfo.Address, ppuInfo)) ) { if(!breakpoint.HasCondition()) { @@ -401,6 +410,13 @@ void Debugger::ProcessAllBreakpoints(OperationInfo &operationInfo, AddressTypeIn ProcessBreakpoints(BreakpointType::Execute, operationInfo, true, true); } + bool checkUninitReads = _enableBreakOnUninitRead && CheckFlag(DebuggerFlags::BreakOnUninitMemoryRead); + + if(!checkUninitReads && !_bpDummyCpuRequired) { + //Nothing to do, no read/write breakpoints are active and don't need to check uninit reads + return; + } + _dummyCpu->SetDummyState(_cpu.get()); _dummyCpu->Exec(); @@ -408,9 +424,10 @@ void Debugger::ProcessAllBreakpoints(OperationInfo &operationInfo, AddressTypeIn uint32_t readCount = _dummyCpu->GetReadCount(); if(readCount > 0) { uint16_t addr; + uint8_t value; bool isDummyRead; for(uint32_t i = 0; i < readCount; i++) { - _dummyCpu->GetReadAddr(i, addr, isDummyRead); + _dummyCpu->GetReadAddr(i, addr, value, isDummyRead); OperationInfo info; @@ -433,7 +450,7 @@ void Debugger::ProcessAllBreakpoints(OperationInfo &operationInfo, AddressTypeIn info.Value = state.PPU.MemoryReadBuffer; } else { - if(_enableBreakOnUninitRead && CheckFlag(DebuggerFlags::BreakOnUninitMemoryRead)) { + if(!isDummyRead && checkUninitReads) { //Break on uninit memory read if(_memoryAccessCounter->IsAddressUninitialized(addressInfo)) { Step(1); @@ -442,34 +459,52 @@ void Debugger::ProcessAllBreakpoints(OperationInfo &operationInfo, AddressTypeIn } } - info.Value = _memoryManager->DebugRead(addr); + info.Value = value; } - if(_hasBreakpoint[BreakpointType::ReadRam]) { - info.Address = addr; - info.OperationType = isDummyRead ? MemoryOperationType::DummyRead : MemoryOperationType::Read; - if(ProcessBreakpoints(BreakpointType::ReadRam, info, true, false)) { - return; + info.Address = addr; + if(isDummyRead) { + if(_hasBreakpoint[BreakpointType::DummyReadRam]) { + info.OperationType = MemoryOperationType::DummyRead; + if(ProcessBreakpoints(BreakpointType::DummyReadRam, info, true, false)) { + return; + } + } + } else { + if(_hasBreakpoint[BreakpointType::ReadRam]) { + info.OperationType = MemoryOperationType::Read; + if(ProcessBreakpoints(BreakpointType::ReadRam, info, true, false)) { + return; + } } } } } uint32_t writeCount = _dummyCpu->GetWriteCount(); - if(writeCount > 0 && (_hasBreakpoint[BreakpointType::WriteRam] || _hasBreakpoint[BreakpointType::WriteVram])) { + if(writeCount > 0) { uint16_t addr; uint8_t value; bool isDummyWrite; for(uint32_t i = 0; i < writeCount; i++) { _dummyCpu->GetWriteAddrValue(i, addr, value, isDummyWrite); - if(_hasBreakpoint[BreakpointType::WriteRam]) { - OperationInfo info; - info.Address = addr; - info.Value = value; - info.OperationType = isDummyWrite ? MemoryOperationType::DummyWrite : MemoryOperationType::Write; - if(ProcessBreakpoints(BreakpointType::WriteRam, info, true, false)) { - return; + OperationInfo info; + info.Address = addr; + info.Value = value; + if(isDummyWrite) { + if(_hasBreakpoint[BreakpointType::DummyWriteRam]) { + info.OperationType = MemoryOperationType::DummyWrite; + if(ProcessBreakpoints(BreakpointType::DummyWriteRam, info, true, false)) { + return; + } + } + } else { + if(_hasBreakpoint[BreakpointType::WriteRam]) { + info.OperationType = MemoryOperationType::Write; + if(ProcessBreakpoints(BreakpointType::WriteRam, info, true, false)) { + return; + } } } @@ -801,10 +836,10 @@ bool Debugger::ProcessRamOperation(MemoryOperationType type, uint16_t &addr, uin if(_breakOnFirstCycle) { if(type == MemoryOperationType::ExecOpCode && !breakDone) { ProcessAllBreakpoints(operationInfo, addressInfo); + } else if(_hasBreakpoint[breakpointType]) { + //Process marked breakpoints + ProcessBreakpoints(breakpointType, operationInfo, false, true); } - - //Process marked breakpoints - ProcessBreakpoints(breakpointType, operationInfo, false, true); } else { if(_hasBreakpoint[breakpointType]) { ProcessBreakpoints(breakpointType, operationInfo, !breakDone, true); @@ -907,9 +942,9 @@ void Debugger::ProcessVramReadOperation(MemoryOperationType type, uint16_t addr, int32_t absoluteAddr = _mapper->ToAbsoluteChrAddress(addr); _codeDataLogger->SetFlag(absoluteAddr, type == MemoryOperationType::Read ? CdlChrFlags::Read : CdlChrFlags::Drawn); - if(!_breakOnFirstCycle && _hasBreakpoint[BreakpointType::ReadVram]) { + if(_hasBreakpoint[BreakpointType::ReadVram]) { OperationInfo operationInfo{ addr, value, type }; - ProcessBreakpoints(BreakpointType::ReadVram, operationInfo); + ProcessBreakpoints(BreakpointType::ReadVram, operationInfo, !_breakOnFirstCycle, true); } ProcessPpuOperation(addr, value, MemoryOperationType::Read); @@ -917,9 +952,9 @@ void Debugger::ProcessVramReadOperation(MemoryOperationType type, uint16_t addr, void Debugger::ProcessVramWriteOperation(uint16_t addr, uint8_t &value) { - if(!_breakOnFirstCycle && _hasBreakpoint[BreakpointType::WriteVram]) { + if(_hasBreakpoint[BreakpointType::WriteVram]) { OperationInfo operationInfo{ addr, value, MemoryOperationType::Write }; - ProcessBreakpoints(BreakpointType::WriteVram, operationInfo); + ProcessBreakpoints(BreakpointType::WriteVram, operationInfo, !_breakOnFirstCycle, true); } ProcessPpuOperation(addr, value, MemoryOperationType::Write); @@ -1231,35 +1266,7 @@ void Debugger::AllowResume() void Debugger::GetAbsoluteAddressAndType(uint32_t relativeAddr, AddressTypeInfo* info) { - if(relativeAddr < 0x2000) { - info->Address = relativeAddr; - info->Type = AddressType::InternalRam; - return; - } - - int32_t addr = _mapper->ToAbsoluteAddress(relativeAddr); - if(addr >= 0) { - info->Address = addr; - info->Type = AddressType::PrgRom; - return; - } - - addr = _mapper->ToAbsoluteWorkRamAddress(relativeAddr); - if(addr >= 0) { - info->Address = addr; - info->Type = AddressType::WorkRam; - return; - } - - addr = _mapper->ToAbsoluteSaveRamAddress(relativeAddr); - if(addr >= 0) { - info->Address = addr; - info->Type = AddressType::SaveRam; - return; - } - - info->Address = -1; - info->Type = AddressType::InternalRam; + return _mapper->GetAbsoluteAddressAndType(relativeAddr, info); } void Debugger::GetPpuAbsoluteAddressAndType(uint32_t relativeAddr, PpuAddressTypeInfo* info) diff --git a/Core/Debugger.h b/Core/Debugger.h index 8d519cdf..4ea3d41b 100644 --- a/Core/Debugger.h +++ b/Core/Debugger.h @@ -38,7 +38,7 @@ enum class CdlStripFlag; class Debugger { private: - static constexpr int BreakpointTypeCount = 6; + static constexpr int BreakpointTypeCount = 8; //Must be static to be thread-safe when switching game static string _disassemblerOutput; @@ -61,6 +61,7 @@ private: shared_ptr _mapper; shared_ptr _dummyCpu; + bool _bpDummyCpuRequired; bool _breakOnFirstCycle; bool _hasScript; @@ -168,6 +169,8 @@ public: bool CheckFlag(DebuggerFlags flag); void SetBreakpoints(Breakpoint breakpoints[], uint32_t length); + + void ProcessMarkedBreakpoints(BreakpointType type, OperationInfo &operationInfo); shared_ptr GetLabelManager(); diff --git a/Core/DebuggerTypes.h b/Core/DebuggerTypes.h index d3caaf1e..262ed973 100644 --- a/Core/DebuggerTypes.h +++ b/Core/DebuggerTypes.h @@ -10,6 +10,8 @@ enum BreakpointType WriteRam = 3, ReadVram = 4, WriteVram = 5, + DummyReadRam = 6, + DummyWriteRam = 7 }; enum class DebuggerFlags diff --git a/GUI.NET/Debugger/frmDebugger.cs b/GUI.NET/Debugger/frmDebugger.cs index e1ee38c0..ffc66fe0 100644 --- a/GUI.NET/Debugger/frmDebugger.cs +++ b/GUI.NET/Debugger/frmDebugger.cs @@ -507,13 +507,7 @@ namespace Mesen.GUI.Debugger if(breakpointId >= 0 && breakpointId < breakpoints.Count) { Breakpoint bp = breakpoints[breakpointId]; if(bpType != BreakpointType.Global) { - string prefix = ""; - if(bpType == BreakpointType.ReadRam || bpType == BreakpointType.WriteRam) { - if(memOpType == InteropMemoryOperationType.DummyRead || memOpType == InteropMemoryOperationType.DummyWrite) { - prefix = "(Dummy) "; - } - } - message += ": " + prefix + ResourceHelper.GetEnumText(bpType) + " ($" + bpAddress.ToString("X4") + ")"; + message += ": " + ResourceHelper.GetEnumText(bpType) + " ($" + bpAddress.ToString("X4") + ")"; } if(!string.IsNullOrWhiteSpace(bp.Condition)) { string cond = bp.Condition.Trim(); diff --git a/GUI.NET/Dependencies/resources.en.xml b/GUI.NET/Dependencies/resources.en.xml index 3673e77e..446f571d 100644 --- a/GUI.NET/Dependencies/resources.en.xml +++ b/GUI.NET/Dependencies/resources.en.xml @@ -1194,6 +1194,8 @@ CPU Write PPU Read PPU Write + (Dummy) CPU Read + (Dummy) CPU Write Breakpoint diff --git a/GUI.NET/InteropEmu.cs b/GUI.NET/InteropEmu.cs index 049936b3..d1980faa 100644 --- a/GUI.NET/InteropEmu.cs +++ b/GUI.NET/InteropEmu.cs @@ -2113,6 +2113,8 @@ namespace Mesen.GUI WriteRam = 3, ReadVram = 4, WriteVram = 5, + DummyReadRam = 6, + DummyWriteRam = 7 } [Flags] diff --git a/PGOHelper/PGOHelper.cpp b/PGOHelper/PGOHelper.cpp index 89f5db16..1fa9f8af 100644 --- a/PGOHelper/PGOHelper.cpp +++ b/PGOHelper/PGOHelper.cpp @@ -56,6 +56,7 @@ extern "C" { void __stdcall Release(); void __stdcall Stop(); void __stdcall DebugInitialize(); + void __stdcall DebugSetFlags(uint32_t flags); } vector GetFilesInFolder(string rootFolder, std::unordered_set extensions) @@ -104,6 +105,7 @@ int main(int argc, char* argv[]) SetVideoFilter(filterTypes[i % 13]); LoadROM(testRoms[i].c_str(), ""); DebugInitialize(); + DebugSetFlags(0x10000 /*DebuggerFlags::BreakOnFirstCycle*/); } std::this_thread::sleep_for(std::chrono::duration(5000)); Stop();