Fairly large refactoring of RWP functionality.

- abstracted RWP code into Cart base class (only F8SC converted for now)
- RWP now works by analysing RAM accesses before and after each instruction, catching all occurrences

Fixes for compiling without debugger support.
This commit is contained in:
Stephen Anthony 2018-12-17 19:51:28 -03:30
parent 8cb235bf19
commit ebe18877f9
11 changed files with 170 additions and 117 deletions

View File

@ -132,10 +132,6 @@ CartDebug::CartDebug(Debugger& dbg, Console& console, const OSystem& osystem)
DiStella::settings.rFlag = myOSystem.settings().getBool("dis.relocate");
DiStella::settings.bFlag = true; // Not currently configurable
DiStella::settings.bytesWidth = 8+1; // TODO - configure based on window size
setReadFromWritePortBreak(myOSystem.settings().getBool(
myOSystem.settings().getBool("dev.settings") ?
"dev.rwportbreak" : "plr.rwportbreak"));
}
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

View File

@ -33,7 +33,20 @@ Cartridge::Cartridge(const Settings& settings)
myStartBank(0),
myBankLocked(false)
{
std::fill(myRWPRandomValues, myRWPRandomValues + 256, 0);
// TODO - get md5 from parameter
string md5 = "d30b89fcc488b7ac2fcbd96c06cb932e";
auto to_uInt32 = [](const string& s, uInt32 pos) {
return uInt32(std::stoul(s.substr(pos, 8), nullptr, 16));
};
uInt32 seed = to_uInt32(md5, 0) ^ to_uInt32(md5, 8) ^
to_uInt32(md5, 16) ^ to_uInt32(md5, 24);
Random rand(seed);
for(uInt32 i = 0; i < 256; ++i)
myRWPRandomValues[i] = rand.next();
myRAMAccesses.reserve(5);
}
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@ -70,6 +83,43 @@ bool Cartridge::bankChanged()
return changed;
}
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
uInt8 Cartridge::peekRAM(uInt8& dest, uInt16 address)
{
uInt8 value = myRWPRandomValues[address & 0xFF];
// Reading from the write port triggers an unwanted write
// But this only happens when in normal emulation mode
#ifdef DEBUGGER_SUPPORT
if(!bankLocked() && !mySystem->autodetectMode())
{
// Record access here; final determination will happen in ::pokeRAM()
myRAMAccesses.push_back(address);
dest = value;
}
#else
if(!mySystem->autodetectMode())
dest = value;
#endif
return value;
}
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
void Cartridge::pokeRAM(uInt8& dest, uInt16 address, uInt8 value)
{
#ifdef DEBUGGER_SUPPORT
for(auto i = myRAMAccesses.begin(); i != myRAMAccesses.end(); ++i)
{
if(*i == address)
{
myRAMAccesses.erase(i);
break;
}
}
#endif
dest = value;
}
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
void Cartridge::triggerReadFromWritePort(uInt16 address)
{
@ -79,20 +129,6 @@ void Cartridge::triggerReadFromWritePort(uInt16 address)
#endif
}
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
void Cartridge::createReadFromWritePortValues(const uInt8* image, uInt32 size)
{
Random rand(MD5::hashToInt(image, size));
for(uInt32 i = 0; i < 256; ++i)
myRWPRandomValues[i] = rand.next();
}
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
uInt8 Cartridge::randomReadFromWritePortValue(uInt16 address) const
{
return myRWPRandomValues[address & 0xFF];
}
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
void Cartridge::createCodeAccessBase(uInt32 size)
{

View File

@ -103,6 +103,25 @@ class Cartridge : public Device
*/
virtual bool bankChanged();
#ifdef DEBUGGER_SUPPORT
/**
To be called at the start of each instruction.
Clears information about all accesses to cart RAM.
*/
void clearAllRAMAccesses() { myRAMAccesses.clear(); }
/**
To be called at the end of each instruction.
Answers whether an access in the last instruction cycle generated
an illegal RAM access.
@return Address of illegal access if one occurred, else 0
*/
uInt16 getIllegalRAMAccess() const {
return myRAMAccesses.size() > 0 ? myRAMAccesses[0] : 0;
}
#endif
public:
//////////////////////////////////////////////////////////////////////
// The following methods are cart-specific and will usually be
@ -188,6 +207,30 @@ class Cartridge : public Device
const GUI::Font& nfont, int x, int y, int w, int h) { return nullptr; }
protected:
/**
Get a random value to use when a read from the write port happens.
Sometimes a RWP means that RAM should be overwritten, sometimes not.
Internally, this method also keeps track of illegal accesses.
@param dest The location to place the value, when an overwrite should happen
@param address The address of the illegal read
@return The value read, whether it is overwritten or not
*/
uInt8 peekRAM(uInt8& dest, uInt16 address);
/**
Use the given value when writing to RAM.
Internally, this method also keeps track of legal accesses, and removes
them from the illegal list.
@param dest The final location (including address) to place the value
@param address The address of the legal write
@param value The value to write to the given address
*/
void pokeRAM(uInt8& dest, uInt16 address, uInt8 value);
/**
Indicate that an illegal read from a write port has occurred.
@ -195,28 +238,6 @@ class Cartridge : public Device
*/
void triggerReadFromWritePort(uInt16 address);
/**
Creates an array of semi-random values to use for returning when a
read from the write port happens. Currently, these values are based in
part on the md5sum of the ROM, so this method needs to be called to
calculate that information. If the method isn't called, then the values
used will be 0.
@param image The cart ROM data
@param size The size of ROM data
*/
void createReadFromWritePortValues(const uInt8* image, uInt32 size);
/**
Return a random value to use when a read from the write port happens.
This functionality is placed in a method, so we can change
implementations as required.
@param address The address of the illegal read
@return A random value somehow associated with the given address
*/
uInt8 randomReadFromWritePortValue(uInt16 address) const;
/**
Create an array that holds code-access information for every byte
of the ROM (indicated by 'size'). Note that this is only used by
@ -294,6 +315,9 @@ class Cartridge : public Device
// Used when we want the 'Cartridge.StartBank' ROM property
StartBankFromPropsFunc myStartBankFromPropsFunc;
// Contains
ShortArray myRAMAccesses;
// Following constructors and assignment operators not supported
Cartridge() = delete;
Cartridge(const Cartridge&) = delete;

View File

@ -27,7 +27,6 @@ CartridgeF8SC::CartridgeF8SC(const BytePtr& image, uInt32 size,
// Copy the ROM image into my buffer
memcpy(myImage, image.get(), std::min(8192u, size));
createCodeAccessBase(8192);
createReadFromWritePortValues(myImage, 8192);
}
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@ -48,16 +47,16 @@ void CartridgeF8SC::install(System& system)
System::PageAccess access(this, System::PA_READ);
// Set the page accessing method for the RAM writing pages
// Map access to this class, since we need to inspect all accesses to
// check if RWP happens
access.type = System::PA_WRITE;
for(uInt16 addr = 0x1000; addr < 0x1080; addr += System::PAGE_SIZE)
{
access.directPokeBase = &myRAM[addr & 0x007F];
access.codeAccessBase = &myCodeAccessBase[addr & 0x007F];
mySystem->setPageAccess(addr, access);
}
// Set the page accessing method for the RAM reading pages
access.directPokeBase = nullptr;
access.type = System::PA_READ;
for(uInt16 addr = 0x1080; addr < 0x1100; addr += System::PAGE_SIZE)
{
@ -94,49 +93,33 @@ uInt8 CartridgeF8SC::peek(uInt16 address)
}
if(address < 0x0080) // Write port is at 0xF000 - 0xF07F (128 bytes)
{
// Reading from the write port triggers an unwanted write
uInt8 value = randomReadFromWritePortValue(address);
if(bankLocked())
return value;
else
{
myRAM[address] = value;
triggerReadFromWritePort(peekAddress);
return value;
}
}
return peekRAM(myRAM[address], peekAddress);
else
return myImage[myBankOffset + address];
}
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
bool CartridgeF8SC::poke(uInt16 address, uInt8)
bool CartridgeF8SC::poke(uInt16 address, uInt8 value)
{
address &= 0x0FFF;
// Switch banks if necessary
switch(address)
switch(address & 0x0FFF)
{
case 0x0FF8:
// Set the current bank to the lower 4k bank
bank(0);
break;
return false;
case 0x0FF9:
// Set the current bank to the upper 4k bank
bank(1);
break;
return false;
default:
break;
}
// NOTE: This does not handle accessing RAM, however, this function
// should never be called for RAM because of the way page accessing
// has been setup
return false;
pokeRAM(myRAM[address & 0x007F], address, value);
return true;
}
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

View File

@ -43,6 +43,7 @@
#include "Settings.hxx"
#include "Vec.hxx"
#include "Cart.hxx"
#include "TIA.hxx"
#include "M6532.hxx"
#include "System.hxx"
@ -74,7 +75,8 @@ M6502::M6502(const Settings& settings)
myDataAddressForPoke(0),
myOnHaltCallback(nullptr),
myHaltRequested(false),
myGhostReadsTrap(true),
myGhostReadsTrap(false),
myReadFromWritePortBreak(false),
myStepStateByInstruction(false)
{
#ifdef DEBUGGER_SUPPORT
@ -123,6 +125,7 @@ void M6502::reset()
myHaltRequested = false;
myGhostReadsTrap = mySettings.getBool("dbg.ghostreadstrap");
myReadFromWritePortBreak = mySettings.getBool(devSettings ? "dev.rwportbreak" : "plr.rwportbreak");
myLastBreakCycle = ULLONG_MAX;
}
@ -302,6 +305,8 @@ inline void M6502::_execute(uInt64 cycles, DispatchResult& result)
msg << "conditional savestate [" << Common::Base::HEX2 << cond << "]";
myDebugger->addState(msg.str());
}
mySystem->cart().clearAllRAMAccesses();
#endif // DEBUGGER_SUPPORT
uInt16 operandAddress = 0, intermediateAddress = 0;
@ -312,6 +317,9 @@ inline void M6502::_execute(uInt64 cycles, DispatchResult& result)
try {
icycles = 0;
#ifdef DEBUGGER_SUPPORT
uInt16 oldPC = PC;
#endif
// Fetch instruction at the program counter
IR = peek(PC++, DISASM_CODE); // This address represents a code section
@ -325,6 +333,25 @@ inline void M6502::_execute(uInt64 cycles, DispatchResult& result)
default:
FatalEmulationError::raise("invalid instruction");
}
#ifdef DEBUGGER_SUPPORT
if(myReadFromWritePortBreak)
{
uInt16 rwpAddr = mySystem->cart().getIllegalRAMAccess();
if(rwpAddr)
{
//////////////////////////////////////////////////////////
// TODO - remove debugging code
cerr << std::hex << "Illegal access: " << rwpAddr
<< " @ " << std::dec << mySystem->cycles() << endl;
//////////////////////////////////////////////////////////
ostringstream msg;
msg << "RWP[@ $" << Common::Base::HEX4 << rwpAddr << "]: ";
result.setDebugger(currentCycles, msg.str(), oldPC);
return;
}
}
#endif // DEBUGGER_SUPPORT
} catch (const FatalEmulationError& e) {
myExecutionStatus |= FatalErrorBit;
result.setMessage(e.what());
@ -443,8 +470,6 @@ bool M6502::save(Serializer& out) const
out.putByte(myFlags);
out.putBool(myHaltRequested);
out.putBool(myStepStateByInstruction);
out.putBool(myGhostReadsTrap);
out.putLong(myLastBreakCycle);
}
catch(...)
@ -492,9 +517,11 @@ bool M6502::load(Serializer& in)
myFlags = in.getByte();
myHaltRequested = in.getBool();
myStepStateByInstruction = in.getBool();
myGhostReadsTrap = in.getBool();
myLastBreakCycle = in.getLong();
#ifdef DEBUGGER_SUPPORT
updateStepStateByInstruction();
#endif
}
catch(...)
{

View File

@ -235,6 +235,7 @@ class M6502 : public Serializable
const StringList& getCondTrapNames() const;
void setGhostReadsTrap(bool enable) { myGhostReadsTrap = enable; }
void setReadFromWritePortBreak(bool enable) { myReadFromWritePortBreak = enable; }
#endif // DEBUGGER_SUPPORT
private:
@ -452,9 +453,8 @@ class M6502 : public Serializable
StringList myTrapCondNames;
#endif // DEBUGGER_SUPPORT
// These are both used only by the debugger, but since they're included
// in save states, they cannot be conditionally compiled
bool myGhostReadsTrap; // trap on ghost reads
bool myGhostReadsTrap; // trap on ghost reads
bool myReadFromWritePortBreak; // trap on reads from write ports
bool myStepStateByInstruction;
private:

View File

@ -351,21 +351,4 @@ string hash(const FilesystemNode& node)
return md5;
}
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
uInt32 hashToInt(const uInt8* buffer, uInt32 length)
{
MD5_CTX context;
uInt8 md5[16];
MD5Init(&context);
MD5Update(&context, buffer, length);
MD5Final(md5, &context);
auto toInt = [](uInt8* arr, uInt32 i) {
return uInt32((arr[i] << 24) + (arr[i+1] << 16) + (arr[i+2] << 8) + arr[i+3]);
};
return toInt(md5, 0) ^ toInt(md5, 4) ^ toInt(md5, 8) ^ toInt(md5, 12);
}
} // Namespace MD5

View File

@ -44,18 +44,6 @@ string hash(const uInt8* buffer, uInt32 length);
*/
string hash(const FilesystemNode& node);
/**
Get the MD5 Message-Digest of the specified message with the
given length. The digest consists of 32 hexadecimal digits,
but here we will get the 4 32-bit integers that make up the
digest and XOR them, resulting in one 32-bit value.
@param buffer The message to compute the digest of
@param length The length of the message
@return The 32-bit integer representing the digest
*/
uInt32 hashToInt(const uInt8* buffer, uInt32 length);
} // Namespace MD5
#endif

View File

@ -645,12 +645,11 @@ double OSystem::dispatchEmulation(EmulationWorker& emulationWorker)
case DispatchResult::Status::fatal:
#ifdef DEBUGGER_SUPPORT
myDebugger->startWithFatalError(dispatchResult.getMessage());
break;
#else
throw runtime_error(dispatchResult.getMessage());
#endif
break;
default:
throw runtime_error("invalid emulation dispatch result");
}

View File

@ -119,6 +119,13 @@ class System : public Serializable
*/
TIA& tia() const { return myTIA; }
/**
Answer the Cart attached to the system.
@return The attached cartridge
*/
Cartridge& cart() const { return myCart; }
/**
Answer the random generator attached to the system.

View File

@ -30,17 +30,17 @@
#include "TabWidget.hxx"
#include "Widget.hxx"
#include "Font.hxx"
#ifdef DEBUGGER_SUPPORT
#include "DebuggerDialog.hxx"
#endif
#include "Console.hxx"
#include "Debugger.hxx"
#include "CartDebug.hxx"
#include "TIA.hxx"
#include "OSystem.hxx"
#include "StateManager.hxx"
#include "RewindManager.hxx"
#include "M6502.hxx"
#ifdef DEBUGGER_SUPPORT
#include "Debugger.hxx"
#include "CartDebug.hxx"
#include "DebuggerDialog.hxx"
#endif
#include "DeveloperDialog.hxx"
// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@ -153,10 +153,12 @@ void DeveloperDialog::addEmulationTab(const GUI::Font& font)
wid.push_back(myUndrivenPinsWidget);
ypos += lineHeight + VGAP;
#ifdef DEBUGGER_SUPPORT
myRWPortBreakWidget = new CheckboxWidget(myTab, font, HBORDER + INDENT * 1, ypos + 1,
"Break on reads from write ports");
wid.push_back(myRWPortBreakWidget);
ypos += lineHeight + VGAP;
#endif
// Thumb ARM emulation exception
myThumbExceptionWidget = new CheckboxWidget(myTab, font, HBORDER + INDENT * 1, ypos + 1,
@ -494,8 +496,10 @@ void DeveloperDialog::loadSettings(SettingsSet set)
myRandomizeCPU[set] = instance().settings().getString(prefix + "cpurandom");
// Undriven TIA pins
myUndrivenPins[set] = instance().settings().getBool(prefix + "tiadriven");
#ifdef DEBUGGER_SUPPORT
// Read from write ports break
myRWPortBreak[set] = instance().settings().getBool(prefix + "rwportbreak");
#endif
// Thumb ARM emulation exception
myThumbException[set] = instance().settings().getBool(prefix + "thumb.trapfatal");
// AtariVox/SaveKey EEPROM access
@ -530,8 +534,10 @@ void DeveloperDialog::saveSettings(SettingsSet set)
instance().settings().setValue(prefix + "cpurandom", myRandomizeCPU[set]);
// Undriven TIA pins
instance().settings().setValue(prefix + "tiadriven", myUndrivenPins[set]);
#ifdef DEBUGGER_SUPPORT
// Read from write ports break
instance().settings().setValue(prefix + "rwportbreak", myRWPortBreak[set]);
#endif
// Thumb ARM emulation exception
instance().settings().setValue(prefix + "thumb.trapfatal", myThumbException[set]);
// AtariVox/SaveKey EEPROM access
@ -569,8 +575,10 @@ void DeveloperDialog::getWidgetStates(SettingsSet set)
myRandomizeCPU[set] = cpurandom;
// Undriven TIA pins
myUndrivenPins[set] = myUndrivenPinsWidget->getState();
#ifdef DEBUGGER_SUPPORT
// Read from write ports break
myRWPortBreak[set] = myRWPortBreakWidget->getState();
#endif
// Thumb ARM emulation exception
myThumbException[set] = myThumbExceptionWidget->getState();
// AtariVox/SaveKey EEPROM access
@ -608,8 +616,10 @@ void DeveloperDialog::setWidgetStates(SettingsSet set)
myRandomizeCPUWidget[i]->setState(BSPF::containsIgnoreCase(cpurandom, cpuregs[i]));
// Undriven TIA pins
myUndrivenPinsWidget->setState(myUndrivenPins[set]);
#ifdef DEBUGGER_SUPPORT
// Read from write ports break
myRWPortBreakWidget->setState(myRWPortBreak[set]);
#endif
// Thumb ARM emulation exception
myThumbExceptionWidget->setState(myThumbException[set]);
// AtariVox/SaveKey EEPROM access
@ -697,12 +707,6 @@ void DeveloperDialog::saveConfig()
saveSettings(SettingsSet::player);
saveSettings(SettingsSet::developer);
#ifdef DEBUGGER_SUPPORT
// Read from write ports break
if(instance().hasConsole())
Debugger::debugger().cartDebug().setReadFromWritePortBreak(myRWPortBreakWidget->getState());
#endif
// activate the current settings
instance().frameBuffer().showFrameStats(myFrameStatsWidget->getState());
// jitter
@ -744,6 +748,10 @@ void DeveloperDialog::saveConfig()
instance().settings().setValue("dbg.ghostreadstrap", myGhostReadsTrapWidget->getState());
if(instance().hasConsole())
instance().console().system().m6502().setGhostReadsTrap(myGhostReadsTrapWidget->getState());
// Read from write ports break
if(instance().hasConsole())
instance().console().system().m6502().setReadFromWritePortBreak(myRWPortBreakWidget->getState());
#endif
}
@ -764,8 +772,10 @@ void DeveloperDialog::setDefaults()
myRandomizeCPU[set] = devSettings ? "SAXYP" : "AXYP";
// Undriven TIA pins
myUndrivenPins[set] = devSettings ? true : false;
#ifdef DEBUGGER_SUPPORT
// Reads from write ports
myRWPortBreak[set] = devSettings ? true : false;
#endif
// Thumb ARM emulation exception
myThumbException[set] = devSettings ? true : false;
// AtariVox/SaveKey EEPROM access