[release-branch.go1.14] 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.

For #41702
Fixes #41703

Change-Id: I91b0add967b315671ddcf73269c4d30136e579b4
Reviewed-on: https://go-review.googlesource.com/c/go/+/262438
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Cherry Zhang <cherryyz@google.com>
(cherry picked from commit 64fb6ae95f1c322486cbfb758552bb8439a8e6e8)
Reviewed-on: https://go-review.googlesource.com/c/go/+/262737

diff --git a/src/runtime/signal_unix.go b/src/runtime/signal_unix.go
index dbac55c3d1d3398d55f483f96701622a546a9b9d..c7a6c2197cc915ed8c86258723abbe859b78323b 100644 (file)
--- a/src/runtime/signal_unix.go
+++ b/src/runtime/signal_unix.go
@@ -360,6 +360,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
@@ -368,6 +375,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.

diff --git a/src/syscall/exec_unix_test.go b/src/syscall/exec_unix_test.go
index 33614f5221233e4533818d117951355d69e99f11..722534af789b1b19e27bdce68a9d6b2aa1bcff6c 100644 (file)
--- a/src/syscall/exec_unix_test.go
+++ b/src/syscall/exec_unix_test.go
@@ -9,11 +9,14 @@ package syscall_test
 import (
        "internal/testenv"
        "io"
+       "math/rand"
        "os"
        "os/exec"
        "os/signal"
+       "runtime"
        "syscall"
        "testing"
+       "time"
        "unsafe"
 )
 
@@ -213,3 +216,45 @@ func TestForeground(t *testing.T) {
 
        signal.Reset()
 }
+
+// TestExec is for issue #41702.
+func TestExec(t *testing.T) {
+       cmd := exec.Command(os.Args[0], "-test.run=TestExecHelper")
+       cmd.Env = append(os.Environ(), "GO_WANT_HELPER_PROCESS=2")
+       o, err := cmd.CombinedOutput()
+       if err != nil {
+               t.Errorf("%s\n%v", o, err)
+       }
+}
+
+// TestExecHelper is used by TestExec. It does nothing by itself.
+// In testing on macOS 10.14, this used to fail with
+// "signal: illegal instruction" more than half the time.
+func TestExecHelper(t *testing.T) {
+       if os.Getenv("GO_WANT_HELPER_PROCESS") != "2" {
+               return
+       }
+
+       // We don't have to worry about restoring these values.
+       // We are in a child process that only runs this test,
+       // and we are going to call syscall.Exec anyhow.
+       runtime.GOMAXPROCS(50)
+       os.Setenv("GO_WANT_HELPER_PROCESS", "3")
+
+       stop := time.Now().Add(time.Second)
+       for i := 0; i < 100; i++ {
+               go func(i int) {
+                       r := rand.New(rand.NewSource(int64(i)))
+                       for time.Now().Before(stop) {
+                               r.Uint64()
+                       }
+               }(i)
+       }
+
+       time.Sleep(10 * time.Millisecond)
+
+       argv := []string{os.Args[0], "-test.run=TestExecHelper"}
+       syscall.Exec(os.Args[0], argv, os.Environ())
+
+       t.Error("syscall.Exec returned")
+}