[compiler-rt] ca50840 - [Sanitizer][Darwin] Cleanup MaybeReexec() function and usage
Julian Lettner via llvm-commits
llvm-commits at lists.llvm.org
Fri Jul 8 14:31:47 PDT 2022
Author: Julian Lettner
Date: 2022-07-08T14:31:42-07:00
New Revision: ca50840b5bc0679e824850ca4fd322504d6713a0
URL: https://github.com/llvm/llvm-project/commit/ca50840b5bc0679e824850ca4fd322504d6713a0
DIFF: https://github.com/llvm/llvm-project/commit/ca50840b5bc0679e824850ca4fd322504d6713a0.diff
LOG: [Sanitizer][Darwin] Cleanup MaybeReexec() function and usage
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.
Differential Revision: https://reviews.llvm.org/D129157
Added:
Modified:
compiler-rt/lib/asan/asan_rtl.cpp
compiler-rt/lib/asan/tests/asan_test_main.cpp
compiler-rt/lib/memprof/memprof_rtl.cpp
compiler-rt/lib/sanitizer_common/sanitizer_common.h
compiler-rt/lib/sanitizer_common/sanitizer_flags.inc
compiler-rt/lib/sanitizer_common/sanitizer_fuchsia.cpp
compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
compiler-rt/lib/sanitizer_common/sanitizer_win.cpp
compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
compiler-rt/lib/tsan/tests/rtl/tsan_test.cpp
compiler-rt/lib/tsan/tests/unit/tsan_unit_test_main.cpp
compiler-rt/test/asan/TestCases/Darwin/init_for_dlopen.cpp
compiler-rt/unittests/lit.common.unit.cfg.py
Removed:
################################################################################
diff --git a/compiler-rt/lib/asan/asan_rtl.cpp b/compiler-rt/lib/asan/asan_rtl.cpp
index 29cf526d9eb0b..2bbf0ac5240a7 100644
--- a/compiler-rt/lib/asan/asan_rtl.cpp
+++ b/compiler-rt/lib/asan/asan_rtl.cpp
@@ -421,9 +421,6 @@ static void AsanInitInternal() {
__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);
diff --git a/compiler-rt/lib/asan/tests/asan_test_main.cpp b/compiler-rt/lib/asan/tests/asan_test_main.cpp
index 64ac1b288e53c..136b752aebb0c 100644
--- a/compiler-rt/lib/asan/tests/asan_test_main.cpp
+++ b/compiler-rt/lib/asan/tests/asan_test_main.cpp
@@ -33,21 +33,6 @@ extern "C" const char* __asan_default_options() {
#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);
diff --git a/compiler-rt/lib/memprof/memprof_rtl.cpp b/compiler-rt/lib/memprof/memprof_rtl.cpp
index 21424fb4f0729..d568a075c3e18 100644
--- a/compiler-rt/lib/memprof/memprof_rtl.cpp
+++ b/compiler-rt/lib/memprof/memprof_rtl.cpp
@@ -170,9 +170,6 @@ static void MemprofInitInternal() {
__sanitizer::InitializePlatformEarly();
- // Re-exec ourselves if we need to set additional env or command line args.
- MaybeReexec();
-
// Setup internal allocator callback.
SetLowLevelAllocateMinAlignment(SHADOW_GRANULARITY);
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_common.h b/compiler-rt/lib/sanitizer_common/sanitizer_common.h
index 345c262af9728..517f776baf6ef 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_common.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_common.h
@@ -1016,7 +1016,6 @@ struct SignalContext {
};
void InitializePlatformEarly();
-void MaybeReexec();
template <typename Fn>
class RunOnDestruction {
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_flags.inc b/compiler-rt/lib/sanitizer_common/sanitizer_flags.inc
index 8627ffd0d01c2..6148ae56067ca 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_flags.inc
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_flags.inc
@@ -68,9 +68,12 @@ COMMON_FLAG(
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,
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_fuchsia.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_fuchsia.cpp
index e253b67fc484f..a92e84cb8ecf7 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_fuchsia.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_fuchsia.cpp
@@ -87,7 +87,6 @@ void GetThreadStackTopAndBottom(bool, uptr *stack_top, uptr *stack_bottom) {
}
void InitializePlatformEarly() {}
-void MaybeReexec() {}
void CheckASLR() {}
void CheckMPROTECT() {}
void PlatformPrepareForSandboxing(void *args) {}
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
index f3fd70af81564..86e2676a6acd1 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
@@ -2180,10 +2180,6 @@ void InitializePlatformEarly() {
// Do nothing.
}
-void MaybeReexec() {
- // No need to re-exec on Linux.
-}
-
void CheckASLR() {
#if SANITIZER_NETBSD
int mib[3];
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
index 327c7167dcf5e..7ce6eff832e54 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp
@@ -943,6 +943,9 @@ static void DisableMmapExcGuardExceptions() {
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 =
@@ -953,17 +956,54 @@ void InitializePlatformEarly() {
#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 bool ShouldCheckInterceptors() {
+ // Restrict "interceptors working?" check to ASan and TSan.
+ const char *sanitizer_names[] = {"AddressSanitizer", "ThreadSanitizer"};
+ size_t count = sizeof(sanitizer_names) / sizeof(sanitizer_names[0]);
+ for (size_t i = 0; i < count; i++) {
+ if (internal_strcmp(sanitizer_names[i], SanitizerToolName) == 0)
+ return true;
+ }
+ return false;
+}
+
+static void VerifyInterceptorsWorking() {
+ if (!common_flags()->verify_interceptors || !ShouldCheckInterceptors())
+ 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) {
@@ -988,100 +1028,28 @@ void LeakyResetEnv(const char *name, const char *name_value) {
}
}
-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();
+static void StripEnv() {
+ if (!common_flags()->strip_env)
+ return;
- if (ReexecDisabled()) return;
+ char *dyld_insert_libraries =
+ const_cast<char *>(GetEnv(kDyldInsertLibraries));
+ if (!dyld_insert_libraries)
+ 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);
+ RAW_CHECK(dladdr((void *)__sanitizer_report_error_summary, &info));
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);
- }
-
+ bool lib_is_in_env = internal_strstr(dyld_insert_libraries, dylib_name);
if (!lib_is_in_env)
return;
- if (!common_flags()->strip_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.
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_win.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_win.cpp
index c997514cfed7f..b4506e52efaaf 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_win.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_win.cpp
@@ -1094,10 +1094,6 @@ void InitializePlatformEarly() {
// Do nothing.
}
-void MaybeReexec() {
- // No need to re-exec on Windows.
-}
-
void CheckASLR() {
// Do nothing
}
diff --git a/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp b/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
index 3977d60c36e54..607f373871b44 100644
--- a/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
+++ b/compiler-rt/lib/tsan/rtl/tsan_rtl.cpp
@@ -651,9 +651,6 @@ void Initialize(ThreadState *thr) {
__tsan::InitializePlatformEarly();
#if !SANITIZER_GO
- // Re-exec ourselves if we need to set additional env or command line args.
- MaybeReexec();
-
InitializeAllocator();
ReplaceSystemMalloc();
#endif
diff --git a/compiler-rt/lib/tsan/tests/rtl/tsan_test.cpp b/compiler-rt/lib/tsan/tests/rtl/tsan_test.cpp
index 594c3ebfeb738..477a41d98c1d2 100644
--- a/compiler-rt/lib/tsan/tests/rtl/tsan_test.cpp
+++ b/compiler-rt/lib/tsan/tests/rtl/tsan_test.cpp
@@ -53,12 +53,6 @@ extern "C" const char* __tsan_default_options() {
}
#endif
-namespace __sanitizer {
-bool ReexecDisabled() {
- return true;
-}
-}
-
int main(int argc, char **argv) {
argv0 = argv[0];
return run_tests(argc, argv);
diff --git a/compiler-rt/lib/tsan/tests/unit/tsan_unit_test_main.cpp b/compiler-rt/lib/tsan/tests/unit/tsan_unit_test_main.cpp
index 0e7b18a2ab3d3..fb01d3e923aa8 100644
--- a/compiler-rt/lib/tsan/tests/unit/tsan_unit_test_main.cpp
+++ b/compiler-rt/lib/tsan/tests/unit/tsan_unit_test_main.cpp
@@ -11,12 +11,6 @@
//===----------------------------------------------------------------------===//
#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);
diff --git a/compiler-rt/test/asan/TestCases/Darwin/init_for_dlopen.cpp b/compiler-rt/test/asan/TestCases/Darwin/init_for_dlopen.cpp
index 8a0fbf943b672..0107f30dd7d9b 100644
--- a/compiler-rt/test/asan/TestCases/Darwin/init_for_dlopen.cpp
+++ b/compiler-rt/test/asan/TestCases/Darwin/init_for_dlopen.cpp
@@ -21,6 +21,7 @@
#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) {
diff --git a/compiler-rt/unittests/lit.common.unit.cfg.py b/compiler-rt/unittests/lit.common.unit.cfg.py
index c72b06d6b5f31..dd6b5bab5b65d 100644
--- a/compiler-rt/unittests/lit.common.unit.cfg.py
+++ b/compiler-rt/unittests/lit.common.unit.cfg.py
@@ -46,7 +46,12 @@ def get_lit_conf(name, default=None):
# 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'
More information about the llvm-commits
mailing list