diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp --- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp @@ -16,6 +16,7 @@ #include #include +#include #include #include #include @@ -50,6 +51,8 @@ return true; } +static const char kAtosMachPortEnvVar[] = "__check_mach_ports_lookup="; + class AtosSymbolizerProcess : public SymbolizerProcess { public: explicit AtosSymbolizerProcess(const char *path) @@ -66,8 +69,30 @@ // the call to GetArgV. internal_snprintf(pid_str_, sizeof(pid_str_), "%d", internal_getpid()); + const char *new_env_entry = nullptr; // Prepare the copy of the environment. - CopyEnv(GetEnviron()); + if (SANITIZER_IOSSIM) { + new_env_entry = &(kAtosMachPortEnvVar[0]); + } + uptr index_to_write_entry = CopyEnv(GetEnviron(), new_env_entry); + + if (SANITIZER_IOSSIM) { + // `atos` in the simulator is restricted in its ability to retrieve the + // task port for the target process (us) so we need to do extra work + // to pass our task port to it. + mach_port_t ports[]{mach_task_self()}; + kern_return_t ret = + mach_ports_register(mach_task_self(), ports, /*count=*/1); + CHECK_EQ(ret, KERN_SUCCESS); + + // Add environment variable that signals to `atos` that it should look for + // our task port. See `StartSymbolizerSubproces()`. + internal_strlcpy(atos_mach_port_env_entry_, kAtosMachPortEnvVar, + sizeof(atos_mach_port_env_entry_)); + internal_strlcat(atos_mach_port_env_entry_, pid_str_, + sizeof(atos_mach_port_env_entry_)); + custom_env_[index_to_write_entry] = atos_mach_port_env_entry_; + } return SymbolizerProcess::StartSymbolizerSubprocess(); } @@ -76,15 +101,48 @@ return (length >= 1 && buffer[length - 1] == '\n'); } - void CopyEnv(char **current_env) { - // Count number of elements (excluding nullptr entry). + // Make a copy of the environment in `current_env`. If `new_env_entry` is + // not null then the copy of the environment is allocated with extra space + // for the new entry if necessary. The index that the new entry should be + // written to in the array is returned. Note the last character of + // `new_env_entry` should be `=`. + uptr CopyEnv(char **current_env, const char *new_env_entry) { + uptr new_entry_index = 0; + uptr new_entry_key_length = 0; + if (new_env_entry) { + new_entry_key_length = internal_strlen(new_env_entry); + CHECK_GT(new_entry_key_length, 0); + CHECK_EQ(new_env_entry[new_entry_key_length - 1], '='); + } + + // Count number of elements (excluding nullptr entry) and + // find if `new_env_entry` already exists. uptr count = 0; + bool existing_entry_exists = false; for (; current_env[count] != nullptr; ++count) { + if (new_env_entry) { + if (internal_strncmp(current_env[count], new_env_entry, + new_entry_key_length) == 0) { + // The environment variable is already set. + CHECK(!existing_entry_exists); + existing_entry_exists = true; + // Tell client to overwrite existing index. + new_entry_index = count; + } + } } - // +1 for nullptr terminator. - uptr custom_env_required_entries = count + 1; + uptr custom_env_required_entries = 0; uptr entries_to_copy = count; + if (new_env_entry && !existing_entry_exists) { + // New entry will have to appended to end of array. + new_entry_index = count; + // +1 for new entry and +1 for nullptr terminator. + custom_env_required_entries = count + 2; + } else { + // +1 for nullptr terminator. + custom_env_required_entries = count + 1; + } if (custom_env_required_entries > kMaxNumEnvPEntries) { // Will have to truncate the copy of the environment. @@ -94,6 +152,11 @@ custom_env_required_entries, kMaxNumEnvPEntries); entries_to_copy = kMaxNumEnvPEntries - 1; custom_env_required_entries = kMaxNumEnvPEntries; + if (new_env_entry) { + // Tell the client to write the entry just before the nullptr + // terminator. + new_entry_index = kMaxNumEnvPEntries - 2; + } } // Copy over existing entries @@ -101,6 +164,8 @@ internal_memcpy(custom_env_, current_env, entries_to_copy * sizeof(uptr)); // Null terminate the array. custom_env_[custom_env_required_entries - 1] = nullptr; + CHECK_LT(new_entry_index, kMaxNumEnvPEntries - 1); + return new_entry_index; } char **GetEnvP() override { @@ -126,6 +191,9 @@ static const uptr kMaxNumEnvPEntries = 512; // Use statically allocated array to avoid allocation at symbolization time. char *custom_env_[kMaxNumEnvPEntries]; + // `-1` is so we don't allocate space for two `\0` terminators. + char atos_mach_port_env_entry_[sizeof(kAtosMachPortEnvVar) + + sizeof(pid_str_) - 1]; }; static bool ParseCommandOutput(const char *str, uptr addr, char **out_name, diff --git a/compiler-rt/test/tsan/Darwin/no_call_setenv_in_symbolize.cpp b/compiler-rt/test/tsan/Darwin/no_call_setenv_in_symbolize.cpp new file mode 100644 --- /dev/null +++ b/compiler-rt/test/tsan/Darwin/no_call_setenv_in_symbolize.cpp @@ -0,0 +1,43 @@ +// RUN: %clangxx_tsan -O1 %s -o %t +// `handle_sigbus=0` is required because when the rdar://problem/58789439 bug was +// present TSan's runtime could derefence bad memory leading to SIGBUS being raised. +// If the signal was caught TSan would deadlock because it would try to run the +// symbolizer again. +// RUN: %env_tsan_opts=handle_sigbus=0,symbolize=1 %run %t 2>&1 | FileCheck %s +// RUN: %env_tsan_opts=handle_sigbus=0,symbolize=1 _check_mach_ports_lookup=some_value %run %t 2>&1 | FileCheck %s +#include +#include +#include + +const char *kEnvName = "__UNLIKELY_ENV_VAR_NAME__"; + +int main() { + if (getenv(kEnvName)) { + fprintf(stderr, "Env var %s should not be set\n", kEnvName); + abort(); + } + + // This will set an environment variable that isn't already in + // the environment array. This will cause Darwin's Libc to + // malloc() a new array. + if (setenv(kEnvName, "some_value", /*overwrite=*/1)) { + fprintf(stderr, "Failed to set %s \n", kEnvName); + abort(); + } + + // rdar://problem/58789439 + // Now trigger symbolization. If symbolization tries to call + // to `setenv` that adds a new environment variable, then Darwin + // Libc will call `realloc()` and TSan's runtime will hit + // an assertion failure because TSan's runtime uses a different + // allocator during symbolization which leads to `realloc()` being + // called on a pointer that the allocator didn't allocate. + // + // CHECK: #{{[0-9]}} main {{.*}}no_call_setenv_in_symbolize.cpp:[[@LINE+1]] + __sanitizer_print_stack_trace(); + + // CHECK: DONE + fprintf(stderr, "DONE\n"); + + return 0; +}