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: cgo does not link dependencies into archives #14985

Open
tamird opened this Issue Mar 27, 2016 · 23 comments

Comments

Projects
None yet
4 participants
@tamird
Contributor

tamird commented Mar 27, 2016

Please answer these questions before submitting your issue. Thanks!

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

go version go1.6 darwin/amd64

  • What operating system and processor architecture are you using (go env)?
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/tamird/src/go"
GORACE=""
GOROOT="/Users/tamird/src/go1.6"
GOTOOLDIR="/Users/tamird/src/go1.6/pkg/tool/darwin_amd64"
GO15VENDOREXPERIMENT="1"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fno-common"
CXX="clang++"
CGO_ENABLED="1"
  • What did you do?

Compiling https://github.com/cockroachdb/c-rocksdb with the LDFLAGS directives at the bottom of https://github.com/cockroachdb/c-rocksdb/blob/master/cgo_flags.go removed.

c-rocksdb is a "thin" wrapper around https://github.com/facebook/rocksdb which allows it to be built using the go toolchain. However, it depends on lz4, so we bring in http://github.com/cockroachdb/c-lz4 which is a similar wrapper.

Unfortunately, c-lz4 is being linked (by cmd/link?) in a later step than the external linking used for c-rocksdb, which causes missing symbols in c-rocksdb (hence the need for the LDFLAGS directives).

Is it possible to ask the toolchain to link c-lz4.a using external linking (in the "main" linking step in c-rocksdb)?

@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented Mar 27, 2016

We don't use the issue tracker for asking questions. Please see https://golang.org/wiki/Questions . Thanks.

I'm not sure I entirely understand the question, but you can add additional linker options on the go tool command line by using -ldflags=-extldflags=/a/b/c/c-lz4.a.

@tamird

This comment has been minimized.

Contributor

tamird commented Mar 28, 2016

Sorry, despite this being phrased as a question, it is really a bug report that should be titled "it is not possible for cgo code to depend on another package's symbols".

I'm not sure I entirely understand the question, but you can add additional linker options on the go tool command line by using -ldflags=-extldflags=/a/b/c/c-lz4.a.

Yes, that's possible, but unfortunately not really practical since c-lz4 is in a platform-specific location. This bug report is about the lack of facility in the cgo toolchain to automatically pass the correct path to c-lz4.

@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented Mar 28, 2016

I don't understand how this can be fixed. What is your suggestion?

@tamird

This comment has been minimized.

Contributor

tamird commented Mar 28, 2016

I don't know enough about the cgo implementation to say that this is a reasonable thing to do, but: if package A uses cgo and depends on package B (via a Go import), could the toolchain pass -l<path_to_packageB.a> to the external linker when linking package A?

@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented Mar 28, 2016

In effect, the linker already does that.

I now suspect that the problem you are seeing is not in the linker at all. I suspect that you are seeing a problem when running the cgo tool. Can you post the output of go install -x for your library? That will show the actual error, and where it is coming from.

@tamird

This comment has been minimized.

Contributor

tamird commented Mar 28, 2016

This is go install -x github.com/cockroachdb/c-rocksdb with the following diff applied:

diff --git a/cgo_flags.go b/cgo_flags.go
index ccc3dd9..bd82520 100644
--- a/cgo_flags.go
+++ b/cgo_flags.go
@@ -22,6 +22,4 @@ import (
 // #cgo darwin CXXFLAGS: -Wshorten-64-to-32
 // #cgo freebsd CXXFLAGS: -Wshorten-64-to-32
 // #cgo dragonfly CXXFLAGS: -Wshorten-64-to-32
-// #cgo darwin LDFLAGS: -Wl,-undefined -Wl,dynamic_lookup
-// #cgo !darwin LDFLAGS: -Wl,-unresolved-symbols=ignore-all
 import "C"

https://gist.github.com/tamird/c3843511bb12b6231c9e

@tamird

This comment has been minimized.

Contributor

tamird commented Mar 28, 2016

Here's the same thing prepended with CGO_FLAGS=-v https://gist.github.com/tamird/335e1c24989024989651

@tamird

This comment has been minimized.

Contributor

tamird commented Mar 28, 2016

Fixed the rocksdb::slice::Slice issue - note the snappy symbols are still missing https://gist.github.com/tamird/4073cef9ae9a109bc5b6

@bradfitz bradfitz added this to the Unplanned milestone Apr 9, 2016

@tamird

This comment has been minimized.

Contributor

tamird commented Apr 25, 2016

@ianlancetaylor any further thoughts on this?

@tamird

This comment has been minimized.

Contributor

tamird commented May 18, 2016

@ianlancetaylor here's a discussion from the mingw-w64 mailing list where it's suggested that the PE specification does not support unresolved symbols after link time - which means that resolving this issue is required to make this style of distribution work on windows.

@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented May 18, 2016

I'm not sure, but the fix for this may be that if package p1 uses cgo and imports package p2, and package p2 also uses cgo, then the go tool should gather together the #cgo lines from p2 and pass them to cgo when running it for package p1.

@tamird

This comment has been minimized.

Contributor

tamird commented May 19, 2016

Sounds like it could lead to unintended consequences with cgo *FLAGS interactions. When p1 is being externally linked, can cgo pass to the external linker p2's .a (or equivalent), in addition to p1's object files?

@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented May 20, 2016

I don't see how that would suffice. p2's .a archive might well depend on other shared libraries elsewhere in the system.

@tamird

This comment has been minimized.

Contributor

tamird commented May 20, 2016

I'd say that in this case p2's cgo directives can point at those shared libraries. Otherwise, we may end up with something quite complex here, and all we're trying to solve is to make the windows situation workable, not perfect.

@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented May 20, 2016

I don't think I understand. If p2.a depends on some shared library, then how does passing p2.a to p1's build help? The link that cgo tries to do will still fail.

Another approach we could take is simply have cgo not worry about undefined symbols, by passing the appropriate linker options. I'm not sure how well that would work in general, but it would solve the problem by ignoring it.

@tamird

This comment has been minimized.

Contributor

tamird commented May 21, 2016

If p2.a depends on some shared library, and p1.a depends on p2.a, the #cgo directives in p1 can specify -l<p2's shared library dependency>. In other words, the user can work around this problem. That's in contrast to the current situation, where p2.a isn't linked into p1.a, and the user can't predict the location of p2.a to work around the situation manually.

Having cgo not worry about undefined symbols won't work because that's not a concept that exists in Portable Executables (windows).

@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented May 21, 2016

OK. Actually, though, I'm not sure your concern about flag interactions is valid, since all the LDFLAGS are going to be agglomerated in the final link anyhow.

@tamird

This comment has been minimized.

Contributor

tamird commented May 21, 2016

all the LDFLAGS are going to be agglomerated in the final link anyhow.

I don't think that's true. LDFLAGS may contain more than just an enumeration of libraries, it may also contain behaviour-modifying linker flags; if I'm the author of p1 but not p2, and I need to pass some linker flag (for p1) that conflicts with some flag the author of p2 is passing, I'm stuck. In other words, it's not the case that all linker flags are agglomerated by the transitive property.

@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented May 21, 2016

My point is that if you do that you are stuck anyhow. All the LDFLAGS are gathered together, and passed together to the external linker when linking the final binary.

@tamird

This comment has been minimized.

Contributor

tamird commented May 21, 2016

Oh, I didn't know that was the case. I still think that linking p1 against p2.a is a nicer solution since p2's artifacts won't need to recompiled, but it's your call - there are no additional constraints that I'm aware of.

@tamird

This comment has been minimized.

Contributor

tamird commented May 26, 2016

To repro using xgo:

go get -u github.com/karalabe/xgo
xgo --image cockroachdb/xgo --targets=windows-6.0/amd64 --go=1.6.2 github.com/cockroachdb/cockroach

@tamird tamird changed the title from cmd/cgo: is it possible for cgo code to depend on another package's symbols? to cmd/cgo: cgo does not link dependencies into archives May 29, 2016

@rafaelaffonsor

This comment has been minimized.

rafaelaffonsor commented Sep 27, 2018

Guys, this is still open ? If so, we have some newer evidences so i can help to work on it ?
Thanks

@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented Sep 27, 2018

To the best of my knowledge this is still open. I don't know whether the original test case still fails, but the behavior of cgo has not changed in this regard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment