mirror of
https://github.com/golang/go
synced 2024-09-04 15:34:21 +00:00
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>
This commit is contained in:
parent
9eee0ed439
commit
70fd4e47d7
|
@ -212,6 +212,8 @@ func cgocallbackg(fn, frame unsafe.Pointer, ctxt uintptr) {
|
||||||
// a different M. The call to unlockOSThread is in unwindm.
|
// a different M. The call to unlockOSThread is in unwindm.
|
||||||
lockOSThread()
|
lockOSThread()
|
||||||
|
|
||||||
|
checkm := gp.m
|
||||||
|
|
||||||
// Save current syscall parameters, so m.syscall can be
|
// Save current syscall parameters, so m.syscall can be
|
||||||
// used again if callback decide to make syscall.
|
// used again if callback decide to make syscall.
|
||||||
syscall := gp.m.syscall
|
syscall := gp.m.syscall
|
||||||
|
@ -227,15 +229,20 @@ func cgocallbackg(fn, frame unsafe.Pointer, ctxt uintptr) {
|
||||||
|
|
||||||
osPreemptExtExit(gp.m)
|
osPreemptExtExit(gp.m)
|
||||||
|
|
||||||
cgocallbackg1(fn, frame, ctxt)
|
cgocallbackg1(fn, frame, ctxt) // will call unlockOSThread
|
||||||
|
|
||||||
// At this point unlockOSThread has been called.
|
// At this point unlockOSThread has been called.
|
||||||
// The following code must not change to a different m.
|
// The following code must not change to a different m.
|
||||||
// This is enforced by checking incgo in the schedule function.
|
// 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)
|
osPreemptExtEnter(gp.m)
|
||||||
|
|
||||||
gp.m.incgo = true
|
|
||||||
// going back to cgo call
|
// going back to cgo call
|
||||||
reentersyscall(savedpc, uintptr(savedsp))
|
reentersyscall(savedpc, uintptr(savedsp))
|
||||||
|
|
||||||
|
@ -244,6 +251,11 @@ func cgocallbackg(fn, frame unsafe.Pointer, ctxt uintptr) {
|
||||||
|
|
||||||
func cgocallbackg1(fn, frame unsafe.Pointer, ctxt uintptr) {
|
func cgocallbackg1(fn, frame unsafe.Pointer, ctxt uintptr) {
|
||||||
gp := getg()
|
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 {
|
if gp.m.needextram || atomic.Load(&extraMWaiters) > 0 {
|
||||||
gp.m.needextram = false
|
gp.m.needextram = false
|
||||||
systemstack(newextram)
|
systemstack(newextram)
|
||||||
|
@ -323,10 +335,6 @@ func unwindm(restore *bool) {
|
||||||
|
|
||||||
releasem(mp)
|
releasem(mp)
|
||||||
}
|
}
|
||||||
|
|
||||||
// Undo the call to lockOSThread in cgocallbackg.
|
|
||||||
// We must still stay on the same m.
|
|
||||||
unlockOSThread()
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// called from assembly
|
// called from assembly
|
||||||
|
|
|
@ -282,6 +282,15 @@ func TestCgoTracebackContext(t *testing.T) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
func TestCgoTracebackContextPreemption(t *testing.T) {
|
||||||
|
t.Parallel()
|
||||||
|
got := runTestProg(t, "testprogcgo", "TracebackContextPreemption")
|
||||||
|
want := "OK\n"
|
||||||
|
if got != want {
|
||||||
|
t.Errorf("expected %q got %v", want, got)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
func testCgoPprof(t *testing.T, buildArg, runArg, top, bottom string) {
|
func testCgoPprof(t *testing.T, buildArg, runArg, top, bottom string) {
|
||||||
t.Parallel()
|
t.Parallel()
|
||||||
if runtime.GOOS != "linux" || (runtime.GOARCH != "amd64" && runtime.GOARCH != "ppc64le") {
|
if runtime.GOOS != "linux" || (runtime.GOARCH != "amd64" && runtime.GOARCH != "ppc64le") {
|
||||||
|
|
|
@ -2,8 +2,6 @@
|
||||||
// Use of this source code is governed by a BSD-style
|
// Use of this source code is governed by a BSD-style
|
||||||
// license that can be found in the LICENSE file.
|
// license that can be found in the LICENSE file.
|
||||||
|
|
||||||
// The __attribute__((weak)) used below doesn't seem to work on Windows.
|
|
||||||
|
|
||||||
package main
|
package main
|
||||||
|
|
||||||
// Test the context argument to SetCgoTraceback.
|
// Test the context argument to SetCgoTraceback.
|
||||||
|
@ -14,20 +12,24 @@ package main
|
||||||
extern void C1(void);
|
extern void C1(void);
|
||||||
extern void C2(void);
|
extern void C2(void);
|
||||||
extern void tcContext(void*);
|
extern void tcContext(void*);
|
||||||
|
extern void tcContextSimple(void*);
|
||||||
extern void tcTraceback(void*);
|
extern void tcTraceback(void*);
|
||||||
extern void tcSymbolizer(void*);
|
extern void tcSymbolizer(void*);
|
||||||
extern int getContextCount(void);
|
extern int getContextCount(void);
|
||||||
|
extern void TracebackContextPreemptionCallGo(int);
|
||||||
*/
|
*/
|
||||||
import "C"
|
import "C"
|
||||||
|
|
||||||
import (
|
import (
|
||||||
"fmt"
|
"fmt"
|
||||||
"runtime"
|
"runtime"
|
||||||
|
"sync"
|
||||||
"unsafe"
|
"unsafe"
|
||||||
)
|
)
|
||||||
|
|
||||||
func init() {
|
func init() {
|
||||||
register("TracebackContext", TracebackContext)
|
register("TracebackContext", TracebackContext)
|
||||||
|
register("TracebackContextPreemption", TracebackContextPreemption)
|
||||||
}
|
}
|
||||||
|
|
||||||
var tracebackOK bool
|
var tracebackOK bool
|
||||||
|
@ -105,3 +107,30 @@ wantLoop:
|
||||||
tracebackOK = false
|
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)
|
||||||
|
}
|
||||||
|
|
|
@ -11,6 +11,7 @@
|
||||||
// Functions exported from Go.
|
// Functions exported from Go.
|
||||||
extern void G1(void);
|
extern void G1(void);
|
||||||
extern void G2(void);
|
extern void G2(void);
|
||||||
|
extern void TracebackContextPreemptionGoFunction(int);
|
||||||
|
|
||||||
void C1() {
|
void C1() {
|
||||||
G1();
|
G1();
|
||||||
|
@ -62,10 +63,17 @@ void tcContext(void* parg) {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void tcContextSimple(void* parg) {
|
||||||
|
struct cgoContextArg* arg = (struct cgoContextArg*)(parg);
|
||||||
|
if (arg->context == 0) {
|
||||||
|
arg->context = 1;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
void tcTraceback(void* parg) {
|
void tcTraceback(void* parg) {
|
||||||
int base, i;
|
int base, i;
|
||||||
struct cgoTracebackArg* arg = (struct cgoTracebackArg*)(parg);
|
struct cgoTracebackArg* arg = (struct cgoTracebackArg*)(parg);
|
||||||
if (arg->context == 0) {
|
if (arg->context == 0 && arg->sigContext == 0) {
|
||||||
// This shouldn't happen in this program.
|
// This shouldn't happen in this program.
|
||||||
abort();
|
abort();
|
||||||
}
|
}
|
||||||
|
@ -89,3 +97,7 @@ void tcSymbolizer(void *parg) {
|
||||||
arg->func = "cFunction";
|
arg->func = "cFunction";
|
||||||
arg->lineno = arg->pc + (arg->more << 16);
|
arg->lineno = arg->pc + (arg->more << 16);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
void TracebackContextPreemptionCallGo(int i) {
|
||||||
|
TracebackContextPreemptionGoFunction(i);
|
||||||
|
}
|
||||||
|
|
Loading…
Reference in a new issue