Emulation/Gopher2600
1
0
Fork 0
mirror of https://github.com/JetSetIlly/Gopher2600.git synced 2024-05-20 13:48:02 -04:00
Code Issues Packages Projects Releases Wiki Activity

CLK breakpoints/targets will prevent debugger entering playmode

Browse source 
two reasons:

1) to keep performance acceptable playmode only checks halting
   conditions on a CPU instruction boundary. however a CLK changes many
   times during an instruction meaning it will never match.

2) a CLK breakpoint will always match within 228 emulation ticks so
   there is no point entering playmode at all because it will definitely
   drop back to the debugger (within microseconds)

added a range change check to SCANLINE and CLK targets in
breakpoints.parseCommand(). we know what the possible values are for
these targets so we can be helpful and inform the user the some values
will never match
This commit is contained in:
JetSetIlly 2021-11-11 07:23:31 +00:00
parent 7af5af322c
commit c19a37a738
10 changed files with 96 additions and 65 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
.gitignore vendored
View file

@ -24,3 +24,4 @@ MISSING_LICENCE
DODIFF
diff
coproc_*
todo.txt

14
debugger/debugger.go
View file

@ -420,6 +420,20 @@ func (dbg *Debugger) setMode(mode emulation.Mode) error {
return nil
}
// if there is a halting condition that is not allowed in playmode (see
// targets type) then do not change the emulation mode
//
// however, because the user has asked to switch to playmode we should
// cause the debugger mode to run until the halting condition is matched
// (which we know will occur in the almost immediate future)
if mode == emulation.ModePlay && !dbg.halting.allowPlaymode() {
if dbg.mode == emulation.ModeDebugger {
dbg.runUntilHalt = true
dbg.continueEmulation = true
}
return nil
}
prevMode := dbg.mode
dbg.mode = mode

36
debugger/halt.go
View file

@ -75,14 +75,10 @@ func (h *haltCoordination) reset() {
// check for a halt condition and set the halt flag if found.
func (h *haltCoordination) check() {
// whether CPU is at an instruction boundary. breakpoints and traps need to
// know this because some targets are sensitive to it
instructionBoundary := h.dbg.vcs.CPU.LastResult.Final
// we don't check for regular break/trap/wathes if there are volatileTraps in place
if h.volatileTraps.isEmpty() && h.volatileBreakpoints.isEmpty() {
breakMessage := h.breakpoints.check(instructionBoundary)
trapMessage := h.traps.check(instructionBoundary)
breakMessage := h.breakpoints.check()
trapMessage := h.traps.check()
watchMessage := h.watches.check()
if breakMessage != "" {
@ -104,7 +100,31 @@ func (h *haltCoordination) check() {
}
// check volatile conditions
breakMessage := h.volatileBreakpoints.check(instructionBoundary)
trapMessage := h.volatileTraps.check(instructionBoundary)
breakMessage := h.volatileBreakpoints.check()
trapMessage := h.volatileTraps.check()
h.halt = h.halt || breakMessage != "" || trapMessage != ""
}
// returns false if a breakpoint or trap target has the notInPlaymode flag set
func (h *haltCoordination) allowPlaymode() bool {
for _, b := range h.breakpoints.breaks {
if b.target.notInPlaymode {
return false
}
n := b.next
for n != nil {
if n.target.notInPlaymode {
return false
}
n = n.next
}
}
for _, t := range h.traps.traps {
if t.target.notInPlaymode {
return false
}
}
return true
}

33
debugger/halt_breakpoints.go
View file

@ -29,6 +29,7 @@ import (
"github.com/jetsetilly/gopher2600/debugger/terminal/commandline"
"github.com/jetsetilly/gopher2600/disassembly"
"github.com/jetsetilly/gopher2600/hardware/memory/memorymap"
"github.com/jetsetilly/gopher2600/hardware/television/specification"
)
// breakpoints keeps track of all the currently defined breakers.
@ -132,8 +133,6 @@ const (
// the break target.
func (bk *breaker) check() checkResult {
if bk.target.value() != bk.value {
// target value differs from break value so the next time it matches
// we don't want to skip the match
bk.skipNext = false
return checkNoMatch
}
@ -213,18 +212,17 @@ func (bp *breakpoints) drop(num int) error {
// check compares the current state of the emulation with every breakpoint
// condition. returns a string listing every condition that matches (separated
// by \n).
func (bp *breakpoints) check(instructionBoundary bool) string {
func (bp *breakpoints) check() string {
if len(bp.breaks) == 0 {
return ""
}
checkString := strings.Builder{}
for i := range bp.breaks {
if bp.breaks[i].target.instructionBoundary && !instructionBoundary {
if bp.breaks[i].target.instructionBoundary && !bp.dbg.vcs.CPU.LastResult.Final {
continue // for loop
}
// check current value of target with the requested value
if bp.breaks[i].check() == checkMatch {
checkString.WriteString(fmt.Sprintf("break on %s\n", bp.breaks[i]))
}
@ -299,15 +297,15 @@ func (bp *breakpoints) parseCommand(tokens *commandline.Tokens) error {
switch tgt.value().(type) {
case string:
// if token is string type then make it uppercase for now
//
// !!TODO: more sophisticated transforms of breakpoint information
// see also "special handling for PC" below
val = strings.ToUpper(tok)
case int:
var v int64
v, err = strconv.ParseInt(tok, 0, 32)
if err == nil {
val = int(v)
} else {
// !!TODO: allow symbol lookup for targets with integer values
err = curated.Errorf("invalid value (%s) for target (%s)", tok, tgt.label)
}
case bool:
switch strings.ToLower(tok) {
@ -323,14 +321,29 @@ func (bp *breakpoints) parseCommand(tokens *commandline.Tokens) error {
}
if err == nil {
// special handling for PC
if tgt.label == "PC" {
// special handling for some targets
switch tgt.label {
case "PC":
ai := bp.dbg.dbgmem.MapAddress(uint16(val.(int)), true)
val = int(ai.MappedAddress)
// unusual case but if PC break is not in cartridge area we
// don't want to add a bank condition
addBankCondition = addBankCondition && ai.Area == memorymap.Cartridge
case "Scanline":
if val.(int) < 0 {
return curated.Errorf("scanline value must be greater than or equal to 0")
}
if val.(int) > specification.AbsoluteMaxScanlines {
return curated.Errorf("scanline value must be less than or equal to %d", specification.AbsoluteMaxScanlines)
}
case "Clock":
if val.(int) < -specification.ClksHBlank {
return curated.Errorf("clock value must be greater than or equal to %d", -specification.ClksHBlank)
}
if val.(int) > specification.ClksVisible {
return curated.Errorf("scanline value must be less than or equal to %d", specification.ClksVisible)
}
}
if andBreaks {

21
debugger/halt_targets.go
View file

@ -44,6 +44,10 @@ type target struct {
// some targets should only be checked on an instruction boundary
instructionBoundary bool
// this target will always break in playmode almost immediately. we use
// this flag to decide whether to allow the debugger to switch to playmode
notInPlaymode bool
}
// returns value() formated by the format string. accepts a target value as an
@ -139,7 +143,6 @@ func parseTarget(dbg *Debugger, tokens *commandline.Tokens) (*target, error) {
value: func() targetValue {
return dbg.vcs.TV.GetCoords().Frame
},
instructionBoundary: false,
}
case "SCANLINE", "SL":
@ -148,7 +151,10 @@ func parseTarget(dbg *Debugger, tokens *commandline.Tokens) (*target, error) {
value: func() targetValue {
return dbg.vcs.TV.GetCoords().Scanline
},
instructionBoundary: false,
// specifying scanline to be notInPlaymode was considered but
// this will occasionaly not be true. for example, a scanline
// value that is beyond the extent of the generated TV frame
}
case "CLOCK", "CL":
@ -157,7 +163,16 @@ func parseTarget(dbg *Debugger, tokens *commandline.Tokens) (*target, error) {
value: func() targetValue {
return dbg.vcs.TV.GetCoords().Clock
},
instructionBoundary: false,
// it is impossible to measure the clock value accurately in
// playmode because the state of the machine is only checked
// after every CPU instruction
//
// another reason not to allow playmode when this halt target
// is being used, is that the emulation will almost immediately
// halt - the clock value *will* be reached within 228 ticks of
// the emulation
notInPlaymode: true,
}
case "BANK":

4
debugger/halt_traps.go
View file

@ -76,14 +76,14 @@ func (tr *traps) drop(num int) error {
// check compares the current state of the emulation with every trap condition.
// returns a string listing every condition that matches (separated by \n).
func (tr *traps) check(instructionBoundary bool) string {
func (tr *traps) check() string {
if len(tr.traps) == 0 {
return ""
}
checkString := strings.Builder{}
for i := range tr.traps {
if tr.traps[i].target.instructionBoundary && !instructionBoundary {
if tr.traps[i].target.instructionBoundary && !tr.dbg.vcs.CPU.LastResult.Final {
continue // for loop
}

5
debugger/halt_watches.go
View file

@ -213,7 +213,10 @@ func (wtc *watches) parseCommand(tokens *commandline.Tokens) error {
// mapping of the address was unsuccessful
if ai == nil {
return curated.Errorf("invalid watch address (%s) expecting 16-bit address or symbol", a)
if read {
return curated.Errorf("invalid watch address (%s) expecting 16-bit address or a read symbol", a)
}
return curated.Errorf("invalid watch address (%s) expecting 16-bit address or a write symbol", a)
}
// get value if possible

2
debugger/halt_watches_test.go
View file

@ -50,7 +50,7 @@ func (trm *mockTerm) testWatches() {
// try adding an invalid read address by symbol
trm.sndInput("WATCH READ VSYNC")
trm.cmpOutput("invalid watch address (VSYNC) expecting 16-bit address or symbol")
trm.cmpOutput("invalid watch address (VSYNC) expecting 16-bit address or a read symbol")
// add address by symbol. no read/write modifier means it tries
trm.sndInput("WATCH WRITE VSYNC")

8
debugger/loop_playmode.go
View file

@ -27,9 +27,11 @@ func (dbg *Debugger) playLoop() error {
// run and handle events
return dbg.vcs.Run(func() (emulation.State, error) {
// run continueCheck() function is called every CPU instruction so fuzziness
// is the number of cycles of the most recent instruction multiplied
// by three (the number of video cycles per CPU cycle)
// run continueCheck() function is called every CPU instruction. for
// some halt conditions this is too infrequent
//
// for this reason we should never find ourselves in the playLoop if
// these halt conditions exist. see setMode() function
dbg.halting.check()
if dbg.halting.halt {
// set debugging mode. halting messages will be preserved and

37
todo.txt
View file

@ -1,37 +0,0 @@
[TODO] simplify breakpoints parser to match help description. (debugger/breakpoints.go:246)
[TODO] more sophisticated transforms of breakpoint information (debugger/breakpoints.go:285)
[TODO] detect other break types? (debugger/breakpoints.go:410)
[TODO] poke/peek static cartridge static data areas (debugger/commands.go:427)
[TODO] poke/peek cartridge registers (debugger/commands.go:448)
[TODO] it would be nice to see partial disassemblies of supercharger tapes (debugger/debugger.go:470)
[TODO] fix flaws in commandline package. (debugger/terminal/commandline/parser.go:29)
[TODO] filename completion (debugger/terminal/commandline/tabcompletion.go:181)
[TODO] introduce a special purpose "address" tag type? (debugger/terminal/commandline/validation.go:127)
[TODO] find practical examples of interbank branch jumping (disassembly/decode.go:114)
[TODO] attempt to decode instructions not in cartridge (disassembly/disassembly.go:204)
[TODO] maybe make sure next entry has been disassembled in it's current form (disassembly/disassembly.go:268)
[TODO] more selecting adding of label from symbols file (disassembly/symbols/file.go:110)
[TODO] SetPosition doesn't seem to set window position as you (gui/sdlimgui/preferences.go:187)
[TODO] duplicate imgui.SetIniFilename so that is uses prefs package. we (gui/sdlimgui/sdlimgui.go:41)
[TODO] warning/help text for chip registers window (gui/sdlimgui/win_chip_registers.go:92)
[TODO] show something meaningful for unplugged controllers (gui/sdlimgui/win_controllers.go:66)
[TODO] remove this once all opcodes are defined/implemented (hardware/cpu/cpu.go:340)
[TODO] remove this once all opcodes are defined/implemented (hardware/cpu/cpu.go:573)
[TODO] FE cartridge mapping (hardware/memory/cartridge/cartridge.go:235)
[TODO] hotspot info for 3e (hardware/memory/cartridge/mapper_3e.go:39)
[TODO] hotspot info for 3e+ (hardware/memory/cartridge/mapper_3eplus.go:40)
[TODO] hotspot info for superbank (hardware/memory/cartridge/mapper_superbank.go:44)
[TODO] hotspot info for tigervision (hardware/memory/cartridge/mapper_tigervision.go:57)
[TODO] lint check for data writes that specify a bank > NumBanks(). the (hardware/memory/cartridge/mapper_tigervision.go:177)
[TODO] embed core.bin address constants, generated during the assembly process of core.bin. (hardware/memory/cartridge/moviecart/coredata.go:33)
[TODO] consider versioning the auto-controller type to help the recorder package. (hardware/riot/ports/controllers/auto.go:73)
[TODO] Consider adding 400ms delay for SWACNT settings to take effect. (hardware/riot/ports/controllers/keypad.go:210)
[TODO] support for paddle player 3 and paddle player 4 (hardware/riot/ports/controllers/paddle.go:73)
[TODO] conditional calling of Ports.Step() (hardware/riot/riot.go:95)
[TODO] more elegant way of handling the additional scanline problem (hardware/television/resizer.go:248)
[TODO] think about how we're sending VSYNC to the pixel renderer (hardware/television/specification/specifications.go:170)
[TODO] consider adding lastSignal information to TV state string. (hardware/television/television.go:88)
[TODO] check accuracy of HSync timing (hardware/tia/tia.go:445)
[TODO] the setNUSIZ() function needs untangling. I reckon with a bit of (hardware/tia/video/player.go:688)
[TODO] noecho hiscore server password (hiscore/settings.go:108)
[TODO] consider versioning the auto-controller and noting the version number in playback script (recorder/fileformat.go:63)