From fd9a38ae30df4a57426fc0a561240f78a16e8715 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Sat, 4 Jun 2022 13:07:39 +0200 Subject: [PATCH] Fix linker error if go_library with cgo references unresolved symbol Before this commit, if a go_library referenced an unresolved C symbol, the build would fail with an error about an undefined symbol since compilepkg.go tries to link all object files into an executable to be consumed by cgo's -dynimport argument. Since this library is never used for any purpose other than having its symbols parsed by the cgo command, retry with additional linker flags intended to make the linker ignore unresolved symbols. If they remain missing in the real link step, they will still be reported as unresolved there. Fixes https://github.com/golang/go/issues/25832 for rules_go. --- go/tools/builders/cgo2.go | 26 ++++++++++++++++++++++- tests/core/cgo/BUILD.bazel | 20 +++++++++++++++++ tests/core/cgo/provide_external_symbol.go | 12 +++++++++++ tests/core/cgo/use_external_symbol.go | 10 +++++++++ 4 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 tests/core/cgo/provide_external_symbol.go create mode 100644 tests/core/cgo/use_external_symbol.go diff --git a/go/tools/builders/cgo2.go b/go/tools/builders/cgo2.go index 36103ae2fb..d6cd6a2d0e 100644 --- a/go/tools/builders/cgo2.go +++ b/go/tools/builders/cgo2.go @@ -197,7 +197,31 @@ func cgo2(goenv *env, goSrcs, cgoSrcs, cSrcs, cxxSrcs, objcSrcs, objcxxSrcs, sSr args = append([]string{cc, "-o", mainBin, mainObj}, cObjs...) args = append(args, combinedLdFlags...) if err := goenv.runCommand(args); err != nil { - return "", nil, nil, err + // If linking the binary for cgo fails, this is usually because the + // object files reference external symbols that can't be resolved yet. + // Since the binary is only produced to have its symbols read by the cgo + // command, there is no harm in trying to build it allowing unresolved + // symbols - the real link that happens at the end will fail if they + // rightfully can't be resolved. + var allowUnresolvedSymbolsLdFlags []string + switch os.Getenv("GOOS") { + case "windows": + // MinGW's linker doesn't seem to support --unresolved-symbols + // and MSVC isn't supported at all. + return "", nil, nil, err + case "darwin": + case "ios": + allowUnresolvedSymbolsLdFlags = []string{"-Wl,-undefined,dynamic_lookup"} + default: + allowUnresolvedSymbolsLdFlags = []string{"-Wl,--unresolved-symbols=ignore-all"} + } + if err2 := goenv.runCommand(append(args, allowUnresolvedSymbolsLdFlags...)); err2 != nil { + // Return the original error if we can't link the binary with the + // additional linker flags as they may simply be incorrect for the + // particular compiler/linker pair and would obscure the true reason + // for the failure of the original command. + return "", nil, nil, err + } } cgoImportsGo := filepath.Join(workDir, "_cgo_imports.go") diff --git a/tests/core/cgo/BUILD.bazel b/tests/core/cgo/BUILD.bazel index 7f39f6b29a..cefa1b9812 100644 --- a/tests/core/cgo/BUILD.bazel +++ b/tests/core/cgo/BUILD.bazel @@ -414,3 +414,23 @@ go_bazel_test( name = "external_includes_test", srcs = ["external_includes_test.go"], ) + +go_library( + name = "use_external_symbol", + srcs = ["use_external_symbol.go"], + cgo = True, + importpath = "github.com/bazelbuild/rules_go/tests/core/cgo/use_external_symbol", + tags = ["manual"], +) + +go_binary( + name = "provide_external_symbol", + srcs = ["provide_external_symbol.go"], + cgo = True, + target_compatible_with = select({ + "@platforms//os:osx": [], + "@platforms//os:linux": [], + "//conditions:default": ["@platforms//:incompatible"], + }), + deps = [":use_external_symbol"], +) diff --git a/tests/core/cgo/provide_external_symbol.go b/tests/core/cgo/provide_external_symbol.go new file mode 100644 index 0000000000..68d9fce1e0 --- /dev/null +++ b/tests/core/cgo/provide_external_symbol.go @@ -0,0 +1,12 @@ +package main + +import "C" + +import "github.com/bazelbuild/rules_go/tests/core/cgo/use_external_symbol" + +//export external_symbol +func external_symbol() {} + +func main() { + use_external_symbol.UseExternalSymbol() +} diff --git a/tests/core/cgo/use_external_symbol.go b/tests/core/cgo/use_external_symbol.go new file mode 100644 index 0000000000..70980ea7db --- /dev/null +++ b/tests/core/cgo/use_external_symbol.go @@ -0,0 +1,10 @@ +package use_external_symbol + +/* +void external_symbol(); +*/ +import "C" + +func UseExternalSymbol() { + C.external_symbol() +}