[release-branch.go1.19] runtime: call __fork instead of fork on darwin

Issues #33565 and #56784 were caused by hangs in the child process
after fork, while it ran atfork handlers that ran into slow paths that
didn't work in the child.

CL 451735 worked around those two issues by calling a couple functions
at startup to try to warm up those child paths. That mostly worked,
but it broke programs using cgo with certain macOS frameworks (#57263).

CL 459175 reverted CL 451735.

This CL introduces a different fix: bypass the atfork child handlers
entirely. For a general fork call where the child and parent are both
meant to keep executing the original program, atfork handlers can be
necessary to fix any state that would otherwise be tied to the parent
process. But Go only uses fork as preparation for exec, and it takes
care to limit what it attempts to do in the child between the fork and
exec. In particular it doesn't use any of the things that the macOS
atfork handlers are trying to fix up (malloc, xpc, others). So we can
use the low-level fork system call (__fork) instead of the
atfork-wrapped one.

The full list of functions that can be called in a child after fork in
exec_libc2.go is:

 - ptrace
 - setsid
 - setpgid
 - getpid
 - ioctl
 - chroot
 - setgroups
 - setgid
 - setuid
 - chdir
 - dup2
 - fcntl
 - close
 - execve
 - write
 - exit

I disassembled all of these while attached to a hung exec.test binary
and confirmed that nearly all of them are making direct kernel calls,
not using anything that the atfork handler needs to fix up.
The exceptions are ioctl, fcntl, and exit.

The ioctl and fcntl implementations do some extra work around the
kernel call but don't call any other functions, so they should still
be OK. (If not, we could use __ioctl and __fcntl instead, but without
a good reason, we should keep using the standard entry points.)

The exit implementation calls atexit handlers. That is almost
certainly inappropriate in a failed fork child, so this CL changes
that call to __exit on darwin. To avoid making unnecessary changes at
this point in the release cycle, this CL leaves OpenBSD calling plain
exit, even though that is probably a bug in the OpenBSD port
(filed #57446).

Fixes #33565.
Fixes #56784.
Fixes #57263.

Fixes #56837.

Change-Id: I26812c26a72bdd7fcf72ec41899ba11cf6b9c4ab
Reviewed-on: https://go-review.googlesource.com/c/go/+/459176
Reviewed-by: David Chase <drchase@google.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-on: https://go-review.googlesource.com/c/go/+/459178

diff --git a/src/syscall/exec_libc2.go b/src/syscall/exec_libc2.go
index 9eb61a5d35168dac5db6e2cba83a4713a6439a1f..f4be784a82c7e08a524630d8a277e9c11adcefde 100644 (file)
--- a/src/syscall/exec_libc2.go
+++ b/src/syscall/exec_libc2.go
@@ -78,7 +78,7 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
        // About to call fork.
        // No more allocation or calls of non-assembly functions.
        runtime_BeforeFork()
-       r1, _, err1 = rawSyscall(abi.FuncPCABI0(libc_fork_trampoline), 0, 0, 0)
+       r1, _, err1 = rawSyscall(forkTrampoline, 0, 0, 0)
        if err1 != 0 {
                runtime_AfterFork()
                return 0, err1
@@ -276,6 +276,6 @@ childerror:
        // send error code on pipe
        rawSyscall(abi.FuncPCABI0(libc_write_trampoline), uintptr(pipe), uintptr(unsafe.Pointer(&err1)), unsafe.Sizeof(err1))
        for {
-               rawSyscall(abi.FuncPCABI0(libc_exit_trampoline), 253, 0, 0)
+               rawSyscall(exitTrampoline, 253, 0, 0)
        }
 }

diff --git a/src/syscall/syscall_darwin.go b/src/syscall/syscall_darwin.go
index 663bd98c104f517dba0a351b268dcdcac4907299..b0d89d036bacc039ab5ee07e97464edf43d8f86e 100644 (file)
--- a/src/syscall/syscall_darwin.go
+++ b/src/syscall/syscall_darwin.go
@@ -22,7 +22,34 @@ func Syscall6(trap, a1, a2, a3, a4, a5, a6 uintptr) (r1, r2 uintptr, err Errno)
 func RawSyscall(trap, a1, a2, a3 uintptr) (r1, r2 uintptr, err Errno)
 func RawSyscall6(trap, a1, a2, a3, a4, a5, a6 uintptr) (r1, r2 uintptr, err Errno)
 
-var dupTrampoline = abi.FuncPCABI0(libc_dup2_trampoline)
+// These are called from exec_libc2.go in the child of fork.
+// The names differ between macOS and OpenBSD, so we need
+// to declare the specific ones used here to keep the exec_libc2.go
+// code portable.
+//
+// We use __fork and __exit, not fork and exit, to avoid the libc atfork
+// and atexit handlers. The atfork handlers have caused fork child
+// hangs in the past (see #33565, #56784). The atexit handlers have
+// not, but the non-libc ports all invoke the system call, so doing
+// the same here makes sense. In general we wouldn't expect
+// atexit handlers to work terribly well in a fork child anyway.
+// (Also, perhaps the atfork handlers clear the atexit handlers,
+// in which case we definitely need to avoid calling the libc exit
+// if we bypass the libc fork.)
+//
+// Other calls that are made in the child after the fork are
+// ptrace, setsid, setpgid, getpid, ioctl, chroot, setgroups,
+// setgid, setuid, chdir, dup2, fcntl, close, execve, and write.
+// Those are all simple kernel wrappers that should be safe
+// to be called directly. The fcntl and ioctl functions do run
+// some code around the kernel call, but they don't call any
+// other functions, so for now we keep using them instead of
+// calling the lower-level __fcntl and __ioctl functions.
+var (
+       dupTrampoline  = abi.FuncPCABI0(libc_dup2_trampoline)
+       exitTrampoline = abi.FuncPCABI0(libc___exit_trampoline)
+       forkTrampoline = abi.FuncPCABI0(libc___fork_trampoline)
+)
 
 type SockaddrDatalink struct {
        Len    uint8
@@ -209,11 +236,12 @@ func Kill(pid int, signum Signal) (err error) { return kill(pid, int(signum), 1)
 //sys  writev(fd int, iovecs []Iovec) (cnt uintptr, err error)
 //sys   mmap(addr uintptr, length uintptr, prot int, flag int, fd int, pos int64) (ret uintptr, err error)
 //sys   munmap(addr uintptr, length uintptr) (err error)
-//sysnb fork() (pid int, err error)
+//sysnb __fork() (pid int, err error)
 //sysnb ioctl(fd int, req int, arg int) (err error)
 //sysnb ioctlPtr(fd int, req uint, arg unsafe.Pointer) (err error) = SYS_ioctl
 //sysnb execve(path *byte, argv **byte, envp **byte) (err error)
 //sysnb exit(res int) (err error)
+//sysnb __exit(res int) (err error)
 //sys  sysctl(mib []_C_int, old *byte, oldlen *uintptr, new *byte, newlen uintptr) (err error)
 //sys  fcntlPtr(fd int, cmd int, arg unsafe.Pointer) (val int, err error) = SYS_fcntl
 //sys   unlinkat(fd int, path string, flags int) (err error)

diff --git a/src/syscall/syscall_openbsd_libc.go b/src/syscall/syscall_openbsd_libc.go
index 516d02975c530a3ea1aea3b826039f7891797c80..6358a9a390a0c3298946e6e6bc139adc5a21317c 100644 (file)
--- a/src/syscall/syscall_openbsd_libc.go
+++ b/src/syscall/syscall_openbsd_libc.go
@@ -10,7 +10,11 @@ import (
        "internal/abi"
 )
 
-var dupTrampoline = abi.FuncPCABI0(libc_dup3_trampoline)
+var (
+       dupTrampoline  = abi.FuncPCABI0(libc_dup3_trampoline)
+       exitTrampoline = abi.FuncPCABI0(libc_exit_trampoline)
+       forkTrampoline = abi.FuncPCABI0(libc_fork_trampoline)
+)
 
 func init() {
        execveOpenBSD = execve

diff --git a/src/syscall/zsyscall_darwin_amd64.go b/src/syscall/zsyscall_darwin_amd64.go
index ee78a572fcbebbdff0030238130251b5df1378e4..fcb038e97abef60ca597721289e7ceb0bf6197e4 100644 (file)
--- a/src/syscall/zsyscall_darwin_amd64.go
+++ b/src/syscall/zsyscall_darwin_amd64.go
@@ -1714,8 +1714,8 @@ func libc_munmap_trampoline()
 
 // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT
 
-func fork() (pid int, err error) {
-       r0, _, e1 := rawSyscall(abi.FuncPCABI0(libc_fork_trampoline), 0, 0, 0)
+func __fork() (pid int, err error) {
+       r0, _, e1 := rawSyscall(abi.FuncPCABI0(libc___fork_trampoline), 0, 0, 0)
        pid = int(r0)
        if e1 != 0 {
                err = errnoErr(e1)
@@ -1723,9 +1723,9 @@ func fork() (pid int, err error) {
        return
 }
 
-func libc_fork_trampoline()
+func libc___fork_trampoline()
 
-//go:cgo_import_dynamic libc_fork fork "/usr/lib/libSystem.B.dylib"
+//go:cgo_import_dynamic libc___fork __fork "/usr/lib/libSystem.B.dylib"
 
 // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT
 
@@ -1781,6 +1781,20 @@ func libc_exit_trampoline()
 
 // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT
 
+func __exit(res int) (err error) {
+       _, _, e1 := rawSyscall(abi.FuncPCABI0(libc___exit_trampoline), uintptr(res), 0, 0)
+       if e1 != 0 {
+               err = errnoErr(e1)
+       }
+       return
+}
+
+func libc___exit_trampoline()
+
+//go:cgo_import_dynamic libc___exit __exit "/usr/lib/libSystem.B.dylib"
+
+// THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT
+
 func sysctl(mib []_C_int, old *byte, oldlen *uintptr, new *byte, newlen uintptr) (err error) {
        var _p0 unsafe.Pointer
        if len(mib) > 0 {

diff --git a/src/syscall/zsyscall_darwin_amd64.s b/src/syscall/zsyscall_darwin_amd64.s
index 563083d441a17c7600c4265e4104adc9498bc60b..1815703803d1c71378ddca834e0b5c3300d3116c 100644 (file)
--- a/src/syscall/zsyscall_darwin_amd64.s
+++ b/src/syscall/zsyscall_darwin_amd64.s
@@ -219,14 +219,16 @@ TEXT ·libc_mmap_trampoline(SB),NOSPLIT,$0-0
        JMP     libc_mmap(SB)
 TEXT ·libc_munmap_trampoline(SB),NOSPLIT,$0-0
        JMP     libc_munmap(SB)
-TEXT ·libc_fork_trampoline(SB),NOSPLIT,$0-0
-       JMP     libc_fork(SB)
+TEXT ·libc___fork_trampoline(SB),NOSPLIT,$0-0
+       JMP     libc___fork(SB)
 TEXT ·libc_ioctl_trampoline(SB),NOSPLIT,$0-0
        JMP     libc_ioctl(SB)
 TEXT ·libc_execve_trampoline(SB),NOSPLIT,$0-0
        JMP     libc_execve(SB)
 TEXT ·libc_exit_trampoline(SB),NOSPLIT,$0-0
        JMP     libc_exit(SB)
+TEXT ·libc___exit_trampoline(SB),NOSPLIT,$0-0
+       JMP     libc___exit(SB)
 TEXT ·libc_sysctl_trampoline(SB),NOSPLIT,$0-0
        JMP     libc_sysctl(SB)
 TEXT ·libc_unlinkat_trampoline(SB),NOSPLIT,$0-0

diff --git a/src/syscall/zsyscall_darwin_arm64.go b/src/syscall/zsyscall_darwin_arm64.go
index ac1eccf755978cd6446b0bc754dabd723528760b..a4bdcee637a271f8188b68717877689a64286cc7 100644 (file)
--- a/src/syscall/zsyscall_darwin_arm64.go
+++ b/src/syscall/zsyscall_darwin_arm64.go
@@ -1714,8 +1714,8 @@ func libc_munmap_trampoline()
 
 // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT
 
-func fork() (pid int, err error) {
-       r0, _, e1 := rawSyscall(abi.FuncPCABI0(libc_fork_trampoline), 0, 0, 0)
+func __fork() (pid int, err error) {
+       r0, _, e1 := rawSyscall(abi.FuncPCABI0(libc___fork_trampoline), 0, 0, 0)
        pid = int(r0)
        if e1 != 0 {
                err = errnoErr(e1)
@@ -1723,9 +1723,9 @@ func fork() (pid int, err error) {
        return
 }
 
-func libc_fork_trampoline()
+func libc___fork_trampoline()
 
-//go:cgo_import_dynamic libc_fork fork "/usr/lib/libSystem.B.dylib"
+//go:cgo_import_dynamic libc___fork __fork "/usr/lib/libSystem.B.dylib"
 
 // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT
 
@@ -1781,6 +1781,20 @@ func libc_exit_trampoline()
 
 // THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT
 
+func __exit(res int) (err error) {
+       _, _, e1 := rawSyscall(abi.FuncPCABI0(libc___exit_trampoline), uintptr(res), 0, 0)
+       if e1 != 0 {
+               err = errnoErr(e1)
+       }
+       return
+}
+
+func libc___exit_trampoline()
+
+//go:cgo_import_dynamic libc___exit __exit "/usr/lib/libSystem.B.dylib"
+
+// THIS FILE IS GENERATED BY THE COMMAND AT THE TOP; DO NOT EDIT
+
 func sysctl(mib []_C_int, old *byte, oldlen *uintptr, new *byte, newlen uintptr) (err error) {
        var _p0 unsafe.Pointer
        if len(mib) > 0 {

diff --git a/src/syscall/zsyscall_darwin_arm64.s b/src/syscall/zsyscall_darwin_arm64.s
index 0567a42fa337985c3b5d0115cf27299c9ebe04d4..1666cf985c35b0a185ff49e8e9c1ad6f04d8f5d2 100644 (file)
--- a/src/syscall/zsyscall_darwin_arm64.s
+++ b/src/syscall/zsyscall_darwin_arm64.s
@@ -219,14 +219,16 @@ TEXT ·libc_mmap_trampoline(SB),NOSPLIT,$0-0
        JMP     libc_mmap(SB)
 TEXT ·libc_munmap_trampoline(SB),NOSPLIT,$0-0
        JMP     libc_munmap(SB)
-TEXT ·libc_fork_trampoline(SB),NOSPLIT,$0-0
-       JMP     libc_fork(SB)
+TEXT ·libc___fork_trampoline(SB),NOSPLIT,$0-0
+       JMP     libc___fork(SB)
 TEXT ·libc_ioctl_trampoline(SB),NOSPLIT,$0-0
        JMP     libc_ioctl(SB)
 TEXT ·libc_execve_trampoline(SB),NOSPLIT,$0-0
        JMP     libc_execve(SB)
 TEXT ·libc_exit_trampoline(SB),NOSPLIT,$0-0
        JMP     libc_exit(SB)
+TEXT ·libc___exit_trampoline(SB),NOSPLIT,$0-0
+       JMP     libc___exit(SB)
 TEXT ·libc_sysctl_trampoline(SB),NOSPLIT,$0-0
        JMP     libc_sysctl(SB)
 TEXT ·libc_unlinkat_trampoline(SB),NOSPLIT,$0-0