From d744e6229afdc1543a3ffe43c8606fd279ac686c Mon Sep 17 00:00:00 2001 From: steve Date: Thu, 9 Aug 2018 16:46:34 +0100 Subject: [PATCH] o debugger - better lexing/parsing of command input - break/trap on instruction effect (read, write, etc.) (we had to implement a more flexible way of satisfying the target interface for this. it works but it's a bit ugly) --- debugger/breakpoints.go | 30 ++----- debugger/commands.go | 6 +- debugger/debugger.go | 167 ++++++++++++++++-------------------- debugger/parser/commands.go | 31 ++++--- debugger/targets.go | 99 ++++++++++++++++----- debugger/tokens.go | 68 +++++++++++++++ debugger/traps.go | 13 ++- errors/categories.go | 6 +- errors/messages.go | 4 + 9 files changed, 269 insertions(+), 155 deletions(-) create mode 100644 debugger/tokens.go diff --git a/debugger/breakpoints.go b/debugger/breakpoints.go index 8ced02b6..8ad5ab4e 100644 --- a/debugger/breakpoints.go +++ b/debugger/breakpoints.go @@ -7,7 +7,6 @@ package debugger import ( "gopher2600/debugger/ui" "strconv" - "strings" ) // breakpoints keeps track of all the currently defined breaker @@ -41,17 +40,6 @@ func (bp *breakpoints) clear() { // of all Targets. we can then use these stored values to know what to // ignore. used primarily so that we're not breaking immediately on a previous // breakstate. -// -// one possible flaw in the current implementation of this idea is that the -// emulation will not honour new breaks until the value has cycled back to the -// break value: -// -// A == v -// break A v -// A == v -> no break -// A == w -> no break -// A == v -> breaks -// func (bp *breakpoints) prepareBreakpoints() { bp.ignoredBreakerStates = make(map[target]interface{}, len(bp.breaks)) for _, b := range bp.breaks { @@ -100,7 +88,7 @@ func (bp breakpoints) list() { } } -func (bp *breakpoints) parseBreakpoint(parts []string) error { +func (bp *breakpoints) parseBreakpoint(tokens *tokens) error { var tgt target // default target of CPU PC. meaning that "BREAK n" will cause a breakpoint @@ -109,15 +97,14 @@ func (bp *breakpoints) parseBreakpoint(parts []string) error { // something appropriate tgt = bp.dbg.vcs.MC.PC - // loop over parts. if part is a number then add the breakpoint for the + // loop over tokens. if token is a number then add the breakpoint for the // current target. if it is not a number, look for a keyword that changes // the target (or run a BREAK meta-command) // // note that this method of looping allows the user to chain break commands - for i := 1; i < len(parts); i++ { - parts[i] = strings.ToUpper(parts[i]) - - val, err := strconv.ParseUint(parts[i], 0, 16) + a, present := tokens.get() + for present { + val, err := strconv.ParseUint(a, 0, 16) if err == nil { // check to see if breakpoint already exists addNewBreak := true @@ -131,14 +118,15 @@ func (bp *breakpoints) parseBreakpoint(parts []string) error { if addNewBreak { bp.breaks = append(bp.breaks, breaker{target: tgt, value: int(val)}) } - } else { - // defer parsing of other keywords to parseTargets() - tgt, err = parseTarget(bp.dbg.vcs, parts[i]) + tokens.unget() + tgt, err = parseTarget(bp.dbg, tokens) if err != nil { return err } } + + a, present = tokens.get() } return nil diff --git a/debugger/commands.go b/debugger/commands.go index 0cbb4b72..afe8fa44 100644 --- a/debugger/commands.go +++ b/debugger/commands.go @@ -51,8 +51,8 @@ const ( var DebuggerCommands = parser.Commands{ KeywordInsert: parser.CommandArgs{parser.Arg{Typ: parser.ArgFile, Req: true}}, KeywordSymbol: parser.CommandArgs{parser.Arg{Typ: parser.ArgString, Req: true}}, - KeywordBreak: parser.CommandArgs{parser.Arg{Typ: parser.ArgTarget, Req: true}, parser.Arg{Typ: parser.ArgValue, Req: false}}, - KeywordTrap: parser.CommandArgs{parser.Arg{Typ: parser.ArgTarget, Req: true}}, + KeywordBreak: parser.CommandArgs{parser.Arg{Typ: parser.ArgTarget, Req: true}, parser.Arg{Typ: parser.ArgValue, Req: false}, parser.Arg{Typ: parser.ArgIndeterminate}}, + KeywordTrap: parser.CommandArgs{parser.Arg{Typ: parser.ArgTarget, Req: true}, parser.Arg{Typ: parser.ArgIndeterminate}}, KeywordList: parser.CommandArgs{parser.Arg{Typ: parser.ArgKeyword, Req: true, Vals: parser.Keywords{SubKeywordBreaks, SubKeywordTraps}}}, KeywordClear: parser.CommandArgs{parser.Arg{Typ: parser.ArgKeyword, Req: true, Vals: parser.Keywords{SubKeywordBreaks, SubKeywordTraps}}}, KeywordOnHalt: parser.CommandArgs{parser.Arg{Typ: parser.ArgIndeterminate}}, @@ -78,7 +78,7 @@ var DebuggerCommands = parser.Commands{ KeywordMissile: parser.CommandArgs{}, KeywordBall: parser.CommandArgs{}, KeywordPlayfield: parser.CommandArgs{}, - KeywordDisplay: parser.CommandArgs{}, + KeywordDisplay: parser.CommandArgs{parser.Arg{Typ: parser.ArgValue, Req: false}}, KeywordScript: parser.CommandArgs{parser.Arg{Typ: parser.ArgFile, Req: true}}, KeywordDisassemble: parser.CommandArgs{}, } diff --git a/debugger/debugger.go b/debugger/debugger.go index d04ea1d9..ee9621b7 100644 --- a/debugger/debugger.go +++ b/debugger/debugger.go @@ -411,67 +411,61 @@ func (dbg *Debugger) parseInput(input string) (bool, error) { func (dbg *Debugger) parseCommand(input string) (bool, error) { // TODO: categorise commands into script-safe and non-script-safe - // remove leading/trailing space - input = strings.TrimSpace(input) + // tokenise input + tokens := tokeniseInput(input) - // if the input is empty then return true, indicating that the emulation - // should "step" forward once - if input == "" { - return true, nil - } - - // divide user input into parts and convert to upper-case for easy parsing - // input is unchanged in case we need the original user-case - parts := strings.Fields(input) - - // normalise variations in syntax - for i := 0; i < len(parts); i++ { - // normalise hex notation - if parts[i][0] == '$' { - parts[i] = fmt.Sprintf("0x%s", parts[i][1:]) + // check validity of input -- this allows us to catch errors early and in + // many cases to ignore the "success" flag when calling tokens.item() + if err := DebuggerCommands.ValidateInput(tokens.tokens); err != nil { + switch err := err.(type) { + case errors.GopherError: + switch err.Errno { + case errors.InputEmpty: + // user pressed return + return true, nil + } } - } - - // normalise case of first entry in parts list (the command) - parts[0] = strings.ToUpper(parts[0]) - - if err := DebuggerCommands.CheckCommandInput(parts); err != nil { return false, err } // most commands do not cause the emulator to step forward stepNext := false - // implement debugging command - switch parts[0] { + tokens.reset() + command, _ := tokens.get() + command = strings.ToUpper(command) + switch command { default: - return false, fmt.Errorf("%s is not yet implemented", parts[0]) + return false, fmt.Errorf("%s is not yet implemented", command) // control of the debugger case KeywordHelp: - if len(parts) == 1 { - for k := range DebuggerCommands { - dbg.print(ui.Help, k) - } - } else { - s := strings.ToUpper(parts[1]) + keyword, present := tokens.get() + if present { + s := strings.ToUpper(keyword) txt, prs := Help[s] if prs == false { dbg.print(ui.Help, "no help for %s", s) } else { dbg.print(ui.Help, txt) } + } else { + for k := range DebuggerCommands { + dbg.print(ui.Help, k) + } } case KeywordInsert: - err := dbg.loadCartridge(parts[1]) + cart, _ := tokens.get() + err := dbg.loadCartridge(cart) if err != nil { return false, err } - dbg.print(ui.Feedback, "machine reset with new cartridge (%s)", parts[1]) + dbg.print(ui.Feedback, "machine reset with new cartridge (%s)", cart) case KeywordScript: - err := dbg.RunScript(parts[1], false) + script, _ := tokens.get() + err := dbg.RunScript(script, false) if err != nil { return false, err } @@ -480,65 +474,65 @@ func (dbg *Debugger) parseCommand(input string) (bool, error) { dbg.print(ui.CPUStep, dbg.disasm.Dump()) case KeywordSymbol: - address, err := dbg.disasm.Symtable.SearchLocation(parts[1]) + symbol, _ := tokens.get() + address, err := dbg.disasm.Symtable.SearchLocation(symbol) if err != nil { switch err := err.(type) { case errors.GopherError: if err.Errno == errors.SymbolUnknown { - dbg.print(ui.Feedback, "%s -> not found", parts[1]) + dbg.print(ui.Feedback, "%s -> not found", symbol) return false, nil } } return false, err } - dbg.print(ui.Feedback, "%s -> %#04x", parts[1], address) + dbg.print(ui.Feedback, "%s -> %#04x", symbol, address) case KeywordBreak: - err := dbg.breakpoints.parseBreakpoint(parts) + err := dbg.breakpoints.parseBreakpoint(tokens) if err != nil { return false, fmt.Errorf("error on break: %s", err) } case KeywordTrap: - err := dbg.traps.parseTrap(parts) + err := dbg.traps.parseTrap(tokens) if err != nil { return false, fmt.Errorf("error on trap: %s", err) } case KeywordList: - if len(parts) > 1 { - switch strings.ToUpper(parts[1]) { - case "BREAKS": - dbg.breakpoints.list() - case "TRAPS": - dbg.traps.list() - } + list, _ := tokens.get() + switch strings.ToUpper(list) { + case "BREAKS": + dbg.breakpoints.list() + case "TRAPS": + dbg.traps.list() } case KeywordClear: - if len(parts) > 1 { - switch strings.ToUpper(parts[1]) { - case "BREAKS": - dbg.breakpoints.clear() - dbg.print(ui.Feedback, "breakpoints cleared") - case "TRAPS": - dbg.traps.clear() - dbg.print(ui.Feedback, "traps cleared") - } + clear, _ := tokens.get() + switch strings.ToUpper(clear) { + case "BREAKS": + dbg.breakpoints.clear() + dbg.print(ui.Feedback, "breakpoints cleared") + case "TRAPS": + dbg.traps.clear() + dbg.print(ui.Feedback, "traps cleared") } case KeywordOnHalt: - if len(parts) < 2 { + if tokens.remaining() == 0 { dbg.commandOnHalt = dbg.commandOnHaltStored } else { - if strings.ToUpper(parts[1]) == "OFF" { + option, _ := tokens.peek() + if strings.ToUpper(option) == "OFF" { dbg.commandOnHalt = "" dbg.print(ui.Feedback, "no auto-command on halt") return false, nil } // use remaininder of command line to form the ONHALT command sequence - dbg.commandOnHalt = strings.Join(parts[1:], " ") + dbg.commandOnHalt = tokens.remainder() // we can't use semi-colons when specifying the sequence so allow use of // commas to act as an alternative @@ -555,17 +549,18 @@ func (dbg *Debugger) parseCommand(input string) (bool, error) { return false, err case KeywordOnStep: - if len(parts) < 2 { + if tokens.remaining() == 0 { dbg.commandOnStep = dbg.commandOnStepStored } else { - if strings.ToUpper(parts[1]) == "OFF" { + option, _ := tokens.peek() + if strings.ToUpper(option) == "OFF" { dbg.commandOnStep = "" dbg.print(ui.Feedback, "no auto-command on step") return false, nil } // use remaininder of command line to form the ONSTEP command sequence - dbg.commandOnStep = strings.Join(parts[1:], " ") + dbg.commandOnStep = tokens.remainder() // we can't use semi-colons when specifying the sequence so allow use of // commas to act as an alternative @@ -618,33 +613,19 @@ func (dbg *Debugger) parseCommand(input string) (bool, error) { case KeywordStep: stepNext = true - if len(parts) > 1 { - switch parts[1] { - case "CPU": - dbg.inputloopVideoClock = false - case "VIDEO": - dbg.inputloopVideoClock = true - } - } case KeywordStepMode: - if len(parts) > 1 { - switch strings.ToUpper(parts[1]) { - case "CPU": - dbg.inputloopVideoClock = false - case "VIDEO": - dbg.inputloopVideoClock = true - default: - return false, fmt.Errorf("unknown step mode (%s)", parts[1]) - } + mode, _ := tokens.get() + mode = strings.ToUpper(mode) + switch mode { + case "CPU": + dbg.inputloopVideoClock = false + case "VIDEO": + dbg.inputloopVideoClock = true + default: + return false, fmt.Errorf("unknown step mode (%s)", mode) } - var stepMode string - if dbg.inputloopVideoClock { - stepMode = "video" - } else { - stepMode = "cpu" - } - dbg.print(ui.Feedback, "step mode: %s", stepMode) + dbg.print(ui.Feedback, "step mode: %s", mode) case KeywordTerse: dbg.machineInfoVerbose = false @@ -673,14 +654,15 @@ func (dbg *Debugger) parseCommand(input string) (bool, error) { dbg.printMachineInfo(dbg.vcs.MC) case KeywordPeek: - for i := 1; i < len(parts); i++ { + a, present := tokens.get() + for present { var addr interface{} var msg string - addr, err := strconv.ParseUint(parts[i], 0, 16) + addr, err := strconv.ParseUint(a, 0, 16) if err != nil { // argument is not a number so argument must be a string - addr = strings.ToUpper(parts[i]) + addr = strings.ToUpper(a) msg = addr.(string) } else { // convert number to type suitable for Peek command @@ -704,6 +686,8 @@ func (dbg *Debugger) parseCommand(input string) (bool, error) { msg = fmt.Sprintf("%s [%s]", msg, addressLabel) } dbg.print(ui.MachineInfo, msg) + + a, present = tokens.get() } case KeywordRAM: @@ -739,8 +723,9 @@ func (dbg *Debugger) parseCommand(input string) (bool, error) { case KeywordDisplay: visibility := true - if len(parts) > 1 { - switch parts[1] { + action, present := tokens.get() + if present { + switch strings.ToUpper(action) { case "OFF": visibility = false } diff --git a/debugger/parser/commands.go b/debugger/parser/commands.go index fd3790a0..72c9f428 100644 --- a/debugger/parser/commands.go +++ b/debugger/parser/commands.go @@ -2,6 +2,8 @@ package parser import ( "fmt" + "gopher2600/errors" + "strings" ) // ArgType defines the expected argument type @@ -58,41 +60,48 @@ func (a CommandArgs) minLen() (m int) { return } -// CheckCommandInput checks whether input is correct according to the +// ValidateInput checks whether input is correct according to the // command definitions -func (options Commands) CheckCommandInput(input []string) error { +func (options Commands) ValidateInput(input []string) error { var args CommandArgs + // if input is empty then return + if len(input) == 0 { + return errors.NewGopherError(errors.InputEmpty) + } + + input[0] = strings.ToUpper(input[0]) + // basic check for whether command is recognised var ok bool if args, ok = options[input[0]]; !ok { - return fmt.Errorf("%s is not a debugging command", input[0]) + return errors.NewGopherError(errors.InputInvalidCommand, fmt.Sprintf("%s is not a debugging command", input[0])) } // too *many* arguments have been supplied if len(input)-1 > args.maxLen() { - return fmt.Errorf("too many arguments for %s", input[0]) + return errors.NewGopherError(errors.InputTooManyArgs, fmt.Sprintf("too many arguments for %s", input[0])) } // too *few* arguments have been supplied if len(input)-1 < args.minLen() { switch args[len(input)-1].Typ { case ArgKeyword: - return fmt.Errorf("keyword required for %s", input[0]) + return errors.NewGopherError(errors.InputTooFewArgs, fmt.Sprintf("keyword required for %s", input[0])) case ArgFile: - return fmt.Errorf("filename required for %s", input[0]) + return errors.NewGopherError(errors.InputTooFewArgs, fmt.Sprintf("filename required for %s", input[0])) case ArgAddress: - return fmt.Errorf("address required for %s", input[0]) + return errors.NewGopherError(errors.InputTooFewArgs, fmt.Sprintf("address required for %s", input[0])) case ArgTarget: - return fmt.Errorf("emulation target required for %s", input[0]) + return errors.NewGopherError(errors.InputTooFewArgs, fmt.Sprintf("emulation target required for %s", input[0])) case ArgValue: - return fmt.Errorf("numeric argument required for %s", input[0]) + return errors.NewGopherError(errors.InputTooFewArgs, fmt.Sprintf("numeric argument required for %s", input[0])) case ArgString: - return fmt.Errorf("string argument required for %s", input[0]) + return errors.NewGopherError(errors.InputTooFewArgs, fmt.Sprintf("string argument required for %s", input[0])) default: // TODO: argument types can be OR'd together. breakdown these types // to give more useful information - return fmt.Errorf("too few arguments for %s", input[0]) + return errors.NewGopherError(errors.InputTooFewArgs, fmt.Sprintf("too few arguments for %s", input[0])) } } diff --git a/debugger/targets.go b/debugger/targets.go index 86fcd778..7eec88c9 100644 --- a/debugger/targets.go +++ b/debugger/targets.go @@ -1,9 +1,10 @@ package debugger import ( + "fmt" "gopher2600/errors" - "gopher2600/hardware" "gopher2600/television" + "strings" ) // defines which types are valid targets @@ -13,30 +14,86 @@ type target interface { Value() interface{} } +// genericTarget is a a way of hacking any object so that it qualifies as a +// target. bit messy but it may more convenient that defining the interface for +// a given type +type genericTarget struct { + label string + shortLabel string + value interface{} +} + +func (trg genericTarget) Label() string { + return trg.label +} + +func (trg genericTarget) ShortLabel() string { + return trg.shortLabel +} + +func (trg genericTarget) Value() interface{} { + switch value := trg.value.(type) { + case func() interface{}: + return value() + default: + return value + } +} + // parseTarget uses a keyword to decide which part of the vcs to target -func parseTarget(vcs *hardware.VCS, keyword string) (target, error) { +func parseTarget(dbg *Debugger, tokens *tokens) (target, error) { var trg target var err error - switch keyword { - case "PC": - trg = vcs.MC.PC - case "A": - trg = vcs.MC.A - case "X": - trg = vcs.MC.X - case "Y": - trg = vcs.MC.Y - case "SP": - trg = vcs.MC.SP - case "FRAMENUM", "FRAME", "FR": - trg, err = vcs.TV.RequestTVState(television.ReqFramenum) - case "SCANLINE", "SL": - trg, err = vcs.TV.RequestTVState(television.ReqScanline) - case "HORIZPOS", "HP": - trg, err = vcs.TV.RequestTVState(television.ReqHorizPos) - default: - return nil, errors.NewGopherError(errors.InvalidTarget, keyword) + keyword, present := tokens.get() + if present { + keyword = strings.ToUpper(keyword) + switch keyword { + case "PC": + trg = dbg.vcs.MC.PC + case "A": + trg = dbg.vcs.MC.A + case "X": + trg = dbg.vcs.MC.X + case "Y": + trg = dbg.vcs.MC.Y + case "SP": + trg = dbg.vcs.MC.SP + case "FRAMENUM", "FRAME", "FR": + trg, err = dbg.vcs.TV.RequestTVState(television.ReqFramenum) + case "SCANLINE", "SL": + trg, err = dbg.vcs.TV.RequestTVState(television.ReqScanline) + case "HORIZPOS", "HP": + trg, err = dbg.vcs.TV.RequestTVState(television.ReqHorizPos) + + // cpu instruction targetting was originally added as an experiment, to + // help investigate a bug in the emulation. I don't think it's much use + // but it was an instructive exercise and may come in useful one day. + case "INSTRUCTION", "INS": + subkey, present := tokens.get() + if present { + subkey = strings.ToUpper(subkey) + switch subkey { + case "EFFECT", "EFF": + return &genericTarget{ + label: "EFFECT", + shortLabel: "EFF", + value: func() interface{} { + if dbg.lastResult == nil { + return -1 + } + return int(dbg.lastResult.Defn.Effect) + }, + }, nil + default: + return nil, errors.NewGopherError(errors.InvalidTarget, fmt.Sprintf("%s/%s", keyword, subkey)) + } + } else { + return nil, errors.NewGopherError(errors.InvalidTarget, keyword) + } + default: + return nil, errors.NewGopherError(errors.InvalidTarget, keyword) + } } return trg, err diff --git a/debugger/tokens.go b/debugger/tokens.go new file mode 100644 index 00000000..0ab908db --- /dev/null +++ b/debugger/tokens.go @@ -0,0 +1,68 @@ +package debugger + +import ( + "fmt" + "strings" +) + +type tokens struct { + tokens []string + curr int +} + +func (tk *tokens) reset() { + tk.curr = 0 +} + +func (tk tokens) remainder() string { + return strings.Join(tk.tokens[tk.curr:], " ") +} + +func (tk tokens) remaining() int { + return len(tk.tokens) - tk.curr +} + +func (tk tokens) num() int { + return len(tk.tokens) +} + +func (tk *tokens) get() (string, bool) { + if tk.curr >= len(tk.tokens) { + return "", false + } + tk.curr++ + return tk.tokens[tk.curr-1], true +} + +func (tk *tokens) unget() { + if tk.curr > 0 { + tk.curr-- + } +} + +func (tk tokens) peek() (string, bool) { + if tk.curr >= len(tk.tokens) { + return "", false + } + return tk.tokens[tk.curr], true +} + +func tokeniseInput(input string) *tokens { + tk := new(tokens) + + // remove leading/trailing space + input = strings.TrimSpace(input) + + // divide user input into tokens + tk.tokens = strings.Fields(input) + + // normalise variations in syntax + for i := 0; i < len(tk.tokens); i++ { + // normalise hex notation + if tk.tokens[i][0] == '$' { + tk.tokens[i] = fmt.Sprintf("0x%s", tk.tokens[i][1:]) + } + } + + return tk +} diff --git a/debugger/traps.go b/debugger/traps.go index 9451c32f..540b0f6c 100644 --- a/debugger/traps.go +++ b/debugger/traps.go @@ -7,7 +7,6 @@ package debugger import ( "fmt" "gopher2600/debugger/ui" - "strings" ) // traps keeps track of all the currently defined trappers @@ -65,12 +64,10 @@ func (tr traps) list() { } } -func (tr *traps) parseTrap(parts []string) error { - // loop over parts, allowing multiple traps to be applied - for i := 1; i < len(parts); i++ { - parts[i] = strings.ToUpper(parts[i]) - - tgt, err := parseTarget(tr.dbg.vcs, parts[i]) +func (tr *traps) parseTrap(tokens *tokens) error { + _, present := tokens.peek() + for present { + tgt, err := parseTarget(tr.dbg, tokens) if err != nil { return err } @@ -87,6 +84,8 @@ func (tr *traps) parseTrap(parts []string) error { if addNewTrap { tr.traps = append(tr.traps, trapper{target: tgt, origValue: tgt.Value()}) } + + _, present = tokens.peek() } return nil diff --git a/errors/categories.go b/errors/categories.go index ef1f3cda..d6fa4b7d 100644 --- a/errors/categories.go +++ b/errors/categories.go @@ -3,7 +3,11 @@ package errors // list of error numbers const ( // Debugger - SymbolsFileCannotOpen Errno = iota + InputInvalidCommand Errno = iota + InputTooManyArgs + InputTooFewArgs + InputEmpty + SymbolsFileCannotOpen SymbolsFileError SymbolUnknown ScriptFileCannotOpen diff --git a/errors/messages.go b/errors/messages.go index eaf5ae23..bae234e7 100644 --- a/errors/messages.go +++ b/errors/messages.go @@ -2,6 +2,10 @@ package errors var messages = map[Errno]string{ // Debugger + InputInvalidCommand: "%s", + InputTooManyArgs: "%s", + InputTooFewArgs: "%s", + InputEmpty: "%s", SymbolsFileCannotOpen: "no symbols file for %s", SymbolsFileError: "error processing symbols file (%s)", SymbolUnknown: "unrecognised symbol (%s)",