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: add race detector for ios/arm64 #39521

Open
steeve opened this issue Jun 11, 2020 · 14 comments
Open

runtime: add race detector for ios/arm64 #39521

steeve opened this issue Jun 11, 2020 · 14 comments
Milestone

Comments

@steeve
Copy link
Contributor

@steeve steeve commented Jun 11, 2020

What version of Go are you using (go version)?

master as of the opening of this issue

$ go version
go version devel +5adaa1290e Thu Jun 11 06:04:57 2020 +0000 darwin/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/steeve/Library/Caches/go-build"
GOENV="/Users/steeve/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/steeve/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/steeve/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/Users/steeve/code/github.com/znly/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/Users/steeve/code/github.com/znly/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/steeve/code/github.com/znly/go/src/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/bs/51dlb_nn5k35xq9qfsxv9wc00000gr/T/go-build749515592=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Since darwin/amd64 and linux/arm64 are supported, I figure I'd give it a shot. Applied this patch to try and "enable" the race detector on ios/arm64:

diff --git a/src/cmd/internal/sys/supported.go b/src/cmd/internal/sys/supported.go
index c27b3b986d..00faf49f8a 100644
--- a/src/cmd/internal/sys/supported.go
+++ b/src/cmd/internal/sys/supported.go
@@ -13,8 +13,10 @@ func RaceDetectorSupported(goos, goarch string) bool {
 	switch goos {
 	case "linux":
 		return goarch == "amd64" || goarch == "ppc64le" || goarch == "arm64"
-	case "darwin", "freebsd", "netbsd", "windows":
+	case "freebsd", "netbsd", "windows":
 		return goarch == "amd64"
+	case "darwin":
+		return goarch == "amd64" || goarch == "arm64"
 	default:
 		return false
 	}

What did you expect to see?

Hopefully a successful build with the race detector enabled.

What did you see instead?

The linker fails/crashes with the following:

Use --sandbox_debug to see verbose messages from the sandbox
runtime.rt0_go: missing section for runtime.tls_g
runtime.load_g: missing section for runtime.tls_g
runtime.save_g: missing section for runtime.tls_g
runtime.rt0_go: reloc 3 (R_ADDRARM64) to non-macho symbol runtime.tls_g type=37 (STLSBSS)
runtime.rt0_go: unsupported obj reloc 3 (R_ADDRARM64)/8 to runtime.tls_g
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x2 pc=0x1213e6e]

goroutine 1 [running]:
cmd/link/internal/arm64.machoreloc1(0x1473b20, 0xc0000b6070, 0xc0049bd000, 0xc005222c40, 0x642e4, 0x1)
	/Users/steeve/code/github.com/znly/go/src/cmd/link/internal/arm64/asm.go:388 +0x4ce
cmd/link/internal/ld.machorelocsect(0xc000071880, 0xc0000b62a0, 0xc001a50000, 0x4279, 0x4279)
	/Users/steeve/code/github.com/znly/go/src/cmd/link/internal/ld/macho.go:1067 +0x18c
cmd/link/internal/ld.Machoemitreloc(0xc000071880)
	/Users/steeve/code/github.com/znly/go/src/cmd/link/internal/ld/macho.go:1081 +0xca
cmd/link/internal/arm64.asmb2(0xc000071880)
	/Users/steeve/code/github.com/znly/go/src/cmd/link/internal/arm64/asm.go:912 +0x5a5
cmd/link/internal/ld.Main(0x1473b20, 0x10, 0x20, 0x1, 0x1f, 0x1e, 0x12d3faa, 0x14, 0x12d767f, 0x1a, ...)
	/Users/steeve/code/github.com/znly/go/src/cmd/link/internal/ld/main.go:349 +0x1509
main.main()
	/Users/steeve/code/github.com/znly/go/src/cmd/link/main.go:68 +0x1dc
link: error running subcommand: exit status 2

Mainly opening this issue so the discussion can be had.

@randall77
Copy link
Contributor

@randall77 randall77 commented Jun 11, 2020

You might want to look at https://go-review.googlesource.com/c/go/+/237057 which enables the race detector on openbsd/amd64. Particularly, getting the race detector library working would be a first step (maybe it already works on darwin/arm64? Not sure).

@dvyukov

@dvyukov
Copy link
Member

@dvyukov dvyukov commented Jun 11, 2020

ThreadSanitizer works on darwin/arm64 for C/C++ IIRC, but some spot fixing may be required for Go.
This is the corresponding change to ThreadSanitizer:
https://reviews.llvm.org/D80469
At the very least the same will be required for darwin/arm64. If it works (build succeeds) then it will possible to copy the syso file into Go and do testing.

@steeve
Copy link
Contributor Author

@steeve steeve commented Jun 11, 2020

Thanks to @randall77, I updated the diff to the following, unfortunately with same errors:

diff --git a/src/cmd/internal/sys/supported.go b/src/cmd/internal/sys/supported.go
index c27b3b986d..00faf49f8a 100644
--- a/src/cmd/internal/sys/supported.go
+++ b/src/cmd/internal/sys/supported.go
@@ -13,8 +13,10 @@ func RaceDetectorSupported(goos, goarch string) bool {
 	switch goos {
 	case "linux":
 		return goarch == "amd64" || goarch == "ppc64le" || goarch == "arm64"
-	case "darwin", "freebsd", "netbsd", "windows":
+	case "freebsd", "netbsd", "windows":
 		return goarch == "amd64"
+	case "darwin":
+		return goarch == "amd64" || goarch == "arm64"
 	default:
 		return false
 	}
diff --git a/src/runtime/race/race.go b/src/runtime/race/race.go
index c894de5f72..6ef78da089 100644
--- a/src/runtime/race/race.go
+++ b/src/runtime/race/race.go
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// +build race,linux,amd64 race,freebsd,amd64 race,netbsd,amd64 race,darwin,amd64 race,windows,amd64 race,linux,ppc64le race,linux,arm64
+// +build race,linux,amd64 race,freebsd,amd64 race,netbsd,amd64 race,darwin,amd64 race,darwin,arm64 race,ios,arm64 race,windows,amd64 race,linux,ppc64le race,linux,arm64
 
 package race
@steeve
Copy link
Contributor Author

@steeve steeve commented Jun 11, 2020

ThreadSanitizer works on darwin/arm64 for C/C++ IIRC, but some spot fixing may be required for Go.
This is the corresponding change to ThreadSanitizer:
https://reviews.llvm.org/D80469
At the very least the same will be required for darwin/arm64. If it works (build succeeds) then it will possible to copy the syso file into Go and do testing.

Oh! I didn't see that tsan was shipped as a .syso file!

@steeve
Copy link
Contributor Author

@steeve steeve commented Jun 11, 2020

@dvyukov I am struggling to find how to 1. build only tsan and 2. cross compile it (as it seems the toolchain, buildgo.sh, expects the host and target to be the same). Would you have any pointers by chance? Thanks!

@dvyukov
Copy link
Member

@dvyukov dvyukov commented Jun 11, 2020

@dvyukov I am struggling to find how to 1. build only tsan and 2. cross compile it (as it seems the toolchain, buildgo.sh, expects the host and target to be the same). Would you have any pointers by chance? Thanks!

  1. That very buildgo.sh builds only tsan (for Go).
  2. We will probably need some env var or flag to tell the script to do a cross-build for another target. Currently no target uses cross-compilation.
@randall77
Copy link
Contributor

@randall77 randall77 commented Jun 11, 2020

We normally build the syso files using the racebuild tool. It's mentioned in runtime/race/README.

You'll just need to add a section to racebuild.go for the target architecture. You'll probably have to start with running the commands in the new section by hand on the target to make sure they work, as the racebuild tool runs everything inside a gomote, which makes it hard to debug.

@steeve
Copy link
Contributor Author

@steeve steeve commented Jun 11, 2020

Thank you @dvyukov. Somehow this morning I couldn't make it work, but I do now.

It seems all I need now is to find the correct mapping for Go on darwin/arm64, it's complaining it can't find kHiAppMemEnd:

./gotsan.cpp:10809:26: error: no member named 'kHiAppMemEnd' in '__tsan::Mapping'; did you mean 'kAppMemEnd'?
  if (max_vm != Mapping::kHiAppMemEnd) {
                ~~~~~~~~~^~~~~~~~~~~~

The offending part is:

void InitializePlatformEarly() {
#if defined(__aarch64__) <------------ how does this work on linux/arm64??
  uptr max_vm = GetMaxUserVirtualAddress() + 1;
  if (max_vm != Mapping::kHiAppMemEnd) {
    Printf("ThreadSanitizer: unsupported vm address limit %p, expected %p.\n",
           max_vm, Mapping::kHiAppMemEnd);
    Die();
  }
#endif
}

Indeed, look at tsan_platform.h, I see:

#elif SANITIZER_GO && defined(__aarch64__)

/* Go on linux/aarch64 (48-bit VMA)
0000 0000 1000 - 0000 1000 0000: executable
0000 1000 0000 - 00c0 0000 0000: -
00c0 0000 0000 - 00e0 0000 0000: heap
00e0 0000 0000 - 2000 0000 0000: -
2000 0000 0000 - 3000 0000 0000: shadow
3000 0000 0000 - 3000 0000 0000: -
3000 0000 0000 - 4000 0000 0000: metainfo (memory blocks and sync objects)
4000 0000 0000 - 6000 0000 0000: -
6000 0000 0000 - 6200 0000 0000: traces
6200 0000 0000 - 8000 0000 0000: -
*/

struct Mapping {
  static const uptr kMetaShadowBeg = 0x300000000000ull;
  static const uptr kMetaShadowEnd = 0x400000000000ull;
  static const uptr kTraceMemBeg   = 0x600000000000ull;
  static const uptr kTraceMemEnd   = 0x620000000000ull;
  static const uptr kShadowBeg     = 0x200000000000ull;
  static const uptr kShadowEnd     = 0x300000000000ull;
  static const uptr kAppMemBeg     = 0x000000001000ull;
  static const uptr kAppMemEnd     = 0x00e000000000ull;
};

So I'm not really sure how this builds on linux/arm64.

Still digging. Thank you folks for the precious help! I'm feeling this might just work soon!

@steeve
Copy link
Contributor Author

@steeve steeve commented Jun 11, 2020

@randall77 That's great information thank you.

@steeve
Copy link
Contributor Author

@steeve steeve commented Jun 11, 2020

Ah got it it's because of SANITIZER_MAC. It seems I'll have to comment that out when SANITIZER_GO is present.

@steeve
Copy link
Contributor Author

@steeve steeve commented Jun 11, 2020

Getting close:

Undefined symbols for architecture arm64:
  "___cxa_guard_acquire", referenced from:
      __sanitizer::GetMaxUserVirtualAddress() in race_darwin_arm64.syso
      __sanitizer::GetMaxVirtualAddress() in race_darwin_arm64.syso
      __sanitizer::FindAvailableMemoryRange(unsigned long, unsigned long, unsigned long, unsigned long*, unsigned long*) in race_darwin_arm64.syso
  "___cxa_guard_release", referenced from:
      __sanitizer::GetMaxUserVirtualAddress() in race_darwin_arm64.syso
      __sanitizer::GetMaxVirtualAddress() in race_darwin_arm64.syso
      __sanitizer::FindAvailableMemoryRange(unsigned long, unsigned long, unsigned long, unsigned long*, unsigned long*) in race_darwin_arm64.syso
ld: symbol(s) not found for architecture arm64
@steeve
Copy link
Contributor Author

@steeve steeve commented Jun 11, 2020

I was missing libc++. It worked:

-rw-r--r--   1 steeve  staff   428K Jun 11 19:14 race_darwin_arm64.syso
-rw-r--r--   1 steeve  staff   1.3M Jun 11 19:14 race_debug_darwin_arm64.syso

Now I'm gonna try it in Go, but so far it's not looking bad.

@steeve
Copy link
Contributor Author

@steeve steeve commented Jun 11, 2020

I Go to recognize the .syso, however, the linker is still crashing, so I think some additional work is needed:

runtime.rt0_go: missing section for runtime.tls_g
runtime.load_g: missing section for runtime.tls_g
runtime.save_g: missing section for runtime.tls_g
runtime.rt0_go: reloc 3 (R_ADDRARM64) to non-macho symbol runtime.tls_g type=36 (STLSBSS)
runtime.rt0_go: unsupported obj reloc 3 (R_ADDRARM64)/8 to runtime.tls_g
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x2 pc=0x11d9f0d]

goroutine 1 [running]:
cmd/link/internal/arm64.machoreloc1(0x144f120, 0xc0006e6000, 0xc00074fe50, 0xc0010c2280, 0x5a824, 0x1)
	/Users/steeve/code/github.com/znly/go/src/cmd/link/internal/arm64/asm.go:360 +0x4dd
cmd/link/internal/ld.machorelocsect(0xc0006e4000, 0xc008d3b9a0, 0xc00b6e2000, 0x42b0, 0x7209)
	/Users/steeve/code/github.com/znly/go/src/cmd/link/internal/ld/macho.go:1034 +0x188
cmd/link/internal/ld.Machoemitreloc(0xc0006e4000)
	/Users/steeve/code/github.com/znly/go/src/cmd/link/internal/ld/macho.go:1048 +0x92
cmd/link/internal/arm64.asmb2(0xc0006e4000)
	/Users/steeve/code/github.com/znly/go/src/cmd/link/internal/arm64/asm.go:907 +0x64a
cmd/link/internal/ld.Main(0x144f120, 0x8, 0x20, 0x1, 0x1f, 0x1e, 0x12980df, 0x1a, 0x128e8a1, 0x3, ...)
	/Users/steeve/code/github.com/znly/go/src/cmd/link/internal/ld/main.go:271 +0xd9a
main.main()
	/Users/steeve/code/github.com/znly/go/src/cmd/link/main.go:65 +0x1d6
link: error running subcommand: exit status 2
@steeve
Copy link
Contributor Author

@steeve steeve commented Jun 11, 2020

If anyone wants to investigate

The diff to Go

diff --git a/src/cmd/internal/sys/supported.go b/src/cmd/internal/sys/supported.go
index 4162858ac1..fe6e88da0a 100644
--- a/src/cmd/internal/sys/supported.go
+++ b/src/cmd/internal/sys/supported.go
@@ -13,8 +13,10 @@ func RaceDetectorSupported(goos, goarch string) bool {
 	switch goos {
 	case "linux":
 		return goarch == "amd64" || goarch == "ppc64le" || goarch == "arm64"
-	case "darwin", "freebsd", "netbsd", "windows":
+	case "freebsd", "netbsd", "windows":
 		return goarch == "amd64"
+	case "darwin":
+		return goarch == "amd64" || goarch == "arm64"
 	default:
 		return false
 	}
diff --git a/src/runtime/race/race.go b/src/runtime/race/race.go
index d298e805cf..c2d50d3e9a 100644
--- a/src/runtime/race/race.go
+++ b/src/runtime/race/race.go
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// +build race,linux,amd64 race,freebsd,amd64 race,netbsd,amd64 race,darwin,amd64 race,windows,amd64 race,linux,ppc64le race,linux,arm64
+// +build race,linux,amd64 race,freebsd,amd64 race,netbsd,amd64 race,darwin,amd64 race,darwin,arm64 race,ios,arm64 race,windows,amd64 race,linux,ppc64le race,linux,arm64
 
 package race

The (dirty) diff to LLVM

diff --git a/compiler-rt/lib/tsan/go/buildgo.sh b/compiler-rt/lib/tsan/go/buildgo.sh
index 2238caf53..74a3e9f13 100755
--- a/compiler-rt/lib/tsan/go/buildgo.sh
+++ b/compiler-rt/lib/tsan/go/buildgo.sh
@@ -130,10 +130,11 @@ elif [ "`uname -a | grep OpenBSD`" != "" ]; then
 		../../sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cpp
 	"
 elif [ "`uname -a | grep Darwin`" != "" ]; then
-	SUFFIX="darwin_amd64"
-	OSCFLAGS="-fPIC -Wno-unused-const-variable -Wno-unknown-warning-option -mmacosx-version-min=10.7"
-	ARCHCFLAGS="-m64"
-	OSLDFLAGS="-lpthread -fPIC -fpie -mmacosx-version-min=10.7"
+	CC="xcrun -sdk iphoneos clang -target arm64-apple-ios"
+	SUFFIX="darwin_arm64"
+	OSCFLAGS="-fPIC -Wno-unused-const-variable -Wno-unknown-warning-option -miphoneos-version-min=11.0 -lc++"
+	ARCHCFLAGS=""
+	OSLDFLAGS="-lpthread -fPIC -fpie -miphoneos-version-min=11.0"
 	SRCS="
 		$SRCS
 		../rtl/tsan_platform_mac.cpp
diff --git a/compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp b/compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp
index eea52a34e..eb104ef1e 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_platform_mac.cpp
@@ -234,7 +234,7 @@ static void my_pthread_introspection_hook(unsigned int event, pthread_t thread,
 #endif
 
 void InitializePlatformEarly() {
-#if defined(__aarch64__)
+#if defined(__aarch64__) && !SANITIZER_GO
   uptr max_vm = GetMaxUserVirtualAddress() + 1;
   if (max_vm != Mapping::kHiAppMemEnd) {
     Printf("ThreadSanitizer: unsupported vm address limit %p, expected %p.\n",

The pre-built syso file

race_darwin_arm64.syso.zip

@toothrot toothrot added this to the Backlog milestone Jun 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.