From dda1c6c466c4f31de31d76c8be7e46c16b4b4209 Mon Sep 17 00:00:00 2001 From: Christopher Ferris Date: Wed, 9 Jul 2014 17:16:07 -0700 Subject: [PATCH] Do not include libc_common in malloc debug code. The inclusion of the static libc_common library in the malloc_debug_XXX.so shared libraries causes constructors to be called twice. This doesn't seem to have caused any issues when setting the libc.debug.malloc property. However, jemalloc crashes because there are two jemalloc implementations, one in the static libc_common library and one in the shared library. Each implementation has created overlapping thread keys that are not the same. The crash comes because one of the jemalloc keys is actually used by the locale setting code. Thus if someone sets the locale, the jemalloc code crashes trying to access the same key. Change-Id: Iaac650a82d69064db148a6333e9403744f68b4a4 --- libc/Android.mk | 14 ++++++------ libc/bionic/malloc_debug_check.cpp | 22 ++++++++++--------- libc/bionic/malloc_debug_common.cpp | 12 ++++++++++- libc/bionic/malloc_debug_common.h | 13 +----------- libc/bionic/malloc_debug_leak.cpp | 33 +++++++++++++++-------------- libc/bionic/malloc_debug_qemu.cpp | 33 +++++++++++++++++------------ 6 files changed, 67 insertions(+), 60 deletions(-) diff --git a/libc/Android.mk b/libc/Android.mk index 078be5942..cfebd9cc6 100644 --- a/libc/Android.mk +++ b/libc/Android.mk @@ -491,6 +491,10 @@ libc_common_cflags := \ -D_LIBC=1 \ -Wall -Wextra -Wunused \ +ifneq ($(TARGET_USES_LOGD),false) +libc_common_cflags += -DTARGET_USES_LOGD +endif + # Try to catch typical 32-bit assumptions that break with 64-bit pointers. libc_common_cflags += \ -Werror=pointer-to-int-cast \ @@ -783,10 +787,6 @@ LOCAL_MODULE := libc_bionic LOCAL_ADDITIONAL_DEPENDENCIES := $(libc_common_additional_dependencies) LOCAL_SYSTEM_SHARED_LIBRARIES := -ifneq ($(TARGET_USES_LOGD),false) -LOCAL_CFLAGS += -DTARGET_USES_LOGD -endif - $(eval $(call patch-up-arch-specific-flags,LOCAL_CFLAGS,libc_common_cflags)) $(eval $(call patch-up-arch-specific-flags,LOCAL_SRC_FILES,libc_bionic_src_files)) include $(BUILD_STATIC_LIBRARY) @@ -1011,6 +1011,7 @@ LOCAL_C_INCLUDES := $(libc_common_c_includes) LOCAL_SRC_FILES := \ bionic/debug_mapinfo.cpp \ bionic/debug_stacktrace.cpp \ + bionic/libc_logging.cpp \ bionic/malloc_debug_leak.cpp \ bionic/malloc_debug_check.cpp \ @@ -1018,7 +1019,6 @@ LOCAL_MODULE := libc_malloc_debug_leak LOCAL_ADDITIONAL_DEPENDENCIES := $(libc_common_additional_dependencies) LOCAL_SHARED_LIBRARIES := libc libdl -LOCAL_WHOLE_STATIC_LIBRARIES := libc_common LOCAL_SYSTEM_SHARED_LIBRARIES := LOCAL_ALLOW_UNDEFINED_SYMBOLS := true @@ -1044,13 +1044,13 @@ LOCAL_CPPFLAGS := $(libc_common_cppflags) LOCAL_C_INCLUDES := $(libc_common_c_includes) LOCAL_SRC_FILES := \ - bionic/malloc_debug_qemu.cpp + bionic/libc_logging.cpp \ + bionic/malloc_debug_qemu.cpp \ LOCAL_MODULE := libc_malloc_debug_qemu LOCAL_ADDITIONAL_DEPENDENCIES := $(libc_common_additional_dependencies) LOCAL_SHARED_LIBRARIES := libc libdl -LOCAL_WHOLE_STATIC_LIBRARIES := libc_common LOCAL_SYSTEM_SHARED_LIBRARIES := # Don't install on release build diff --git a/libc/bionic/malloc_debug_check.cpp b/libc/bionic/malloc_debug_check.cpp index e4e4c2eb0..1b6f8ea9d 100644 --- a/libc/bionic/malloc_debug_check.cpp +++ b/libc/bionic/malloc_debug_check.cpp @@ -123,6 +123,7 @@ static pthread_mutex_t backlog_lock = PTHREAD_MUTEX_INITIALIZER; static unsigned g_malloc_debug_backlog = 100; __LIBC_HIDDEN__ HashTable* g_hash_table; +__LIBC_HIDDEN__ const MallocDebug* g_malloc_dispatch; static inline void init_front_guard(hdr_t* hdr) { memset(hdr->front_guard, FRONT_GUARD, FRONT_GUARD_LEN); @@ -324,7 +325,7 @@ static inline void add_to_backlog(hdr_t* hdr) { while (backlog_num > g_malloc_debug_backlog) { hdr_t* gone = backlog_tail; del_from_backlog_locked(gone); - Malloc(free)(gone->base); + g_malloc_dispatch->free(gone->base); } } @@ -336,7 +337,7 @@ extern "C" void* chk_malloc(size_t bytes) { errno = ENOMEM; return NULL; } - hdr_t* hdr = static_cast(Malloc(malloc)(size)); + hdr_t* hdr = static_cast(g_malloc_dispatch->malloc(size)); if (hdr) { hdr->base = hdr; hdr->bt_depth = get_backtrace(hdr->bt, MAX_BACKTRACE_DEPTH); @@ -364,7 +365,7 @@ extern "C" void* chk_memalign(size_t alignment, size_t bytes) { return NULL; } - void* base = Malloc(malloc)(sizeof(hdr_t) + size + sizeof(ftr_t)); + void* base = g_malloc_dispatch->malloc(sizeof(hdr_t) + size + sizeof(ftr_t)); if (base != NULL) { // Check that the actual pointer that will be returned is aligned // properly. @@ -461,7 +462,7 @@ extern "C" void* chk_realloc(void* ptr, size_t bytes) { user(hdr), bytes); log_backtrace(bt, depth); // just get a whole new allocation and leak the old one - return Malloc(realloc)(0, bytes); + return g_malloc_dispatch->realloc(0, bytes); // return realloc(user(hdr), bytes); // assuming it was allocated externally } } @@ -474,15 +475,15 @@ extern "C" void* chk_realloc(void* ptr, size_t bytes) { if (hdr->base != hdr) { // An allocation from memalign, so create another allocation and // copy the data out. - void* newMem = Malloc(malloc)(size); + void* newMem = g_malloc_dispatch->malloc(size); if (newMem == NULL) { return NULL; } memcpy(newMem, hdr, sizeof(hdr_t) + hdr->size); - Malloc(free)(hdr->base); + g_malloc_dispatch->free(hdr->base); hdr = static_cast(newMem); } else { - hdr = static_cast(Malloc(realloc)(hdr, size)); + hdr = static_cast(g_malloc_dispatch->realloc(hdr, size)); } if (hdr) { hdr->base = hdr; @@ -501,7 +502,7 @@ extern "C" void* chk_calloc(size_t nmemb, size_t bytes) { errno = ENOMEM; return NULL; } - hdr_t* hdr = static_cast(Malloc(calloc)(1, size)); + hdr_t* hdr = static_cast(g_malloc_dispatch->calloc(1, size)); if (hdr) { hdr->base = hdr; hdr->bt_depth = get_backtrace(hdr->bt, MAX_BACKTRACE_DEPTH); @@ -524,7 +525,7 @@ extern "C" size_t chk_malloc_usable_size(const void* ptr) { } extern "C" struct mallinfo chk_mallinfo() { - return Malloc(mallinfo)(); + return g_malloc_dispatch->mallinfo(); } extern "C" int chk_posix_memalign(void** memptr, size_t alignment, size_t size) { @@ -584,8 +585,9 @@ static void ReportMemoryLeaks() { } } -extern "C" bool malloc_debug_initialize(HashTable* hash_table) { +extern "C" bool malloc_debug_initialize(HashTable* hash_table, const MallocDebug* malloc_dispatch) { g_hash_table = hash_table; + g_malloc_dispatch = malloc_dispatch; char debug_backlog[PROP_VALUE_MAX]; if (__system_property_get("libc.debug.malloc.backlog", debug_backlog)) { diff --git a/libc/bionic/malloc_debug_common.cpp b/libc/bionic/malloc_debug_common.cpp index 9399237fb..be16625b8 100644 --- a/libc/bionic/malloc_debug_common.cpp +++ b/libc/bionic/malloc_debug_common.cpp @@ -46,6 +46,16 @@ #include "private/ScopedPthreadMutexLocker.h" +#if defined(USE_JEMALLOC) +#include "jemalloc.h" +#define Malloc(function) je_ ## function +#elif defined(USE_DLMALLOC) +#include "dlmalloc.h" +#define Malloc(function) dl ## function +#else +#error "Either one of USE_DLMALLOC or USE_JEMALLOC must be defined." +#endif + // In a VM process, this is set to 1 after fork()ing out of zygote. int gMallocLeakZygoteChild = 0; @@ -408,7 +418,7 @@ static void malloc_init_impl() { dlclose(malloc_impl_handle); return; } - if (!malloc_debug_initialize(&g_hash_table)) { + if (!malloc_debug_initialize(&g_hash_table, &__libc_malloc_default_dispatch)) { dlclose(malloc_impl_handle); return; } diff --git a/libc/bionic/malloc_debug_common.h b/libc/bionic/malloc_debug_common.h index fb2f03d06..5c73da31a 100644 --- a/libc/bionic/malloc_debug_common.h +++ b/libc/bionic/malloc_debug_common.h @@ -51,17 +51,6 @@ #define MALLOC_ALIGNMENT ((size_t)(2 * sizeof(void *))) #endif -#ifdef USE_JEMALLOC -#include "jemalloc.h" -#define Malloc(function) je_ ## function -#else -#ifndef USE_DLMALLOC -#error "Either one of USE_DLMALLOC or USE_JEMALLOC must be defined." -#endif -#include "dlmalloc.h" -#define Malloc(function) dl ## function -#endif - // ============================================================================= // Structures // ============================================================================= @@ -116,7 +105,7 @@ struct MallocDebug { #endif }; -typedef bool (*MallocDebugInit)(HashTable*); +typedef bool (*MallocDebugInit)(HashTable*, const MallocDebug*); typedef void (*MallocDebugFini)(int); // ============================================================================= diff --git a/libc/bionic/malloc_debug_leak.cpp b/libc/bionic/malloc_debug_leak.cpp index 308d40b9a..bc88d2397 100644 --- a/libc/bionic/malloc_debug_leak.cpp +++ b/libc/bionic/malloc_debug_leak.cpp @@ -62,6 +62,7 @@ extern int gMallocLeakZygoteChild; extern HashTable* g_hash_table; +extern const MallocDebug* g_malloc_dispatch; // ============================================================================= // stack trace functions @@ -143,7 +144,7 @@ static HashEntry* record_backtrace(uintptr_t* backtrace, size_t numEntries, size entry->allocations++; } else { // create a new entry - entry = static_cast(Malloc(malloc)(sizeof(HashEntry) + numEntries*sizeof(uintptr_t))); + entry = static_cast(g_malloc_dispatch->malloc(sizeof(HashEntry) + numEntries*sizeof(uintptr_t))); if (!entry) { return NULL; } @@ -208,11 +209,11 @@ static void remove_entry(HashEntry* entry) { #define CHK_SENTINEL_VALUE 0xeb extern "C" void* fill_calloc(size_t n_elements, size_t elem_size) { - return Malloc(calloc)(n_elements, elem_size); + return g_malloc_dispatch->calloc(n_elements, elem_size); } extern "C" void* fill_malloc(size_t bytes) { - void* buffer = Malloc(malloc)(bytes); + void* buffer = g_malloc_dispatch->malloc(bytes); if (buffer) { memset(buffer, CHK_SENTINEL_VALUE, bytes); } @@ -220,17 +221,17 @@ extern "C" void* fill_malloc(size_t bytes) { } extern "C" void fill_free(void* mem) { - size_t bytes = Malloc(malloc_usable_size)(mem); + size_t bytes = g_malloc_dispatch->malloc_usable_size(mem); memset(mem, CHK_FILL_FREE, bytes); - Malloc(free)(mem); + g_malloc_dispatch->free(mem); } extern "C" void* fill_realloc(void* mem, size_t bytes) { - size_t oldSize = Malloc(malloc_usable_size)(mem); - void* newMem = Malloc(realloc)(mem, bytes); + size_t oldSize = g_malloc_dispatch->malloc_usable_size(mem); + void* newMem = g_malloc_dispatch->realloc(mem, bytes); if (newMem) { // If this is larger than before, fill the extra with our pattern. - size_t newSize = Malloc(malloc_usable_size)(newMem); + size_t newSize = g_malloc_dispatch->malloc_usable_size(newMem); if (newSize > oldSize) { memset(reinterpret_cast(reinterpret_cast(newMem)+oldSize), CHK_FILL_FREE, newSize-oldSize); } @@ -239,7 +240,7 @@ extern "C" void* fill_realloc(void* mem, size_t bytes) { } extern "C" void* fill_memalign(size_t alignment, size_t bytes) { - void* buffer = Malloc(memalign)(alignment, bytes); + void* buffer = g_malloc_dispatch->memalign(alignment, bytes); if (buffer) { memset(buffer, CHK_SENTINEL_VALUE, bytes); } @@ -249,11 +250,11 @@ extern "C" void* fill_memalign(size_t alignment, size_t bytes) { extern "C" size_t fill_malloc_usable_size(const void* mem) { // Since we didn't allocate extra bytes before or after, we can // report the normal usable size here. - return Malloc(malloc_usable_size)(mem); + return g_malloc_dispatch->malloc_usable_size(mem); } extern "C" struct mallinfo fill_mallinfo() { - return Malloc(mallinfo)(); + return g_malloc_dispatch->mallinfo(); } extern "C" int fill_posix_memalign(void** memptr, size_t alignment, size_t size) { @@ -298,7 +299,7 @@ extern "C" void* leak_malloc(size_t bytes) { return NULL; } - void* base = Malloc(malloc)(size); + void* base = g_malloc_dispatch->malloc(size); if (base != NULL) { ScopedPthreadMutexLocker locker(&g_hash_table->lock); @@ -342,11 +343,11 @@ extern "C" void leak_free(void* mem) { entry->allocations--; if (entry->allocations <= 0) { remove_entry(entry); - Malloc(free)(entry); + g_malloc_dispatch->free(entry); } // now free the memory! - Malloc(free)(header); + g_malloc_dispatch->free(header); } else { debug_log("WARNING bad header guard: '0x%x'! and invalid entry: %p\n", header->guard, header->entry); @@ -452,7 +453,7 @@ extern "C" size_t leak_malloc_usable_size(const void* mem) { return 0; } - size_t ret = Malloc(malloc_usable_size)(header); + size_t ret = g_malloc_dispatch->malloc_usable_size(header); if (ret != 0) { // The usable area starts at 'mem' and stops at 'header+ret'. return reinterpret_cast(header) + ret - reinterpret_cast(mem); @@ -462,7 +463,7 @@ extern "C" size_t leak_malloc_usable_size(const void* mem) { } extern "C" struct mallinfo leak_mallinfo() { - return Malloc(mallinfo)(); + return g_malloc_dispatch->mallinfo(); } extern "C" int leak_posix_memalign(void** memptr, size_t alignment, size_t size) { diff --git a/libc/bionic/malloc_debug_qemu.cpp b/libc/bionic/malloc_debug_qemu.cpp index fd5161a7e..d0069e1d1 100644 --- a/libc/bionic/malloc_debug_qemu.cpp +++ b/libc/bionic/malloc_debug_qemu.cpp @@ -336,6 +336,9 @@ static void dump_malloc_descriptor(char* str, // Static data // ============================================================================= +// The underlying malloc implementation to use to get memory. +static const MallocDebug* g_malloc_dispatch = NULL; + /* Emulator's magic page address. * This page (mapped on /dev/qemu_trace device) is used to fire up events * in the emulator. */ @@ -595,7 +598,9 @@ extern "C" void* qemu_instrumented_valloc(size_t); * Return: * 0 on success, or -1 on failure. */ -extern "C" bool malloc_debug_initialize(HashTable*) { +extern "C" bool malloc_debug_initialize(HashTable*, const MallocDebug* malloc_dispatch) { + g_malloc_dispatch = malloc_dispatch; + /* We will be using emulator's magic page to report memory allocation * activities. In essence, what magic page does, it translates writes to * the memory mapped spaces into writes to an I/O port that emulator @@ -693,7 +698,7 @@ extern "C" void* qemu_instrumented_malloc(size_t bytes) { errno = ENOMEM; return NULL; } - desc.ptr = Malloc(malloc)(size); + desc.ptr = g_malloc_dispatch->malloc(size); if (desc.ptr == NULL) { qemu_error_log(" malloc(%zu): malloc(%zu) failed.", malloc_pid, getpid(), bytes, size); @@ -704,7 +709,7 @@ extern "C" void* qemu_instrumented_malloc(size_t bytes) { if (notify_qemu_malloc(&desc)) { log_mdesc(error, &desc, ": malloc: notify_malloc failed for ", malloc_pid, getpid()); - Malloc(free)(desc.ptr); + g_malloc_dispatch->free(desc.ptr); errno = ENOMEM; return NULL; } else { @@ -726,7 +731,7 @@ extern "C" void qemu_instrumented_free(void* mem) { if (mem == NULL) { // Just let go NULL free - Malloc(free)(mem); + g_malloc_dispatch->free(mem); return; } @@ -757,7 +762,7 @@ extern "C" void qemu_instrumented_free(void* mem) { } else { log_mdesc(info, &desc, "--- free(%p) -> ", malloc_pid, getpid(), mem); - Malloc(free)(desc.ptr); + g_malloc_dispatch->free(desc.ptr); } } @@ -816,7 +821,7 @@ extern "C" void* qemu_instrumented_calloc(size_t n_elements, size_t elem_size) { total_elements++; desc.suffix_size += (elem_size - total_size); } - desc.ptr = Malloc(calloc)(total_elements, elem_size); + desc.ptr = g_malloc_dispatch->calloc(total_elements, elem_size); if (desc.ptr == NULL) { error_log(" calloc: calloc(%zu(%zu), %zu) (prx=%u, sfx=%u) failed.", malloc_pid, getpid(), n_elements, total_elements, elem_size, @@ -827,7 +832,7 @@ extern "C" void* qemu_instrumented_calloc(size_t n_elements, size_t elem_size) { if (notify_qemu_malloc(&desc)) { log_mdesc(error, &desc, ": calloc(%zu(%zu), %zu): notify_malloc failed for ", malloc_pid, getpid(), n_elements, total_elements, elem_size); - Malloc(free)(desc.ptr); + g_malloc_dispatch->free(desc.ptr); errno = ENOMEM; return NULL; } else { @@ -905,7 +910,7 @@ extern "C" void* qemu_instrumented_realloc(void* mem, size_t bytes) { errno = ENOMEM; return NULL; } - new_desc.ptr = Malloc(malloc)(new_size); + new_desc.ptr = g_malloc_dispatch->malloc(new_size); if (new_desc.ptr == NULL) { log_mdesc(error, &cur_desc, ": realloc(%p, %zu): malloc(%zu) failed on ", malloc_pid, getpid(), mem, bytes, new_size); @@ -924,7 +929,7 @@ extern "C" void* qemu_instrumented_realloc(void* mem, size_t bytes) { log_mdesc(error, &new_desc, ": realloc(%p, %zu) notify_malloc failed -> ", malloc_pid, getpid(), mem, bytes); log_mdesc(error, &cur_desc, " <- "); - Malloc(free)(new_desc.ptr); + g_malloc_dispatch->free(new_desc.ptr); errno = ENOMEM; return NULL; } @@ -940,11 +945,11 @@ extern "C" void* qemu_instrumented_realloc(void* mem, size_t bytes) { /* Since we registered new decriptor with the emulator, we need * to unregister it before freeing newly allocated block. */ notify_qemu_free(mallocdesc_user_ptr(&new_desc)); - Malloc(free)(new_desc.ptr); + g_malloc_dispatch->free(new_desc.ptr); errno = ENOMEM; return NULL; } - Malloc(free)(cur_desc.ptr); + g_malloc_dispatch->free(cur_desc.ptr); log_mdesc(info, &new_desc, "=== : realloc(%p, %zu) -> ", malloc_pid, getpid(), mem, bytes); @@ -985,7 +990,7 @@ extern "C" void* qemu_instrumented_memalign(size_t alignment, size_t bytes) { return NULL; } - desc.ptr = Malloc(memalign)(desc.prefix_size, size); + desc.ptr = g_malloc_dispatch->memalign(desc.prefix_size, size); if (desc.ptr == NULL) { error_log(" memalign(%zx, %zu): malloc(%zu) failed.", malloc_pid, getpid(), alignment, bytes, size); @@ -994,7 +999,7 @@ extern "C" void* qemu_instrumented_memalign(size_t alignment, size_t bytes) { if (notify_qemu_malloc(&desc)) { log_mdesc(error, &desc, ": memalign(%zx, %zu): notify_malloc failed for ", malloc_pid, getpid(), alignment, bytes); - Malloc(free)(desc.ptr); + g_malloc_dispatch->free(desc.ptr); return NULL; } @@ -1032,7 +1037,7 @@ extern "C" size_t qemu_instrumented_malloc_usable_size(const void* mem) { } extern "C" struct mallinfo qemu_instrumented_mallinfo() { - return Malloc(mallinfo)(); + return g_malloc_dispatch->mallinfo(); } extern "C" int qemu_instrumented_posix_memalign(void** memptr, size_t alignment, size_t size) {