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

syscall: wrong TCGETS value on ppc64le #22000

Closed
zhsj opened this issue Sep 24, 2017 · 6 comments
Closed

syscall: wrong TCGETS value on ppc64le #22000

zhsj opened this issue Sep 24, 2017 · 6 comments

Comments

@zhsj
Copy link

@zhsj zhsj commented Sep 24, 2017

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

go version go1.9 linux/ppc64le

Does this issue reproduce with the latest release?

yes, with go1.9

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

GOARCH="ppc64le"
GOBIN=""
GOEXE=""
GOHOSTARCH="ppc64le"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/debian/go"
GORACE=""
GOROOT="/usr/lib/go-1.9"
GOTOOLDIR="/usr/lib/go-1.9/pkg/tool/linux_ppc64le"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build617591770=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"

What did you do?

I meet strange output when I use github.com/mattn/go-isatty, the following snippet explains.


/*
#include <unistd.h>
*/
import "C"
import "unsafe"
import "syscall"
import "golang.org/x/sys/unix"

func main() {
        C.isatty(C.int(0))
        var termios syscall.Termios
        syscall.Syscall6(syscall.SYS_IOCTL, 0, syscall.TCGETS, uintptr(unsafe.Pointer(&termios)), 0, 0, 0)
        unix.Syscall6(unix.SYS_IOCTL, 0, unix.TCGETS, uintptr(unsafe.Pointer(&termios)), 0, 0, 0)
        syscall.Syscall6(syscall.SYS_IOCTL, 0, 0x402c7413, uintptr(unsafe.Pointer(&termios)), 0, 0, 0)
}

run strace to see the real syscall.

strace ./isatty 2>&1 |grep -i ioc
ioctl(0, TCGETS, {B38400 opost isig icanon echo ...}) = 0
ioctl(0, _IOC(_IOC_READ, 0x74, 0x13, 0x3c), 0xc42004fef8) = -1 ENOTTY (Inappropriate ioctl for device)
ioctl(0, TCGETS, {B38400 opost isig icanon echo ...}) = 0
ioctl(0, TCGETS, {B38400 opost isig icanon echo ...}) = 0

The syscall.TCGETS is wrong and should be 0x402c7413(the one in sys/unix)

@zhsj
Copy link
Author

@zhsj zhsj commented Sep 24, 2017

And I get same result when I use gccgo

go version go1.8.3 gccgo (Debian 20170830-1) 8.0.0 20170830 (experimental) [trunk revision 251527] linux/ppc64le
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Sep 24, 2017

This seems the same as #19560, but that issue was closed by a seemingly irrelevant change. I'm not sure what's going on here; I thought that in #19560 we agreed that we should change the values in the syscall package.

CC @laboger

@ianlancetaylor ianlancetaylor added this to the Go1.10 milestone Sep 24, 2017
@zhsj
Copy link
Author

@zhsj zhsj commented Sep 25, 2017

@ianlancetaylor thanks for the reference issue. I didn't find it since I forgot to search on the closed issues.
After reading the discussion, I think I need to raise the issue on github.com/mattn/go-isatty too, since it's using std syscall, regarding the std syscall should be fixed or not.

@laboger
Copy link
Contributor

@laboger laboger commented Sep 25, 2017

My understanding is that x/sys should be used wherever possible, but there are still cases where std syscall has to be used because not everything has been migrated over. We opened #19697 for this question. The documentation in the file syscall/syscall.go was updated to indicate what is missing from the migration. I think at the time we had hit this problem, x/sys was undergoing major changes and @bradfitz asked us to wait for that to be finished before making any changes.

I think Signal and Errno could be migrated, but I don't think SysProcAttr could be for compatibility reasons.

@ceseo

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Sep 25, 2017

@laboger I think the question in this specific issue is: why not just correct the value of TCGETS in the syscall package? As discussed in #19560 correcting the value of syscall.TCGETS would be OK per syscall package rules.

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 27, 2017

Change https://golang.org/cl/66510 mentions this issue: syscall: correct TCGETS/TCSETS values on ppc64/ppc64le

@gopherbot gopherbot closed this in 197f9ba Sep 27, 2017
@golang golang locked and limited conversation to collaborators Sep 27, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.