runtime: fix TestGCTestMoveStackOnNextCall flakes

gcTestMoveStackOnNextCall can fail to move the stack in very rare
cases if there's an unfortunately timed preemption that clobbers the
stack guard. This won't happen multiple times in quick succession, so
make the test just retry a few times.

Change-Id: I247dc0551514e269e7132cee7945291429b0e865
Reviewed-on: https://go-review.googlesource.com/c/go/+/306671
Trust: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
Reviewed-by: Than McIntosh <thanm@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>

diff --git a/src/runtime/gc_test.go b/src/runtime/gc_test.go
index 1ea1c2c745a4b7b3fc073c18fe9e3932b8149cb6..273f399864b90f9c1ff45384914a676ac8cdf69b 100644 (file)
--- a/src/runtime/gc_test.go
+++ b/src/runtime/gc_test.go
@@ -205,15 +205,24 @@ func TestGcZombieReporting(t *testing.T) {
 func TestGCTestMoveStackOnNextCall(t *testing.T) {
        t.Parallel()
        var onStack int
-       runtime.GCTestMoveStackOnNextCall()
-       moveStackCheck(t, &onStack, uintptr(unsafe.Pointer(&onStack)))
+       // GCTestMoveStackOnNextCall can fail in rare cases if there's
+       // a preemption. This won't happen many times in quick
+       // succession, so just retry a few times.
+       for retry := 0; retry < 5; retry++ {
+               runtime.GCTestMoveStackOnNextCall()
+               if moveStackCheck(t, &onStack, uintptr(unsafe.Pointer(&onStack))) {
+                       // Passed.
+                       return
+               }
+       }
+       t.Fatal("stack did not move")
 }
 
 // This must not be inlined because the point is to force a stack
 // growth check and move the stack.
 //
 //go:noinline
-func moveStackCheck(t *testing.T, new *int, old uintptr) {
+func moveStackCheck(t *testing.T, new *int, old uintptr) bool {
        // new should have been updated by the stack move;
        // old should not have.
 
@@ -228,8 +237,9 @@ func moveStackCheck(t *testing.T, new *int, old uintptr) {
                        t.Fatalf("test bug: new (%#x) should be a stack pointer, not %s", new2, cls)
                }
                // This was a real failure.
-               t.Fatal("stack did not move")
+               return false
        }
+       return true
 }
 
 func TestGCTestIsReachable(t *testing.T) {

diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go
index ecac354d83399ee68b106d6b9abe4ce725f77f7e..f0c03b5102855858e4ff3e39d17d6f7ffec0800c 100644 (file)
--- a/src/runtime/mgc.go
+++ b/src/runtime/mgc.go
@@ -2347,6 +2347,9 @@ func fmtNSAsMS(buf []byte, ns uint64) []byte {
 // if any other work appears after this call (such as returning).
 // Typically the following call should be marked go:noinline so it
 // performs a stack check.
+//
+// In rare cases this may not cause the stack to move, specifically if
+// there's a preemption between this call and the next.
 func gcTestMoveStackOnNextCall() {
        gp := getg()
        gp.stackguard0 = getcallersp()