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

Jump to bottom

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

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

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

kevinburke opened this issue Jul 31, 2018 · 8 comments
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-Darwin release-blocker
Milestone
Go1.11

Comments

@kevinburke
Copy link
Contributor

kevinburke commented Jul 31, 2018 •
edited

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 kevinburke mentioned this issue Jul 31, 2018
Closed
@kevinburke
Copy link
Contributor Author

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!

@kevinburke kevinburke mentioned this issue Jul 31, 2018
Closed
@odeke-em
Copy link
Member

/cc @randall77 @ianlancetaylor

@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
@randall77
Copy link
Contributor

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
@kevinburke
Fix C code to work with Go 1.11
976cc72 
Go commit golang/go@94076feef 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
@kevinburke
Fix C code to work with Go 1.11
b934599 
Go commit golang/go@94076feef 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 kevinburkeomg mentioned this issue Jul 31, 2018
Closed
@ianlancetaylor
Copy link
Contributor

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 the NeedsFix The path to resolution is known, but the work has not been done. label Jul 31, 2018
@ianlancetaylor ianlancetaylor added this to the Go1.11 milestone Jul 31, 2018
@kevinburke
Copy link
Contributor Author

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
Copy link
Contributor Author

kevinburke commented Aug 1, 2018 •
edited

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
Copy link

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
@kevinburkeomg
Update go-keychain to fix build on Go 1.11
33ebdf9 
Previously this would fail with a compilation error. For more
information see:

golang/go@94076feef
golang/go#26721
keybase/go-keychain#30
@kevinburkeomg kevinburkeomg mentioned this issue Aug 1, 2018
Merged
@gopherbot
Copy link

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

@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.
Assignees
No one assigned
Labels
Documentation FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-Darwin release-blocker
Projects
None yet
Milestone
Go1.11
Development

No branches or pull requests
6 participants
@kevinburke @ianlancetaylor @odeke-em @randall77 @gopherbot @akalin-keybase