[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

2 files changed, 56 insertions, 0 deletions

diff --git a/src/runtime/signal_unix.go b/src/runtime/signal_unix.go
index dbac55c3d1..c7a6c2197c 100644
--- 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 33614f5221..722534af78 100644
--- 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")
+}