From: Ian Lance Taylor Date: Thu, 21 Jan 2016 20:38:05 +0000 (-0800) Subject: runtime: on NetBSD and DragonFly drop signal stack in new thread X-Git-Tag: go1.6rc1~78 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=4c4476c297e0a43bf92e8303da369cdc18e5745c;p=gostls13.git runtime: on NetBSD and DragonFly drop signal stack in new thread On NetBSD and DragonFly a newly created thread inherits the signal stack of the creating thread. This breaks horribly if both threads get a signal at the same time. Fix this by dropping the signal stack in the newly created thread. The right signal stack will then get installed later. Note that cgo code that calls pthread_create will have the wrong, duplicated, signal stack in the newly created thread. I don't see any way to fix that in Go. People using cgo to call pthread_create will have to be aware of the problem. Fixes #13945. Fixes #13947. Change-Id: I0c7bd2cdf9ada575d57182ca5e9523060de34931 Reviewed-on: https://go-review.googlesource.com/18814 TryBot-Result: Gobot Gobot Reviewed-by: Brad Fitzpatrick Reviewed-by: Russ Cox --- diff --git a/src/runtime/cgo/gcc_dragonfly_amd64.c b/src/runtime/cgo/gcc_dragonfly_amd64.c index f79f652e46..f41b9b408a 100644 --- a/src/runtime/cgo/gcc_dragonfly_amd64.c +++ b/src/runtime/cgo/gcc_dragonfly_amd64.c @@ -56,6 +56,7 @@ static void* threadentry(void *v) { ThreadStart ts; + stack_t ss; ts = *(ThreadStart*)v; free(v); @@ -65,6 +66,17 @@ threadentry(void *v) */ setg_gcc((void*)ts.g); + // On DragonFly, a new thread inherits the signal stack of the + // creating thread. That confuses minit, so we remove that + // signal stack here before calling the regular mstart. It's + // a bit baroque to remove a signal stack here only to add one + // in minit, but it's a simple change that keeps DragonFly + // working like other OS's. At this point all signals are + // blocked, so there is no race. + memset(&ss, 0, sizeof ss); + ss.ss_flags = SS_DISABLE; + sigaltstack(&ss, nil); + crosscall_amd64(ts.fn); return nil; } diff --git a/src/runtime/cgo/gcc_netbsd_386.c b/src/runtime/cgo/gcc_netbsd_386.c index 2505e6dc7c..6fc7a122b4 100644 --- a/src/runtime/cgo/gcc_netbsd_386.c +++ b/src/runtime/cgo/gcc_netbsd_386.c @@ -55,6 +55,7 @@ static void* threadentry(void *v) { ThreadStart ts; + stack_t ss; ts = *(ThreadStart*)v; free(v); @@ -64,6 +65,17 @@ threadentry(void *v) */ setg_gcc((void*)ts.g); + // On NetBSD, a new thread inherits the signal stack of the + // creating thread. That confuses minit, so we remove that + // signal stack here before calling the regular mstart. It's + // a bit baroque to remove a signal stack here only to add one + // in minit, but it's a simple change that keeps NetBSD + // working like other OS's. At this point all signals are + // blocked, so there is no race. + memset(&ss, 0, sizeof ss); + ss.ss_flags = SS_DISABLE; + sigaltstack(&ss, nil); + crosscall_386(ts.fn); return nil; } diff --git a/src/runtime/cgo/gcc_netbsd_amd64.c b/src/runtime/cgo/gcc_netbsd_amd64.c index 8f646502d7..f0ecfac575 100644 --- a/src/runtime/cgo/gcc_netbsd_amd64.c +++ b/src/runtime/cgo/gcc_netbsd_amd64.c @@ -56,6 +56,7 @@ static void* threadentry(void *v) { ThreadStart ts; + stack_t ss; ts = *(ThreadStart*)v; free(v); @@ -65,6 +66,17 @@ threadentry(void *v) */ setg_gcc((void*)ts.g); + // On NetBSD, a new thread inherits the signal stack of the + // creating thread. That confuses minit, so we remove that + // signal stack here before calling the regular mstart. It's + // a bit baroque to remove a signal stack here only to add one + // in minit, but it's a simple change that keeps NetBSD + // working like other OS's. At this point all signals are + // blocked, so there is no race. + memset(&ss, 0, sizeof ss); + ss.ss_flags = SS_DISABLE; + sigaltstack(&ss, nil); + crosscall_amd64(ts.fn); return nil; } diff --git a/src/runtime/cgo/gcc_netbsd_arm.c b/src/runtime/cgo/gcc_netbsd_arm.c index 7a98c0de24..3567aaae72 100644 --- a/src/runtime/cgo/gcc_netbsd_arm.c +++ b/src/runtime/cgo/gcc_netbsd_arm.c @@ -57,10 +57,22 @@ static void* threadentry(void *v) { ThreadStart ts; + stack_t ss; ts = *(ThreadStart*)v; free(v); + // On NetBSD, a new thread inherits the signal stack of the + // creating thread. That confuses minit, so we remove that + // signal stack here before calling the regular mstart. It's + // a bit baroque to remove a signal stack here only to add one + // in minit, but it's a simple change that keeps NetBSD + // working like other OS's. At this point all signals are + // blocked, so there is no race. + memset(&ss, 0, sizeof ss); + ss.ss_flags = SS_DISABLE; + sigaltstack(&ss, nil); + crosscall_arm1(ts.fn, setg_gcc, (void*)ts.g); return nil; } diff --git a/src/runtime/os1_netbsd.go b/src/runtime/os1_netbsd.go index 42199020e5..9ab39ba97d 100644 --- a/src/runtime/os1_netbsd.go +++ b/src/runtime/os1_netbsd.go @@ -104,7 +104,7 @@ func newosproc(mp *m, stk unsafe.Pointer) { uc.uc_link = nil uc.uc_sigmask = sigset_all - lwp_mcontext_init(&uc.uc_mcontext, stk, mp, mp.g0, funcPC(mstart)) + lwp_mcontext_init(&uc.uc_mcontext, stk, mp, mp.g0, funcPC(netbsdMstart)) ret := lwp_create(unsafe.Pointer(&uc), 0, unsafe.Pointer(&mp.procid)) if ret < 0 { @@ -113,6 +113,19 @@ func newosproc(mp *m, stk unsafe.Pointer) { } } +// netbsdMStart is the function call that starts executing a newly +// created thread. On NetBSD, a new thread inherits the signal stack +// of the creating thread. That confuses minit, so we remove that +// signal stack here before calling the regular mstart. It's a bit +// baroque to remove a signal stack here only to add one in minit, but +// it's a simple change that keeps NetBSD working like other OS's. +// At this point all signals are blocked, so there is no race. +//go:nosplit +func netbsdMstart() { + signalstack(nil) + mstart() +} + func osinit() { ncpu = getncpu() } diff --git a/src/runtime/sys_dragonfly_amd64.s b/src/runtime/sys_dragonfly_amd64.s index d1b94e1bfd..4e4d793c43 100644 --- a/src/runtime/sys_dragonfly_amd64.s +++ b/src/runtime/sys_dragonfly_amd64.s @@ -51,6 +51,18 @@ TEXT runtime·lwp_start(SB),NOSPLIT,$0 MOVQ R13, g_m(DI) MOVQ DI, g(CX) + // On DragonFly, a new thread inherits the signal stack of the + // creating thread. That confuses minit, so we remove that + // signal stack here before calling the regular mstart. It's + // a bit baroque to remove a signal stack here only to add one + // in minit, but it's a simple change that keeps DragonFly + // working like other OS's. At this point all signals are + // blocked, so there is no race. + SUBQ $8, SP + MOVQ $0, 0(SP) + CALL runtime·signalstack(SB) + ADDQ $8, SP + CALL runtime·stackcheck(SB) CALL runtime·mstart(SB)