runtime: avoid possible preemption when returning from Go to C

When returning from Go to C, it was possible for the goroutine to be
preempted after calling unlockOSThread. This could happen when there
a context function installed by SetCgoTraceback set a non-zero context,
leading to a defer call in cgocallbackg1. The defer function wrapper,
introduced in 1.17 as part of the regabi support, was not nosplit,
and hence was a potential preemption point. If it did get preempted,
the G would move to a new M. It would then attempt to return to C
code on a different stack, typically leading to a SIGSEGV.

Fix this in a simple way by postponing the unlockOSThread until after
the other defer. Also check for the failure condition and fail early,
rather than waiting for a SIGSEGV.

Without the fix to cgocall.go, the test case fails about 50% of the
time on my laptop.

Fixes #47441

Change-Id: Ib8ca13215bd36cddc2a49e86698824a29c6a68ba
Reviewed-on: https://go-review.googlesource.com/c/go/+/338197
Trust: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>

1 files changed, 14 insertions, 6 deletions

diff --git a/src/runtime/cgocall.go b/src/runtime/cgocall.go
index 8ffb48a888..2626216f95 100644
--- a/src/runtime/cgocall.go
+++ b/src/runtime/cgocall.go
@@ -212,6 +212,8 @@ func cgocallbackg(fn, frame unsafe.Pointer, ctxt uintptr) {
 	// a different M. The call to unlockOSThread is in unwindm.
 	lockOSThread()
 
+	checkm := gp.m
+
 	// Save current syscall parameters, so m.syscall can be
 	// used again if callback decide to make syscall.
 	syscall := gp.m.syscall
@@ -227,15 +229,20 @@ func cgocallbackg(fn, frame unsafe.Pointer, ctxt uintptr) {
 
 	osPreemptExtExit(gp.m)
 
-	cgocallbackg1(fn, frame, ctxt)
+	cgocallbackg1(fn, frame, ctxt) // will call unlockOSThread
 
 	// At this point unlockOSThread has been called.
 	// The following code must not change to a different m.
 	// This is enforced by checking incgo in the schedule function.
 
+	gp.m.incgo = true
+
+	if gp.m != checkm {
+		throw("m changed unexpectedly in cgocallbackg")
+	}
+
 	osPreemptExtEnter(gp.m)
 
-	gp.m.incgo = true
 	// going back to cgo call
 	reentersyscall(savedpc, uintptr(savedsp))
 
@@ -244,6 +251,11 @@ func cgocallbackg(fn, frame unsafe.Pointer, ctxt uintptr) {
 
 func cgocallbackg1(fn, frame unsafe.Pointer, ctxt uintptr) {
 	gp := getg()
+
+	// When we return, undo the call to lockOSThread in cgocallbackg.
+	// We must still stay on the same m.
+	defer unlockOSThread()
+
 	if gp.m.needextram || atomic.Load(&extraMWaiters) > 0 {
 		gp.m.needextram = false
 		systemstack(newextram)
@@ -323,10 +335,6 @@ func unwindm(restore *bool) {
 
 		releasem(mp)
 	}
-
-	// Undo the call to lockOSThread in cgocallbackg.
-	// We must still stay on the same m.
-	unlockOSThread()
 }
 
 // called from assembly