proc/gdbserver: support watchpoints (#2659)

Adds watchpoint support to gdbserver backend for rr debugger and
debugserver on macOS/amd64 and macOS/arm64.

Also changes stack watchpoints to support reverse execution.

Documentation/backend_test_health.md

@@ -4,12 +4,9 @@ Tests skipped by each supported backend:
 	* 1 broken
 	* 3 broken - cgo stacktraces
 	* 3 not implemented
-* arm64 skipped = 5
+* arm64 skipped = 2
 	* 1 broken
 	* 1 broken - global variable symbolication
-	* 3 not implemented
-* darwin skipped = 3
-	* 3 not implemented
 * darwin/arm64 skipped = 1
 	* 1 broken - cgo stacktraces
 * darwin/lldb skipped = 1
@@ -19,12 +16,11 @@ Tests skipped by each supported backend:
 	* 4 not implemented
 * linux/386/pie skipped = 1
 	* 1 broken
-* linux/arm64 skipped = 1
+* linux/arm64 skipped = 4
 	* 1 broken - cgo stacktraces
+	* 3 not implemented
 * pie skipped = 2
 	* 2 upstream issue - https://github.com/golang/go/issues/29322
-* rr skipped = 3
-	* 3 not implemented
 * windows skipped = 2
 	* 1 broken
 	* 1 upstream issue

Documentation/cli/README.md

@@ -669,6 +669,8 @@ The memory location is specified with the same expression language used by 'prin
 
 will watch the address of variable 'v'.
 
+Note that writes that do not change the value of the watched memory address might not be reported.
+
 See also: "help print".
 
 

_fixtures/databpeasy.go

@@ -15,13 +15,13 @@ func main() { // Position 0
 	globalvar2 = globalvar1 + 1
 	globalvar1 = globalvar2 + 1
 	fmt.Printf("%d %d\n", globalvar1, globalvar2) // Position 1
-	runtime.Breakpoint()
+
 	globalvar2 = globalvar2 + 1          // Position 2
 	globalvar2 = globalvar1 + globalvar2 // Position 3
 	fmt.Printf("%d %d\n", globalvar1, globalvar2)
 	globalvar1 = globalvar2 + 1
 	fmt.Printf("%d %d\n", globalvar1, globalvar2)
-	runtime.Breakpoint()
+
 	done := make(chan struct{}) // Position 4
 	go f(done)
 	<-done
@@ -29,6 +29,6 @@ func main() { // Position 0
 
 func f(done chan struct{}) {
 	runtime.LockOSThread()
-	globalvar1 = globalvar2 + 1
+	globalvar1 = globalvar2 + 2
 	close(done) // Position 5
 }

_fixtures/databpstack.go

@@ -2,12 +2,12 @@ package main
 
 import (
 	"fmt"
-	"runtime"
+	//
 )
 
 func f() {
 	w := 0
-	runtime.Breakpoint()
+
 	g(1000, &w) // Position 0
 }
 

pkg/proc/gdbserial/gdbserver.go

@@ -174,8 +174,9 @@ type gdbThread struct {
 	regs              gdbRegisters
 	CurrentBreakpoint proc.BreakpointState
 	p                 *gdbProcess
-	sig               uint8 // signal received by thread after last stop
+	sig               uint8  // signal received by thread after last stop
-	setbp             bool  // thread was stopped because of a breakpoint
+	setbp             bool   // thread was stopped because of a breakpoint
+	watchAddr         uint64 // if > 0 this is the watchpoint address
 	common            proc.CommonThread
 }
 
@@ -225,6 +226,7 @@ func newProcess(process *os.Process) *gdbProcess {
 			inbuf:               make([]byte, 0, initialInputBufferSize),
 			direction:           proc.Forward,
 			log:                 logger,
+			goarch:              runtime.GOARCH,
 		},
 		threads:        make(map[int]*gdbThread),
 		bi:             proc.NewBinaryInfo(runtime.GOOS, runtime.GOARCH),
@@ -822,10 +824,9 @@ func (p *gdbProcess) ContinueOnce() (proc.Thread, proc.StopReason, error) {
 	var atstart bool
 continueLoop:
 	for {
-		var err error
-		var sig uint8
 		tu.Reset()
-		threadID, sig, err = p.conn.resume(p.threads, &tu)
+		sp, err := p.conn.resume(p.threads, &tu)
+		threadID = sp.threadID
 		if err != nil {
 			if _, exited := err.(proc.ErrProcessExited); exited {
 				p.exited = true
@@ -842,7 +843,8 @@ continueLoop:
 		if trapthread != nil && !p.threadStopInfo {
 			// For stubs that do not support qThreadStopInfo we manually set the
 			// reason the thread returned by resume() stopped.
-			trapthread.sig = sig
+			trapthread.sig = sp.sig
+			trapthread.watchAddr = sp.watchAddr
 		}
 
 		var shouldStop bool
@@ -1075,7 +1077,7 @@ func (p *gdbProcess) Restart(pos string) (proc.Thread, error) {
 
 	// for some reason we have to send a vCont;c after a vRun to make rr behave
 	// properly, because that's what gdb does.
-	_, _, err = p.conn.resume(nil, nil)
+	_, err = p.conn.resume(nil, nil)
 	if err != nil {
 		return nil, err
 	}
@@ -1087,8 +1089,8 @@ func (p *gdbProcess) Restart(pos string) (proc.Thread, error) {
 	p.clearThreadSignals()
 	p.clearThreadRegisters()
 
-	for addr := range p.breakpoints.M {
+	for _, bp := range p.breakpoints.M {
-		p.conn.setBreakpoint(addr, p.breakpointKind)
+		p.WriteBreakpoint(bp)
 	}
 
 	return p.currentThread, p.setCurrentBreakpoints()
@@ -1224,15 +1226,33 @@ func (p *gdbProcess) FindBreakpoint(pc uint64) (*proc.Breakpoint, bool) {
 	return nil, false
 }
 
-func (p *gdbProcess) WriteBreakpoint(bp *proc.Breakpoint) error {
+func watchTypeToBreakpointType(wtype proc.WatchType) breakpointType {
-	if bp.WatchType != 0 {
+	switch {
-		return errors.New("hardware breakpoints not supported")
+	case wtype.Read() && wtype.Write():
+		return accessWatchpoint
+	case wtype.Write():
+		return writeWatchpoint
+	case wtype.Read():
+		return readWatchpoint
+	default:
+		return swBreakpoint
 	}
-	return p.conn.setBreakpoint(bp.Addr, p.breakpointKind)
+}
+
+func (p *gdbProcess) WriteBreakpoint(bp *proc.Breakpoint) error {
+	kind := p.breakpointKind
+	if bp.WatchType != 0 {
+		kind = bp.WatchType.Size()
+	}
+	return p.conn.setBreakpoint(bp.Addr, watchTypeToBreakpointType(bp.WatchType), kind)
 }
 
 func (p *gdbProcess) EraseBreakpoint(bp *proc.Breakpoint) error {
-	return p.conn.clearBreakpoint(bp.Addr, p.breakpointKind)
+	kind := p.breakpointKind
+	if bp.WatchType != 0 {
+		kind = bp.WatchType.Size()
+	}
+	return p.conn.clearBreakpoint(bp.Addr, watchTypeToBreakpointType(bp.WatchType), kind)
 }
 
 type threadUpdater struct {
@@ -1324,7 +1344,7 @@ func (p *gdbProcess) updateThreadList(tu *threadUpdater) error {
 
 	for _, th := range p.threads {
 		if p.threadStopInfo {
-			sig, reason, err := p.conn.threadStopInfo(th.strID)
+			sp, err := p.conn.threadStopInfo(th.strID)
 			if err != nil {
 				if isProtocolErrorUnsupported(err) {
 					p.threadStopInfo = false
@@ -1332,10 +1352,12 @@ func (p *gdbProcess) updateThreadList(tu *threadUpdater) error {
 				}
 				return err
 			}
-			th.setbp = (reason == "breakpoint" || (reason == "" && sig == breakpointSignal))
+			th.setbp = (sp.reason == "breakpoint" || (sp.reason == "" && sp.sig == breakpointSignal) || (sp.watchAddr > 0))
-			th.sig = sig
+			th.sig = sp.sig
+			th.watchAddr = sp.watchAddr
 		} else {
 			th.sig = 0
+			th.watchAddr = 0
 		}
 	}
 
@@ -1452,12 +1474,12 @@ func (t *gdbThread) Common() *proc.CommonThread {
 // StepInstruction will step exactly 1 CPU instruction.
 func (t *gdbThread) StepInstruction() error {
 	pc := t.regs.PC()
-	if _, atbp := t.p.breakpoints.M[pc]; atbp {
+	if bp, atbp := t.p.breakpoints.M[pc]; atbp && bp.WatchType == 0 {
-		err := t.p.conn.clearBreakpoint(pc, t.p.breakpointKind)
+		err := t.p.conn.clearBreakpoint(pc, swBreakpoint, t.p.breakpointKind)
 		if err != nil {
 			return err
 		}
-		defer t.p.conn.setBreakpoint(pc, t.p.breakpointKind)
+		defer t.p.conn.setBreakpoint(pc, swBreakpoint, t.p.breakpointKind)
 	}
 	// Reset thread registers so the next call to
 	// Thread.Registers will not be cached.
@@ -1677,13 +1699,16 @@ func (t *gdbThread) reloadGAtPC() error {
 	// around by clearing and re-setting the breakpoint in a specific sequence
 	// with the memory writes.
 	// Additionally all breakpoints in [pc, pc+len(movinstr)] need to be removed
-	for addr := range t.p.breakpoints.M {
+	for addr, bp := range t.p.breakpoints.M {
+		if bp.WatchType != 0 {
+			continue
+		}
 		if addr >= pc && addr <= pc+uint64(len(movinstr)) {
-			err := t.p.conn.clearBreakpoint(addr, t.p.breakpointKind)
+			err := t.p.conn.clearBreakpoint(addr, swBreakpoint, t.p.breakpointKind)
 			if err != nil {
 				return err
 			}
-			defer t.p.conn.setBreakpoint(addr, t.p.breakpointKind)
+			defer t.p.conn.setBreakpoint(addr, swBreakpoint, t.p.breakpointKind)
 		}
 	}
 
@@ -1795,6 +1820,13 @@ func (t *gdbThread) SetCurrentBreakpoint(adjustPC bool) error {
 	// adjustPC is ignored, it is the stub's responsibiility to set the PC
 	// address correctly after hitting a breakpoint.
 	t.clearBreakpointState()
+	if t.watchAddr > 0 {
+		t.CurrentBreakpoint.Breakpoint = t.p.Breakpoints().M[t.watchAddr]
+		if t.CurrentBreakpoint.Breakpoint == nil {
+			return fmt.Errorf("could not find watchpoint at address %#x", t.watchAddr)
+		}
+		return nil
+	}
 	regs, err := t.Registers()
 	if err != nil {
 		return err

pkg/proc/gdbserial/gdbserver_conn.go

@@ -45,6 +45,7 @@ type gdbConn struct {
 	threadSuffixSupported bool // thread suffix supported by stub
 	isDebugserver         bool // true if the stub is debugserver
 	xcmdok                bool // x command can be used to transfer memory
+	goarch                string
 
 	log *logrus.Entry
 }
@@ -404,18 +405,28 @@ func (conn *gdbConn) qXfer(kind, annex string, binary bool) ([]byte, error) {
 	return out, nil
 }
 
+type breakpointType uint8
+
+const (
+	swBreakpoint     breakpointType = 0
+	hwBreakpoint     breakpointType = 1
+	writeWatchpoint  breakpointType = 2
+	readWatchpoint   breakpointType = 3
+	accessWatchpoint breakpointType = 4
+)
+
 // setBreakpoint executes a 'Z' (insert breakpoint) command of type '0' and kind '1' or '4'
-func (conn *gdbConn) setBreakpoint(addr uint64, kind int) error {
+func (conn *gdbConn) setBreakpoint(addr uint64, typ breakpointType, kind int) error {
 	conn.outbuf.Reset()
-	fmt.Fprintf(&conn.outbuf, "$Z0,%x,%d", addr, kind)
+	fmt.Fprintf(&conn.outbuf, "$Z%d,%x,%d", typ, addr, kind)
 	_, err := conn.exec(conn.outbuf.Bytes(), "set breakpoint")
 	return err
 }
 
 // clearBreakpoint executes a 'z' (remove breakpoint) command of type '0' and kind '1' or '4'
-func (conn *gdbConn) clearBreakpoint(addr uint64, kind int) error {
+func (conn *gdbConn) clearBreakpoint(addr uint64, typ breakpointType, kind int) error {
 	conn.outbuf.Reset()
-	fmt.Fprintf(&conn.outbuf, "$z0,%x,%d", addr, kind)
+	fmt.Fprintf(&conn.outbuf, "$z%d,%x,%d", typ, addr, kind)
 	_, err := conn.exec(conn.outbuf.Bytes(), "clear breakpoint")
 	return err
 }
@@ -537,7 +548,7 @@ func (conn *gdbConn) writeRegister(threadID string, regnum int, data []byte) err
 // resume each thread. If a thread has sig == 0 the 'c' action will be used,
 // otherwise the 'C' action will be used and the value of sig will be passed
 // to it.
-func (conn *gdbConn) resume(threads map[int]*gdbThread, tu *threadUpdater) (string, uint8, error) {
+func (conn *gdbConn) resume(threads map[int]*gdbThread, tu *threadUpdater) (stopPacket, error) {
 	if conn.direction == proc.Forward {
 		conn.outbuf.Reset()
 		fmt.Fprintf(&conn.outbuf, "$vCont")
@@ -549,7 +560,7 @@ func (conn *gdbConn) resume(threads map[int]*gdbThread, tu *threadUpdater) (stri
 		fmt.Fprintf(&conn.outbuf, ";c")
 	} else {
 		if err := conn.selectThread('c', "p-1.-1", "resume"); err != nil {
-			return "", 0, err
+			return stopPacket{}, err
 		}
 		conn.outbuf.Reset()
 		fmt.Fprint(&conn.outbuf, "$bc")
@@ -557,7 +568,7 @@ func (conn *gdbConn) resume(threads map[int]*gdbThread, tu *threadUpdater) (stri
 	conn.manualStopMutex.Lock()
 	if err := conn.send(conn.outbuf.Bytes()); err != nil {
 		conn.manualStopMutex.Unlock()
-		return "", 0, err
+		return stopPacket{}, err
 	}
 	conn.running = true
 	conn.manualStopMutex.Unlock()
@@ -584,7 +595,7 @@ func (conn *gdbConn) step(threadID string, tu *threadUpdater, ignoreFaultSignal
 		if err := conn.send(conn.outbuf.Bytes()); err != nil {
 			return err
 		}
-		_, _, err := conn.waitForvContStop("singlestep", threadID, tu)
+		_, err := conn.waitForvContStop("singlestep", threadID, tu)
 		return err
 	}
 	var sig uint8 = 0
@@ -601,8 +612,8 @@ func (conn *gdbConn) step(threadID string, tu *threadUpdater, ignoreFaultSignal
 		if tu != nil {
 			tu.Reset()
 		}
-		var err error
+		sp, err := conn.waitForvContStop("singlestep", threadID, tu)
-		_, sig, err = conn.waitForvContStop("singlestep", threadID, tu)
+		sig = sp.sig
 		if err != nil {
 			return err
 		}
@@ -626,7 +637,7 @@ func (conn *gdbConn) step(threadID string, tu *threadUpdater, ignoreFaultSignal
 
 var errThreadBlocked = errors.New("thread blocked")
 
-func (conn *gdbConn) waitForvContStop(context string, threadID string, tu *threadUpdater) (string, uint8, error) {
+func (conn *gdbConn) waitForvContStop(context, threadID string, tu *threadUpdater) (stopPacket, error) {
 	count := 0
 	failed := false
 	for {
@@ -647,24 +658,36 @@ func (conn *gdbConn) waitForvContStop(context string, threadID string, tu *threa
 			}
 			count++
 		} else if failed {
-			return "", 0, errThreadBlocked
+			return stopPacket{}, errThreadBlocked
 		} else if err != nil {
-			return "", 0, err
+			return stopPacket{}, err
 		} else {
 			repeat, sp, err := conn.parseStopPacket(resp, threadID, tu)
 			if !repeat {
-				return sp.threadID, sp.sig, err
+				return sp, err
 			}
 		}
 	}
 }
 
 type stopPacket struct {
-	threadID string
+	threadID  string
-	sig      uint8
+	sig       uint8
-	reason   string
+	reason    string
+	watchAddr uint64
 }
 
+// Mach exception codes used to decode metype/medata keys in stop packets (necessary to support watchpoints with debugserver).
+// See:
+//  https://opensource.apple.com/source/xnu/xnu-4570.1.46/osfmk/mach/exception_types.h.auto.html
+//  https://opensource.apple.com/source/xnu/xnu-4570.1.46/osfmk/mach/i386/exception.h.auto.html
+//  https://opensource.apple.com/source/xnu/xnu-4570.1.46/osfmk/mach/arm/exception.h.auto.html
+const (
+	_EXC_BREAKPOINT   = 6     // mach exception type for hardware breakpoints
+	_EXC_I386_SGL     = 1     // mach exception code for single step on x86, for some reason this is also used for watchpoints
+	_EXC_ARM_DA_DEBUG = 0x102 // mach exception code for debug fault on arm/arm64
+)
+
 // executes 'vCont' (continue/step) command
 func (conn *gdbConn) parseStopPacket(resp []byte, threadID string, tu *threadUpdater) (repeat bool, sp stopPacket, err error) {
 	switch resp[0] {
@@ -683,6 +706,9 @@ func (conn *gdbConn) parseStopPacket(resp []byte, threadID string, tu *threadUpd
 			conn.log.Debugf("full stop packet: %s", string(resp))
 		}
 
+		var metype int
+		var medata = make([]uint64, 0, 10)
+
 		buf := resp[3:]
 		for buf != nil {
 			colon := bytes.Index(buf, []byte{':'})
@@ -712,6 +738,32 @@ func (conn *gdbConn) parseStopPacket(resp []byte, threadID string, tu *threadUpd
 				}
 			case "reason":
 				sp.reason = string(value)
+			case "watch", "awatch", "rwatch":
+				sp.watchAddr, err = strconv.ParseUint(string(value), 16, 64)
+				if err != nil {
+					return false, stopPacket{}, fmt.Errorf("malformed stop packet: %s (wrong watch address)", string(resp))
+				}
+			case "metype":
+				// mach exception type (debugserver extension)
+				metype, _ = strconv.Atoi(string(value))
+			case "medata":
+				// mach exception data (debugserver extension)
+				d, _ := strconv.ParseUint(string(value), 16, 64)
+				medata = append(medata, d)
+			}
+		}
+
+		// Debugserver does not report watchpoint stops in the standard way preferring
+		// instead the semi-undocumented metype/medata keys.
+		// These values also have different meanings depending on the CPU architecture.
+		switch conn.goarch {
+		case "amd64":
+			if metype == _EXC_BREAKPOINT && len(medata) >= 2 && medata[0] == _EXC_I386_SGL {
+				sp.watchAddr = medata[1] // this should be zero if this is really a single step stop and non-zero for watchpoints
+			}
+		case "arm64":
+			if metype == _EXC_BREAKPOINT && len(medata) >= 2 && medata[0] == _EXC_ARM_DA_DEBUG {
+				sp.watchAddr = medata[1]
 			}
 		}
 
@@ -967,18 +1019,18 @@ func (conn *gdbConn) allocMemory(sz uint64) (uint64, error) {
 
 // threadStopInfo executes a 'qThreadStopInfo' and returns the reason the
 // thread stopped.
-func (conn *gdbConn) threadStopInfo(threadID string) (sig uint8, reason string, err error) {
+func (conn *gdbConn) threadStopInfo(threadID string) (sp stopPacket, err error) {
 	conn.outbuf.Reset()
 	fmt.Fprintf(&conn.outbuf, "$qThreadStopInfo%s", threadID)
 	resp, err := conn.exec(conn.outbuf.Bytes(), "thread stop info")
 	if err != nil {
-		return 0, "", err
+		return stopPacket{}, err
 	}
-	_, sp, err := conn.parseStopPacket(resp, "", nil)
+	_, sp, err = conn.parseStopPacket(resp, "", nil)
 	if err != nil {
-		return 0, "", err
+		return stopPacket{}, err
 	}
-	return sp.sig, sp.reason, nil
+	return sp, nil
 }
 
 // restart executes a 'vRun' command.

pkg/proc/proc_test.go

@@ -5364,13 +5364,22 @@ func TestVariablesWithExternalLinking(t *testing.T) {
 
 func TestWatchpointsBasic(t *testing.T) {
 	skipOn(t, "not implemented", "freebsd")
-	skipOn(t, "not implemented", "darwin")
 	skipOn(t, "not implemented", "386")
-	skipOn(t, "not implemented", "arm64")
+	skipOn(t, "not implemented", "linux", "arm64")
-	skipOn(t, "not implemented", "rr")
+	protest.AllowRecording(t)
+
+	position1 := 17
+	position5 := 33
+
+	if runtime.GOARCH == "arm64" {
+		position1 = 16
+		position5 = 32
+	}
 
 	withTestProcess("databpeasy", t, func(p *proc.Target, fixture protest.Fixture) {
 		setFunctionBreakpoint(p, t, "main.main")
+		setFileBreakpoint(p, t, fixture.Source, 19) // Position 2 breakpoint
+		setFileBreakpoint(p, t, fixture.Source, 25) // Position 4 breakpoint
 		assertNoError(p.Continue(), t, "Continue 0")
 		assertLineNumber(p, t, 11, "Continue 0") // Position 0
 
@@ -5381,7 +5390,11 @@ func TestWatchpointsBasic(t *testing.T) {
 		assertNoError(err, t, "SetDataBreakpoint(write-only)")
 
 		assertNoError(p.Continue(), t, "Continue 1")
-		assertLineNumber(p, t, 17, "Continue 1") // Position 1
+		assertLineNumber(p, t, position1, "Continue 1") // Position 1
+
+		if curbp := p.CurrentThread().Breakpoint().Breakpoint; curbp == nil || (curbp.LogicalID() != bp.LogicalID()) {
+			t.Fatal("breakpoint not set")
+		}
 
 		p.ClearBreakpoint(bp.Addr)
 
@@ -5399,22 +5412,21 @@ func TestWatchpointsBasic(t *testing.T) {
 		assertNoError(p.Continue(), t, "Continue 4")
 		assertLineNumber(p, t, 25, "Continue 4") // Position 4
 
+		t.Logf("setting final breakpoint")
 		_, err = p.SetWatchpoint(scope, "globalvar1", proc.WatchWrite, nil)
 		assertNoError(err, t, "SetDataBreakpoint(write-only, again)")
 
 		assertNoError(p.Continue(), t, "Continue 5")
-		assertLineNumber(p, t, 33, "Continue 5") // Position 5
+		assertLineNumber(p, t, position5, "Continue 5") // Position 5
 	})
 }
 
 func TestWatchpointCounts(t *testing.T) {
 	skipOn(t, "not implemented", "freebsd")
-	skipOn(t, "not implemented", "darwin")
 	skipOn(t, "not implemented", "386")
-	skipOn(t, "not implemented", "arm64")
+	skipOn(t, "not implemented", "linux", "arm64")
-	skipOn(t, "not implemented", "rr")
-
 	protest.AllowRecording(t)
+
 	withTestProcess("databpcountstest", t, func(p *proc.Target, fixture protest.Fixture) {
 		setFunctionBreakpoint(p, t, "main.main")
 		assertNoError(p.Continue(), t, "Continue 0")
@@ -5526,12 +5538,18 @@ func TestDwrapStartLocation(t *testing.T) {
 
 func TestWatchpointStack(t *testing.T) {
 	skipOn(t, "not implemented", "freebsd")
-	skipOn(t, "not implemented", "darwin")
 	skipOn(t, "not implemented", "386")
-	skipOn(t, "not implemented", "arm64")
+	skipOn(t, "not implemented", "linux", "arm64")
-	skipOn(t, "not implemented", "rr")
+	protest.AllowRecording(t)
+
+	position1 := 17
+
+	if runtime.GOARCH == "arm64" {
+		position1 = 16
+	}
 
 	withTestProcess("databpstack", t, func(p *proc.Target, fixture protest.Fixture) {
+		setFileBreakpoint(p, t, fixture.Source, 11) // Position 0 breakpoint
 		clearlen := len(p.Breakpoints().M)
 
 		assertNoError(p.Continue(), t, "Continue 0")
@@ -5543,8 +5561,13 @@ func TestWatchpointStack(t *testing.T) {
 		_, err = p.SetWatchpoint(scope, "w", proc.WatchWrite, nil)
 		assertNoError(err, t, "SetDataBreakpoint(write-only)")
 
-		if len(p.Breakpoints().M) != clearlen+3 {
+		watchbpnum := 3
-			// want 1 watchpoint, 1 stack resize breakpoint, 1 out of scope sentinel
+		if recorded, _ := p.Recorded(); recorded {
+			watchbpnum = 4
+		}
+
+		if len(p.Breakpoints().M) != clearlen+watchbpnum {
+			// want 1 watchpoint, 1 stack resize breakpoint, 1 out of scope sentinel (2 if recorded)
 			t.Errorf("wrong number of breakpoints after setting watchpoint: %d", len(p.Breakpoints().M)-clearlen)
 		}
 
@@ -5558,16 +5581,21 @@ func TestWatchpointStack(t *testing.T) {
 			}
 		}
 
+		// Note: for recorded processes retaddr will not always be the return
+		// address, ~50% of the times it will be the address of the CALL
+		// instruction preceding the return address, this does not matter for this
+		// test.
+
 		_, err = p.SetBreakpoint(retaddr, proc.UserBreakpoint, nil)
 		assertNoError(err, t, "SetBreakpoint")
 
-		if len(p.Breakpoints().M) != clearlen+3 {
+		if len(p.Breakpoints().M) != clearlen+watchbpnum {
-			// want 1 watchpoint, 1 stack resize breakpoint, 1 out of scope sentinel (which is also a user breakpoint)
+			// want 1 watchpoint, 1 stack resize breakpoint, 1 out of scope sentinel (which is also a user breakpoint) (and another out of scope sentinel if recorded)
 			t.Errorf("wrong number of breakpoints after setting watchpoint: %d", len(p.Breakpoints().M)-clearlen)
 		}
 
 		assertNoError(p.Continue(), t, "Continue 1")
-		assertLineNumber(p, t, 17, "Continue 1") // Position 1
+		assertLineNumber(p, t, position1, "Continue 1") // Position 1
 
 		assertNoError(p.Continue(), t, "Continue 2")
 		t.Logf("%#v", p.CurrentThread().Breakpoint().Breakpoint)
@@ -5591,3 +5619,52 @@ func TestWatchpointStack(t *testing.T) {
 		}
 	})
 }
+
+func TestWatchpointStackBackwardsOutOfScope(t *testing.T) {
+	skipUnlessOn(t, "only for recorded targets", "rr")
+	protest.AllowRecording(t)
+
+	withTestProcess("databpstack", t, func(p *proc.Target, fixture protest.Fixture) {
+		setFileBreakpoint(p, t, fixture.Source, 11) // Position 0 breakpoint
+		clearlen := len(p.Breakpoints().M)
+
+		assertNoError(p.Continue(), t, "Continue 0")
+		assertLineNumber(p, t, 11, "Continue 0") // Position 0
+
+		scope, err := proc.GoroutineScope(p, p.CurrentThread())
+		assertNoError(err, t, "GoroutineScope")
+
+		_, err = p.SetWatchpoint(scope, "w", proc.WatchWrite, nil)
+		assertNoError(err, t, "SetDataBreakpoint(write-only)")
+
+		assertNoError(p.Continue(), t, "Continue 1")
+		assertLineNumber(p, t, 17, "Continue 1") // Position 1
+
+		p.ChangeDirection(proc.Backward)
+
+		assertNoError(p.Continue(), t, "Continue 2")
+		t.Logf("%#v", p.CurrentThread().Breakpoint().Breakpoint)
+		assertLineNumber(p, t, 16, "Continue 2") // Position 1 again (because of inverted movement)
+
+		assertNoError(p.Continue(), t, "Continue 3")
+		t.Logf("%#v", p.CurrentThread().Breakpoint().Breakpoint)
+		assertLineNumber(p, t, 11, "Continue 3") // Position 0 (breakpoint 1 hit)
+
+		assertNoError(p.Continue(), t, "Continue 4")
+		t.Logf("%#v", p.CurrentThread().Breakpoint().Breakpoint)
+		assertLineNumber(p, t, 23, "Continue 4") // Position 2 (watchpoint gone out of scope)
+
+		if len(p.Breakpoints().M) != clearlen {
+			t.Errorf("wrong number of breakpoints after watchpoint goes out of scope: %d", len(p.Breakpoints().M)-clearlen)
+		}
+
+		if len(p.Breakpoints().WatchOutOfScope) != 1 {
+			t.Errorf("wrong number of out-of-scope watchpoints after watchpoint goes out of scope: %d", len(p.Breakpoints().WatchOutOfScope))
+		}
+
+		if len(p.Breakpoints().M) != clearlen {
+			// want 1 user breakpoint set at retaddr
+			t.Errorf("wrong number of breakpoints after removing user breakpoint: %d", len(p.Breakpoints().M)-clearlen)
+		}
+	})
+}

pkg/proc/stackwatch.go

@@ -72,6 +72,28 @@ func (t *Target) setStackWatchBreakpoints(scope *EvalScope, watchpoint *Breakpoi
 	retbreaklet.watchpoint = watchpoint
 	retbreaklet.callback = woos
 
+	if recorded, _ := t.Recorded(); recorded && retframe.Current.Fn != nil {
+		// Must also set a breakpoint on the call instruction immediately
+		// preceding retframe.Current.PC, because the watchpoint could also go out
+		// of scope while we are running backwards.
+		callerText, err := disassemble(t.Memory(), nil, t.Breakpoints(), t.BinInfo(), retframe.Current.Fn.Entry, retframe.Current.Fn.End, false)
+		if err != nil {
+			return err
+		}
+		for i, instr := range callerText {
+			if instr.Loc.PC == retframe.Current.PC && i > 0 {
+				retbp2, err := t.SetBreakpoint(callerText[i-1].Loc.PC, WatchOutOfScopeBreakpoint, retFrameCond)
+				if err != nil {
+					return err
+				}
+				retbreaklet2 := retbp2.Breaklets[len(retbp.Breaklets)-1]
+				retbreaklet2.watchpoint = watchpoint
+				retbreaklet2.callback = woos
+				break
+			}
+		}
+	}
+
 	// Stack Resize Sentinel
 
 	fn := t.BinInfo().LookupFunc["runtime.copystack"]

pkg/terminal/command.go

@@ -147,6 +147,8 @@ The memory location is specified with the same expression language used by 'prin
 
 will watch the address of variable 'v'.
 
+Note that writes that do not change the value of the watched memory address might not be reported.
+
 See also: "help print".`},
 		{aliases: []string{"restart", "r"}, group: runCmds, cmdFn: restart, helpMsg: `Restart process.