Skip to content

Commit

Permalink
Fix linker error if go_library with cgo references unresolved symbol
Browse files Browse the repository at this point in the history
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 golang/go#25832 for rules_go.
  • Loading branch information
fmeum committed Jun 4, 2022
1 parent e43a3bc commit fd9a38a
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 1 deletion.
26 changes: 25 additions & 1 deletion go/tools/builders/cgo2.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
20 changes: 20 additions & 0 deletions tests/core/cgo/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
)
12 changes: 12 additions & 0 deletions tests/core/cgo/provide_external_symbol.go
Original file line number Diff line number Diff line change
@@ -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()
}
10 changes: 10 additions & 0 deletions tests/core/cgo/use_external_symbol.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package use_external_symbol

/*
void external_symbol();
*/
import "C"

func UseExternalSymbol() {
C.external_symbol()
}

0 comments on commit fd9a38a

Please sign in to comment.