runtime: never call into race detector with retaken P

cgocall could previously invoke the race detector on an M whose P had
been retaken. The race detector would attempt to use the P-local state
from this stale P, racing with the thread that was actually wired to
that P. The result was memory corruption of ThreadSanitizer's internal
data structures that presented as hard-to-understand assertion failures
and segfaults.

Reorder cgocall so that it always acquires a P before invoking the race
detector, and add a test that stresses the interaction between cgo and
the race detector to protect against future bugs of this kind.

Fixes #27660.

Change-Id: Ide93f96a23490314d6647547140e0a412a97f0d4
Reviewed-on: https://go-review.googlesource.com/c/148717
Run-TryBot: Dmitry Vyukov <dvyukov@google.com>
Reviewed-by: Dmitry Vyukov <dvyukov@google.com>

1 files changed, 13 insertions, 15 deletions

diff --git a/src/runtime/cgocall.go b/src/runtime/cgocall.go
index 86bd2fb01c..ca31408b50 100644
--- a/src/runtime/cgocall.go
+++ b/src/runtime/cgocall.go
@@ -130,12 +130,19 @@ func cgocall(fn, arg unsafe.Pointer) int32 {
 	mp.incgo = true
 	errno := asmcgocall(fn, arg)
 
-	// Call endcgo before exitsyscall because exitsyscall may
+	// Update accounting before exitsyscall because exitsyscall may
 	// reschedule us on to a different M.
-	endcgo(mp)
+	mp.incgo = false
+	mp.ncgo--
 
 	exitsyscall()
 
+	// Note that raceacquire must be called only after exitsyscall has
+	// wired this M to a P.
+	if raceenabled {
+		raceacquire(unsafe.Pointer(&racecgosync))
+	}
+
 	// From the garbage collector's perspective, time can move
 	// backwards in the sequence above. If there's a callback into
 	// Go code, GC will see this function at the call to
@@ -153,16 +160,6 @@ func cgocall(fn, arg unsafe.Pointer) int32 {
 	return errno
 }
 
-//go:nosplit
-func endcgo(mp *m) {
-	mp.incgo = false
-	mp.ncgo--
-
-	if raceenabled {
-		raceacquire(unsafe.Pointer(&racecgosync))
-	}
-}
-
 // Call from C back to Go.
 //go:nosplit
 func cgocallbackg(ctxt uintptr) {
@@ -347,13 +344,14 @@ func unwindm(restore *bool) {
 			sched.sp = *(*uintptr)(unsafe.Pointer(sched.sp + 16))
 		}
 
-		// Call endcgo to do the accounting that cgocall will not have a
-		// chance to do during an unwind.
+		// Do the accounting that cgocall will not have a chance to do
+		// during an unwind.
 		//
 		// In the case where a Go call originates from C, ncgo is 0
 		// and there is no matching cgocall to end.
 		if mp.ncgo > 0 {
-			endcgo(mp)
+			mp.incgo = false
+			mp.ncgo--
 		}
 
 		releasem(mp)