Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Sat, 30 Aug 2014 20:47:46 +0200
From: Jens Gustedt <Jens.Gustedt@...ia.fr>
To: musl@...ts.openwall.com
Subject: [PATCH 8/8] Separate pthread_create and thrd_create

This moves the core of pthread_exit from pthread_create.c to its own TU,
pthread_exit.c. This is what the two thread implementations really have
to share.

The "create" functions that each have their own TU.

This is less intrusive for both thread models. With that, pthread goes
back to completely equivalent code than before the merge, only that
pthread_exit is in a different TU.

For C threads this also cuts off some dead branches in thrd_create.
---
 src/thread/pthread_create.c |  166 +++----------------------------------------
 src/thread/pthread_detach.c |    6 +-
 src/thread/pthread_exit.c   |  130 +++++++++++++++++++++++++++++++++
 src/thread/pthread_join.c   |    4 +-
 src/thread/thrd_create.c    |   98 +++++++++++++++++++++++++
 src/thread/thrd_detach.c    |   14 ++--
 6 files changed, 246 insertions(+), 172 deletions(-)
 create mode 100644 src/thread/pthread_exit.c
 create mode 100644 src/thread/thrd_create.c

diff --git a/src/thread/pthread_create.c b/src/thread/pthread_create.c
index 3212502..193b295 100644
--- a/src/thread/pthread_create.c
+++ b/src/thread/pthread_create.c
@@ -9,104 +9,16 @@
 void *__mmap(void *, size_t, int, int, int, off_t);
 int __munmap(void *, size_t);
 int __mprotect(void *, size_t, int);
-
-#define __THRD_C11 ((void*)(uintptr_t)-1)
+void __thread_enable(void);
+_Noreturn void __pthread_exit(void *);
+void *__copy_tls(unsigned char *);
+extern volatile size_t __pthread_tsd_size;
 
 static void dummy_0()
 {
 }
 weak_alias(dummy_0, __acquire_ptc);
 weak_alias(dummy_0, __release_ptc);
-weak_alias(dummy_0, __pthread_tsd_run_dtors);
-weak_alias(dummy_0, __do_private_robust_list);
-weak_alias(dummy_0, __do_orphaned_stdio_locks);
-
-_Noreturn void __pthread_exit(void *result)
-{
-	pthread_t self = __pthread_self();
-	sigset_t set;
-
-	self->result = result;
-
-	while (self->cancelbuf) {
-		void (*f)(void *) = self->cancelbuf->__f;
-		void *x = self->cancelbuf->__x;
-		self->cancelbuf = self->cancelbuf->__next;
-		f(x);
-	}
-
-	__pthread_tsd_run_dtors();
-
-	__lock(self->exitlock);
-
-	/* Mark this thread dead before decrementing count */
-	__lock(self->killlock);
-	self->dead = 1;
-
-	/* Block all signals before decrementing the live thread count.
-	 * This is important to ensure that dynamically allocated TLS
-	 * is not under-allocated/over-committed, and possibly for other
-	 * reasons as well. */
-	__block_all_sigs(&set);
-
-	/* Wait to unlock the kill lock, which governs functions like
-	 * pthread_kill which target a thread id, until signals have
-	 * been blocked. This precludes observation of the thread id
-	 * as a live thread (with application code running in it) after
-	 * the thread was reported dead by ESRCH being returned. */
-	__unlock(self->killlock);
-
-	/* It's impossible to determine whether this is "the last thread"
-	 * until performing the atomic decrement, since multiple threads
-	 * could exit at the same time. For the last thread, revert the
-	 * decrement and unblock signals to give the atexit handlers and
-	 * stdio cleanup code a consistent state. */
-	if (a_fetch_add(&libc.threads_minus_1, -1)==0) {
-		libc.threads_minus_1 = 0;
-		__restore_sigs(&set);
-		exit(0);
-	}
-
-	if (self->locale != &libc.global_locale) {
-		a_dec(&libc.uselocale_cnt);
-		if (self->locale->ctype_utf8)
-			a_dec(&libc.bytelocale_cnt_minus_1);
-	}
-
-	__do_private_robust_list();
-	__do_orphaned_stdio_locks();
-
-	if (self->detached && self->map_base) {
-		/* Detached threads must avoid the kernel clear_child_tid
-		 * feature, since the virtual address will have been
-		 * unmapped and possibly already reused by a new mapping
-		 * at the time the kernel would perform the write. In
-		 * the case of threads that started out detached, the
-		 * initial clone flags are correct, but if the thread was
-		 * detached later (== 2), we need to clear it here. */
-		if (self->detached == 2) __syscall(SYS_set_tid_address, 0);
-
-		/* The following call unmaps the thread's stack mapping
-		 * and then exits without touching the stack. */
-		__unmapself(self->map_base, self->map_size);
-	}
-
-	for (;;) __syscall(SYS_exit, 0);
-}
-
-void __do_cleanup_push(struct __ptcb *cb)
-{
-	if (!libc.has_thread_pointer) return;
-	struct pthread *self = __pthread_self();
-	cb->__next = self->cancelbuf;
-	self->cancelbuf = cb;
-}
-
-void __do_cleanup_pop(struct __ptcb *cb)
-{
-	if (!libc.has_thread_pointer) return;
-	__pthread_self()->cancelbuf = cb->__next;
-}
 
 static int start(void *p)
 {
@@ -126,42 +38,11 @@ static int start(void *p)
 	return 0;
 }
 
-static _Noreturn void __thrd_exit(int result) {
-	__pthread_exit((void*)(intptr_t)result);
-}
-
-
-static int start_c11(void *p)
-{
-	thrd_t self = p;
-	int (*start)(void*) = (int(*)(void*)) self->start;
-	__thrd_exit(start(self->start_arg));
-	return 0;
-}
-
 #define ROUND(x) (((x)+PAGE_SIZE-1)&-PAGE_SIZE)
 
-/* pthread_key_create.c overrides this */
-static volatile size_t dummy = 0;
-weak_alias(dummy, __pthread_tsd_size);
-static void *dummy_tsd[1] = { 0 };
-weak_alias(dummy_tsd, __pthread_tsd_main);
-
-static FILE *volatile dummy_file = 0;
-weak_alias(dummy_file, __stdin_used);
-weak_alias(dummy_file, __stdout_used);
-weak_alias(dummy_file, __stderr_used);
-
-static void init_file_lock(FILE *f)
+int pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict attrp, void *(*entry)(void *), void *restrict arg)
 {
-	if (f && f->lock<0) f->lock = 0;
-}
-
-void *__copy_tls(unsigned char *);
-
-static int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict attrp, void *(*entry)(void *), void *restrict arg)
-{
-	int ret, c11 = (attrp == __THRD_C11);
+	int ret;
 	size_t size, guard = 0;
 	struct pthread *self, *new;
 	unsigned char *map = 0, *stack = 0, *tsd = 0, *stack_limit;
@@ -173,19 +54,7 @@ static int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restr
 
 	if (!libc.can_do_threads) return ENOSYS;
 	self = __pthread_self();
-	if (!libc.threaded) {
-		for (FILE *f=libc.ofl_head; f; f=f->next)
-			init_file_lock(f);
-		init_file_lock(__stdin_used);
-		init_file_lock(__stdout_used);
-		init_file_lock(__stderr_used);
-		__syscall(SYS_rt_sigprocmask, SIG_UNBLOCK, SIGPT_SET, 0, _NSIG/8);
-		self->tsd = (void **)__pthread_tsd_main;
-		libc.threaded = 1;
-	}
-        if (c11) {
-          attrp = 0;
-        }
+	if (!libc.threaded) __thread_enable();
 	if (attrp) attr = *attrp;
 
 	__acquire_ptc();
@@ -253,10 +122,7 @@ static int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restr
 	new->canary = self->canary;
 
 	a_inc(&libc.threads_minus_1);
-	if (c11)
-		ret = __clone(start_c11, stack, flags, new, &new->tid, TP_ADJ(new), &new->tid);
-	else
-		ret = __clone(start, stack, flags, new, &new->tid, TP_ADJ(new), &new->tid);
+	ret = __clone(start, stack, flags, new, &new->tid, TP_ADJ(new), &new->tid);
 
 	__release_ptc();
 
@@ -284,19 +150,3 @@ fail:
 	__release_ptc();
 	return EAGAIN;
 }
-
-static int __thrd_create(thrd_t *thr,
-                         thrd_start_t func,
-                         void *arg) {
-	/* In the best of all worlds this is a tail call. */
-	int ret = __pthread_create(thr, __THRD_C11, (void * (*)(void *))func, arg);
-	if (thrd_success)
-		return ret ? thrd_error : thrd_success;
-	/* In case of UB may also return ENOSYS or EAGAIN. */
-	return ret;
-}
-
-weak_alias(__pthread_exit, pthread_exit);
-weak_alias(__pthread_create, pthread_create);
-weak_alias(__thrd_create, thrd_create);
-weak_alias(__thrd_exit, thrd_exit);
diff --git a/src/thread/pthread_detach.c b/src/thread/pthread_detach.c
index 5ca7855..e8ec758 100644
--- a/src/thread/pthread_detach.c
+++ b/src/thread/pthread_detach.c
@@ -1,15 +1,11 @@
 #include "pthread_impl.h"
 
-int __pthread_join(pthread_t t, void **res);
-
 int __pthread_detach(pthread_t t)
 {
 	/* Cannot detach a thread that's already exiting */
 	if (a_swap(t->exitlock, 1))
-		return __pthread_join(t, 0);
+		return pthread_join(t, 0);
 	t->detached = 2;
 	__unlock(t->exitlock);
 	return 0;
 }
-
-weak_alias(__pthread_detach, pthread_detach);
diff --git a/src/thread/pthread_exit.c b/src/thread/pthread_exit.c
new file mode 100644
index 0000000..4f51269
--- /dev/null
+++ b/src/thread/pthread_exit.c
@@ -0,0 +1,130 @@
+#define _GNU_SOURCE
+#include "pthread_impl.h"
+#include "stdio_impl.h"
+#include "libc.h"
+#include <sys/mman.h>
+#include <threads.h>
+
+static void dummy_0()
+{
+}
+weak_alias(dummy_0, __pthread_tsd_run_dtors);
+weak_alias(dummy_0, __do_private_robust_list);
+weak_alias(dummy_0, __do_orphaned_stdio_locks);
+
+_Noreturn void __pthread_exit(void *result)
+{
+	pthread_t self = __pthread_self();
+	sigset_t set;
+
+	self->result = result;
+
+	while (self->cancelbuf) {
+		void (*f)(void *) = self->cancelbuf->__f;
+		void *x = self->cancelbuf->__x;
+		self->cancelbuf = self->cancelbuf->__next;
+		f(x);
+	}
+
+	__pthread_tsd_run_dtors();
+
+	__lock(self->exitlock);
+
+	/* Mark this thread dead before decrementing count */
+	__lock(self->killlock);
+	self->dead = 1;
+
+	/* Block all signals before decrementing the live thread count.
+	 * This is important to ensure that dynamically allocated TLS
+	 * is not under-allocated/over-committed, and possibly for other
+	 * reasons as well. */
+	__block_all_sigs(&set);
+
+	/* Wait to unlock the kill lock, which governs functions like
+	 * pthread_kill which target a thread id, until signals have
+	 * been blocked. This precludes observation of the thread id
+	 * as a live thread (with application code running in it) after
+	 * the thread was reported dead by ESRCH being returned. */
+	__unlock(self->killlock);
+
+	/* It's impossible to determine whether this is "the last thread"
+	 * until performing the atomic decrement, since multiple threads
+	 * could exit at the same time. For the last thread, revert the
+	 * decrement and unblock signals to give the atexit handlers and
+	 * stdio cleanup code a consistent state. */
+	if (a_fetch_add(&libc.threads_minus_1, -1)==0) {
+		libc.threads_minus_1 = 0;
+		__restore_sigs(&set);
+		exit(0);
+	}
+
+	if (self->locale != &libc.global_locale) {
+		a_dec(&libc.uselocale_cnt);
+		if (self->locale->ctype_utf8)
+			a_dec(&libc.bytelocale_cnt_minus_1);
+	}
+
+	__do_private_robust_list();
+	__do_orphaned_stdio_locks();
+
+	if (self->detached && self->map_base) {
+		/* Detached threads must avoid the kernel clear_child_tid
+		 * feature, since the virtual address will have been
+		 * unmapped and possibly already reused by a new mapping
+		 * at the time the kernel would perform the write. In
+		 * the case of threads that started out detached, the
+		 * initial clone flags are correct, but if the thread was
+		 * detached later (== 2), we need to clear it here. */
+		if (self->detached == 2) __syscall(SYS_set_tid_address, 0);
+
+		/* The following call unmaps the thread's stack mapping
+		 * and then exits without touching the stack. */
+		__unmapself(self->map_base, self->map_size);
+	}
+
+	for (;;) __syscall(SYS_exit, 0);
+}
+
+void __do_cleanup_push(struct __ptcb *cb)
+{
+	if (!libc.has_thread_pointer) return;
+	struct pthread *self = __pthread_self();
+	cb->__next = self->cancelbuf;
+	self->cancelbuf = cb;
+}
+
+void __do_cleanup_pop(struct __ptcb *cb)
+{
+	if (!libc.has_thread_pointer) return;
+	__pthread_self()->cancelbuf = cb->__next;
+}
+
+/* pthread_key_create.c overrides this */
+static volatile size_t dummy = 0;
+weak_alias(dummy, __pthread_tsd_size);
+static void *dummy_tsd[1] = { 0 };
+weak_alias(dummy_tsd, __pthread_tsd_main);
+
+static FILE *volatile dummy_file = 0;
+weak_alias(dummy_file, __stdin_used);
+weak_alias(dummy_file, __stdout_used);
+weak_alias(dummy_file, __stderr_used);
+
+static void init_file_lock(FILE *f)
+{
+	if (f && f->lock<0) f->lock = 0;
+}
+
+void __thread_enable(void)
+{
+	for (FILE *f=libc.ofl_head; f; f=f->next)
+		init_file_lock(f);
+	init_file_lock(__stdin_used);
+	init_file_lock(__stdout_used);
+	init_file_lock(__stderr_used);
+	__syscall(SYS_rt_sigprocmask, SIG_UNBLOCK, SIGPT_SET, 0, _NSIG/8);
+	__pthread_self()->tsd = (void **)__pthread_tsd_main;
+	libc.threaded = 1;
+}
+
+weak_alias(__pthread_exit, pthread_exit);
diff --git a/src/thread/pthread_join.c b/src/thread/pthread_join.c
index bca89aa..23efbf0 100644
--- a/src/thread/pthread_join.c
+++ b/src/thread/pthread_join.c
@@ -7,7 +7,7 @@ static void dummy(void *p)
 {
 }
 
-int __pthread_join(pthread_t t, void **res)
+int pthread_join(pthread_t t, void **res)
 {
 	int tmp;
 	while ((tmp = t->tid)) __timedwait(&t->tid, tmp, 0, 0, dummy, 0, 0);
@@ -15,5 +15,3 @@ int __pthread_join(pthread_t t, void **res)
 	if (t->map_base) __munmap(t->map_base, t->map_size);
 	return 0;
 }
-
-weak_alias(__pthread_join, pthread_join);
diff --git a/src/thread/thrd_create.c b/src/thread/thrd_create.c
new file mode 100644
index 0000000..f72f992
--- /dev/null
+++ b/src/thread/thrd_create.c
@@ -0,0 +1,98 @@
+#define _GNU_SOURCE
+#include "pthread_impl.h"
+#include "stdio_impl.h"
+#include "libc.h"
+#include <sys/mman.h>
+#include <threads.h>
+
+void *__mmap(void *, size_t, int, int, int, off_t);
+int __munmap(void *, size_t);
+int __mprotect(void *, size_t, int);
+void __thread_enable(void);
+_Noreturn void __pthread_exit(void *);
+void *__copy_tls(unsigned char *);
+extern volatile size_t __pthread_tsd_size;
+
+_Noreturn void thrd_exit(int result) {
+	__pthread_exit((void*)(intptr_t)result);
+}
+
+static void dummy_0()
+{
+}
+weak_alias(dummy_0, __acquire_ptc);
+weak_alias(dummy_0, __release_ptc);
+
+static int start(void *p)
+{
+	thrd_t self = p;
+	int (*start)(void*) = (int(*)(void*)) self->start;
+	thrd_exit(start(self->start_arg));
+	return 0;
+}
+
+#define ROUND(x) (((x)+PAGE_SIZE-1)&-PAGE_SIZE)
+
+int thrd_create(thrd_t *res, thrd_start_t entry, void *arg)
+{
+	int ret = -ENOMEM;
+	size_t guard = ROUND(DEFAULT_GUARD_SIZE);
+	size_t size = guard + ROUND(DEFAULT_STACK_SIZE + libc.tls_size +  __pthread_tsd_size);
+	struct pthread *self, *new;
+	unsigned char *map, *stack, *tsd, *stack_limit;
+	unsigned flags = CLONE_VM | CLONE_FS | CLONE_FILES | CLONE_SIGHAND
+		| CLONE_THREAD | CLONE_SYSVSEM | CLONE_SETTLS
+		| CLONE_PARENT_SETTID | CLONE_CHILD_CLEARTID | CLONE_DETACHED;
+
+	if (!libc.can_do_threads) return thrd_error;
+	self = __pthread_self();
+	if (!libc.threaded) __thread_enable();
+
+	__acquire_ptc();
+
+	if (guard) {
+		map = __mmap(0, size, PROT_NONE, MAP_PRIVATE|MAP_ANON, -1, 0);
+		if (map == MAP_FAILED) goto CLEANUP;
+		if (__mprotect(map+guard, size-guard, PROT_READ|PROT_WRITE)) {
+			__munmap(map, size);
+			goto CLEANUP;
+		}
+	} else {
+		map = __mmap(0, size, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANON, -1, 0);
+		if (map == MAP_FAILED) goto CLEANUP;
+	}
+	tsd = map + size - __pthread_tsd_size;
+	stack = tsd - libc.tls_size;
+	stack_limit = map + guard;
+
+	new = __copy_tls(tsd - libc.tls_size);
+	new->map_base = map;
+	new->map_size = size;
+	new->stack = stack;
+	new->stack_size = stack - stack_limit;
+	new->start = (void *(*)(void*))entry;
+	new->start_arg = arg;
+	new->self = new;
+	new->tsd = (void *)tsd;
+	new->locale = &libc.global_locale;
+	new->unblock_cancel = self->cancel;
+	new->canary = self->canary;
+
+	a_inc(&libc.threads_minus_1);
+	ret = __clone(start, stack, flags, new, &new->tid, TP_ADJ(new), &new->tid);
+
+ CLEANUP:
+	__release_ptc();
+
+	if (ret > 0) {
+          *res = new;
+          ret = thrd_success;
+        } else if (ret == -ENOMEM) {
+          ret = thrd_nomem;
+        } else {
+          a_dec(&libc.threads_minus_1);
+          if (map) __munmap(map, size);
+          ret = thrd_error;
+	}
+        return ret;
+}
diff --git a/src/thread/thrd_detach.c b/src/thread/thrd_detach.c
index 72cdfba..2597ae5 100644
--- a/src/thread/thrd_detach.c
+++ b/src/thread/thrd_detach.c
@@ -1,12 +1,14 @@
+#include "pthread_impl.h"
 #include <threads.h>
 
-int __pthread_detach(thrd_t t);
+int __munmap(void *, size_t);
 
 int thrd_detach(thrd_t t)
 {
-	/* In the best of all worlds this is a tail call. */
-	int ret = __pthread_detach(t);
-	if (thrd_success)
-		return ret ? thrd_error : thrd_success;
-	return ret;
+	/* Cannot detach a thread that's already exiting */
+	if (a_swap(t->exitlock, 1))
+		return thrd_join(t, 0);
+	t->detached = 2;
+	__unlock(t->exitlock);
+	return 0;
 }
-- 
1.7.10.4

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.