From 72a9b8ef8cdc8ccf681c10aa371b13469a540866 Mon Sep 17 00:00:00 2001
From: Jeff Parsons <Jeff@pcjs.org>
Date: Sat, 1 Aug 2015 01:03:49 -0700
Subject: [PATCH] Applied some more breakpoint fixes

---
 .../5150/cga/384kb/softkbd/machine.xml        |  2 +-
 .../5160/cga/256kb/softkbd/machine.xml        |  2 +-
 modules/pcjs/lib/debugger.js                  | 18 +++--
 modules/pcjs/lib/memory.js                    | 75 ++++++++++---------
 modules/pcjs/lib/x86cpu.js                    |  2 +-
 modules/pcjs/lib/x86op0f.js                   | 16 ++--
 6 files changed, 62 insertions(+), 53 deletions(-)

diff --git a/devices/pc/machine/5150/cga/384kb/softkbd/machine.xml b/devices/pc/machine/5150/cga/384kb/softkbd/machine.xml
index 1eb896b33..ac170b58a 100644
--- a/devices/pc/machine/5150/cga/384kb/softkbd/machine.xml
+++ b/devices/pc/machine/5150/cga/384kb/softkbd/machine.xml
@@ -11,7 +11,7 @@
 	<video ref="/devices/pc/video/ibm/cga/ibm-cga-960.xml"/>
 	<keyboard ref="/devices/pc/keyboard/keyboard-us83.xml"/>
 	<panel ref="/devices/pc/panel/wide.xml"/>
-	<fdc ref="/disks/pc/samples.xml" width="280px"/>
+	<fdc ref="/disks/pc/library.xml" width="320px"/>
 	<chipset id="chipset" model="5150" sw1="01001001" sw2="10100000" pos="left" padleft="8px" padbottom="8px">
 		<control type="switches" label="SW1" binding="sw1" left="0px"/>
 		<control type="switches" label="SW2" binding="sw2" left="0px"/>
diff --git a/devices/pc/machine/5160/cga/256kb/softkbd/machine.xml b/devices/pc/machine/5160/cga/256kb/softkbd/machine.xml
index b13a14284..71c5210c9 100644
--- a/devices/pc/machine/5160/cga/256kb/softkbd/machine.xml
+++ b/devices/pc/machine/5160/cga/256kb/softkbd/machine.xml
@@ -12,7 +12,7 @@
 	<keyboard ref="/devices/pc/keyboard/keyboard-us83.xml"/>
 	<debugger id="debugger"/>
 	<panel ref="/devices/pc/panel/default.xml"/>
-	<fdc ref="/disks/pc/samples.xml" width="280px"/>
+	<fdc ref="/disks/pc/library.xml" width="320px"/>
 	<chipset id="chipset" model="5160" sw1="01001001" pos="left" padleft="8px" padbottom="8px">
 		<control type="switches" label="SW1" binding="sw1" left="0px"/>
 		<control type="description" binding="swdesc" left="0px"/>
diff --git a/modules/pcjs/lib/debugger.js b/modules/pcjs/lib/debugger.js
index 4013ba510..957cd1354 100644
--- a/modules/pcjs/lib/debugger.js
+++ b/modules/pcjs/lib/debugger.js
@@ -1484,9 +1484,8 @@ if (DEBUGGER) {
     Debugger.prototype.getSegment = function(sel, fProt)
     {
         var fProtMode = this.getProtMode();
-        if (fProt === undefined) {
-            fProt = fProtMode;
-        }
+        if (fProt === undefined) fProt = fProtMode;
+
         if (fProt == fProtMode) {
             if (sel === this.cpu.getCS()) return this.cpu.segCS;
             if (sel === this.cpu.getDS()) return this.cpu.segDS;
@@ -1496,8 +1495,13 @@ if (DEBUGGER) {
                 if (sel === this.cpu.getFS()) return this.cpu.segFS;
                 if (sel === this.cpu.getGS()) return this.cpu.segGS;
             }
+            /*
+             * Even if nSuppressBreaks is set, we'll allow the call if we're in real-mode, because
+             * a loadReal() request using segDebugger should generally be safe.
+             */
+            if (this.nSuppressBreaks && fProt || !this.segDebugger) return null;
         }
-        if (this.nSuppressBreaks || !this.segDebugger) return null;
+
         /*
          * Note the load() function's fSuppress parameter, which the Debugger should ALWAYS set to true
          * to avoid triggering a fault.  Unfortunately, when paging is enabled, there's still the risk of
@@ -3160,8 +3164,7 @@ if (DEBUGGER) {
                  * (hence the assertion that there IS a linear address stored in dbgAddr);
                  * this allows us to step over calls or interrupts that change the processor mode
                  */
-                dbgAddr.sel = null;
-                this.assert(dbgAddr.addr);
+                if (dbgAddr.addr) dbgAddr.sel = null;
             } else {
                 this.println("breakpoint enabled: " + this.hexAddr(dbgAddr) + " (" + aBreak[0] + ")");
             }
@@ -3366,7 +3369,7 @@ if (DEBUGGER) {
                  */
                 var addrBreak = this.mapBreakpoint(this.getAddr(dbgAddrBreak));
                 for (var n = 0; n < nb; n++) {
-                    if (addr == addrBreak) {
+                    if (addr + n == addrBreak) {
                         if (dbgAddrBreak.fTempBreak) {
                             this.findBreakpoint(aBreak, dbgAddrBreak, true);
                         } else if (!fTemp) {
@@ -3376,7 +3379,6 @@ if (DEBUGGER) {
                         break;
                     }
                     addrBreak++;
-                    addr++;
                     n++;
                 }
             }
diff --git a/modules/pcjs/lib/memory.js b/modules/pcjs/lib/memory.js
index f6eb2e609..5d5ec5b02 100644
--- a/modules/pcjs/lib/memory.js
+++ b/modules/pcjs/lib/memory.js
@@ -522,51 +522,58 @@ Memory.prototype = {
         this.bitPTEAccessed = blockPhys? Memory.adjustEndian(X86.PTE.ACCESSED) : 0;
     },
     /**
-     * addBreakpoint(off, fWrite)
+     * addBreakpoint(off, fWrite, cpu)
+     *
+     * NOTE: Some Memory blocks already require access to the CPU (eg, UNPAGED blocks that need to call cpu.mapPageBlock()),
+     * while others require access only if the CPU has set a read or write breakpoint in one of its Debug registers; the latter
+     * case is handled here by virtue of the cpu parameter.
      *
      * @this {Memory}
      * @param {number} off
      * @param {boolean} fWrite
+     * @param {X86CPU} [cpu] (required for breakpoints set by the CPU, as opposed to the Debugger)
      */
-    addBreakpoint: function(off, fWrite) {
-        if (DEBUGGER && this.dbg) {
-            if (!fWrite) {
-                if (this.cReadBreakpoints++ === 0) {
-                    this.setReadAccess(Memory.afnChecked, false);
-                }
-                if (DEBUG) this.dbg.println("read breakpoint added to memory block " + str.toHex(this.addr));
+    addBreakpoint: function(off, fWrite, cpu) {
+        if (!fWrite) {
+            if (this.cReadBreakpoints++ === 0) {
+                if (cpu) this.cpu = cpu;
+                this.setReadAccess(Memory.afnChecked, false);
             }
-            else {
-                if (this.cWriteBreakpoints++ === 0) {
-                    this.setWriteAccess(Memory.afnChecked, false);
-                }
-                if (DEBUG) this.dbg.println("write breakpoint added to memory block " + str.toHex(this.addr));
+            if (DEBUG && this.dbg) this.dbg.println("read breakpoint added to memory block " + str.toHex(this.addr));
+        }
+        else {
+            if (this.cWriteBreakpoints++ === 0) {
+                if (cpu) this.cpu = cpu;
+                this.setWriteAccess(Memory.afnChecked, false);
             }
+            if (DEBUG && this.dbg) this.dbg.println("write breakpoint added to memory block " + str.toHex(this.addr));
         }
     },
     /**
      * removeBreakpoint(off, fWrite)
      *
+     * NOTE: If this Memory block is not an UNPAGED block that might need to call cpu.mapPageBlock()), and it no longer
+     * has any read or write breakpoints associated with it, then it no longer needs a CPU reference.  However, the latter
+     * is a moot point, because the "checked" memory access functions should be swapped out when this function is done.
+     *
      * @this {Memory}
      * @param {number} off
      * @param {boolean} fWrite
      */
     removeBreakpoint: function(off, fWrite) {
-        if (DEBUGGER && this.dbg) {
-            if (!fWrite) {
-                if (--this.cReadBreakpoints === 0) {
-                    this.resetReadAccess();
-                    if (DEBUG) this.dbg.println("all read breakpoints removed from memory block " + str.toHex(this.addr));
-                }
-                this.dbg.assert(this.cReadBreakpoints >= 0);
+        if (!fWrite) {
+            if (--this.cReadBreakpoints === 0) {
+                this.resetReadAccess();
+                if (DEBUG && this.dbg) this.dbg.println("all read breakpoints removed from memory block " + str.toHex(this.addr));
             }
-            else {
-                if (--this.cWriteBreakpoints === 0) {
-                    this.resetWriteAccess();
-                    if (DEBUG) this.dbg.println("all write breakpoints removed from memory block " + str.toHex(this.addr));
-                }
-                this.dbg.assert(this.cWriteBreakpoints >= 0);
+            if (DEBUG && this.dbg) this.dbg.assert(this.cReadBreakpoints >= 0);
+        }
+        else {
+            if (--this.cWriteBreakpoints === 0) {
+                this.resetWriteAccess();
+                if (DEBUG && this.dbg) this.dbg.println("all write breakpoints removed from memory block " + str.toHex(this.addr));
             }
+            if (DEBUG && this.dbg) this.dbg.assert(this.cWriteBreakpoints >= 0);
         }
     },
     /**
@@ -805,7 +812,7 @@ Memory.prototype = {
      */
     readByteChecked: function readByteChecked(off, addr) {
         if (!DEBUGGER || !this.dbg || !this.dbg.checkMemoryRead(addr)) {
-            if (I386) this.cpu.checkMemoryException(addr, 1, false);
+            if (I386 && this.cpu) this.cpu.checkMemoryException(addr, 1, false);
         }
         return this.readByteDirect(off, addr);
     },
@@ -821,8 +828,8 @@ Memory.prototype = {
      * @return {number}
      */
     readShortChecked: function readShortChecked(off, addr) {
-        if (!DEBUGGER || !this.dbg && !this.dbg.checkMemoryRead(addr, 2)) {
-            if (I386) this.cpu.checkMemoryException(addr, 2, false);
+        if (!DEBUGGER || !this.dbg || !this.dbg.checkMemoryRead(addr, 2)) {
+            if (I386 && this.cpu) this.cpu.checkMemoryException(addr, 2, false);
         }
         return this.readShortDirect(off, addr);
     },
@@ -839,7 +846,7 @@ Memory.prototype = {
      */
     readLongChecked: function readLongChecked(off, addr) {
         if (!DEBUGGER || !this.dbg || !this.dbg.checkMemoryRead(addr, 4)) {
-            if (I386) this.cpu.checkMemoryException(addr, 4, false);
+            if (I386 && this.cpu) this.cpu.checkMemoryException(addr, 4, false);
         }
         return this.readLongDirect(off, addr);
     },
@@ -856,7 +863,7 @@ Memory.prototype = {
      */
     writeByteChecked: function writeByteChecked(off, b, addr) {
         if (!DEBUGGER || !this.dbg || !this.dbg.checkMemoryWrite(addr)) {
-            if (I386) this.cpu.checkMemoryException(addr, 1, true);
+            if (I386 && this.cpu) this.cpu.checkMemoryException(addr, 1, true);
         }
         if (this.fReadOnly) this.writeNone(off, b, addr); else this.writeByteDirect(off, b, addr);
     },
@@ -872,8 +879,8 @@ Memory.prototype = {
      * @param {number} w
      */
     writeShortChecked: function writeShortChecked(off, w, addr) {
-        if (!DEBUGGER || !this.dbg && !this.dbg.checkMemoryWrite(addr, 2)) {
-            if (I386) this.cpu.checkMemoryException(addr, 2, true);
+        if (!DEBUGGER || !this.dbg || !this.dbg.checkMemoryWrite(addr, 2)) {
+            if (I386 && this.cpu) this.cpu.checkMemoryException(addr, 2, true);
         }
         if (this.fReadOnly) this.writeNone(off, w, addr); else this.writeShortDirect(off, w, addr);
     },
@@ -890,7 +897,7 @@ Memory.prototype = {
      */
     writeLongChecked: function writeLongChecked(off, l, addr) {
         if (!DEBUGGER || !this.dbg || !this.dbg.checkMemoryWrite(addr, 4)) {
-            if (I386) this.cpu.checkMemoryException(addr, 4, true);
+            if (I386 && this.cpu) this.cpu.checkMemoryException(addr, 4, true);
         }
         if (this.fReadOnly) this.writeNone(off, l, addr); else this.writeLongDirect(off, l, addr);
     },
diff --git a/modules/pcjs/lib/x86cpu.js b/modules/pcjs/lib/x86cpu.js
index e7719b51c..32c0b0695 100644
--- a/modules/pcjs/lib/x86cpu.js
+++ b/modules/pcjs/lib/x86cpu.js
@@ -1696,7 +1696,7 @@ X86CPU.prototype.checkIntReturn = function(addr)
 X86CPU.prototype.addMemCheck = function(addr, fWrite)
 {
     var iBlock = addr >>> this.nBlockShift;
-    this.aMemBlocks[iBlock].addBreakpoint(addr & this.nBlockLimit, fWrite);
+    this.aMemBlocks[iBlock].addBreakpoint(addr & this.nBlockLimit, fWrite, this);
 };
 
 /**
diff --git a/modules/pcjs/lib/x86op0f.js b/modules/pcjs/lib/x86op0f.js
index 9f6fbc604..23571e7a9 100644
--- a/modules/pcjs/lib/x86op0f.js
+++ b/modules/pcjs/lib/x86op0f.js
@@ -309,7 +309,7 @@ X86.opLOADALL386 = function LOADALL386()
      */
     X86.opUndefined.call(this);
 
-    this.nStepCycles -= 100;            // I've not seen a documented time for the 80386 LOADALL, so we'll make a guess
+    this.nStepCycles -= 100;            // TODO: I've not seen a documented time for the 80386 LOADALL; update this random guess
 };
 
 /**
@@ -366,7 +366,7 @@ X86.opMOVrc = function MOVrc()
     this.nStepCycles -= 6;
 
     /*
-     * TODO: Implement BACKTRACK for this instruction....
+     * TODO: Implement BACKTRACK for this instruction (although Control registers are not likely to be a conduit for much interesting data).
      */
 };
 
@@ -406,7 +406,7 @@ X86.opMOVrd = function MOVrd()
     this.nStepCycles -= 22;
 
     /*
-     * TODO: Implement BACKTRACK for this instruction....
+     * TODO: Implement BACKTRACK for this instruction (although Debug registers are not likely to be a conduit for much interesting data).
      */
 };
 
@@ -464,7 +464,7 @@ X86.opMOVcr = function MOVcr()
     }
 
     /*
-     * TODO: Implement BACKTRACK for this instruction....
+     * TODO: Implement BACKTRACK for this instruction (although Control registers are not likely to be a conduit for much interesting data).
      */
 };
 
@@ -529,7 +529,7 @@ X86.opMOVdr = function MOVdr()
     this.regDR[iDst] = regDR;
     this.nStepCycles -= (iDst < 4? 22 : 14);
     /*
-     * TODO: Implement BACKTRACK for this instruction....
+     * TODO: Implement BACKTRACK for this instruction (although Debug registers are not likely to be a conduit for much interesting data).
      */
 };
 
@@ -567,7 +567,7 @@ X86.opMOVrt = function MOVrt()
     this.nStepCycles -= 12;
 
     /*
-     * TODO: Implement BACKTRACK for this instruction....
+     * TODO: Implement BACKTRACK for this instruction (although Test registers are not likely to be a conduit for much interesting data).
      */
 };
 
@@ -603,14 +603,14 @@ X86.opMOVtr = function MOVtr()
     }
 
     /*
-     * TODO: Do something with the Test registers....
+     * TODO: Do something useful with the Test registers.
      */
     this.regTR[iDst] = this.getReg(bModRM & 0x7);
 
     this.nStepCycles -= 12;
 
     /*
-     * TODO: Implement BACKTRACK for this instruction....
+     * TODO: Implement BACKTRACK for this instruction (although Test registers are not likely to be a conduit for much interesting data).
      */
 };