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, 31 insertions, 2 deletions

diff --git a/src/runtime/testdata/testprogcgo/tracebackctxt.go b/src/runtime/testdata/testprogcgo/tracebackctxt.go
index 51fa4ad25c..62ff8eccd6 100644
--- a/src/runtime/testdata/testprogcgo/tracebackctxt.go
+++ b/src/runtime/testdata/testprogcgo/tracebackctxt.go
@@ -2,8 +2,6 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// The __attribute__((weak)) used below doesn't seem to work on Windows.
-
 package main
 
 // Test the context argument to SetCgoTraceback.
@@ -14,20 +12,24 @@ package main
 extern void C1(void);
 extern void C2(void);
 extern void tcContext(void*);
+extern void tcContextSimple(void*);
 extern void tcTraceback(void*);
 extern void tcSymbolizer(void*);
 extern int getContextCount(void);
+extern void TracebackContextPreemptionCallGo(int);
 */
 import "C"
 
 import (
 	"fmt"
 	"runtime"
+	"sync"
 	"unsafe"
 )
 
 func init() {
 	register("TracebackContext", TracebackContext)
+	register("TracebackContextPreemption", TracebackContextPreemption)
 }
 
 var tracebackOK bool
@@ -105,3 +107,30 @@ wantLoop:
 		tracebackOK = false
 	}
 }
+
+// Issue 47441.
+func TracebackContextPreemption() {
+	runtime.SetCgoTraceback(0, unsafe.Pointer(C.tcTraceback), unsafe.Pointer(C.tcContextSimple), unsafe.Pointer(C.tcSymbolizer))
+
+	const funcs = 10
+	const calls = 1e5
+	var wg sync.WaitGroup
+	for i := 0; i < funcs; i++ {
+		wg.Add(1)
+		go func(i int) {
+			defer wg.Done()
+			for j := 0; j < calls; j++ {
+				C.TracebackContextPreemptionCallGo(C.int(i*calls + j))
+			}
+		}(i)
+	}
+	wg.Wait()
+
+	fmt.Println("OK")
+}
+
+//export TracebackContextPreemptionGoFunction
+func TracebackContextPreemptionGoFunction(i C.int) {
+	// Do some busy work.
+	fmt.Sprintf("%d\n", i)
+}