Retro/pcjs
Watch
1
0
Fork
You've already forked pcjs
 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fixed internal stack overflow caused by overly rapid keyboard interrupts

Browse source
This commit is contained in:
Jeff Parsons 2014-11-06 15:31:50 -08:00 • committed by jeffpar
commit 88e71cdb17
17 changed files with 1972 additions and 1809 deletions
Download patch file Download diff file Expand all files Collapse all files

110
my_modules/pcjs-client/lib/chipset.js
View file

@ -3005,7 +3005,9 @@ ChipSet.prototype.getIRRVector = function(iPIC)
}
var nIRQ = pic.nIRQBase + nIRL;
if (DEBUG) this.messageDebugger("getIRRVector(): IRQ " + nIRQ + " going into service", Debugger.MESSAGE_PIC, nIRQ);
if (DEBUG) {
this.messageDebugger("getIRRVector(): IRQ " + nIRQ + " interrupting @" + str.toHexAddr(this.cpu.regIP, this.cpu.segCS.sel) + " stack=" + str.toHexAddr(this.cpu.regSP, this.cpu.segSS.sel), 0, nIRQ);
}
if (MAXDEBUG && DEBUGGER) {
this.acInterrupts[nIRQ]++;
}
@ -4013,6 +4015,7 @@ ChipSet.prototype.out8042InBuffCmd = function(port, bOut, addrFrom)
*/
ChipSet.prototype.set8042CmdData = function(b)
{
var bClockWasEnabled = !(this.b8042CmdData & ChipSet.KBC.DATA.CMD.NO_CLOCK);
this.b8042CmdData = b;
Component.assert(ChipSet.KBC.DATA.CMD.SYS_FLAG === ChipSet.KBC.STATUS.SYS_FLAG);
this.b8042Status = (this.b8042Status & ~ChipSet.KBC.STATUS.SYS_FLAG) | (b & ChipSet.KBC.DATA.CMD.SYS_FLAG);
@ -4032,9 +4035,13 @@ ChipSet.prototype.set8042CmdData = function(b)
* powered on, it performs the BAT, and then when the clock and data lines go high, the keyboard sends
* a completion code (eg, 0xAA for success, or 0xFC or something else for failure).
*/
if (this.kbd.setEnable(!!(b & ChipSet.KBC.DATA.CMD.NO_INHIBIT), !(b & ChipSet.KBC.DATA.CMD.NO_CLOCK))) {
var bClockEnabled = !(b & ChipSet.KBC.DATA.CMD.NO_CLOCK);
if (this.kbd.setEnable(!!(b & ChipSet.KBC.DATA.CMD.NO_INHIBIT), bClockEnabled)) {
this.set8042OutBuff(this.kbd.readScanCode(true));
}
if (!bClockWasEnabled && bClockEnabled && this.kbd.readScanCode()) {
this.notifyKbdData(true);
}
}
};
@ -4077,6 +4084,103 @@ ChipSet.prototype.set8042OutPort = function(b)
}
};
/**
* notifyKbdData(fAvail)
*
* Previously, the Keyboard would simply call setIRR() when it had some data for the keyboard controller.
* Now the interface is a little more nuanced, giving the ChipSet/8042 the opportunity to decide when to
* raise IRQ.KBD.
*
* If there's an 8042, we check (this.b8042CmdData & ChipSet.KBC.DATA.CMD.NO_CLOCK); if NO_CLOCK is clear,
* we can raise the IRQ immediately. Well, not quite immediately....
*
* Notes regarding the MODEL_5170 (eg, /devices/pc/machine/5170/ega/1152kb/rev3/machine.xml):
*
* The "Rev3" BIOS, dated 11-Nov-1985, contains the following code in the keyboard interrupt handler at K26A:
*
* F000:3704 FA CLI
* F000:3705 B020 MOV AL,20
* F000:3707 E620 OUT 20,AL
* F000:3709 B0AE MOV AL,AE
* F000:370B E88D02 CALL SHIP_IT
* F000:370E FA CLI <-- window of opportunity
* F000:370F 07 POP ES
* F000:3710 1F POP DS
* F000:3711 5F POP DI
* F000:3712 5E POP SI
* F000:3713 5A POP DX
* F000:3714 59 POP CX
* F000:3715 5B POP BX
* F000:3716 58 POP AX
* F000:3717 5D POP BP
* F000:3718 CF IRET
*
* and SHIP_IT looks like this:
*
* F000:399B 50 PUSH AX
* F000:399C FA CLI
* F000:399D 2BC9 SUB CX,CX
* F000:399F E464 IN AL,64
* F000:39A1 A802 TEST AL,02
* F000:39A3 E0FA LOOPNZ 399F
* F000:39A5 58 POP AX
* F000:39A6 E664 OUT 64,AL
* F000:39A8 FB STI
* F000:39A9 C3 RET
*
* This code *appears* to be trying to ensure that another keyboard interrupt won't occur until after the IRET,
* but sadly, it looks to me like the CLI following the call to SHIP_IT is too late. SHIP_IT should have been
* written with PUSHF/CLI and POPF intro/outro sequences, thereby honoring the first CLI at the top of K26A and
* eliminating the need for the second CLI (@F000:370E).
*
* Of course, in REAL LIFE, this was probably never a problem, because the 8042 probably wasn't fast enough to
* generate another interrupt so soon after receiving the ChipSet.KBC.CMD.ENABLE_KBD command. In my case, I ran
* into this problem by 1) turning on "kbd" Debugger messages and 2) rapidly typing lots of keys. The Debugger
* messages bogged the machine down enough for me to hit the "window of opportunity", generating this message in
* PC-DOS 3.20:
*
* "FATAL: Internal Stack Failure, System Halted."
*
* and halting the system @0070:0923 (JMP 0923).
*
* That wasn't the only spot in the BIOS where I hit this problem; here's another "window of opportunity":
*
* F000:3975 FA CLI
* F000:3976 B020 MOV AL,20
* F000:3978 E620 OUT 20,AL
* F000:397A B0AE MOV AL,AE
* F000:397C E81C00 CALL SHIP_IT
* F000:397F B80291 MOV AX,9102 <-- window of opportunity
* F000:3982 CD15 INT 15
* F000:3984 80269600FC AND [0096],FC
* F000:3989 E982FD JMP 370E
*
* In this second, lengthier, example, I counted about 60 instructions being executed from the EOI @F000:3978 to
* the final IRET @F000:3718, most of them in the INT 0x15 handler. So, I'm going to double that count to 120
* instructions, just to be safe, and pass that along to every setIRR() call we make here.
*
* @this {ChipSet}
* @param {boolean} fAvail is true if the Keyboard has data to send, false if not
*/
ChipSet.prototype.notifyKbdData = function(fAvail)
{
if (this.model < ChipSet.MODEL_5170) {
/*
* TODO: Should we be checking bPPI for PPI_B.CLK_KBD on these older machines, before called setIRR()?
*/
this.setIRR(ChipSet.IRQ.KBD, 4);
}
else {
if (!(this.b8042CmdData & ChipSet.KBC.DATA.CMD.NO_CLOCK) && fAvail) {
/*
* A delay of 4 instructions was originally requested as part of the the Keyboard's resetDevice()
* response, but a much larger delay (120) is now needed for MODEL_5170 machines, per the discussion above.
*/
this.setIRR(ChipSet.IRQ.KBD, 120);
}
}
};
/**
* inCMOSAddr(port, addrFrom)
*
@ -4311,7 +4415,7 @@ ChipSet.prototype.messageDebugger = function(sMessage, bitsMessage, nIRQ)
if (DEBUGGER && this.dbg) {
if (bitsMessage == null) bitsMessage = Debugger.MESSAGE_CHIPSET;
if (nIRQ !== undefined) {
bitsMessage |= (nIRQ == ChipSet.IRQ.TIMER0? Debugger.MESSAGE_TIMER : (nIRQ == ChipSet.IRQ.KBD? Debugger.MESSAGE_KBD : (nIRQ == ChipSet.IRQ.FDC? Debugger.MESSAGE_FDC : 0)));
bitsMessage |= (nIRQ == ChipSet.IRQ.TIMER0? Debugger.MESSAGE_TIMER : (nIRQ == ChipSet.IRQ.KBD? Debugger.MESSAGE_KBD : (nIRQ == ChipSet.IRQ.FDC? Debugger.MESSAGE_FDC : Debugger.MESSAGE_PIC)));
}
if (this.dbg.messageEnabled(bitsMessage)) this.dbg.message(sMessage);
}

21
my_modules/pcjs-client/lib/debugger.js
View file

@ -3806,9 +3806,18 @@ if (DEBUGGER) {
if (fListSymbols) {
var aSymbol = this.findSymbolAtAddr(aAddr, true);
if (aSymbol.length) {
if (aSymbol[0]) this.println(str.toHexWord(aSymbol[1]) + ": " + aSymbol[0]);
if (aSymbol.length > 4) {
if (aSymbol[4]) this.println(str.toHexWord(aSymbol[5]) + ": " + aSymbol[4]);
var nDelta, sDelta;
if (aSymbol[0]) {
sDelta = "";
nDelta = aAddr[0] - aSymbol[1];
if (nDelta) sDelta = " + " + str.toHexWord(nDelta);
this.println(aSymbol[0] + " (" + str.toHexAddr(aSymbol[1], aAddr[1]) + ")" + sDelta);
}
if (aSymbol.length > 4 && aSymbol[4]) {
sDelta = "";
nDelta = aSymbol[5] - aAddr[0];
if (nDelta) sDelta = " - " + str.toHexWord(nDelta);
this.println(aSymbol[4] + " (" + str.toHexAddr(aSymbol[5], aAddr[1]) + ")" + sDelta);
}
} else {
this.println("no symbols");
@ -4419,11 +4428,12 @@ if (DEBUGGER) {
var fBlank = (aAddr[0] != this.aAddrNextCode[0]);
while (n-- && (aAddr[1] != null? (aAddr[0] < aAddrEnd[0]) : (aAddr[2] < aAddrEnd[2]))) {
while (n && (aAddr[1] != null? (aAddr[0] < aAddrEnd[0]) : (aAddr[2] < aAddrEnd[2]))) {
/*
* I pass nCycles instead of cInstructions to getInstruction() now, to assist with visual
* verification of the accuracy (or inaccuracy) of instruction cycle counts.
*/
n--;
var bOpcode = this.getByte(aAddr);
/*
* We don't want to leave the disassembly ending with a prefix, especially now that stepCPU(0) continues
@ -4453,6 +4463,7 @@ if (DEBUGGER) {
this.aAddrNextCode = aAddr;
fBlank = false;
}
if (n) this.println("end of memory");
};
/**
@ -4536,7 +4547,7 @@ if (DEBUGGER) {
if (this.cmp) this.cmp.reset();
return true;
case "ver":
this.println((APPNAME || "PCjs") + " version " + APPVERSION + " (" + (COMPILED? "release" : (DEBUG? "debug" : "nodebug")) + (PREFETCH? ",prefetch" : ",noprefetch") + (EAFUNCS? "eafuncs" : ",eatests") + (TYPEDARRAYS? ",typedarrays" : (FATARRAYS? ",fatarrays" : ",dwords")) + ")");
this.println((APPNAME || "PCjs") + " version " + APPVERSION + " (" + (COMPILED? "release" : (DEBUG? "debug" : "nodebug")) + (PREFETCH? ",prefetch" : ",noprefetch") + (EAFUNCS? "eafuncs" : ",eatests") + (TYPEDARRAYS? ",typedarrays" : (FATARRAYS? ",fatarrays" : ",dwordarrays")) + ")");
return true;
default:
ch0 = sCmd.charAt(0);

4
my_modules/pcjs-client/lib/defines.js
View file

@ -98,8 +98,8 @@ var FATARRAYS = false;
* TYPEDARRAYS enables use of typed arrays for Memory blocks. This used to be a compile-time-only option, but I've
* added Memory access functions for typed arrays (see Memory.afnTypedArray), so support can be enabled dynamically.
*
* However, TYPEDARRAYS has always been slightly slower than the original DWORDS implementation (which uses an Array
* of numbers that stores 32 bits -- 4 consecutive bytes -- per number), so TYPEDARRAYS is completely disabled for now.
* However, TYPEDARRAYS has always been slightly slower than the original DWORDARRAYS implementation (which uses an
* Array of numbers that stores 32 bits -- 4 consecutive bytes -- per number), so TYPEDARRAYS is completely disabled.
*
* See the Memory component for details.
*/

109
my_modules/pcjs-client/lib/keyboard.js
View file

@ -124,19 +124,22 @@ Keyboard.CMD = {
*/
Keyboard.CMDRES = {
OVERRUN: 0x00,
LOAD_TEST: 0x65, // undocumented "LOAD MANUFACTURING TEST REQUEST" response code
BAT_SUCC: 0xAA, // Basic Assurance Test (BAT) completed successfully
LOAD_TEST: 0x65, // undocumented "LOAD MANUFACTURING TEST REQUEST" response code
BAT_SUCC: 0xAA, // Basic Assurance Test (BAT) completed successfully
ECHO: 0xEE,
BREAK_PREF: 0xF0,
BREAK_PREF: 0xF0, // break prefix
ACK: 0xFA,
BAT_FAIL: 0xFC, // Basic Assurance Test (BAT) failed
BAT_FAIL: 0xFC, // Basic Assurance Test (BAT) failed
DIAG_FAIL: 0xFD,
RESEND: 0xFE
};
/**
* TODO: Determine what we can do to get ALL constants like these inlined (enum doesn't seem to be getting
* the job done); the basic problem seems to be caused by referencing the properties with quotes.
* Alphanumeric and other common (printable) ASCII codes.
*
* TODO: Determine what we can do to get ALL constants like these inlined (enum doesn't seem to
* get the job done); the problem seems to be limited to property references that use quotes, which
* is why I've 'unquoted' as many of them as possible.
*
* @enum {number}
*/
@ -156,7 +159,15 @@ Keyboard.ASCII = {
};
/**
* Browser keyCodes we must pay particular attention to.
* Browser keyCodes we must pay particular attention to. For the most part, these are
* non-alphanumeric or function keys, some which may require special treatment (eg,
* preventDefault() if returning false on the initial keyDown event is insufficient).
*
* keyCodes for most common ASCII keys can simply use the appropriate ASCII code above.
*
* Most of these represent non-ASCII keys (eg, the LEFT arrow key), yet for some reason,
* browsers defined them using ASCII codes (eg, the LEFT arrow key uses the ASCII code
* for '%' or 37). This conflict is discussed further in the definition of aButtonCodes below.
*
* @enum {number}
*/
@ -194,20 +205,25 @@ Keyboard.KEYCODE = {
/* 0x7A */ F11: 122,
/* 0x7B */ F12: 123,
//
// This is a bias we add to browser keyCodes that we want to handle on "down" rather than "press"
// ONDOWN is a bias we add to browser keyCodes that we want to handle on "down" rather than "press".
//
// Note that these biases use what I'll call "Decimal Coded Binary" or DCB (the reverse of BCD),
// where decimal digits are used to represent binary bit values, which can be added together without
// affecting neighboring digits as long as you stick to 1, 2 or 4 in any given column.
//
ONDOWN: 1000,
//
// This is a bias we add to browser keyCodes that need to check for a "right" location (default is "left")
// ONRIGHT is a bias we add to browser keyCodes that need to check for a "right" location (default is "left")
//
ONRIGHT: 2000,
//
// This is a bias we add to create fake keyCodes that correspond to special keystroke sequences
// FAKE is a bias we add to signal these are fake keyCodes corresponding to internal keystroke combinations.
// The actual values are for internal use only and merely need to be unique and used consistently.
//
FAKE: 4000,
FAKE_CTRLC: 4003,
FAKE_CTRLBREAK: 4101,
FAKE_CTRLALTDEL: 4102
FAKE_CTRLBREAK: 4063,
FAKE_CTRLALTDEL: 4081
};
/**
@ -255,15 +271,16 @@ Keyboard.STATEKEYS = {
};
/**
* Browsers brilliantly define keyCodes for non-ASCII keys (eg, 37 for the LEFT arrow key) with
* values that overlap ASCII keyCodes (eg, 37 is also the ASCII code for '%'), which unnecessarily
* complicates life for keyDown/keyPress/keyUp handlers, all of which receive a keyCode property,
* but which does NOT always contain the same value for the same key press.
* In a perfect world, each one of our "button" codes would map to a unique browser keyCode.
*
* We solve the problem by adding an ONDOWN bias to all these particular keyCodes, and then make
* sure we store these keyCodes in aKeyCodes with the same bias. ONDOWN is also a signal to our
* keyCode handlers that the key in question should be handled during keyDown, not keyPress, often
* because they don't generate a keyPress event anyway.
* However, because most of these codes are for non-ASCII keys, which browsers brilliantly
* map to ASCII keyCodes that conflict with *actual* ASCII keys, we must add an ONDOWN bias
* to all these particular keyCodes, and make sure we store these keyCodes in our aKeyCodes
* lookup table with the same bias.
*
* The good news is that ONDOWN also serves as a signal to our keyCode handlers that the key
* in question should be handled during keyDown (not keyPress), since most if not all of these
* non-alphanumeric keys don't generate a keyPress event anyway.
*
* @enum {number}
*/
@ -298,19 +315,21 @@ Keyboard.aButtonCodes = {
};
/**
* Define "soft keyboard" identifiers for all possible keys, based on their primary (unshifted) character
* or function. This also serves as a definition of all supported scan codes.
* Define identifiers for all possible keys, based on their primary (unshifted) character or function.
* This also serves as a definition of all supported scan codes, making it possible to create full-featured
* "soft keyboards".
*
* One exception to the above rule is 'prtsc': on the original IBM 83-key and 84-key keyboards, its primary
* (unshifted) character was '*', but on 101-key keyboards, it became a separate key ('prtsc', now labeled
* 'Print Screen'), as did the num-pad '*' ('num-mul'), so 'prtsc' seems worthy of an exception to the rule.
* One exception to the (unshifted) rule above is 'prtsc': on the original IBM 83-key and 84-key keyboards,
* its primary (unshifted) character was '*', but on 101-key keyboards, it became a separate key ('prtsc',
* now labeled "Print Screen"), as did the num-pad '*' ('num-mul'), so 'prtsc' seems worthy of an exception
* to the rule.
*
* On 83-key and 84-key keyboards, 'ctrl'+'num-lock' triggered a "pause" operation and 'ctrl'+'scroll-lock'
* triggered a "break" operation (I'm using double-quotes to describe the operations and single-quotes to
* describe the keys).
* triggered a "break" operation.
*
* On 101-key keyboards, IBM decided to move both those special operations to a new 'pause' key, alongside
* the dedicated 'prtsc' ('Print Screen') key. Those keys behaved as follows:
* On 101-key keyboards, IBM decided to move both those special operations to a new 'pause' ("Pause/Break")
* key, near the new dedicated 'prtsc' ("Print Screen/SysRq") key -- and to drop the "e" from "SysReq".
* Those keys behave as follows:
*
* When 'pause' is pressed alone, it generates 0xe1 0x1d 0x45 0xe1 0x9d 0xc5 on make (nothing on break),
* which essentially simulates the make-and-break of the 'ctrl' and 'num-lock' keys (ignoring the 0xe1),
@ -774,7 +793,7 @@ Keyboard.prototype.resetDevice = function()
*/
this.messageDebugger("keyboard reset", Debugger.MESSAGE_PORT);
this.abScanBuffer = [Keyboard.CMDRES.BAT_SUCC];
if (this.chipset) this.chipset.setIRR(ChipSet.IRQ.KBD, 4);
if (this.chipset) this.chipset.notifyKbdData(true);
};
/**
@ -847,7 +866,8 @@ Keyboard.prototype.sendCmd = function(bCmd)
/**
* readScanCode(fShift)
*
* This is the ChipSet's interface for reading scan codes.
* This is the ChipSet's interface for reading scan codes. This also doubles as the ChipSet's interface for checking
* whether or not any data is available.
*
* @this {Keyboard}
* @param {boolean} [fShift] is used by the MODEL_5170 8042 Keyboard Controller (supersedes the old setEnable() interface)
@ -889,7 +909,7 @@ Keyboard.prototype.shiftScanCode = function(fFlush)
*/
this.abScanBuffer.shift();
if (this.abScanBuffer.length > 0) {
if (this.chipset) this.chipset.setIRR(ChipSet.IRQ.KBD);
if (this.chipset) this.chipset.notifyKbdData(true);
}
}
}
@ -950,7 +970,7 @@ Keyboard.prototype.reset = function()
/*
* The physical (not virtual) state of various shift keys.
*
* QUESTION: In JavaScript, how do you query initial key states?
* TODO: Determine how (or whether) we can query the browser's initial key states.
*/
this.bitsShift = 0;
@ -960,15 +980,13 @@ Keyboard.prototype.reset = function()
this.abScanBuffer = [];
/*
* When a key "down" is simulated on behalf of some keyCode, I save
* the timer object responsible for simulating the key "up" here, so that
* if I detect the actual key going up sooner, I can cancel the timer and
* simulate the "up" immediately. Similarly, if another press for the same
* key arrives before last one expired (eg, auto-repeat), I need to cancel
* the previous timer for that key before setting another.
* When a key "down" is simulated on behalf of some keyCode, I save the timer object responsible for
* simulating the key "up" here, so that if I detect the actual key going up sooner, I can cancel the
* timer and simulate the "up" immediately. Similarly, if another press for the same key arrives before
* last one expired (eg, auto-repeat), I need to cancel the previous timer for that key before setting another.
*
* NOTE: If this is anything other than an initial reset, then we need to
* make sure there are no outstanding timers before we blow the array away.
* NOTE: If this is anything other than an initial reset, then we need to make sure there are no outstanding
* timers before we blow the array away.
*/
if (this.aKeyTimers) {
for (var i in this.aKeyTimers) {
@ -981,8 +999,7 @@ Keyboard.prototype.reset = function()
this.prevKeyDown = 0;
/*
* Make sure the auto-injection buffer is empty, too (an injection could have been
* in progress on any reset after the first).
* Make sure the auto-injection buffer is empty (an injection could have been in progress on any reset after the first).
*/
this.sInjectBuffer = "";
};
@ -1065,8 +1082,8 @@ Keyboard.prototype.setSoftKeyState = function(control, f)
*
* An actual IBM keyboard will only buffer up to 20 scan codes, so we impose the same limit here.
*
* Just as 0xAA is a special scan code response to a software reset, 0xFF is a special scan code response
* to an internal buffer overrun. I try to simulate both.
* Just as 0xAA is a special scan code response to a software reset, 0xFF is a special scan code response to
* an internal buffer overrun. I try to simulate both. TODO: Define and document these limits and special codes.
*
* @this {Keyboard}
* @param {number} bScan
@ -1093,7 +1110,7 @@ Keyboard.prototype.addScanCode = function(bScan, fRepeat)
this.messageDebugger("scan code 0x" + str.toHexByte(bScan) + " buffered");
this.abScanBuffer.push(bScan);
if (this.abScanBuffer.length == 1) {
if (this.chipset) this.chipset.setIRR(ChipSet.IRQ.KBD);
if (this.chipset) this.chipset.notifyKbdData(true);
}
this.findBinding(bKey, "key", fDown);
return;

13
my_modules/pcjs-client/lib/mem.js
View file

@ -84,12 +84,13 @@ if (typeof module !== 'undefined') {
* Because Memory blocks now allow us to have a "sparse" address space, we could choose to
* take the memory hit of allocating 4K arrays per block, where each element stores only one byte,
* instead of the more frugal but slightly slower approach of allocating arrays of 32-bit dwords
* and shifting/masking bytes/words to/from dwords; in theory, byte accesses would be faster and
* word accesses somewhat less faster. However, preliminary testing of that feature (FATARRAYS)
* did not yield significantly faster performance, so it is OFF by default to minimize our memory
* consumption. Using TYPEDARRAYS is probably best, although not all JavaScript implementations
* support them (IE9 is probably the only real outlier: it lacks typed arrays but otherwise has
* all the necessary HTML5 support).
* (DWORDARRAYS) and shifting/masking bytes/words to/from dwords; in theory, byte accesses would
* be faster and word accesses somewhat less faster.
*
* However, preliminary testing of that feature (FATARRAYS) did not yield significantly faster
* performance, so it is OFF by default to minimize our memory consumption. Using TYPEDARRAYS is
* probably best, although not all JavaScript implementations support them (IE9 is probably the
* only real outlier: it lacks typed arrays but otherwise has all the necessary HTML5 support).
*
* @constructor
* @param {number} addr of block (must be some multiple of bus.blockSize)