From dd45b06b2c787ad5f48aaeb92b4d5aa87afc61ba Mon Sep 17 00:00:00 2001 From: steve Date: Mon, 30 Dec 2019 18:57:30 +0000 Subject: [PATCH] o debugger - watches now uses addressInfo - fixed bug in debugger memory mapAddress() function --- debugger/commands.go | 14 +---- debugger/help.go | 40 ++++++++++++- debugger/memory.go | 35 ++++++----- debugger/watches.go | 125 +++++++++++++++++---------------------- debugger/watches_test.go | 35 ++++++++++- 5 files changed, 146 insertions(+), 103 deletions(-) diff --git a/debugger/commands.go b/debugger/commands.go index e62aa83b..abc30f7a 100644 --- a/debugger/commands.go +++ b/debugger/commands.go @@ -190,18 +190,6 @@ func (dbg *Debugger) parseCommand(userInput *string, interactive bool) (parseCom return doNothing, err } - // make sure all tokens have been handled. this should only happen if - // input has been allowed by ValidateTokens() but has not been - // explicitely consumed by entactCommand(). a false positive might occur if - // the token queue has been Peek()ed rather than Get()ed - if interactive { - defer func() { - if !tokens.IsEnd() { - dbg.print(terminal.StyleError, fmt.Sprintf("unhandled arguments in user input (%s)", tokens.Remainder())) - } - }() - } - // the absolute best thing about the ValidateTokens() function is that we // don't need to worrying too much about the success of tokens.Get() in the // enactCommand() function below: @@ -1082,7 +1070,7 @@ func (dbg *Debugger) parseCommand(userInput *string, interactive bool) (parseCom } case cmdWatch: - err := dbg.watches.parseWatch(tokens, dbg.dbgmem) + err := dbg.watches.parseWatch(tokens) if err != nil { return doNothing, errors.New(errors.CommandError, err) } diff --git a/debugger/help.go b/debugger/help.go index c75c5cf4..76eadb32 100644 --- a/debugger/help.go +++ b/debugger/help.go @@ -41,7 +41,45 @@ var help = map[string]string{ // halt conditions cmdBreak: "Cause emulator to halt when conditions are met", cmdTrap: "Cause emulator to halt when specified machine component is touched", - cmdWatch: "Watch a memory address for activity", + + cmdWatch: `Watch a memory address for activity. Emulation will halt when the watch +is triggered. An individual watch can wait for either read access or write +access of specific address address. Addresses can be specified numerically or +by symbol. + +By default, watching a numeric address will specifically watch for write +events. This can be changed by specifiying READ as the first argument. For +example: + + WATCH 0x80 + + WATCH READ 0x81 + +The first example watches address 0x80 for write access, while the second will +watch for read access of address 0x81. To watch a single address for both read and +write access, two watches are requireed. + +Symbolic address refer to either read or write addresses (possibly both) and +this affects how symbolic addresses are watched. Consider the following two +examples: + + WATCH VSYNC + + WATCH CXM0P + +The symbols in both examples refer to memory address 0x00 but specifcally, +VSYNC is used in the context of the CPU writing to memory and CXM0P in the +context of reading from memory. Accordingly, the watches will react to write +or read events. + +A watch can also watch for a specific value to be written or read from the specified +address. + + WATCH 0x80 10 + +The above example will watch for the value 10 (decimal) to be written to memory +address 0x80.`, + cmdList: "List current entries for breaks, traps and watches", cmdDrop: "Drop a specific break, trap or watch condition, using the number of the condition reported by LIST", cmdClear: "Clear all breaks, traps and watches", diff --git a/debugger/memory.go b/debugger/memory.go index a6b0de6a..77813b8a 100644 --- a/debugger/memory.go +++ b/debugger/memory.go @@ -27,10 +27,13 @@ type addressInfo struct { addressLabel string area memorymap.Area - // the data at the address. if we don't know what the address is then - // useData is false and the value of data is not valid - data uint8 - useData bool + // addresses and symbols are mapped differently depending on whether + // address is to be used for reading or writing + read bool + + // the data at the address. if peeked is false then data mays not be valid + peeked bool + data uint8 } func (ai addressInfo) String() string { @@ -46,9 +49,9 @@ func (ai addressInfo) String() string { s.WriteString(fmt.Sprintf(" [mirror of %#04x]", ai.mappedAddress)) } - s.WriteString(fmt.Sprintf(" :: %s", ai.area.String())) + s.WriteString(fmt.Sprintf(" (%s)", ai.area.String())) - if ai.useData { + if ai.peeked { s.WriteString(fmt.Sprintf(" -> %#02x", ai.data)) } @@ -56,12 +59,12 @@ func (ai addressInfo) String() string { } // mapAddress allows addressing by symbols in addition to numerically -func (dbgmem memoryDebug) mapAddress(address interface{}, cpuRead bool) *addressInfo { - ai := &addressInfo{} +func (dbgmem memoryDebug) mapAddress(address interface{}, read bool) *addressInfo { + ai := &addressInfo{read: read} var symbolTable map[uint16]string - if cpuRead { + if read { symbolTable = (dbgmem.symtable).Read.Symbols } else { symbolTable = (dbgmem.symtable).Write.Symbols @@ -70,7 +73,7 @@ func (dbgmem memoryDebug) mapAddress(address interface{}, cpuRead bool) *address switch address := address.(type) { case uint16: ai.address = address - ai.mappedAddress, ai.area = memorymap.MapAddress(address, cpuRead) + ai.mappedAddress, ai.area = memorymap.MapAddress(address, read) case string: var found bool var err error @@ -80,7 +83,7 @@ func (dbgmem memoryDebug) mapAddress(address interface{}, cpuRead bool) *address for a, sym := range symbolTable { if sym == address { ai.address = a - ai.mappedAddress = a + ai.mappedAddress, ai.area = memorymap.MapAddress(ai.address, read) found = true break // for loop } @@ -94,7 +97,7 @@ func (dbgmem memoryDebug) mapAddress(address interface{}, cpuRead bool) *address for a, sym := range symbolTable { if sym == address { ai.address = a - ai.mappedAddress, ai.area = memorymap.MapAddress(ai.address, cpuRead) + ai.mappedAddress, ai.area = memorymap.MapAddress(ai.address, read) found = true break // for loop } @@ -108,7 +111,7 @@ func (dbgmem memoryDebug) mapAddress(address interface{}, cpuRead bool) *address } ai.address = uint16(addr) - ai.mappedAddress, ai.area = memorymap.MapAddress(ai.address, cpuRead) + ai.mappedAddress, ai.area = memorymap.MapAddress(ai.address, read) } } @@ -135,7 +138,7 @@ func (dbgmem memoryDebug) peek(address interface{}) (*addressInfo, error) { return nil, errors.New(errors.DebuggerError, err) } - ai.useData = true + ai.peeked = true return ai, err } @@ -143,7 +146,7 @@ func (dbgmem memoryDebug) peek(address interface{}) (*addressInfo, error) { // Poke writes a value at the specified address, which may be numeric or // symbolic. func (dbgmem memoryDebug) poke(address interface{}, data uint8) (*addressInfo, error) { - ai := dbgmem.mapAddress(address, true) + ai := dbgmem.mapAddress(address, false) if ai == nil { return nil, errors.New(errors.DebuggerError, errors.New(errors.UnpokeableAddress, address)) } @@ -159,7 +162,7 @@ func (dbgmem memoryDebug) poke(address interface{}, data uint8) (*addressInfo, e } ai.data = data - ai.useData = true + ai.peeked = true return ai, err } diff --git a/debugger/watches.go b/debugger/watches.go index 96a954f6..887ff34a 100644 --- a/debugger/watches.go +++ b/debugger/watches.go @@ -10,32 +10,8 @@ import ( "strings" ) -// the watch system can watch for read, write events specifically or for either -// event -type watchEvent int - -const ( - watchEventAny watchEvent = iota - watchEventRead - watchEventWrite -) - -func (ev watchEvent) String() string { - switch ev { - case watchEventRead: - return "read" - case watchEventWrite: - return "write" - case watchEventAny: - return "read/write" - default: - return "" - } -} - type watcher struct { - address uint16 - event watchEvent + ai addressInfo // whether to watch for a specific value. a matchValue of false means the // watcher will match regardless of the value @@ -48,7 +24,11 @@ func (wtr watcher) String() string { if wtr.matchValue { val = fmt.Sprintf(" (value=%#02x)", wtr.value) } - return fmt.Sprintf("%#04x %s%s", wtr.address, wtr.event, val) + event := "write" + if wtr.ai.read { + event = "read" + } + return fmt.Sprintf("%s %s%s", wtr.ai, event, val) } // the list of currently defined watches in the system @@ -99,7 +79,7 @@ func (wtc *watches) check(previousResult string) string { for i := range wtc.watches { // continue loop if we're not matching last address accessed - if wtc.watches[i].address != wtc.vcsmem.LastAccessAddress { + if wtc.watches[i].ai.address != wtc.vcsmem.LastAccessAddress { continue } @@ -109,20 +89,24 @@ func (wtc *watches) check(previousResult string) string { } // match watch event to the type of memory access - if wtc.watches[i].event == watchEventAny || - (wtc.watches[i].event == watchEventWrite && wtc.vcsmem.LastAccessWrite) || - (wtc.watches[i].event == watchEventRead && !wtc.vcsmem.LastAccessWrite) { + if (wtc.watches[i].ai.read == false && wtc.vcsmem.LastAccessWrite) || + (wtc.watches[i].ai.read == true && !wtc.vcsmem.LastAccessWrite) { // match watched-for value to the value that was read/written to the // watched address - if !wtc.watches[i].matchValue || - (wtc.watches[i].matchValue && (wtc.watches[i].value == wtc.vcsmem.LastAccessValue)) { - + if !wtc.watches[i].matchValue { // prepare string according to event if wtc.vcsmem.LastAccessWrite { - checkString.WriteString(fmt.Sprintf("watch at %s -> %#02x\n", wtc.watches[i], wtc.vcsmem.LastAccessValue)) + checkString.WriteString(fmt.Sprintf("watch at %s (write)\n", wtc.watches[i])) } else { - checkString.WriteString(fmt.Sprintf("watch at %s\n", wtc.watches[i])) + checkString.WriteString(fmt.Sprintf("watch at %s (read)\n", wtc.watches[i])) + } + } else if wtc.watches[i].matchValue && (wtc.watches[i].value == wtc.vcsmem.LastAccessValue) { + // prepare string according to event + if wtc.vcsmem.LastAccessWrite { + checkString.WriteString(fmt.Sprintf("watch at %s (write) %#02x\n", wtc.watches[i], wtc.vcsmem.LastAccessValue)) + } else { + checkString.WriteString(fmt.Sprintf("watch at %s (read) %#02x\n", wtc.watches[i], wtc.vcsmem.LastAccessValue)) } } } @@ -148,19 +132,25 @@ func (wtc *watches) list() { // parse tokens and add new watch. unlike breakpoints and traps, only one watch // at a time can be specified on the command line. -func (wtc *watches) parseWatch(tokens *commandline.Tokens, dbgmem *memoryDebug) error { - var event watchEvent +func (wtc *watches) parseWatch(tokens *commandline.Tokens) error { + var event int + + const ( + either int = iota + read + write + ) // read mode mode, _ := tokens.Get() mode = strings.ToUpper(mode) switch mode { case "READ": - event = watchEventRead + event = read case "WRITE": - event = watchEventWrite + event = write default: - event = watchEventAny + event = either tokens.Unget() } @@ -169,16 +159,18 @@ func (wtc *watches) parseWatch(tokens *commandline.Tokens, dbgmem *memoryDebug) // convert address var ai *addressInfo + switch event { - case watchEventRead: - ai = dbgmem.mapAddress(a, true) - case watchEventWrite: - ai = dbgmem.mapAddress(a, false) + case read: + ai = wtc.dbg.dbgmem.mapAddress(a, true) + case write: + ai = wtc.dbg.dbgmem.mapAddress(a, false) default: - // try both perspectives - ai = dbgmem.mapAddress(a, false) + // default to write address and then read address if that's not + // possible + ai = wtc.dbg.dbgmem.mapAddress(a, false) if ai == nil { - ai = dbgmem.mapAddress(a, true) + ai = wtc.dbg.dbgmem.mapAddress(a, true) } } @@ -199,34 +191,27 @@ func (wtc *watches) parseWatch(tokens *commandline.Tokens, dbgmem *memoryDebug) } nw := watcher{ - address: ai.mappedAddress, + ai: *ai, matchValue: useVal, value: uint8(val), - event: event, } // check to see if watch already exists - for i, w := range wtc.watches { - if w.address == nw.address && w.matchValue == nw.matchValue && w.value == nw.value { - // we've found a matching watcher (address and value if - // appropriate). the following switch handles how the watcher event - // matches: - switch w.event { - case watchEventRead: - if nw.event == watchEventRead { - return errors.New(errors.CommandError, fmt.Sprintf("already being watched (%s)", w)) - } - wtc.watches[i].event = watchEventAny - return nil - case watchEventWrite: - if nw.event == watchEventWrite { - return errors.New(errors.CommandError, fmt.Sprintf("already being watched (%s)", w)) - } - wtc.watches[i].event = watchEventAny - return nil - case watchEventAny: - return errors.New(errors.CommandError, fmt.Sprintf("already being watched (%s)", w)) - } + for _, w := range wtc.watches { + + // the conditions for a watch matching are very specific: both must + // have the same address, be the same /type/ of address (read or + // write), and the same watch value (if applicable) + // + // note that this method means we can add a watch that is a subset of + // an existing watch (or vice-versa) but that's okay, the check() + // function will list all matches. plus, if we combine two watches such + // that only the larger set remains, it may confuse the user + if w.ai.address == nw.ai.address && + w.ai.read == nw.ai.read && + w.matchValue == nw.matchValue && w.value == nw.value { + + return errors.New(errors.CommandError, fmt.Sprintf("already being watched (%s)", w)) } } diff --git a/debugger/watches_test.go b/debugger/watches_test.go index 6f12248e..de391f92 100644 --- a/debugger/watches_test.go +++ b/debugger/watches_test.go @@ -11,11 +11,11 @@ func (trm *mockTerm) testWatches() { // try to re-add the same watch trm.sndInput("WATCH READ 0x80") - trm.cmpOutput("already being watched (0x0080 read)") + trm.cmpOutput("already being watched (0x0080 (RAM) read)") // list watches trm.sndInput("LIST WATCHES") - trm.cmpOutput(" 0: 0x0080 read") + trm.cmpOutput(" 0: 0x0080 (RAM) read") // try to re-add the same watch but with a different event selector trm.sndInput("WATCH WRITE 0x80") @@ -23,5 +23,34 @@ func (trm *mockTerm) testWatches() { // list watches trm.sndInput("LIST WATCHES") - trm.cmpOutput(" 0: 0x0080 read/write") + trm.cmpOutput(" 1: 0x0080 (RAM) write") + + // clear watches + trm.sndInput("CLEAR WATCHES") + trm.cmpOutput("watches cleared") + + // no watches after succesful clear + trm.sndInput("LIST WATCHES") + trm.cmpOutput("no watches") + + // try adding an invalid read address by symbol + trm.sndInput("WATCH READ VSYNC") + trm.cmpOutput("invalid watch address: VSYNC") + + // add address by symbol. no read/write modifier means it tries + trm.sndInput("WATCH VSYNC") + trm.cmpOutput("") + + // last item in list watches should be the new entry + trm.sndInput("LIST WATCHES") + trm.cmpOutput(" 0: 0x0000 (VSYNC) (TIA) write") + + // add address by symbol. no read/write modifier means it tries + // plus a specific value + trm.sndInput("WATCH VSYNC 0x1") + trm.cmpOutput("") + + // last item in list watches should be the new entry + trm.sndInput("LIST WATCHES") + trm.cmpOutput(" 1: 0x0000 (VSYNC) (TIA) write (value=0x01)") }