From f39d1d33e39f0dc03d951e7f0a020ce30f889b2f Mon Sep 17 00:00:00 2001 From: steve Date: Sat, 28 Dec 2019 11:18:01 +0000 Subject: [PATCH] o debugger - removed REFLECT ON/OFF system - the performance hit is not that great o cpu - added backward branching test - added branching page fault test o debugger_test - increased timeout for rcvOutput() --- debugger/commands.go | 28 ---------- debugger/debugger.go | 1 - debugger/debugger_test.go | 2 +- debugger/events.go | 6 -- debugger/help.go | 1 - debugger/reflection/reflection.go | 17 ------ debugger/targets.go | 5 +- errors/messages.go | 13 ++--- hardware/cpu/cpu_test.go | 56 ++++++++++++++++++- hardware/cpu/registers/program_counter.go | 5 ++ hardware/cpu/registers/status.go | 2 - .../cpu/registers/test/equate_registers.go | 6 +- 12 files changed, 71 insertions(+), 71 deletions(-) diff --git a/debugger/commands.go b/debugger/commands.go index cb26330e..4f254f01 100644 --- a/debugger/commands.go +++ b/debugger/commands.go @@ -40,7 +40,6 @@ const ( cmdLast = "LAST" cmdList = "LIST" cmdMemMap = "MEMMAP" - cmdReflect = "REFLECT" cmdMissile = "MISSILE" cmdOnHalt = "ONHALT" cmdOnStep = "ONSTEP" @@ -89,7 +88,6 @@ var commandTemplate = []string{ cmdLast + " (DEFN|BYTECODE)", cmdList + " [BREAKS|TRAPS|WATCHES|ALL]", cmdMemMap, - cmdReflect + " (ON|OFF)", cmdMissile + " (0|1)", cmdOnHalt + " (OFF|ON|%S {%S})", cmdOnStep + " (OFF|ON|%S {%S})", @@ -642,28 +640,6 @@ func (dbg *Debugger) enactCommand(tokens *commandline.Tokens, interactive bool) case cmdMemMap: dbg.print(terminal.StyleInstrument, "%v", memorymap.Summary()) - case cmdReflect: - option, _ := tokens.Get() - switch strings.ToUpper(option) { - case "OFF": - err := dbg.scr.SetFeature(gui.ReqSetOverlay, false) - if err != nil { - dbg.print(terminal.StyleError, err.Error()) - } - dbg.relfectMonitor.Activate(false) - case "ON": - err := dbg.scr.SetFeature(gui.ReqSetOverlay, true) - if err != nil { - dbg.print(terminal.StyleError, err.Error()) - } - dbg.relfectMonitor.Activate(true) - } - if dbg.relfectMonitor.IsActive() { - dbg.print(terminal.StyleEmulatorInfo, "reflection: ON") - } else { - dbg.print(terminal.StyleEmulatorInfo, "reflection: OFF") - } - case cmdExit: fallthrough @@ -1135,10 +1111,6 @@ func (dbg *Debugger) enactCommand(tokens *commandline.Tokens, interactive bool) } } case "OVERLAY": - if !dbg.relfectMonitor.IsActive() { - return doNothing, errors.New(errors.ReflectionNotRunning) - } - action, _ := tokens.Get() action = strings.ToUpper(action) switch action { diff --git a/debugger/debugger.go b/debugger/debugger.go index 7f23e098..fee37b72 100644 --- a/debugger/debugger.go +++ b/debugger/debugger.go @@ -147,7 +147,6 @@ func NewDebugger(tv television.Television, scr gui.GUI, term terminal.Terminal) // set up reflection monitor dbg.relfectMonitor = reflection.NewMonitor(dbg.vcs, dbg.scr) - dbg.relfectMonitor.Activate(true) // set up breakpoints/traps dbg.breakpoints = newBreakpoints(dbg) diff --git a/debugger/debugger_test.go b/debugger/debugger_test.go index 19a90050..a62f9527 100644 --- a/debugger/debugger_test.go +++ b/debugger/debugger_test.go @@ -120,7 +120,7 @@ func (trm *mockTerm) rcvOutput() { // the amount of output sent by the debugger is unpredictable so a // timeout is necessary. a matter of milliseconds should be sufficient - case <-time.After(1 * time.Millisecond): + case <-time.After(10 * time.Millisecond): empty = true } } diff --git a/debugger/events.go b/debugger/events.go index 6f2b3a69..136647df 100644 --- a/debugger/events.go +++ b/debugger/events.go @@ -33,12 +33,6 @@ func (dbg *Debugger) guiEventHandler(event gui.Event) error { err = dbg.scr.SetFeature(gui.ReqToggleAltColors) case "2": // toggle overlay - - // !!TODO: handle error if reflection is not being processed - // if !dbg.reflectProcess { - // return errors.New(errors.ReflectionNotRunning) - // } - err = dbg.scr.SetFeature(gui.ReqToggleOverlay) case "=": diff --git a/debugger/help.go b/debugger/help.go index 768d7508..c411f9df 100644 --- a/debugger/help.go +++ b/debugger/help.go @@ -18,7 +18,6 @@ var help = map[string]string{ cmdLast: "Prints the result of the last cpu/video cycle", cmdList: "List current entries for BREAKS and TRAPS", cmdMemMap: "Display high-level VCS memory map", - cmdReflect: "Turn reflection on/off. this will slow down the debugger.", cmdMissile: "Display the current state of the missile 0/1 sprite", cmdOnHalt: "Commands to run whenever emulation is halted (separate commands with comma)", cmdOnStep: "Commands to run whenever emulation steps forward an cpu/video cycle (separate commands with comma)", diff --git a/debugger/reflection/reflection.go b/debugger/reflection/reflection.go index 8618de29..54572d0c 100644 --- a/debugger/reflection/reflection.go +++ b/debugger/reflection/reflection.go @@ -43,8 +43,6 @@ type Monitor struct { groupMissile0 addressMonitor groupMissile1 addressMonitor groupBall addressMonitor - - active bool } // NewMonitor is the preferred method of initialisation for the Monitor type @@ -84,24 +82,9 @@ func NewMonitor(vcs *hardware.VCS, renderer gui.MetaPixelRenderer) *Monitor { return mon } -// Activate the reflection monitor -func (mon *Monitor) Activate(active bool) { - mon.active = active -} - -// IsActive returns whether reflection monitor is currently active -func (mon *Monitor) IsActive() bool { - return mon.active -} - // Check should be called every video cycle to record the current state of the // emulation/system func (mon *Monitor) Check() error { - // silently return if monitor is not active - if !mon.IsActive() { - return nil - } - if err := mon.checkWSYNC(); err != nil { return err } diff --git a/debugger/targets.go b/debugger/targets.go index e167957c..5bb7742c 100644 --- a/debugger/targets.go +++ b/debugger/targets.go @@ -12,10 +12,11 @@ import ( type target interface { Label() string - // the current value of the target + // the current value of the target. should return a value of type int or + // bool. CurrentValue() interface{} - // format an arbitrary value using suitable formatting method of the target + // format an arbitrary value using suitable formatting method for the target FormatValue(val interface{}) string } diff --git a/errors/messages.go b/errors/messages.go index 8464c8b7..223540f7 100644 --- a/errors/messages.go +++ b/errors/messages.go @@ -20,13 +20,12 @@ const ( DisassemblyError = "error debugging disassembly: %v" // debugger - ParserError = "parser error: %v: %v (char %d)" // first placeholder is the command definition - ValidationError = "%v for %v" - InvalidTarget = "invalid target (%v)" - CommandError = "%v" - TerminalError = "%v" - GUIEventError = "%v" - ReflectionNotRunning = "reflection process is not running" + ParserError = "parser error: %v: %v (char %d)" // first placeholder is the command definition + ValidationError = "%v for %v" + InvalidTarget = "invalid target (%v)" + CommandError = "%v" + TerminalError = "%v" + GUIEventError = "%v" // dissassembly DisasmError = "disassembly error: %v" diff --git a/hardware/cpu/cpu_test.go b/hardware/cpu/cpu_test.go index 2785b149..7f6c8d1f 100644 --- a/hardware/cpu/cpu_test.go +++ b/hardware/cpu/cpu_test.go @@ -349,9 +349,6 @@ func testStorageInstructions(t *testing.T, mc *cpu.CPU, mem *mockMem) { } func testBranching(t *testing.T, mc *cpu.CPU, mem *mockMem) { - // !!TODO: test page faults - // !!TODO: test backwards branching - var origin uint16 mem.Clear() _ = mc.Reset() @@ -416,6 +413,57 @@ func testBranching(t *testing.T, mc *cpu.CPU, mem *mockMem) { rtest.EquateRegisters(t, mc.PC, 0x12) } +func testBranchingBackwards(t *testing.T, mc *cpu.CPU, mem *mockMem) { + var origin uint16 + mem.Clear() + _ = mc.Reset() + + mem.Clear() + _ = mc.Reset() + + origin = 0x20 + mc.LoadPC(0x20) + + // BPL backwards + _ = mem.putInstructions(origin, 0x10, 0xfd) + step(t, mc) // BPL $FF + rtest.EquateRegisters(t, mc.PC, 0x1f) + + // BVS backwards + origin = 0x20 + mc.LoadPC(0x20) + mc.Status.Overflow = true + _ = mem.putInstructions(origin, 0x70, 0xfd) + step(t, mc) // BVS $FF + rtest.EquateRegisters(t, mc.PC, 0x1f) +} + +func testBranchingPageFaults(t *testing.T, mc *cpu.CPU, mem *mockMem) { + var origin uint16 + mem.Clear() + _ = mc.Reset() + + // BNE backwards - with PC wrap (causing a page fault) + origin = 0x20 + mc.LoadPC(0x20) + mc.Status.Zero = false + _ = mem.putInstructions(origin, 0xd0, 0x80) + step(t, mc) // BNE $F0 + rtest.EquateRegisters(t, mc.PC, 0xffa2) + + // pagefault flag should be set + if !mc.LastResult.PageFault { + t.Errorf("expected pagefault on branch") + } + + // number of cycles should be 4 instead of 2 + // +1 for failed branch test (causing PC to jump) + // +1 for page fault + if mc.LastResult.ActualCycles != 4 { + t.Errorf("expected pagefault on branch") + } +} + func testJumps(t *testing.T, mc *cpu.CPU, mem *mockMem) { var origin uint16 mem.Clear() @@ -597,6 +645,8 @@ func TestCPU(t *testing.T) { testPostIndexedIndirect(t, mc, mem) testStorageInstructions(t, mc, mem) testBranching(t, mc, mem) + testBranchingBackwards(t, mc, mem) + testBranchingPageFaults(t, mc, mem) testJumps(t, mc, mem) testComparisonInstructions(t, mc, mem) testSubroutineInstructions(t, mc, mem) diff --git a/hardware/cpu/registers/program_counter.go b/hardware/cpu/registers/program_counter.go index 81cde73a..8de71f34 100644 --- a/hardware/cpu/registers/program_counter.go +++ b/hardware/cpu/registers/program_counter.go @@ -21,6 +21,11 @@ func (pc ProgramCounter) String() string { return fmt.Sprintf("%#04x", pc.value) } +// Value returns the current value of the register +func (pc ProgramCounter) Value() uint16 { + return pc.value +} + // FormatValue formats an arbitary value to look like a PC value func (pc ProgramCounter) FormatValue(val interface{}) string { return fmt.Sprintf("%#04x", val) diff --git a/hardware/cpu/registers/status.go b/hardware/cpu/registers/status.go index 112690a3..b888dfd1 100644 --- a/hardware/cpu/registers/status.go +++ b/hardware/cpu/registers/status.go @@ -4,8 +4,6 @@ import ( "strings" ) -// !!TODO: Status register N,V,Z flag bug - // StatusRegister is the special purpose register that stores the flags of the CPU type StatusRegister struct { Sign bool diff --git a/hardware/cpu/registers/test/equate_registers.go b/hardware/cpu/registers/test/equate_registers.go index b8dc7202..09c3e213 100644 --- a/hardware/cpu/registers/test/equate_registers.go +++ b/hardware/cpu/registers/test/equate_registers.go @@ -22,7 +22,7 @@ func EquateRegisters(t *testing.T, value, expectedValue interface{}) { case int: if int(value.Value()) != expectedValue { - t.Errorf("unexpected Register value (%d wanted %d)", value.Value(), expectedValue) + t.Errorf("unexpected Register value (%#02x wanted %#02x)", value.Value(), expectedValue) } } @@ -33,7 +33,7 @@ func EquateRegisters(t *testing.T, value, expectedValue interface{}) { case int: if int(value.Address()) != expectedValue { - t.Errorf("unexpected ProgramCounter value (%d wanted %d)", value, expectedValue) + t.Errorf("unexpected ProgramCounter value (%#04x wanted %#04x)", value.Value(), expectedValue) } } @@ -44,7 +44,7 @@ func EquateRegisters(t *testing.T, value, expectedValue interface{}) { case int: if int(value.Value()) != expectedValue { - t.Errorf("unexpected StatusRegister value (%d wanted %d)", value.Value(), expectedValue) + t.Errorf("unexpected StatusRegister value (%#02x wanted %#02x)", value.Value(), expectedValue) } case string: