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

runtime/race: adding support for the race detector on other platforms #19273

Open
owenwaller opened this issue Feb 24, 2017 · 20 comments

Comments

@owenwaller
Copy link

commented Feb 24, 2017

Hi Everyone,

Following on from this discussion on the golang-nuts list, about supporting the race detector on ARM hardware, I'm opening this issue to track the suggestion.

The ThreadSanitizer that the Go race detector is based on supports x86_64, aarch64 (i.e.Arm8 64-bit), mips64 and powerpc64. Currently Go only supports the race detector on x86_64 hardware.

Would it be possible for a future release of Go to support the race detector on the other platforms - aarch64, mips64 and powerpc64.

Thanks

Owen

@ianlancetaylor ianlancetaylor changed the title Adding support for the race detector on other platforms runtime/race: adding support for the race detector on other platforms Feb 24, 2017

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Feb 24, 2017

@bradfitz bradfitz added this to the Go1.9Maybe milestone Feb 24, 2017

@bradfitz bradfitz modified the milestones: Go1.10, Go1.9Maybe Jun 7, 2017

@bradfitz

This comment has been minimized.

Copy link
Member

commented Jun 7, 2017

No objections, if somebody does the work.

@krytarowski

This comment has been minimized.

Copy link
Contributor

commented Dec 5, 2017

I've added NetBSD/amd64 support in LLVM TSan. This could be integrated with golang CI infrastructure.

https://blog.netbsd.org/tnf/entry/the_llvm_thread_sanitizer_has

@bradfitz

This comment has been minimized.

Copy link
Member

commented Dec 5, 2017

Note that we check in binary blobs to the Go repo:

bradfitz@gdev:~/go$ find . | grep syso$
./src/runtime/race/race_linux_amd64.syso
./src/runtime/race/race_darwin_amd64.syso
./src/runtime/race/race_windows_amd64.syso
./src/runtime/race/race_freebsd_amd64.syso

They are built using:

https://github.com/golang/build/tree/master/cmd/racebuild

@krytarowski

This comment has been minimized.

Copy link
Contributor

commented Dec 5, 2017

I've repeated the steps for building race_netbsd_amd64.syso.

$ file ./src/runtime/race/race_netbsd_amd64.syso
./src/runtime/race/race_netbsd_amd64.syso: ELF 64-bit LSB relocatable, x86-64, version 1 (SYSV), not stripped

My golang patch:

diff --git a/src/cmd/go/internal/work/init.go b/src/cmd/go/internal/work/init.go
index 7f894f5c6d..1df9963521 100644
--- a/src/cmd/go/internal/work/init.go
+++ b/src/cmd/go/internal/work/init.go
@@ -43,8 +43,8 @@ func instrumentInit() {
 		fmt.Fprintf(os.Stderr, "-msan is not supported on %s/%s\n", cfg.Goos, cfg.Goarch)
 		os.Exit(2)
 	}
-	if cfg.Goarch != "amd64" || cfg.Goos != "linux" && cfg.Goos != "freebsd" && cfg.Goos != "darwin" && cfg.Goos != "windows" {
-		fmt.Fprintf(os.Stderr, "go %s: -race and -msan are only supported on linux/amd64, freebsd/amd64, darwin/amd64 and windows/amd64\n", flag.Args()[0])
+	if cfg.Goarch != "amd64" || cfg.Goos != "linux" && cfg.Goos != "freebsd" && cfg.Goos != "netbsd" && cfg.Goos != "darwin" && cfg.Goos != "windows" {
+		fmt.Fprintf(os.Stderr, "go %s: -race and -msan are only supported on linux/amd64, freebsd/amd64, netbsd/amd64, darwin/amd64 and windows/amd64\n", flag.Args()[0])
 		os.Exit(2)
 	}
 
diff --git a/src/race.bash b/src/race.bash
index adf2297c2f..84b8df24c7 100755
--- a/src/race.bash
+++ b/src/race.bash
@@ -9,7 +9,7 @@
 set -e
 
 function usage {
-	echo 'race detector is only supported on linux/amd64, freebsd/amd64 and darwin/amd64' 1>&2
+	echo 'race detector is only supported on linux/amd64, freebsd/amd64, netbsd/amd64 and darwin/amd64' 1>&2
 	exit 1
 }
 
@@ -30,6 +30,11 @@ case $(uname) in
 		usage
 	fi
 	;;
+"NetBSD")
+	if [ $(uname -m) != "amd64" ]; then
+		usage
+	fi
+	;;
 *)
 	usage
 	;;

But I'm getting the following error when attempting to use it:

##### Testing packages.
# compress/bzip2 (testmain)
runtime/race(.text): relocation target malloc not defined
runtime/race(.text): relocation target malloc not defined
runtime/race(.text): relocation target free not defined
runtime/race(.text): relocation target free not defined
runtime/race(.text): relocation target malloc not defined
runtime/race(.text): relocation target malloc not defined
runtime/race(.text): relocation target malloc not defined
runtime/race(.text): relocation target malloc not defined
runtime/race(.text): relocation target malloc not defined
runtime/race(.text): relocation target malloc not defined
runtime/race(.text): relocation target malloc not defined
runtime/race(.text): relocation target malloc not defined
runtime/race(.text): relocation target environ not defined
runtime/race(.text): relocation target malloc not defined
runtime/race(.text): relocation target setrlimit not defined
runtime/race(.text): relocation target syscall not defined
runtime/race(.text): relocation target _lwp_self not defined
runtime/race(.text): relocation target syscall not defined
runtime/race(.text): relocation target __errno not defined
runtime/race(.text): relocation target exit not defined
runtime/race(.text): relocation target __libc_thr_yield not defined
/tmp/test/go/pkg/tool/netbsd_amd64/link: too many errors

Investigating.

@krytarowski

This comment has been minimized.

Copy link
Contributor

commented Dec 5, 2017

@bradfitz other .syso files seem to have similar symbols undefined. Are there suggestions how to address it?

@bradfitz

This comment has been minimized.

Copy link
Member

commented Dec 5, 2017

I have no clue. I've never built these binaries before, nor have I worked on the race detector.

Maybe @ianlancetaylor or @dvyukov know.

@ianlancetaylor

This comment has been minimized.

Copy link
Contributor

commented Dec 5, 2017

Most likely you need to edit the build constraints in the flie runtime/race/race.go so that your system is recognized.

@krytarowski

This comment has been minimized.

Copy link
Contributor

commented Dec 5, 2017

This has improved the situation. I'm now down to unrecognized __ps_strings.

runtime/race(.text): relocation target __ps_strings not defined
runtime/race(.text): relocation target __ps_strings not defined
runtime/race(.text): relocation target __ps_strings not defined
runtime/race(.text): undefined: "__ps_strings"
runtime/race(.text): undefined: "__ps_strings"
runtime/race(.text): undefined: "__ps_strings"

I'm researching how to fix it.

@krytarowski

This comment has been minimized.

Copy link
Contributor

commented Dec 5, 2017

http://www.netbsd.org/~kamil/llvm/go-tsan-2017-12-06.txt

Passed 340 of 347 tests (97.98%, 0+, 7-)
0 expected failures (0 has not fail)
@krytarowski

This comment has been minimized.

Copy link
Contributor

commented Dec 5, 2017

http://www.netbsd.org/~kamil/llvm/0001-Add-initial-TSan-NetBSD-support.patch

@bradfitz feel free to merge this with HEAD. (my CLA is signed for Google projects)

My .syso file: http://www.netbsd.org/~kamil/llvm/race_netbsd_amd64.syso

Feel free to regenerate it from scratch or take this one as is and commit to the golang repository.

I cannot focus on the racy tests soon, as I'm burdened with MSan now.

@bradfitz

This comment has been minimized.

Copy link
Member

commented Dec 5, 2017

@krytarowski, the tree is closed right now for release. Only bug fixes can go in, not features.
See https://golang.org/wiki/Go-Release-Cycle (Unlike NetBSD, we intentionally do not have a development branch open during release freezes to encourage people to do boring stuff instead of just hacking on fun stuff)

We can merge fun things in February.

Also, we'd need to generate the syso ourselves (not shipping a random binary off the network), so you'd first need to send a change to x/build/cmd/racebuild

See https://golang.org/doc/contribute.html for how to submit a git patch to Gerrit. We only do it for people in very rare circumstance.

@krytarowski

This comment has been minimized.

Copy link
Contributor

commented Dec 5, 2017

OK, thanks. In February hopefully there will be an option to get MSan aboard.

@ceseo

This comment has been minimized.

Copy link
Contributor

commented Dec 6, 2017

FYI, I'm working on adding the necessary changes in LLVM TSan so we can get the race detectors on ppc64le.

(edit: sorry, little endian only)

@krytarowski

This comment has been minimized.

Copy link
Contributor

commented Dec 6, 2017

Great to see ppc64le development around. FreeBSD is improving their POWER8 support lately.. I would love to see it with NetBSD as well (but hw price isn't easily reachable for a hobbyist).

@ceseo

This comment has been minimized.

Copy link
Contributor

commented Dec 6, 2017

@krytarowski if you need machine access, you may try a POWER8 VM in one of our OpenPOWER Mini Clouds, like this.

@krytarowski

This comment has been minimized.

Copy link
Contributor

commented Dec 8, 2017

Personally I prefer to use a real hardware when porting the kernel and userland. But thanks!

@gopherbot

This comment has been minimized.

Copy link

commented May 11, 2018

Change https://golang.org/cl/112881 mentions this issue: cmd/racebuild: add linux/ppc64le

@gopherbot

This comment has been minimized.

Copy link

commented May 11, 2018

Change https://golang.org/cl/112880 mentions this issue: cmd/racebuild: add netbsd/amd64

gopherbot pushed a commit to golang/build that referenced this issue May 11, 2018
cmd/racebuild: add netbsd/amd64
Updates golang/go#24354.
Updates golang/go#19273.
Updates golang/go#24322.

Change-Id: Ia67fde51d7698ca94d86c4697fd153a551a8ceee
Reviewed-on: https://go-review.googlesource.com/112880
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
gopherbot pushed a commit to golang/build that referenced this issue May 11, 2018
cmd/racebuild: add linux/ppc64le
Updates golang/go#19273.
Updates golang/go#24354.
Updates golang/go#23731.

Change-Id: Iad8870b265e7e3b56b5219d3367ccef70dcc0679
Reviewed-on: https://go-review.googlesource.com/112881
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@gopherbot

This comment has been minimized.

Copy link

commented Jul 10, 2018

Change https://golang.org/cl/99835 mentions this issue: runtime/cgo: Add initial NetBSD Thread Sanitizer support

gopherbot pushed a commit that referenced this issue Jul 10, 2018
runtime/cgo: Add initial NetBSD Thread Sanitizer support
Recognize NetBSD in:
 - go/internal/work/init.go
 - race.bash
 - runtime/race/race.go

Add __ps_strings symbol in runtime/cgo/netbsd.go as this is
used internally in the TSan library for NetBSD and used for
ReExec().

Tested on NetBSD/amd64 v. 8.99.12.

Around 98% tests are passing for the ./race.bash target.

Updates #19273

Change-Id: Ic0e48d2fb159a7868aab5e17156eeaca1225e513
GitHub-Last-Rev: d6e0827
GitHub-Pull-Request: #24322
Reviewed-on: https://go-review.googlesource.com/99835
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.