From 64fb6ae95f1c322486cbfb758552bb8439a8e6e8 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Wed, 14 Oct 2020 16:03:48 -0700 Subject: runtime: stop preemption during syscall.Exec on Darwin On current macOS versions a program that receives a signal during an execve can fail with a SIGILL signal. This appears to be a macOS kernel bug. It has been reported to Apple. This CL partially works around the problem by using execLock to not send preemption signals during execve. Of course some other stray signal could occur, but at least we can avoid exacerbating the problem. We can't simply disable signals, as that would mean that the exec'ed process would start with all signals blocked, which it likely does not expect. Fixes #41702 Change-Id: I91b0add967b315671ddcf73269c4d30136e579b4 Reviewed-on: https://go-review.googlesource.com/c/go/+/262438 Trust: Ian Lance Taylor Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Cherry Zhang --- src/runtime/signal_unix.go | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'src/runtime/signal_unix.go') diff --git a/src/runtime/signal_unix.go b/src/runtime/signal_unix.go index a3d6f34c88..c228de47b4 100644 --- a/src/runtime/signal_unix.go +++ b/src/runtime/signal_unix.go @@ -356,6 +356,13 @@ func preemptM(mp *m) { // required). return } + + // On Darwin, don't try to preempt threads during exec. + // Issue #41702. + if GOOS == "darwin" { + execLock.rlock() + } + if atomic.Cas(&mp.signalPending, 0, 1) { // If multiple threads are preempting the same M, it may send many // signals to the same M such that it hardly make progress, causing @@ -364,6 +371,10 @@ func preemptM(mp *m) { // Only send a signal if there isn't already one pending. signalM(mp, sigPreempt) } + + if GOOS == "darwin" { + execLock.runlock() + } } // sigFetchG fetches the value of G safely when running in a signal handler. -- cgit v1.2.1 From 05739d6f17c57f09264272621b88725a463234d0 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Thu, 15 Oct 2020 14:39:12 -0700 Subject: runtime: wait for preemption signals before syscall.Exec Fixes #41702 Fixes #42023 Change-Id: If07f40b1d73b8f276ee28ffb8b7214175e56c24d Reviewed-on: https://go-review.googlesource.com/c/go/+/262817 Trust: Ian Lance Taylor Trust: Bryan C. Mills Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Cherry Zhang --- src/runtime/signal_unix.go | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'src/runtime/signal_unix.go') diff --git a/src/runtime/signal_unix.go b/src/runtime/signal_unix.go index c228de47b4..e8b6f95d8f 100644 --- a/src/runtime/signal_unix.go +++ b/src/runtime/signal_unix.go @@ -335,6 +335,10 @@ func doSigPreempt(gp *g, ctxt *sigctxt) { // Acknowledge the preemption. atomic.Xadd(&gp.m.preemptGen, 1) atomic.Store(&gp.m.signalPending, 0) + + if GOOS == "darwin" { + atomic.Xadd(&pendingPreemptSignals, -1) + } } const preemptMSupported = true @@ -364,6 +368,10 @@ func preemptM(mp *m) { } if atomic.Cas(&mp.signalPending, 0, 1) { + if GOOS == "darwin" { + atomic.Xadd(&pendingPreemptSignals, 1) + } + // If multiple threads are preempting the same M, it may send many // signals to the same M such that it hardly make progress, causing // live-lock problem. Apparently this could happen on darwin. See @@ -435,6 +443,9 @@ func sigtrampgo(sig uint32, info *siginfo, ctx unsafe.Pointer) { // no non-Go signal handler for sigPreempt. // The default behavior for sigPreempt is to ignore // the signal, so badsignal will be a no-op anyway. + if GOOS == "darwin" { + atomic.Xadd(&pendingPreemptSignals, -1) + } return } c.fixsigcode(sig) -- cgit v1.2.1 From c91dffbc9aeaacd087eb0c0c3f718739bc5f8c4a Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Wed, 7 Oct 2020 22:53:52 -0400 Subject: runtime: tidy cgocallback On amd64 and 386, we have a very roundabout way of remembering that we need to dropm on return that currently involves saving a zero to needm's argument slot and later bringing it back. Just store the zero. This also makes amd64 and 386 more consistent with cgocallback on all other platforms: rather than saving the old M to the G stack, they now save it to a named slot on the G0 stack. The needm function no longer needs a dummy argument to get the SP, so we drop that. Change-Id: I7e84bb4a5ff9552de70dcf41d8accf02310535e7 Reviewed-on: https://go-review.googlesource.com/c/go/+/263268 Trust: Austin Clements Run-TryBot: Austin Clements TryBot-Result: Go Bot Reviewed-by: Cherry Zhang --- src/runtime/signal_unix.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/runtime/signal_unix.go') diff --git a/src/runtime/signal_unix.go b/src/runtime/signal_unix.go index e8b6f95d8f..9318a9b8bc 100644 --- a/src/runtime/signal_unix.go +++ b/src/runtime/signal_unix.go @@ -504,14 +504,14 @@ func adjustSignalStack(sig uint32, mp *m, gsigStack *gsignalStack) bool { sigaltstack(nil, &st) if st.ss_flags&_SS_DISABLE != 0 { setg(nil) - needm(0) + needm() noSignalStack(sig) dropm() } stsp := uintptr(unsafe.Pointer(st.ss_sp)) if sp < stsp || sp >= stsp+st.ss_size { setg(nil) - needm(0) + needm() sigNotOnStack(sig) dropm() } @@ -951,7 +951,7 @@ func badsignal(sig uintptr, c *sigctxt) { exit(2) *(*uintptr)(unsafe.Pointer(uintptr(123))) = 2 } - needm(0) + needm() if !sigsend(uint32(sig)) { // A foreign thread received the signal sig, and the // Go code does not want to handle it. -- cgit v1.2.1 From 368c40116434532dc0b53b72fa04788ca6742898 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Tue, 27 Oct 2020 16:09:40 -0700 Subject: runtime: block signals in needm before allocating M Otherwise, if a signal occurs just after we allocated the M, we can deadlock if the signal handler needs to allocate an M itself. Fixes #42207 Change-Id: I76f44547f419e8b1c14cbf49bf602c6e645d8c14 Reviewed-on: https://go-review.googlesource.com/c/go/+/265759 Trust: Ian Lance Taylor Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Bryan C. Mills --- src/runtime/signal_unix.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src/runtime/signal_unix.go') diff --git a/src/runtime/signal_unix.go b/src/runtime/signal_unix.go index 9318a9b8bc..bf4a319b37 100644 --- a/src/runtime/signal_unix.go +++ b/src/runtime/signal_unix.go @@ -1031,15 +1031,15 @@ func sigfwdgo(sig uint32, info *siginfo, ctx unsafe.Pointer) bool { return true } -// msigsave saves the current thread's signal mask into mp.sigmask. +// sigsave saves the current thread's signal mask into *p. // This is used to preserve the non-Go signal mask when a non-Go // thread calls a Go function. // This is nosplit and nowritebarrierrec because it is called by needm // which may be called on a non-Go thread with no g available. //go:nosplit //go:nowritebarrierrec -func msigsave(mp *m) { - sigprocmask(_SIG_SETMASK, nil, &mp.sigmask) +func sigsave(p *sigset) { + sigprocmask(_SIG_SETMASK, nil, p) } // msigrestore sets the current thread's signal mask to sigmask. @@ -1111,7 +1111,7 @@ func minitSignalStack() { // thread's signal mask. When this is called all signals have been // blocked for the thread. This starts with m.sigmask, which was set // either from initSigmask for a newly created thread or by calling -// msigsave if this is a non-Go thread calling a Go function. It +// sigsave if this is a non-Go thread calling a Go function. It // removes all essential signals from the mask, thus causing those // signals to not be blocked. Then it sets the thread's signal mask. // After this is called the thread can receive signals. -- cgit v1.2.1