[PATCH 9/9] Separate pthread_create and thrd_create

[Jens Gustedt <Jens.Gustedt@inria.fr>]

The "create" and "exit" functions 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 and related code is in a different TU.

For C threads this also cuts off some dead branches in thrd_create.
---
 src/thread/pthread_create.c |   23 +++--------
 src/thread/pthread_detach.c |    6 +--
 src/thread/pthread_join.c   |    4 +-
 src/thread/thrd_create.c    |   95 +++++++++++++++++++++++++++++++++++++++----
 src/thread/thrd_detach.c    |   13 +++---
 src/thread/thrd_join.c      |   22 +++++++---
 6 files changed, 118 insertions(+), 45 deletions(-)

diff --git a/src/thread/pthread_create.c b/src/thread/pthread_create.c
index 7bc7b0f..51316bc 100644
--- a/src/thread/pthread_create.c
+++ b/src/thread/pthread_create.c
@@ -9,7 +9,6 @@
 void *__mmap(void *, size_t, int, int, int, off_t);
 int __munmap(void *, size_t);
 int __mprotect(void *, size_t, int);
-_Noreturn void __thrd_exit(int);
 void __thread_enable(void);
 _Noreturn void __pthread_exit(void *);
 void *__copy_tls(unsigned char *);
@@ -39,19 +38,11 @@ static int start(void *p)
 	return 0;
 }
 
-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)
 
-int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict attrp, void *(*entry)(void *), void *restrict arg)
+int pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict attrp, void *(*entry)(void *), void *restrict arg)
 {
-	int ret, c11 = (attrp == __ATTRP_C11_THREAD);
+	int ret;
 	size_t size, guard = 0;
 	struct pthread *self, *new;
 	unsigned char *map = 0, *stack = 0, *tsd = 0, *stack_limit;
@@ -64,7 +55,7 @@ int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict att
 	if (!libc.can_do_threads) return ENOSYS;
 	self = __pthread_self();
 	if (!libc.threaded) __thread_enable();
-	if (attrp && !c11) attr = *attrp;
+	if (attrp) attr = *attrp;
 
 	__acquire_ptc();
 
@@ -131,7 +122,7 @@ int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict att
 	new->canary = self->canary;
 
 	a_inc(&libc.threads_minus_1);
-	ret = __clone((c11 ? start_c11 : 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();
 
@@ -142,7 +133,7 @@ int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict att
 	if (ret < 0) {
 		a_dec(&libc.threads_minus_1);
 		if (map) __munmap(map, size);
-		return (!c11 || ret != -ENOMEM) ? EAGAIN : ENOMEM;
+		return EAGAIN;
 	}
 
 	if (do_sched) {
@@ -157,7 +148,5 @@ int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict att
 	return 0;
 fail_nomem:
 	__release_ptc();
-	return c11 ? ENOMEM : EAGAIN;
+	return EAGAIN;
 }
-
-weak_alias(__pthread_create, pthread_create);
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_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
index 09c1d9e..7eb1c9b 100644
--- a/src/thread/thrd_create.c
+++ b/src/thread/thrd_create.c
@@ -1,14 +1,93 @@
+#define _GNU_SOURCE
 #include "pthread_impl.h"
+#include "stdio_impl.h"
+#include "libc.h"
+#include <sys/mman.h>
 #include <threads.h>
 
-int __pthread_create(pthread_t *restrict, const pthread_attr_t *restrict, void *(*)(void *), void *restrict);
+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);
+void *__copy_tls(unsigned char *);
+extern volatile size_t __pthread_tsd_size;
 
-int thrd_create(thrd_t *thr, thrd_start_t func, void *arg) {
-	int ret = __pthread_create(thr, __ATTRP_C11_THREAD, (void * (*)(void *))func, arg);
-	switch (ret) {
-	case 0:      return thrd_success;
-	case ENOMEM: return thrd_nomem;
-	/* The call may also return ENOSYS or EAGAIN. */
-	default:     return thrd_error;
+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 b0b153c..8079326 100644
--- a/src/thread/thrd_detach.c
+++ b/src/thread/thrd_detach.c
@@ -1,11 +1,12 @@
+#include "pthread_impl.h"
 #include <threads.h>
 
-int __pthread_detach(thrd_t t);
-
 int thrd_detach(thrd_t t)
 {
-	/* This internal function never fails, so it always returns
-	 * 0. Under the assumption that thrd_success is 0 this is a
-	 * tail call. */
-	return __pthread_detach(t);
+	/* 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;
 }
diff --git a/src/thread/thrd_join.c b/src/thread/thrd_join.c
index ac66789..d690676 100644
--- a/src/thread/thrd_join.c
+++ b/src/thread/thrd_join.c
@@ -1,12 +1,22 @@
-#include <stdint.h>
+#include "pthread_impl.h"
+#include <sys/mman.h>
 #include <threads.h>
 
-int __pthread_join(thrd_t, void**);
+int __munmap(void *, size_t);
 
+
+/* We have to be careful to not modify memory at res[1], which we
+ * might do on 64 bit archs, if we'd just interpret *res (an int) as
+ * void*. Therefore this can't be a simple tail call to
+ * __pthread_join.
+ *
+ * As additional bonus, C11 threads cannot be canceled, so there is no
+ * need for a cancelation function pointer, here. */
 int thrd_join(thrd_t t, int *res)
 {
-        void *pthread_res;
-        __pthread_join(t, &pthread_res);
-        if (res) *res = (int)(intptr_t)pthread_res;
-        return thrd_success;
+	int tmp;
+	while ((tmp = t->tid)) __timedwait(&t->tid, tmp, 0, 0, 0, 0, 0);
+	if (res) *res = (int)(intptr_t)t->result;
+	if (t->map_base) __munmap(t->map_base, t->map_size);
+	return thrd_success;
 }
-- 
1.7.10.4