Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compilepkg: cgo assembly uses the C compiler #3648

Merged
merged 6 commits into from
Aug 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 20 additions & 16 deletions go/tools/builders/compilepkg.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 != "" {
Expand Down Expand Up @@ -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
}
Expand All @@ -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
Expand Down Expand Up @@ -458,28 +458,32 @@ 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.
if err := compileGo(goenv, goSrcs, packagePath, importcfgPath, embedcfgPath, asmHdrPath, symabisPath, gcFlags, pgoprofile, outPath); err != nil {
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: {},
Expand Down
20 changes: 20 additions & 0 deletions tests/core/cgo/asm/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -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"],
)
22 changes: 22 additions & 0 deletions tests/core/cgo/asm/asm_amd64.S
Original file line number Diff line number Diff line change
@@ -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
15 changes: 15 additions & 0 deletions tests/core/cgo/asm/asm_arm64.S
Original file line number Diff line number Diff line change
@@ -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
12 changes: 12 additions & 0 deletions tests/core/cgo/asm/cgoasm.go
Original file line number Diff line number Diff line change
@@ -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())
}
23 changes: 23 additions & 0 deletions tests/core/cgo/asm/cgoasm_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
//go:build amd64 || arm64

package asm
fmeum marked this conversation as resolved.
Show resolved Hide resolved

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)
}
}