[compiler-rt] r339408 - Revert "SafeStack: Delay thread stack clean-up"
Vlad Tsyrklevich via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 9 17:36:05 PDT 2018
Author: vlad.tsyrklevich
Date: Thu Aug 9 17:36:04 2018
New Revision: 339408
URL: http://llvm.org/viewvc/llvm-project?rev=339408&view=rev
Log:
Revert "SafeStack: Delay thread stack clean-up"
This reverts commit r339405, it's failing on Darwin buildbots because
it doesn't seem to have a tgkill/thr_kill2 interface. It has a
__pthread_kill() syscall, but that relies on having a handle to the
thread's port which is not equivalent to it's tid.
Modified:
compiler-rt/trunk/lib/safestack/safestack.cc
compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h
compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc
compiler-rt/trunk/test/safestack/pthread-cleanup.c
Modified: compiler-rt/trunk/lib/safestack/safestack.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/safestack/safestack.cc?rev=339408&r1=339407&r2=339408&view=diff
==============================================================================
--- compiler-rt/trunk/lib/safestack/safestack.cc (original)
+++ compiler-rt/trunk/lib/safestack/safestack.cc Thu Aug 9 17:36:04 2018
@@ -14,13 +14,11 @@
//
//===----------------------------------------------------------------------===//
-#include <errno.h>
#include <limits.h>
#include <pthread.h>
#include <stddef.h>
#include <stdint.h>
#include <unistd.h>
-#include <stdlib.h>
#include <sys/resource.h>
#include <sys/types.h>
#if !defined(__NetBSD__)
@@ -117,6 +115,14 @@ static inline void unsafe_stack_setup(vo
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;
@@ -143,68 +149,26 @@ static void *thread_start(void *arg) {
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);
}
-/// 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.
+/// Thread-specific data destructor
static void thread_cleanup_handler(void *_iter) {
- 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;
+ // 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();
}
-
- 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();
Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h?rev=339408&r1=339407&r2=339408&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h Thu Aug 9 17:36:04 2018
@@ -73,7 +73,6 @@ uptr GetMaxVirtualAddress();
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);
Modified: compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc?rev=339408&r1=339407&r2=339408&view=diff
==============================================================================
--- compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc (original)
+++ compiler-rt/trunk/lib/sanitizer_common/sanitizer_linux.cc Thu Aug 9 17:36:04 2018
@@ -486,14 +486,6 @@ tid_t GetTid() {
#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
Modified: compiler-rt/trunk/test/safestack/pthread-cleanup.c
URL: http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/test/safestack/pthread-cleanup.c?rev=339408&r1=339407&r2=339408&view=diff
==============================================================================
--- compiler-rt/trunk/test/safestack/pthread-cleanup.c (original)
+++ compiler-rt/trunk/test/safestack/pthread-cleanup.c Thu Aug 9 17:36:04 2018
@@ -1,9 +1,7 @@
// RUN: %clang_safestack %s -pthread -o %t
-// RUN: %run %t 0
-// RUN: not --crash %run %t 1
+// RUN: not --crash %run %t
-// Test unsafe stack deallocation. Unsafe stacks are not deallocated immediately
-// at thread exit. They are deallocated by following exiting threads.
+// Test that unsafe stacks are deallocated correctly on thread exit.
#include <stdlib.h>
#include <string.h>
@@ -11,7 +9,7 @@
enum { kBufferSize = (1 << 15) };
-void *start(void *ptr)
+void *t1_start(void *ptr)
{
char buffer[kBufferSize];
return buffer;
@@ -19,28 +17,15 @@ void *start(void *ptr)
int main(int argc, char **argv)
{
- int arg = atoi(argv[1]);
+ pthread_t t1;
+ char *buffer = 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))
+ if (pthread_create(&t1, NULL, t1_start, NULL))
abort();
- // Second thread destructor cleans up the first thread's stack.
- if (pthread_join(t2, NULL))
+ if (pthread_join(t1, &buffer))
abort();
// should segfault here
- memset(t1_buffer, 0, kBufferSize);
+ memset(buffer, 0, kBufferSize);
return 0;
}
More information about the llvm-commits
mailing list