From 19f146038d34940989df9cf2e45f754a3c63cda8 Mon Sep 17 00:00:00 2001
From: stephena <stephena@8b62c5a3-ac7e-4cc8-8f21-d9a121418aba>
Date: Wed, 11 Nov 2009 20:53:57 +0000
Subject: [PATCH] Added more accurate functionality for the _rwport debugger
 command.  It now properly distinguishes between intermediate reads which are
 part of writes, and ordinary reads.  In the former case, only a read which
 has a different address than a write is flagged as an error; intermediate
 reads acting on the same address as the corresponding write are considered
 normal, and won't trigger a break.

git-svn-id: svn://svn.code.sf.net/p/stella/code/trunk@1901 8b62c5a3-ac7e-4cc8-8f21-d9a121418aba
---
 src/debugger/Debugger.cxx       |  4 +---
 src/debugger/RamDebug.cxx       | 35 +++++++++++++++++++++------------
 src/debugger/RamDebug.hxx       | 15 +++++---------
 src/emucore/Cart.cxx            |  9 +--------
 src/emucore/Cart.hxx            | 33 +++++++++++--------------------
 src/emucore/Cart3E.cxx          |  9 ++-------
 src/emucore/CartCV.cxx          |  9 ++-------
 src/emucore/CartE7.cxx          |  9 ++-------
 src/emucore/CartEFSC.cxx        |  9 ++-------
 src/emucore/CartF4SC.cxx        |  9 ++-------
 src/emucore/CartF6SC.cxx        |  9 ++-------
 src/emucore/CartF8SC.cxx        |  9 ++-------
 src/emucore/CartFA.cxx          |  9 ++-------
 src/emucore/CartMC.cxx          |  9 ++-------
 src/emucore/Console.hxx         |  1 -
 src/emucore/m6502/src/M6502.cxx | 14 +++++++++----
 src/emucore/m6502/src/M6502.hxx | 18 +++++++++++++++++
 src/gui/Rect.hxx                |  2 +-
 18 files changed, 87 insertions(+), 125 deletions(-)

diff --git a/src/debugger/Debugger.cxx b/src/debugger/Debugger.cxx
index 07ac93c47..86affcc2a 100644
--- a/src/debugger/Debugger.cxx
+++ b/src/debugger/Debugger.cxx
@@ -198,9 +198,7 @@ void Debugger::setConsole(Console* console)
 
   // Register any RAM areas in the Cartridge
   // Zero-page RAM is automatically recognized by RamDebug
-  const Cartridge::RamAreaList& areas = myConsole->cartridge().ramAreas();
-  for(Cartridge::RamAreaList::const_iterator i = areas.begin(); i != areas.end(); ++i)
-    myRamDebug->addRamArea(i->start, i->size, i->roffset, i->woffset);
+  myRamDebug->addRamArea(myConsole->cartridge().ramAreas());
 
   delete myRiotDebug;
   myRiotDebug = new RiotDebug(*this, *myConsole);
diff --git a/src/debugger/RamDebug.cxx b/src/debugger/RamDebug.cxx
index a2ff12e2d..1684958c8 100644
--- a/src/debugger/RamDebug.cxx
+++ b/src/debugger/RamDebug.cxx
@@ -23,8 +23,7 @@
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 RamDebug::RamDebug(Debugger& dbg, Console& console)
-  : DebuggerSystem(dbg, console),
-    myReadFromWritePortAddress(0)
+  : DebuggerSystem(dbg, console)
 {
   // Zero-page RAM is always present
   addRamArea(0x80, 128, 0, 0);
@@ -51,6 +50,14 @@ void RamDebug::addRamArea(uInt16 start, uInt16 size,
   }
 }
 
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+void RamDebug::addRamArea(const RamAreaList& areas)
+{
+  myRamAreas = areas;
+  for(RamAreaList::const_iterator i = areas.begin(); i != areas.end(); ++i)
+    addRamArea(i->start, i->size, i->roffset, i->woffset);
+}
+
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 const DebuggerState& RamDebug::getState()
 {
@@ -84,17 +91,19 @@ void RamDebug::write(uInt16 addr, uInt8 value)
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 int RamDebug::readFromWritePort()
 {
-  int retval = myReadFromWritePortAddress;
-  if(retval > 0)
-    myReadFromWritePortAddress = 0;
-
-  return retval;
-}
-
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-void RamDebug::setReadFromWritePort(uInt16 address)
-{
-  myReadFromWritePortAddress = address;
+  uInt16 addr = mySystem.m6502().lastReadAddress();
+  if(addr & 0x1000)
+  {
+    addr &= 0x0FFF;
+    for(RamAreaList::const_iterator i = myRamAreas.begin(); i != myRamAreas.end(); ++i)
+    {
+      uInt16 start = (i->start + i->woffset) & 0x0FFF;
+      uInt16 end = (i->start + i->woffset + i->size) & 0x0FFF;
+      if(addr >= start && addr < end)
+        return addr;
+    }
+  }
+  return 0;
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
diff --git a/src/debugger/RamDebug.hxx b/src/debugger/RamDebug.hxx
index 39a0c6931..710ce22fe 100644
--- a/src/debugger/RamDebug.hxx
+++ b/src/debugger/RamDebug.hxx
@@ -23,6 +23,7 @@ class System;
 
 #include "bspf.hxx"
 #include "Array.hxx"
+#include "Cart.hxx"
 #include "DebuggerSystem.hxx"
 
 // pointer types for RamDebug instance methods
@@ -53,6 +54,7 @@ class RamDebug : public DebuggerSystem
       @param woffset  Offset to use when writing to RAM (write port)
     */
     void addRamArea(uInt16 start, uInt16 size, uInt16 roffset, uInt16 woffset);
+    void addRamArea(const RamAreaList& areas);
 
     const DebuggerState& getState();
     const DebuggerState& getOldState() { return myOldState; }
@@ -66,22 +68,15 @@ class RamDebug : public DebuggerSystem
     uInt8 read(uInt16 addr);
     void write(uInt16 addr, uInt8 value);
 
-    // These methods are used by the debugger when we wish to know
-    // if an illegal read from a write port has been performed.
-    // It's up to each Cartridge to report the error, and a
-    // conditional breakpoint must be set in the debugger to check
-    // for occurrences of this.
-    //
-    // Note that each time readFromWritePort() returns a hit, the status
-    // is reset.
+    // Return the address at which an invalid read was performed in a
+    // write port area.
     int readFromWritePort();
-    void setReadFromWritePort(uInt16 address);
 
   private:
     RamState myState;
     RamState myOldState;
 
-    uInt16 myReadFromWritePortAddress;
+    RamAreaList myRamAreas;
 };
 
 #endif
diff --git a/src/emucore/Cart.cxx b/src/emucore/Cart.cxx
index b4247f19c..54b9d6282 100644
--- a/src/emucore/Cart.cxx
+++ b/src/emucore/Cart.cxx
@@ -251,20 +251,13 @@ bool Cartridge::save(ofstream& out)
 void Cartridge::registerRamArea(uInt16 start, uInt16 size,
                                 uInt16 roffset, uInt16 woffset)
 {
+#ifdef DEBUGGER_SUPPORT
   RamArea area;
   area.start   = start;
   area.size    = size;
   area.roffset = roffset;
   area.woffset = woffset;
   myRamAreaList.push_back(area);
-}
-
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-void Cartridge::triggerReadFromWritePort(uInt16 address)
-{
-#ifdef DEBUGGER_SUPPORT
-  if(&Debugger::debugger().ramDebug())
-    Debugger::debugger().ramDebug().setReadFromWritePort(address);
 #endif
 }
 
diff --git a/src/emucore/Cart.hxx b/src/emucore/Cart.hxx
index f702eb890..ef8164f2b 100644
--- a/src/emucore/Cart.hxx
+++ b/src/emucore/Cart.hxx
@@ -30,6 +30,13 @@ class Settings;
 #include "Array.hxx"
 #include "Device.hxx"
 
+#ifdef DEBUGGER_SUPPORT
+struct RamArea {
+  uInt16 start;  uInt16 size;  uInt16 roffset;  uInt16 woffset;
+};
+typedef Common::Array<RamArea> RamAreaList;
+#endif
+
 /**
   A cartridge is a device which contains the machine code for a 
   game and handles any bankswitching performed by the cartridge.
@@ -85,20 +92,9 @@ class Cartridge : public Device
     void lockBank()   { myBankLocked = true;  }
     void unlockBank() { myBankLocked = false; }
 
-  public:
-    /**
-      The following list contains addressable areas of ROM that are mapped
-      to RAM (usually Superchip, but there are other types).  Since such
-      RAM is normally mapped in at different addresses for read and write
-      ports, read and write offsets must be considered.
-
-      @return  List of addressable RAM areas (can be empty)
-    */
-    struct RamArea {
-      uInt16 start;  uInt16 size;  uInt16 roffset;  uInt16 woffset;
-    };
-    typedef Common::Array<RamArea> RamAreaList;
+#ifdef DEBUGGER_SUPPORT
     const RamAreaList& ramAreas() { return myRamAreaList; }
+#endif
 
   public:
     //////////////////////////////////////////////////////////////////////
@@ -173,14 +169,6 @@ class Cartridge : public Device
     */
     void registerRamArea(uInt16 start, uInt16 size, uInt16 roffset, uInt16 woffset);
 
-    /**
-      Indicate that an illegal read from the write port has occurred.
-      This message is sent to the debugger (if support exists).
-
-      @param address  The write port address where the read occurred
-    */
-    void triggerReadFromWritePort(uInt16 address);
-
   private:
     /**
       Get an image pointer and size for a ROM that is part of a larger,
@@ -292,9 +280,10 @@ class Cartridge : public Device
     bool myBankLocked;
 
   private:
+#ifdef DEBUGGER_SUPPORT
     // Contains RamArea entries for those carts with accessible RAM.
     RamAreaList myRamAreaList;
-
+#endif
     // Contains info about this cartridge in string format
     static string myAboutString;
 
diff --git a/src/emucore/Cart3E.cxx b/src/emucore/Cart3E.cxx
index 10d298e97..7e091afac 100644
--- a/src/emucore/Cart3E.cxx
+++ b/src/emucore/Cart3E.cxx
@@ -110,13 +110,8 @@ uInt8 Cartridge3E::peek(uInt16 address)
         // Reading from the write port triggers an unwanted write
         uInt8 value = mySystem->getDataBusState(0xFF);
 
-        if(myBankLocked)
-          return value;
-        else
-        {
-          triggerReadFromWritePort(address);
-          return myRam[(address & 0x03FF) + ((myCurrentBank - 256) << 10)] = value;
-        }
+        if(myBankLocked) return value;
+        else return myRam[(address & 0x03FF) + ((myCurrentBank - 256) << 10)] = value;
       }
     }
   }
diff --git a/src/emucore/CartCV.cxx b/src/emucore/CartCV.cxx
index 5f1e40205..4ccf43179 100644
--- a/src/emucore/CartCV.cxx
+++ b/src/emucore/CartCV.cxx
@@ -113,13 +113,8 @@ uInt8 CartridgeCV::peek(uInt16 address)
     // Reading from the write port triggers an unwanted write
     uInt8 value = mySystem->getDataBusState(0xFF);
 
-    if(myBankLocked)
-      return value;
-    else
-    {
-      triggerReadFromWritePort(address);
-      return myRAM[address & 0x03FF] = value;
-    }
+    if(myBankLocked) return value;
+    else return myRAM[address & 0x03FF] = value;
   }  
   else
   {
diff --git a/src/emucore/CartE7.cxx b/src/emucore/CartE7.cxx
index 771820b81..a57e48d88 100644
--- a/src/emucore/CartE7.cxx
+++ b/src/emucore/CartE7.cxx
@@ -111,13 +111,8 @@ uInt8 CartridgeE7::peek(uInt16 address)
     // Reading from the write port triggers an unwanted write
     uInt8 value = mySystem->getDataBusState(0xFF);
 
-    if(myBankLocked)
-      return value;
-    else
-    {
-      triggerReadFromWritePort(address);
-      return myImage[(myCurrentSlice[address >> 11] << 11) + (address & 0x07FF)] = value;
-    }
+    if(myBankLocked) return value;
+    else return myImage[(myCurrentSlice[address >> 11] << 11) + (address & 0x07FF)] = value;
   }
   else
     return myImage[(myCurrentSlice[address >> 11] << 11) + (address & 0x07FF)];
diff --git a/src/emucore/CartEFSC.cxx b/src/emucore/CartEFSC.cxx
index f2e90395d..56af8592d 100644
--- a/src/emucore/CartEFSC.cxx
+++ b/src/emucore/CartEFSC.cxx
@@ -104,13 +104,8 @@ uInt8 CartridgeEFSC::peek(uInt16 address)
     // Reading from the write port triggers an unwanted write
     uInt8 value = mySystem->getDataBusState(0xFF);
 
-    if(myBankLocked)
-      return value;
-    else
-    {
-      triggerReadFromWritePort(address);
-      return myRAM[address] = value;
-    }
+    if(myBankLocked) return value;
+    else return myRAM[address] = value;
   }  
   else
     return myImage[(myCurrentBank << 12) + address];
diff --git a/src/emucore/CartF4SC.cxx b/src/emucore/CartF4SC.cxx
index 001203c1a..632346bce 100644
--- a/src/emucore/CartF4SC.cxx
+++ b/src/emucore/CartF4SC.cxx
@@ -104,13 +104,8 @@ uInt8 CartridgeF4SC::peek(uInt16 address)
     // Reading from the write port triggers an unwanted write
     uInt8 value = mySystem->getDataBusState(0xFF);
 
-    if(myBankLocked)
-      return value;
-    else
-    {
-      triggerReadFromWritePort(address);
-      return myRAM[address] = value;
-    }
+    if(myBankLocked) return value;
+    else return myRAM[address] = value;
   }  
   else
     return myImage[(myCurrentBank << 12) + address];
diff --git a/src/emucore/CartF6SC.cxx b/src/emucore/CartF6SC.cxx
index 7917fee29..3e26e416b 100644
--- a/src/emucore/CartF6SC.cxx
+++ b/src/emucore/CartF6SC.cxx
@@ -127,13 +127,8 @@ uInt8 CartridgeF6SC::peek(uInt16 address)
     // Reading from the write port triggers an unwanted write
     uInt8 value = mySystem->getDataBusState(0xFF);
 
-    if(myBankLocked)
-      return value;
-    else
-    {
-      triggerReadFromWritePort(address);
-      return myRAM[address] = value;
-    }
+    if(myBankLocked) return value;
+    else return myRAM[address] = value;
   }  
   else
     return myImage[(myCurrentBank << 12) + address];
diff --git a/src/emucore/CartF8SC.cxx b/src/emucore/CartF8SC.cxx
index c24a83b9b..64a3c04ff 100644
--- a/src/emucore/CartF8SC.cxx
+++ b/src/emucore/CartF8SC.cxx
@@ -117,13 +117,8 @@ uInt8 CartridgeF8SC::peek(uInt16 address)
     // Reading from the write port triggers an unwanted write
     uInt8 value = mySystem->getDataBusState(0xFF);
 
-    if(myBankLocked)
-      return value;
-    else
-    {
-      triggerReadFromWritePort(address);
-      return myRAM[address] = value;
-    }
+    if(myBankLocked) return value;
+    else return myRAM[address] = value;
   }  
   else
     return myImage[(myCurrentBank << 12) + address];
diff --git a/src/emucore/CartFA.cxx b/src/emucore/CartFA.cxx
index 95f23f193..aa81293b0 100644
--- a/src/emucore/CartFA.cxx
+++ b/src/emucore/CartFA.cxx
@@ -122,13 +122,8 @@ uInt8 CartridgeFA::peek(uInt16 address)
     // Reading from the write port triggers an unwanted write
     uInt8 value = mySystem->getDataBusState(0xFF);
 
-    if(myBankLocked)
-      return value;
-    else
-    {
-      triggerReadFromWritePort(address);
-      return myRAM[address] = value;
-    }
+    if(myBankLocked) return value;
+    else return myRAM[address] = value;
   }  
   else
     return myImage[(myCurrentBank << 12) + address];
diff --git a/src/emucore/CartMC.cxx b/src/emucore/CartMC.cxx
index 87849fbfb..b041139f3 100644
--- a/src/emucore/CartMC.cxx
+++ b/src/emucore/CartMC.cxx
@@ -146,13 +146,8 @@ uInt8 CartridgeMC::peek(uInt16 address)
         // Reading from the write port triggers an unwanted write
         uInt8 value = mySystem->getDataBusState(0xFF);
 
-        if(myBankLocked)
-          return value;
-        else
-        {
-          triggerReadFromWritePort(address);
-          return myRAM[(uInt32)((block & 0x3F) << 9) + (address & 0x01FF)] = value;
-        }
+        if(myBankLocked) return value;
+        else return myRAM[(uInt32)((block & 0x3F) << 9) + (address & 0x01FF)] = value;
       }
     }
   }  
diff --git a/src/emucore/Console.hxx b/src/emucore/Console.hxx
index 3d5c54b47..93a0eac46 100644
--- a/src/emucore/Console.hxx
+++ b/src/emucore/Console.hxx
@@ -19,7 +19,6 @@
 #ifndef CONSOLE_HXX
 #define CONSOLE_HXX
 
-class Console;
 class Controller;
 class Event;
 class Switches;
diff --git a/src/emucore/m6502/src/M6502.cxx b/src/emucore/m6502/src/M6502.cxx
index a1c755f7f..cf15e3cfc 100644
--- a/src/emucore/m6502/src/M6502.cxx
+++ b/src/emucore/m6502/src/M6502.cxx
@@ -34,7 +34,9 @@ M6502::M6502(uInt32 systemCyclesPerProcessorCycle)
     myLastAccessWasRead(true),
     myTotalInstructionCount(0),
     myNumberOfDistinctAccesses(0),
-    myLastAddress(0)
+    myLastAddress(0),
+    myLastPeekAddress(0),
+    myLastPokeAddress(0)
 {
 #ifdef DEBUGGER_SUPPORT
   myDebugger    = NULL;
@@ -169,6 +171,7 @@ inline uInt8 M6502::peek(uInt16 address)
 
   uInt8 result = mySystem->peek(address);
   myLastAccessWasRead = true;
+  myLastPeekAddress = address;
   return result;
 }
 
@@ -193,6 +196,7 @@ inline void M6502::poke(uInt16 address, uInt8 value)
 
   mySystem->poke(address, value);
   myLastAccessWasRead = false;
+  myLastPokeAddress = address;
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@@ -206,9 +210,6 @@ bool M6502::execute(uInt32 number)
   {
     for(; !myExecutionStatus && (number != 0); --number)
     {
-      uInt16 operandAddress = 0;
-      uInt8 operand = 0;
-
 #ifdef DEBUGGER_SUPPORT
       if(myJustHitTrapFlag)
       {
@@ -236,6 +237,11 @@ bool M6502::execute(uInt32 number)
           return true;
       }
 #endif
+      uInt16 operandAddress = 0;
+      uInt8 operand = 0;
+
+      // Reset the peek/poke address pointers
+      myLastPeekAddress = myLastPokeAddress = 0;
 
       // Fetch instruction at the program counter
       IR = peek(PC++);
diff --git a/src/emucore/m6502/src/M6502.hxx b/src/emucore/m6502/src/M6502.hxx
index bc16aa6a3..a04ad0843 100644
--- a/src/emucore/m6502/src/M6502.hxx
+++ b/src/emucore/m6502/src/M6502.hxx
@@ -154,6 +154,20 @@ class M6502 : public Serializable
     */ 
     bool lastAccessWasRead() const { return myLastAccessWasRead; }
 
+    /**
+      Return the last address that was part of a read/peek.  Note that
+      reads which are part of a write are not considered here, unless
+      they're not the same as the last write address.  This eliminates
+      accesses that are part of a normal read/write cycle.
+
+      @return The address of the last read
+    */
+    uInt16 lastReadAddress() const {
+      return myLastPokeAddress ?
+        (myLastPokeAddress != myLastPeekAddress ? myLastPeekAddress : 0) :
+        myLastPeekAddress;
+    }
+
     /**
       Get the total number of instructions executed so far.
 
@@ -325,6 +339,10 @@ class M6502 : public Serializable
     /// Indicates the last address which was accessed
     uInt16 myLastAddress;
 
+    /// Indicates the last address which was accessed specifically
+    /// by a peek or poke command
+    uInt16 myLastPeekAddress, myLastPokeAddress;
+
 #ifdef DEBUGGER_SUPPORT
     /// Pointer to the debugger for this processor or the null pointer
     Debugger* myDebugger;
diff --git a/src/gui/Rect.hxx b/src/gui/Rect.hxx
index 31f4e7c03..502786b8c 100644
--- a/src/gui/Rect.hxx
+++ b/src/gui/Rect.hxx
@@ -28,7 +28,7 @@
 
 namespace GUI {
 
-/*! 	@brief simple class for handling both 2D position and size
+/*! @brief simple class for handling both 2D position and size
 
 	This small class is an helper for position and size values.
 */