[compiler-rt] Allow golang race detector to run on musl-c

[graywolf-at-work]

Summary

tsan while used by golang's race detector was not working on alpine
linux, since it is using musl-c instead of glibc. Since alpine is very
popular distribution for container deployments, having working race
detector would be nice. This commits adds some ifdefs to get it working.

It fixes https://github.com/golang/go/issues/14481 on golang's issue
tracker.

[graywolf-at-work]

/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp:15:10: error: 'sanitizer_common/sanitizer_platform.h' file not found [clang-diagnostic-error]
#include "sanitizer_common/sanitizer_platform.h"

Hm, I don't understand why would my change result in such error. I'm not nothing anywhere near that line nor did I delete the file.

[dvyukov]

In D75849#1912918, @graywolf-at-work wrote:

/mnt/disks/ssd0/agent/workspace/amd64_debian_testing_clang8/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp:15:10: error: 'sanitizer_common/sanitizer_platform.h' file not found [clang-diagnostic-error]
#include "sanitizer_common/sanitizer_platform.h"

Hm, I don't understand why would my change result in such error. I'm not nothing anywhere near that line nor did I delete the file.

Where did this error come from?

At In https://reviews.llvm.org/B48551 I only see:
Build 52335 pre-merge checks

So maybe these checks failed _before_ merging your change?

[dvyukov]

How did you test this?

It would be good to add a test because it will be broken the very next release again. People changing C++ mode don't have any good means to foresee/test for any Go problems.

After running buildgo.sh I see:

go$ nm test | grep libc; ldd test
000000000002e9b0 T libc_csu_fini
000000000002e950 T libc_csu_init

U __libc_free@@GLIBC_2.2.5
U __libc_malloc@@GLIBC_2.2.5
U __libc_realloc@@GLIBC_2.2.5
U __libc_stack_end@@GLIBC_2.2.5
U __libc_start_main@@GLIBC_2.2.5

linux-vdso.so.1 (0x00007ffd695dd000)
libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007ff66aa37000)
libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007ff66a877000)
/lib64/ld-linux-x86-64.so.2 (0x00007ff66b44a000)

We could grep nm and/or link something without libc in buildgo.sh.

compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp

1075

Looking at the comment below, this seems to be a hard-earned bit of logic:
// EXEC_PAGESIZE may not be trustworthy.

Why do we need this change?

compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp

65 ↗

(On Diff #249101)

Why is this needed? How does this affect alpine linux?

[graywolf-at-work]

Sorry for the late response, for some reason I did not receive any emails about your comments :/

In D75849#1913818, @dvyukov wrote:

Where did this error come from?

https://results.llvm-merge-guard.org/amd64_debian_testing_clang8-4478/clang-tidy.txt , the clang-tidy linux button.

[graywolf-at-work]

In D75849#1913927, @dvyukov wrote:

How did you test this?

Fired up Alpine (musl) docker container (golang:1.14.1-alpine3.11), build
compiler-rt with this patch, replaced golang's race_linux_amd64.syso with the
new one and tried running go test with -race flag and it worked.

Same on Archlinux (glibc) to make sure I did not break it.

Tested program was:

~/foo $ cat /etc/os-release
NAME="Alpine Linux"
ID=alpine
VERSION_ID=3.11.3
PRETTY_NAME="Alpine Linux v3.11"
HOME_URL="https://alpinelinux.org/"
BUG_REPORT_URL="https://bugs.alpinelinux.org/"
~/foo $ cat main.go
package main

import "time"
import "fmt"
import "math/rand"

func main() {
        start := time.Now()
        var t *time.Timer
        t = time.AfterFunc(randomDuration(), func() {
                fmt.Println(time.Now().Sub(start))
                t.Reset(randomDuration())
        })
        time.Sleep(5 * time.Second)
}

func randomDuration() time.Duration {
        return time.Duration(rand.Int63n(1e9))
}
~/foo $ go run -race .
948.325306ms
==================
WARNING: DATA RACE
Read at 0x00c000010028 by goroutine 8:
  main.main.func1()
      /home/ci/foo/main.go:12 +0x121

Previous write at 0x00c000010028 by main goroutine:
  main.main()
      /home/ci/foo/main.go:10 +0x18d

Goroutine 8 (running) created at:
  time.goFunc()
      /usr/local/go/src/time/sleep.go:168 +0x51
==================
1.033222843s
1.699865735s
1.935354245s
2.222914073s
2.772484274s
3.405927055s
3.738185435s
3.921800399s
4.40249537s
Found 1 data race(s)
exit status 66

It would be good to add a test because it will be broken the very next
release again. People changing C++ mode don't have any good means to
foresee/test for any Go problems.

I agree, but short of running it on actual Alpine I do not see bullet-proof way
to do it.

After running buildgo.sh I see:

go$ nm test | grep libc; ldd test
000000000002e9b0 T libc_csu_fini
000000000002e950 T libc_csu_init

U __libc_free@@GLIBC_2.2.5
U __libc_malloc@@GLIBC_2.2.5
U __libc_realloc@@GLIBC_2.2.5
U __libc_stack_end@@GLIBC_2.2.5
U __libc_start_main@@GLIBC_2.2.5

linux-vdso.so.1 (0x00007ffd695dd000)
libpthread.so.0 => /lib/x86_64-linux-gnu/libpthread.so.0 (0x00007ff66aa37000)
libc.so.6 => /lib/x86_64-linux-gnu/libc.so.6 (0x00007ff66a877000)
/lib64/ld-linux-x86-64.so.2 (0x00007ff66b44a000)

We could grep nm and/or link something without libc in buildgo.sh.

I've tried putting something together. I did nm to race_linux_amd64.syso
instead, seemed like a better way. Do you think it is ok to do it that way?

compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp

1075

Musl libc does not have that define:

# cat /etc/os-release
NAME="Alpine Linux"
ID=alpine
VERSION_ID=3.11.3
PRETTY_NAME="Alpine Linux v3.11"
HOME_URL="https://alpinelinux.org/"
BUG_REPORT_URL="https://bugs.alpinelinux.org/"
# grep -r EXEC_PAGESIZE /usr/include ; echo $?
1

I've returned the check for the architecture back in there.

compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp

65 ↗

(On Diff #249101)

Musl libc does not have this symbol and it is not used in golang sanitizers so I did no see a reason to keep it. But at the same time no strong reason to remove it either. I've dropped this part of the patch.

[graywolf-at-work]

• add rudimentary test
• drop unnecessary changes
• make check for EXEC_PAGESIZE more tight

[graywolf-at-work]

• Fix check for glibc linking

[dvyukov]

Submitted in https://github.com/llvm/llvm-project/commit/5f149ef51fcf08d52dd0977d54e7b3e4f15bede7

[arphaman]

Hi @graywolf-at-work, @dvyukov

This change causes compiler-rt to fail to build lib/CMakeFiles/SanitizerLintCheck on Darwin with the following error:

clang-11: warning: include path for libstdc++ headers not found; pass '-stdlib=libc++' on the command line to use the libc++ standard library instead [-Wstdlibcxx-not-found]
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/nm: race_darwin_amd64.syso: No such file or directory.

Example: http://lab.llvm.org:8080/green/job/clang-stage1-RA/7900/consoleFull#-87031640a1ca8a51-895e-46c6-af87-ce24fa4cd561

Can you please take a look at this issue as soon as you can? Let me know if you need help reproducing this.

[arphaman]

In D75849#1942382, @arphaman wrote:

Hi @graywolf-at-work, @dvyukov

This change causes compiler-rt to fail to build lib/CMakeFiles/SanitizerLintCheck on Darwin with the following error:

clang-11: warning: include path for libstdc++ headers not found; pass '-stdlib=libc++' on the command line to use the libc++ standard library instead [-Wstdlibcxx-not-found]
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/nm: race_darwin_amd64.syso: No such file or directory.

Example: http://lab.llvm.org:8080/green/job/clang-stage1-RA/7900/consoleFull#-87031640a1ca8a51-895e-46c6-af87-ce24fa4cd561

Can you please take a look at this issue as soon as you can? Let me know if you need help reproducing this.

Apologies, I think I misdiagnosed the failure. I think there's another issue that follows the missing race_darwin_amd64.syso that is failing the build. I'll get back to you once I know more.

[dyung]

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

172

I believe this should be $DIR/race_$SUFFIX.syso otherwise it won't find the file.

[dyung]

In D75849#1942411, @arphaman wrote:

In D75849#1942382, @arphaman wrote:

Hi @graywolf-at-work, @dvyukov

This change causes compiler-rt to fail to build lib/CMakeFiles/SanitizerLintCheck on Darwin with the following error:

clang-11: warning: include path for libstdc++ headers not found; pass '-stdlib=libc++' on the command line to use the libc++ standard library instead [-Wstdlibcxx-not-found]
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/nm: race_darwin_amd64.syso: No such file or directory.

Example: http://lab.llvm.org:8080/green/job/clang-stage1-RA/7900/consoleFull#-87031640a1ca8a51-895e-46c6-af87-ce24fa4cd561

Can you please take a look at this issue as soon as you can? Let me know if you need help reproducing this.

Apologies, I think I misdiagnosed the failure. I think there's another issue that follows the missing race_darwin_amd64.syso that is failing the build. I'll get back to you once I know more.

Hi, I believe this problem was due to an issue in D76073 which @wolfgangp fixed in 93f77438.

[graywolf-at-work]

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

172

That is correct, I'm sorry for missing that:

From b518ffeb8e2d4e71809304a3e926db4daa0dda1b Mon Sep 17 00:00:00 2001
From: Tomas Volf <[e-mail]>
Date: Thu, 26 Mar 2020 14:56:38 +0100
Subject: [PATCH] [compiler-rt][tsan][go] Fix check for linking against libc

---
 compiler-rt/lib/tsan/go/buildgo.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/compiler-rt/lib/tsan/go/buildgo.sh b/compiler-rt/lib/tsan/go/buildgo.sh
index 5156bd6f67d..72627324ae4 100755
--- a/compiler-rt/lib/tsan/go/buildgo.sh
+++ b/compiler-rt/lib/tsan/go/buildgo.sh
@@ -169,7 +169,7 @@ $CC $DIR/gotsan.cpp -c -o $DIR/race_$SUFFIX.syso $FLAGS $CFLAGS
 $CC $OSCFLAGS $ARCHCFLAGS test.c $DIR/race_$SUFFIX.syso -g -o $DIR/test $OSLDFLAGS $LDFLAGS

 # Verify that no glibc specific code is present
-if nm race_$SUFFIX.syso | grep -q __libc_; then
+if nm $DIR/race_$SUFFIX.syso | grep -q __libc_; then
        printf -- '%s seems to link to libc\n' "race_$SUFFIX.syso"
        exit 1
 fi
--
2.25.1

compiler-rt/lib/sanitizer_common/sanitizer_allocator.cpp

 const char *SecondaryAllocatorName = "LargeMmapAllocator";
 
 // ThreadSanitizer for Go uses libc malloc/free.
-#if SANITIZER_GO || defined(SANITIZER_USE_MALLOC)
+#if defined(SANITIZER_USE_MALLOC)
 # if SANITIZER_LINUX && !SANITIZER_ANDROID
 extern "C" void *__libc_malloc(uptr size);
 #  if !SANITIZER_GO

compiler-rt/lib/sanitizer_common/sanitizer_common.cpp

   return name_len;
 }
 
+#if !SANITIZER_GO
 void PrintCmdline() {
   char **argv = GetArgv();
   if (!argv) return;

     Printf("%s ", argv[i]);
   Printf("\n\n");
 }
+#endif
 
 // Malloc hooks.
 static const int kMaxMallocFreeHooks = 5;

compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp

 #include "sanitizer_placement_new.h"
 #include "sanitizer_procmaps.h"
 
-#if SANITIZER_LINUX
+#if SANITIZER_LINUX && !SANITIZER_GO
 #include <asm/param.h>
 #endif
 

 #endif
 }
 
-#if !SANITIZER_FREEBSD && !SANITIZER_NETBSD && !SANITIZER_OPENBSD
+#if !SANITIZER_FREEBSD && !SANITIZER_NETBSD && !SANITIZER_OPENBSD && \
+    !SANITIZER_GO
 extern "C" {
 SANITIZER_WEAK_ATTRIBUTE extern void *__libc_stack_end;
 }

 }
 #endif
 
-#if !SANITIZER_OPENBSD
+#if !SANITIZER_OPENBSD && !SANITIZER_GO
 static void GetArgsAndEnv(char ***argv, char ***envp) {
 #if SANITIZER_FREEBSD
   // On FreeBSD, retrieving the argument and environment arrays is done via the

 
 #if !SANITIZER_ANDROID
 uptr GetPageSize() {
-#if SANITIZER_LINUX && (defined(__x86_64__) || defined(__i386__))
+#if SANITIZER_LINUX && (defined(__x86_64__) || defined(__i386__)) && \
+    defined(EXEC_PAGESIZE)
   return EXEC_PAGESIZE;
 #elif SANITIZER_FREEBSD || SANITIZER_NETBSD
 // Use sysctl as sysconf can trigger interceptors internally.

compiler-rt/lib/sanitizer_common/sanitizer_linux_libcdep.cpp

 }
 #endif  // SANITIZER_LINUX && !SANITIZER_GO
 
-#if !SANITIZER_OPENBSD
+#if !SANITIZER_OPENBSD && !SANITIZER_GO
 void ReExec() {
   const char *pathname = "/proc/self/exe";
 

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

 
 $CC $OSCFLAGS $ARCHCFLAGS test.c $DIR/race_$SUFFIX.syso -g -o $DIR/test $OSLDFLAGS $LDFLAGS
 
+# Verify that no glibc specific code is present
+if nm race_$SUFFIX.syso | grep -q __libc_; then
+  printf -- '%s seems to link to libc\n' "race_$SUFFIX.syso"
+  exit 1
+fi
+
 export GORACE="exitcode=0 atexit_sleep_ms=0"
 if [ "$SILENT" != "1" ]; then
   $DIR/test