#25682

It seems to be intensional not to support VMA 39 https://reviews.llvm.org/D52167

@dvyukov @cherrymui @zorrorffm

@mengzhuo Your observation is correct. It is a combined problem with Golang and compiler-rt. 39-bit VMA cannot easily be supported by TSAN for Go heap in compiler-rt. Actually there is also some problems with 42-bit VMA. TSAN runtime requires GO heap to be in the range [0x00c000000000, 0x00e000000000], a part of shadow memory region will be out of 42bits address(See: https://github.com/llvm-mirror/compiler-rt/blob/58d43607862096aeb32d72173911c9df244a30f1/lib/tsan/rtl/tsan_platform.h#L795). In theory, we should be able to support 47-bit VMA. But it is not a common configuration in Linux distos, so only 48-bit VMA is supported.

Thanks @mengzhuo and @Zheng-Xu for the investigation.

What is the right solution here? Reconfigure the kernel to use 48-bit VMA? Is there anything we can do on the Go side?

On the Go side, it is better that we ignore the tests on arm64 when the kernel is not configured as 48-bit VMA.

Thanks, @Zheng-Xu. How do we check that? Reading kernel config? Or we can just try to map some address and see if it works? Mind contributing a CL? Thanks.

We should probably also document this in the release note.

@cherrymui I took a quick look on disabling the test. But I don't have a good solution so far. I try to list what I found and the difficulty here:

• Check VM bits
  1. Reading kernel config can be difficult in some cases. Exporting kernel config is an optional feature.
  2. Mapping some addresses may also be unreliable due to unknown memory layout during the execution.
  3. llvm/compiler-rt get VMA size by checking the most significant bit of the stack. https://github.com/llvm-mirror/compiler-rt/blob/58d43607862096aeb32d72173911c9df244a30f1/lib/tsan/rtl/tsan_platform_linux.cc#L212
• runtime check
  1. Only g0 stack of the main thread is allocated by kernel. It is almost impossible to get VMA size by normal Go application. We will have to add some new interface in runtime to return VMA size.
• existing checks for race detector support
  1. build time check. All other architectures either support race detector or doesn't support. So they can do build time check: https://go.googlesource.com/go/+/master/src/cmd/go/internal/work/init.go#51 and https://go.googlesource.com/go/+/master/src/cmd/internal/sys/supported.go#9
  2. dist/test will enable race detector if race detector is supported. https://go.googlesource.com/go/+/master/src/cmd/dist/test.go#1335 and https://go.googlesource.com/go/+/master/src/cmd/dist/test.go#1474
  3. I don't have a good solution to check VMA size in dist/test because cmd/dist has to be buildable by Go 1.4. And I didn't have an easy way to check VMA size, except adding some new interface in runtime.
• Possible solution
  1. no build time check for arm64. We are always able to build with race. This is reasonable, because it can be cross-compiled.
  2. execution will fail if the binary is build with race and kernel is not configured to use 48-bit VMA.
  3. Always disable "-race" for arm64 in https://go.googlesource.com/go/+/master/src/cmd/dist/test.go#1474

Do you think above solution is reasonable? I can push a CL if it is OK.

Thanks for the information.

As you said, we should not do anything at build time, exactly because of cross compilation.

Always disabling -race test in cmd/dist is also not a good idea, as we'll not know if it breaks.

Maybe in cmd/dist, we can check the stderr and if it is the unsupported VMA failure, we turn it into SKIP instead of FAIL?

Users who use the race detector can still encounter this in their own programs. So we still need to document this.

Change https://golang.org/cl/160919 mentions this issue: cmd/dist: skip race detector test failure for unsupported VMA

@cherrymui Could you mind take a look the patch catching unsupported VMA failure and skipping that.

If I understand this correctly, the race detector simply doesn't work reliably on arm64 systems. And the problem can only be fixed in race.syso, not in Go code.

That's too bad, but I think that for 1.12 we should remove arm64 from raceDetectorSupported in cmd/dist/test.go and cmd/internal/sys/supported.go. It wasn't supported in 1.11, so this is not a regression. We can try again to enable it in 1.13.

@ianlancetaylor Actually for 44-bit VMA , ppc64le has same problem. Only 46-bit VMA and 47-bit VMA are currently supported for powercp64 on Go.

@ianlancetaylor, what do you want to do here (if anything) for Go 1.12? I see that https://go-review.googlesource.com/c/go/+/160919/ stalled and hasn't been updated with review feedback.

@cherrymui @mengzhuo Should we keep or disable arm64 race detector for the 1.12 release? I don't fully understand the circumstances under which it fails.

@ianlancetaylor I think keep linux/arm64 race detector is fine. This issue only happens when you try to run race builds on unsupported kernel

OK, thanks.

If I understand correctly, the ARM64 race detector support works fine under certain kernel configurations (probably the popular one?), but doesn't work at all under some other kernel configurations. So I was suggesting we skip the tests under unsupported kernel.

@cherrymui, skipping tests is fine (even for Go 1.12, or a later point release) if you want. But I don't think this is a release blocker at this point. That is, all.bash passing for everybody's distro/version isn't a blocker for shipping a release.

I agree that this is not a release-blocker but more generally I don't think skipping the tests is really a fix here. If we can skip the tests, we can and should also report the problem when running a binary built with -race.

@cherrymui @ianlancetaylor Currently data race works fine on the kernel configured with 48-bits VMA which is popular. Data race doesn't support the kernel configured with 39-bit VMA and 42-bit VMA because the shadow memory calculated in TSAN runtime will be out of the address space. I will investigate how to support 39-bit VMA in TSAN runtime.

@bradfitz I also agree that this does not block release.

@ianlancetaylor TSAN runtime will report the error message when running a binary built with -race on unsupported VMA.
FATAL: ThreadSanitizer: unsupported VMA range
FATAL: Found 39 - Supported 48

OK, thanks.