From 9098d1d85494810a21c6342c8f501dae8fc757d6 Mon Sep 17 00:00:00 2001
From: Austin Clements <austin@google.com>
Date: Mon, 19 Nov 2018 10:37:14 -0500
Subject: [PATCH] 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>
---
 src/cmd/compile/internal/gc/inl_test.go |  2 +-
 src/runtime/mgc.go                      |  6 +++++-
 src/runtime/mgcwork.go                  | 25 +++++++++++++++++++++++++
 3 files changed, 31 insertions(+), 2 deletions(-)

diff --git a/src/cmd/compile/internal/gc/inl_test.go b/src/cmd/compile/internal/gc/inl_test.go
index 5a8c19e2cb..ba74981e9a 100644
--- 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 d4e5d055de..db589c3f8f 100644
--- 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 f2f20fcdac..da2129ee50 100644
--- 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 {