From ca33e408f102b74515a16440f7df3791b978ea47 Mon Sep 17 00:00:00 2001 From: steve Date: Sat, 7 Sep 2019 11:31:35 +0100 Subject: [PATCH] o CPU - reworked index bug handling in CPU - fixed slo opcode defintion (wrong addressing mode specified) o debugger - added BUG option to CPU command - debugger now optionally prints CPU BUG messages on the step it occurs --- debugger/commands.go | 18 +++++++- debugger/debugger.go | 10 +++++ errors/messages.go | 2 +- gopher2600.go | 8 +++- hardware/cpu/cpu.go | 41 +++++++++++++------ .../definitions/generator/instructions.csv | 2 +- hardware/cpu/definitions/instructions.go | 2 +- hardware/cpu/result/validity.go | 27 +++++++++--- hardware/vcs.go | 12 +++++- 9 files changed, 96 insertions(+), 26 deletions(-) diff --git a/debugger/commands.go b/debugger/commands.go index 3c84cf61..12c2cca0 100644 --- a/debugger/commands.go +++ b/debugger/commands.go @@ -70,7 +70,7 @@ const cmdHelp = "HELP" var commandTemplate = []string{ cmdBall, cmdBreak + " [%S %N|%N] {& %S %N|& %N}", - cmdCPU + " (SET [PC|A|X|Y|SP] [%N])", + cmdCPU + " (SET [PC|A|X|Y|SP] [%N]|BUG (ON|OFF))", cmdCartridge + " (ANALYSIS|BANK %N)", cmdClear + " [BREAKS|TRAPS|WATCHES|ALL]", cmdDebuggerState, @@ -752,6 +752,22 @@ func (dbg *Debugger) enactCommand(tokens *commandline.Tokens, interactive bool) reg.Load(v) + case "BUG": + option, _ := tokens.Get() + + switch strings.ToUpper(option) { + case "ON": + dbg.reportCPUBugs = true + case "OFF": + dbg.reportCPUBugs = false + } + + if dbg.reportCPUBugs { + dbg.print(console.StyleFeedback, "CPU bug reporting: ON") + } else { + dbg.print(console.StyleFeedback, "CPU bug reporting: OFF") + } + default: // already caught by command line ValidateTokens() } diff --git a/debugger/debugger.go b/debugger/debugger.go index 22767167..f4a46ebf 100644 --- a/debugger/debugger.go +++ b/debugger/debugger.go @@ -91,6 +91,11 @@ type Debugger struct { // machineInfoVerbose controls the verbosity of commands that echo machine state machineInfoVerbose bool + // whether to display the triggering of a known CPU bug. these are bugs + // that are known about in the emulated hardware but which might catch an + // unwary programmer by surprise + reportCPUBugs bool + // input loop fields. we're storing these here because inputLoop can be // called from within another input loop (via a video step callback) and we // want these properties to persist (when a video step input loop has @@ -291,6 +296,11 @@ func (dbg *Debugger) videoCycle(result *result.Instruction) error { if result.Defn.Effect == definitions.Flow || result.Defn.Effect == definitions.Subroutine || result.Defn.Effect == definitions.Interrupt { return nil } + + // display information about any CPU bugs that may have been triggered + if dbg.reportCPUBugs && result.Bug != "" { + dbg.print(console.StyleMachineInfo, result.Bug) + } } dbg.breakMessages = dbg.breakpoints.check(dbg.breakMessages) diff --git a/errors/messages.go b/errors/messages.go index 24ba4085..c5427392 100644 --- a/errors/messages.go +++ b/errors/messages.go @@ -50,7 +50,7 @@ var messages = map[Errno]string{ // cpu UnimplementedInstruction: "cpu error: unimplemented instruction (%0#x) at (%#04x)", InvalidOpcode: "cpu error: invalid opcode (%#04x)", - InvalidResult: "cpu error: %s: %s", + InvalidResult: "cpu error: %s", ProgramCounterCycled: "cpu error: program counter cycled back to 0x0000", InvalidOperationMidInstruction: "cpu error: invalid operation mid-instruction (%s)", diff --git a/gopher2600.go b/gopher2600.go index 4b55c174..4c97b8d4 100644 --- a/gopher2600.go +++ b/gopher2600.go @@ -199,7 +199,7 @@ func main() { runner := func() error { err := dbg.Start(term, *initScript, modeFlags.Arg(0)) if err != nil { - return errors.NewFormattedError(errors.PerformanceError, err) + return err } return nil } @@ -216,7 +216,11 @@ func main() { os.Exit(2) } } else { - runner() + err := runner() + if err != nil { + fmt.Printf("* %s\n", err) + os.Exit(2) + } } default: fmt.Printf("* too many arguments for %s mode\n", mode) diff --git a/hardware/cpu/cpu.go b/hardware/cpu/cpu.go index 4b915d2f..d1f274c6 100644 --- a/hardware/cpu/cpu.go +++ b/hardware/cpu/cpu.go @@ -471,12 +471,16 @@ func (mc *CPU) ExecuteInstruction(cycleCallback func(*result.Instruction) error) if err != nil { return nil, err } + result.InstructionData = indirectAddress adder := register.NewAnonRegister(indirectAddress, 8) adder.Add(mc.X, false) address = adder.ToUint16() - // !!TODO: zero page index bug + // handle zero page index bug + if (uint16(indirectAddress)+mc.X.ToUint16())&0xff00 != uint16(indirectAddress)&0xff00 { + result.Bug = fmt.Sprintf("zero page index bug") + } // +1 cycle err = mc.endCycle() @@ -497,7 +501,10 @@ func (mc *CPU) ExecuteInstruction(cycleCallback func(*result.Instruction) error) adder.Add(mc.Y, false) address = adder.ToUint16() - // !!TODO: zero page index bug + // handle zero page index bug + if (uint16(indirectAddress)+mc.Y.ToUint16())&0xff00 != uint16(indirectAddress)&0xff00 { + result.Bug = fmt.Sprintf("zero page index bug") + } // +1 cycle err = mc.endCycle() @@ -515,7 +522,7 @@ func (mc *CPU) ExecuteInstruction(cycleCallback func(*result.Instruction) error) } result.InstructionData = indirectAddress - // implement NMOS 6502 Indirect JMP bug + // handle indirect addressing JMP bug if indirectAddress&0x00ff == 0x00ff { result.Bug = fmt.Sprintf("indirect addressing bug (JMP bug)") @@ -530,6 +537,10 @@ func (mc *CPU) ExecuteInstruction(cycleCallback func(*result.Instruction) error) return nil, err } + // in this bug path, the lower byte of the indirect address is on a + // page boundary. because of the bug we must read high byte of JMP + // address from the zero byte of the same page (rather than the + // zero byte of the next page) hi, err := mc.mem.Read(indirectAddress & 0xff00) if err != nil { return nil, err @@ -573,8 +584,8 @@ func (mc *CPU) ExecuteInstruction(cycleCallback func(*result.Instruction) error) adder := register.NewAnonRegister(mc.X, 8) adder.Add(indirectAddress, false) - // indirect addressing / page boundary bug - if uint16(indirectAddress)+mc.X.ToUint16() > 0x00ff { + // note whether indirect addressing / page boundary bug has occurred + if (uint16(indirectAddress)+mc.X.ToUint16())&0xff00 != uint16(indirectAddress)&0xff00 { result.Bug = fmt.Sprintf("indirect addressing bug") } @@ -1296,9 +1307,13 @@ func (mc *CPU) ExecuteInstruction(cycleCallback func(*result.Instruction) error) mc.Status.Sign = mc.A.IsNegative() case "slo": - var r *register.Register - r = register.NewAnonRegister(value, mc.A.Size()) - mc.Status.Carry = r.ROL(mc.Status.Carry) + // the slo opcode starts off with an ASL operation + // all versions of this opcode are RMW so we always work with + // the anonymous register + r := register.NewAnonRegister(value, mc.A.Size()) + mc.Status.Carry = r.ASL() + mc.Status.Zero = r.IsZero() + mc.Status.Sign = r.IsNegative() value = r.ToUint8() mc.A.ORA(value) mc.Status.Zero = mc.A.IsZero() @@ -1311,16 +1326,16 @@ func (mc *CPU) ExecuteInstruction(cycleCallback func(*result.Instruction) error) // for RMW instructions: write altered value back to memory if defn.Effect == definitions.RMW { - err = mc.write8Bit(address, value) - if err != nil { - return nil, err - - } // +1 cycle err = mc.endCycle() if err != nil { return nil, err } + + err = mc.write8Bit(address, value) + if err != nil { + return nil, err + } } // finalise result diff --git a/hardware/cpu/definitions/generator/instructions.csv b/hardware/cpu/definitions/generator/instructions.csv index 224be359..31a6f5d4 100644 --- a/hardware/cpu/definitions/generator/instructions.csv +++ b/hardware/cpu/definitions/generator/instructions.csv @@ -261,5 +261,5 @@ 0xcb, sax, 2, IMMEDIATE, False 0x6b, arr, 2, IMMEDIATE, False -0x03, slo, 8, INDEXED_ZERO_PAGE_X, False, RMW # aso +0x03, slo, 8, PRE_INDEX_INDIRECT, False, RMW # aso 0x07, slo, 5, ZERO_PAGE, False, RMW # aso diff --git a/hardware/cpu/definitions/instructions.go b/hardware/cpu/definitions/instructions.go index b46d9f24..a134d3a9 100644 --- a/hardware/cpu/definitions/instructions.go +++ b/hardware/cpu/definitions/instructions.go @@ -8,7 +8,7 @@ return []*InstructionDefinition{ &InstructionDefinition{ObjectCode:0x0, Mnemonic:"BRK", Bytes:1, Cycles:7, AddressingMode:0, PageSensitive:false, Effect:5}, &InstructionDefinition{ObjectCode:0x1, Mnemonic:"ORA", Bytes:2, Cycles:6, AddressingMode:6, PageSensitive:false, Effect:0}, nil, -&InstructionDefinition{ObjectCode:0x3, Mnemonic:"slo", Bytes:2, Cycles:8, AddressingMode:10, PageSensitive:false, Effect:2}, +&InstructionDefinition{ObjectCode:0x3, Mnemonic:"slo", Bytes:2, Cycles:8, AddressingMode:6, PageSensitive:false, Effect:2}, &InstructionDefinition{ObjectCode:0x4, Mnemonic:"dop", Bytes:2, Cycles:3, AddressingMode:4, PageSensitive:false, Effect:0}, &InstructionDefinition{ObjectCode:0x5, Mnemonic:"ORA", Bytes:2, Cycles:3, AddressingMode:4, PageSensitive:false, Effect:0}, &InstructionDefinition{ObjectCode:0x6, Mnemonic:"ASL", Bytes:2, Cycles:5, AddressingMode:4, PageSensitive:false, Effect:2}, diff --git a/hardware/cpu/result/validity.go b/hardware/cpu/result/validity.go index f23579bb..55da3e2b 100644 --- a/hardware/cpu/result/validity.go +++ b/hardware/cpu/result/validity.go @@ -33,19 +33,34 @@ func (result Instruction) IsValid() error { if result.Bug == "" { if result.Defn.AddressingMode == definitions.Relative { if result.ActualCycles != result.Defn.Cycles && result.ActualCycles != result.Defn.Cycles+1 && result.ActualCycles != result.Defn.Cycles+2 { - msg := fmt.Sprintf("number of cycles wrong (%d instead of %d, %d or %d)", result.ActualCycles, result.Defn.Cycles, result.Defn.Cycles+1, result.Defn.Cycles+2) - return errors.NewFormattedError(errors.InvalidResult, msg, result) + msg := fmt.Sprintf("number of cycles wrong for opcode %#02x [%s] (%d instead of %d, %d or %d)", + result.Defn.ObjectCode, + result.Defn.Mnemonic, + result.ActualCycles, + result.Defn.Cycles, + result.Defn.Cycles+1, + result.Defn.Cycles+2) + return errors.NewFormattedError(errors.InvalidResult, msg) } } else { if result.Defn.PageSensitive { if result.PageFault && result.ActualCycles != result.Defn.Cycles && result.ActualCycles != result.Defn.Cycles+1 { - msg := fmt.Sprintf("number of cycles wrong (actual %d instead of %d or %d)", result.ActualCycles, result.Defn.Cycles, result.Defn.Cycles+1) - return errors.NewFormattedError(errors.InvalidResult, msg, result) + msg := fmt.Sprintf("number of cycles wrong for opcode %#02x [%s] (%d instead of %d, %d)", + result.Defn.ObjectCode, + result.Defn.Mnemonic, + result.ActualCycles, + result.Defn.Cycles, + result.Defn.Cycles+1) + return errors.NewFormattedError(errors.InvalidResult, msg) } } else { if result.ActualCycles != result.Defn.Cycles { - msg := fmt.Sprintf("number of cycles wrong (actual %d instead of %d)", result.ActualCycles, result.Defn.Cycles) - return errors.NewFormattedError(errors.InvalidResult, msg, result) + msg := fmt.Sprintf("number of cycles wrong for opcode %#02x [%s] (%d instead of %d)", + result.Defn.ObjectCode, + result.Defn.Mnemonic, + result.ActualCycles, + result.Defn.Cycles) + return errors.NewFormattedError(errors.InvalidResult, msg) } } } diff --git a/hardware/vcs.go b/hardware/vcs.go index 0c6ccd56..d420f1da 100644 --- a/hardware/vcs.go +++ b/hardware/vcs.go @@ -310,10 +310,20 @@ func (vcs *VCS) Run(continueCheck func() (bool, error)) error { cont := true for cont { - _, err = vcs.CPU.ExecuteInstruction(videoCycle) + var r *result.Instruction + r, err = vcs.CPU.ExecuteInstruction(videoCycle) if err != nil { return err } + + // check validity of result + if r != nil { + err = r.IsValid() + if err != nil { + return err + } + } + cont, err = continueCheck() }