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

race build does not respect race build tag #1559

Closed
dvyukov opened this issue Jun 19, 2018 · 4 comments
Closed

race build does not respect race build tag #1559

dvyukov opened this issue Jun 19, 2018 · 4 comments
Labels

Comments

@dvyukov
Copy link

dvyukov commented Jun 19, 2018

I am trying to build gvisor in race more:

git clone https://gvisor.googlesource.com/gvisor
cd gvisor
bazel build --features=race runsc

This will also require disabling pure mode for the binary, as it silently disables race more:

diff --git a/runsc/BUILD b/runsc/BUILD
index 2f0bbaf..c2e7510 100644
--- a/runsc/BUILD
+++ b/runsc/BUILD
@@ -7,7 +7,7 @@ go_binary(
     srcs = [
         "main.go",
     ],
-    pure = "on",
+    #pure = "on",
     visibility = [
         "//runsc:__subpackages__",
     ],

runsc/boot/filter/extra_filters_race.go file contains race build tag // +build race, but if I do:

diff --git a/runsc/boot/filter/extra_filters_race.go b/runsc/boot/filter/extra_filters_race.go
index c810773..b2599f1 100644
--- a/runsc/boot/filter/extra_filters_race.go
+++ b/runsc/boot/filter/extra_filters_race.go
@@ -22,6 +22,8 @@ import (
        "gvisor.googlesource.com/gvisor/pkg/seccomp"
 )
 
+this won't compile
+
 // instrumentationFilters returns additional filters for syscalls used by TSAN.
 func instrumentationFilters() seccomp.SyscallRules {
        Report("TSAN is enabled: syscall filters less restrictive!")

The build still passes. Which suggests that --features=race does not respect race tag.

Both --features=race and race = "on" binary attribute need to respect race build tag.

While we are here, it can make sense to do the same for msan and --features=msan, there is a similar file runsc/boot/filter/extra_filters_msan.go.

@dvyukov
Copy link
Author

dvyukov commented Jun 19, 2018

@amscanne @prattmic

jayconrod pushed a commit to jayconrod/rules_go that referenced this issue Jun 19, 2018
Also: clean up the race tests, move to tests/core, and test that the
race tag is set.

Fixes bazel-contrib#1559
@jayconrod
Copy link
Contributor

Thanks for reporting. This was overlooked functionality. #1562 will enable race and msan tags in those modes.

jayconrod added a commit that referenced this issue Jun 19, 2018
Also: clean up the race tests, move to tests/core, and test that the
race tag is set.

Fixes #1559
ArielleA pushed a commit to ArielleA/rules_go that referenced this issue Jun 19, 2018
…b#1562)

Also: clean up the race tests, move to tests/core, and test that the
race tag is set.

Fixes bazel-contrib#1559
@dvyukov
Copy link
Author

dvyukov commented Jun 20, 2018

After pulling this change into gvisor I am getting:

$ bazel build //runsc:runsc-race
INFO: Analysed target //runsc:runsc-race (0 packages loaded).
INFO: Found 1 target...
ERROR: /usr/local/google/home/dvyukov/.cache/bazel/_bazel_dvyukov/42571c499bb753a6e5c4fdf61e6007b2/external/org_golang_x_sys/unix/BUILD.bazel:3:1: GoCompile external/org_golang_x_sys/unix/linux_amd64_static_race_stripped/go_default_library~/golang.org/x/sys/unix.a~partial.a failed (Exit 1)
/usr/local/google/home/dvyukov/.cache/bazel/_bazel_dvyukov/42571c499bb753a6e5c4fdf61e6007b2/sandbox/linux-sandbox/4/execroot/__main__/bazel-out/k8-fastbuild/bin/external/org_golang_x_sys/unix/linux_amd64_stripped/go_default_library.cgo_codegen~/syscall_unix.cgo1.go:115:5: undefined: raceenabled

golang.org/x/sys has 2 files which define raceenabled :

$ head -n 10 /gopath/src/golang.org/x/sys/unix/race0.go 
// +build darwin,!race linux,!race freebsd,!race netbsd openbsd solaris dragonfly

$ head -n 10 /gopath/src/golang.org/x/sys/unix/race.go 
// +build darwin,race linux,race freebsd,race

Is it that bazel is somehow confused by these complex expressions in build tags?

@jayconrod
Copy link
Contributor

Bazel is doing the right thing. The problem is that Gazelle is not including race.go in the srcs attribute for that rule because it doesn't recognize the race tag. I've filed bazel-contrib/bazel-gazelle#233 to track this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants