[sanitizer] On OS X, verify that interceptors work and abort if not, take 2

[kubamracek]

Summary

This is a new version of http://reviews.llvm.org/D18121, which I had to revert due to test failures. It's not possible to reopen a closed revision here, so I'm creating a new one.

The issue is that TSan unit tests run with a statically linked runtime, where interceptors don't work, but that's fine, we don't need them. With the patch, this is now detected and the unit tests are aborted. To avoid this, we have DisableReexec(), but due to static initializers order, it's not possible to call this function early enough (we'd have to make more invasive changes). So I'm proposing a simpler fix, which simply adds a "disable_reexec" flag that is set in unit tests.

[dvyukov]

The issue is that TSan unit tests run with a statically linked runtime, where interceptors don't work, but that's fine, we don't need them.

What is the difference between that unit test and normal programs? Normal programs also link tsan runtime statically (or is not the case on Mac?). Can we instead make the test more like a normal program so that it does not need any special handling? Adding runtime flags for the sake of a single weird test does not look good to me.

[kubamracek]

In D18212#376450, @dvyukov wrote:

The issue is that TSan unit tests run with a statically linked runtime, where interceptors don't work, but that's fine, we don't need them.

What is the difference between that unit test and normal programs? Normal programs also link tsan runtime statically (or is not the case on Mac?). Can we instead make the test more like a normal program so that it does not need any special handling? Adding runtime flags for the sake of a single weird test does not look good to me.

Normal programs and lit tests always link TSan dynamically on OS X. Unit tests need a static library because they need to access non-exported symbols.

[samsonov]

This is really unfortunate. I also don't like adding the runtime flag for the sake of TSan unit test. Why can we call DisableReexec() from ASan unit tests, but not from TSan ones? The former also use static runtime on OS X?

[kubamracek]

In D18212#376617, @samsonov wrote:

This is really unfortunate. I also don't like adding the runtime flag for the sake of TSan unit test. Why can we call DisableReexec() from ASan unit tests, but not from TSan ones? The former also use static runtime on OS X?

The difference is in the interceptors of new/delete C++ operators. In ASan, we have INTERCEPTOR(void *, _Znwm, size_t size), which is just a regular interceptor, and in the static library, it doesn't work (just as all other interceptors). But in TSan, we declare a custom operator with void *operator new(__sanitizer::uptr size), which works even in the static library. The TEST() macro from GTest expands to a static initializer that calls new, which gets intercepted and TSan will initialize at this point. We'd need to call DisableReexec() before any other initializers.

One solution would be to put the static initializer that calls DisableReexec() to a file that's first in the CMake source file list. That's quite fragile, I don't think we want to depend on the order of file lists in CMake.

Converting the new/delete operators to be the same as ASan uncovers some test failures and needs more changes/fixes. One thing that breaks is demangling in stack trace, as things like wrap__Znwm will appear (instead of operator new), because the wrap_ prefix prevents demangling. This bug currently exist in ASan as well, but no testcase covers it (on OS X). But TSan expects this to work in a few lit tests already.

[dvyukov]

One solution would be to put the static initializer that calls DisableReexec() to a file that's first in the CMake source file list. That's quite fragile, I don't think we want to depend on the order of file lists in CMake.

Maybe add a weak function in runtime that says whether we need to reexec and override it in the test (since it links runtime statically, it should work). I would prefer such function to an additional public flag. Should be less code as well.

[samsonov]

... that is, make weak bool DisableReexec() { return false; } and override it with function returning true in unit tests.

[kubamracek]

Updating patch to use a weak function.

[dvyukov]

LGTM but wait for Alexey

[samsonov]

LGTM

compiler-rt/trunk/lib/asan/tests/asan_noinst_test.cc

 // ATTENTION!
 // Please don't call intercepted functions (including malloc() and friends)
 // in this test. The static runtime library is linked explicitly (without
 // -fsanitize=address), thus the interceptors do not work correctly on OS X.
 
 // Make sure __asan_init is called before any test case is run.
 struct AsanInitCaller {
   AsanInitCaller() {
-    DisableReexec();
     __asan_init();
   }
 };
 static AsanInitCaller asan_init_caller;
 
 TEST(AddressSanitizer, InternalSimpleDeathTest) {
   EXPECT_DEATH(exit(1), "");
 }

compiler-rt/trunk/lib/asan/tests/asan_test_main.cc

#if SANITIZER_MAC
   // much slower. Let's override this and run lit tests with 'abort_on_error=0'.
   // Also, make sure we do not overwhelm the syslog while testing.
   return "symbolize=false:abort_on_error=0:log_to_syslog=0";
 #else
   return "symbolize=false";
 #endif
 }
 
+namespace __sanitizer {
+bool ReexecDisabled() {
+  return true;
+}
+}
+
 int main(int argc, char **argv) {
   testing::GTEST_FLAG(death_test_style) = "threadsafe";
   testing::InitGoogleTest(&argc, argv);
   return RUN_ALL_TESTS();
 }

compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h

struct SignalContext {
   static SignalContext Create(void *siginfo, void *context);
 
   // Returns true if the "context" indicates a memory write.
   static WriteFlag GetWriteFlag(void *context);
 };
 
 void GetPcSpBp(void *context, uptr *pc, uptr *sp, uptr *bp);
 
-void DisableReexec();
 void MaybeReexec();
 
 template <typename Fn>
 class RunOnDestruction {
  public:
   explicit RunOnDestruction(Fn fn) : fn_(fn) {}
   ~RunOnDestruction() { fn_(); }
 

compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc

#elif defined(__mips__)
   *pc = ucontext->uc_mcontext.pc;
   *bp = ucontext->uc_mcontext.gregs[30];
   *sp = ucontext->uc_mcontext.gregs[29];
 #else
 # error "Unsupported arch"
 #endif
 }
 
-void DisableReexec() {
-  // No need to re-exec on Linux.
-}
-
 void MaybeReexec() {
   // No need to re-exec on Linux.
 }
 
 } // namespace __sanitizer
 
 #endif // SANITIZER_FREEBSD || SANITIZER_LINUX

compiler-rt/trunk/lib/sanitizer_common/sanitizer_mac.cc

if (len > name_len) {
           } while (*del++);
         }
       }
     }
     env++;
   }
 }
 
-static bool reexec_disabled = false;
-
-void DisableReexec() {
-  reexec_disabled = true;
+SANITIZER_WEAK_CXX_DEFAULT_IMPL
+bool ReexecDisabled() {
+  return false;
 }
 
 extern "C" double dyldVersionNumber;
 static const double kMinDyldVersionWithAutoInterposition = 360.0;
 
 bool DyldNeedsEnvVariable() {
   // If running on OS X 10.11+ or iOS 9.0+, dyld will interpose even if
   // DYLD_INSERT_LIBRARIES is not set. However, checking OS version via
   // GetMacosVersion() doesn't work for the simulator. Let's instead check
   // `dyldVersionNumber`, which is exported by dyld, against a known version
   // number from the first OS release where this appeared.
   return dyldVersionNumber < kMinDyldVersionWithAutoInterposition;
 }
 
 void MaybeReexec() {
-  if (reexec_disabled) return;
+  if (ReexecDisabled()) return;
 
   // Make sure the dynamic runtime library is preloaded so that the
   // wrappers work. If it is not, set DYLD_INSERT_LIBRARIES and re-exec
   // ourselves.
   Dl_info info;
   CHECK(dladdr((void*)((uptr)&__sanitizer_report_error_summary), &info));
   char *dyld_insert_libraries =
       const_cast<char*>(GetEnv(kDyldInsertLibraries));
if (DyldNeedsEnvVariable() && !lib_is_in_env) {
     Report("ERROR: The process is launched without DYLD_INSERT_LIBRARIES, "
            "which is required for the sanitizer to work. We tried to set the "
            "environment variable and re-execute itself, but execv() failed, "
            "possibly because of sandbox restrictions. Make sure to launch the "
            "executable with:\n%s=%s\n", kDyldInsertLibraries, new_env);
     CHECK("execv failed" && 0);
   }
 
+  // Verify that interceptors really work.  We'll use dlsym to locate
+  // "pthread_create", if interceptors are working, it should really point to
+  // "wrap_pthread_create" within our own dylib.
+  Dl_info info_pthread_create;
+  void *dlopen_addr = dlsym(RTLD_DEFAULT, "pthread_create");
+  CHECK(dladdr(dlopen_addr, &info_pthread_create));
+  Report("info.dli_fname = %p = %s, info_pthread_create.dli_fname = %p = %s\n",
+         info.dli_fname, info.dli_fname, info_pthread_create.dli_fname,
+         info_pthread_create.dli_fname);
+  if (internal_strcmp(info.dli_fname, info_pthread_create.dli_fname) != 0) {
+    Report(
+        "ERROR: Interceptors are not working. This may be because %s is "
+        "loaded too late (e.g. via dlopen). Please launch the executable "
+        "with:\n%s=%s\n",
+        SanitizerToolName, kDyldInsertLibraries, info.dli_fname);
+    CHECK("interceptors not installed" && 0);
+  }
+
   if (!lib_is_in_env)
     return;
 
   // DYLD_INSERT_LIBRARIES is set and contains the runtime library. Let's remove
   // the dylib from the environment variable, because interceptors are installed
   // and we don't want our children to inherit the variable.
 
   uptr env_name_len = internal_strlen(kDyldInsertLibraries);

compiler-rt/trunk/lib/sanitizer_common/sanitizer_win.cc

 uptr ReadLongProcessName(/*out*/char *buf, uptr buf_len) {
   return ReadBinaryName(buf, buf_len);
 }
 
 void CheckVMASize() {
   // Do nothing.
 }
 
-void DisableReexec() {
-  // No need to re-exec on Windows.
-}
-
 void MaybeReexec() {
   // No need to re-exec on Windows.
 }
 
 char **GetArgv() {
   // FIXME: Actually implement this function.
   return 0;
 }

compiler-rt/trunk/lib/tsan/tests/rtl/tsan_test.cc

 
 #ifdef __APPLE__
 // On Darwin, turns off symbolication and crash logs to make tests faster.
 extern "C" const char* __tsan_default_options() {
   return "symbolize=false:abort_on_error=0";
 }
 #endif
 
+namespace __sanitizer {
+bool ReexecDisabled() {
+  return true;
+}
+}
+
 int main(int argc, char **argv) {
   argv0 = argv[0];
   return run_tests(argc, argv);
 }

compiler-rt/trunk/lib/tsan/tests/unit/tsan_unit_test_main.cc

 //===-- tsan_unit_test_main.cc --------------------------------------------===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file is a part of ThreadSanitizer (TSan), a race detector.
 //
 //===----------------------------------------------------------------------===//
 #include "gtest/gtest.h"
 
+namespace __sanitizer {
+bool ReexecDisabled() {
+  return true;
+}
+}
+
 int main(int argc, char **argv) {
   testing::GTEST_FLAG(death_test_style) = "threadsafe";
   testing::InitGoogleTest(&argc, argv);
   return RUN_ALL_TESTS();
 }

compiler-rt/trunk/test/lit.common.cfg

 if config.has_lld:
   config.available_features.add('lld')
 
 if config.can_symbolize:
   config.available_features.add('can-symbolize')
 
 lit.util.usePlatformSdkOnDarwin(config, lit_config)
 
+if config.host_os == 'Darwin':
+  ld_cmd = subprocess.Popen(["bash", "-c", "sw_vers -productVersion | awk -F '.' '{print $1 \".\" $2}'"], stdout = subprocess.PIPE)
+  ld_out = ld_cmd.stdout.read().decode()
+  ld_cmd.wait()
+  osx_version = float(ld_out)
+  if osx_version >= 10.11:
+    config.available_features.add('osx-autointerception')
+
 sancovcc_path = os.path.join(llvm_tools_dir, "sancov") 
 if os.path.exists(sancovcc_path):
   config.available_features.add("has_sancovcc")
   config.substitutions.append( ("%sancovcc ", sancovcc_path) )
 
 def is_darwin_lto_supported():
   return os.path.exists(os.path.join(config.llvm_shlib_dir, 'libLTO.dylib'))
 

compiler-rt/trunk/test/tsan/Darwin/dlopen.cc

+// Checks that on OS X 10.11+ (where we do not re-exec anymore, because
+// interceptors work automatically), dlopen'ing a TSanified library from a
+// non-instrumented program exits with a user-friendly message.
+
+// REQUIRES: osx-autointerception
+
+// RUN: %clangxx_tsan %s -o %t.so -shared -DSHARED_LIB
+// RUN: %clangxx_tsan -fno-sanitize=thread %s -o %t
+
+// RUN: TSAN_DYLIB_PATH=`%clangxx_tsan %s -### 2>&1 \
+// RUN:   | grep "libclang_rt.tsan_osx_dynamic.dylib" \
+// RUN:   | sed -e 's/.*"\(.*libclang_rt.tsan_osx_dynamic.dylib\)".*/\1/'`
+
+// Launching a non-instrumented binary that dlopen's an instrumented library should fail.
+// RUN: not %run %t %t.so 2>&1 | FileCheck %s --check-prefix=CHECK-FAIL
+// Launching a non-instrumented binary with an explicit DYLD_INSERT_LIBRARIES should work.
+// RUN: DYLD_INSERT_LIBRARIES=$TSAN_DYLIB_PATH %run %t %t.so 2>&1 | FileCheck %s
+
+#include <dlfcn.h>
+#include <pthread.h>
+#include <stdio.h>
+
+#if defined(SHARED_LIB)
+extern "C" void foo() {
+  fprintf(stderr, "Hello world.\n");
+}
+#else  // defined(SHARED_LIB)
+int main(int argc, char *argv[]) {
+  void *handle = dlopen(argv[1], RTLD_NOW);
+  fprintf(stderr, "handle = %p\n", handle);
+  void (*foo)() = (void (*)())dlsym(handle, "foo");
+  fprintf(stderr, "foo = %p\n", foo);
+  foo();
+}
+#endif  // defined(SHARED_LIB)
+
+// CHECK: Hello world.
+// CHECK-NOT: ERROR: Interceptors are not working.
+
+// CHECK-FAIL-NOT: Hello world.
+// CHECK-FAIL: ERROR: Interceptors are not working.