diff --git a/go/tools/builders/compilepkg.go b/go/tools/builders/compilepkg.go index 4c7779c697..b3a6d283a9 100644 --- a/go/tools/builders/compilepkg.go +++ b/go/tools/builders/compilepkg.go @@ -272,8 +272,13 @@ func compileArchive( for i, src := range srcs.hSrcs { hSrcs[i] = src.filename } + + // haveCgo is true if the package contains Cgo files. haveCgo := len(cgoSrcs)+len(cSrcs)+len(cxxSrcs)+len(objcSrcs)+len(objcxxSrcs) > 0 - packageUsesCgo := cgoEnabled && haveCgo + // compilingWithCgo is true if the package contains Cgo files AND Cgo is enabled. A package + // containing Cgo files can also be built with Cgo disabled, and will work if there are build + // constraints. + compilingWithCgo := haveCgo && cgoEnabled // Instrument source files for coverage. if coverMode != "" { @@ -330,7 +335,7 @@ func compileArchive( // If we have cgo, generate separate C and go files, and compile the // C files. var objFiles []string - if packageUsesCgo { + if compilingWithCgo { // If the package uses Cgo, compile .s and .S files with cgo2, not the Go assembler. // Otherwise: the .s/.S files will be compiled with the Go assembler later var srcDir string @@ -355,7 +360,7 @@ func compileArchive( if err != nil { return err } - if packageUsesCgo { + if compilingWithCgo { // cgo generated code imports some extra packages. imports["runtime/cgo"] = nil imports["syscall"] = nil @@ -465,7 +470,7 @@ func compileArchive( asmHdrPath = filepath.Join(workDir, "go_asm.h") } var symabisPath string - if !packageUsesCgo { + if !haveCgo { symabisPath, err = buildSymabisFile(goenv, srcs.sSrcs, srcs.hSrcs, asmHdrPath) if symabisPath != "" { if !goenv.shouldPreserveWorkDir { @@ -482,8 +487,9 @@ func compileArchive( return err } - // Compile the .s files if we are not a cgo package; cgo is assembled by cc above - if len(srcs.sSrcs) > 0 && !packageUsesCgo { + // Compile the .s files with Go's assembler, if this is not a cgo package. + // Cgo is assembled by cc above. + if len(srcs.sSrcs) > 0 && !haveCgo { includeSet := map[string]struct{}{ filepath.Join(os.Getenv("GOROOT"), "pkg", "include"): {}, workDir: {}, diff --git a/go/tools/builders/filter.go b/go/tools/builders/filter.go index fbb0f2ac4d..328caac95b 100644 --- a/go/tools/builders/filter.go +++ b/go/tools/builders/filter.go @@ -69,11 +69,15 @@ type archiveSrcs struct { // them by extension. func filterAndSplitFiles(fileNames []string) (archiveSrcs, error) { var res archiveSrcs + packageContainsCgo := false for _, s := range fileNames { src, err := readFileInfo(build.Default, s) if err != nil { return archiveSrcs{}, err } + if src.isCgo { + packageContainsCgo = true + } if !src.matched { continue } @@ -96,6 +100,12 @@ func filterAndSplitFiles(fileNames []string) (archiveSrcs, error) { } *srcs = append(*srcs, src) } + if packageContainsCgo && !build.Default.CgoEnabled { + // Cgo packages use the C compiler for asm files, rather than Go's assembler. + // This is a package with cgo files, but we are compiling with Cgo disabled: + // Remove the assembly files. + res.sSrcs = nil + } return res, nil } diff --git a/tests/core/cgo/asm/BUILD.bazel b/tests/core/cgo/asm/BUILD.bazel index 98e618a01e..2e03cd90cc 100644 --- a/tests/core/cgo/asm/BUILD.bazel +++ b/tests/core/cgo/asm/BUILD.bazel @@ -9,6 +9,7 @@ go_library( ], cgo = True, importpath = "github.com/bazelbuild/rules_go/tests/core/cgo/asm", + visibility = ["//tests/core/cgo:__subpackages__"], ) go_test( diff --git a/tests/core/cgo/asm/cgoasm.go b/tests/core/cgo/asm/cgoasm.go index f0a3347493..188dec8238 100644 --- a/tests/core/cgo/asm/cgoasm.go +++ b/tests/core/cgo/asm/cgoasm.go @@ -7,6 +7,6 @@ extern int example_asm_func(); */ import "C" -func callAssembly() int { +func CallAssembly() int { return int(C.example_asm_func()) } diff --git a/tests/core/cgo/asm/cgoasm_test.go b/tests/core/cgo/asm/cgoasm_test.go index 9227ec0ea9..fbc0661b39 100644 --- a/tests/core/cgo/asm/cgoasm_test.go +++ b/tests/core/cgo/asm/cgoasm_test.go @@ -16,7 +16,7 @@ func TestNativeAssembly(t *testing.T) { if expected == 0 { t.Fatalf("expected=0 for GOARCH=%s; missing value?", runtime.GOARCH) } - actual := callAssembly() + actual := CallAssembly() if actual != expected { t.Errorf("callAssembly()=%d; expected=%d", actual, expected) } diff --git a/tests/core/cgo/asm_conditional_cgo/BUILD.bazel b/tests/core/cgo/asm_conditional_cgo/BUILD.bazel new file mode 100644 index 0000000000..ffd7b185ad --- /dev/null +++ b/tests/core/cgo/asm_conditional_cgo/BUILD.bazel @@ -0,0 +1,33 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "asm_conditional_cgo", + srcs = [ + "asm_amd64.S", + "asm_arm64.S", + "asm_cgo.go", + "asm_nocgo.go", + ], + cgo = True, + importpath = "github.com/bazelbuild/rules_go/tests/core/cgo/asm_conditional_cgo", + deps = ["//tests/core/cgo/asm"], +) + +# this is a "native" target: it uses cgo and calls the assembly function as expected +go_test( + name = "asm_conditional_cgo_test", + srcs = [ + "asm_conditional_cgo_test.go", + ], + embed = [":asm_conditional_cgo"], +) + +# this is a CGO_ENABLED=0 target: it does not import the cgo target +go_test( + name = "asm_conditional_nocgo_test", + srcs = [ + "asm_conditional_cgo_test.go", + ], + embed = [":asm_conditional_cgo"], + pure = "on", +) diff --git a/tests/core/cgo/asm_conditional_cgo/asm_amd64.S b/tests/core/cgo/asm_conditional_cgo/asm_amd64.S new file mode 100644 index 0000000000..97c5d1a36d --- /dev/null +++ b/tests/core/cgo/asm_conditional_cgo/asm_amd64.S @@ -0,0 +1,22 @@ +/* +https://stackoverflow.com/questions/73435637/how-can-i-fix-usr-bin-ld-warning-trap-o-missing-note-gnu-stack-section-imp +*/ +#if defined(__ELF__) && defined(__GNUC__) +.section .note.GNU-stack,"",@progbits +#endif + +/* +define this symbol both with and without underscore. +It seems like Mac OS X wants the underscore, but Linux does not. +*/ +.global example_asm_func +.global _example_asm_func +.text +example_asm_func: +_example_asm_func: + mov $42, %rax + ret + +#if NOT_DEFINED +#error "should not fail" +#endif diff --git a/tests/core/cgo/asm_conditional_cgo/asm_arm64.S b/tests/core/cgo/asm_conditional_cgo/asm_arm64.S new file mode 100644 index 0000000000..0585bf75db --- /dev/null +++ b/tests/core/cgo/asm_conditional_cgo/asm_arm64.S @@ -0,0 +1,15 @@ +/* +define this symbol both with and without underscore. +It seems like Mac OS X wants the underscore, but Linux does not. +*/ +.global example_asm_func +.global _example_asm_func +.p2align 2 /* ld: warning: arm64 function not 4-byte aligned */ +example_asm_func: +_example_asm_func: + mov x0, #44 + ret + +#if NOT_DEFINED +#error "should not fail" +#endif diff --git a/tests/core/cgo/asm_conditional_cgo/asm_cgo.go b/tests/core/cgo/asm_conditional_cgo/asm_cgo.go new file mode 100644 index 0000000000..9bef73d6fb --- /dev/null +++ b/tests/core/cgo/asm_conditional_cgo/asm_cgo.go @@ -0,0 +1,16 @@ +//go:build amd64 || arm64 + +// build constraints must match the constraints in the tests/core/cgo/asm package + +package main + +/* +extern int example_asm_func(); +*/ +import "C" + +func callASM() (int, error) { + return int(C.example_asm_func()), nil +} + +const builtWithCgo = true diff --git a/tests/core/cgo/asm_conditional_cgo/asm_conditional_cgo_test.go b/tests/core/cgo/asm_conditional_cgo/asm_conditional_cgo_test.go new file mode 100644 index 0000000000..b76dac2b77 --- /dev/null +++ b/tests/core/cgo/asm_conditional_cgo/asm_conditional_cgo_test.go @@ -0,0 +1,22 @@ +package main + +import "testing" + +func TestConditionalCgo(t *testing.T) { + // Uses build constraints to depend on assembly code when cgo is available, or a + // pure go version if it is not. This can be run both with and without cgo. E.g.: + // CGO_ENABLED=0 go test . && CGO_ENABLED=1 go test . + result, err := callASM() + if builtWithCgo { + if err != nil { + t.Errorf("builtWithCgo=%t; err must be nil, was %s", builtWithCgo, err.Error()) + } else if result <= 0 { + t.Errorf("builtWithCgo=%t; result must be > 0 was %d", builtWithCgo, result) + } + } else { + // does not support calling the cgo library + if !(result == 0 && err != nil) { + t.Errorf("builtWithCgo=%t; expected result=0, err != nil; result=%d, err=%#v", builtWithCgo, result, err) + } + } +} diff --git a/tests/core/cgo/asm_conditional_cgo/asm_nocgo.go b/tests/core/cgo/asm_conditional_cgo/asm_nocgo.go new file mode 100644 index 0000000000..ef4d0c0bfd --- /dev/null +++ b/tests/core/cgo/asm_conditional_cgo/asm_nocgo.go @@ -0,0 +1,11 @@ +//go:build !(cgo && (amd64 || arm64)) + +package main + +import "errors" + +func callASM() (int, error) { + return 0, errors.New("cgo disabled and/or unsupported platform") +} + +const builtWithCgo = false diff --git a/tests/core/cgo/asm_dep_conditional_cgo/BUILD.bazel b/tests/core/cgo/asm_dep_conditional_cgo/BUILD.bazel new file mode 100644 index 0000000000..6467ad3e73 --- /dev/null +++ b/tests/core/cgo/asm_dep_conditional_cgo/BUILD.bazel @@ -0,0 +1,30 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "asm_dep_conditional_cgo", + srcs = [ + "asm_dep_cgo.go", + "asm_dep_nocgo.go", + ], + importpath = "github.com/bazelbuild/rules_go/tests/core/cgo/asm_dep_conditional_cgo", + deps = ["//tests/core/cgo/asm"], +) + +# this is a "native" target: it uses cgo and calls the assembly function as expected +go_test( + name = "asm_dep_conditional_cgo_test", + srcs = [ + "asm_dep_conditional_cgo_test.go", + ], + embed = [":asm_dep_conditional_cgo"], +) + +# this is a CGO_ENABLED=0 target: it does not import the cgo target +go_test( + name = "asm_dep_conditional_nocgo_test", + srcs = [ + "asm_dep_conditional_cgo_test.go", + ], + embed = [":asm_dep_conditional_cgo"], + pure = "on", +) diff --git a/tests/core/cgo/asm_dep_conditional_cgo/asm_dep_cgo.go b/tests/core/cgo/asm_dep_conditional_cgo/asm_dep_cgo.go new file mode 100644 index 0000000000..531cc7dcb3 --- /dev/null +++ b/tests/core/cgo/asm_dep_conditional_cgo/asm_dep_cgo.go @@ -0,0 +1,15 @@ +//go:build cgo && (amd64 || arm64) + +// build constraints must match the constraints in the tests/core/cgo/asm package + +package main + +import ( + "github.com/bazelbuild/rules_go/tests/core/cgo/asm" +) + +func callASMPackage() (int, error) { + return asm.CallAssembly(), nil +} + +const builtWithCgo = true diff --git a/tests/core/cgo/asm_dep_conditional_cgo/asm_dep_conditional_cgo_test.go b/tests/core/cgo/asm_dep_conditional_cgo/asm_dep_conditional_cgo_test.go new file mode 100644 index 0000000000..0f1f1521cb --- /dev/null +++ b/tests/core/cgo/asm_dep_conditional_cgo/asm_dep_conditional_cgo_test.go @@ -0,0 +1,22 @@ +package main + +import "testing" + +func TestConditionalCgo(t *testing.T) { + // Uses build constraints to depend on a native Cgo package when cgo is available, or a + // pure go version if it is not. This can be run both with and without cgo. E.g.: + // CGO_ENABLED=0 go test . && CGO_ENABLED=1 go test . + result, err := callASMPackage() + if builtWithCgo { + if err != nil { + t.Errorf("builtWithCgo=%t; err must be nil, was %s", builtWithCgo, err.Error()) + } else if result <= 0 { + t.Errorf("builtWithCgo=%t; result must be > 0 was %d", builtWithCgo, result) + } + } else { + // does not support calling the cgo library + if !(result == 0 && err != nil) { + t.Errorf("builtWithCgo=%t; expected result=0, err != nil; result=%d, err=%#v", builtWithCgo, result, err) + } + } +} diff --git a/tests/core/cgo/asm_dep_conditional_cgo/asm_dep_nocgo.go b/tests/core/cgo/asm_dep_conditional_cgo/asm_dep_nocgo.go new file mode 100644 index 0000000000..e8968dac18 --- /dev/null +++ b/tests/core/cgo/asm_dep_conditional_cgo/asm_dep_nocgo.go @@ -0,0 +1,13 @@ +//go:build !(cgo && (amd64 || arm64)) + +// build constraints must match the constraints in the tests/core/cgo/asm package + +package main + +import "errors" + +func callASMPackage() (int, error) { + return 0, errors.New("cgo disabled and/or unsupported platform") +} + +const builtWithCgo = false