Merge changes I4f001910,Ic251afec

* changes:
  android_mallopt: opcode preprocess define + missing header
  conditional zygote child heap profiling + android_internal_mallopt

libc/bionic/malloc_common.cpp

@@ -47,6 +47,7 @@
 #include <private/bionic_defs.h>
 #include <private/bionic_config.h>
 #include <private/bionic_globals.h>
+#include <private/bionic_malloc.h>
 #include <private/bionic_malloc_dispatch.h>
 
 #if __has_feature(hwaddress_sanitizer)
@@ -304,6 +305,13 @@ typedef ssize_t (*malloc_backtrace_func_t)(void*, uintptr_t*, size_t);
     async_safe_format_log(ANDROID_LOG_INFO, "libc", (format), ##__VA_ARGS__ )
 // =============================================================================
 
+// In a Zygote child process, this is set to true if profiling of this process
+// is allowed. Note that this set at a later time than the above
+// gMallocLeakZygoteChild. 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 gMallocZygoteChildProfileable = false;
+
 // =============================================================================
 // Exported for use by ddms.
 // =============================================================================
@@ -641,39 +649,36 @@ static void install_hooks(libc_globals* globals, const char* options,
   }
 }
 
-// The logic for triggering heapprofd below is as following.
-// 1. HEAPPROFD_SIGNAL is received by the process.
-// 2. If neither InitHeapprofd nor InitHeapprofdHook are currently installed
-//    (g_heapprofd_init_hook_installed is false), InitHeapprofdHook is
-//    installed and g_heapprofd_init_in_progress is set to true.
-//
-// On the next subsequent malloc, InitHeapprofdHook is called and
-// 3a. If the signal is currently being handled (g_heapprofd_init_in_progress
-//     is true), no action is taken.
-// 3b. Otherwise, The signal handler (InstallInitHeapprofdHook) installs a
-//     temporary malloc hook (InitHeapprofdHook).
-// 4. When this hook gets run the first time, it uninstalls itself and spawns
-//    a thread running InitHeapprofd that loads heapprofd.so and installs the
-//    hooks within.
+// The logic for triggering heapprofd (at runtime) is as follows:
+// 1. HEAPPROFD_SIGNAL is received by the process, entering the
+//    MaybeInstallInitHeapprofdHook signal handler.
+// 2. If the initialization is not already in flight
+//    (g_heapprofd_init_in_progress is false), the malloc hook is set to
+//    point at InitHeapprofdHook, and g_heapprofd_init_in_progress 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.
+//    (g_heapprofd_init_hook_installed 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.
 // 5. g_heapprofd_init_in_progress and g_heapprofd_init_hook_installed are
-//    reset to false so heapprofd can be reinitialized. Reinitialization
-//    means that a new profiling session is started and any still active is
+//    reset to false such that heapprofd can be reinitialized. Reinitialization
+//    means that a new profiling session is started, and any still active is
 //    torn down.
 //
-// This roundabout way is needed because we are running non AS-safe code, so
-// we cannot run it directly in the signal handler. The other approach of
-// running a standby thread and signalling through write(2) and read(2) would
-// significantly increase the number of active threads in the system.
+// The incremental hooking and a dedicated task thread are used since we cannot
+// do heavy work within a signal handler, or when blocking a malloc invocation.
 
 static _Atomic bool g_heapprofd_init_in_progress = false;
 static _Atomic bool g_heapprofd_init_hook_installed = false;
 
-extern "C" void InstallInitHeapprofdHook(int);
+extern "C" void MaybeInstallInitHeapprofdHook(int);
 
 // Initializes memory allocation framework once per process.
 static void malloc_init_impl(libc_globals* globals) {
   struct sigaction action = {};
-  action.sa_handler = InstallInitHeapprofdHook;
+  action.sa_handler = MaybeInstallInitHeapprofdHook;
   sigaction(HEAPPROFD_SIGNAL, &action, nullptr);
 
   const char* prefix;
@@ -735,7 +740,13 @@ static void* InitHeapprofdHook(size_t bytes) {
   return Malloc(malloc)(bytes);
 }
 
-extern "C" void InstallInitHeapprofdHook(int) {
+extern "C" void MaybeInstallInitHeapprofdHook(int) {
+  // Zygote child processes must be marked profileable.
+  if (gMallocLeakZygoteChild &&
+      !atomic_load_explicit_const(&gMallocZygoteChildProfileable, memory_order_acquire)) {
+    return;
+  }
+
   if (!atomic_exchange(&g_heapprofd_init_in_progress, true)) {
     __libc_globals.mutate([](libc_globals* globals) {
       atomic_store(&globals->malloc_dispatch.malloc, InitHeapprofdHook);
@@ -745,6 +756,46 @@ extern "C" void InstallInitHeapprofdHook(int) {
 
 #endif  // !LIBC_STATIC
 
+// =============================================================================
+// Platform-internal mallopt variant.
+// =============================================================================
+
+#if !defined(LIBC_STATIC)
+// Marks this process as a profileable zygote child.
+bool HandleInitZygoteChildProfiling() {
+  atomic_store_explicit(&gMallocZygoteChildProfileable, true,
+                        memory_order_release);
+
+  // Conditionally start "from startup" profiling.
+  if (CheckLoadHeapprofd()) {
+    // Directly call the signal handler (will correctly guard against
+    // concurrent signal delivery).
+    MaybeInstallInitHeapprofdHook(HEAPPROFD_SIGNAL);
+  }
+  return true;
+}
+
+#else
+
+bool HandleInitZygoteChildProfiling() {
+  return true;
+}
+
+#endif  // !defined(LIBC_STATIC)
+
+bool android_mallopt(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();
+  }
+
+  errno = ENOTSUP;
+  return false;
+}
+
 // =============================================================================
 // Exported for use by libmemunreachable.
 // =============================================================================

libc/libc.map.txt

@@ -1480,6 +1480,7 @@ LIBC_Q { # introduced=Q
     android_getaddrinfofornet; # apex
 
     # Used by libandroid_runtime
+    android_mallopt; # apex
     gMallocLeakZygoteChild; # apex
 } LIBC_P;
 

libc/private/bionic_malloc.h

@@ -0,0 +1,46 @@
+/*
+ * Copyright (C) 2018 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.
+ */
+
+#pragma once
+
+#include <stdbool.h>
+
+// Opcodes for android_mallopt.
+
+// Marks the calling process as a profileable zygote child, possibly
+// initializing profiling infrastructure.
+enum {
+  M_INIT_ZYGOTE_CHILD_PROFILING = 1,
+#define M_INIT_ZYGOTE_CHILD_PROFILING M_INIT_ZYGOTE_CHILD_PROFILING
+};
+
+// Manipulates bionic-specific handling of memory allocation APIs such as
+// malloc. Only for use by the Android platform itself.
+//
+// On success, returns true. On failure, returns false and sets errno.
+extern "C" bool android_mallopt(int opcode, void* arg, size_t arg_size);

tests/malloc_test.cpp

@@ -25,6 +25,7 @@
 #include <tinyxml2.h>
 
 #include "private/bionic_config.h"
+#include "private/bionic_malloc.h"
 #include "utils.h"
 
 #if defined(__BIONIC__)
@@ -601,3 +602,32 @@ TEST(malloc, mallinfo) {
   GTEST_LOG_(INFO) << "Host glibc does not pass this test, skipping.\n";
 #endif
 }
+
+TEST(android_mallopt, error_on_unexpected_option) {
+#if defined(__BIONIC__)
+  const int unrecognized_option = -1;
+  errno = 0;
+  EXPECT_EQ(false, android_mallopt(unrecognized_option, nullptr, 0));
+  EXPECT_EQ(ENOTSUP, errno);
+#else
+  GTEST_LOG_(INFO) << "This tests a bionic implementation detail.\n";
+#endif
+}
+
+TEST(android_mallopt, init_zygote_child_profiling) {
+#if defined(__BIONIC__)
+  // Successful call.
+  errno = 0;
+  EXPECT_EQ(true, android_mallopt(M_INIT_ZYGOTE_CHILD_PROFILING, nullptr, 0));
+  EXPECT_EQ(0, errno);
+
+  // Unexpected arguments rejected.
+  errno = 0;
+  char unexpected = 0;
+  EXPECT_EQ(false, android_mallopt(M_INIT_ZYGOTE_CHILD_PROFILING, &unexpected, 1));
+  EXPECT_EQ(EINVAL, errno);
+#else
+  GTEST_LOG_(INFO) << "This tests a bionic implementation detail.\n";
+#endif
+}
+