[tsan] On OS X, build Go runtime with -mmacosx-version-min

[kubamracek]

Summary

We're not building the Go runtime with -mmacosx-version-min, which means it'll have a minimum deployment target set to the system you're building on. Let's make the code compile (and link) with the same -mmacosx-version-min flag as the rest of TSan.

[dvyukov]

lib/tsan/CMakeLists.txt

129 ↗

(On Diff #58582)

Result of this invocation is discarded. buildgo.sh itself should produce valid object file. Add -mmacosx-version-min to buildgo.sh and set it to 10.7 -- that's the minimum supported Mac version for Go.

[dvyukov]

Wait, will it work if -mmacosx-version-min is _not_ passed to linker? As far as I see, Go toolchain does not use -mmacosx-version-min when linking this object file.

[kubamracek]

-mmacosx-version-min needs to be passed to both compiler and linker. The compiler uses it to make sure it doesn’t use functions/symbols that are not available on older OS versions (and error out or warn). Linker needs this to weak-link appropriately (and strong-link whatever is already available in the specified OS version). When it’s not specified, it uses your system version, which is almost certainly not what you want -- your library will then only work on your system and higher.

There are cases where you’ll be lucky (e.g. if you only use functions/symbols that are available everywhere) even without -mmacosx-version-min.

[dvyukov]

When it’s not specified, it uses your system version

This applies to both compiler and linker? Just want to make sure.
What we will have is: compiler uses -mmacosx-version-min=10.7 when building this object file, but then linker does not have this flag. This still should work, right?
I understand that the resulting binary will work only versions >= build machine version. Nobody complained about this for Go as far as I know. Go runtime does not use any system headers and libraries and is very conservative about syscalls. If a particular user program uses system-dependent C/C++ code, then I guess it's its concern to pass -mmacosx-version-min.

[kubamracek]

Yes, this needs to be both compiler and linker. If not specified (in either case), it will assume you’re building only for your system and higher. If you “otool -l” a dylib, look for LC_VERSION_MIN_MACOSX, which says what minimal OS X version this dylib supports.

[dvyukov]

I don't have a mac machine right now. OK, let's land this. I know what to revert if anything :)

[kubamracek]

How are you shipping TSan with Go? Is it a dylib? It seems to me that it’s in a “.a” archive, linking it statically into user’s programs. In this case it depends how you link the resulting programs.

[dvyukov]

It's a single .o file (renamed to .syso).

[kubamracek]

In D20670#448152, @dvyukov wrote:

It's a single .o file (renamed to .syso).

I see. In that case the Go compiler should already be passing *some* -mmacosx-version-min to the linker. If it isn’t then the resulting program will inherit your system OS version as the minimum supported OS X. Anyway, this is either a non-issue (Go compiler is already passing the correct flags) or the issue is not TSan-specific (Go compiler doesn’t set -mmacosx-version-min, all resulting binaries can only run on your system and newer).

[kubamracek]

Note that it’s okay to compile with some -mmacosx-version-min and link with a higher version. In this case only the higher version is supported.

It’s *not* okay to do it the other way, i.e. compile with some version and link with a lower version. This can introduce terrible bugs.

[kubamracek]

This is the 1.6.2 Go distribution from golang.org:

$ otool -l usr/local/go/src/runtime/race/race_darwin_amd64.syso | grep -A3 LC_VERSION_MIN_MACOSX
      cmd LC_VERSION_MIN_MACOSX
  cmdsize 16
  version 10.10
      sdk n/a

[dvyukov]

What is the version of the binaries produced by the toolchain? You need to do:
$ go test -c -race fmt
This will produce fmt.test binary linked with race_darwin_amd64.syso.
There are changes that Go linker does the right thing and plain ignores LC_VERSION_MIN_MACOSX. Yes, there is own linker, check src/cmd/link/internal/ld/macho.go.

[dvyukov]

But there is also "external" linking mode when the host system linker is invoked. And this change can affect that mode. So that's still a positive change... though I don't know if external linking mode supports tsan.

[kubamracek]

In D20670#448232, @dvyukov wrote:

What is the version of the binaries produced by the toolchain? You need to do:
$ go test -c -race fmt

    cmd LC_VERSION_MIN_MACOSX
cmdsize 16
version 10.7
    sdk 10.7

This will produce fmt.test binary linked with race_darwin_amd64.syso.
There are changes that Go linker does the right thing and plain ignores LC_VERSION_MIN_MACOSX. Yes, there is own linker, check src/cmd/link/internal/ld/macho.go.

Wow. I didn’t expect that.

compiler-rt/trunk/lib/tsan/go/buildgo.sh

SRCS="
     ../../sanitizer_common/sanitizer_procmaps_common.cc
     ../../sanitizer_common/sanitizer_procmaps_freebsd.cc
     ../../sanitizer_common/sanitizer_linux.cc
     ../../sanitizer_common/sanitizer_linux_libcdep.cc
     ../../sanitizer_common/sanitizer_stoptheworld_linux_libcdep.cc
   "
 elif [ "`uname -a | grep Darwin`" != "" ]; then
   SUFFIX="darwin_amd64"
-  OSCFLAGS="-fPIC -Wno-unused-const-variable -Wno-unknown-warning-option"
-  OSLDFLAGS="-lpthread -fPIC -fpie"
+  OSCFLAGS="-fPIC -Wno-unused-const-variable -Wno-unknown-warning-option -mmacosx-version-min=10.7"
+  OSLDFLAGS="-lpthread -fPIC -fpie -mmacosx-version-min=10.7"
   SRCS="
     $SRCS
     ../rtl/tsan_platform_mac.cc
     ../../sanitizer_common/sanitizer_mac.cc
     ../../sanitizer_common/sanitizer_posix.cc
     ../../sanitizer_common/sanitizer_posix_libcdep.cc
     ../../sanitizer_common/sanitizer_procmaps_mac.cc
   "