Merge "Enable malloc debug using environment variables"

10 files changed, 33 insertions, 100 deletions

diff --git a/libc/bionic/malloc_common.cpp b/libc/bionic/malloc_common.cpp
index e05061917..3fceb71ee 100644
--- a/libc/bionic/malloc_common.cpp
+++ b/libc/bionic/malloc_common.cpp
@@ -175,8 +175,7 @@ extern "C" int __cxa_atexit(void (*func)(void *), void *arg, void *dso);
 static const char* DEBUG_SHARED_LIB = "libc_malloc_debug.so";
 static const char* DEBUG_MALLOC_PROPERTY_OPTIONS = "libc.debug.malloc.options";
 static const char* DEBUG_MALLOC_PROPERTY_PROGRAM = "libc.debug.malloc.program";
-static const char* DEBUG_MALLOC_PROPERTY_ENV_ENABLED = "libc.debug.malloc.env_enabled";
-static const char* DEBUG_MALLOC_ENV_ENABLE = "LIBC_DEBUG_MALLOC_ENABLE";
+static const char* DEBUG_MALLOC_ENV_OPTIONS = "LIBC_DEBUG_MALLOC_OPTIONS";
 
 static void* libc_malloc_impl_handle = nullptr;
 
@@ -309,20 +308,21 @@ static void malloc_fini_impl(void*) {
 // Initializes memory allocation framework once per process.
 static void malloc_init_impl(libc_globals* globals) {
   char value[PROP_VALUE_MAX];
-  if (__system_property_get(DEBUG_MALLOC_PROPERTY_OPTIONS, value) == 0 || value[0] == '\0') {
-    return;
-  }
 
-  // Check to see if only a specific program should have debug malloc enabled.
-  if (__system_property_get(DEBUG_MALLOC_PROPERTY_PROGRAM, value) != 0 &&
-      strstr(getprogname(), value) == nullptr) {
-    return;
-  }
+  // If DEBUG_MALLOC_ENV_OPTIONS is set then it overrides the system properties.
+  const char* options = getenv(DEBUG_MALLOC_ENV_OPTIONS);
+  if (options == nullptr || options[0] == '\0') {
+    if (__system_property_get(DEBUG_MALLOC_PROPERTY_OPTIONS, value) == 0 || value[0] == '\0') {
+      return;
+    }
+    options = value;
 
-  // Check for the special environment variable instead.
-  if (__system_property_get(DEBUG_MALLOC_PROPERTY_ENV_ENABLED, value) != 0
-      && value[0] != '\0' && getenv(DEBUG_MALLOC_ENV_ENABLE) == nullptr) {
-    return;
+    // Check to see if only a specific program should have debug malloc enabled.
+    char program[PROP_VALUE_MAX];
+    if (__system_property_get(DEBUG_MALLOC_PROPERTY_PROGRAM, program) != 0 &&
+        strstr(getprogname(), program) == nullptr) {
+      return;
+    }
   }
 
   // Load the debug malloc shared library.
@@ -334,7 +334,7 @@ static void malloc_init_impl(libc_globals* globals) {
   }
 
   // Initialize malloc debugging in the loaded module.
-  auto init_func = reinterpret_cast<bool (*)(const MallocDispatch*, int*)>(
+  auto init_func = reinterpret_cast<bool (*)(const MallocDispatch*, int*, const char*)>(
       dlsym(malloc_impl_handle, "debug_initialize"));
   if (init_func == nullptr) {
     error_log("%s: debug_initialize routine not found in %s", getprogname(), DEBUG_SHARED_LIB);
@@ -374,7 +374,7 @@ static void malloc_init_impl(libc_globals* globals) {
     return;
   }
 
-  if (!init_func(&__libc_malloc_default_dispatch, &gMallocLeakZygoteChild)) {
+  if (!init_func(&__libc_malloc_default_dispatch, &gMallocLeakZygoteChild, options)) {
     dlclose(malloc_impl_handle);
     return;
   }
diff --git a/libc/malloc_debug/Android.bp b/libc/malloc_debug/Android.bp
index 8ce3ff373..708c10151 100644
--- a/libc/malloc_debug/Android.bp
+++ b/libc/malloc_debug/Android.bp
@@ -111,7 +111,6 @@ cc_test {
         "tests/backtrace_fake.cpp",
         "tests/log_fake.cpp",
         "tests/libc_fake.cpp",
-        "tests/property_fake.cpp",
         "tests/malloc_debug_config_tests.cpp",
         "tests/malloc_debug_unit_tests.cpp",
     ],
diff --git a/libc/malloc_debug/Config.cpp b/libc/malloc_debug/Config.cpp
index c9fb850d8..cb75bd603 100644
--- a/libc/malloc_debug/Config.cpp
+++ b/libc/malloc_debug/Config.cpp
@@ -37,8 +37,6 @@
 #include <string>
 #include <vector>
 
-#include <sys/system_properties.h>
-
 #include <private/bionic_macros.h>
 
 #include "Config.h"
@@ -329,13 +327,7 @@ void PropertyParser::LogUsage() {
 
 // This function is designed to be called once. A second call will not
 // reset all variables.
-bool Config::SetFromProperties() {
-  char property_str[PROP_VALUE_MAX];
-  memset(property_str, 0, sizeof(property_str));
-  if (!__system_property_get("libc.debug.malloc.options", property_str)) {
-    return false;
-  }
-
+bool Config::Set(const char* options_str) {
   // Initialize a few default values.
   fill_alloc_value = DEFAULT_FILL_ALLOC_VALUE;
   fill_free_value = DEFAULT_FILL_FREE_VALUE;
@@ -426,7 +418,7 @@ bool Config::SetFromProperties() {
   }
 
   // Process each property name we can find.
-  PropertyParser parser(property_str);
+  PropertyParser parser(options_str);
   bool valid = true;
   std::string property;
   std::string value;
diff --git a/libc/malloc_debug/Config.h b/libc/malloc_debug/Config.h
index ac620adfe..ca56dc8c9 100644
--- a/libc/malloc_debug/Config.h
+++ b/libc/malloc_debug/Config.h
@@ -56,7 +56,7 @@ constexpr size_t MINIMUM_ALIGNMENT_BYTES = 8;
 constexpr uint64_t HEADER_OPTIONS = FRONT_GUARD | REAR_GUARD | BACKTRACE | FREE_TRACK | LEAK_TRACK;
 
 struct Config {
-  bool SetFromProperties();
+  bool Set(const char* str);
 
   size_t front_guard_bytes = 0;
   size_t rear_guard_bytes = 0;
diff --git a/libc/malloc_debug/DebugData.cpp b/libc/malloc_debug/DebugData.cpp
index fdc28100f..339efdf1b 100644
--- a/libc/malloc_debug/DebugData.cpp
+++ b/libc/malloc_debug/DebugData.cpp
@@ -37,8 +37,8 @@
 #include "malloc_debug.h"
 #include "TrackData.h"
 
-bool DebugData::Initialize() {
-  if (!config_.SetFromProperties()) {
+bool DebugData::Initialize(const char* options) {
+  if (!config_.Set(options)) {
     return false;
   }
 
diff --git a/libc/malloc_debug/DebugData.h b/libc/malloc_debug/DebugData.h
index 7e2df0c36..7228a722e 100644
--- a/libc/malloc_debug/DebugData.h
+++ b/libc/malloc_debug/DebugData.h
@@ -49,7 +49,7 @@ class DebugData {
   DebugData() = default;
   ~DebugData() = default;
 
-  bool Initialize();
+  bool Initialize(const char* options);
 
   static bool Disabled();
 
diff --git a/libc/malloc_debug/malloc_debug.cpp b/libc/malloc_debug/malloc_debug.cpp
index d2bcf9967..bb16faaae 100644
--- a/libc/malloc_debug/malloc_debug.cpp
+++ b/libc/malloc_debug/malloc_debug.cpp
@@ -62,7 +62,8 @@ const MallocDispatch* g_dispatch;
 // ------------------------------------------------------------------------
 __BEGIN_DECLS
 
-bool debug_initialize(const MallocDispatch* malloc_dispatch, int* malloc_zygote_child);
+bool debug_initialize(const MallocDispatch* malloc_dispatch, int* malloc_zygote_child,
+    const char* options);
 void debug_finalize();
 void debug_get_malloc_leak_info(
     uint8_t** info, size_t* overall_size, size_t* info_size, size_t* total_memory,
@@ -178,8 +179,9 @@ static void* InitHeader(Header* header, void* orig_pointer, size_t size) {
   return g_debug->GetPointer(header);
 }
 
-bool debug_initialize(const MallocDispatch* malloc_dispatch, int* malloc_zygote_child) {
-  if (malloc_zygote_child == nullptr) {
+bool debug_initialize(const MallocDispatch* malloc_dispatch, int* malloc_zygote_child,
+    const char* options) {
+  if (malloc_zygote_child == nullptr || options == nullptr) {
     return false;
   }
 
@@ -194,7 +196,7 @@ bool debug_initialize(const MallocDispatch* malloc_dispatch, int* malloc_zygote_
   }
 
   DebugData* debug = new DebugData();
-  if (!debug->Initialize()) {
+  if (!debug->Initialize(options)) {
     delete debug;
     DebugDisableFinalize();
     return false;
diff --git a/libc/malloc_debug/tests/malloc_debug_config_tests.cpp b/libc/malloc_debug/tests/malloc_debug_config_tests.cpp
index 49edabae6..f988124fe 100644
--- a/libc/malloc_debug/tests/malloc_debug_config_tests.cpp
+++ b/libc/malloc_debug/tests/malloc_debug_config_tests.cpp
@@ -25,8 +25,6 @@
 
 #include "log_fake.h"
 
-extern "C" int property_set(const char*, const char*);
-
 class MallocDebugConfigTest : public ::testing::Test {
  protected:
   void SetUp() override {
@@ -38,10 +36,9 @@ class MallocDebugConfigTest : public ::testing::Test {
 
   std::unique_ptr<Config> config;
 
-  bool InitConfig(const char* property_value) {
+  bool InitConfig(const char* options) {
     config.reset(new Config);
-    property_set("libc.debug.malloc.options", property_value);
-    return config->SetFromProperties();
+    return config->Set(options);
   }
 };
 
diff --git a/libc/malloc_debug/tests/malloc_debug_unit_tests.cpp b/libc/malloc_debug/tests/malloc_debug_unit_tests.cpp
index edb03f6de..1b08a39dc 100644
--- a/libc/malloc_debug/tests/malloc_debug_unit_tests.cpp
+++ b/libc/malloc_debug/tests/malloc_debug_unit_tests.cpp
@@ -44,8 +44,7 @@
 
 __BEGIN_DECLS
 
-int property_set(const char*, const char*);
-bool debug_initialize(const MallocDispatch*, int*);
+bool debug_initialize(const MallocDispatch*, int*, const char*);
 void debug_finalize();
 
 void* debug_malloc(size_t);
@@ -98,10 +97,9 @@ class MallocDebugTest : public ::testing::Test {
     }
   }
 
-  void Init(const char* property_value) {
-    property_set("libc.debug.malloc.options", property_value);
+  void Init(const char* options) {
     zygote = 0;
-    ASSERT_TRUE(debug_initialize(&dispatch, &zygote));
+    ASSERT_TRUE(debug_initialize(&dispatch, &zygote, options));
     initialized = true;
   }
 
diff --git a/libc/malloc_debug/tests/property_fake.cpp b/libc/malloc_debug/tests/property_fake.cpp
deleted file mode 100644
index d9f0ad897..000000000
--- a/libc/malloc_debug/tests/property_fake.cpp
+++ /dev/null
@@ -1,55 +0,0 @@
-/*
- * Copyright (C) 2015 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include <string.h>
-
-#include <string>
-#include <unordered_map>
-
-#include <sys/system_properties.h>
-
-std::unordered_map<std::string, std::string> g_properties;
-
-extern "C" int property_set(const char* name, const char* value) {
-  if (g_properties.count(name) != 0) {
-    g_properties.erase(name);
-  }
-  g_properties[name] = value;
-  return 0;
-}
-
-extern "C" int property_get(const char* key, char* value, const char* default_value) {
-  if (g_properties.count(key) == 0) {
-    if (default_value == nullptr) {
-      return 0;
-    }
-    strncpy(value, default_value, PROP_VALUE_MAX-1);
-  } else {
-    strncpy(value, g_properties[key].c_str(), PROP_VALUE_MAX-1);
-  }
-  value[PROP_VALUE_MAX-1] = '\0';
-  return strlen(value);
-}
-
-extern "C" int __system_property_get(const char* key, char* value) {
-  if (g_properties.count(key) == 0) {
-    return 0;
-  } else {
-    strncpy(value, g_properties[key].c_str(), PROP_VALUE_MAX-1);
-  }
-  value[PROP_VALUE_MAX-1] = '\0';
-  return strlen(value);
-}