Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/sys/unix: IoctlGetInt broken on 64-bit big-endian platforms #60429

Open
greatroar opened this issue May 25, 2023 · 5 comments
Open

x/sys/unix: IoctlGetInt broken on 64-bit big-endian platforms #60429

greatroar opened this issue May 25, 2023 · 5 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. help wanted NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@greatroar
Copy link

What version of Go are you using (go version)?

$ go version
go version go1.20.3 linux/amd64
$ go list -m golang.org/x/sys
golang.org/x/sys v0.8.1-0.20230523194307-b5c7a0975ddc

What operating system and processor architecture are you using (go env)?

GOARCH=s390x
GOOS=linux

(using QEMU on an amd64 host)

What did you do?

package main

import (
	"fmt"
	"os"

	"golang.org/x/sys/unix"
)

func main() {
	tty, err := os.OpenFile("/dev/tty", os.O_RDWR, 0)
	if err != nil {
		panic(err)
	}
	pgid, err := unix.IoctlGetInt(int(tty.Fd()), unix.TIOCGPGRP)
	fmt.Println(pgid, unix.Getpgrp())
}

What did you expect to see?

Same number twice, as on amd64:

$ go run pgrp.go
101981 101981

What did you see instead?

$ GOARCH=s390x go run pgrp.go
438060894388224 101994

The first number is 101994<<32. I believe this happens because IoctlGetInt passes a pointer to a Go int to the system call:

func IoctlGetInt(fd int, req uint) (int, error) {
        var value int
        err := ioctlPtr(fd, req, unsafe.Pointer(&value))
        return value, err
}

The ioctl system call expects a pointer to a C int. Passing a Go int means it stores the pgid in the upper 32 bits.

Note that IoctlSetPointerInt passes a pointer to an int32:

func IoctlSetPointerInt(fd int, req uint, value int) error {
        v := int32(value)
        return ioctlPtr(fd, req, unsafe.Pointer(&v))
}
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label May 25, 2023
@gopherbot gopherbot added this to the Unreleased milestone May 25, 2023
@bcmills
Copy link
Contributor

bcmills commented May 25, 2023

(CC @tklauser @golang/s390x @golang/ppc64)

@greatroar
Copy link
Author

I guess the underlying issue was already discovered, #45585.

@greatroar
Copy link
Author

I also wasn't aware of IoctlGetUint32. That solves my original problem (emulating tcgetpgrp), so the only remaining issue is documentation: IoctlGetInt is inconsistent with IoctlSetPointerInt, which is needed to emulate tcsetpgrp.

@mknyszek mknyszek added the NeedsFix The path to resolution is known, but the work has not been done. label May 30, 2023
@mknyszek
Copy link
Contributor

Just quickly skimming this, I think that indeed it would be sufficient to just document this. Though, we should also just fix GetInt on the platforms where it's broken. (Document the "int" discrepancy and then do a conversion inside GetInt on the affected platforms.)

Would you be willing to send a CL for this? :) (https://go.dev/doc/contribute)

@mknyszek mknyszek self-assigned this May 31, 2023
@mknyszek mknyszek removed their assignment Jul 21, 2023
@aimuz
Copy link
Contributor

aimuz commented Nov 7, 2023

Does this need fixing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. help wanted NeedsFix The path to resolution is known, but the work has not been done.
Projects
Development

No branches or pull requests

5 participants