[compiler-rt] 861b69f - [Darwin] Fix symbolization for recent simulator runtimes.
Dan Liew via llvm-commits
llvm-commits at lists.llvm.org
Fri Apr 17 15:08:38 PDT 2020
Author: Dan Liew
Date: 2020-04-17T15:08:14-07:00
New Revision: 861b69faee5df8d4e13ef316c7474a10e4069e81
URL: https://github.com/llvm/llvm-project/commit/861b69faee5df8d4e13ef316c7474a10e4069e81
DIFF: https://github.com/llvm/llvm-project/commit/861b69faee5df8d4e13ef316c7474a10e4069e81.diff
LOG: [Darwin] Fix symbolization for recent simulator runtimes.
Summary:
Due to sandbox restrictions in the recent versions of the simulator runtime the
atos program is no longer able to access the task port of a parent process
without additional help.
This patch fixes this by registering a task port for the parent process
before spawning atos and also tells atos to look for this by setting
a special environment variable.
This patch is based on an Apple internal fix (rdar://problem/43693565) that
unfortunately contained a bug (rdar://problem/58789439) because it used
setenv() to set the special environment variable. This is not safe because in
certain circumstances this can trigger a call to realloc() which can fail
during symbolization leading to deadlock. A test case is included that captures
this problem.
The approach used to set the necessary environment variable is as
follows:
1. Calling `putenv()` early during process init (but late enough that
malloc/realloc works) to set a dummy value for the environment variable.
2. Just before `atos` is spawned the storage for the environment
variable is modified to contain the correct PID.
A flaw with this approach is that if the application messes with the
atos environment variable (i.e. unsets it or changes it) between the
time its set and the time we need it then symbolization will fail. We
will ignore this issue for now but a `DCHECK()` is included in the patch
that documents this assumption but doesn't check it at runtime to avoid
calling `getenv()`.
The issue reported in rdar://problem/58789439 manifested as a deadlock
during symbolization in the following situation:
1. Before TSan detects an issue something outside of the runtime calls
setenv() that sets a new environment variable that wasn't previously
set. This triggers a call to malloc() to allocate a new environment
array. This uses TSan's normal user-facing allocator. LibC stores this
pointer for future use later.
2. TSan detects an issue and tries to launch the symbolizer. When we are in the
symbolizer we switch to a different (internal allocator) and then we call
setenv() to set a new environment variable. When this happen setenv() sees
that it needs to make the environment array larger and calls realloc() on the
existing enviroment array because it remembers that it previously allocated
memory for it. Calling realloc() fails here because it is being called on a
pointer its never seen before.
The included test case closely reproduces the originally reported
problem but it doesn't replicate the `((kBlockMagic)) ==
((((u64*)addr)[0])` assertion failure exactly. This is due to the way
TSan's normal allocator allocates the environment array the first time
it is allocated. In the test program addr[0] accesses an inaccessible
page and raises SIGBUS. If TSan's SIGBUS signal handler is active, the
signal is caught and symbolication is attempted again which results in
deadlock.
In the originally reported problem the pointer is successfully derefenced but
then the assert fails due to the provided pointer not coming from the active
allocator. When the assert fails TSan tries to symbolicate the stacktrace while
already being in the middle of symbolication which results in deadlock.
rdar://problem/58789439
Reviewers: kubamracek, yln
Subscribers: jfb, #sanitizers, llvm-commits
Tags: #sanitizers
Differential Revision: https://reviews.llvm.org/D78179
Added:
compiler-rt/test/tsan/Darwin/no_call_setenv_in_symbolize.cpp
Modified:
compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp
compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.h
Removed:
################################################################################
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp
index 1379ed82d03d..4623c3f76b12 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp
@@ -20,6 +20,7 @@
#include <dlfcn.h>
#include <errno.h>
+#include <mach/mach.h>
#include <stdlib.h>
#include <sys/wait.h>
#include <unistd.h>
@@ -50,6 +51,8 @@ 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)
@@ -57,6 +60,20 @@ class AtosSymbolizerProcess : public SymbolizerProcess {
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.
@@ -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();
}
@@ -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) {
@@ -191,6 +235,8 @@ bool AtosSymbolizer::SymbolizeData(uptr addr, DataInfo *info) {
return true;
}
+void AtosSymbolizer::LateInitialize() { process_->LateInitialize(); }
+
} // namespace __sanitizer
#endif // SANITIZER_MAC
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.h b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.h
index 68521375e64c..8996131fc138 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.h
@@ -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_;
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
index 000000000000..1e31693cc74d
--- /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 <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
diff erent
+ // 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;
+}
More information about the llvm-commits
mailing list