[release-branch.go1.15] 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 #41704

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/+/262717

diff --git a/src/runtime/signal_unix.go b/src/runtime/signal_unix.go
index dd6d79f8ecf5dd2e83b2f8c4eb3d48862ed01c22..f9784b84a759a7d3d36e3c20dd924aa6087cbdf1 100644 (file)
--- a/src/runtime/signal_unix.go
+++ b/src/runtime/signal_unix.go
@@ -357,6 +357,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
@@ -365,6 +372,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 4eb3c5c6c8a691f5dee1dd60bec2dbcd882122bb..d005bba610ea7d213c7963d61dd59ce8cc5f29a3 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"
 )
 
@@ -241,3 +244,45 @@ func TestInvalidExec(t *testing.T) {
                }
        })
 }
+
+// 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")
+}