cgo: C++ static initialization not working #1486
Comments
|
Note that this also applies to code like: __attribute__((constructor))
static void init() {
puts("Hello from init!");
} |
|
Here's the build log, along with some observations paced inline: Let's look at those params: And the symbols: So Here's where things get interesting. Let's look at the param file: So this is where we're linking cxxlib and cxxdep. To confirm that all the expected symbols are present: Looks good! Curiously, the subcommand above is the last time we'll see any use of This concludes our spelunking. On a hunch, I tried using the following configuration: And with the sandbox disabled so we can refer to We see that our binary works as expected: So what's the deal here? Should the If someone can identify and outline what needs to be done, I can whip up a pull request. |
|
I should note that the aforementioned hack doesn't work if we're building a static binary: go_binary(
name = "demogo",
srcs = [
"main.go",
],
cdeps = [
":cxxlib",
":cxxdep",
],
cgo = True,
gc_linkopts = [
"-linkmode",
"external",
"-extldflags",
"-Wl,bazel-out/k8-fastbuild/bin/demogo._cgo_.o"
],
static = "on", # <==== !!!
) |
|
@jayconrod If you could lend me a hand with this, you would be my hero ❤️. |
|
@cstrahan I'll look into this as soon as I can. Sorry I haven't gotten to this already, but I've been swamped with the Go summit last week and some travel afterward. |
|
Ok, I looked into this a bit. The library with the static initializer does get passed to the linker, but the linker doesn't include it in the final binary because it doesn't provide any symbols that are undefined at that point. In order for this to work correctly, To work around this, you could try defining a dummy symbol in the library with the static initializer and referencing it somewhere. For example: A long-winded explanation may be useful here. When we build a Go package that contains cgo, we follow these steps (roughly):
When we link a Go binary containing cgo:
In order for us to handle this case correctly, we would need to patch the code that |
|
@jayconrod Thanks for the pointers! Here's my (admittedly gross) hack: diff --git a/go/tools/builders/cgo.go b/go/tools/builders/cgo.go
index 61f3f80..6285b19 100644
--- a/go/tools/builders/cgo.go
+++ b/go/tools/builders/cgo.go
@@ -30,10 +30,13 @@ import (
"log"
"os"
"path/filepath"
+ "regexp"
"strings"
"unicode"
)
+var linkLine = regexp.MustCompile(`^//go:cgo_ldflag ".*\.(o|lo|a)"$`)
+
func run(args []string) error {
builderArgs, toolArgs := splitArgs(args)
sources := multiFlag{}
@@ -57,6 +60,7 @@ func run(args []string) error {
if err != nil {
return err
}
+
goargs := append([]string{"tool", "cgo", "-dynpackage", dynpackage}, toolArgs...)
return goenv.runGoCommand(goargs)
}
@@ -195,6 +199,44 @@ func run(args []string) error {
return err
}
}
+
+ // Begin hack
+ // 1. Determine path to _cgo_gotypes.go
+ objdir := ""
+ for i, a := range toolArgs {
+ if a == "-objdir" {
+ objdir = toolArgs[i+1]
+ break
+ }
+ }
+ if objdir == "" {
+ return errors.New("could not determine -objdir")
+ }
+ gotypesPath := objdir + "/_cgo_gotypes.go"
+
+ dat, err := ioutil.ReadFile(gotypesPath)
+ if err != nil {
+ return err
+ }
+
+ // 2. Add --{,no-}whole-archive around each linked object/archive
+ lines := []string{}
+ for _, line := range strings.Split(string(dat), "\n") {
+ if linkLine.MatchString(line) {
+ lines = append(lines, `//go:cgo_ldflag "-Wl,--whole-archive"`)
+ lines = append(lines, line)
+ lines = append(lines, `//go:cgo_ldflag "-Wl,--no-whole-archive"`)
+ } else {
+ lines = append(lines, line)
+ }
+ }
+
+ err = ioutil.WriteFile(gotypesPath, []byte(strings.Join(lines, "\n")), 0555)
+ if err != nil {
+ return err
+ }
+ // End hack
+
return nil
}
That makes my minimal repro work, and I can't see any reason it wouldn't work for my actual project (though I now need to go test it out). |
Did you get a chance to confirm either way? If this is surfaced somewhere, I can try to implement and submit a fix this weekend. |
|
@cstrahan When I asked about this earlier, I was told there was no way to know, but passing Since that time, the C/C++ compile and link API has been overhauled (see |
|
@jayconrod Should this be resolved as of #1744? I see this quote there:
|
|
Sadly I don't think this is fixed. I originally intended to rewrite all of the cgo logic and fix this bug in the process. Now that Unfortunately, I haven't had time to do a proper rewrite. #1768 ended up being a quick fix to avoid being broken by incompatible Bazel changes and nothing more. |
Wouldn't that be relatively easy with an aspect ? |
|
I did a wip PR based on an aspect, it works well for us: #1879 |
|
@jayconrod Can you share any update on this? Is
Do you know if this has changed? My understanding is that the Bazel developers are attempting reach version 1.0 in September; I'd like to see if we can get that surfaced by then (if it's not already). |
|
I think this is now fixed with CcInfo, and can be used yes. Confirmed: https://docs.bazel.build/versions/0.28.0/skylark/lib/LibraryToLink.html#alwayslink |
|
@cstrahan The blocking issue for this was bazelbuild/bazel#7033, which is now fixed. So we'd need to pass |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Here's the scenario:
go_binarywhich depends on acc_library, let's call itcxxlibcxxlibin turn has a dependency on a secondcc_library, let's call itcxxdepcxxdepdoes one thing: it uses static initialization for some sort of side effect (say, registering some type in a plugin system)cc_libraryuselinkalways = 1, as we want to ensure that our symbols remain, despite the fact that some may not appear to the linker as used.What we expect:
What happens:
cxxdepwasn't actually linked.I have a minimal example that reproduces this here: https://github.com/cstrahan/rules_go_repro
Instructions:
$ bazel build :demogo$ bazel-bin/linux_amd64_stripped/demogoIn theory, you should see the following two lines, but you'll find that only the second line is printed:
You can compare that to what happens with
cc_binarylike so:$ bazel build :democc$ bazel-bin/democcYou'll see:
The text was updated successfully, but these errors were encountered: