From 9ce21b527637eaef00a220907d15b5ac592e0e9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Motiejus=20Jak=C5=A1tys?= Date: Thu, 5 May 2022 13:36:58 +0300 Subject: [PATCH] [zig ld] --gc-sections workaround + tests `zig cc` emits `--gc-sections` for the linker, which is incompatbile with what CGo thinks about linking. This commit adds a workaround: it will add `--no-gc-sections` to the linking step if the command is not specified (falling back to the default behavior of gcc/clang). Related: https://github.com/golang/go/issues/52690 --- README.md | 44 ++++++++++++++++++++-- test/{go => cgo}/BUILD | 30 +++++++-------- test/{go/hello.go => cgo/cgo.go} | 0 test/{go/hello_test.go => cgo/cgo_test.go} | 0 test/gorace/BUILD | 24 ++++++++++++ test/gorace/main.go | 13 +++++++ test/gorace/main_test.go | 8 ++++ toolchain/defs.bzl | 28 +++++++++++++- 8 files changed, 128 insertions(+), 19 deletions(-) rename test/{go => cgo}/BUILD (76%) rename test/{go/hello.go => cgo/cgo.go} (100%) rename test/{go/hello_test.go => cgo/cgo_test.go} (100%) create mode 100644 test/gorace/BUILD create mode 100644 test/gorace/main.go create mode 100644 test/gorace/main_test.go diff --git a/README.md b/README.md index a93c07f..c9f52d6 100644 --- a/README.md +++ b/README.md @@ -270,7 +270,12 @@ used, run: $ bazel query @zig_sdk//toolchain/... ``` -# Note: UBSAN and "SIGILL: Illegal Instruction" +# Incompatibilities with clang and gcc + +`zig cc` is *almost* a drop-in replacement for clang/gcc. This section lists +some of the discovered differences and ways to live with them. + +## UBSAN and "SIGILL: Illegal Instruction" `zig cc` differs from "mainstream" compilers by [enabling UBSAN by default][ubsan1]. Which means your program may compile successfully and crash @@ -280,9 +285,14 @@ with: SIGILL: illegal instruction ``` -This is by design: it encourages program authors to fix the undefined behavior. -There are [many ways][ubsan2] to find the undefined behavior. +This flag encourages program authors to fix the undefined behavior. There are +[many ways][ubsan2] to find the undefined behavior. +## Use of `--gc-sections` by default + +`zig cc` passes `--gc-sections` to the ld.lld linker by default, this causes +problems for CGo. See +[below][#go-linker-does-not-put-libc-onto-the-linker-line]. # Known Issues In bazel-zig-cc @@ -313,6 +323,34 @@ currently targets the lowest version, without ability to change it. This section lists issues that I've stumbled into when using `zig cc`, and is outside of bazel-zig-cc's control. +## Go linker does not put libc onto the linker line + +**Severity: Low** + +Task: [golang/go #52690 Go linker does not put libc onto the linker line, causing undefined symbol errors](https://github.com/golang/go/issues/52690) + +Background: when linking CGo programs that do not have C code by itself, +the Golang linker does not link the C library, causing undefined symbols and +a message similar to this: + +``` +runtime/race(.text): relocation target getuid not defined +runtime/race(.text): relocation target pthread_self not defined +``` + +This is because `zig cc` emits `--gc-sections` for the linker, which is +incompatbile with what CGo thinks about linking. + +A workaround until [#52690](https://github.com/golang/go/issues/52690) is +resolved: add `--no-gc-sections` to the link step. So the resulting command to +compile CGo code on Linux is: + +``` +CGO_ENABLED=1 CC="zig cc -Wl,--no-gc-sections" go build main.go +``` + +This is done automatically in bazel-zig-cc. + ## using glibc 2.27 or older **Severity: Low** diff --git a/test/go/BUILD b/test/cgo/BUILD similarity index 76% rename from test/go/BUILD rename to test/cgo/BUILD index 6c40c2e..1e11a22 100644 --- a/test/go/BUILD +++ b/test/cgo/BUILD @@ -3,29 +3,29 @@ load("//rules:rules_go.bzl", "go_binary") load("@bazel-zig-cc//rules:platform.bzl", "platform_binary", "platform_test") go_library( - name = "go_lib", - srcs = ["hello.go"], + name = "cgo_lib", + srcs = ["cgo.go"], cgo = True, - importpath = "git.sr.ht/~motiejus/bazel-zig-cc/test/go", + importpath = "git.sr.ht/~motiejus/bazel-zig-cc/test/cgo", visibility = ["//visibility:private"], ) -go_binary( - name = "go", - embed = [":go_lib"], - visibility = ["//visibility:public"], +go_test( + name = "cgo_test", + srcs = ["cgo_test.go"], + embed = [":cgo_lib"], ) -go_test( - name = "go_test", - srcs = ["hello_test.go"], - embed = [":go_lib"], +go_binary( + name = "cgo", + embed = [":cgo_lib"], + visibility = ["//visibility:public"], ) [ platform_binary( - name = "go_{}".format(name), - src = "go", + name = "cgo_{}".format(name), + src = "cgo", platform = platform, ) for name, platform in [ @@ -39,8 +39,8 @@ go_test( [ platform_test( - name = "go_test_{}".format(name), - src = "go_test", + name = "cgo_test_{}".format(name), + src = "cgo_test", platform = platform, ) for name, platform in [ diff --git a/test/go/hello.go b/test/cgo/cgo.go similarity index 100% rename from test/go/hello.go rename to test/cgo/cgo.go diff --git a/test/go/hello_test.go b/test/cgo/cgo_test.go similarity index 100% rename from test/go/hello_test.go rename to test/cgo/cgo_test.go diff --git a/test/gorace/BUILD b/test/gorace/BUILD new file mode 100644 index 0000000..c12112a --- /dev/null +++ b/test/gorace/BUILD @@ -0,0 +1,24 @@ +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") +load("//rules:rules_go.bzl", "go_binary") + +go_library( + name = "gorace_lib", + srcs = ["main.go"], + # keep + cgo = True, + importpath = "git.sr.ht/~motiejus/bazel-zig-cc/test/gorace", + visibility = ["//visibility:private"], +) + +go_binary( + name = "gorace", + embed = [":gorace_lib"], + visibility = ["//visibility:public"], +) + +go_test( + name = "gorace_test", + srcs = ["main_test.go"], + embed = [":gorace_lib"], + race = "on", +) diff --git a/test/gorace/main.go b/test/gorace/main.go new file mode 100644 index 0000000..dd52075 --- /dev/null +++ b/test/gorace/main.go @@ -0,0 +1,13 @@ +// Package main tests that Zig can compile race-enabled tests. +// +// As of writing, this fails: +// CGO_ENABLED=1 CC="zig cc" go test -race . +// +// More context: https://github.com/ziglang/zig/issues/11398 +// +// This fails, because `zig cc` adds `--gc-sections` to the linker +// flag by default, which is incompatible with cgo. bazel-zig-cc +// adds a workaround for it. +package main + +func main() {} diff --git a/test/gorace/main_test.go b/test/gorace/main_test.go new file mode 100644 index 0000000..2ad7482 --- /dev/null +++ b/test/gorace/main_test.go @@ -0,0 +1,8 @@ +package main + +import "testing" + +func TestFoo(t *testing.T) { + _ = t + main() +} diff --git a/toolchain/defs.bzl b/toolchain/defs.bzl index 696931a..3a65eeb 100644 --- a/toolchain/defs.bzl +++ b/toolchain/defs.bzl @@ -78,7 +78,33 @@ fi export ZIG_LOCAL_CACHE_DIR="$_cache_prefix/bazel-zig-cc" export ZIG_GLOBAL_CACHE_DIR=$ZIG_LOCAL_CACHE_DIR -exec "{zig}" "{zig_tool}" "$@" +lookup_args=() +add_arg= +case "{zig_tool}" in + cc|c++) + lookup_args=("-Wl,--gc-sections" "-Wl,--no-gc-sections") + add_arg="-Wl,--no-gc-sections" + ;; + ld.lld) + lookup_args=("--gc-sections" "--no-gc-sections") + add_arg="--no-gc-sections" + ;; +esac + +has_special_arg=0 +for arg in "${{lookup_args[@]}}"; do + if [[ " ${{args[*]}} " == *" $arg "* ]]; then + has_special_arg=1 + break + fi +done + +args=( "$@" ) +if [[ "$has_special_arg" == 0 ]]; then + args+=( "$add_arg" ) +fi + +exec "{zig}" "{zig_tool}" "${{args[@]}}" """ _ZIG_TOOLS = [