From: Elliott Hughes Date: Tue, 16 Jul 2013 20:14:24 +0000 (-0700) Subject: Improve stack overflow diagnostics. X-Git-Tag: android-x86-4.4-r1~19^2~23^2^2~1 X-Git-Url: http://git.osdn.net/view?a=commitdiff_plain;h=aa754dca90487356cabf07ade0e8d88c2630b784;p=android-x86%2Fbionic.git Improve stack overflow diagnostics. We notify debuggerd of problems by installing signal handlers. That's fine except for when the signal is caused by us running off the end of a thread's stack and into the guard page. Bug: 8557703 Change-Id: I1ef65b4bb3bbca7e9a9743056177094921e60ed3 --- diff --git a/libc/bionic/pthread_attr.cpp b/libc/bionic/pthread_attr.cpp index fe1ed4a98..d7c6c1396 100644 --- a/libc/bionic/pthread_attr.cpp +++ b/libc/bionic/pthread_attr.cpp @@ -30,12 +30,16 @@ #include "pthread_internal.h" -#define DEFAULT_STACK_SIZE (1024 * 1024) +// Traditionally we give threads a 1MiB stack. When we started allocating per-thread +// alternate signal stacks to ease debugging of stack overflows, we subtracted the +// same amount we were using there from the default thread stack size. This should +// keep memory usage roughly constant. +#define DEFAULT_THREAD_STACK_SIZE ((1 * 1024 * 1024) - SIGSTKSZ) int pthread_attr_init(pthread_attr_t* attr) { attr->flags = 0; attr->stack_base = NULL; - attr->stack_size = DEFAULT_STACK_SIZE; + attr->stack_size = DEFAULT_THREAD_STACK_SIZE; attr->guard_size = PAGE_SIZE; attr->sched_policy = SCHED_NORMAL; attr->sched_priority = 0; diff --git a/libc/bionic/pthread_create.cpp b/libc/bionic/pthread_create.cpp index f45f5e769..5908a1bd2 100644 --- a/libc/bionic/pthread_create.cpp +++ b/libc/bionic/pthread_create.cpp @@ -62,6 +62,15 @@ void __init_tls(pthread_internal_t* thread) { thread->tls[i] = NULL; } + // Create and set an alternate signal stack. + stack_t ss; + ss.ss_sp = mmap(NULL, SIGSTKSZ, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, 0, 0); + if (ss.ss_sp != MAP_FAILED) { + ss.ss_size = SIGSTKSZ; + ss.ss_flags = 0; + sigaltstack(&ss, NULL); + } + // Slot 0 must point to itself. The x86 Linux kernel reads the TLS from %fs:0. thread->tls[TLS_SLOT_SELF] = thread->tls; thread->tls[TLS_SLOT_THREAD_ID] = thread; @@ -71,7 +80,9 @@ void __init_tls(pthread_internal_t* thread) { __set_tls(thread->tls); } -// This trampoline is called from the assembly _pthread_clone() function. +// This trampoline is called from the assembly _pthread_clone function. +// Our 'tls' and __pthread_clone's 'child_stack' are one and the same, just growing in +// opposite directions. extern "C" void __thread_entry(void* (*func)(void*), void* arg, void** tls) { // Wait for our creating thread to release us. This lets it have time to // notify gdb about this thread before we start doing anything. @@ -187,8 +198,12 @@ int pthread_create(pthread_t* thread_out, pthread_attr_t const* attr, thread->attr.flags |= PTHREAD_ATTR_FLAG_USER_STACK; } - // Make room for TLS. + // Make room for the TLS area. + // The child stack is the same address, just growing in the opposite direction. + // At offsets >= 0, we have the TLS slots. + // At offsets < 0, we have the child stack. void** tls = (void**)((uint8_t*)(thread->attr.stack_base) + thread->attr.stack_size - BIONIC_TLS_SLOTS * sizeof(void*)); + void* child_stack = tls; // Create a mutex for the thread in TLS_SLOT_SELF to wait on once it starts so we can keep // it from doing anything until after we notify the debugger about it @@ -204,7 +219,7 @@ int pthread_create(pthread_t* thread_out, pthread_attr_t const* attr, int flags = CLONE_FILES | CLONE_FS | CLONE_VM | CLONE_SIGHAND | CLONE_THREAD | CLONE_SYSVSEM; - int tid = __pthread_clone(start_routine, tls, flags, arg); + int tid = __pthread_clone(start_routine, child_stack, flags, arg); if (tid < 0) { int clone_errno = errno; if ((thread->attr.flags & PTHREAD_ATTR_FLAG_USER_STACK) == 0) { diff --git a/linker/debugger.cpp b/linker/debugger.cpp index a7c0591d9..d72aa39ef 100644 --- a/linker/debugger.cpp +++ b/linker/debugger.cpp @@ -28,14 +28,15 @@ #include "linker.h" +#include +#include #include #include -#include -#include +#include #include -#include #include #include +#include extern "C" int tgkill(int tgid, int tid, int sig); @@ -109,7 +110,7 @@ static int socket_abstract_client(const char* name, int type) { * mutex is being held, so we don't want to use any libc functions that * could allocate memory or hold a lock. */ -static void logSignalSummary(int signum, const siginfo_t* info) { +static void log_signal_summary(int signum, const siginfo_t* info) { const char* signal_name; switch (signum) { case SIGILL: signal_name = "SIGILL"; break; @@ -149,26 +150,26 @@ static void logSignalSummary(int signum, const siginfo_t* info) { /* * Returns true if the handler for signal "signum" has SA_SIGINFO set. */ -static bool haveSiginfo(int signum) { - struct sigaction oldact, newact; +static bool have_siginfo(int signum) { + struct sigaction old_action, new_action; - memset(&newact, 0, sizeof(newact)); - newact.sa_handler = SIG_DFL; - newact.sa_flags = SA_RESTART; - sigemptyset(&newact.sa_mask); + memset(&new_action, 0, sizeof(new_action)); + new_action.sa_handler = SIG_DFL; + new_action.sa_flags = SA_RESTART; + sigemptyset(&new_action.sa_mask); - if (sigaction(signum, &newact, &oldact) < 0) { + if (sigaction(signum, &new_action, &old_action) < 0) { __libc_format_log(ANDROID_LOG_WARN, "libc", "Failed testing for SA_SIGINFO: %s", strerror(errno)); return false; } - bool ret = (oldact.sa_flags & SA_SIGINFO) != 0; + bool result = (old_action.sa_flags & SA_SIGINFO) != 0; - if (sigaction(signum, &oldact, NULL) == -1) { + if (sigaction(signum, &old_action, NULL) == -1) { __libc_format_log(ANDROID_LOG_WARN, "libc", "Restore failed in test for SA_SIGINFO: %s", strerror(errno)); } - return ret; + return result; } /* @@ -180,11 +181,11 @@ void debuggerd_signal_handler(int n, siginfo_t* info, void*) { * It's possible somebody cleared the SA_SIGINFO flag, which would mean * our "info" arg holds an undefined value. */ - if (!haveSiginfo(n)) { + if (!have_siginfo(n)) { info = NULL; } - logSignalSummary(n, info); + log_signal_summary(n, info); pid_t tid = gettid(); int s = socket_abstract_client(DEBUGGER_SOCKET_NAME, SOCK_STREAM); @@ -245,19 +246,23 @@ void debuggerd_signal_handler(int n, siginfo_t* info, void*) { } void debuggerd_init() { - struct sigaction act; - memset(&act, 0, sizeof(act)); - act.sa_sigaction = debuggerd_signal_handler; - act.sa_flags = SA_RESTART | SA_SIGINFO; - sigemptyset(&act.sa_mask); - - sigaction(SIGILL, &act, NULL); - sigaction(SIGABRT, &act, NULL); - sigaction(SIGBUS, &act, NULL); - sigaction(SIGFPE, &act, NULL); - sigaction(SIGSEGV, &act, NULL); + struct sigaction action; + memset(&action, 0, sizeof(action)); + sigemptyset(&action.sa_mask); + action.sa_sigaction = debuggerd_signal_handler; + action.sa_flags = SA_RESTART | SA_SIGINFO; + + // Use the alternate signal stack if available so we can catch stack overflows. + action.sa_flags |= SA_ONSTACK; + + sigaction(SIGABRT, &action, NULL); + sigaction(SIGBUS, &action, NULL); + sigaction(SIGFPE, &action, NULL); + sigaction(SIGILL, &action, NULL); + sigaction(SIGPIPE, &action, NULL); + sigaction(SIGSEGV, &action, NULL); #if defined(SIGSTKFLT) - sigaction(SIGSTKFLT, &act, NULL); + sigaction(SIGSTKFLT, &action, NULL); #endif - sigaction(SIGPIPE, &act, NULL); + sigaction(SIGTRAP, &action, NULL); }