[PATCH] asan/tsan/msan: properly intercept pthread_cond_xxx

[Alexander Potapenko]

  This patch doesn't seem to fix Chromium GPU hangs for me. Will double-check.


================
Comment at: lib/sanitizer_common/sanitizer.ver:6
@@ +5,2 @@
+} GLIBC_2.2.5;
+
----------------
nit: spare newline

================
Comment at: lib/sanitizer_common/sanitizer.ver:1
@@ +1,2 @@
+GLIBC_2.2.5 {
+};
----------------
Can you please add a comment describing what this script is for and who needs to use it?
It may also need a license header, as this is something you're going to ship to the users.

================
Comment at: lib/tsan/Makefile.old:2
@@ -1,3 +1,3 @@
 DEBUG=0
-LDFLAGS=-ldl -lpthread -pie
+LDFLAGS=-ldl -lpthread -lrt -pie
 CXXFLAGS = -std=c++11 -fPIE -fno-rtti -g -Wall -Werror \
----------------
Is -lrt related to this CL? If no, please commit separately.

================
Comment at: test/tsan/runtime_in_shared_lib.cc:29
@@ +28,2 @@
+// CHECK: DONE
+
----------------
nit: spare newline

================
Comment at: lib/sanitizer_common/sanitizer_internal_defs.h:151
@@ +150,3 @@
+#if defined(__PIC__) && !defined(__PIE__)
+# define THREADLOCAL   __thread __attribute__((tls_model("initial-exec")))
+#else
----------------
RHS of definitions in this file aren't horizontally aligned, except for __thread and __builtin_expect.
I suggest to make this line consistent with the rest of the file.

================
Comment at: lib/sanitizer_common/sanitizer_common_interceptors.inc:2667
@@ -2704,5 +2666,3 @@
 
-INTERCEPTOR(int, pthread_cond_init, void *c, void *a) {
-  void *cond = init_cond(c, true);
-  void *ctx;
-  COMMON_INTERCEPTOR_ENTER(ctx, pthread_cond_init, cond, a);
+static int cond_init(void *ctx, int(*real)(void *c, void *a),
+    void *c, void *a) {
----------------
I suggest putting the function parameter on a separate line and/or adding a typedef for it (here and below)

================
Comment at: lib/asan/CMakeLists.txt:167
@@ -166,1 +166,3 @@
 
+    if (UNIX AND NOT ANDROID)
+      add_sanitizer_version_script(clang_rt.asan-${arch} ../sanitizer_common/sanitizer.ver)
----------------
Is this condition true for OSX? You don't need your version script there.

================
Comment at: lib/sanitizer_common/sanitizer.ver:2
@@ +1,3 @@
+GLIBC_2.2.5 {
+};
+GLIBC_2.3.2 {
----------------
How about naming the file "sanitizer_linux.ver"? It's Linux-only, right?
Another idea is to name it "sanitizer_pthread.ver" to reflect the fact it fixes a specific problem with pthreads.

================
Comment at: lib/interception/interception_linux.h:38
@@ -37,3 +37,3 @@
 #if !defined(__ANDROID__)  // android does not have dlvsym
-# define INTERCEPT_FUNCTION_VER_LINUX_OR_FREEBSD(func, symver) \
+# define INTERCEPT_FUNCTION_VER_SINGLE(func, symver) \
      ::__interception::real_##func = (func##_f)(unsigned long) \
----------------
It's unclear what does "single" stand for here. Can you please add comments for both INTERCEPT_FUNCTION_VER_SINGLE and INTERCEPT_FUNCTION_VER?


http://reviews.llvm.org/D3309