os/signal: TestTerminalSignal failing on linux-s390x-ibm as of CL 440220

[bcmills]

https://build.golang.org/log/59c45ddde98d0d3c660c381aa819fa1a5b85e154:

session leader error: error setting tty process group: no such processerror running second subprocess: error setting tty process group: no such process
--- FAIL: TestTerminalSignal (0.01s)
    signal_cgo_test.go:186: Sending ^Z...
    signal_cgo_test.go:195: error reading readiness: EOF
FAIL
FAIL	os/signal	1.782s

TestTerminalSignal was rewritten in https://go.dev/cl/440220 to make direct system calls.
It appears that the call that is failing is an ioctl with argument TIOCSPGRP.
It could possibly be caused by an invalid return from syscall.Getpgrp().

(attn @golang/s390x; CC @prattmic)

[gopherbot]

Change https://go.dev/cl/443066 mentions this issue: os/signal: add missing newlines to TestTerminalSignal

[prattmic]

I tried to take a quick look at this on a linux-s390x gomote, but that builder apparently doesn't have strace installed. :(

[jonathan-albrecht-ibm]

I tried to take a quick look at this on a linux-s390x gomote, but that builder apparently doesn't have strace installed. :(

@prattmic I've installed strace on all the s390x builders if you can try again

[prattmic]

Awesome, thanks. @bcmills was right that the ioctl argument is incorrect (at least on s390x, still waiting on mips buildlet):

1185859 getpgid(0 <unfinished ...>
1185859 <... getpgid resumed>)          = 1185859
1185859 ioctl(3, TIOCSPGRP, [0] <unfinished ...>
1185859 <... ioctl resumed>)            = -1 ESRCH (No such process)

[0] should be [1185859].

Is there an ABI difference for ioctl arguments on s390x? That seems like the most obvious cause, though I don't see evidence of that in ioctl calls in syscall or x/sys or from a quick glance at the Linux source.

I'm going to send a CL to skip this test on s390x for now.

[jonathan-albrecht-ibm]

@prattmic it looks like this is an endian issue on s390x. If I cast the pids to int32 it passes for me locally. Not sure if int32 is correct for all platforms this test runs on.

diff --git a/src/os/signal/signal_cgo_test.go b/src/os/signal/signal_cgo_test.go
index 3625637432..086d56a251 100644
--- a/src/os/signal/signal_cgo_test.go
+++ b/src/os/signal/signal_cgo_test.go
@@ -279,7 +279,7 @@ func runSessionLeader(pause time.Duration) {
 		}
 
 		// Take TTY.
-		pgrp := syscall.Getpgrp()
+		pgrp := int32(syscall.Getpgrp())
 		_, _, errno = syscall.Syscall(syscall.SYS_IOCTL, ptyFD, syscall.TIOCSPGRP, uintptr(unsafe.Pointer(&pgrp)))
 		if errno != 0 {
 			return fmt.Errorf("error setting tty process group: %w", errno)
@@ -290,7 +290,7 @@ func runSessionLeader(pause time.Duration) {
 		time.Sleep(pause)
 
 		// Give TTY back.
-		pid := uint64(cmd.Process.Pid)
+		pid := int32(cmd.Process.Pid)
 		_, _, errno = syscall.Syscall(syscall.SYS_IOCTL, ptyFD, syscall.TIOCSPGRP, uintptr(unsafe.Pointer(&pid)))
 		if errno != 0 {
 			return fmt.Errorf("error setting tty process group back: %w", errno)

[ianlancetaylor]

The code is passing the address of a Go uintptr to a function that takes a pointer to a C int. That will fail on any big-endian 64-bit system. Sent CL 443175.

[gopherbot]

Change https://go.dev/cl/443175 mentions this issue: os/signal: pass *int32 to ioctl that expects pid_t