Skip to content

Commit fded5db

Browse files
runtime: don't crash if signal delivered on g0 stack
Also, if we changed the gsignal stack to match the stack we are executing on, restore it when returning from the signal handler, for safety. Fixes #18255. Change-Id: Ic289b36e4e38a56f8a6d4b5d74f68121c242e81a Reviewed-on: https://go-review.googlesource.com/34239 Run-TryBot: Ian Lance Taylor <iant@golang.org> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Bryan Mills <bcmills@google.com> Reviewed-by: David Crawshaw <crawshaw@golang.org>
1 parent 265e547 commit fded5db

File tree

3 files changed

+131
-18
lines changed

3 files changed

+131
-18
lines changed

misc/cgo/testsanitizers/test.bash

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -145,6 +145,7 @@ if test "$tsan" = "yes"; then
145145
testtsan tsan3.go
146146
testtsan tsan4.go
147147
testtsan tsan8.go
148+
testtsan tsan9.go
148149

149150
# These tests are only reliable using clang or GCC version 7 or later.
150151
# Otherwise runtime/cgo/libcgo.h can't tell whether TSAN is in use.

misc/cgo/testsanitizers/tsan9.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// Copyright 2016 The Go Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style
3+
// license that can be found in the LICENSE file.
4+
5+
package main
6+
7+
// This program failed when run under the C/C++ ThreadSanitizer. The
8+
// TSAN library was not keeping track of whether signals should be
9+
// delivered on the alternate signal stack.
10+
11+
/*
12+
#cgo CFLAGS: -g -fsanitize=thread
13+
#cgo LDFLAGS: -g -fsanitize=thread
14+
15+
#include <stdlib.h>
16+
#include <sys/time.h>
17+
18+
void spin() {
19+
size_t n;
20+
struct timeval tvstart, tvnow;
21+
int diff;
22+
23+
gettimeofday(&tvstart, NULL);
24+
for (n = 0; n < 1<<20; n++) {
25+
free(malloc(n));
26+
gettimeofday(&tvnow, NULL);
27+
diff = (tvnow.tv_sec - tvstart.tv_sec) * 1000 * 1000 + (tvnow.tv_usec - tvstart.tv_usec);
28+
29+
// Profile frequency is 100Hz so we should definitely
30+
// get a signal in 50 milliseconds.
31+
if (diff > 50 * 1000) {
32+
break;
33+
}
34+
}
35+
}
36+
*/
37+
import "C"
38+
39+
import (
40+
"io/ioutil"
41+
"runtime/pprof"
42+
"time"
43+
)
44+
45+
func goSpin() {
46+
start := time.Now()
47+
for n := 0; n < 1<<20; n++ {
48+
_ = make([]byte, n)
49+
if time.Since(start) > 50*time.Millisecond {
50+
break
51+
}
52+
}
53+
}
54+
55+
func main() {
56+
pprof.StartCPUProfile(ioutil.Discard)
57+
go C.spin()
58+
goSpin()
59+
pprof.StopCPUProfile()
60+
}

src/runtime/signal_unix.go

Lines changed: 70 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -212,32 +212,53 @@ func sigtrampgo(sig uint32, info *siginfo, ctx unsafe.Pointer) {
212212
}
213213

214214
// If some non-Go code called sigaltstack, adjust.
215+
setStack := false
216+
var gsignalStack gsignalStack
215217
sp := uintptr(unsafe.Pointer(&sig))
216218
if sp < g.m.gsignal.stack.lo || sp >= g.m.gsignal.stack.hi {
217-
var st stackt
218-
sigaltstack(nil, &st)
219-
if st.ss_flags&_SS_DISABLE != 0 {
220-
setg(nil)
221-
needm(0)
222-
noSignalStack(sig)
223-
dropm()
224-
}
225-
stsp := uintptr(unsafe.Pointer(st.ss_sp))
226-
if sp < stsp || sp >= stsp+st.ss_size {
227-
setg(nil)
228-
needm(0)
229-
sigNotOnStack(sig)
230-
dropm()
219+
if sp >= g.m.g0.stack.lo && sp < g.m.g0.stack.hi {
220+
// The signal was delivered on the g0 stack.
221+
// This can happen when linked with C code
222+
// using the thread sanitizer, which collects
223+
// signals then delivers them itself by calling
224+
// the signal handler directly when C code,
225+
// including C code called via cgo, calls a
226+
// TSAN-intercepted function such as malloc.
227+
st := stackt{ss_size: g.m.g0.stack.hi - g.m.g0.stack.lo}
228+
setSignalstackSP(&st, g.m.g0.stack.lo)
229+
setGsignalStack(&st, &gsignalStack)
230+
g.m.gsignal.stktopsp = getcallersp(unsafe.Pointer(&sig))
231+
setStack = true
232+
} else {
233+
var st stackt
234+
sigaltstack(nil, &st)
235+
if st.ss_flags&_SS_DISABLE != 0 {
236+
setg(nil)
237+
needm(0)
238+
noSignalStack(sig)
239+
dropm()
240+
}
241+
stsp := uintptr(unsafe.Pointer(st.ss_sp))
242+
if sp < stsp || sp >= stsp+st.ss_size {
243+
setg(nil)
244+
needm(0)
245+
sigNotOnStack(sig)
246+
dropm()
247+
}
248+
setGsignalStack(&st, &gsignalStack)
249+
g.m.gsignal.stktopsp = getcallersp(unsafe.Pointer(&sig))
250+
setStack = true
231251
}
232-
setGsignalStack(&st)
233-
g.m.gsignal.stktopsp = getcallersp(unsafe.Pointer(&sig))
234252
}
235253

236254
setg(g.m.gsignal)
237255
c := &sigctxt{info, ctx}
238256
c.fixsigcode(sig)
239257
sighandler(sig, info, ctx, g)
240258
setg(g)
259+
if setStack {
260+
restoreGsignalStack(&gsignalStack)
261+
}
241262
}
242263

243264
// sigpanic turns a synchronous signal into a run-time panic.
@@ -585,7 +606,7 @@ func minitSignalStack() {
585606
signalstack(&_g_.m.gsignal.stack)
586607
_g_.m.newSigstack = true
587608
} else {
588-
setGsignalStack(&st)
609+
setGsignalStack(&st, nil)
589610
_g_.m.newSigstack = false
590611
}
591612
}
@@ -618,14 +639,32 @@ func unminitSignals() {
618639
}
619640
}
620641

642+
// gsignalStack saves the fields of the gsignal stack changed by
643+
// setGsignalStack.
644+
type gsignalStack struct {
645+
stack stack
646+
stackguard0 uintptr
647+
stackguard1 uintptr
648+
stackAlloc uintptr
649+
stktopsp uintptr
650+
}
651+
621652
// setGsignalStack sets the gsignal stack of the current m to an
622653
// alternate signal stack returned from the sigaltstack system call.
654+
// It saves the old values in *old for use by restoreGsignalStack.
623655
// This is used when handling a signal if non-Go code has set the
624656
// alternate signal stack.
625657
//go:nosplit
626658
//go:nowritebarrierrec
627-
func setGsignalStack(st *stackt) {
659+
func setGsignalStack(st *stackt, old *gsignalStack) {
628660
g := getg()
661+
if old != nil {
662+
old.stack = g.m.gsignal.stack
663+
old.stackguard0 = g.m.gsignal.stackguard0
664+
old.stackguard1 = g.m.gsignal.stackguard1
665+
old.stackAlloc = g.m.gsignal.stackAlloc
666+
old.stktopsp = g.m.gsignal.stktopsp
667+
}
629668
stsp := uintptr(unsafe.Pointer(st.ss_sp))
630669
g.m.gsignal.stack.lo = stsp
631670
g.m.gsignal.stack.hi = stsp + st.ss_size
@@ -634,6 +673,19 @@ func setGsignalStack(st *stackt) {
634673
g.m.gsignal.stackAlloc = st.ss_size
635674
}
636675

676+
// restoreGsignalStack restores the gsignal stack to the value it had
677+
// before entering the signal handler.
678+
//go:nosplit
679+
//go:nowritebarrierrec
680+
func restoreGsignalStack(st *gsignalStack) {
681+
gp := getg().m.gsignal
682+
gp.stack = st.stack
683+
gp.stackguard0 = st.stackguard0
684+
gp.stackguard1 = st.stackguard1
685+
gp.stackAlloc = st.stackAlloc
686+
gp.stktopsp = st.stktopsp
687+
}
688+
637689
// signalstack sets the current thread's alternate signal stack to s.
638690
//go:nosplit
639691
func signalstack(s *stack) {

0 commit comments

Comments
 (0)