Skip to content

Commit ca50840

Browse files
author
Julian Lettner
committed
[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
1 parent 24849c9 commit ca50840

14 files changed

+65
-134
lines changed

compiler-rt/lib/asan/asan_rtl.cpp

-3
Original file line numberDiff line numberDiff line change
@@ -421,9 +421,6 @@ static void AsanInitInternal() {
421421

422422
__sanitizer::InitializePlatformEarly();
423423

424-
// Re-exec ourselves if we need to set additional env or command line args.
425-
MaybeReexec();
426-
427424
// Setup internal allocator callback.
428425
SetLowLevelAllocateMinAlignment(ASAN_SHADOW_GRANULARITY);
429426
SetLowLevelAllocateCallback(OnLowLevelAllocate);

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

-15
Original file line numberDiff line numberDiff line change
@@ -33,21 +33,6 @@ extern "C" const char* __asan_default_options() {
3333
#endif
3434
}
3535

36-
namespace __sanitizer {
37-
bool ReexecDisabled() {
38-
#if __has_feature(address_sanitizer) && SANITIZER_APPLE
39-
// Allow re-exec in instrumented unit tests on Darwin. Technically, we only
40-
// need this for 10.10 and below, where re-exec is required for the
41-
// interceptors to work, but to avoid duplicating the version detection logic,
42-
// let's just allow re-exec for all Darwin versions. On newer OS versions,
43-
// returning 'false' doesn't do anything anyway, because we don't re-exec.
44-
return false;
45-
#else
46-
return true;
47-
#endif
48-
}
49-
} // namespace __sanitizer
50-
5136
int main(int argc, char **argv) {
5237
testing::GTEST_FLAG(death_test_style) = "threadsafe";
5338
testing::InitGoogleTest(&argc, argv);

compiler-rt/lib/memprof/memprof_rtl.cpp

-3
Original file line numberDiff line numberDiff line change
@@ -170,9 +170,6 @@ static void MemprofInitInternal() {
170170

171171
__sanitizer::InitializePlatformEarly();
172172

173-
// Re-exec ourselves if we need to set additional env or command line args.
174-
MaybeReexec();
175-
176173
// Setup internal allocator callback.
177174
SetLowLevelAllocateMinAlignment(SHADOW_GRANULARITY);
178175

compiler-rt/lib/sanitizer_common/sanitizer_common.h

-1
Original file line numberDiff line numberDiff line change
@@ -1016,7 +1016,6 @@ struct SignalContext {
10161016
};
10171017

10181018
void InitializePlatformEarly();
1019-
void MaybeReexec();
10201019

10211020
template <typename Fn>
10221021
class RunOnDestruction {

compiler-rt/lib/sanitizer_common/sanitizer_flags.inc

+5-2
Original file line numberDiff line numberDiff line change
@@ -68,9 +68,12 @@ COMMON_FLAG(
6868
COMMON_FLAG(
6969
int, verbosity, 0,
7070
"Verbosity level (0 - silent, 1 - a bit of output, 2+ - more output).")
71-
COMMON_FLAG(bool, strip_env, 1,
71+
COMMON_FLAG(bool, strip_env, true,
7272
"Whether to remove the sanitizer from DYLD_INSERT_LIBRARIES to "
73-
"avoid passing it to children. Default is true.")
73+
"avoid passing it to children on Apple platforms. Default is true.")
74+
COMMON_FLAG(bool, verify_interceptors, true,
75+
"Verify that interceptors are working on Apple platforms. Default "
76+
"is true.")
7477
COMMON_FLAG(bool, detect_leaks, !SANITIZER_APPLE, "Enable memory leak detection.")
7578
COMMON_FLAG(
7679
bool, leak_check_at_exit, true,

compiler-rt/lib/sanitizer_common/sanitizer_fuchsia.cpp

-1
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,6 @@ void GetThreadStackTopAndBottom(bool, uptr *stack_top, uptr *stack_bottom) {
8787
}
8888

8989
void InitializePlatformEarly() {}
90-
void MaybeReexec() {}
9190
void CheckASLR() {}
9291
void CheckMPROTECT() {}
9392
void PlatformPrepareForSandboxing(void *args) {}

compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp

-4
Original file line numberDiff line numberDiff line change
@@ -2180,10 +2180,6 @@ void InitializePlatformEarly() {
21802180
// Do nothing.
21812181
}
21822182

2183-
void MaybeReexec() {
2184-
// No need to re-exec on Linux.
2185-
}
2186-
21872183
void CheckASLR() {
21882184
#if SANITIZER_NETBSD
21892185
int mib[3];

compiler-rt/lib/sanitizer_common/sanitizer_mac.cpp

+52-84
Original file line numberDiff line numberDiff line change
@@ -943,6 +943,9 @@ static void DisableMmapExcGuardExceptions() {
943943
set_behavior(mach_task_self(), task_exc_guard_none);
944944
}
945945

946+
static void VerifyInterceptorsWorking();
947+
static void StripEnv();
948+
946949
void InitializePlatformEarly() {
947950
// Only use xnu_fast_mmap when on x86_64 and the kernel supports it.
948951
use_xnu_fast_mmap =
@@ -953,17 +956,54 @@ void InitializePlatformEarly() {
953956
#endif
954957
if (GetDarwinKernelVersion() >= DarwinKernelVersion(19, 0))
955958
DisableMmapExcGuardExceptions();
959+
960+
# if !SANITIZER_GO
961+
MonotonicNanoTime(); // Call to initialize mach_timebase_info
962+
VerifyInterceptorsWorking();
963+
StripEnv();
964+
# endif
956965
}
957966

958967
#if !SANITIZER_GO
959968
static const char kDyldInsertLibraries[] = "DYLD_INSERT_LIBRARIES";
960969
LowLevelAllocator allocator_for_env;
961970

971+
static bool ShouldCheckInterceptors() {
972+
// Restrict "interceptors working?" check to ASan and TSan.
973+
const char *sanitizer_names[] = {"AddressSanitizer", "ThreadSanitizer"};
974+
size_t count = sizeof(sanitizer_names) / sizeof(sanitizer_names[0]);
975+
for (size_t i = 0; i < count; i++) {
976+
if (internal_strcmp(sanitizer_names[i], SanitizerToolName) == 0)
977+
return true;
978+
}
979+
return false;
980+
}
981+
982+
static void VerifyInterceptorsWorking() {
983+
if (!common_flags()->verify_interceptors || !ShouldCheckInterceptors())
984+
return;
985+
986+
// Verify that interceptors really work. We'll use dlsym to locate
987+
// "puts", if interceptors are working, it should really point to
988+
// "wrap_puts" within our own dylib.
989+
Dl_info info_puts, info_runtime;
990+
RAW_CHECK(dladdr(dlsym(RTLD_DEFAULT, "puts"), &info_puts));
991+
RAW_CHECK(dladdr((void *)__sanitizer_report_error_summary, &info_runtime));
992+
if (internal_strcmp(info_puts.dli_fname, info_runtime.dli_fname) != 0) {
993+
Report(
994+
"ERROR: Interceptors are not working. This may be because %s is "
995+
"loaded too late (e.g. via dlopen). Please launch the executable "
996+
"with:\n%s=%s\n",
997+
SanitizerToolName, kDyldInsertLibraries, info_runtime.dli_fname);
998+
RAW_CHECK("interceptors not installed" && 0);
999+
}
1000+
}
1001+
9621002
// Change the value of the env var |name|, leaking the original value.
9631003
// If |name_value| is NULL, the variable is deleted from the environment,
9641004
// otherwise the corresponding "NAME=value" string is replaced with
9651005
// |name_value|.
966-
void LeakyResetEnv(const char *name, const char *name_value) {
1006+
static void LeakyResetEnv(const char *name, const char *name_value) {
9671007
char **env = GetEnviron();
9681008
uptr name_len = internal_strlen(name);
9691009
while (*env != 0) {
@@ -988,100 +1028,28 @@ void LeakyResetEnv(const char *name, const char *name_value) {
9881028
}
9891029
}
9901030

991-
SANITIZER_WEAK_CXX_DEFAULT_IMPL
992-
bool ReexecDisabled() {
993-
return false;
994-
}
995-
996-
static bool DyldNeedsEnvVariable() {
997-
// If running on OS X 10.11+ or iOS 9.0+, dyld will interpose even if
998-
// DYLD_INSERT_LIBRARIES is not set.
999-
return GetMacosAlignedVersion() < MacosVersion(10, 11);
1000-
}
1001-
1002-
void MaybeReexec() {
1003-
// FIXME: This should really live in some "InitializePlatform" method.
1004-
MonotonicNanoTime();
1031+
static void StripEnv() {
1032+
if (!common_flags()->strip_env)
1033+
return;
10051034

1006-
if (ReexecDisabled()) return;
1035+
char *dyld_insert_libraries =
1036+
const_cast<char *>(GetEnv(kDyldInsertLibraries));
1037+
if (!dyld_insert_libraries)
1038+
return;
10071039

1008-
// Make sure the dynamic runtime library is preloaded so that the
1009-
// wrappers work. If it is not, set DYLD_INSERT_LIBRARIES and re-exec
1010-
// ourselves.
10111040
Dl_info info;
1012-
RAW_CHECK(dladdr((void*)((uptr)&__sanitizer_report_error_summary), &info));
1013-
char *dyld_insert_libraries =
1014-
const_cast<char*>(GetEnv(kDyldInsertLibraries));
1015-
uptr old_env_len = dyld_insert_libraries ?
1016-
internal_strlen(dyld_insert_libraries) : 0;
1017-
uptr fname_len = internal_strlen(info.dli_fname);
1041+
RAW_CHECK(dladdr((void *)__sanitizer_report_error_summary, &info));
10181042
const char *dylib_name = StripModuleName(info.dli_fname);
1019-
uptr dylib_name_len = internal_strlen(dylib_name);
1020-
1021-
bool lib_is_in_env = dyld_insert_libraries &&
1022-
internal_strstr(dyld_insert_libraries, dylib_name);
1023-
if (DyldNeedsEnvVariable() && !lib_is_in_env) {
1024-
// DYLD_INSERT_LIBRARIES is not set or does not contain the runtime
1025-
// library.
1026-
InternalMmapVector<char> program_name(1024);
1027-
uint32_t buf_size = program_name.size();
1028-
_NSGetExecutablePath(program_name.data(), &buf_size);
1029-
char *new_env = const_cast<char*>(info.dli_fname);
1030-
if (dyld_insert_libraries) {
1031-
// Append the runtime dylib name to the existing value of
1032-
// DYLD_INSERT_LIBRARIES.
1033-
new_env = (char*)allocator_for_env.Allocate(old_env_len + fname_len + 2);
1034-
internal_strncpy(new_env, dyld_insert_libraries, old_env_len);
1035-
new_env[old_env_len] = ':';
1036-
// Copy fname_len and add a trailing zero.
1037-
internal_strncpy(new_env + old_env_len + 1, info.dli_fname,
1038-
fname_len + 1);
1039-
// Ok to use setenv() since the wrappers don't depend on the value of
1040-
// asan_inited.
1041-
setenv(kDyldInsertLibraries, new_env, /*overwrite*/1);
1042-
} else {
1043-
// Set DYLD_INSERT_LIBRARIES equal to the runtime dylib name.
1044-
setenv(kDyldInsertLibraries, info.dli_fname, /*overwrite*/0);
1045-
}
1046-
VReport(1, "exec()-ing the program with\n");
1047-
VReport(1, "%s=%s\n", kDyldInsertLibraries, new_env);
1048-
VReport(1, "to enable wrappers.\n");
1049-
execv(program_name.data(), *_NSGetArgv());
1050-
1051-
// We get here only if execv() failed.
1052-
Report("ERROR: The process is launched without DYLD_INSERT_LIBRARIES, "
1053-
"which is required for the sanitizer to work. We tried to set the "
1054-
"environment variable and re-execute itself, but execv() failed, "
1055-
"possibly because of sandbox restrictions. Make sure to launch the "
1056-
"executable with:\n%s=%s\n", kDyldInsertLibraries, new_env);
1057-
RAW_CHECK("execv failed" && 0);
1058-
}
1059-
1060-
// Verify that interceptors really work. We'll use dlsym to locate
1061-
// "puts", if interceptors are working, it should really point to
1062-
// "wrap_puts" within our own dylib.
1063-
Dl_info info_puts;
1064-
void *dlopen_addr = dlsym(RTLD_DEFAULT, "puts");
1065-
RAW_CHECK(dladdr(dlopen_addr, &info_puts));
1066-
if (internal_strcmp(info.dli_fname, info_puts.dli_fname) != 0) {
1067-
Report(
1068-
"ERROR: Interceptors are not working. This may be because %s is "
1069-
"loaded too late (e.g. via dlopen). Please launch the executable "
1070-
"with:\n%s=%s\n",
1071-
SanitizerToolName, kDyldInsertLibraries, info.dli_fname);
1072-
RAW_CHECK("interceptors not installed" && 0);
1073-
}
1074-
1043+
bool lib_is_in_env = internal_strstr(dyld_insert_libraries, dylib_name);
10751044
if (!lib_is_in_env)
10761045
return;
10771046

1078-
if (!common_flags()->strip_env)
1079-
return;
1080-
10811047
// DYLD_INSERT_LIBRARIES is set and contains the runtime library. Let's remove
10821048
// the dylib from the environment variable, because interceptors are installed
10831049
// and we don't want our children to inherit the variable.
10841050

1051+
uptr old_env_len = internal_strlen(dyld_insert_libraries);
1052+
uptr dylib_name_len = internal_strlen(dylib_name);
10851053
uptr env_name_len = internal_strlen(kDyldInsertLibraries);
10861054
// Allocate memory to hold the previous env var name, its value, the '='
10871055
// sign and the '\0' char.

compiler-rt/lib/sanitizer_common/sanitizer_win.cpp

-4
Original file line numberDiff line numberDiff line change
@@ -1094,10 +1094,6 @@ void InitializePlatformEarly() {
10941094
// Do nothing.
10951095
}
10961096

1097-
void MaybeReexec() {
1098-
// No need to re-exec on Windows.
1099-
}
1100-
11011097
void CheckASLR() {
11021098
// Do nothing
11031099
}

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

-3
Original file line numberDiff line numberDiff line change
@@ -651,9 +651,6 @@ void Initialize(ThreadState *thr) {
651651
__tsan::InitializePlatformEarly();
652652

653653
#if !SANITIZER_GO
654-
// Re-exec ourselves if we need to set additional env or command line args.
655-
MaybeReexec();
656-
657654
InitializeAllocator();
658655
ReplaceSystemMalloc();
659656
#endif

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

-6
Original file line numberDiff line numberDiff line change
@@ -53,12 +53,6 @@ extern "C" const char* __tsan_default_options() {
5353
}
5454
#endif
5555

56-
namespace __sanitizer {
57-
bool ReexecDisabled() {
58-
return true;
59-
}
60-
}
61-
6256
int main(int argc, char **argv) {
6357
argv0 = argv[0];
6458
return run_tests(argc, argv);

compiler-rt/lib/tsan/tests/unit/tsan_unit_test_main.cpp

-6
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,6 @@
1111
//===----------------------------------------------------------------------===//
1212
#include "gtest/gtest.h"
1313

14-
namespace __sanitizer {
15-
bool ReexecDisabled() {
16-
return true;
17-
}
18-
}
19-
2014
int main(int argc, char **argv) {
2115
testing::GTEST_FLAG(death_test_style) = "threadsafe";
2216
testing::InitGoogleTest(&argc, argv);

compiler-rt/test/asan/TestCases/Darwin/init_for_dlopen.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
#include <stdio.h>
2222

2323
// CHECK-DL-OPEN-FAIL: ERROR: Interceptors are not working
24+
// CHECK-SAME-DL-OPEN-FAIL: Please launch the executable with: DYLD_INSERT_LIBRARIES={{.+}}/libclang_rt.asan_{{.+}}_dynamic.dylib
2425

2526
int main(int argc, char **argv) {
2627
if (argc != 2) {

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

+7-2
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,12 @@ def get_lit_conf(name, default=None):
4646
# 64-bit Darwin. Using more scales badly and hogs the system due to
4747
# inefficient handling of large mmap'd regions (terabytes) by the kernel.
4848
lit_config.parallelism_groups["shadow-memory"] = 3
49-
# Disable libmalloc nanoallocator due to crashes running on macOS 12.0.
50-
#
49+
50+
# Disable libmalloc nano allocator due to crashes running on macOS 12.0.
5151
# rdar://80086125
5252
config.environment['MallocNanoZone'] = '0'
53+
54+
# We crash when we set DYLD_INSERT_LIBRARIES for unit tests, so interceptors
55+
# don't work.
56+
config.environment['ASAN_OPTIONS'] = 'verify_interceptors=0'
57+
config.environment['TSAN_OPTIONS'] = 'verify_interceptors=0'

0 commit comments

Comments
 (0)