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/syscall/exec_unix_test.go | 45 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) (limited to 'src/syscall/exec_unix_test.go') diff --git a/src/syscall/exec_unix_test.go b/src/syscall/exec_unix_test.go index 4eb3c5c6c8..d005bba610 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" ) @@ -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") +} -- cgit v1.3 From 11cfb48df192c14d185c1cfcaad1ba3e7b84c807 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Thu, 15 Oct 2020 14:19:10 -0700 Subject: syscall: use MustHaveExec in TestExec For #41702 Change-Id: Ib2b15e52aa1fef2f5e644b316c726150252fa9f8 Reviewed-on: https://go-review.googlesource.com/c/go/+/262738 Trust: Ian Lance Taylor Run-TryBot: Ian Lance Taylor TryBot-Result: Go Bot Reviewed-by: Cherry Zhang --- src/syscall/exec_unix_test.go | 1 + 1 file changed, 1 insertion(+) (limited to 'src/syscall/exec_unix_test.go') diff --git a/src/syscall/exec_unix_test.go b/src/syscall/exec_unix_test.go index d005bba610..4431f7fc90 100644 --- a/src/syscall/exec_unix_test.go +++ b/src/syscall/exec_unix_test.go @@ -247,6 +247,7 @@ func TestInvalidExec(t *testing.T) { // TestExec is for issue #41702. func TestExec(t *testing.T) { + testenv.MustHaveExec(t) cmd := exec.Command(os.Args[0], "-test.run=TestExecHelper") cmd.Env = append(os.Environ(), "GO_WANT_HELPER_PROCESS=2") o, err := cmd.CombinedOutput() -- cgit v1.3 From 31f71506d7026595be76713af25197a8c0022ac8 Mon Sep 17 00:00:00 2001 From: Joel Sing Date: Tue, 3 Nov 2020 02:11:51 +1100 Subject: syscall: use correct type for TIOCSPGRP/TIOCGPGRP These ioctls take a pid_t (generally a C integer aka int32) and not an int64 - we currently get away with this on little endian 64 bit platforms, since the bytes fall into the correct place, however this breaks on big endian 64 bit platforms (like openbsd/mips64). Update #40995 Change-Id: I622a0543fd562d97f76a7376a84fd2641e6d6a24 Reviewed-on: https://go-review.googlesource.com/c/go/+/267605 Trust: Joel Sing Run-TryBot: Joel Sing TryBot-Result: Go Bot Reviewed-by: Ian Lance Taylor --- src/syscall/exec_bsd.go | 6 ++++-- src/syscall/exec_unix_test.go | 4 +++- 2 files changed, 7 insertions(+), 3 deletions(-) (limited to 'src/syscall/exec_unix_test.go') diff --git a/src/syscall/exec_bsd.go b/src/syscall/exec_bsd.go index af6c836961..b297db96cc 100644 --- a/src/syscall/exec_bsd.go +++ b/src/syscall/exec_bsd.go @@ -117,14 +117,16 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr } if sys.Foreground { - pgrp := sys.Pgid + // This should really be pid_t, however _C_int (aka int32) is + // generally equivalent. + pgrp := _C_int(sys.Pgid) if pgrp == 0 { r1, _, err1 = RawSyscall(SYS_GETPID, 0, 0, 0) if err1 != 0 { goto childerror } - pgrp = int(r1) + pgrp = _C_int(r1) } // Place process group in foreground. diff --git a/src/syscall/exec_unix_test.go b/src/syscall/exec_unix_test.go index 4431f7fc90..d6b6f51fa6 100644 --- a/src/syscall/exec_unix_test.go +++ b/src/syscall/exec_unix_test.go @@ -172,7 +172,9 @@ func TestForeground(t *testing.T) { t.Skipf("Can't test Foreground. Couldn't open /dev/tty: %s", err) } - fpgrp := 0 + // This should really be pid_t, however _C_int (aka int32) is generally + // equivalent. + fpgrp := int32(0) errno := syscall.Ioctl(tty.Fd(), syscall.TIOCGPGRP, uintptr(unsafe.Pointer(&fpgrp))) if errno != 0 { -- cgit v1.3