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.

Sign up for GitHub

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

Jump to bottom

cmd/cgo: value inconsistency on s390x when passing an int reference to a C function #24212

Closed
freeekanayaka opened this issue Mar 2, 2018 · 3 comments
Closed

cmd/cgo: value inconsistency on s390x when passing an int reference to a C function #24212

freeekanayaka opened this issue Mar 2, 2018 · 3 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.

Comments

@freeekanayaka
Copy link

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

ubuntu@bionic:~$ go version
go version go1.9.4 linux/s390x

Does this issue reproduce with the latest release?

Yes, same with 1.10.

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

ubuntu@bionic:~$ go env
GOARCH="s390x"
GOBIN=""
GOEXE=""
GOHOSTARCH="s390x"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/ubuntu/go"
GORACE=""
GOROOT="/usr/lib/go-1.9"
GOTOOLDIR="/usr/lib/go-1.9/pkg/tool/linux_s390x"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -march=z196 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build535251359=/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?

package main

/*

int foo(int *pValue) {
  *pValue = 1;
}
*/
import "C"
import "fmt"
import "unsafe"

func main() {
        value := 0
        pValue := (*C.int)(unsafe.Pointer(&value))
        C.foo(pValue)
        fmt.Println(value, *pValue)

(unfortunately it seems the Go playground does not support cgo)

What did you expect to see?

free@x1:~$ go run foo.go
1 1

This is the output on Linux x86_64 (Ubuntu 18.04) .

What did you see instead?

ubuntu@bionic:~$ go run foo.go
4294967296 1

This is the output on Linux s390x (Ubuntu 18.04) .

I think the code above respects all rules of cgo, and yet the variable value gets assigned a different integer than *pValue.
freeekanayaka added a commit to CanonicalLtd/go-sqlite3 that referenced this issue Mar 2, 2018
@freeekanayaka
Workaround cgo bug on s390x
90f12cb 
See golang/go#24212
@ALTree ALTree changed the title Value inconsistency on s390x when passing an int reference to a C function cmd/cgo: value inconsistency on s390x when passing an int reference to a C function Mar 2, 2018
@ALTree ALTree added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 2, 2018
@ALTree
Copy link
Member

ALTree commented Mar 2, 2018

cc @mundaym

@slrz
Copy link

slrz commented Mar 2, 2018 •
edited
Loading

It's just that the size of a Go int is different from a C int. Conversions involving unsafe.Pointer are called that way for a reason. The differing results are likely due to endianess (s390x is BE), but it's just as wrong on x86.

Set some of the high bits in the value variable before making the cgo call and you'll see they don't get cleared by the C assignment on x86.

func main() {
        value := ^0 // ← inserted the ^ here
        pValue := (*C.int)(unsafe.Pointer(&value))
        C.foo(pValue)
        fmt.Println(value, *pValue)
}

Output on AMD64:

-4294967295 1

@mundaym
Copy link
Member

mundaym commented Mar 2, 2018

@slrz is correct. This appears to be a bug in the example code. You don't need to use unsafe for this, just allocate value as a C.int and then use a regular cast to convert it to an int if you need to.

@mundaym mundaym closed this as completed Mar 2, 2018
@golang golang locked and limited conversation to collaborators Mar 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@freeekanayaka @ALTree @slrz @gopherbot @mundaym