Index: lib/safestack/safestack.cc =================================================================== --- lib/safestack/safestack.cc +++ lib/safestack/safestack.cc @@ -14,11 +14,13 @@ // //===----------------------------------------------------------------------===// +#include #include #include #include #include #include +#include #include #include #if !defined(__NetBSD__) @@ -115,14 +117,6 @@ unsafe_stack_guard = guard; } -static void unsafe_stack_free() { - if (unsafe_stack_start) { - UnmapOrDie((char *)unsafe_stack_start - unsafe_stack_guard, - unsafe_stack_size + unsafe_stack_guard); - } - unsafe_stack_start = nullptr; -} - /// Thread data for the cleanup handler static pthread_key_t thread_cleanup_key; @@ -149,26 +143,68 @@ tinfo->unsafe_stack_guard); // Make sure out thread-specific destructor will be called - // FIXME: we can do this only any other specific key is set by - // intercepting the pthread_setspecific function itself pthread_setspecific(thread_cleanup_key, (void *)1); return start_routine(start_routine_arg); } -/// Thread-specific data destructor +/// Linked list used to store exiting threads stack/thread information. +struct thread_stack_ll { + struct thread_stack_ll *next; + void *stack_base; + pid_t pid; + tid_t tid; +}; + +/// Linked list of unsafe stacks for threads that are exiting. We delay +/// unmapping them until the thread exits. +static thread_stack_ll *thread_stacks = nullptr; +static pthread_mutex_t thread_stacks_mutex = PTHREAD_MUTEX_INITIALIZER; + +/// Thread-specific data destructor. We want to free the unsafe stack only after +/// this thread is terminated. libc can call functions in safestack-instrumented +/// code (like free) after thread-specific data destructors have run. static void thread_cleanup_handler(void *_iter) { - // We want to free the unsafe stack only after all other destructors - // have already run. We force this function to be called multiple times. - // User destructors that might run more then PTHREAD_DESTRUCTOR_ITERATIONS-1 - // times might still end up executing after the unsafe stack is deallocated. - size_t iter = (size_t)_iter; - if (iter < PTHREAD_DESTRUCTOR_ITERATIONS) { - pthread_setspecific(thread_cleanup_key, (void *)(iter + 1)); - } else { - // This is the last iteration - unsafe_stack_free(); + CHECK_NE(unsafe_stack_start, nullptr); + pthread_setspecific(thread_cleanup_key, NULL); + + pthread_mutex_lock(&thread_stacks_mutex); + // Temporary list to hold the previous threads stacks so we don't hold the + // thread_stacks_mutex for long. + thread_stack_ll *temp_stacks = thread_stacks; + thread_stacks = nullptr; + pthread_mutex_unlock(&thread_stacks_mutex); + + pid_t pid = getpid(); + tid_t tid = GetTid(); + + // Free stacks for dead threads + thread_stack_ll **stackp = &temp_stacks; + while (*stackp) { + thread_stack_ll *stack = *stackp; + if (stack->pid != pid || TgKill(stack->pid, stack->tid, 0) == -ESRCH) { + UnmapOrDie(stack->stack_base, unsafe_stack_size + unsafe_stack_guard); + *stackp = stack->next; + free(stack); + } else + stackp = &stack->next; } + + thread_stack_ll *cur_stack = + (thread_stack_ll *)malloc(sizeof(thread_stack_ll)); + cur_stack->stack_base = (char *)unsafe_stack_start - unsafe_stack_guard; + cur_stack->pid = pid; + cur_stack->tid = tid; + + pthread_mutex_lock(&thread_stacks_mutex); + // Merge thread_stacks with the current thread's stack and any remaining + // temp_stacks + *stackp = thread_stacks; + cur_stack->next = temp_stacks; + thread_stacks = cur_stack; + pthread_mutex_unlock(&thread_stacks_mutex); + + unsafe_stack_start = nullptr; } static void EnsureInterceptorsInitialized(); Index: lib/sanitizer_common/sanitizer_common.h =================================================================== --- lib/sanitizer_common/sanitizer_common.h +++ lib/sanitizer_common/sanitizer_common.h @@ -73,6 +73,7 @@ uptr GetMaxUserVirtualAddress(); // Threads tid_t GetTid(); +int TgKill(pid_t pid, tid_t tid, int sig); uptr GetThreadSelf(); void GetThreadStackTopAndBottom(bool at_initialization, uptr *stack_top, uptr *stack_bottom); Index: lib/sanitizer_common/sanitizer_linux.cc =================================================================== --- lib/sanitizer_common/sanitizer_linux.cc +++ lib/sanitizer_common/sanitizer_linux.cc @@ -481,6 +481,14 @@ #endif } +int TgKill(pid_t pid, tid_t tid, int sig) { +#if SANITIZER_LINUX + return internal_syscall(SYSCALL(tgkill), pid, tid, sig); +#else + return internal_syscall(SYSCALL(thr_kill2), pid, tid, sig); +#endif +} + #if !SANITIZER_SOLARIS u64 NanoTime() { #if SANITIZER_FREEBSD || SANITIZER_NETBSD || SANITIZER_OPENBSD Index: test/safestack/pthread-cleanup.c =================================================================== --- test/safestack/pthread-cleanup.c +++ test/safestack/pthread-cleanup.c @@ -1,7 +1,9 @@ // RUN: %clang_safestack %s -pthread -o %t -// RUN: not --crash %run %t +// RUN: %run %t 0 +// RUN: not --crash %run %t 1 -// Test that unsafe stacks are deallocated correctly on thread exit. +// Test unsafe stack deallocation. Unsafe stacks are not deallocated immediately +// at thread exit. They are deallocated by following exiting threads. #include #include @@ -9,7 +11,7 @@ enum { kBufferSize = (1 << 15) }; -void *t1_start(void *ptr) +void *start(void *ptr) { char buffer[kBufferSize]; return buffer; @@ -17,15 +19,28 @@ int main(int argc, char **argv) { - pthread_t t1; - char *buffer = NULL; + int arg = atoi(argv[1]); - if (pthread_create(&t1, NULL, t1_start, NULL)) + pthread_t t1, t2; + char *t1_buffer = NULL; + + if (pthread_create(&t1, NULL, start, NULL)) + abort(); + if (pthread_join(t1, &t1_buffer)) + abort(); + + memset(t1_buffer, 0, kBufferSize); + + if (arg == 0) + return 0; + + if (pthread_create(&t2, NULL, start, NULL)) abort(); - if (pthread_join(t1, &buffer)) + // Second thread destructor cleans up the first thread's stack. + if (pthread_join(t2, NULL)) abort(); // should segfault here - memset(buffer, 0, kBufferSize); + memset(t1_buffer, 0, kBufferSize); return 0; }