syscall: clean up variable declarations in forkAndExecInChild
The various forkAndExecInChild implementations have comments
explaining that they pre-declare variables to force allocations
to occur before forking, but then later use ":=" declarations
for additional variables.
To make it clearer that those ":=" declarations do not allocate,
we move their declarations up to the predeclared blocks.
For #57208.
Change-Id: Ie8cb577fa7180b51b64d6dc398169053fdf8ea97
Reviewed-on: https://go-review.googlesource.com/c/go/+/456516
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
diff --git a/src/syscall/exec_bsd.go b/src/syscall/exec_bsd.go
index 32c3ebd..3798755 100644
--- a/src/syscall/exec_bsd.go
+++ b/src/syscall/exec_bsd.go
@@ -55,10 +55,13 @@
// Declare all variables at top in case any
// declarations require heap allocation (e.g., err1).
var (
- r1 uintptr
- err1 Errno
- nextfd int
- i int
+ r1 uintptr
+ err1 Errno
+ nextfd int
+ i int
+ pgrp _C_int
+ cred *Credential
+ ngroups, groups uintptr
)
// guard against side effects of shuffling fds below.
@@ -119,7 +122,7 @@
if sys.Foreground {
// This should really be pid_t, however _C_int (aka int32) is
// generally equivalent.
- pgrp := _C_int(sys.Pgid)
+ pgrp = _C_int(sys.Pgid)
if pgrp == 0 {
r1, _, err1 = RawSyscall(SYS_GETPID, 0, 0, 0)
if err1 != 0 {
@@ -149,9 +152,9 @@
}
// User and groups
- if cred := sys.Credential; cred != nil {
- ngroups := uintptr(len(cred.Groups))
- groups := uintptr(0)
+ if cred = sys.Credential; cred != nil {
+ ngroups = uintptr(len(cred.Groups))
+ groups = uintptr(0)
if ngroups > 0 {
groups = uintptr(unsafe.Pointer(&cred.Groups[0]))
}
diff --git a/src/syscall/exec_freebsd.go b/src/syscall/exec_freebsd.go
index af5a415..9e1cc46 100644
--- a/src/syscall/exec_freebsd.go
+++ b/src/syscall/exec_freebsd.go
@@ -60,10 +60,14 @@
// Declare all variables at top in case any
// declarations require heap allocation (e.g., err1).
var (
- r1 uintptr
- err1 Errno
- nextfd int
- i int
+ r1 uintptr
+ err1 Errno
+ nextfd int
+ i int
+ pgrp _C_int
+ cred *Credential
+ ngroups, groups uintptr
+ upid uintptr
)
// Record parent PID so child can test if it has died.
@@ -127,7 +131,7 @@
if sys.Foreground {
// This should really be pid_t, however _C_int (aka int32) is
// generally equivalent.
- pgrp := _C_int(sys.Pgid)
+ pgrp = _C_int(sys.Pgid)
if pgrp == 0 {
r1, _, err1 = RawSyscall(SYS_GETPID, 0, 0, 0)
if err1 != 0 {
@@ -157,9 +161,9 @@
}
// User and groups
- if cred := sys.Credential; cred != nil {
- ngroups := uintptr(len(cred.Groups))
- groups := uintptr(0)
+ if cred = sys.Credential; cred != nil {
+ ngroups = uintptr(len(cred.Groups))
+ groups = uintptr(0)
if ngroups > 0 {
groups = uintptr(unsafe.Pointer(&cred.Groups[0]))
}
@@ -204,8 +208,8 @@
// using SIGKILL.
r1, _, _ = RawSyscall(SYS_GETPPID, 0, 0, 0)
if r1 != ppid {
- pid, _, _ := RawSyscall(SYS_GETPID, 0, 0, 0)
- _, _, err1 = RawSyscall(SYS_KILL, pid, uintptr(sys.Pdeathsig), 0)
+ upid, _, _ = RawSyscall(SYS_GETPID, 0, 0, 0)
+ _, _, err1 = RawSyscall(SYS_KILL, upid, uintptr(sys.Pdeathsig), 0)
if err1 != 0 {
goto childerror
}
diff --git a/src/syscall/exec_libc.go b/src/syscall/exec_libc.go
index ef0c87e..f8769b9 100644
--- a/src/syscall/exec_libc.go
+++ b/src/syscall/exec_libc.go
@@ -81,10 +81,13 @@
// Declare all variables at top in case any
// declarations require heap allocation (e.g., err1).
var (
- r1 uintptr
- err1 Errno
- nextfd int
- i int
+ r1 uintptr
+ err1 Errno
+ nextfd int
+ i int
+ pgrp _Pid_t
+ cred *Credential
+ ngroups, groups uintptr
)
// guard against side effects of shuffling fds below.
@@ -135,7 +138,7 @@
}
if sys.Foreground {
- pgrp := _Pid_t(sys.Pgid)
+ pgrp = _Pid_t(sys.Pgid)
if pgrp == 0 {
r1, err1 = getpid()
if err1 != 0 {
@@ -165,9 +168,9 @@
}
// User and groups
- if cred := sys.Credential; cred != nil {
- ngroups := uintptr(len(cred.Groups))
- groups := uintptr(0)
+ if cred = sys.Credential; cred != nil {
+ ngroups = uintptr(len(cred.Groups))
+ groups = uintptr(0)
if ngroups > 0 {
groups = uintptr(unsafe.Pointer(&cred.Groups[0]))
}
diff --git a/src/syscall/exec_libc2.go b/src/syscall/exec_libc2.go
index 41bc79a..9b04f96 100644
--- a/src/syscall/exec_libc2.go
+++ b/src/syscall/exec_libc2.go
@@ -52,14 +52,17 @@
// functions that do not grow the stack.
//
//go:norace
-func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr *ProcAttr, sys *SysProcAttr, pipe int) (pid int, err Errno) {
+func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr *ProcAttr, sys *SysProcAttr, pipe int) (pid int, err1 Errno) {
// Declare all variables at top in case any
// declarations require heap allocation (e.g., err1).
var (
- r1 uintptr
- err1 Errno
- nextfd int
- i int
+ r1 uintptr
+ nextfd int
+ i int
+ err error
+ pgrp _C_int
+ cred *Credential
+ ngroups, groups uintptr
)
// guard against side effects of shuffling fds below.
@@ -94,7 +97,7 @@
// Enable tracing if requested.
if sys.Ptrace {
- if err := ptrace(PTRACE_TRACEME, 0, 0, 0); err != nil {
+ if err = ptrace(PTRACE_TRACEME, 0, 0, 0); err != nil {
err1 = err.(Errno)
goto childerror
}
@@ -120,7 +123,7 @@
if sys.Foreground {
// This should really be pid_t, however _C_int (aka int32) is
// generally equivalent.
- pgrp := _C_int(sys.Pgid)
+ pgrp = _C_int(sys.Pgid)
if pgrp == 0 {
r1, _, err1 = rawSyscall(abi.FuncPCABI0(libc_getpid_trampoline), 0, 0, 0)
if err1 != 0 {
@@ -149,9 +152,9 @@
}
// User and groups
- if cred := sys.Credential; cred != nil {
- ngroups := uintptr(len(cred.Groups))
- groups := uintptr(0)
+ if cred = sys.Credential; cred != nil {
+ ngroups = uintptr(len(cred.Groups))
+ groups = uintptr(0)
if ngroups > 0 {
groups = uintptr(unsafe.Pointer(&cred.Groups[0]))
}
diff --git a/src/syscall/exec_linux.go b/src/syscall/exec_linux.go
index 7e0c3d2..a8eb4bf 100644
--- a/src/syscall/exec_linux.go
+++ b/src/syscall/exec_linux.go
@@ -127,19 +127,19 @@
func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr *ProcAttr, sys *SysProcAttr, pipe int) (pid int, err Errno) {
// Set up and fork. This returns immediately in the parent or
// if there's an error.
- r1, err1, p, locked := forkAndExecInChild1(argv0, argv, envv, chroot, dir, attr, sys, pipe)
+ upid, err, mapPipe, locked := forkAndExecInChild1(argv0, argv, envv, chroot, dir, attr, sys, pipe)
if locked {
runtime_AfterFork()
}
- if err1 != 0 {
- return 0, err1
+ if err != 0 {
+ return 0, err
}
// parent; return PID
- pid = int(r1)
+ pid = int(upid)
if sys.UidMappings != nil || sys.GidMappings != nil {
- Close(p[0])
+ Close(mapPipe[0])
var err2 Errno
// uid/gid mappings will be written after fork and unshare(2) for user
// namespaces.
@@ -148,8 +148,8 @@
err2 = err.(Errno)
}
}
- RawSyscall(SYS_WRITE, uintptr(p[1]), uintptr(unsafe.Pointer(&err2)), unsafe.Sizeof(err2))
- Close(p[1])
+ RawSyscall(SYS_WRITE, uintptr(mapPipe[1]), uintptr(unsafe.Pointer(&err2)), unsafe.Sizeof(err2))
+ Close(mapPipe[1])
}
return pid, 0
@@ -203,7 +203,7 @@
//
//go:noinline
//go:norace
-func forkAndExecInChild1(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr *ProcAttr, sys *SysProcAttr, pipe int) (r1 uintptr, err1 Errno, p [2]int, locked bool) {
+func forkAndExecInChild1(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr *ProcAttr, sys *SysProcAttr, pipe int) (pid uintptr, err1 Errno, mapPipe [2]int, locked bool) {
// Defined in linux/prctl.h starting with Linux 4.3.
const (
PR_CAP_AMBIENT = 0x2f
@@ -215,8 +215,15 @@
// processing in this stack frame and never returns, while the
// parent returns immediately from this frame and does all
// post-fork processing in the outer frame.
+ //
// Declare all variables at top in case any
- // declarations require heap allocation (e.g., err1).
+ // declarations require heap allocation (e.g., err2).
+ // ":=" should not be used to declare any variable after
+ // the call to runtime_BeforeFork.
+ //
+ // NOTE(bcmills): The allocation behavior described in the above comment
+ // seems to lack a corresponding test, and it may be rendered invalid
+ // by an otherwise-correct change in the compiler.
var (
err2 Errno
nextfd int
@@ -226,6 +233,11 @@
puid, psetgroups, pgid []byte
uidmap, setgroups, gidmap []byte
clone3 *cloneArgs
+ pgrp int32
+ dirfd int
+ cred *Credential
+ ngroups, groups uintptr
+ c uintptr
)
if sys.UidMappings != nil {
@@ -264,7 +276,7 @@
// Allocate another pipe for parent to child communication for
// synchronizing writing of User ID/Group ID mappings.
if sys.UidMappings != nil || sys.GidMappings != nil {
- if err := forkExecPipe(p[:]); err != nil {
+ if err := forkExecPipe(mapPipe[:]); err != nil {
err1 = err.(Errno)
return
}
@@ -288,17 +300,17 @@
runtime_BeforeFork()
locked = true
if clone3 != nil {
- r1, err1 = rawVforkSyscall(_SYS_clone3, uintptr(unsafe.Pointer(clone3)), unsafe.Sizeof(*clone3))
+ pid, err1 = rawVforkSyscall(_SYS_clone3, uintptr(unsafe.Pointer(clone3)), unsafe.Sizeof(*clone3))
} else {
flags |= uintptr(SIGCHLD)
if runtime.GOARCH == "s390x" {
// On Linux/s390, the first two arguments of clone(2) are swapped.
- r1, err1 = rawVforkSyscall(SYS_CLONE, 0, flags)
+ pid, err1 = rawVforkSyscall(SYS_CLONE, 0, flags)
} else {
- r1, err1 = rawVforkSyscall(SYS_CLONE, flags, 0)
+ pid, err1 = rawVforkSyscall(SYS_CLONE, flags, 0)
}
}
- if err1 != 0 || r1 != 0 {
+ if err1 != 0 || pid != 0 {
// If we're in the parent, we must return immediately
// so we're not in the same stack frame as the child.
// This can at most use the return PC, which the child
@@ -320,14 +332,14 @@
// Wait for User ID/Group ID mappings to be written.
if sys.UidMappings != nil || sys.GidMappings != nil {
- if _, _, err1 = RawSyscall(SYS_CLOSE, uintptr(p[1]), 0, 0); err1 != 0 {
+ if _, _, err1 = RawSyscall(SYS_CLOSE, uintptr(mapPipe[1]), 0, 0); err1 != 0 {
goto childerror
}
- r1, _, err1 = RawSyscall(SYS_READ, uintptr(p[0]), uintptr(unsafe.Pointer(&err2)), unsafe.Sizeof(err2))
+ pid, _, err1 = RawSyscall(SYS_READ, uintptr(mapPipe[0]), uintptr(unsafe.Pointer(&err2)), unsafe.Sizeof(err2))
if err1 != 0 {
goto childerror
}
- if r1 != unsafe.Sizeof(err2) {
+ if pid != unsafe.Sizeof(err2) {
err1 = EINVAL
goto childerror
}
@@ -355,11 +367,11 @@
}
if sys.Foreground {
- pgrp := int32(sys.Pgid)
+ pgrp = int32(sys.Pgid)
if pgrp == 0 {
- r1, _ = rawSyscallNoError(SYS_GETPID, 0, 0, 0)
+ pid, _ = rawSyscallNoError(SYS_GETPID, 0, 0, 0)
- pgrp = int32(r1)
+ pgrp = int32(pid)
}
// Place process group in foreground.
@@ -381,11 +393,11 @@
}
if sys.Unshareflags&CLONE_NEWUSER != 0 && sys.GidMappings != nil {
- dirfd := int(_AT_FDCWD)
+ dirfd = int(_AT_FDCWD)
if fd1, _, err1 = RawSyscall6(SYS_OPENAT, uintptr(dirfd), uintptr(unsafe.Pointer(&psetgroups[0])), uintptr(O_WRONLY), 0, 0, 0); err1 != 0 {
goto childerror
}
- r1, _, err1 = RawSyscall(SYS_WRITE, uintptr(fd1), uintptr(unsafe.Pointer(&setgroups[0])), uintptr(len(setgroups)))
+ pid, _, err1 = RawSyscall(SYS_WRITE, uintptr(fd1), uintptr(unsafe.Pointer(&setgroups[0])), uintptr(len(setgroups)))
if err1 != 0 {
goto childerror
}
@@ -396,7 +408,7 @@
if fd1, _, err1 = RawSyscall6(SYS_OPENAT, uintptr(dirfd), uintptr(unsafe.Pointer(&pgid[0])), uintptr(O_WRONLY), 0, 0, 0); err1 != 0 {
goto childerror
}
- r1, _, err1 = RawSyscall(SYS_WRITE, uintptr(fd1), uintptr(unsafe.Pointer(&gidmap[0])), uintptr(len(gidmap)))
+ pid, _, err1 = RawSyscall(SYS_WRITE, uintptr(fd1), uintptr(unsafe.Pointer(&gidmap[0])), uintptr(len(gidmap)))
if err1 != 0 {
goto childerror
}
@@ -406,11 +418,11 @@
}
if sys.Unshareflags&CLONE_NEWUSER != 0 && sys.UidMappings != nil {
- dirfd := int(_AT_FDCWD)
+ dirfd = int(_AT_FDCWD)
if fd1, _, err1 = RawSyscall6(SYS_OPENAT, uintptr(dirfd), uintptr(unsafe.Pointer(&puid[0])), uintptr(O_WRONLY), 0, 0, 0); err1 != 0 {
goto childerror
}
- r1, _, err1 = RawSyscall(SYS_WRITE, uintptr(fd1), uintptr(unsafe.Pointer(&uidmap[0])), uintptr(len(uidmap)))
+ pid, _, err1 = RawSyscall(SYS_WRITE, uintptr(fd1), uintptr(unsafe.Pointer(&uidmap[0])), uintptr(len(uidmap)))
if err1 != 0 {
goto childerror
}
@@ -443,9 +455,9 @@
}
// User and groups
- if cred := sys.Credential; cred != nil {
- ngroups := uintptr(len(cred.Groups))
- groups := uintptr(0)
+ if cred = sys.Credential; cred != nil {
+ ngroups = uintptr(len(cred.Groups))
+ groups = uintptr(0)
if ngroups > 0 {
groups = uintptr(unsafe.Pointer(&cred.Groups[0]))
}
@@ -474,7 +486,7 @@
goto childerror
}
- for _, c := range sys.AmbientCaps {
+ for _, c = range sys.AmbientCaps {
// Add the c capability to the permitted and inheritable capability mask,
// otherwise we will not be able to add it to the ambient capability mask.
caps.data[capToIndex(c)].permitted |= capToMask(c)
@@ -485,7 +497,7 @@
goto childerror
}
- for _, c := range sys.AmbientCaps {
+ for _, c = range sys.AmbientCaps {
_, _, err1 = RawSyscall6(SYS_PRCTL, PR_CAP_AMBIENT, uintptr(PR_CAP_AMBIENT_RAISE), c, 0, 0, 0)
if err1 != 0 {
goto childerror
@@ -511,9 +523,9 @@
// Signal self if parent is already dead. This might cause a
// duplicate signal in rare cases, but it won't matter when
// using SIGKILL.
- r1, _ = rawSyscallNoError(SYS_GETPPID, 0, 0, 0)
- if r1 != ppid {
- pid, _ := rawSyscallNoError(SYS_GETPID, 0, 0, 0)
+ pid, _ = rawSyscallNoError(SYS_GETPPID, 0, 0, 0)
+ if pid != ppid {
+ pid, _ = rawSyscallNoError(SYS_GETPID, 0, 0, 0)
_, _, err1 = RawSyscall(SYS_KILL, pid, uintptr(sys.Pdeathsig), 0)
if err1 != 0 {
goto childerror
diff --git a/src/syscall/exec_plan9.go b/src/syscall/exec_plan9.go
index 8f28b5a..8762237 100644
--- a/src/syscall/exec_plan9.go
+++ b/src/syscall/exec_plan9.go
@@ -135,6 +135,8 @@
errbuf [ERRMAX]byte
statbuf [STATMAX]byte
dupdevfd int
+ n int
+ b []byte
)
// Guard against side effects of shuffling fds below.
@@ -177,14 +179,14 @@
dirloop:
for {
r1, _, _ = RawSyscall6(SYS_PREAD, uintptr(dupdevfd), uintptr(unsafe.Pointer(&statbuf[0])), uintptr(len(statbuf)), ^uintptr(0), ^uintptr(0), 0)
- n := int(r1)
+ n = int(r1)
switch n {
case -1:
goto childerror
case 0:
break dirloop
}
- for b := statbuf[:n]; len(b) > 0; {
+ for b = statbuf[:n]; len(b) > 0; {
var s []byte
s, b = gdirname(b)
if s == nil {
