Skip to content

Commit

Permalink
Unbreak ASan runtime in the simulators.
Browse files Browse the repository at this point in the history
Summary:
861b69f (rdar://problem/58789439) while
fixing symbolization for TSan completely broke ASan's runtime for the
simulators.

The problem with the previous patch is that the memory passed to
`putenv()` was poisoned and when passed to `putenv()` it tripped
an interceptor for `strchr()` which saw the memory was poisoned and
raised an ASan issue.

The memory was poisoned because `AtosSymbolizerProcess` objects
are created using ASan's internal allocator. Memory from this
allocator gets poisoned with `kAsanInternalHeapMagic`.

To workaround this, this patch makes the memory for the environment
variable entry a global variable that isn't poisoned.

This pass also adds a `DCHECK(getenv(K_ATOS_ENV_VAR))` because the
following DCHECK would crash because `internal_strcmp()` doesn't
work on nullptr.

rdar://problem/62067724

Reviewers: kubamracek, yln

Subscribers: #sanitizers, llvm-commits

Tags: #sanitizers

Differential Revision: https://reviews.llvm.org/D78525

(cherry picked from commit 7039773)
  • Loading branch information
danliew authored and danliew-apple committed Apr 22, 2020
1 parent 150d68e commit e8a928d
Showing 1 changed file with 13 additions and 6 deletions.
19 changes: 13 additions & 6 deletions compiler-rt/lib/sanitizer_common/sanitizer_symbolizer_mac.cpp
Expand Up @@ -53,6 +53,11 @@ bool DlAddrSymbolizer::SymbolizeData(uptr addr, DataInfo *datainfo) {

#define K_ATOS_ENV_VAR "__check_mach_ports_lookup"

// This cannot live in `AtosSymbolizerProcess` because instances of that object
// are allocated by the internal allocator which under ASan is poisoned with
// kAsanInternalHeapMagic.
static char kAtosMachPortEnvEntry[] = K_ATOS_ENV_VAR "=000000000000000";

class AtosSymbolizerProcess : public SymbolizerProcess {
public:
explicit AtosSymbolizerProcess(const char *path)
Expand All @@ -69,7 +74,7 @@ class AtosSymbolizerProcess : public SymbolizerProcess {
// 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_);
int result = putenv(kAtosMachPortEnvEntry);
CHECK_EQ(result, 0);
}
}
Expand All @@ -95,12 +100,13 @@ class AtosSymbolizerProcess : public SymbolizerProcess {
// 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_),
uptr count = internal_snprintf(kAtosMachPortEnvEntry,
sizeof(kAtosMachPortEnvEntry),
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(getenv(K_ATOS_ENV_VAR));
DCHECK_EQ(internal_strcmp(getenv(K_ATOS_ENV_VAR), pid_str_), 0);
}

Expand All @@ -127,9 +133,10 @@ 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";
// Space for `\0` in `K_ATOS_ENV_VAR` is reused for `=`.
static_assert(sizeof(kAtosMachPortEnvEntry) ==
(sizeof(K_ATOS_ENV_VAR) + sizeof(pid_str_)),
"sizes should match");
};

#undef K_ATOS_ENV_VAR
Expand Down

0 comments on commit e8a928d

Please sign in to comment.