runtime: debug code to catch bad gcWork.puts

This adds a debug check to throw immediately if any pointers are added
to the gcWork buffer after the mark completion barrier. The intent is
to catch the source of the cached GC work that occasionally produces
"P has cached GC work at end of mark termination" failures.

The result should be that we get "throwOnGCWork" throws instead of "P
has cached GC work at end of mark termination" throws, but with useful
stack traces.

This should be reverted before the release. I've been unable to
reproduce this issue locally, but this issue appears fairly regularly
on the builders, so the intent is to catch it on the builders.

This probably slows down the GC slightly.

For #27993.

Change-Id: I5035e14058ad313bfbd3d68c41ec05179147a85c
Reviewed-on: https://go-review.googlesource.com/c/149969
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

diff --git a/src/cmd/compile/internal/gc/inl_test.go b/src/cmd/compile/internal/gc/inl_test.go
index 5a8c19e2cb447a67ca3a2f0e9f1ea097088288a3..ba74981e9afbee67cc547de9dfb08721cfd8063b 100644 (file)
--- a/src/cmd/compile/internal/gc/inl_test.go
+++ b/src/cmd/compile/internal/gc/inl_test.go
@@ -85,7 +85,7 @@ func TestIntendedInlining(t *testing.T) {
                        "puintptr.ptr",
                        "spanOf",
                        "spanOfUnchecked",
-                       "(*gcWork).putFast",
+                       //"(*gcWork).putFast", // TODO(austin): For debugging #27993
                        "(*gcWork).tryGetFast",
                        "(*guintptr).set",
                        "(*markBits).advance",

diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go
index d4e5d055ded6feb2741842408b810f16ebf88cc4..db589c3f8fae6ad30aaa3267450719073da7b8ab 100644 (file)
--- a/src/runtime/mgc.go
+++ b/src/runtime/mgc.go
@@ -1431,6 +1431,8 @@ top:
                goto top
        }
 
+       throwOnGCWork = true
+
        // There was no global work, no local work, and no Ps
        // communicated work since we took markDoneSema. Therefore
        // there are no grey objects and no more objects can be
@@ -1924,7 +1926,7 @@ func gcMark(start_time int64) {
                // ensured all reachable objects were marked, all of
                // these must be pointers to black objects. Hence we
                // can just discard the write barrier buffer.
-               if debug.gccheckmark > 0 {
+               if debug.gccheckmark > 0 || throwOnGCWork {
                        // For debugging, flush the buffer and make
                        // sure it really was all marked.
                        wbBufFlush1(p)
@@ -1956,6 +1958,8 @@ func gcMark(start_time int64) {
                gcw.dispose()
        }
 
+       throwOnGCWork = false
+
        cachestats()
 
        // Update the marked heap stat.

diff --git a/src/runtime/mgcwork.go b/src/runtime/mgcwork.go
index f2f20fcdac62608718bfdd2c8ff7fb2655bc77b3..da2129ee508dd19c29cfe6cfa09b1a94ffa490f2 100644 (file)
--- a/src/runtime/mgcwork.go
+++ b/src/runtime/mgcwork.go
@@ -22,6 +22,13 @@ const (
        workbufAlloc = 32 << 10
 )
 
+// throwOnGCWork causes any operations that add pointers to a gcWork
+// buffer to throw.
+//
+// TODO(austin): This is a temporary debugging measure for issue
+// #27993. To be removed before release.
+var throwOnGCWork bool
+
 func init() {
        if workbufAlloc%pageSize != 0 || workbufAlloc%_WorkbufSize != 0 {
                throw("bad workbufAlloc")
@@ -108,6 +115,10 @@ func (w *gcWork) init() {
 // obj must point to the beginning of a heap object or an oblet.
 //go:nowritebarrierrec
 func (w *gcWork) put(obj uintptr) {
+       if throwOnGCWork {
+               throw("throwOnGCWork")
+       }
+
        flushed := false
        wbuf := w.wbuf1
        if wbuf == nil {
@@ -142,6 +153,10 @@ func (w *gcWork) put(obj uintptr) {
 // otherwise it returns false and the caller needs to call put.
 //go:nowritebarrierrec
 func (w *gcWork) putFast(obj uintptr) bool {
+       if throwOnGCWork {
+               throw("throwOnGCWork")
+       }
+
        wbuf := w.wbuf1
        if wbuf == nil {
                return false
@@ -163,6 +178,10 @@ func (w *gcWork) putBatch(obj []uintptr) {
                return
        }
 
+       if throwOnGCWork {
+               throw("throwOnGCWork")
+       }
+
        flushed := false
        wbuf := w.wbuf1
        if wbuf == nil {
@@ -284,10 +303,16 @@ func (w *gcWork) balance() {
                return
        }
        if wbuf := w.wbuf2; wbuf.nobj != 0 {
+               if throwOnGCWork {
+                       throw("throwOnGCWork")
+               }
                putfull(wbuf)
                w.flushedWork = true
                w.wbuf2 = getempty()
        } else if wbuf := w.wbuf1; wbuf.nobj > 4 {
+               if throwOnGCWork {
+                       throw("throwOnGCWork")
+               }
                w.wbuf1 = handoff(wbuf)
                w.flushedWork = true // handoff did putfull
        } else {