Fixed handling of redundant address/data override prefixes

modules/pcjs/lib/debugger.js

@@ -3228,15 +3228,25 @@ if (DEBUGGER) {
         var bOpcode = this.getByte(dbgAddr, 1);
 
         /*
-         * Incorporate the following prefixes into the current instruction byte stream.
+         * Incorporate OS and AS prefixes into the current instruction.
-         * TODO: Determine the actual effect of multiple OS (and/or multiple AS) prefixes.
+         *
+         * And the verdict is in: redundant OS and AS prefixes must be ignored;
+         * see opOS() and opAS() for details.  We limit the amount of redundancy
+         * to something reasonable (ie, 4).
          */
-        var cMax = 2;           // let's make sure unfortunate memory contents don't screw us
+        var cMax = 4;
+        var fDataPrefix = false, fAddrPrefix = false;
         while ((bOpcode == X86.OPCODE.OS || bOpcode == X86.OPCODE.AS) && cMax--) {
             if (bOpcode == X86.OPCODE.OS) {
-                dbgAddr.fData32 = !dbgAddr.fData32;
+                if (!fDataPrefix) {
+                    dbgAddr.fData32 = !dbgAddr.fData32;
+                    fDataPrefix = true;
+                }
             } else {
-                dbgAddr.fAddr32 = !dbgAddr.fAddr32;
+                if (!fAddrPrefix) {
+                    dbgAddr.fAddr32 = !dbgAddr.fAddr32;
+                    fAddrPrefix = true;
+                }
             }
             bOpcode = this.getByte(dbgAddr, 1);
         }

modules/pcjs/lib/x86cpu.js

@@ -58,8 +58,8 @@ if (!I386) {
     }
 } else {
     /*
-     * These are the more general-purpose ModRM decoders, required for I386 suppport.  The current addressing
+     * These are the more general-purpose ModRM decoders, required for I386 support.  The current addressing
-     * mode (16-bit or 32-bit) dynamically selects the appropriate byte and word decoders.
+     * mode (16-bit or 32-bit) dynamically selects the appropriate set of decoders.
      */
     if (typeof module !== 'undefined') {
         var X86ModB16   = require("./x86modb16");
@@ -141,9 +141,9 @@ function X86CPU(parmsCPU)
      * if any function returns false, the software interrupt will be skipped (presumed to be emulated),
      * and no further notification functions will be called.
      *
-     * NOTE: Registered functions are called only for "INT N" instructions -- NOT "INT 3" or "INTO" or the
+     * NOTE: Registered functions are called only for INT N instructions -- *not* INT 0x03 or INTO or the
-     * "INT 0x00" generated by a divide-by-zero or any other kind of interrupt (nor any interrupt simulated
+     * INT 0x00 generated by a divide-by-zero or any other kind of interrupt (nor any interrupt simulated
-     * with "PUSHF/CALLF").
+     * with PUSHF/CALLF).
      *
      * aIntReturn is a hash of return address notifications set up by software interrupt notification
      * functions that want to receive return notifications.  A software interrupt function must call
@@ -1163,7 +1163,7 @@ X86CPU.prototype.reset = function()
  * the D0 stepping reported 0x0305; beyond that, it's not known exactly what revision numbers Intel used for all
  * 80386 revisions.
  *
- * We define some additional "registers", such as regLIP. which mirrors the linear address corresponding to
+ * We define some additional "registers", such as regLIP, which mirrors the linear address corresponding to
  * CS:IP (the address of the next opcode byte).  In fact, regLIP functions as our internal IP register, so any
  * code that needs the real IP must call getIP().  This, in turn, means that whenever CS or IP must be modified,
  * regLIP must be recalculated, so you must use either setCSIP(), which takes both an offset and a segment,
@@ -2791,6 +2791,11 @@ X86CPU.prototype.setBinding = function(sHTMLType, sBinding, control)
     case "DS":
     case "SS":
     case "ES":
+    case "FS":
+    case "GS":
+    case "CR0":
+    case "CR2":
+    case "CR3":
     case "PS":      // this refers to "Processor Status", aka the 16-bit flags register (although DEBUG.COM refers to this as "PC", surprisingly)
     case "C":
     case "P":
@@ -3822,6 +3827,8 @@ X86CPU.prototype.updateStatus = function(fForce)
             this.updateReg("DS", this.getDS());
             this.updateReg("SS", this.getSS());
             this.updateReg("ES", this.getES());
+            this.updateReg("FS", this.getFS());
+            this.updateReg("GS", this.getGS());
             this.updateReg("EIP", this.getIP());
             var regPS = this.getPS();
             this.updateReg("PS", regPS);
@@ -3834,6 +3841,9 @@ X86CPU.prototype.updateStatus = function(fForce)
             this.updateReg("A", (regPS & X86.PS.AF));
             this.updateReg("P", (regPS & X86.PS.PF));
             this.updateReg("C", (regPS & X86.PS.CF));
+            this.updateReg("CR0", this.regCR0);
+            this.updateReg("CR2", this.regCR2);
+            this.updateReg("CR3", this.regCR3);
         }
     }
 

modules/pcjs/lib/x86func.js

@@ -3724,7 +3724,7 @@ X86.fnFaultMessage = function(nFault, nError, fHalt)
      * However, the foregoing notwithstanding, if MESSAGE.HALT is enabled along with all the other required
      * MESSAGE bits, then we want to halt regardless.
      */
-    if (this.messageEnabled(bitsMessage | Messages.HALT)) {
+    if (DEBUG && nFault == X86.EXCEPTION.GP_FAULT || this.messageEnabled(bitsMessage | Messages.HALT)) {
         fHalt = true;
     }
 

modules/pcjs/lib/x86ops.js

@@ -219,7 +219,7 @@ X86.opPUSHCS = function PUSHCS()
 };
 
 /**
- * op=0x0F (POP CS) (undocumented on 8086/8088; replaced with opInvalid on 80186/80188, and op0F on 80286 and up)
+ * op=0x0F (POP CS) (undocumented on 8086/8088; replaced with opInvalid() on 80186/80188, and op0F() on 80286 and up)
  *
  * @this {X86CPU}
  */
@@ -1375,10 +1375,18 @@ X86.opGS = function GS()
 X86.opOS = function OS()
 {
     if (I386) {
+        /*
+         * See opAS() for a discussion of multiple prefixes, which applies equally to both
+         * operand-size and address-size prefixes.
+         *
+         * The simple fix here is to skip the bulk of the operation if the prefix is redundant.
+         */
         this.opFlags |= X86.OPFLAG.DATASIZE;
-        this.dataSize ^= 0x6;               // that which is 2 shall become 4, and vice versa
+        if (!(this.opPrefixes & X86.OPFLAG.DATASIZE)) {
-        this.dataMask ^= (0xffff0000|0);    // that which is 0x0000ffff shall become 0xffffffff, and vice versa
+            this.dataSize ^= 0x6;               // that which is 2 shall become 4, and vice versa
-        this.updateDataSize();
+            this.dataMask ^= (0xffff0000|0);    // that which is 0x0000ffff shall become 0xffffffff, and vice versa
+            this.updateDataSize();
+        }
         this.nStepCycles -= this.cycleCounts.nOpCyclesPrefix;
     }
 };
@@ -1393,10 +1401,31 @@ X86.opOS = function OS()
 X86.opAS = function AS()
 {
     if (I386) {
+        /*
+         * Live and learn: multiple address-size prefixes can and do occur on a single instruction,
+         * and contrary to my original assumption that the prefixes act independently, they do not.
+         * During Windows 95 SETUP, the following instruction is executed:
+         *
+         *      06AF:1B4D 67672E          CS:
+         *      06AF:1B50 FFA25A1B        JMP      [BP+SI+1B5A]
+         *
+         * which is in fact:
+         *
+         *      06AF:1B4D 67672E          CS:
+         *      06AF:1B50 FFA25A1B0000    JMP      [EDX+00001B5A]
+         *
+         * The other interesting question is: why/how did this instruction get encoded that way?
+         * All I can say is, there were no explicit prefixes in the source (BSG.ASM), so we'll chalk
+         * it up to a glitch in MASM.
+         *
+         * The simple fix here is to skip the bulk of the operation if the prefix is redundant.
+         */
         this.opFlags |= X86.OPFLAG.ADDRSIZE;
-        this.addrSize ^= 0x06;              // that which is 2 shall become 4, and vice versa
+        if (!(this.opPrefixes & X86.OPFLAG.ADDRSIZE)) {
-        this.addrMask ^= (0xffff0000|0);    // that which is 0x0000ffff shall become 0xffffffff, and vice versa
+            this.addrSize ^= 0x06;              // that which is 2 shall become 4, and vice versa
-        this.updateAddrSize();
+            this.addrMask ^= (0xffff0000|0);    // that which is 0x0000ffff shall become 0xffffffff, and vice versa
+            this.updateAddrSize();
+        }
         this.nStepCycles -= this.cycleCounts.nOpCyclesPrefix;
     }
 };