Skip to content

Commit

Permalink
syscall: clean up variable declarations in forkAndExecInChild
Browse files Browse the repository at this point in the history 
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>
  • Loading branch information
@gopherbot
Bryan C. Mills authored and gopherbot committed Jan 25, 2023
1 parent 01636cf commit e216ee7
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 71 deletions.
19 changes: 11 additions & 8 deletions src/syscall/exec_bsd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,10 +55,13 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
// 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.
Expand Down Expand Up @@ -119,7 +122,7 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
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 {
Expand Down Expand Up @@ -149,9 +152,9 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
}

// 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]))
}
Expand Down
24 changes: 14 additions & 10 deletions src/syscall/exec_freebsd.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,14 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
// 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.
Expand Down Expand Up @@ -127,7 +131,7 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
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 {
Expand Down Expand Up @@ -157,9 +161,9 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
}

// 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]))
}
Expand Down Expand Up @@ -204,8 +208,8 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
// 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
}
Expand Down
19 changes: 11 additions & 8 deletions src/syscall/exec_libc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,13 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
// 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.
Expand Down Expand Up @@ -135,7 +138,7 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
}

if sys.Foreground {
pgrp := _Pid_t(sys.Pgid)
pgrp = _Pid_t(sys.Pgid)
if pgrp == 0 {
r1, err1 = getpid()
if err1 != 0 {
Expand Down Expand Up @@ -165,9 +168,9 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
}

// 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]))
}
Expand Down
23 changes: 13 additions & 10 deletions src/syscall/exec_libc2.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,17 @@ func runtime_AfterForkInChild()
// 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.
Expand Down Expand Up @@ -94,7 +97,7 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr

// 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
}
Expand All @@ -120,7 +123,7 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
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 {
Expand Down Expand Up @@ -149,9 +152,9 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
}

// 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]))
}
Expand Down
Loading

0 comments on commit e216ee7

Please sign in to comment.