Skip to content

Commit 2e821f6

Browse files
evanjevanj
authored
cgo packages with assembly: Support CGO_ENABLED=0 (#3661)
* cgo packages with assembly: Support CGO_ENABLED=0 Go supports building cgo packages both with and without Cgo. It uses build constraints to exclude the appropriate files. When building a Cgo package with assembly files, we must exclude them. Previous to this change, rules_go would run the Go assembler on them. This meant that packages which had "conditional" imports of cgo libraries with assembly would fail when compiled with cgo disabled. Add tests for these two cases: * asm_conditional_cgo: A Go package that compiles both with and without Cgo. * asm_dep_conditional_cgo: A Go package that conditionally imports a package with Cgo. Fixes: #3411 * code review improvements: remove unused main; clarify comment
1 parent f64211a commit 2e821f6

File tree

15 files changed

+224
-8
lines changed

15 files changed

+224
-8
lines changed

‎go/tools/builders/compilepkg.go

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -272,8 +272,13 @@ func compileArchive(
272272
for i, src := range srcs.hSrcs {
273273
hSrcs[i] = src.filename
274274
}
275+
276+
// haveCgo is true if the package contains Cgo files.
275277
haveCgo := len(cgoSrcs)+len(cSrcs)+len(cxxSrcs)+len(objcSrcs)+len(objcxxSrcs) > 0
276-
packageUsesCgo := cgoEnabled && haveCgo
278+
// compilingWithCgo is true if the package contains Cgo files AND Cgo is enabled. A package
279+
// containing Cgo files can also be built with Cgo disabled, and will work if there are build
280+
// constraints.
281+
compilingWithCgo := haveCgo && cgoEnabled
277282

278283
// Instrument source files for coverage.
279284
if coverMode != "" {
@@ -330,7 +335,7 @@ func compileArchive(
330335
// If we have cgo, generate separate C and go files, and compile the
331336
// C files.
332337
var objFiles []string
333-
if packageUsesCgo {
338+
if compilingWithCgo {
334339
// If the package uses Cgo, compile .s and .S files with cgo2, not the Go assembler.
335340
// Otherwise: the .s/.S files will be compiled with the Go assembler later
336341
var srcDir string
@@ -355,7 +360,7 @@ func compileArchive(
355360
if err != nil {
356361
return err
357362
}
358-
if packageUsesCgo {
363+
if compilingWithCgo {
359364
// cgo generated code imports some extra packages.
360365
imports["runtime/cgo"] = nil
361366
imports["syscall"] = nil
@@ -465,7 +470,7 @@ func compileArchive(
465470
asmHdrPath = filepath.Join(workDir, "go_asm.h")
466471
}
467472
var symabisPath string
468-
if !packageUsesCgo {
473+
if !haveCgo {
469474
symabisPath, err = buildSymabisFile(goenv, srcs.sSrcs, srcs.hSrcs, asmHdrPath)
470475
if symabisPath != "" {
471476
if !goenv.shouldPreserveWorkDir {
@@ -482,8 +487,9 @@ func compileArchive(
482487
return err
483488
}
484489

485-
// Compile the .s files if we are not a cgo package; cgo is assembled by cc above
486-
if len(srcs.sSrcs) > 0 && !packageUsesCgo {
490+
// Compile the .s files with Go's assembler, if this is not a cgo package.
491+
// Cgo is assembled by cc above.
492+
if len(srcs.sSrcs) > 0 && !haveCgo {
487493
includeSet := map[string]struct{}{
488494
filepath.Join(os.Getenv("GOROOT"), "pkg", "include"): {},
489495
workDir: {},

‎go/tools/builders/filter.go

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,15 @@ type archiveSrcs struct {
6969
// them by extension.
7070
func filterAndSplitFiles(fileNames []string) (archiveSrcs, error) {
7171
var res archiveSrcs
72+
packageContainsCgo := false
7273
for _, s := range fileNames {
7374
src, err := readFileInfo(build.Default, s)
7475
if err != nil {
7576
return archiveSrcs{}, err
7677
}
78+
if src.isCgo {
79+
packageContainsCgo = true
80+
}
7781
if !src.matched {
7882
continue
7983
}
@@ -96,6 +100,12 @@ func filterAndSplitFiles(fileNames []string) (archiveSrcs, error) {
96100
}
97101
*srcs = append(*srcs, src)
98102
}
103+
if packageContainsCgo && !build.Default.CgoEnabled {
104+
// Cgo packages use the C compiler for asm files, rather than Go's assembler.
105+
// This is a package with cgo files, but we are compiling with Cgo disabled:
106+
// Remove the assembly files.
107+
res.sSrcs = nil
108+
}
99109
return res, nil
100110
}
101111

‎tests/core/cgo/asm/BUILD.bazel

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ go_library(
99
],
1010
cgo = True,
1111
importpath = "github.com/bazelbuild/rules_go/tests/core/cgo/asm",
12+
visibility = ["//tests/core/cgo:__subpackages__"],
1213
)
1314

1415
go_test(

‎tests/core/cgo/asm/cgoasm.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,6 @@ extern int example_asm_func();
77
*/
88
import "C"
99

10-
func callAssembly() int {
10+
func CallAssembly() int {
1111
return int(C.example_asm_func())
1212
}

‎tests/core/cgo/asm/cgoasm_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ func TestNativeAssembly(t *testing.T) {
1616
if expected == 0 {
1717
t.Fatalf("expected=0 for GOARCH=%s; missing value?", runtime.GOARCH)
1818
}
19-
actual := callAssembly()
19+
actual := CallAssembly()
2020
if actual != expected {
2121
t.Errorf("callAssembly()=%d; expected=%d", actual, expected)
2222
}

‎tests/core/cgo/asm_conditional_cgo/BUILD.bazel

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
2+
3+
go_library(
4+
name = "asm_conditional_cgo",
5+
srcs = [
6+
"asm_amd64.S",
7+
"asm_arm64.S",
8+
"asm_cgo.go",
9+
"asm_nocgo.go",
10+
],
11+
cgo = True,
12+
importpath = "github.com/bazelbuild/rules_go/tests/core/cgo/asm_conditional_cgo",
13+
deps = ["//tests/core/cgo/asm"],
14+
)
15+
16+
# this is a "native" target: it uses cgo and calls the assembly function as expected
17+
go_test(
18+
name = "asm_conditional_cgo_test",
19+
srcs = [
20+
"asm_conditional_cgo_test.go",
21+
],
22+
embed = [":asm_conditional_cgo"],
23+
)
24+
25+
# this is a CGO_ENABLED=0 target: it does not import the cgo target
26+
go_test(
27+
name = "asm_conditional_nocgo_test",
28+
srcs = [
29+
"asm_conditional_cgo_test.go",
30+
],
31+
embed = [":asm_conditional_cgo"],
32+
pure = "on",
33+
)

‎tests/core/cgo/asm_conditional_cgo/asm_amd64.S

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
/*
2+
https://stackoverflow.com/questions/73435637/how-can-i-fix-usr-bin-ld-warning-trap-o-missing-note-gnu-stack-section-imp
3+
*/
4+
#if defined(__ELF__) && defined(__GNUC__)
5+
.section .note.GNU-stack,"",@progbits
6+
#endif
7+
8+
/*
9+
define this symbol both with and without underscore.
10+
It seems like Mac OS X wants the underscore, but Linux does not.
11+
*/
12+
.global example_asm_func
13+
.global _example_asm_func
14+
.text
15+
example_asm_func:
16+
_example_asm_func:
17+
mov $42, %rax
18+
ret
19+
20+
#if NOT_DEFINED
21+
#error "should not fail"
22+
#endif

‎tests/core/cgo/asm_conditional_cgo/asm_arm64.S

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
/*
2+
define this symbol both with and without underscore.
3+
It seems like Mac OS X wants the underscore, but Linux does not.
4+
*/
5+
.global example_asm_func
6+
.global _example_asm_func
7+
.p2align 2 /* ld: warning: arm64 function not 4-byte aligned */
8+
example_asm_func:
9+
_example_asm_func:
10+
mov x0, #44
11+
ret
12+
13+
#if NOT_DEFINED
14+
#error "should not fail"
15+
#endif

‎tests/core/cgo/asm_conditional_cgo/asm_cgo.go

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
//go:build amd64 || arm64
2+
3+
// build constraints must match the constraints in the tests/core/cgo/asm package
4+
5+
package main
6+
7+
/*
8+
extern int example_asm_func();
9+
*/
10+
import "C"
11+
12+
func callASM() (int, error) {
13+
return int(C.example_asm_func()), nil
14+
}
15+
16+
const builtWithCgo = true

‎tests/core/cgo/asm_conditional_cgo/asm_conditional_cgo_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
package main
2+
3+
import "testing"
4+
5+
func TestConditionalCgo(t *testing.T) {
6+
// Uses build constraints to depend on assembly code when cgo is available, or a
7+
// pure go version if it is not. This can be run both with and without cgo. E.g.:
8+
// CGO_ENABLED=0 go test . && CGO_ENABLED=1 go test .
9+
result, err := callASM()
10+
if builtWithCgo {
11+
if err != nil {
12+
t.Errorf("builtWithCgo=%t; err must be nil, was %s", builtWithCgo, err.Error())
13+
} else if result <= 0 {
14+
t.Errorf("builtWithCgo=%t; result must be > 0 was %d", builtWithCgo, result)
15+
}
16+
} else {
17+
// does not support calling the cgo library
18+
if !(result == 0 && err != nil) {
19+
t.Errorf("builtWithCgo=%t; expected result=0, err != nil; result=%d, err=%#v", builtWithCgo, result, err)
20+
}
21+
}
22+
}

‎tests/core/cgo/asm_conditional_cgo/asm_nocgo.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
//go:build !(cgo && (amd64 || arm64))
2+
3+
package main
4+
5+
import "errors"
6+
7+
func callASM() (int, error) {
8+
return 0, errors.New("cgo disabled and/or unsupported platform")
9+
}
10+
11+
const builtWithCgo = false

‎tests/core/cgo/asm_dep_conditional_cgo/BUILD.bazel

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
2+
3+
go_library(
4+
name = "asm_dep_conditional_cgo",
5+
srcs = [
6+
"asm_dep_cgo.go",
7+
"asm_dep_nocgo.go",
8+
],
9+
importpath = "github.com/bazelbuild/rules_go/tests/core/cgo/asm_dep_conditional_cgo",
10+
deps = ["//tests/core/cgo/asm"],
11+
)
12+
13+
# this is a "native" target: it uses cgo and calls the assembly function as expected
14+
go_test(
15+
name = "asm_dep_conditional_cgo_test",
16+
srcs = [
17+
"asm_dep_conditional_cgo_test.go",
18+
],
19+
embed = [":asm_dep_conditional_cgo"],
20+
)
21+
22+
# this is a CGO_ENABLED=0 target: it does not import the cgo target
23+
go_test(
24+
name = "asm_dep_conditional_nocgo_test",
25+
srcs = [
26+
"asm_dep_conditional_cgo_test.go",
27+
],
28+
embed = [":asm_dep_conditional_cgo"],
29+
pure = "on",
30+
)

‎tests/core/cgo/asm_dep_conditional_cgo/asm_dep_cgo.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
//go:build cgo && (amd64 || arm64)
2+
3+
// build constraints must match the constraints in the tests/core/cgo/asm package
4+
5+
package main
6+
7+
import (
8+
"github.com/bazelbuild/rules_go/tests/core/cgo/asm"
9+
)
10+
11+
func callASMPackage() (int, error) {
12+
return asm.CallAssembly(), nil
13+
}
14+
15+
const builtWithCgo = true

‎tests/core/cgo/asm_dep_conditional_cgo/asm_dep_conditional_cgo_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
package main
2+
3+
import "testing"
4+
5+
func TestConditionalCgo(t *testing.T) {
6+
// Uses build constraints to depend on a native Cgo package when cgo is available, or a
7+
// pure go version if it is not. This can be run both with and without cgo. E.g.:
8+
// CGO_ENABLED=0 go test . && CGO_ENABLED=1 go test .
9+
result, err := callASMPackage()
10+
if builtWithCgo {
11+
if err != nil {
12+
t.Errorf("builtWithCgo=%t; err must be nil, was %s", builtWithCgo, err.Error())
13+
} else if result <= 0 {
14+
t.Errorf("builtWithCgo=%t; result must be > 0 was %d", builtWithCgo, result)
15+
}
16+
} else {
17+
// does not support calling the cgo library
18+
if !(result == 0 && err != nil) {
19+
t.Errorf("builtWithCgo=%t; expected result=0, err != nil; result=%d, err=%#v", builtWithCgo, result, err)
20+
}
21+
}
22+
}

‎tests/core/cgo/asm_dep_conditional_cgo/asm_dep_nocgo.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
//go:build !(cgo && (amd64 || arm64))
2+
3+
// build constraints must match the constraints in the tests/core/cgo/asm package
4+
5+
package main
6+
7+
import "errors"
8+
9+
func callASMPackage() (int, error) {
10+
return 0, errors.New("cgo disabled and/or unsupported platform")
11+
}
12+
13+
const builtWithCgo = false

0 commit comments

Comments
 (0)