Correct BRK behaviour

IRQ/NMI clear the BRK flag
BRK is Non-Maskable

src/main/java/com/loomcom/symon/Cpu.java

@@ -260,9 +260,8 @@ public class Cpu implements InstructionTable {
 
             /** Single Byte Instructions; Implied and Relative **/
             case 0x00: // BRK - Force Interrupt - Implied
-                if (!getIrqDisableFlag()) {
+                // BRK is non-maskable. - http://www.atarimax.com/jindroush.atari.org/aopc.html#BRK
-                    handleIrq(state.pc + 1);
+                handleBrk(state.pc + 1);
-                }
                 break;
             case 0x08: // PHP - Push Processor Status - Implied
                 // Break flag is always set in the stack value.
@@ -734,13 +733,18 @@ public class Cpu implements InstructionTable {
         delayLoop(state.ir);
     }
 
+    private void handleBrk(int returnPc) throws MemoryAccessException {
+        handleInterrupt(returnPc, IRQ_VECTOR_L, IRQ_VECTOR_H, true);
+        clearIrq();
+    }
+
     private void handleIrq(int returnPc) throws MemoryAccessException {
-        handleInterrupt(returnPc, IRQ_VECTOR_L, IRQ_VECTOR_H);
+        handleInterrupt(returnPc, IRQ_VECTOR_L, IRQ_VECTOR_H, false);
         clearIrq();
     }
 
     private void handleNmi() throws MemoryAccessException {
-        handleInterrupt(state.pc, NMI_VECTOR_L, NMI_VECTOR_H);
+        handleInterrupt(state.pc, NMI_VECTOR_L, NMI_VECTOR_H, false);
         clearNmi();
     }
 
@@ -749,9 +753,19 @@ public class Cpu implements InstructionTable {
      *
      * @throws MemoryAccessException
      */
-    private void handleInterrupt(int returnPc, int vectorLow, int vectorHigh) throws MemoryAccessException {
+    private void handleInterrupt(int returnPc, int vectorLow, int vectorHigh, boolean isBreak) throws MemoryAccessException {
-        // Set the break flag before pushing.
+
-        setBreakFlag();
+        if (isBreak)
+        {
+            // Set the break flag before pushing.
+            setBreakFlag();
+        }
+        else
+        {
+            // IRQ & NMI clear break flag - http://www.pagetable.com/?p=410
+            clearBreakFlag();
+        }
+
         // Push program counter + 1 onto the stack
         stackPush((returnPc >> 8) & 0xff); // PC high byte
         stackPush(returnPc & 0xff);        // PC low byte

src/test/java/com/loomcom/symon/CpuImpliedModeTest.java

@@ -117,9 +117,22 @@ public class CpuImpliedModeTest {
     }
 
     @Test
-    public void test_BRK_HonorsIrqDisableFlag() throws MemoryAccessException {
+    public void test_BRK_IgnoresIrqDisableFlag() throws MemoryAccessException {
         cpu.setIrqDisableFlag();
 
+        cpu.setCarryFlag();
+        cpu.setOverflowFlag();
+        assertEquals(0x20 | Cpu.P_CARRY | Cpu.P_OVERFLOW | Cpu.P_IRQ_DISABLE,
+                     cpu.getProcessorStatus());
+        assertEquals(0x00, cpu.stackPeek());
+        assertFalse(cpu.getBreakFlag());
+        assertEquals(0x0200, cpu.getProgramCounter());
+        assertEquals(0xff, cpu.getStackPointer());
+        
+        // Set the IRQ vector
+        bus.write(0xffff, 0x12);
+        bus.write(0xfffe, 0x34);
+
         bus.loadProgram(0xea,
                         0xea,
                         0xea,
@@ -135,19 +148,23 @@ public class CpuImpliedModeTest {
         // of the Interrupt Disable flag
         cpu.step();
 
-        // Reset to original contents of PC
-        assertEquals(0x0204, cpu.getProgramCounter());
-        // Empty stack
-        assertEquals(0xff, cpu.getStackPointer());
 
-        cpu.step(2); // Two more NOPs
+        // Was at PC = 0x204.  PC+1 should now be on the stack
+        assertEquals(0x02, bus.read(0x1ff)); // PC high byte
+        assertEquals(0x05, bus.read(0x1fe)); // PC low byte
+        assertEquals(0x20 | Cpu.P_CARRY | Cpu.P_OVERFLOW | Cpu.P_BREAK | Cpu.P_IRQ_DISABLE,
+                     bus.read(0x1fd));       // Processor Status, with B set
 
-        // Reset to original contents of PC
+        // Interrupt vector held 0x1234, so we should be there.
-        assertEquals(0x0206, cpu.getProgramCounter());
+        assertEquals(0x1234, cpu.getProgramCounter());
-        // Empty stack
+        assertEquals(0xfc, cpu.getStackPointer());
-        assertEquals(0xff, cpu.getStackPointer());
+
+        // B and I flags should have been set on P
+        assertEquals(0x20 | Cpu.P_CARRY | Cpu.P_OVERFLOW | Cpu.P_BREAK | Cpu.P_IRQ_DISABLE,
+                     cpu.getProcessorStatus());
     }
 
+
     /* CLC - Clear Carry Flag - $18 */
     @Test
     public void test_CLC() throws MemoryAccessException {
@@ -625,4 +642,4 @@ public class CpuImpliedModeTest {
         assertFalse(cpu.getZeroFlag());
         assertTrue(cpu.getNegativeFlag());
     }
-}
+}

src/test/java/com/loomcom/symon/CpuTest.java

@@ -383,6 +383,41 @@ public class CpuTest extends TestCase {
         assertFalse(cpu.getCpuState().irqAsserted);
     }
 
+    public void testIrqDoesNotSetBRK() throws Exception {
+        // Ensure the IRQ disable flag is cleared
+        cpu.clearIrqDisableFlag();
+
+        // Set the IRQ vector
+        bus.write(0xffff, 0x12);
+        bus.write(0xfffe, 0x34);
+
+        // Create an IRQ handler at 0x1234
+        cpu.setProgramCounter(0x1234);
+        bus.loadProgram(0xa9, 0x33,  // LDA #$33
+                        0x69, 0x01); // ADC #$01
+
+        cpu.setProgramCounter(0x0200);
+        // Create a little program at 0x0200
+        bus.loadProgram(0x18,        // CLC
+                        0xa9, 0x01,  // LDA #$00
+                        0x69, 0x01); // ADC #$01
+
+        cpu.step();
+        assertEquals(0x0201, cpu.getProgramCounter()); // First instruction executed.
+        assertEquals(0x00, cpu.getAccumulator());
+
+        cpu.step();
+        assertEquals(0x0203, cpu.getProgramCounter());
+        assertEquals(0x01, cpu.getAccumulator());
+
+        cpu.assertIrq();
+
+        cpu.step();
+        assertTrue(cpu.getIrqDisableFlag()); // Should have been set by the IRQ
+        assertFalse(cpu.getBreakFlag());
+
+    }
+
     public void testIrqHonorsIrqDisabledFlag() throws Exception {
         // Ensure the IRQ disable flag is set
         cpu.setIrqDisableFlag();
@@ -497,6 +532,37 @@ public class CpuTest extends TestCase {
         assertFalse(cpu.getCpuState().nmiAsserted);
     }
 
+    public void testNmiDoesNotSetBRK() throws Exception {
+        // Set the NMI vector to 0x1000
+        bus.write(0xfffb, 0x10);
+        bus.write(0xfffa, 0x00);
+
+        // Create an NMI handler at 0x1000
+        cpu.setProgramCounter(0x1000);
+        bus.loadProgram(0xa9, 0x33,  // LDA #$33
+                        0x69, 0x01); // ADC #$01
+
+        // Create a little program at 0x0200
+        cpu.setProgramCounter(0x0200);
+        bus.loadProgram(0x18,        // CLC
+                        0xa9, 0x01,  // LDA #$00
+                        0x69, 0x01); // ADC #$01
+
+        cpu.step();
+        assertEquals(0x0201, cpu.getProgramCounter()); // First instruction executed.
+        assertEquals(0x00, cpu.getAccumulator());
+
+        cpu.step();
+        assertEquals(0x0203, cpu.getProgramCounter());
+        assertEquals(0x01, cpu.getAccumulator());
+
+        cpu.assertNmi();
+
+        cpu.step();
+        assertFalse(cpu.getBreakFlag());
+
+}
+
     public void testNmiIgnoresIrqDisableFlag() throws Exception {
         // Set the IRQ disable flag, which should be ignored by the NMI
         cpu.setIrqDisableFlag();