Improve stack overflow diagnostics (take 2).

This reverts commits eb1b07469f and
d14dc3b87f, and fixes the bug where
we were calling mmap (which might cause errno to be set) before
__set_tls (which is required to implement errno).

Bug: 8557703
Change-Id: I2c36d00240c56e156e1bb430d8c22a73a068b70c
This commit is contained in:
Elliott Hughes 2013-07-17 13:33:19 -07:00
parent 40e7a87864
commit 84114c8dd5
5 changed files with 74 additions and 33 deletions

View file

@ -31,6 +31,7 @@
#include <errno.h> #include <errno.h>
#include <limits.h> #include <limits.h>
#include <sys/atomics.h> #include <sys/atomics.h>
#include <sys/mman.h>
#include <unistd.h> #include <unistd.h>
#include "bionic_atomic_inline.h" #include "bionic_atomic_inline.h"
@ -102,6 +103,18 @@ void pthread_exit(void * retval)
// space (see pthread_key_delete) // space (see pthread_key_delete)
pthread_key_clean_all(); pthread_key_clean_all();
if (thread->alternate_signal_stack != NULL) {
// Tell the kernel to stop using the alternate signal stack.
stack_t ss;
ss.ss_sp = NULL;
ss.ss_flags = SS_DISABLE;
sigaltstack(&ss, NULL);
// Free it.
munmap(thread->alternate_signal_stack, SIGSTKSZ);
thread->alternate_signal_stack = NULL;
}
// if the thread is detached, destroy the pthread_internal_t // if the thread is detached, destroy the pthread_internal_t
// otherwise, keep it in memory and signal any joiners. // otherwise, keep it in memory and signal any joiners.
pthread_mutex_lock(&gThreadListLock); pthread_mutex_lock(&gThreadListLock);

View file

@ -30,12 +30,16 @@
#include "pthread_internal.h" #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) { int pthread_attr_init(pthread_attr_t* attr) {
attr->flags = 0; attr->flags = 0;
attr->stack_base = NULL; attr->stack_base = NULL;
attr->stack_size = DEFAULT_STACK_SIZE; attr->stack_size = DEFAULT_THREAD_STACK_SIZE;
attr->guard_size = PAGE_SIZE; attr->guard_size = PAGE_SIZE;
attr->sched_policy = SCHED_NORMAL; attr->sched_policy = SCHED_NORMAL;
attr->sched_priority = 0; attr->sched_priority = 0;

View file

@ -69,9 +69,22 @@ void __init_tls(pthread_internal_t* thread) {
thread->tls[TLS_SLOT_STACK_GUARD] = (void*) __stack_chk_guard; thread->tls[TLS_SLOT_STACK_GUARD] = (void*) __stack_chk_guard;
__set_tls(thread->tls); __set_tls(thread->tls);
// Create and set an alternate signal stack.
// This must happen after __set_tls, in case a system call fails and tries to set errno.
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);
thread->alternate_signal_stack = ss.ss_sp;
}
} }
// 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) { 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 // Wait for our creating thread to release us. This lets it have time to
// notify gdb about this thread before we start doing anything. // notify gdb about this thread before we start doing anything.
@ -187,8 +200,12 @@ int pthread_create(pthread_t* thread_out, pthread_attr_t const* attr,
thread->attr.flags |= PTHREAD_ATTR_FLAG_USER_STACK; 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** 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 // 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 // it from doing anything until after we notify the debugger about it
@ -204,7 +221,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 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) { if (tid < 0) {
int clone_errno = errno; int clone_errno = errno;
if ((thread->attr.flags & PTHREAD_ATTR_FLAG_USER_STACK) == 0) { if ((thread->attr.flags & PTHREAD_ATTR_FLAG_USER_STACK) == 0) {

View file

@ -47,6 +47,8 @@ typedef struct pthread_internal_t
__pthread_cleanup_t* cleanup_stack; __pthread_cleanup_t* cleanup_stack;
void** tls; /* thread-local storage area */ void** tls; /* thread-local storage area */
void* alternate_signal_stack;
/* /*
* The dynamic linker implements dlerror(3), which makes it hard for us to implement this * The dynamic linker implements dlerror(3), which makes it hard for us to implement this
* per-thread buffer by simply using malloc(3) and free(3). * per-thread buffer by simply using malloc(3) and free(3).

View file

@ -28,14 +28,15 @@
#include "linker.h" #include "linker.h"
#include <errno.h>
#include <signal.h>
#include <stdio.h> #include <stdio.h>
#include <stdlib.h> #include <stdlib.h>
#include <unistd.h> #include <sys/mman.h>
#include <signal.h>
#include <sys/prctl.h> #include <sys/prctl.h>
#include <errno.h>
#include <sys/socket.h> #include <sys/socket.h>
#include <sys/un.h> #include <sys/un.h>
#include <unistd.h>
extern "C" int tgkill(int tgid, int tid, int sig); 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 * mutex is being held, so we don't want to use any libc functions that
* could allocate memory or hold a lock. * 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; const char* signal_name;
switch (signum) { switch (signum) {
case SIGILL: signal_name = "SIGILL"; break; 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. * Returns true if the handler for signal "signum" has SA_SIGINFO set.
*/ */
static bool haveSiginfo(int signum) { static bool have_siginfo(int signum) {
struct sigaction oldact, newact; struct sigaction old_action, new_action;
memset(&newact, 0, sizeof(newact)); memset(&new_action, 0, sizeof(new_action));
newact.sa_handler = SIG_DFL; new_action.sa_handler = SIG_DFL;
newact.sa_flags = SA_RESTART; new_action.sa_flags = SA_RESTART;
sigemptyset(&newact.sa_mask); 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", __libc_format_log(ANDROID_LOG_WARN, "libc", "Failed testing for SA_SIGINFO: %s",
strerror(errno)); strerror(errno));
return false; 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", __libc_format_log(ANDROID_LOG_WARN, "libc", "Restore failed in test for SA_SIGINFO: %s",
strerror(errno)); 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 * It's possible somebody cleared the SA_SIGINFO flag, which would mean
* our "info" arg holds an undefined value. * our "info" arg holds an undefined value.
*/ */
if (!haveSiginfo(n)) { if (!have_siginfo(n)) {
info = NULL; info = NULL;
} }
logSignalSummary(n, info); log_signal_summary(n, info);
pid_t tid = gettid(); pid_t tid = gettid();
int s = socket_abstract_client(DEBUGGER_SOCKET_NAME, SOCK_STREAM); 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() { void debuggerd_init() {
struct sigaction act; struct sigaction action;
memset(&act, 0, sizeof(act)); memset(&action, 0, sizeof(action));
act.sa_sigaction = debuggerd_signal_handler; sigemptyset(&action.sa_mask);
act.sa_flags = SA_RESTART | SA_SIGINFO; action.sa_sigaction = debuggerd_signal_handler;
sigemptyset(&act.sa_mask); action.sa_flags = SA_RESTART | SA_SIGINFO;
sigaction(SIGILL, &act, NULL); // Use the alternate signal stack if available so we can catch stack overflows.
sigaction(SIGABRT, &act, NULL); action.sa_flags |= SA_ONSTACK;
sigaction(SIGBUS, &act, NULL);
sigaction(SIGFPE, &act, NULL); sigaction(SIGABRT, &action, NULL);
sigaction(SIGSEGV, &act, NULL); sigaction(SIGBUS, &action, NULL);
sigaction(SIGFPE, &action, NULL);
sigaction(SIGILL, &action, NULL);
sigaction(SIGPIPE, &action, NULL);
sigaction(SIGSEGV, &action, NULL);
#if defined(SIGSTKFLT) #if defined(SIGSTKFLT)
sigaction(SIGSTKFLT, &act, NULL); sigaction(SIGSTKFLT, &action, NULL);
#endif #endif
sigaction(SIGPIPE, &act, NULL); sigaction(SIGTRAP, &action, NULL);
} }