Skip to content

Cherry pick rdar://problem/58789439 part3 #1095

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,4 +126,10 @@ Symbolizer::SymbolizerScope::~SymbolizerScope() {
sym_->end_hook_();
}

void Symbolizer::LateInitializeTools() {
for (auto &tool : tools_) {
tool.LateInitialize();
}
}

} // namespace __sanitizer
3 changes: 3 additions & 0 deletions compiler-rt/lib/sanitizer_common/sanitizer_symbolizer.h
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,9 @@ class Symbolizer final {
private:
const Symbolizer *sym_;
};

// Calls `LateInitialize()` on all items in `tools_`.
void LateInitializeTools();
};

#ifdef SANITIZER_WINDOWS
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,11 @@ class SymbolizerTool {
virtual const char *Demangle(const char *name) {
return nullptr;
}

// Called during the LateInitialize phase of Sanitizer initialization.
// Usually this is a safe place to call code that might need to use user
// memory allocators.
virtual void LateInitialize() {}
};

// SymbolizerProcess encapsulates communication between the tool and
Expand Down
46 changes: 46 additions & 0 deletions compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

#include <dlfcn.h>
#include <errno.h>
#include <mach/mach.h>
#include <stdlib.h>
#include <sys/wait.h>
#include <unistd.h>
Expand Down Expand Up @@ -50,13 +51,29 @@ bool DlAddrSymbolizer::SymbolizeData(uptr addr, DataInfo *datainfo) {
return true;
}

#define K_ATOS_ENV_VAR "__check_mach_ports_lookup"

class AtosSymbolizerProcess : public SymbolizerProcess {
public:
explicit AtosSymbolizerProcess(const char *path)
: SymbolizerProcess(path, /*use_posix_spawn*/ true) {
pid_str_[0] = '\0';
}

void LateInitialize() {
if (SANITIZER_IOSSIM) {
// `putenv()` may call malloc/realloc so it is only safe to do this
// during LateInitialize() or later (i.e. we can't do this in the
// constructor). We also can't do this in `StartSymbolizerSubprocess()`
// because in TSan we switch allocators when we're symbolizing.
// We use `putenv()` rather than `setenv()` so that we can later directly
// write into the storage without LibC getting involved to change what the
// variable is set to
int result = putenv(mach_port_env_var_entry_);
CHECK_EQ(result, 0);
}
}

private:
bool StartSymbolizerSubprocess() override {
// Configure sandbox before starting atos process.
Expand All @@ -65,6 +82,28 @@ class AtosSymbolizerProcess : public SymbolizerProcess {
// the call to GetArgV.
internal_snprintf(pid_str_, sizeof(pid_str_), "%d", internal_getpid());

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);

// Set environment variable that signals to `atos` that it should look
// for our task port. We can't call `setenv()` here because it might call
// malloc/realloc. To avoid that we instead update the
// `mach_port_env_var_entry_` variable with our current PID.
uptr count = internal_snprintf(mach_port_env_var_entry_,
sizeof(mach_port_env_var_entry_),
K_ATOS_ENV_VAR "=%s", pid_str_);
CHECK_GE(count, sizeof(K_ATOS_ENV_VAR) + internal_strlen(pid_str_));
// Document our assumption but without calling `getenv()` in normal
// builds.
DCHECK_EQ(internal_strcmp(getenv(K_ATOS_ENV_VAR), pid_str_), 0);
}

return SymbolizerProcess::StartSymbolizerSubprocess();
}

Expand All @@ -88,8 +127,13 @@ class AtosSymbolizerProcess : public SymbolizerProcess {
}

char pid_str_[16];
// Space for `\0` in `kAtosEnvVar_` is reused for `=`.
char mach_port_env_var_entry_[sizeof(K_ATOS_ENV_VAR) + sizeof(pid_str_)] =
K_ATOS_ENV_VAR "=0";
};

#undef K_ATOS_ENV_VAR

static bool ParseCommandOutput(const char *str, uptr addr, char **out_name,
char **out_module, char **out_file, uptr *line,
uptr *start_address) {
Expand Down Expand Up @@ -191,6 +235,8 @@ bool AtosSymbolizer::SymbolizeData(uptr addr, DataInfo *info) {
return true;
}

void AtosSymbolizer::LateInitialize() { process_->LateInitialize(); }

} // namespace __sanitizer

#endif // SANITIZER_MAC
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ class AtosSymbolizer : public SymbolizerTool {

bool SymbolizePC(uptr addr, SymbolizedStack *stack) override;
bool SymbolizeData(uptr addr, DataInfo *info) override;
void LateInitialize() override;

private:
AtosSymbolizerProcess *process_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,9 @@ Symbolizer *Symbolizer::PlatformInit() {
return new (symbolizer_allocator_) Symbolizer({});
}

void Symbolizer::LateInitialize() { Symbolizer::GetOrInit(); }
void Symbolizer::LateInitialize() {
Symbolizer::GetOrInit()->LateInitializeTools();
}

void StartReportDeadlySignal() {}
void ReportDeadlySignal(const SignalContext &sig, u32 tid,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ Symbolizer *Symbolizer::PlatformInit() {
}

void Symbolizer::LateInitialize() {
Symbolizer::GetOrInit();
Symbolizer::GetOrInit()->LateInitializeTools();
InitializeSwiftDemangler();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -310,7 +310,7 @@ Symbolizer *Symbolizer::PlatformInit() {
}

void Symbolizer::LateInitialize() {
Symbolizer::GetOrInit();
Symbolizer::GetOrInit()->LateInitializeTools();
}

} // namespace __sanitizer
Expand Down
43 changes: 43 additions & 0 deletions compiler-rt/test/tsan/Darwin/no_call_setenv_in_symbolize.cpp
Original file line number Diff line number Diff line change
@@ -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 <sanitizer/common_interface_defs.h>
#include <stdio.h>
#include <stdlib.h>

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;
}