proc: fix bug with stack watchpoints going out of scope

Documentation/backend_test_health.md

@@ -1,9 +1,9 @@
 Tests skipped by each supported backend:
 
-* 386 skipped = 7
+* 386 skipped = 8
 	* 1 broken
 	* 3 broken - cgo stacktraces
-	* 3 not implemented
+	* 4 not implemented
 * arm64 skipped = 1
 	* 1 broken - global variable symbolication
 * darwin skipped = 3
@@ -13,10 +13,10 @@ Tests skipped by each supported backend:
 	* 1 broken - cgo stacktraces
 * darwin/lldb skipped = 1
 	* 1 upstream issue
-* freebsd skipped = 10
+* freebsd skipped = 11
 	* 2 flaky
 	* 2 follow exec not implemented on freebsd
-	* 4 not implemented
+	* 5 not implemented
 	* 2 not working on freebsd
 * linux/386 skipped = 2
 	* 2 not working on linux/386
@@ -31,14 +31,14 @@ Tests skipped by each supported backend:
 	* 3 broken - pie mode
 * pie skipped = 2
 	* 2 upstream issue - https://github.com/golang/go/issues/29322
-* ppc64le skipped = 11
+* ppc64le skipped = 12
 	* 6 broken
 	* 1 broken - global variable symbolication
-	* 4 not implemented
-* windows skipped = 6
+	* 5 not implemented
+* windows skipped = 7
 	* 1 broken
 	* 2 not working on windows
-	* 3 see https://github.com/go-delve/delve/issues/2768
+	* 4 see https://github.com/go-delve/delve/issues/2768
 * windows/arm64 skipped = 5
 	* 3 broken
 	* 1 broken - cgo stacktraces

_fixtures/stackwatchbug.go

@@ -0,0 +1,20 @@
+package main
+
+import (
+	"fmt"
+)
+
+func multiRound() {
+	vars := []int{0, 1, 2, 3, 4, 5}
+	for i := range vars { // line 9: set watchpoints
+		if i > 0 {
+			vars[i] = vars[i-1]
+			fmt.Println() // line 12: watchpoints hit
+		}
+	}
+}
+
+func main() {
+	multiRound() // line 18: after restart, last watchpoint out of scope
+	return       // line 19: all watchpoints should go out of scope
+}

pkg/proc/amd64util/debugregs.go

@@ -115,7 +115,7 @@ func (drs *DebugRegisters) ClearBreakpoint(idx uint8) {
 // GetActiveBreakpoint returns the active hardware breakpoint and resets the
 // condition flags.
 func (drs *DebugRegisters) GetActiveBreakpoint() (ok bool, idx uint8) {
-	for idx := uint8(0); idx < 3; idx++ {
+	for idx := uint8(0); idx <= 3; idx++ {
 		enable := *(drs.pDR7) & (1 << enableBitOffset(idx))
 		if enable == 0 {
 			continue

pkg/proc/proc_test.go

@@ -6680,3 +6680,51 @@ func TestRangeOverFuncStepOut(t *testing.T) {
 		{contStepout, 237},
 	})
 }
+
+func TestStackwatchClearBug(t *testing.T) {
+	skipOn(t, "not implemented", "freebsd")
+	skipOn(t, "not implemented", "386")
+	skipOn(t, "not implemented", "ppc64le")
+	skipOn(t, "see https://github.com/go-delve/delve/issues/2768", "windows")
+
+	showbps := func(bps *proc.BreakpointMap) {
+		for _, bp := range bps.M {
+			t.Logf("\t%s\n", bp)
+		}
+	}
+
+	withTestProcess("stackwatchbug", t, func(p *proc.Target, grp *proc.TargetGroup, fixture protest.Fixture) {
+		setFileBreakpoint(p, t, fixture.Source, 9)
+		bpsBefore := p.Breakpoints()
+
+		assertNoError(grp.Continue(), t, "Continue 0")
+
+		scope, err := proc.GoroutineScope(p, p.CurrentThread())
+		assertNoError(err, t, "GoroutineScope")
+
+		for _, s := range []string{"vars[0]", "vars[3]", "vars[2]", "vars[1]"} {
+			_, err := p.SetWatchpoint(0, scope, s, proc.WatchWrite, nil)
+			assertNoError(err, t, "SetWatchpoint(write-only)")
+		}
+
+		t.Logf("After setting watchpoints:")
+		showbps(p.Breakpoints())
+
+		assertNoError(grp.Continue(), t, "Continue 1")
+		assertNoError(grp.Continue(), t, "Continue 2")
+		assertNoError(grp.Continue(), t, "Continue 3")
+		assertNoError(grp.Continue(), t, "Continue 4")
+		f, l := currentLineNumber(p, t)
+		t.Logf("at %s:%d", f, l)
+		if l != 19 {
+			t.Error("Wrong position after fourth continue")
+		}
+
+		bpsAfter := p.Breakpoints()
+		t.Logf("After watchpoint goes out of scope:")
+		showbps(bpsAfter)
+		if len(bpsBefore.M) != len(bpsAfter.M) {
+			t.Errorf("wrong number of breakpoints")
+		}
+	})
+}

pkg/proc/stackwatch.go

@@ -29,7 +29,7 @@ func (t *Target) setStackWatchBreakpoints(scope *EvalScope, watchpoint *Breakpoi
 	// Watchpoint Out-of-scope Sentinel
 
 	woos := func(_ Thread, _ *Target) (bool, error) {
-		watchpointOutOfScope(t, watchpoint)
+		t.Breakpoints().WatchOutOfScope = append(t.Breakpoints().WatchOutOfScope, watchpoint)
 		return true, nil
 	}
 
@@ -141,20 +141,6 @@ func (t *Target) clearStackWatchBreakpoints(watchpoint *Breakpoint) error {
 	return nil
 }
 
-// watchpointOutOfScope is called when the watchpoint goes out of scope. It
-// is used as a breaklet callback function.
-// Its responsibility is to delete the watchpoint and make sure that the
-// user is notified of the watchpoint going out of scope.
-func watchpointOutOfScope(t *Target, watchpoint *Breakpoint) {
-	t.Breakpoints().WatchOutOfScope = append(t.Breakpoints().WatchOutOfScope, watchpoint)
-	err := t.ClearBreakpoint(watchpoint.Addr)
-	if err != nil {
-		log := logflags.DebuggerLogger()
-		log.Errorf("could not clear out-of-scope watchpoint: %v", err)
-	}
-	delete(t.Breakpoints().Logical, watchpoint.LogicalID())
-}
-
 // adjustStackWatchpoint is called when the goroutine of watchpoint resizes
 // its stack. It is used as a breaklet callback function.
 // Its responsibility is to move the watchpoint to a its new address.

pkg/proc/target_exec.go

@@ -110,6 +110,14 @@ func (grp *TargetGroup) Continue() error {
 				}
 			}
 			it.currentThread = curthread
+			// Clear watchpoints that have gone out of scope
+			for _, watchpoint := range it.Breakpoints().WatchOutOfScope {
+				err := it.ClearBreakpoint(watchpoint.Addr)
+				if err != nil {
+					logflags.DebuggerLogger().Errorf("could not clear out-of-scope watchpoint: %v", err)
+				}
+				delete(it.Breakpoints().Logical, watchpoint.LogicalID())
+			}
 		}
 
 		if contOnceErr != nil {