[Sanitizer][Darwin] Cleanup MaybeReexec() function and usage

[yln]

Summary

While investigating another issue, I noticed that MaybeReexec() never
actually "re-executes via execv()" anymore. DyldNeedsEnvVariable()
only returned true on macOS 10.10 and below.

Usually, I try to avoid "unnecessary" cleanups (it's hard to be certain
that there truly is no fallout), but I decided to do this one because:

• I initially tricked myself into thinking that MaybeReexec() was relevant to my original investigation (instead of being dead code).
• The deleted code itself is quite complicated.
• Over time a few other things were mushed into MaybeReexec(): initializing MonotonicNanoTime(), verifying interceptors are working, and stripping the DYLD_INSERT_LIBRARIES env var to avoid problems when forking.
• This platform-specific thing leaked into sanitizer_common.h.
• The ReexecDisabled() config nob relies on the "strong overrides weak pattern", which is now problematic and can be completely removed.
• ReexecDisabled() actually hid another issue with interceptors not working in unit tests. I added an explicit verify_interceptors (defaults to true) option instead.

[rsundahl]

This is great. The interceptors are either installed correctly and the environment variable stripped for any possible children or it's an error (with useful instructions). This also removes the cumbersome statefulness of the process and the use of so-called strong-over-weak symbol overrides.

[protze.joachim]

This change bricks the old tsan runtime. Should we remove the call from rtl-old as well? Or is rtl-old not maintained at all?

[yln]

In D129157#3806107, @protze.joachim wrote:

This change bricks the old tsan runtime. Should we remove the call from rtl-old as well? Or is rtl-old not maintained at all?

Yes, this was on oversight: https://reviews.llvm.org/D134389

Are you still using the old TSan runtime? @dvyukov what is the plan/timeline for removing it?

[dvyukov]

In D129157#3807047, @yln wrote:

In D129157#3806107, @protze.joachim wrote:

This change bricks the old tsan runtime. Should we remove the call from rtl-old as well? Or is rtl-old not maintained at all?

Yes, this was on oversight: https://reviews.llvm.org/D134389

Are you still using the old TSan runtime? @dvyukov what is the plan/timeline for removing it?

I dunno. As far as I know only @thakis used it. Nico, is it still needed?

[yln]

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

1012

It looks like __sanitizer_report_error_summary isn't a good function to use here, since it can be overridden by the client.

compiler-rt/lib/asan/asan_rtl.cpp

static void AsanInitInternal() {
 
   __sanitizer_set_report_path(common_flags()->log_path);
 
   __asan_option_detect_stack_use_after_return =
       flags()->detect_stack_use_after_return;
 
   __sanitizer::InitializePlatformEarly();
 
-  // Re-exec ourselves if we need to set additional env or command line args.
-  MaybeReexec();
-
   // Setup internal allocator callback.
   SetLowLevelAllocateMinAlignment(ASAN_SHADOW_GRANULARITY);
   SetLowLevelAllocateCallback(OnLowLevelAllocate);
 
   InitializeAsanInterceptors();
   CheckASLR();
 
   // Enable system log ("adb logcat") on Android.

compiler-rt/lib/asan/tests/asan_test_main.cpp

#elif SANITIZER_SUPPRESS_LEAK_ON_PTHREAD_EXIT
   return "";
 #else
   // Let's turn symbolization off to speed up testing (more than 3 times speedup
   // observed).
   return "symbolize=false";
 #endif
 }
 
-namespace __sanitizer {
-bool ReexecDisabled() {
-#if __has_feature(address_sanitizer) && SANITIZER_APPLE
-  // Allow re-exec in instrumented unit tests on Darwin.  Technically, we only
-  // need this for 10.10 and below, where re-exec is required for the
-  // interceptors to work, but to avoid duplicating the version detection logic,
-  // let's just allow re-exec for all Darwin versions.  On newer OS versions,
-  // returning 'false' doesn't do anything anyway, because we don't re-exec.
-  return false;
-#else
-  return true;
-#endif
-}
-}  // namespace __sanitizer
-
 int main(int argc, char **argv) {
   testing::GTEST_FLAG(death_test_style) = "threadsafe";
   testing::InitGoogleTest(&argc, argv);
   return RUN_ALL_TESTS();
 }

compiler-rt/lib/memprof/memprof_rtl.cpp

static void MemprofInitInternal() {
   // been overrriden by a flag at runtime.
   if (__memprof_profile_filename[0] != 0 && !common_flags()->log_path)
     __sanitizer_set_report_path(__memprof_profile_filename);
   else
     __sanitizer_set_report_path(common_flags()->log_path);
 
   __sanitizer::InitializePlatformEarly();
 
-  // Re-exec ourselves if we need to set additional env or command line args.
-  MaybeReexec();
-
   // Setup internal allocator callback.
   SetLowLevelAllocateMinAlignment(SHADOW_GRANULARITY);
 
   InitializeMemprofInterceptors();
   CheckASLR();
 
   ReplaceSystemMalloc();
 

compiler-rt/lib/sanitizer_common/sanitizer_common.h

private:
   void InitPcSpBp();
   uptr GetAddress() const;
   WriteFlag GetWriteFlag() const;
   bool IsMemoryAccess() const;
   bool IsTrueFaultingAddress() const;
 };
 
 void InitializePlatformEarly();
-void MaybeReexec();
 
 template <typename Fn>
 class RunOnDestruction {
  public:
   explicit RunOnDestruction(Fn fn) : fn_(fn) {}
   ~RunOnDestruction() { fn_(); }
 
  private:

compiler-rt/lib/sanitizer_common/sanitizer_flags.inc

COMMON_FLAG(const char *, log_suffix, nullptr,
             "String to append to log file name, e.g. \".txt\".")
 COMMON_FLAG(
     bool, log_to_syslog, (bool)SANITIZER_ANDROID || (bool)SANITIZER_APPLE,
     "Write all sanitizer output to syslog in addition to other means of "
     "logging.")
 COMMON_FLAG(
     int, verbosity, 0,
     "Verbosity level (0 - silent, 1 - a bit of output, 2+ - more output).")
-COMMON_FLAG(bool, strip_env, 1,
+COMMON_FLAG(bool, strip_env, true,
             "Whether to remove the sanitizer from DYLD_INSERT_LIBRARIES to "
-            "avoid passing it to children. Default is true.")
+            "avoid passing it to children on Apple platforms. Default is true.")
+COMMON_FLAG(bool, verify_interceptors, true,
+            "Verify that interceptors are working on Apple platforms. Default "
+            "is true.")
 COMMON_FLAG(bool, detect_leaks, !SANITIZER_APPLE, "Enable memory leak detection.")
 COMMON_FLAG(
     bool, leak_check_at_exit, true,
     "Invoke leak checking in an atexit handler. Has no effect if "
     "detect_leaks=false, or if __lsan_do_leak_check() is called before the "
     "handler has a chance to run.")
 COMMON_FLAG(bool, allocator_may_return_null, false,
             "If false, the allocator will crash instead of returning 0 on "

compiler-rt/lib/sanitizer_common/sanitizer_fuchsia.cpp

void GetThreadStackTopAndBottom(bool, uptr *stack_top, uptr *stack_bottom) {
   CHECK_EQ(pthread_attr_getstack(&attr, &base, &size), 0);
   CHECK_EQ(pthread_attr_destroy(&attr), 0);
 
   *stack_bottom = reinterpret_cast<uptr>(base);
   *stack_top = *stack_bottom + size;
 }
 
 void InitializePlatformEarly() {}
-void MaybeReexec() {}
 void CheckASLR() {}
 void CheckMPROTECT() {}
 void PlatformPrepareForSandboxing(void *args) {}
 void DisableCoreDumperIfNecessary() {}
 void InstallDeadlySignalHandlers(SignalHandlerType handler) {}
 void SetAlternateSignalStack() {}
 void UnsetAlternateSignalStack() {}
 void InitTlsSize() {}

compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp

 }
 
 void SignalContext::InitPcSpBp() { GetPcSpBp(context, &pc, &sp, &bp); }
 
 void InitializePlatformEarly() {
   // Do nothing.
 }
 
-void MaybeReexec() {
-  // No need to re-exec on Linux.
-}
-
 void CheckASLR() {
 #if SANITIZER_NETBSD
   int mib[3];
   int paxflags;
   uptr len = sizeof(paxflags);
 
   mib[0] = CTL_PROC;
   mib[1] = internal_getpid();

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

using task_set_exc_guard_behavior_t =
       kern_return_t(task_t task, task_exc_guard_behavior_t behavior);
   auto *set_behavior = (task_set_exc_guard_behavior_t *)dlsym(
       RTLD_DEFAULT, "task_set_exc_guard_behavior");
   if (set_behavior == nullptr) return;
   const task_exc_guard_behavior_t task_exc_guard_none = 0;
   set_behavior(mach_task_self(), task_exc_guard_none);
 }
 
+static void VerifyInterceptorsWorking();
+static void StripEnv();
+
 void InitializePlatformEarly() {
   // Only use xnu_fast_mmap when on x86_64 and the kernel supports it.
   use_xnu_fast_mmap =
 #if defined(__x86_64__)
       GetDarwinKernelVersion() >= DarwinKernelVersion(17, 5);
 #else
       false;
 #endif
   if (GetDarwinKernelVersion() >= DarwinKernelVersion(19, 0))
     DisableMmapExcGuardExceptions();
+
+#  if !SANITIZER_GO
+  MonotonicNanoTime();  // Call to initialize mach_timebase_info
+  VerifyInterceptorsWorking();
+  StripEnv();
+#  endif
 }
 
 #if !SANITIZER_GO
 static const char kDyldInsertLibraries[] = "DYLD_INSERT_LIBRARIES";
 LowLevelAllocator allocator_for_env;
 
+static void VerifyInterceptorsWorking() {
+  if (!common_flags()->verify_interceptors)
+    return;
+
+  // Verify that interceptors really work.  We'll use dlsym to locate
+  // "puts", if interceptors are working, it should really point to
+  // "wrap_puts" within our own dylib.
+  Dl_info info_puts, info_runtime;
+  RAW_CHECK(dladdr(dlsym(RTLD_DEFAULT, "puts"), &info_puts));
+  RAW_CHECK(dladdr((void *)__sanitizer_report_error_summary, &info_runtime));
+  if (internal_strcmp(info_puts.dli_fname, info_runtime.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_runtime.dli_fname);
+    RAW_CHECK("interceptors not installed" && 0);
+  }
+}
+
 // Change the value of the env var |name|, leaking the original value.
 // If |name_value| is NULL, the variable is deleted from the environment,
 // otherwise the corresponding "NAME=value" string is replaced with
 // |name_value|.
-void LeakyResetEnv(const char *name, const char *name_value) {
+static void LeakyResetEnv(const char *name, const char *name_value) {
   char **env = GetEnviron();
   uptr name_len = internal_strlen(name);
   while (*env != 0) {
     uptr len = internal_strlen(*env);
     if (len > name_len) {
       const char *p = *env;
       if (!internal_memcmp(p, name, name_len) && p[name_len] == '=') {
         // Match.
         if (name_value) {
           // Replace the old value with the new one.
           *env = const_cast<char*>(name_value);
         } else {
           // Shift the subsequent pointers back.
           char **del = env;
           do {
             del[0] = del[1];
           } while (*del++);
         }
       }
     }
     env++;
   }
 }
 
-SANITIZER_WEAK_CXX_DEFAULT_IMPL
-bool ReexecDisabled() {
-  return false;
-}
-
-static bool DyldNeedsEnvVariable() {
-  // If running on OS X 10.11+ or iOS 9.0+, dyld will interpose even if
-  // DYLD_INSERT_LIBRARIES is not set.
-  return GetMacosAlignedVersion() < MacosVersion(10, 11);
-}
-
-void MaybeReexec() {
-  // FIXME: This should really live in some "InitializePlatform" method.
-  MonotonicNanoTime();
-
-  if (ReexecDisabled()) return;
+static void StripEnv() {
+  if (!common_flags()->strip_env)
+    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;
-  RAW_CHECK(dladdr((void*)((uptr)&__sanitizer_report_error_summary), &info));
   char *dyld_insert_libraries =
-      const_cast<char*>(GetEnv(kDyldInsertLibraries));
-  uptr old_env_len = dyld_insert_libraries ?
-      internal_strlen(dyld_insert_libraries) : 0;
-  uptr fname_len = internal_strlen(info.dli_fname);
-  const char *dylib_name = StripModuleName(info.dli_fname);
-  uptr dylib_name_len = internal_strlen(dylib_name);
-
-  bool lib_is_in_env = dyld_insert_libraries &&
-                       internal_strstr(dyld_insert_libraries, dylib_name);
-  if (DyldNeedsEnvVariable() && !lib_is_in_env) {
-    // DYLD_INSERT_LIBRARIES is not set or does not contain the runtime
-    // library.
-    InternalMmapVector<char> program_name(1024);
-    uint32_t buf_size = program_name.size();
-    _NSGetExecutablePath(program_name.data(), &buf_size);
-    char *new_env = const_cast<char*>(info.dli_fname);
-    if (dyld_insert_libraries) {
-      // Append the runtime dylib name to the existing value of
-      // DYLD_INSERT_LIBRARIES.
-      new_env = (char*)allocator_for_env.Allocate(old_env_len + fname_len + 2);
-      internal_strncpy(new_env, dyld_insert_libraries, old_env_len);
-      new_env[old_env_len] = ':';
-      // Copy fname_len and add a trailing zero.
-      internal_strncpy(new_env + old_env_len + 1, info.dli_fname,
-                       fname_len + 1);
-      // Ok to use setenv() since the wrappers don't depend on the value of
-      // asan_inited.
-      setenv(kDyldInsertLibraries, new_env, /*overwrite*/1);
-    } else {
-      // Set DYLD_INSERT_LIBRARIES equal to the runtime dylib name.
-      setenv(kDyldInsertLibraries, info.dli_fname, /*overwrite*/0);
-    }
-    VReport(1, "exec()-ing the program with\n");
-    VReport(1, "%s=%s\n", kDyldInsertLibraries, new_env);
-    VReport(1, "to enable wrappers.\n");
-    execv(program_name.data(), *_NSGetArgv());
-
-    // We get here only if execv() failed.
-    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);
-    RAW_CHECK("execv failed" && 0);
-  }
-
-  // Verify that interceptors really work.  We'll use dlsym to locate
-  // "puts", if interceptors are working, it should really point to
-  // "wrap_puts" within our own dylib.
-  Dl_info info_puts;
-  void *dlopen_addr = dlsym(RTLD_DEFAULT, "puts");
-  RAW_CHECK(dladdr(dlopen_addr, &info_puts));
-  if (internal_strcmp(info.dli_fname, info_puts.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);
-    RAW_CHECK("interceptors not installed" && 0);
-  }
-
-  if (!lib_is_in_env)
+      const_cast<char *>(GetEnv(kDyldInsertLibraries));
+  if (!dyld_insert_libraries)
     return;
 
-  if (!common_flags()->strip_env)
+  Dl_info info;
+  RAW_CHECK(dladdr((void *)__sanitizer_report_error_summary, &info));
+  const char *dylib_name = StripModuleName(info.dli_fname);
+  bool lib_is_in_env = internal_strstr(dyld_insert_libraries, dylib_name);
+  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 old_env_len = internal_strlen(dyld_insert_libraries);
+  uptr dylib_name_len = internal_strlen(dylib_name);
   uptr env_name_len = internal_strlen(kDyldInsertLibraries);
   // Allocate memory to hold the previous env var name, its value, the '='
   // sign and the '\0' char.
   char *new_env = (char*)allocator_for_env.Allocate(
       old_env_len + 2 + env_name_len);
   RAW_CHECK(new_env);
   internal_memset(new_env, '\0', old_env_len + 2 + env_name_len);
   internal_strncpy(new_env, kDyldInsertLibraries, env_name_len);

compiler-rt/lib/sanitizer_common/sanitizer_win.cpp

 void CheckVMASize() {
   // Do nothing.
 }
 
 void InitializePlatformEarly() {
   // Do nothing.
 }
 
-void MaybeReexec() {
-  // No need to re-exec on Windows.
-}
-
 void CheckASLR() {
   // Do nothing
 }
 
 void CheckMPROTECT() {
   // Do nothing
 }
 

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

void Initialize(ThreadState *thr) {
   CacheBinaryName();
   CheckASLR();
   InitializeFlags(&ctx->flags, options, env_name);
   AvoidCVE_2016_2143();
   __sanitizer::InitializePlatformEarly();
   __tsan::InitializePlatformEarly();
 
 #if !SANITIZER_GO
-  // Re-exec ourselves if we need to set additional env or command line args.
-  MaybeReexec();
-
   InitializeAllocator();
   ReplaceSystemMalloc();
 #endif
   if (common_flags()->detect_deadlocks)
     ctx->dd = DDetector::Create(flags());
   Processor *proc = ProcCreate();
   ProcWire(proc, thr);
   InitializeInterceptors();

compiler-rt/lib/tsan/tests/rtl/tsan_test.cpp

 
 #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/lib/tsan/tests/unit/tsan_unit_test_main.cpp

 //===-- tsan_unit_test_main.cpp -------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // 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/test/asan/TestCases/Darwin/init_for_dlopen.cpp

 // environment variable.
 // RUN: env APPLE_ASAN_INIT_FOR_DLOPEN=1 %run %t %shared_libasan 2>&1 | \
 // RUN:   FileCheck -check-prefix=CHECK-DL-OPEN-SUCCESS %s
 
 #include <dlfcn.h>
 #include <stdio.h>
 
 // CHECK-DL-OPEN-FAIL: ERROR: Interceptors are not working
+// CHECK-SAME-DL-OPEN-FAIL: Please launch the executable with: DYLD_INSERT_LIBRARIES={{.+}}/libclang_rt.asan_{{.+}}_dynamic.dylib
 
 int main(int argc, char **argv) {
   if (argc != 2) {
     fprintf(stderr, "Usage: %s <dylib_path>\n", argv[0]);
     return 1;
   }
   const char *dylib_path = argv[1];
   void *handle = dlopen(dylib_path, RTLD_LAZY);

compiler-rt/unittests/lit.common.unit.cfg.py

 if 'TEMP' in os.environ:
     config.environment['TEMP'] = os.environ['TEMP']
 
 if config.host_os == 'Darwin':
   # Only run up to 3 processes that require shadow memory simultaneously on
   # 64-bit Darwin. Using more scales badly and hogs the system due to
   # inefficient handling of large mmap'd regions (terabytes) by the kernel.
   lit_config.parallelism_groups["shadow-memory"] = 3
-  # Disable libmalloc nanoallocator due to crashes running on macOS 12.0.
-  #
+
+  # Disable libmalloc nano allocator due to crashes running on macOS 12.0.
   # rdar://80086125
   config.environment['MallocNanoZone'] = '0'
+
+  # We crash when we set DYLD_INSERT_LIBRARIES for unit tests, so interceptors
+  # don't work.
+  config.environment['ASAN_OPTIONS'] = 'verify_interceptors=0'
+  config.environment['TSAN_OPTIONS'] = 'verify_interceptors=0'