diff --git a/go/tools/builders/compilepkg.go b/go/tools/builders/compilepkg.go index 9c6202bb5c..4c7779c697 100644 --- a/go/tools/builders/compilepkg.go +++ b/go/tools/builders/compilepkg.go @@ -273,6 +273,7 @@ func compileArchive( hSrcs[i] = src.filename } haveCgo := len(cgoSrcs)+len(cSrcs)+len(cxxSrcs)+len(objcSrcs)+len(objcxxSrcs) > 0 + packageUsesCgo := cgoEnabled && haveCgo // Instrument source files for coverage. if coverMode != "" { @@ -329,12 +330,11 @@ func compileArchive( // If we have cgo, generate separate C and go files, and compile the // C files. var objFiles []string - if cgoEnabled && haveCgo { - // TODO(#2006): Compile .s and .S files with cgo2, not the Go assembler. - // If cgo is not enabled or we don't have other cgo sources, don't - // compile .S files. + if packageUsesCgo { + // 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 - srcDir, goSrcs, objFiles, err = cgo2(goenv, goSrcs, cgoSrcs, cSrcs, cxxSrcs, objcSrcs, objcxxSrcs, nil, hSrcs, packagePath, packageName, cc, cppFlags, cFlags, cxxFlags, objcFlags, objcxxFlags, ldFlags, cgoExportHPath) + srcDir, goSrcs, objFiles, err = cgo2(goenv, goSrcs, cgoSrcs, cSrcs, cxxSrcs, objcSrcs, objcxxSrcs, sSrcs, hSrcs, packagePath, packageName, cc, cppFlags, cFlags, cxxFlags, objcFlags, objcxxFlags, ldFlags, cgoExportHPath) if err != nil { return err } @@ -355,7 +355,7 @@ func compileArchive( if err != nil { return err } - if cgoEnabled && len(cgoSrcs) != 0 { + if packageUsesCgo { // cgo generated code imports some extra packages. imports["runtime/cgo"] = nil imports["syscall"] = nil @@ -458,19 +458,23 @@ func compileArchive( }() } - // If there are assembly files, and this is go1.12+, generate symbol ABIs. + // If there are Go assembly files and this is go1.12+: generate symbol ABIs. + // This excludes Cgo packages: they use the C compiler for assembly. asmHdrPath := "" if len(srcs.sSrcs) > 0 { asmHdrPath = filepath.Join(workDir, "go_asm.h") } - symabisPath, err := buildSymabisFile(goenv, srcs.sSrcs, srcs.hSrcs, asmHdrPath) - if symabisPath != "" { - if !goenv.shouldPreserveWorkDir { - defer os.Remove(symabisPath) + var symabisPath string + if !packageUsesCgo { + symabisPath, err = buildSymabisFile(goenv, srcs.sSrcs, srcs.hSrcs, asmHdrPath) + if symabisPath != "" { + if !goenv.shouldPreserveWorkDir { + defer os.Remove(symabisPath) + } + } + if err != nil { + return err } - } - if err != nil { - return err } // Compile the filtered .go files. @@ -478,8 +482,8 @@ func compileArchive( return err } - // Compile the .s files. - if len(srcs.sSrcs) > 0 { + // Compile the .s files if we are not a cgo package; cgo is assembled by cc above + if len(srcs.sSrcs) > 0 && !packageUsesCgo { includeSet := map[string]struct{}{ filepath.Join(os.Getenv("GOROOT"), "pkg", "include"): {}, workDir: {}, diff --git a/tests/core/cgo/asm/BUILD.bazel b/tests/core/cgo/asm/BUILD.bazel new file mode 100644 index 0000000000..98e618a01e --- /dev/null +++ b/tests/core/cgo/asm/BUILD.bazel @@ -0,0 +1,20 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") + +go_library( + name = "asm", + srcs = [ + "asm_amd64.S", + "asm_arm64.S", + "cgoasm.go", + ], + cgo = True, + importpath = "github.com/bazelbuild/rules_go/tests/core/cgo/asm", +) + +go_test( + name = "asm_test", + srcs = [ + "cgoasm_test.go", + ], + embed = [":asm"], +) diff --git a/tests/core/cgo/asm/asm_amd64.S b/tests/core/cgo/asm/asm_amd64.S new file mode 100644 index 0000000000..97c5d1a36d --- /dev/null +++ b/tests/core/cgo/asm/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/asm_arm64.S b/tests/core/cgo/asm/asm_arm64.S new file mode 100644 index 0000000000..0585bf75db --- /dev/null +++ b/tests/core/cgo/asm/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/cgoasm.go b/tests/core/cgo/asm/cgoasm.go new file mode 100644 index 0000000000..f0a3347493 --- /dev/null +++ b/tests/core/cgo/asm/cgoasm.go @@ -0,0 +1,12 @@ +//go:build amd64 || arm64 + +package asm + +/* +extern int example_asm_func(); +*/ +import "C" + +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 new file mode 100644 index 0000000000..9227ec0ea9 --- /dev/null +++ b/tests/core/cgo/asm/cgoasm_test.go @@ -0,0 +1,23 @@ +//go:build amd64 || arm64 + +package asm + +import ( + "runtime" + "testing" +) + +func TestNativeAssembly(t *testing.T) { + expectedGOARCH := map[string]int{ + "amd64": 42, + "arm64": 44, + } + expected := expectedGOARCH[runtime.GOARCH] + if expected == 0 { + t.Fatalf("expected=0 for GOARCH=%s; missing value?", runtime.GOARCH) + } + actual := callAssembly() + if actual != expected { + t.Errorf("callAssembly()=%d; expected=%d", actual, expected) + } +}