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

cmd/cgo: CoreFoundation go-keychain compile failure on tip #26721

Closed
kevinburke opened this issue Jul 31, 2018 · 8 comments

Comments

@kevinburke
Copy link
Contributor

commented Jul 31, 2018

Please answer these questions before submitting your issue. Thanks!

What did you do?

Check out tip of github.com/keybase/go-keychain (commit keybase/go-keychain@70b98e9) and run go build github.com/keybase/go-keychain

What did you expect to see?

I expected the program to compile.

What did you see instead?

$ go build .
# github.com/keybase/go-keychain
./corefoundation_1.10.go:55: cannot use nil as type _Ctype_CFAllocatorRef in argument to _Cfunc_CFDataCreate
./corefoundation_1.10.go:81: cannot use nil as type _Ctype_CFAllocatorRef in argument to func literal
./corefoundation_1.10.go:118: cannot use nil as type _Ctype_CFAllocatorRef in argument to _Cfunc_CFStringCreateWithBytes
./corefoundation_1.10.go:153: cannot use nil as type _Ctype_CFAllocatorRef in argument to func literal

Does this issue reproduce with the latest release (go1.10.3)?

No. I can reproduce it using tip though, which suggests that this code will break in the upcoming Go 1.11 release.

A git bisect revealed that commit 94076fe is the first commit to break; versions of Go compiled with commits earlier than this commit successfully compile the go-keychain library, newer ones don't.

System details

go version devel +94076feef5 Mon Jul 9 22:19:21 2018 +0000 darwin/amd64
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/kevin.burke/Library/Caches/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/kevin.burke"
GOPROXY=""
GORACE=""
GOROOT="/Users/kevin.burke/go"
GOTMPDIR=""
GOTOOLDIR="/Users/kevin.burke/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
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"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/bs/n3zm0wvn7j14n9gm_ybk4klsg6mgyh/T/go-build618247201=/tmp/go-build -gno-record-gcc-switches -fno-common"
VGOMODROOT=""
GOROOT/bin/go version: go version devel +94076feef5 Mon Jul 9 22:19:21 2018 +0000 darwin/amd64
GOROOT/bin/go tool compile -V: compile version devel +94076feef5 Mon Jul 9 22:19:21 2018 +0000
uname -v: Darwin Kernel Version 17.7.0: Thu Jun 21 22:53:14 PDT 2018; root:xnu-4570.71.2~1/RELEASE_X86_64
ProductName:	Mac OS X
ProductVersion:	10.13.6
BuildVersion:	17G65
lldb --version: lldb-902.0.79.7
  Swift-4.1
@kevinburke

This comment has been minimized.

Copy link
Contributor Author

commented Jul 31, 2018

I added the release blocker tag since the code in question works fine on Go 1.10 but breaks on Go 1.11. If the problem is determined to be in the go-keychain code, please feel free to remove the tag!

@odeke-em

This comment has been minimized.

Copy link
Member

commented Jul 31, 2018

@odeke-em odeke-em changed the title CoreFoundation go-keychain compile failure on tip cmd/cgo: CoreFoundation go-keychain compile failure on tip Jul 31, 2018

@odeke-em odeke-em added the OS-Darwin label Jul 31, 2018

@randall77

This comment has been minimized.

Copy link
Contributor

commented Jul 31, 2018

This is expected. We've expanded the cases where we change the CF*Ref types from real pointers to uintptrs. Places where they weren't being converted caused mismatches between source files in the same package. See issue #24161.
You'll need to change nil to 0 in a few places.
Sorry about that. Looks like you'll need a corefoundation_1.11.go :(

kevinburkeomg pushed a commit to kevinburkeomg/go-keychain that referenced this issue Jul 31, 2018
Fix C code to work with Go 1.11
Go commit golang/go@94076fe changed the
way that some C types are handled. Per Keith Randall, "places where
they weren't being converted caused mismatches between source files in
the same package."

He suggested on golang/go#26721 that we should change the
`nil` types to `0`. Doing so makes the code compile on
tip. Further, add a `.travis.yml` so we can automatically
compile + test on master. A sample build can be found here:
https://travis-ci.org/kevinburkeomg/go-keychain/jobs/410508844
kevinburkeomg pushed a commit to kevinburkeomg/go-keychain that referenced this issue Jul 31, 2018
Fix C code to work with Go 1.11
Go commit golang/go@94076fe changed the
way that some C types are handled. Per Keith Randall, "places where
they weren't being converted caused mismatches between source files in
the same package."

He suggested on golang/go#26721 that we should change the
`nil` types to `0`. Doing so makes the code compile on
tip. Further, add a `.travis.yml` so we can automatically
compile + test on master. A sample build can be found here:
https://travis-ci.org/kevinburkeomg/go-keychain/jobs/410508844
@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Jul 31, 2018

If there are more cases in Go 1.10, then we should say something in the 1.11 release notes. I don't see anything there now.

@ianlancetaylor ianlancetaylor added this to the Go1.11 milestone Jul 31, 2018

@kevinburke

This comment has been minimized.

Copy link
Contributor Author

commented Aug 1, 2018

If there are more cases in Go 1.10, then we should say something in the 1.11 release notes. I don't see anything there now.

This is the only library I've seen with a compatibility issue, though maybe we could do a fancy Github code search for, say, CFDataCreate(nil to see how many libraries have this problem.

@kevinburke

This comment has been minimized.

Copy link
Contributor Author

commented Aug 1, 2018

There are a fair number of results here and there are 3 other function calls in the linked file that have compatibility issues so maybe I'll submit some patches against these projects. To be honest I don't really understand the underlying problem that well - even after reading through the linked issue - so I might just write "this needs to change for Go 1.11 and changing nil to 0 will make your code compile again."

https://github.com/search?p=2&q=language%3Ago+CFDataCreate%28nil&type=Code

@akalin-keybase

This comment has been minimized.

Copy link

commented Aug 1, 2018

An alternate fix I implemented in keybase/go-keychain#31 is to use the typed constant kCFAllocatorDefault, which works with both go 1.10 and 1.11. That saves having to have 1.11-specific files.

This takes care of the common case in CoreFoundation where either NULL or a typed constant can be passed in as an argument. I suspect a lot of the other cases can be handled similarly. At worst, one might need to add a typed constant in the C preamble.

kevinburkeomg added a commit to kevinburkeomg/aws-vault that referenced this issue Aug 1, 2018
Update go-keychain to fix build on Go 1.11
Previously this would fail with a compilation error. For more
information see:

golang/go@94076fe
golang/go#26721
keybase/go-keychain#30
@gopherbot

This comment has been minimized.

Copy link

commented Aug 6, 2018

Change https://golang.org/cl/127975 mentions this issue: doc: describe cgo ptr->uintptr changes for 1.11.

@gopherbot gopherbot closed this in 8cc7540 Aug 6, 2018

@golang golang locked and limited conversation to collaborators Aug 6, 2019

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
6 participants
You can’t perform that action at this time.