Fix bug where jumping to the PC would still advance the PC

src/cpu.rs

@@ -33,8 +33,10 @@ impl Processor {
         let (instruction, bytes_read) =
             parse::next_instruction(&memory_view).expect("invalid instruction");
 
+        // Must advance the PC before running the instruction to ensure CALLs/JUMPs operate on the correct addresses
+        self.registers.program_counter = pc.wrapping_add(bytes_read);
+
         let interrupt_enable_pending_already = self.interrupt_enable_pending;
-        let pc_before_instruction_run = self.registers.program_counter;
         self.run_instruction(instruction);
 
         // EI does not work until after the following instruction, so we check after running an instruction if
@@ -43,13 +45,6 @@ impl Processor {
             self.interrupt_enable_pending = false;
             self.registers.enable_interrupts();
         }
-
-        if pc_before_instruction_run == self.registers.program_counter {
-            // The program counter should be advanced *ONLY* if the instruction did not modify the PC register
-
-            let next_pc = pc.wrapping_add(bytes_read);
-            self.registers.program_counter = next_pc;
-        }
     }
 
     /// Run a single instruction on the CPU.

src/cpu/run/control.rs

@@ -49,8 +49,7 @@ impl Run for ControlFlowInstruction {
                     .registers
                     .get_single_16bit_register(register::SingleSixteenBit::ProgramCounter);
 
-                // We know the rest instruction to be 1 byte. A bit of a hack, but eh.
+                processor.push_16bit_value_to_stack(current_pc)?;
-                processor.push_16bit_value_to_stack(current_pc.wrapping_add(1))?;
 
                 processor
                     .registers
@@ -89,8 +88,7 @@ fn do_call(processor: &mut Processor, to_addr: u16) -> Result<(), super::Error>
         .registers
         .get_single_16bit_register(register::SingleSixteenBit::ProgramCounter);
 
-    // We know the call instruction to be 3 bytes. A bit of a hack, but eh.
+    processor.push_16bit_value_to_stack(current_pc)?;
-    processor.push_16bit_value_to_stack(current_pc.wrapping_add(3))?;
 
     jump_to(processor, to_addr);
 

tests/cpu/control.rs

@@ -64,8 +64,9 @@ fn test_call_pushes_pc_onto_stack() {
     processor.run_instruction(ins);
 
     assert_eq!(0xBE, processor.memory.get((old_sp - 1).into()).unwrap());
-    // 0xEF + 3, which is the size of the CALL instruction
+    // HACK: This SHOULD be F2 in reality (0xEF + 3, the size of the call instruction)
-    assert_eq!(0xF2, processor.memory.get((old_sp - 2).into()).unwrap());
+    // however since run_instruction does not advance the PC, we need to adjust our test
+    assert_eq!(0xEF, processor.memory.get((old_sp - 2).into()).unwrap());
     assert_eq!(old_sp - 2, processor.registers.stack_pointer);
 }
 
@@ -111,8 +112,9 @@ fn test_restart_adjusts_pushes_current_program_counter_to_stack(opcode: u8) {
     processor.run_instruction(ins);
 
     assert_eq!(0xDE, processor.memory.get((old_sp - 1).into()).unwrap());
-    // 0xAD + 1, which is the size of the RST instruction
+    // HACK: This SHOULD be AE in reality (0xAD + 1, the size of the rst instruction)
-    assert_eq!(0xAE, processor.memory.get((old_sp - 2).into()).unwrap());
+    // however since run_instruction does not advance the PC, we need to adjust our test
+    assert_eq!(0xAD, processor.memory.get((old_sp - 2).into()).unwrap());
     assert_eq!(old_sp - 2, processor.registers.stack_pointer);
 }
 
@@ -185,6 +187,29 @@ fn test_call_and_jump_does_nothing_if_condition_fails(opcode: u8, flag: Flag, ex
     assert_eq!(old_pc, processor.registers.program_counter);
 }
 
+#[test]
+fn test_jump_to_same_addr_does_not_advance_pc() {
+    let mut processor = Processor::default();
+    processor.registers.program_counter = 0x1337;
+
+    [0xC3, 0x37, 0x13]
+        .iter()
+        .copied()
+        .enumerate()
+        .for_each(|(idx, opcode)| {
+            processor
+                .memory
+                .set(
+                    usize::from(processor.registers.program_counter) + idx,
+                    opcode,
+                )
+                .expect("could not program data");
+        });
+
+    processor.run_next_instruction();
+    assert_eq!(0x1337, processor.registers.program_counter);
+}
+
 #[test_case(0xC9; "RET")]
 #[test_case(0xD9; "RETI")]
 fn test_ret_jumps_to_address_on_stack(opcode: u8) {