From: Cherry Mui <cherryyz@google.com>
Date: Wed, 25 May 2022 14:15:28 +0000 (-0400)
Subject: runtime: don't inspect the stack for delayed signals from TSAN
X-Git-Tag: go1.19beta1~108
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=d29d0314c1ee64b2cf925b9c8ee535a6e52c5386;p=gostls13.git

runtime: don't inspect the stack for delayed signals from TSAN

Cgo TSAN (not the Go race detector) intercepts signals and calls
the signal handler at a later time. When the signal handler is
called, the memory may have changed, but the signal context
remains old. As the signal context and the memory don't match, it
is unsafe to unwind the stack from the signal PC and SP. We have
to ignore the signal.

It is probably also not safe to do async preemption, which relies
on the signal PC, and inspects and even writes to the stack (for
call injection).

We also inspect the stack for fatal signals (e.g. SIGSEGV), but I
think they are not delayed. For other signals we don't inspect
the stack, so they are probably fine.

Fixes #27540.

Change-Id: I5c80a7512265b8ea4a91422954dbff32c6c3a0d0
Reviewed-on: https://go-review.googlesource.com/c/go/+/408218
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Cherry Mui <cherryyz@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
---

diff --git a/src/runtime/signal_unix.go b/src/runtime/signal_unix.go
index 69a4103948..0be499b2e9 100644
--- a/src/runtime/signal_unix.go
+++ b/src/runtime/signal_unix.go
@@ -608,12 +608,24 @@ var testSigusr1 func(gp *g) bool
 func sighandler(sig uint32, info *siginfo, ctxt unsafe.Pointer, gp *g) {
 	_g_ := getg()
 	c := &sigctxt{info, ctxt}
+	mp := _g_.m
+
+	// Cgo TSAN (not the Go race detector) intercepts signals and calls the
+	// signal handler at a later time. When the signal handler is called, the
+	// memory may have changed, but the signal context remains old. The
+	// unmatched signal context and memory makes it unsafe to unwind or inspect
+	// the stack. So we ignore delayed non-fatal signals that will cause a stack
+	// inspection (profiling signal and preemption signal).
+	// cgo_yield is only non-nil for TSAN, and is specifically used to trigger
+	// signal delivery. We use that as an indicator of delayed signals.
+	// For delayed signals, the handler is called on the g0 stack (see
+	// adjustSignalStack).
+	delayedSignal := *cgo_yield != nil && mp != nil && _g_.stack == mp.g0.stack
 
 	if sig == _SIGPROF {
-		mp := _g_.m
 		// Some platforms (Linux) have per-thread timers, which we use in
 		// combination with the process-wide timer. Avoid double-counting.
-		if validSIGPROF(mp, c) {
+		if !delayedSignal && validSIGPROF(mp, c) {
 			sigprof(c.sigpc(), c.sigsp(), c.siglr(), gp, mp)
 		}
 		return
@@ -636,7 +648,7 @@ func sighandler(sig uint32, info *siginfo, ctxt unsafe.Pointer, gp *g) {
 		return
 	}
 
-	if sig == sigPreempt && debug.asyncpreemptoff == 0 {
+	if sig == sigPreempt && debug.asyncpreemptoff == 0 && !delayedSignal {
 		// Might be a preemption signal.
 		doSigPreempt(gp, c)
 		// Even if this was definitely a preemption signal, it