From 87105e5b2e3462cb5d62d9a8e08b9e45f65ed671 Mon Sep 17 00:00:00 2001 From: Russ Cox Date: Sun, 8 Jan 2023 17:33:12 -0500 Subject: [PATCH] [release-branch.go1.18] runtime: revert "call __fork instead of fork on darwin" A recent comment on #57263 reports an unexplained crash in a cgo program that is fixed by reverting the __fork fix. We don't have any viable fix for the os/exec bug at this point, so give up on a fix for the January point releases. This reverts CL 459179 (commit 07b6ffb79c90). Fixes #57689. Change-Id: I3b81de6bded399f47862325129e86a65c83d8e3b Reviewed-on: https://go-review.googlesource.com/c/go/+/461116 Reviewed-by: Ian Lance Taylor Run-TryBot: Russ Cox TryBot-Result: Gopher Robot Reviewed-by: Cherry Mui --- src/syscall/exec_libc2.go | 4 ++-- src/syscall/syscall_darwin.go | 31 +--------------------------- src/syscall/syscall_openbsd_libc.go | 5 ----- src/syscall/zsyscall_darwin_amd64.go | 22 ++++---------------- src/syscall/zsyscall_darwin_amd64.s | 6 ++---- src/syscall/zsyscall_darwin_arm64.go | 22 ++++---------------- src/syscall/zsyscall_darwin_arm64.s | 6 ++---- 7 files changed, 15 insertions(+), 81 deletions(-) diff --git a/src/syscall/exec_libc2.go b/src/syscall/exec_libc2.go index bd2ce6820d..b05f053bbf 100644 --- a/src/syscall/exec_libc2.go +++ b/src/syscall/exec_libc2.go @@ -76,7 +76,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(forkTrampoline, 0, 0, 0) + r1, _, err1 = rawSyscall(abi.FuncPCABI0(libc_fork_trampoline), 0, 0, 0) if err1 != 0 { runtime_AfterFork() return 0, err1 @@ -260,6 +260,6 @@ childerror: // send error code on pipe rawSyscall(abi.FuncPCABI0(libc_write_trampoline), uintptr(pipe), uintptr(unsafe.Pointer(&err1)), unsafe.Sizeof(err1)) for { - rawSyscall(exitTrampoline, 253, 0, 0) + rawSyscall(abi.FuncPCABI0(libc_exit_trampoline), 253, 0, 0) } } diff --git a/src/syscall/syscall_darwin.go b/src/syscall/syscall_darwin.go index 346e6862cd..902d6e77e1 100644 --- a/src/syscall/syscall_darwin.go +++ b/src/syscall/syscall_darwin.go @@ -17,34 +17,6 @@ import ( "unsafe" ) -// 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 ( - exitTrampoline = abi.FuncPCABI0(libc___exit_trampoline) - forkTrampoline = abi.FuncPCABI0(libc___fork_trampoline) -) - type SockaddrDatalink struct { Len uint8 Family uint8 @@ -230,12 +202,11 @@ 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 ff9fa00259..15b68fd0fc 100644 --- a/src/syscall/syscall_openbsd_libc.go +++ b/src/syscall/syscall_openbsd_libc.go @@ -10,11 +10,6 @@ import ( "internal/abi" ) -var ( - 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 2ad03801b2..0ccdaf2d0e 100644 --- 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,20 +1781,6 @@ 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 1815703803..563083d441 100644 --- a/src/syscall/zsyscall_darwin_amd64.s +++ b/src/syscall/zsyscall_darwin_amd64.s @@ -219,16 +219,14 @@ 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 559bb49c4b..09bf34bb3c 100644 --- 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,20 +1781,6 @@ 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 1666cf985c..0567a42fa3 100644 --- a/src/syscall/zsyscall_darwin_arm64.s +++ b/src/syscall/zsyscall_darwin_arm64.s @@ -219,16 +219,14 @@ 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