allow for heapprofd's signal to be multiplexed
This patch refactors heapprofd_malloc to make it easier to reuse the
reserved signal for multiple purposes. We define a new generic signal
handler for profilers, which dispatches to more specific logic based on
the signal's payload (si_value).
The profiler signal handler is installed during libc preinit, after
malloc initialization (so races against synchronous heapprofd
initialization need not be considered). In terms of code organization, I
copied the existing approach with a loosely referenced function in
bionic_globals.h. Do tell if you'd rather a different approach here.
The profileability of a process is quite tied to the malloc
files/interfaces in bionic - in particular, it's set through
android_mallopt. I do not change that, but instead introduce a new
android_mallopt option to be able to query profileability of the
process (which is now used by the new profiler signal handler). As part
of that, gZygoteChildProfileable is moved from heapprofd_malloc to
common (alongside gZygoteChild).
I've removed the masking and reraising of the heapprofd signal when
racing against malloc_limit init. We're ok with taking a simpler
approach and dropping the heapprofd signal in such an unlikely race.
Note: this requires a corresponding change in heapprofd to use sigqueue()
instead of kill(), as the latter leaves the si_value uninitialized(?) on
the receiving side.
Bug: 144281346
Change-Id: I93bb2e82cff5870e5ca499cf86439860aca9dfa5
diff --git a/libc/bionic/android_profiling_dynamic.cpp b/libc/bionic/android_profiling_dynamic.cpp
new file mode 100644
index 0000000..dd87ed8
--- /dev/null
+++ b/libc/bionic/android_profiling_dynamic.cpp
@@ -0,0 +1,79 @@
+/*
+ * Copyright (C) 2020 The Android Open Source Project
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
+ * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
+ * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
+ * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
+ * SUCH DAMAGE.
+ */
+
+#if defined(LIBC_STATIC)
+#error This file should not be compiled for static targets.
+#endif
+
+#include <signal.h>
+
+#include <async_safe/log.h>
+#include <platform/bionic/malloc.h>
+#include <platform/bionic/reserved_signals.h>
+
+#include "malloc_heapprofd.h"
+
+// This file defines the handler for the reserved signal sent by the Android
+// platform's profilers. The accompanying signal value discriminates between
+// specific requestors:
+// 0: heapprofd heap profiler.
+static constexpr int kHeapprofdSignalValue = 0;
+
+static void HandleProfilingSignal(int, siginfo_t*, void*);
+
+// Called during dynamic libc preinit.
+__LIBC_HIDDEN__ void __libc_init_profiling_handlers() {
+ struct sigaction action = {};
+ action.sa_flags = SA_SIGINFO | SA_RESTART;
+ action.sa_sigaction = HandleProfilingSignal;
+ sigaction(BIONIC_SIGNAL_PROFILER, &action, nullptr);
+}
+
+static void HandleProfilingSignal(int /*signal_number*/, siginfo_t* info, void* /*ucontext*/) {
+ if (info->si_code != SI_QUEUE)
+ return;
+
+ int signal_value = info->si_value.sival_int;
+ async_safe_format_log(ANDROID_LOG_WARN, "libc", "%s: received profiling signal with si_value: %d",
+ getprogname(), signal_value);
+
+ // Proceed only if the process is considered profileable.
+ bool profileable = false;
+ android_mallopt(M_GET_PROCESS_PROFILEABLE, &profileable, sizeof(profileable));
+ if (!profileable) {
+ async_safe_write_log(ANDROID_LOG_ERROR, "libc", "profiling signal rejected (not profileable)");
+ return;
+ }
+
+ if (signal_value == kHeapprofdSignalValue) {
+ HandleHeapprofdSignal();
+ } else {
+ async_safe_format_log(ANDROID_LOG_ERROR, "libc", "unrecognized profiling signal si_value: %d",
+ signal_value);
+ }
+}
diff --git a/libc/bionic/libc_init_dynamic.cpp b/libc/bionic/libc_init_dynamic.cpp
index aafefab..4eaf37a 100644
--- a/libc/bionic/libc_init_dynamic.cpp
+++ b/libc/bionic/libc_init_dynamic.cpp
@@ -99,6 +99,9 @@
// Hooks for various libraries to let them know that we're starting up.
__libc_globals.mutate(__libc_init_malloc);
+ // Install reserved signal handlers for assisting the platform's profilers.
+ __libc_init_profiling_handlers();
+
__libc_init_fork_handler();
#if __has_feature(hwaddress_sanitizer)
diff --git a/libc/bionic/malloc_common.h b/libc/bionic/malloc_common.h
index 4a726db..79bf98c 100644
--- a/libc/bionic/malloc_common.h
+++ b/libc/bionic/malloc_common.h
@@ -74,8 +74,6 @@
#endif
-extern bool gZygoteChild;
-
static inline const MallocDispatch* GetDispatchTable() {
return atomic_load_explicit(&__libc_globals->current_dispatch_table, memory_order_acquire);
}
diff --git a/libc/bionic/malloc_common_dynamic.cpp b/libc/bionic/malloc_common_dynamic.cpp
index 8068f4e..2cb1e8a 100644
--- a/libc/bionic/malloc_common_dynamic.cpp
+++ b/libc/bionic/malloc_common_dynamic.cpp
@@ -75,9 +75,19 @@
// =============================================================================
pthread_mutex_t gGlobalsMutateLock = PTHREAD_MUTEX_INITIALIZER;
-bool gZygoteChild = false;
-
_Atomic bool gGlobalsMutating = false;
+
+static bool gZygoteChild = false;
+
+// In a Zygote child process, this is set to true if profiling of this process
+// is allowed. Note that this is set at a later time than gZygoteChild. The
+// latter is set during the fork (while still in zygote's SELinux domain). While
+// this bit is set after the child is specialized (and has transferred SELinux
+// domains if applicable). These two flags are read by the
+// BIONIC_SIGNAL_PROFILER handler, which does nothing if the process is not
+// profileable.
+static _Atomic bool gZygoteChildProfileable = false;
+
// =============================================================================
static constexpr MallocDispatch __libc_malloc_default_dispatch
@@ -391,13 +401,10 @@
if (HeapprofdShouldLoad()) {
HeapprofdInstallHooksAtInit(globals);
}
-
- // Install this last to avoid as many race conditions as possible.
- HeapprofdInstallSignalHandler();
} else {
- // Install a signal handler that prints an error since we don't support
- // heapprofd and any other hook to be installed at the same time.
- HeapprofdInstallErrorSignalHandler();
+ // Record the fact that incompatible hooks are active, to skip any later
+ // heapprofd signal handler invocations.
+ HeapprofdRememberHookConflict();
}
}
@@ -476,6 +483,27 @@
gZygoteChild = true;
return true;
}
+ if (opcode == M_INIT_ZYGOTE_CHILD_PROFILING) {
+ if (arg != nullptr || arg_size != 0) {
+ errno = EINVAL;
+ return false;
+ }
+ atomic_store_explicit(&gZygoteChildProfileable, true, memory_order_release);
+ // Also check if heapprofd should start profiling from app startup.
+ HeapprofdInitZygoteChildProfiling();
+ return true;
+ }
+ if (opcode == M_GET_PROCESS_PROFILEABLE) {
+ if (arg == nullptr || arg_size != sizeof(bool)) {
+ errno = EINVAL;
+ return false;
+ }
+ // Native processes are considered profileable. Zygote children are considered
+ // profileable only when appropriately tagged.
+ *reinterpret_cast<bool*>(arg) =
+ !gZygoteChild || atomic_load_explicit(&gZygoteChildProfileable, memory_order_acquire);
+ return true;
+ }
if (opcode == M_SET_ALLOCATION_LIMIT_BYTES) {
return LimitEnable(arg, arg_size);
}
@@ -503,6 +531,7 @@
if (opcode == M_SET_HEAP_TAGGING_LEVEL) {
return SetHeapTaggingLevel(arg, arg_size);
}
+ // Try heapprofd's mallopt, as it handles options not covered here.
return HeapprofdMallopt(opcode, arg, arg_size);
}
// =============================================================================
diff --git a/libc/bionic/malloc_heapprofd.cpp b/libc/bionic/malloc_heapprofd.cpp
index 5860222..3d2dc2b 100644
--- a/libc/bionic/malloc_heapprofd.cpp
+++ b/libc/bionic/malloc_heapprofd.cpp
@@ -49,18 +49,18 @@
static constexpr char kHeapprofdSharedLib[] = "heapprofd_client.so";
static constexpr char kHeapprofdPrefix[] = "heapprofd";
static constexpr char kHeapprofdPropertyEnable[] = "heapprofd.enable";
-static constexpr int kHeapprofdSignal = __SIGRTMIN + 4;
// The logic for triggering heapprofd (at runtime) is as follows:
-// 1. HEAPPROFD_SIGNAL is received by the process, entering the
-// MaybeInstallInitHeapprofdHook signal handler.
+// 1. A reserved profiling signal is received by the process, its si_value
+// discriminating between different handlers. For the case of heapprofd,
+// HandleHeapprofdSignal is called.
// 2. If the initialization is not already in flight
// (gHeapprofdInitInProgress is false), the malloc hook is set to
// point at InitHeapprofdHook, and gHeapprofdInitInProgress is set to
// true.
// 3. The next malloc call enters InitHeapprofdHook, which removes the malloc
// hook, and spawns a detached pthread to run the InitHeapprofd task.
-// (gHeapprofdInitHook_installed atomic is used to perform this once.)
+// (gHeapprofdInitHookInstalled atomic is used to perform this once.)
// 4. InitHeapprofd, on a dedicated pthread, loads the heapprofd client library,
// installs the full set of heapprofd hooks, and invokes the client's
// initializer. The dedicated pthread then terminates.
@@ -79,12 +79,9 @@
static _Atomic bool gHeapprofdInitInProgress = false;
static _Atomic bool gHeapprofdInitHookInstalled = false;
-// In a Zygote child process, this is set to true if profiling of this process
-// is allowed. Note that this is set at a later time than the global
-// gZygoteChild. The latter is set during the fork (while still in
-// zygote's SELinux domain). While this bit is set after the child is
-// specialized (and has transferred SELinux domains if applicable).
-static _Atomic bool gZygoteChildProfileable = false;
+// Set to true if the process has enabled malloc_debug or malloc_hooks, which
+// are incompatible (and take precedence over) heapprofd.
+static _Atomic bool gHeapprofdIncompatibleHooks = false;
extern "C" void* MallocInitHeapprofdHook(size_t);
@@ -93,7 +90,7 @@
Malloc(calloc),
Malloc(free),
Malloc(mallinfo),
- MallocInitHeapprofdHook,
+ MallocInitHeapprofdHook, // malloc replacement
Malloc(malloc_usable_size),
Malloc(memalign),
Malloc(posix_memalign),
@@ -112,38 +109,6 @@
Malloc(malloc_info),
};
-static void MaybeInstallInitHeapprofdHook(int) {
- // Zygote child processes must be marked profileable.
- if (gZygoteChild &&
- !atomic_load_explicit(&gZygoteChildProfileable, memory_order_acquire)) {
- error_log("%s: not enabling heapprofd, not marked profileable.", getprogname());
- return;
- }
-
- // Checking this variable is only necessary when this could conflict with
- // the change to enable the allocation limit. All other places will
- // not ever have a conflict modifying the globals.
- if (!atomic_exchange(&gGlobalsMutating, true)) {
- if (!atomic_exchange(&gHeapprofdInitInProgress, true)) {
- __libc_globals.mutate([](libc_globals* globals) {
- atomic_store(&globals->default_dispatch_table, &__heapprofd_init_dispatch);
- auto dispatch_table = GetDispatchTable();
- if (dispatch_table == nullptr || dispatch_table == &globals->malloc_dispatch_table) {
- atomic_store(&globals->current_dispatch_table, &__heapprofd_init_dispatch);
- }
- });
- }
- atomic_store(&gGlobalsMutating, false);
- } else {
- // The only way you can get to this point is if the signal has been
- // blocked by a call to HeapprofdMaskSignal. The raise below will
- // do nothing until a call to HeapprofdUnmaskSignal, which will cause
- // the signal to be resent. Using this avoids the need for a busy loop
- // waiting for gGlobalsMutating to change back to false.
- raise(kHeapprofdSignal);
- }
-}
-
constexpr char kHeapprofdProgramPropertyPrefix[] = "heapprofd.enable.";
constexpr size_t kHeapprofdProgramPropertyPrefixSize = sizeof(kHeapprofdProgramPropertyPrefix) - 1;
constexpr size_t kMaxCmdlineSize = 512;
@@ -203,6 +168,39 @@
return true;
}
+// Runtime triggering entry-point. Two possible call sites:
+// * when receiving a profiling signal with a si_value indicating heapprofd.
+// * when a Zygote child is marking itself as profileable, and there's a
+// matching profiling request for this process (in which case heapprofd client
+// is loaded synchronously).
+// In both cases, the caller is responsible for verifying that the process is
+// considered profileable.
+void HandleHeapprofdSignal() {
+ if (atomic_load_explicit(&gHeapprofdIncompatibleHooks, memory_order_acquire)) {
+ error_log("%s: not enabling heapprofd, malloc_debug/malloc_hooks are enabled.", getprogname());
+ return;
+ }
+
+ // Checking this variable is only necessary when this could conflict with
+ // the change to enable the allocation limit. All other places will
+ // not ever have a conflict modifying the globals.
+ if (!atomic_exchange(&gGlobalsMutating, true)) {
+ if (!atomic_exchange(&gHeapprofdInitInProgress, true)) {
+ __libc_globals.mutate([](libc_globals* globals) {
+ atomic_store(&globals->default_dispatch_table, &__heapprofd_init_dispatch);
+ auto dispatch_table = GetDispatchTable();
+ if (dispatch_table == nullptr || dispatch_table == &globals->malloc_dispatch_table) {
+ atomic_store(&globals->current_dispatch_table, &__heapprofd_init_dispatch);
+ }
+ });
+ }
+ atomic_store(&gGlobalsMutating, false);
+ }
+ // Otherwise, we're racing against malloc_limit's enable logic (at most once
+ // per process, and a niche feature). This is highly unlikely, so simply give
+ // up if it does happen.
+}
+
bool HeapprofdShouldLoad() {
// First check for heapprofd.enable. If it is set to "all", enable
// heapprofd for all processes. Otherwise, check heapprofd.enable.${prog},
@@ -226,38 +224,8 @@
return property_value[0] != '\0';
}
-void HeapprofdInstallSignalHandler() {
- struct sigaction action = {};
- action.sa_handler = MaybeInstallInitHeapprofdHook;
- sigaction(kHeapprofdSignal, &action, nullptr);
-}
-
-extern "C" int __rt_sigprocmask(int, const sigset64_t*, sigset64_t*, size_t);
-
-void HeapprofdMaskSignal() {
- sigset64_t mask_set;
- // Need to use this function instead because sigprocmask64 filters
- // out this signal.
- __rt_sigprocmask(SIG_SETMASK, nullptr, &mask_set, sizeof(mask_set));
- sigaddset64(&mask_set, kHeapprofdSignal);
- __rt_sigprocmask(SIG_SETMASK, &mask_set, nullptr, sizeof(mask_set));
-}
-
-void HeapprofdUnmaskSignal() {
- sigset64_t mask_set;
- __rt_sigprocmask(SIG_SETMASK, nullptr, &mask_set, sizeof(mask_set));
- sigdelset64(&mask_set, kHeapprofdSignal);
- __rt_sigprocmask(SIG_SETMASK, &mask_set, nullptr, sizeof(mask_set));
-}
-
-static void DisplayError(int) {
- error_log("Cannot install heapprofd while malloc debug/malloc hooks are enabled.");
-}
-
-void HeapprofdInstallErrorSignalHandler() {
- struct sigaction action = {};
- action.sa_handler = DisplayError;
- sigaction(kHeapprofdSignal, &action, nullptr);
+void HeapprofdRememberHookConflict() {
+ atomic_store_explicit(&gHeapprofdIncompatibleHooks, true, memory_order_release);
}
static void CommonInstallHooks(libc_globals* globals) {
@@ -326,15 +294,12 @@
return Malloc(malloc)(bytes);
}
-// Marks this process as a profileable zygote child.
-static bool HandleInitZygoteChildProfiling() {
- atomic_store_explicit(&gZygoteChildProfileable, true, memory_order_release);
-
+bool HeapprofdInitZygoteChildProfiling() {
// Conditionally start "from startup" profiling.
if (HeapprofdShouldLoad()) {
- // Directly call the signal handler (will correctly guard against
- // concurrent signal delivery).
- MaybeInstallInitHeapprofdHook(kHeapprofdSignal);
+ // Directly call the signal handler codepath (properly protects against
+ // concurrent invocations).
+ HandleHeapprofdSignal();
}
return true;
}
@@ -358,13 +323,6 @@
}
bool HeapprofdMallopt(int opcode, void* arg, size_t arg_size) {
- if (opcode == M_INIT_ZYGOTE_CHILD_PROFILING) {
- if (arg != nullptr || arg_size != 0) {
- errno = EINVAL;
- return false;
- }
- return HandleInitZygoteChildProfiling();
- }
if (opcode == M_RESET_HOOKS) {
if (arg != nullptr || arg_size != 0) {
errno = EINVAL;
diff --git a/libc/bionic/malloc_heapprofd.h b/libc/bionic/malloc_heapprofd.h
index 9e846b6..d67a788 100644
--- a/libc/bionic/malloc_heapprofd.h
+++ b/libc/bionic/malloc_heapprofd.h
@@ -36,12 +36,11 @@
void HeapprofdInstallHooksAtInit(libc_globals* globals);
-void HeapprofdInstallSignalHandler();
+void HeapprofdRememberHookConflict();
-void HeapprofdInstallErrorSignalHandler();
+void HandleHeapprofdSignal();
-void HeapprofdMaskSignal();
-
-void HeapprofdUnmaskSignal();
+bool HeapprofdInitZygoteChildProfiling();
bool HeapprofdMallopt(int optcode, void* arg, size_t arg_size);
+
diff --git a/libc/bionic/malloc_limit.cpp b/libc/bionic/malloc_limit.cpp
index d8c0ebe..ebc33ab 100644
--- a/libc/bionic/malloc_limit.cpp
+++ b/libc/bionic/malloc_limit.cpp
@@ -264,7 +264,6 @@
}
#else
static bool EnableLimitDispatchTable() {
- HeapprofdMaskSignal();
pthread_mutex_lock(&gGlobalsMutateLock);
// All other code that calls mutate will grab the gGlobalsMutateLock.
// However, there is one case where the lock cannot be acquired, in the
@@ -272,7 +271,7 @@
// threads calling mutate at the same time, use an atomic variable to
// verify that only this function or the signal handler are calling mutate.
// If this function is called at the same time as the signal handler is
- // being called, allow up to five ms for the signal handler to complete
+ // being called, allow a short period for the signal handler to complete
// before failing.
bool enabled = false;
size_t num_tries = 20;
@@ -291,7 +290,6 @@
usleep(1000);
}
pthread_mutex_unlock(&gGlobalsMutateLock);
- HeapprofdUnmaskSignal();
if (enabled) {
info_log("malloc_limit: Allocation limit enabled, max size %" PRId64 " bytes\n", gAllocLimit);
} else {