diff options
author | android-build-team Robot <android-build-team-robot@google.com> | 2020-05-22 07:05:22 +0000 |
---|---|---|
committer | android-build-team Robot <android-build-team-robot@google.com> | 2020-05-22 07:05:22 +0000 |
commit | fae1c4e23b161c3843bc4edc35c5bd04e99af9e1 (patch) | |
tree | 936cf492400fea213549c28e1357f86e869f002f | |
parent | a4cd8d6c7aeaef346106ba159c2b8326ab72a625 (diff) | |
parent | c9ecba8d4a9f33f11e676d60db9e1262deb352db (diff) | |
download | bionic-fae1c4e23b161c3843bc4edc35c5bd04e99af9e1.tar.gz |
Snap for 6520975 from c9ecba8d4a9f33f11e676d60db9e1262deb352db to mainline-release
Change-Id: I9e1fdc50ed244ec80a09d7bd91a323e4b37edbf4
-rw-r--r-- | libc/malloc_debug/Android.bp | 1 | ||||
-rw-r--r-- | libc/malloc_debug/malloc_debug.cpp | 51 | ||||
-rw-r--r-- | libc/malloc_debug/tests/malloc_debug_system_tests.cpp | 47 | ||||
-rw-r--r-- | libc/platform/bionic/reserved_signals.h | 1 |
4 files changed, 98 insertions, 2 deletions
diff --git a/libc/malloc_debug/Android.bp b/libc/malloc_debug/Android.bp index b04cfe1b2..760039b9c 100644 --- a/libc/malloc_debug/Android.bp +++ b/libc/malloc_debug/Android.bp @@ -173,6 +173,7 @@ cc_test { shared_libs: [ "libbase", + "libbacktrace", "liblog", "libunwindstack", ], diff --git a/libc/malloc_debug/malloc_debug.cpp b/libc/malloc_debug/malloc_debug.cpp index 6eaac7de8..609f030bf 100644 --- a/libc/malloc_debug/malloc_debug.cpp +++ b/libc/malloc_debug/malloc_debug.cpp @@ -30,11 +30,13 @@ #include <inttypes.h> #include <malloc.h> #include <pthread.h> +#include <signal.h> #include <stdio.h> #include <stdlib.h> #include <string.h> #include <sys/cdefs.h> #include <sys/param.h> +#include <sys/syscall.h> #include <unistd.h> #include <mutex> @@ -44,8 +46,9 @@ #include <android-base/properties.h> #include <android-base/stringprintf.h> #include <bionic/malloc_tagged_pointers.h> -#include <private/bionic_malloc_dispatch.h> +#include <platform/bionic/reserved_signals.h> #include <private/MallocXmlElem.h> +#include <private/bionic_malloc_dispatch.h> #include "Config.h" #include "DebugData.h" @@ -131,6 +134,40 @@ class ScopedConcurrentLock { }; pthread_rwlock_t ScopedConcurrentLock::lock_; +// Use this because the sigprocmask* functions filter out the reserved bionic +// signals including the signal this code blocks. +static inline int __rt_sigprocmask(int how, const sigset64_t* new_set, sigset64_t* old_set, + size_t sigset_size) { + return syscall(SYS_rt_sigprocmask, how, new_set, old_set, sigset_size); +} + +// Need to block the backtrace signal while in malloc debug routines +// otherwise there is a chance of a deadlock and timeout when unwinding. +// This can occur if a thread is paused while owning a malloc debug +// internal lock. +class ScopedBacktraceSignalBlocker { + public: + ScopedBacktraceSignalBlocker() { + sigemptyset64(&backtrace_set_); + sigaddset64(&backtrace_set_, BIONIC_SIGNAL_BACKTRACE); + sigset64_t old_set; + __rt_sigprocmask(SIG_BLOCK, &backtrace_set_, &old_set, sizeof(backtrace_set_)); + if (sigismember64(&old_set, BIONIC_SIGNAL_BACKTRACE)) { + unblock_ = false; + } + } + + ~ScopedBacktraceSignalBlocker() { + if (unblock_) { + __rt_sigprocmask(SIG_UNBLOCK, &backtrace_set_, nullptr, sizeof(backtrace_set_)); + } + } + + private: + bool unblock_ = true; + sigset64_t backtrace_set_; +}; + static void InitAtfork() { static pthread_once_t atfork_init = PTHREAD_ONCE_INIT; pthread_once(&atfork_init, []() { @@ -334,8 +371,8 @@ void debug_finalize() { void debug_get_malloc_leak_info(uint8_t** info, size_t* overall_size, size_t* info_size, size_t* total_memory, size_t* backtrace_size) { ScopedConcurrentLock lock; - ScopedDisableDebugCalls disable; + ScopedBacktraceSignalBlocker blocked; // Verify the arguments. if (info == nullptr || overall_size == nullptr || info_size == nullptr || total_memory == nullptr || @@ -370,6 +407,7 @@ size_t debug_malloc_usable_size(void* pointer) { } ScopedConcurrentLock lock; ScopedDisableDebugCalls disable; + ScopedBacktraceSignalBlocker blocked; if (!VerifyPointer(pointer, "malloc_usable_size")) { return 0; @@ -434,6 +472,7 @@ void* debug_malloc(size_t size) { } ScopedConcurrentLock lock; ScopedDisableDebugCalls disable; + ScopedBacktraceSignalBlocker blocked; void* pointer = InternalMalloc(size); @@ -510,6 +549,7 @@ void debug_free(void* pointer) { } ScopedConcurrentLock lock; ScopedDisableDebugCalls disable; + ScopedBacktraceSignalBlocker blocked; if (g_debug->config().options() & RECORD_ALLOCS) { g_debug->record->AddEntry(new FreeEntry(pointer)); @@ -528,6 +568,7 @@ void* debug_memalign(size_t alignment, size_t bytes) { } ScopedConcurrentLock lock; ScopedDisableDebugCalls disable; + ScopedBacktraceSignalBlocker blocked; if (bytes == 0) { bytes = 1; @@ -607,6 +648,7 @@ void* debug_realloc(void* pointer, size_t bytes) { } ScopedConcurrentLock lock; ScopedDisableDebugCalls disable; + ScopedBacktraceSignalBlocker blocked; if (pointer == nullptr) { pointer = InternalMalloc(bytes); @@ -726,6 +768,7 @@ void* debug_calloc(size_t nmemb, size_t bytes) { } ScopedConcurrentLock lock; ScopedDisableDebugCalls disable; + ScopedBacktraceSignalBlocker blocked; size_t size; if (__builtin_mul_overflow(nmemb, bytes, &size)) { @@ -792,6 +835,7 @@ int debug_malloc_info(int options, FILE* fp) { ScopedConcurrentLock lock; ScopedDisableDebugCalls disable; + ScopedBacktraceSignalBlocker blocked; // Avoid any issues where allocations are made that will be freed // in the fclose. @@ -880,6 +924,7 @@ ssize_t debug_malloc_backtrace(void* pointer, uintptr_t* frames, size_t max_fram } ScopedConcurrentLock lock; ScopedDisableDebugCalls disable; + ScopedBacktraceSignalBlocker blocked; if (!(g_debug->config().options() & BACKTRACE)) { return 0; @@ -938,6 +983,7 @@ bool debug_write_malloc_leak_info(FILE* fp) { ScopedConcurrentLock lock; ScopedDisableDebugCalls disable; + ScopedBacktraceSignalBlocker blocked; std::lock_guard<std::mutex> guard(g_dump_lock); @@ -953,6 +999,7 @@ bool debug_write_malloc_leak_info(FILE* fp) { void debug_dump_heap(const char* file_name) { ScopedConcurrentLock lock; ScopedDisableDebugCalls disable; + ScopedBacktraceSignalBlocker blocked; std::lock_guard<std::mutex> guard(g_dump_lock); diff --git a/libc/malloc_debug/tests/malloc_debug_system_tests.cpp b/libc/malloc_debug/tests/malloc_debug_system_tests.cpp index 9e612f03b..f260db71a 100644 --- a/libc/malloc_debug/tests/malloc_debug_system_tests.cpp +++ b/libc/malloc_debug/tests/malloc_debug_system_tests.cpp @@ -42,12 +42,20 @@ #include <gtest/gtest.h> #include <log/log.h> +#include <atomic> #include <string> #include <thread> #include <vector> +#include <backtrace/Backtrace.h> +#include <backtrace/BacktraceMap.h> + #include <bionic/malloc.h> +// All DISABLED_ tests are designed to be executed after malloc debug +// is enabled. These tests don't run be default, and are executed +// by wrappers that will enable various malloc debug features. + static constexpr time_t kTimeoutSeconds = 10; extern "C" bool GetInitialArgs(const char*** args, size_t* num_args) { @@ -525,3 +533,42 @@ TEST(MallocDebugSystemTest, write_leak_info_header) { ASSERT_NO_FATAL_FAILURE(FindStrings(pid, std::vector<const char*>{"malloc debug enabled"}, std::vector<const char*>{" HAS INVALID TAG ", "USED AFTER FREE ", "UNKNOWN POINTER "})); } + +TEST(MallocTests, DISABLED_malloc_and_backtrace_deadlock) { + std::atomic_bool running(false); + pid_t tid; + std::thread thread([&tid, &running] { + tid = gettid(); + running = true; + while (running) { + void* ptr = malloc(200); + if (ptr == nullptr) { + return; + } + free(ptr); + } + }); + + while (!running) { + } + + static constexpr size_t kNumUnwinds = 1000; + for (size_t i = 0; i < kNumUnwinds; i++) { + std::unique_ptr<Backtrace> backtrace(Backtrace::Create(getpid(), tid)); + ASSERT_TRUE(backtrace->Unwind(0)) << "Failed on unwind " << i; + ASSERT_LT(1, backtrace->NumFrames()) << "Failed on unwind " << i; + } + running = false; + thread.join(); +} + +TEST(MallocDebugSystemTest, malloc_and_backtrace_deadlock) { + pid_t pid; + ASSERT_NO_FATAL_FAILURE(Exec("MallocTests.DISABLED_malloc_and_backtrace_deadlock", + "verbose verify_pointers", &pid, 0, 180)); + + // Make sure that malloc debug is enabled and that no timeouts occur during + // unwinds. + ASSERT_NO_FATAL_FAILURE(FindStrings(pid, std::vector<const char*>{"malloc debug enabled"}, + std::vector<const char*>{"Timed out waiting for "})); +} diff --git a/libc/platform/bionic/reserved_signals.h b/libc/platform/bionic/reserved_signals.h index c90fc0698..e8e517e20 100644 --- a/libc/platform/bionic/reserved_signals.h +++ b/libc/platform/bionic/reserved_signals.h @@ -48,6 +48,7 @@ // in <android/legacy_signal_inlines.h> to match. #define BIONIC_SIGNAL_POSIX_TIMERS (__SIGRTMIN + 0) +#define BIONIC_SIGNAL_BACKTRACE (__SIGRTMIN + 1) #define BIONIC_SIGNAL_DEBUGGER (__SIGRTMIN + 3) #define BIONIC_SIGNAL_PROFILER (__SIGRTMIN + 4) #define BIONIC_SIGNAL_ART_PROFILER (__SIGRTMIN + 6) |