* [PATCH 1/9] interface additions for the C thread implementation
2014-08-31 22:45 [PATCH 0/9] C thread patch series, v. 8.6 and 9.7 Jens Gustedt
@ 2014-08-31 22:45 ` Jens Gustedt
2014-09-07 0:21 ` Rich Felker
2014-09-07 1:19 ` Rich Felker
2014-08-31 22:46 ` [PATCH 2/9] additions to src/time and some implied minor changes here and there Jens Gustedt
` (7 subsequent siblings)
8 siblings, 2 replies; 28+ messages in thread
From: Jens Gustedt @ 2014-08-31 22:45 UTC (permalink / raw)
To: musl
This adds all the constant, type and function interfaces.
It makes pthread_mutex_t, mtx_t, pthread_cond_t and cnd_t different
types.
This only works because
- under hood the corresponding pairs of types use exactly the same
definition for the type
- the types are a struct types without tag name
- no comparison or assignment is allowed for any of these types. For
the POSIX types this interdiction is written in the standard. For the
C thread types, this is an extension that this implementation
imposes, but which might be integrated in a later version of the C
standard.
- initialization is default initialization of an array of int. For the
POSIX types, initialization expressions are provided. For C thread
types the only initialization foreseen by the standard are the init
functions.
- any calls to standard functions use pointers, and because pointer
representations for struct types are the same.
For the C++ API/ABI, these also are different types, now, with type names
(that are used for name mangling, e.g) as listed above.
Somebody better versed in C++ could perhaps contribute code that
overloads the comparison and assignment operators such that a compilation
that tries to compare or copy these types fails.
---
arch/arm/bits/alltypes.h.in | 10 +++-
arch/i386/bits/alltypes.h.in | 10 +++-
arch/microblaze/bits/alltypes.h.in | 10 +++-
arch/mips/bits/alltypes.h.in | 10 +++-
arch/or1k/bits/alltypes.h.in | 10 +++-
arch/powerpc/bits/alltypes.h.in | 10 +++-
arch/sh/bits/alltypes.h.in | 10 +++-
arch/x32/bits/alltypes.h.in | 10 +++-
arch/x86_64/bits/alltypes.h.in | 10 +++-
include/alltypes.h.in | 10 ++++
include/threads.h | 110 ++++++++++++++++++++++++++++++++++++
include/time.h | 11 ++++
12 files changed, 212 insertions(+), 9 deletions(-)
create mode 100644 include/threads.h
diff --git a/arch/arm/bits/alltypes.h.in b/arch/arm/bits/alltypes.h.in
index 183c4c4..1dcb920 100644
--- a/arch/arm/bits/alltypes.h.in
+++ b/arch/arm/bits/alltypes.h.in
@@ -18,8 +18,16 @@ TYPEDEF struct { long long __ll; long double __ld; } max_align_t;
TYPEDEF long time_t;
TYPEDEF long suseconds_t;
-TYPEDEF struct { union { int __i[9]; unsigned __s[9]; } __u; } pthread_attr_t;
+/* The pairs of equivalent definitions for pthread and C thread types
+ * should always be kept in sync.
+ *
+ * Also this only works because the underlying struct has no struct
+ * tag. Don't introduce one. */
TYPEDEF struct { union { int __i[6]; volatile void *volatile __p[6]; } __u; } pthread_mutex_t;
+TYPEDEF struct { union { int __i[6]; volatile void *volatile __p[6]; } __u; } mtx_t;
TYPEDEF struct { union { int __i[12]; void *__p[12]; } __u; } pthread_cond_t;
+TYPEDEF struct { union { int __i[12]; void *__p[12]; } __u; } cnd_t;
+
+TYPEDEF struct { union { int __i[9]; unsigned __s[9]; } __u; } pthread_attr_t;
TYPEDEF struct { union { int __i[8]; void *__p[8]; } __u; } pthread_rwlock_t;
TYPEDEF struct { union { int __i[5]; void *__p[5]; } __u; } pthread_barrier_t;
diff --git a/arch/i386/bits/alltypes.h.in b/arch/i386/bits/alltypes.h.in
index 8ba8f6f..6fe9d05 100644
--- a/arch/i386/bits/alltypes.h.in
+++ b/arch/i386/bits/alltypes.h.in
@@ -32,8 +32,16 @@ TYPEDEF struct { _Alignas(8) long long __ll; long double __ld; } max_align_t;
TYPEDEF long time_t;
TYPEDEF long suseconds_t;
-TYPEDEF struct { union { int __i[9]; unsigned __s[9]; } __u; } pthread_attr_t;
+/* The pairs of equivalent definitions for pthread and C thread types
+ * should always be kept in sync.
+ *
+ * Also this only works because the underlying struct has no struct
+ * tag. Don't introduce one. */
TYPEDEF struct { union { int __i[6]; volatile void *volatile __p[6]; } __u; } pthread_mutex_t;
+TYPEDEF struct { union { int __i[6]; volatile void *volatile __p[6]; } __u; } mtx_t;
TYPEDEF struct { union { int __i[12]; void *__p[12]; } __u; } pthread_cond_t;
+TYPEDEF struct { union { int __i[12]; void *__p[12]; } __u; } cnd_t;
+
+TYPEDEF struct { union { int __i[9]; unsigned __s[9]; } __u; } pthread_attr_t;
TYPEDEF struct { union { int __i[8]; void *__p[8]; } __u; } pthread_rwlock_t;
TYPEDEF struct { union { int __i[5]; void *__p[5]; } __u; } pthread_barrier_t;
diff --git a/arch/microblaze/bits/alltypes.h.in b/arch/microblaze/bits/alltypes.h.in
index a03e1b8..ad4d898 100644
--- a/arch/microblaze/bits/alltypes.h.in
+++ b/arch/microblaze/bits/alltypes.h.in
@@ -18,8 +18,16 @@ TYPEDEF struct { long long __ll; long double __ld; } max_align_t;
TYPEDEF long time_t;
TYPEDEF long suseconds_t;
-TYPEDEF struct { union { int __i[9]; unsigned __s[9]; } __u; } pthread_attr_t;
+/* The pairs of equivalent definitions for pthread and C thread types
+ * should always be kept in sync.
+ *
+ * Also this only works because the underlying struct has no struct
+ * tag. Don't introduce one. */
TYPEDEF struct { union { int __i[6]; volatile void *volatile __p[6]; } __u; } pthread_mutex_t;
+TYPEDEF struct { union { int __i[6]; volatile void *volatile __p[6]; } __u; } mtx_t;
TYPEDEF struct { union { int __i[12]; void *__p[12]; } __u; } pthread_cond_t;
+TYPEDEF struct { union { int __i[12]; void *__p[12]; } __u; } cnd_t;
+
+TYPEDEF struct { union { int __i[9]; unsigned __s[9]; } __u; } pthread_attr_t;
TYPEDEF struct { union { int __i[8]; void *__p[8]; } __u; } pthread_rwlock_t;
TYPEDEF struct { union { int __i[5]; void *__p[5]; } __u; } pthread_barrier_t;
diff --git a/arch/mips/bits/alltypes.h.in b/arch/mips/bits/alltypes.h.in
index a03e1b8..ad4d898 100644
--- a/arch/mips/bits/alltypes.h.in
+++ b/arch/mips/bits/alltypes.h.in
@@ -18,8 +18,16 @@ TYPEDEF struct { long long __ll; long double __ld; } max_align_t;
TYPEDEF long time_t;
TYPEDEF long suseconds_t;
-TYPEDEF struct { union { int __i[9]; unsigned __s[9]; } __u; } pthread_attr_t;
+/* The pairs of equivalent definitions for pthread and C thread types
+ * should always be kept in sync.
+ *
+ * Also this only works because the underlying struct has no struct
+ * tag. Don't introduce one. */
TYPEDEF struct { union { int __i[6]; volatile void *volatile __p[6]; } __u; } pthread_mutex_t;
+TYPEDEF struct { union { int __i[6]; volatile void *volatile __p[6]; } __u; } mtx_t;
TYPEDEF struct { union { int __i[12]; void *__p[12]; } __u; } pthread_cond_t;
+TYPEDEF struct { union { int __i[12]; void *__p[12]; } __u; } cnd_t;
+
+TYPEDEF struct { union { int __i[9]; unsigned __s[9]; } __u; } pthread_attr_t;
TYPEDEF struct { union { int __i[8]; void *__p[8]; } __u; } pthread_rwlock_t;
TYPEDEF struct { union { int __i[5]; void *__p[5]; } __u; } pthread_barrier_t;
diff --git a/arch/or1k/bits/alltypes.h.in b/arch/or1k/bits/alltypes.h.in
index 183c4c4..1dcb920 100644
--- a/arch/or1k/bits/alltypes.h.in
+++ b/arch/or1k/bits/alltypes.h.in
@@ -18,8 +18,16 @@ TYPEDEF struct { long long __ll; long double __ld; } max_align_t;
TYPEDEF long time_t;
TYPEDEF long suseconds_t;
-TYPEDEF struct { union { int __i[9]; unsigned __s[9]; } __u; } pthread_attr_t;
+/* The pairs of equivalent definitions for pthread and C thread types
+ * should always be kept in sync.
+ *
+ * Also this only works because the underlying struct has no struct
+ * tag. Don't introduce one. */
TYPEDEF struct { union { int __i[6]; volatile void *volatile __p[6]; } __u; } pthread_mutex_t;
+TYPEDEF struct { union { int __i[6]; volatile void *volatile __p[6]; } __u; } mtx_t;
TYPEDEF struct { union { int __i[12]; void *__p[12]; } __u; } pthread_cond_t;
+TYPEDEF struct { union { int __i[12]; void *__p[12]; } __u; } cnd_t;
+
+TYPEDEF struct { union { int __i[9]; unsigned __s[9]; } __u; } pthread_attr_t;
TYPEDEF struct { union { int __i[8]; void *__p[8]; } __u; } pthread_rwlock_t;
TYPEDEF struct { union { int __i[5]; void *__p[5]; } __u; } pthread_barrier_t;
diff --git a/arch/powerpc/bits/alltypes.h.in b/arch/powerpc/bits/alltypes.h.in
index ee7f137..63f88ca 100644
--- a/arch/powerpc/bits/alltypes.h.in
+++ b/arch/powerpc/bits/alltypes.h.in
@@ -18,8 +18,16 @@ TYPEDEF struct { long long __ll; long double __ld; } max_align_t;
TYPEDEF long time_t;
TYPEDEF long suseconds_t;
-TYPEDEF struct { union { int __i[9]; unsigned __s[9]; } __u; } pthread_attr_t;
+/* The pairs of equivalent definitions for pthread and C thread types
+ * should always be kept in sync.
+ *
+ * Also this only works because the underlying struct has no struct
+ * tag. Don't introduce one. */
TYPEDEF struct { union { int __i[6]; volatile void *volatile __p[6]; } __u; } pthread_mutex_t;
+TYPEDEF struct { union { int __i[6]; volatile void *volatile __p[6]; } __u; } mtx_t;
TYPEDEF struct { union { int __i[12]; void *__p[12]; } __u; } pthread_cond_t;
+TYPEDEF struct { union { int __i[12]; void *__p[12]; } __u; } cnd_t;
+
+TYPEDEF struct { union { int __i[9]; unsigned __s[9]; } __u; } pthread_attr_t;
TYPEDEF struct { union { int __i[8]; void *__p[8]; } __u; } pthread_rwlock_t;
TYPEDEF struct { union { int __i[5]; void *__p[5]; } __u; } pthread_barrier_t;
diff --git a/arch/sh/bits/alltypes.h.in b/arch/sh/bits/alltypes.h.in
index ee7f137..63f88ca 100644
--- a/arch/sh/bits/alltypes.h.in
+++ b/arch/sh/bits/alltypes.h.in
@@ -18,8 +18,16 @@ TYPEDEF struct { long long __ll; long double __ld; } max_align_t;
TYPEDEF long time_t;
TYPEDEF long suseconds_t;
-TYPEDEF struct { union { int __i[9]; unsigned __s[9]; } __u; } pthread_attr_t;
+/* The pairs of equivalent definitions for pthread and C thread types
+ * should always be kept in sync.
+ *
+ * Also this only works because the underlying struct has no struct
+ * tag. Don't introduce one. */
TYPEDEF struct { union { int __i[6]; volatile void *volatile __p[6]; } __u; } pthread_mutex_t;
+TYPEDEF struct { union { int __i[6]; volatile void *volatile __p[6]; } __u; } mtx_t;
TYPEDEF struct { union { int __i[12]; void *__p[12]; } __u; } pthread_cond_t;
+TYPEDEF struct { union { int __i[12]; void *__p[12]; } __u; } cnd_t;
+
+TYPEDEF struct { union { int __i[9]; unsigned __s[9]; } __u; } pthread_attr_t;
TYPEDEF struct { union { int __i[8]; void *__p[8]; } __u; } pthread_rwlock_t;
TYPEDEF struct { union { int __i[5]; void *__p[5]; } __u; } pthread_barrier_t;
diff --git a/arch/x32/bits/alltypes.h.in b/arch/x32/bits/alltypes.h.in
index 8e396c9..3e8f086 100644
--- a/arch/x32/bits/alltypes.h.in
+++ b/arch/x32/bits/alltypes.h.in
@@ -23,8 +23,16 @@ TYPEDEF struct { long long __ll; long double __ld; } max_align_t;
TYPEDEF long long time_t;
TYPEDEF long long suseconds_t;
-TYPEDEF struct { union { int __i[14]; unsigned long __s[7]; } __u; } pthread_attr_t;
+/* The pairs of equivalent definitions for pthread and C thread types
+ * should always be kept in sync.
+ *
+ * Also this only works because the underlying struct has no struct
+ * tag. Don't introduce one. */
TYPEDEF struct { union { int __i[10]; volatile void *volatile __p[5]; } __u; } pthread_mutex_t;
+TYPEDEF struct { union { int __i[10]; volatile void *volatile __p[5]; } __u; } mtx_t;
TYPEDEF struct { union { int __i[12]; void *__p[6]; } __u; } pthread_cond_t;
+TYPEDEF struct { union { int __i[12]; void *__p[6]; } __u; } cnd_t;
+
+TYPEDEF struct { union { int __i[14]; unsigned long __s[7]; } __u; } pthread_attr_t;
TYPEDEF struct { union { int __i[14]; void *__p[7]; } __u; } pthread_rwlock_t;
TYPEDEF struct { union { int __i[8]; void *__p[4]; } __u; } pthread_barrier_t;
diff --git a/arch/x86_64/bits/alltypes.h.in b/arch/x86_64/bits/alltypes.h.in
index 7b4f3e7..c0169c7 100644
--- a/arch/x86_64/bits/alltypes.h.in
+++ b/arch/x86_64/bits/alltypes.h.in
@@ -23,8 +23,16 @@ TYPEDEF struct { long long __ll; long double __ld; } max_align_t;
TYPEDEF long time_t;
TYPEDEF long suseconds_t;
-TYPEDEF struct { union { int __i[14]; unsigned long __s[7]; } __u; } pthread_attr_t;
+/* The pairs of equivalent definitions for pthread and C thread types
+ * should always be kept in sync.
+ *
+ * Also this only works because the underlying struct has no struct
+ * tag. Don't introduce one. */
TYPEDEF struct { union { int __i[10]; volatile void *volatile __p[5]; } __u; } pthread_mutex_t;
+TYPEDEF struct { union { int __i[10]; volatile void *volatile __p[5]; } __u; } mtx_t;
TYPEDEF struct { union { int __i[12]; void *__p[6]; } __u; } pthread_cond_t;
+TYPEDEF struct { union { int __i[12]; void *__p[6]; } __u; } cnd_t;
+
+TYPEDEF struct { union { int __i[14]; unsigned long __s[7]; } __u; } pthread_attr_t;
TYPEDEF struct { union { int __i[14]; void *__p[7]; } __u; } pthread_rwlock_t;
TYPEDEF struct { union { int __i[8]; void *__p[4]; } __u; } pthread_barrier_t;
diff --git a/include/alltypes.h.in b/include/alltypes.h.in
index c4ca5d5..94364e8 100644
--- a/include/alltypes.h.in
+++ b/include/alltypes.h.in
@@ -43,13 +43,23 @@ TYPEDEF unsigned gid_t;
TYPEDEF int key_t;
TYPEDEF unsigned useconds_t;
+/* The pairs of equivalent definitions for pthread and C thread types
+ * should always be kept in sync. */
#ifdef __cplusplus
TYPEDEF unsigned long pthread_t;
+TYPEDEF unsigned long thrd_t;
#else
TYPEDEF struct __pthread * pthread_t;
+TYPEDEF struct __pthread * thrd_t;
#endif
TYPEDEF int pthread_once_t;
+TYPEDEF int once_flag;
TYPEDEF unsigned pthread_key_t;
+TYPEDEF unsigned tss_t;
+
+TYPEDEF pthread_cond_t cnd_t;
+TYPEDEF pthread_mutex_t mtx_t;
+
TYPEDEF int pthread_spinlock_t;
TYPEDEF struct { unsigned __attr; } pthread_mutexattr_t;
TYPEDEF struct { unsigned __attr; } pthread_condattr_t;
diff --git a/include/threads.h b/include/threads.h
new file mode 100644
index 0000000..0e99443
--- /dev/null
+++ b/include/threads.h
@@ -0,0 +1,110 @@
+#ifndef _THREADS_H
+#define _THREADS_H
+
+/* This one is explicitly allowed to be included. */
+#include <time.h>
+
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/* These may or may not be implemented to be the same as some POSIX
+ * types. Don't rely on any assumption about that, in particular if
+ * you happen to use both interfaces in the same code. */
+
+#define __NEED_cnd_t
+#define __NEED_mtx_t
+#define __NEED_once_flag
+#define __NEED_thrd_t
+#define __NEED_tss_t
+
+#include <bits/alltypes.h>
+
+typedef int (*thrd_start_t)(void*);
+typedef void (*tss_dtor_t)(void*);
+
+/* This should be a keyword for C++11 */
+#ifndef __cplusplus
+# define thread_local _Thread_local
+#endif
+
+ /* The following list of 10 integer constants makes up for the binary
+ compatibility of this C thread implementation. You must never
+ link code against versions of the C library that do not agree
+ upon these ABI parameters.
+
+ Additionally this implementation assumes that the 5 types have
+ the same size across C libraries and that these types can be
+ initialized by the default initializer.
+
+ The values for the 10 parameters are those that we proposed to
+ glibc, in the hope that they will agree.
+ */
+
+#define TSS_DTOR_ITERATIONS 4
+
+enum {
+ thrd_success = 0,
+ thrd_busy = 1,
+ thrd_error = 2,
+ thrd_nomem = 3,
+ thrd_timedout = 4,
+};
+
+enum {
+ mtx_plain = 0,
+ mtx_recursive = 1,
+ // all mutexes are timed, here. so this is a no-op
+ mtx_timed = 2,
+};
+
+#define ONCE_FLAG_INIT { 0 }
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+int thrd_create(thrd_t *, thrd_start_t, void *);
+_Noreturn void thrd_exit(int);
+
+int thrd_detach(thrd_t);
+int thrd_join(thrd_t, int *);
+
+int thrd_sleep(const struct timespec *, struct timespec *);
+void thrd_yield(void);
+
+thrd_t thrd_current(void);
+int thrd_equal(thrd_t, thrd_t);
+#define thrd_equal(A, B) ((A) == (B))
+
+void call_once(once_flag *, void (*)(void));
+
+int mtx_init(mtx_t *, int);
+void mtx_destroy(mtx_t *);
+
+int mtx_lock(mtx_t *);
+int mtx_timedlock(mtx_t *restrict, const struct timespec *restrict);
+int mtx_trylock(mtx_t *);
+int mtx_unlock(mtx_t *);
+
+int cnd_init(cnd_t *);
+void cnd_destroy(cnd_t *);
+
+int cnd_broadcast(cnd_t *);
+int cnd_signal(cnd_t *);
+
+int cnd_timedwait(cnd_t *restrict, mtx_t *restrict, const struct timespec *restrict);
+int cnd_wait(cnd_t *, mtx_t *);
+
+int tss_create(tss_t *, tss_dtor_t);
+void tss_delete(tss_t key);
+
+int tss_set(tss_t, void *);
+void *tss_get(tss_t);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif
diff --git a/include/time.h b/include/time.h
index dc88070..9ea9c73 100644
--- a/include/time.h
+++ b/include/time.h
@@ -129,6 +129,17 @@ int stime(const time_t *);
time_t timegm(struct tm *);
#endif
+ /* Implementation specific choice: The epoch that the TIME_UTC clock
+ is based upon is the Unix Epoch, that is a struct timespec
+ represents the number of seconds that have elapsed since 00:00:00
+ Coordinated Universal Time (UTC), Thursday, 1 January
+ 1970. Because of differences in leap seconds this is not
+ completely equivalent to UTC. */
+ /* Beware that the TIME_UTC constant itself per the standard must be
+ greater than 0. */
+#define TIME_UTC 1
+int timespec_get(struct timespec *, int);
+
#ifdef __cplusplus
}
#endif
--
1.7.10.4
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 1/9] interface additions for the C thread implementation
2014-08-31 22:45 ` [PATCH 1/9] interface additions for the C thread implementation Jens Gustedt
@ 2014-09-07 0:21 ` Rich Felker
2014-09-07 9:13 ` Jens Gustedt
2014-09-07 1:19 ` Rich Felker
1 sibling, 1 reply; 28+ messages in thread
From: Rich Felker @ 2014-09-07 0:21 UTC (permalink / raw)
To: musl
On Mon, Sep 01, 2014 at 12:45:47AM +0200, Jens Gustedt wrote:
> This adds all the constant, type and function interfaces.
Including some comments here as I work on committing it. Please don't
take them as critical; just explaining choices made during commit and
noting where things are different in the version to be committed.
> It makes pthread_mutex_t, mtx_t, pthread_cond_t and cnd_t different
> types.
>
> This only works because
>
> - under hood the corresponding pairs of types use exactly the same
> definition for the type
>
> - the types are a struct types without tag name
>
> - no comparison or assignment is allowed for any of these types. For
> the POSIX types this interdiction is written in the standard. For the
> C thread types, this is an extension that this implementation
> imposes, but which might be integrated in a later version of the C
> standard.
>
> - initialization is default initialization of an array of int. For the
> POSIX types, initialization expressions are provided. For C thread
> types the only initialization foreseen by the standard are the init
> functions.
>
> - any calls to standard functions use pointers, and because pointer
> representations for struct types are the same.
>
> For the C++ API/ABI, these also are different types, now, with type names
> (that are used for name mangling, e.g) as listed above.
>
> Somebody better versed in C++ could perhaps contribute code that
> overloads the comparison and assignment operators such that a compilation
> that tries to compare or copy these types fails.
I'm not sure what you meant by this last paragraph.
> ---
> arch/arm/bits/alltypes.h.in | 10 +++-
> arch/i386/bits/alltypes.h.in | 10 +++-
> arch/microblaze/bits/alltypes.h.in | 10 +++-
> arch/mips/bits/alltypes.h.in | 10 +++-
> arch/or1k/bits/alltypes.h.in | 10 +++-
> arch/powerpc/bits/alltypes.h.in | 10 +++-
> arch/sh/bits/alltypes.h.in | 10 +++-
> arch/x32/bits/alltypes.h.in | 10 +++-
> arch/x86_64/bits/alltypes.h.in | 10 +++-
> include/alltypes.h.in | 10 ++++
> include/threads.h | 110 ++++++++++++++++++++++++++++++++++++
> include/time.h | 11 ++++
> 12 files changed, 212 insertions(+), 9 deletions(-)
> create mode 100644 include/threads.h
>
> diff --git a/arch/arm/bits/alltypes.h.in b/arch/arm/bits/alltypes.h.in
> index 183c4c4..1dcb920 100644
> --- a/arch/arm/bits/alltypes.h.in
> +++ b/arch/arm/bits/alltypes.h.in
> @@ -18,8 +18,16 @@ TYPEDEF struct { long long __ll; long double __ld; } max_align_t;
> TYPEDEF long time_t;
> TYPEDEF long suseconds_t;
>
> -TYPEDEF struct { union { int __i[9]; unsigned __s[9]; } __u; } pthread_attr_t;
> +/* The pairs of equivalent definitions for pthread and C thread types
> + * should always be kept in sync.
> + *
> + * Also this only works because the underlying struct has no struct
> + * tag. Don't introduce one. */
No need for comments like these; introducing a tag would be an ABI
change anyway, so it can't be done.
> diff --git a/include/alltypes.h.in b/include/alltypes.h.in
> index c4ca5d5..94364e8 100644
> --- a/include/alltypes.h.in
> +++ b/include/alltypes.h.in
> @@ -43,13 +43,23 @@ TYPEDEF unsigned gid_t;
> TYPEDEF int key_t;
> TYPEDEF unsigned useconds_t;
>
> +/* The pairs of equivalent definitions for pthread and C thread types
> + * should always be kept in sync. */
> #ifdef __cplusplus
> TYPEDEF unsigned long pthread_t;
> +TYPEDEF unsigned long thrd_t;
> #else
> TYPEDEF struct __pthread * pthread_t;
> +TYPEDEF struct __pthread * thrd_t;
> #endif
> TYPEDEF int pthread_once_t;
> +TYPEDEF int once_flag;
> TYPEDEF unsigned pthread_key_t;
> +TYPEDEF unsigned tss_t;
The threads.h types don't need to be in alltypes.h since they're only
exposed by one file. (The pthread types are there because sys/types.h
exposes them too, in addition to pthread.h.)
> +TYPEDEF pthread_cond_t cnd_t;
> +TYPEDEF pthread_mutex_t mtx_t;
These two lines were left over and seem to have been nops due to the
order of concatenation.
> diff --git a/include/threads.h b/include/threads.h
> new file mode 100644
> index 0000000..0e99443
> --- /dev/null
> +++ b/include/threads.h
> @@ -0,0 +1,110 @@
> +#ifndef _THREADS_H
> +#define _THREADS_H
> +
> +/* This one is explicitly allowed to be included. */
> +#include <time.h>
I'm also adding features.h here because musl needs it internally for
any header that wants to use restrict (we use __restrict because gcc
disables restrict by default in its ugly gnu89 mode) and _Noreturn.
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
This appeared twice with only one closing "}". I'm merging all the C++
logic into one conditional.
> +
> +/* These may or may not be implemented to be the same as some POSIX
> + * types. Don't rely on any assumption about that, in particular if
> + * you happen to use both interfaces in the same code. */
> +
This text (or similar) should eventually make it into the
documentation, but headers don't carry documentation like this in
musl.
> +int cnd_timedwait(cnd_t *restrict, mtx_t *restrict, const struct timespec *restrict);
> +int cnd_wait(cnd_t *, mtx_t *);
The lack of restrict on cnd_wait is really odd, but I checked and it's
like that in the standard too... File this as another bug for C11...
:-)
Rich
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 1/9] interface additions for the C thread implementation
2014-09-07 0:21 ` Rich Felker
@ 2014-09-07 9:13 ` Jens Gustedt
2014-09-07 10:05 ` Alexander Monakov
0 siblings, 1 reply; 28+ messages in thread
From: Jens Gustedt @ 2014-09-07 9:13 UTC (permalink / raw)
To: musl
[-- Attachment #1: Type: text/plain, Size: 3005 bytes --]
Hello,
Am Samstag, den 06.09.2014, 20:21 -0400 schrieb Rich Felker:
> On Mon, Sep 01, 2014 at 12:45:47AM +0200, Jens Gustedt wrote:
> > This adds all the constant, type and function interfaces.
>
> Including some comments here as I work on committing it. Please don't
> take them as critical; just explaining choices made during commit and
> noting where things are different in the version to be committed.
no problem
parts of the inconsistencies you detected come form me refactoring the
patches into more readable chunks than was the development order
> > It makes pthread_mutex_t, mtx_t, pthread_cond_t and cnd_t different
> > types.
> >
> > This only works because
> >
> > - under hood the corresponding pairs of types use exactly the same
> > definition for the type
> >
> > - the types are a struct types without tag name
> >
> > - no comparison or assignment is allowed for any of these types. For
> > the POSIX types this interdiction is written in the standard. For the
> > C thread types, this is an extension that this implementation
> > imposes, but which might be integrated in a later version of the C
> > standard.
> >
> > - initialization is default initialization of an array of int. For the
> > POSIX types, initialization expressions are provided. For C thread
> > types the only initialization foreseen by the standard are the init
> > functions.
> >
> > - any calls to standard functions use pointers, and because pointer
> > representations for struct types are the same.
> >
> > For the C++ API/ABI, these also are different types, now, with type names
> > (that are used for name mangling, e.g) as listed above.
> >
> > Somebody better versed in C++ could perhaps contribute code that
> > overloads the comparison and assignment operators such that a compilation
> > that tries to compare or copy these types fails.
>
> I'm not sure what you meant by this last paragraph.
AFAIR in C++ there are ways to inhibit usage of copy assignment by
declaring some "operator=" function that is never defined. But my C++
has really become rusty.
> > +int cnd_timedwait(cnd_t *restrict, mtx_t *restrict, const struct timespec *restrict);
> > +int cnd_wait(cnd_t *, mtx_t *);
>
> The lack of restrict on cnd_wait is really odd, but I checked and it's
> like that in the standard too... File this as another bug for C11...
> :-)
huh, yea, I didn't even notice, just copied the prototypes from the
standard at the start.
(So implicitly they (willingly or not) assume that cnd_t and mtx can't
alias against each other but with timespec they can :)
On my list, now.
Jens
--
:: INRIA Nancy Grand Est ::: AlGorille ::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536 ::
:: :::::::::::::::::::::: gsm France : +33 651400183 ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/9] interface additions for the C thread implementation
2014-09-07 9:13 ` Jens Gustedt
@ 2014-09-07 10:05 ` Alexander Monakov
2014-09-07 11:16 ` Jens Gustedt
0 siblings, 1 reply; 28+ messages in thread
From: Alexander Monakov @ 2014-09-07 10:05 UTC (permalink / raw)
To: musl
On Sun, 7 Sep 2014, Jens Gustedt wrote:
> > > For the C++ API/ABI, these also are different types, now, with type names
> > > (that are used for name mangling, e.g) as listed above.
> > >
> > > Somebody better versed in C++ could perhaps contribute code that
> > > overloads the comparison and assignment operators such that a compilation
> > > that tries to compare or copy these types fails.
> >
> > I'm not sure what you meant by this last paragraph.
>
> AFAIR in C++ there are ways to inhibit usage of copy assignment by
> declaring some "operator=" function that is never defined. But my C++
> has really become rusty.
There's no need to do that since those are unrelated structs, and therefore no
operator== and operator= are available in the first place. You also can't do
that in C (but in C++ you get an error rather than a warning when trying
to assign pointers).
Alexander
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/9] interface additions for the C thread implementation
2014-09-07 10:05 ` Alexander Monakov
@ 2014-09-07 11:16 ` Jens Gustedt
2014-09-07 11:31 ` Alexander Monakov
2014-09-07 11:32 ` Rich Felker
0 siblings, 2 replies; 28+ messages in thread
From: Jens Gustedt @ 2014-09-07 11:16 UTC (permalink / raw)
To: musl
[-- Attachment #1: Type: text/plain, Size: 1651 bytes --]
Am Sonntag, den 07.09.2014, 14:05 +0400 schrieb Alexander Monakov:
> On Sun, 7 Sep 2014, Jens Gustedt wrote:
> > > > For the C++ API/ABI, these also are different types, now, with type names
> > > > (that are used for name mangling, e.g) as listed above.
> > > >
> > > > Somebody better versed in C++ could perhaps contribute code that
> > > > overloads the comparison and assignment operators such that a compilation
> > > > that tries to compare or copy these types fails.
> > >
> > > I'm not sure what you meant by this last paragraph.
> >
> > AFAIR in C++ there are ways to inhibit usage of copy assignment by
> > declaring some "operator=" function that is never defined. But my C++
> > has really become rusty.
>
> There's no need to do that since those are unrelated structs, and therefore no
> operator== and operator= are available in the first place. You also can't do
> that in C (but in C++ you get an error rather than a warning when trying
> to assign pointers).
This is not about assignment between different types and also not for
pointers but for the struct themselves.
With the current C threads version the following is a priori allowed,
but shouldn't:
mtx_t a, b;
mtx_init(&a, mtx_plain);
b = a;
This "works" in C and in C++.
The corresponding code in pthreads would be UB.
Jens
--
:: INRIA Nancy Grand Est ::: AlGorille ::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536 ::
:: :::::::::::::::::::::: gsm France : +33 651400183 ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/9] interface additions for the C thread implementation
2014-09-07 11:16 ` Jens Gustedt
@ 2014-09-07 11:31 ` Alexander Monakov
2014-09-07 11:32 ` Rich Felker
1 sibling, 0 replies; 28+ messages in thread
From: Alexander Monakov @ 2014-09-07 11:31 UTC (permalink / raw)
To: musl
On Sun, 7 Sep 2014, Jens Gustedt wrote:
> Am Sonntag, den 07.09.2014, 14:05 +0400 schrieb Alexander Monakov:
> > On Sun, 7 Sep 2014, Jens Gustedt wrote:
> > > > > For the C++ API/ABI, these also are different types, now, with type names
> > > > > (that are used for name mangling, e.g) as listed above.
> > > > >
> > > > > Somebody better versed in C++ could perhaps contribute code that
> > > > > overloads the comparison and assignment operators such that a compilation
> > > > > that tries to compare or copy these types fails.
> > > >
> > > > I'm not sure what you meant by this last paragraph.
> > >
> > > AFAIR in C++ there are ways to inhibit usage of copy assignment by
> > > declaring some "operator=" function that is never defined. But my C++
> > > has really become rusty.
> >
> > There's no need to do that since those are unrelated structs, and therefore no
> > operator== and operator= are available in the first place. You also can't do
> > that in C (but in C++ you get an error rather than a warning when trying
> > to assign pointers).
>
> This is not about assignment between different types and also not for
> pointers but for the struct themselves.
>
> With the current C threads version the following is a priori allowed,
> but shouldn't:
>
> mtx_t a, b;
> mtx_init(&a, mtx_plain);
> b = a;
>
> This "works" in C and in C++.
>
> The corresponding code in pthreads would be UB.
Ah, I completely misunderstood then. But then your suggestion is equally
applicable to pthread types, no? It sounded like you were suggesting to do
that for C11 types only.
Alexander
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/9] interface additions for the C thread implementation
2014-09-07 11:16 ` Jens Gustedt
2014-09-07 11:31 ` Alexander Monakov
@ 2014-09-07 11:32 ` Rich Felker
2014-09-07 14:45 ` Jens Gustedt
1 sibling, 1 reply; 28+ messages in thread
From: Rich Felker @ 2014-09-07 11:32 UTC (permalink / raw)
To: musl
On Sun, Sep 07, 2014 at 01:16:43PM +0200, Jens Gustedt wrote:
> Am Sonntag, den 07.09.2014, 14:05 +0400 schrieb Alexander Monakov:
> > On Sun, 7 Sep 2014, Jens Gustedt wrote:
> > > > > For the C++ API/ABI, these also are different types, now, with type names
> > > > > (that are used for name mangling, e.g) as listed above.
> > > > >
> > > > > Somebody better versed in C++ could perhaps contribute code that
> > > > > overloads the comparison and assignment operators such that a compilation
> > > > > that tries to compare or copy these types fails.
> > > >
> > > > I'm not sure what you meant by this last paragraph.
> > >
> > > AFAIR in C++ there are ways to inhibit usage of copy assignment by
> > > declaring some "operator=" function that is never defined. But my C++
> > > has really become rusty.
> >
> > There's no need to do that since those are unrelated structs, and therefore no
> > operator== and operator= are available in the first place. You also can't do
> > that in C (but in C++ you get an error rather than a warning when trying
> > to assign pointers).
>
> This is not about assignment between different types and also not for
> pointers but for the struct themselves.
>
> With the current C threads version the following is a priori allowed,
> but shouldn't:
>
> mtx_t a, b;
> mtx_init(&a, mtx_plain);
> b = a;
>
> This "works" in C and in C++.
>
> The corresponding code in pthreads would be UB.
I'm not clear on whether the assignment is well-defined in pthreads,
but actually attempting to use the mutex (by passing it to any of the
pthread_mutex_* functions) would be UB. The same should be true for
C11 threads; if not, it's a defect. Assignment cannot have predictable
behavior because:
1. It could copy a reference (that would later be double-freed if you
destroyed both after the copy) in which case both copies would be a
reference to the same underlying mutex.
2. It could contain pointers to its own storage, in which case the
copy would be invalid.
3. It could be completely represented by its internal state, in which
case you'd have two potentially working mutexes.
4. It could be a reference to some system-level object linked purely
to the address of the mtx_t object, in which case the copy would be
unusable and might even cause system state corruption if used.
Etc.
I don't think the committee intended to forbid any of the above types
of implementation; on the contrary it seems they went out of their way
to support crazy types of implementations, e.g. by omitting
initializers.
Rich
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/9] interface additions for the C thread implementation
2014-09-07 11:32 ` Rich Felker
@ 2014-09-07 14:45 ` Jens Gustedt
2014-09-07 15:16 ` Rich Felker
0 siblings, 1 reply; 28+ messages in thread
From: Jens Gustedt @ 2014-09-07 14:45 UTC (permalink / raw)
To: musl
[-- Attachment #1: Type: text/plain, Size: 3505 bytes --]
Am Sonntag, den 07.09.2014, 07:32 -0400 schrieb Rich Felker:
> On Sun, Sep 07, 2014 at 01:16:43PM +0200, Jens Gustedt wrote:
> > Am Sonntag, den 07.09.2014, 14:05 +0400 schrieb Alexander Monakov:
> > > On Sun, 7 Sep 2014, Jens Gustedt wrote:
> > > > > > For the C++ API/ABI, these also are different types, now, with type names
> > > > > > (that are used for name mangling, e.g) as listed above.
> > > > > >
> > > > > > Somebody better versed in C++ could perhaps contribute code that
> > > > > > overloads the comparison and assignment operators such that a compilation
> > > > > > that tries to compare or copy these types fails.
> > > > >
> > > > > I'm not sure what you meant by this last paragraph.
> > > >
> > > > AFAIR in C++ there are ways to inhibit usage of copy assignment by
> > > > declaring some "operator=" function that is never defined. But my C++
> > > > has really become rusty.
> > >
> > > There's no need to do that since those are unrelated structs, and therefore no
> > > operator== and operator= are available in the first place. You also can't do
> > > that in C (but in C++ you get an error rather than a warning when trying
> > > to assign pointers).
> >
> > This is not about assignment between different types and also not for
> > pointers but for the struct themselves.
> >
> > With the current C threads version the following is a priori allowed,
> > but shouldn't:
> >
> > mtx_t a, b;
> > mtx_init(&a, mtx_plain);
> > b = a;
> >
> > This "works" in C and in C++.
> >
> > The corresponding code in pthreads would be UB.
>
> I'm not clear on whether the assignment is well-defined in pthreads,
> but actually attempting to use the mutex (by passing it to any of the
> pthread_mutex_* functions) would be UB. The same should be true for
> C11 threads; if not, it's a defect.
It is.
> Assignment cannot have predictable
> behavior because:
>
> 1. It could copy a reference (that would later be double-freed if you
> destroyed both after the copy) in which case both copies would be a
> reference to the same underlying mutex.
>
> 2. It could contain pointers to its own storage, in which case the
> copy would be invalid.
>
> 3. It could be completely represented by its internal state, in which
> case you'd have two potentially working mutexes.
>
> 4. It could be a reference to some system-level object linked purely
> to the address of the mtx_t object, in which case the copy would be
> unusable and might even cause system state corruption if used.
>
> Etc.
sure, we all (should) know that, but the average user wouldn't
> I don't think the committee intended to forbid any of the above types
> of implementation; on the contrary it seems they went out of their way
> to support crazy types of implementations, e.g. by omitting
> initializers.
No, unfortunately for the later, the lack of a definition for default
initialization and initializers seems to be intentional. There are
people on the committee who defend the interdiction of statically
initialized mutexes, seemingly because some oldish windows thread
implementation didn't have it.
Jens
--
:: INRIA Nancy Grand Est ::: AlGorille ::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536 ::
:: :::::::::::::::::::::: gsm France : +33 651400183 ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/9] interface additions for the C thread implementation
2014-09-07 14:45 ` Jens Gustedt
@ 2014-09-07 15:16 ` Rich Felker
2014-09-07 16:51 ` Jens Gustedt
0 siblings, 1 reply; 28+ messages in thread
From: Rich Felker @ 2014-09-07 15:16 UTC (permalink / raw)
To: musl
On Sun, Sep 07, 2014 at 04:45:01PM +0200, Jens Gustedt wrote:
> Am Sonntag, den 07.09.2014, 07:32 -0400 schrieb Rich Felker:
> > On Sun, Sep 07, 2014 at 01:16:43PM +0200, Jens Gustedt wrote:
> > > Am Sonntag, den 07.09.2014, 14:05 +0400 schrieb Alexander Monakov:
> > > > On Sun, 7 Sep 2014, Jens Gustedt wrote:
> > > > > > > For the C++ API/ABI, these also are different types, now, with type names
> > > > > > > (that are used for name mangling, e.g) as listed above.
> > > > > > >
> > > > > > > Somebody better versed in C++ could perhaps contribute code that
> > > > > > > overloads the comparison and assignment operators such that a compilation
> > > > > > > that tries to compare or copy these types fails.
> > > > > >
> > > > > > I'm not sure what you meant by this last paragraph.
> > > > >
> > > > > AFAIR in C++ there are ways to inhibit usage of copy assignment by
> > > > > declaring some "operator=" function that is never defined. But my C++
> > > > > has really become rusty.
> > > >
> > > > There's no need to do that since those are unrelated structs, and therefore no
> > > > operator== and operator= are available in the first place. You also can't do
> > > > that in C (but in C++ you get an error rather than a warning when trying
> > > > to assign pointers).
> > >
> > > This is not about assignment between different types and also not for
> > > pointers but for the struct themselves.
> > >
> > > With the current C threads version the following is a priori allowed,
> > > but shouldn't:
> > >
> > > mtx_t a, b;
> > > mtx_init(&a, mtx_plain);
> > > b = a;
> > >
> > > This "works" in C and in C++.
> > >
> > > The corresponding code in pthreads would be UB.
> >
> > I'm not clear on whether the assignment is well-defined in pthreads,
> > but actually attempting to use the mutex (by passing it to any of the
> > pthread_mutex_* functions) would be UB. The same should be true for
> > C11 threads; if not, it's a defect.
>
> It is.
It is a defect? Or..?
> sure, we all (should) know that, but the average user wouldn't
>
> > I don't think the committee intended to forbid any of the above types
> > of implementation; on the contrary it seems they went out of their way
> > to support crazy types of implementations, e.g. by omitting
> > initializers.
>
> No, unfortunately for the later, the lack of a definition for default
> initialization and initializers seems to be intentional. There are
> people on the committee who defend the interdiction of statically
> initialized mutexes, seemingly because some oldish windows thread
> implementation didn't have it.
That's what I mean. By refusing to support static initialization of
mutexes, they seem to be supporting the possibility of implementations
for which static initialization is impractical, much like some of the
crazy ideas I mentioned above.
Rich
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/9] interface additions for the C thread implementation
2014-09-07 15:16 ` Rich Felker
@ 2014-09-07 16:51 ` Jens Gustedt
2014-09-07 16:55 ` Rich Felker
0 siblings, 1 reply; 28+ messages in thread
From: Jens Gustedt @ 2014-09-07 16:51 UTC (permalink / raw)
To: musl
[-- Attachment #1: Type: text/plain, Size: 2605 bytes --]
Am Sonntag, den 07.09.2014, 11:16 -0400 schrieb Rich Felker:
> On Sun, Sep 07, 2014 at 04:45:01PM +0200, Jens Gustedt wrote:
> > Am Sonntag, den 07.09.2014, 07:32 -0400 schrieb Rich Felker:
> > > I'm not clear on whether the assignment is well-defined in pthreads,
> > > but actually attempting to use the mutex (by passing it to any of the
> > > pthread_mutex_* functions) would be UB. The same should be true for
> > > C11 threads; if not, it's a defect.
> >
> > It is.
>
> It is a defect? Or..?
sorry, I meant it is a defect. Yet another proof of the severe lack of
specification and semantic that this whole C thread thing has in the
standard.
> > sure, we all (should) know that, but the average user wouldn't
> >
> > > I don't think the committee intended to forbid any of the above types
> > > of implementation; on the contrary it seems they went out of their way
> > > to support crazy types of implementations, e.g. by omitting
> > > initializers.
> >
> > No, unfortunately for the later, the lack of a definition for default
> > initialization and initializers seems to be intentional. There are
> > people on the committee who defend the interdiction of statically
> > initialized mutexes, seemingly because some oldish windows thread
> > implementation didn't have it.
>
> That's what I mean. By refusing to support static initialization of
> mutexes, they seem to be supporting the possibility of implementations
> for which static initialization is impractical, much like some of the
> crazy ideas I mentioned above.
<rant>
Even with those crazy ideas it is easily possible to have the
corresponding function do such a lacking initialization based on a
default 0 initialized field, in the same way pthread_once_t
works. This is a bit of an overhead at each call, but I wouldn't mind
at all penalizing any implementation that deviates from the
"all-default-initialization-is-0" rule.
That's already something the standard has for years for pointers and
floating point. A platform may have different representations for null
pointers or for 0.0. But it is the problem of the platform provider to
do everything that 0 initialization does the right thing, and not to
leave such crazy thing to the user of the type.
</rant>
Jens
--
:: INRIA Nancy Grand Est ::: AlGorille ::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536 ::
:: :::::::::::::::::::::: gsm France : +33 651400183 ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/9] interface additions for the C thread implementation
2014-09-07 16:51 ` Jens Gustedt
@ 2014-09-07 16:55 ` Rich Felker
0 siblings, 0 replies; 28+ messages in thread
From: Rich Felker @ 2014-09-07 16:55 UTC (permalink / raw)
To: musl
On Sun, Sep 07, 2014 at 06:51:12PM +0200, Jens Gustedt wrote:
> <rant>
> Even with those crazy ideas it is easily possible to have the
> corresponding function do such a lacking initialization based on a
> default 0 initialized field, in the same way pthread_once_t
> works. This is a bit of an overhead at each call, but I wouldn't mind
> at all penalizing any implementation that deviates from the
> "all-default-initialization-is-0" rule.
>
> That's already something the standard has for years for pointers and
> floating point. A platform may have different representations for null
> pointers or for 0.0. But it is the problem of the platform provider to
> do everything that 0 initialization does the right thing, and not to
> leave such crazy thing to the user of the type.
> </rant>
I agree totally. In principle you can always have a self-initializing
mutex, since it can use whatever mechanism call_once uses. There is
some cost to this approach, but I don't think it would be
unreasonable. I don't mind encouraging use of call_once though. It's a
shame pthread_once never got more widely publicized/used...
Rich
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 1/9] interface additions for the C thread implementation
2014-08-31 22:45 ` [PATCH 1/9] interface additions for the C thread implementation Jens Gustedt
2014-09-07 0:21 ` Rich Felker
@ 2014-09-07 1:19 ` Rich Felker
1 sibling, 0 replies; 28+ messages in thread
From: Rich Felker @ 2014-09-07 1:19 UTC (permalink / raw)
To: musl
On Mon, Sep 01, 2014 at 12:45:47AM +0200, Jens Gustedt wrote:
> This adds all the constant, type and function interfaces.
>
> It makes pthread_mutex_t, mtx_t, pthread_cond_t and cnd_t different
> types.
>
> This only works because
>
> - under hood the corresponding pairs of types use exactly the same
> definition for the type
>
> - the types are a struct types without tag name
>
> - no comparison or assignment is allowed for any of these types. For
> the POSIX types this interdiction is written in the standard. For the
> C thread types, this is an extension that this implementation
> imposes, but which might be integrated in a later version of the C
> standard.
This would only matter if assignment (comparison of aggregates doesn't
even exist in C) were happening between objects of the corresponding
C11 and pthread type in the same TU, which is invalid anyway. Nothing
in the way these types are implemented precludes assignment between
objects of the same type (e.g. mtx_t and mtx_t) or assignment to the
opposite (but compatible) type from a different TU (think of
pthread_mutex_init writing to a mtx_t in the application via
assignment to the dereferenced pthread_mutex_t pointer, which would
have been a possible implementation choice for mtx_init).
BTW there's nothing in the standard to preclude assignment of mtx_t
objects or cnd_t objects that would otherwise be legal, but there's
also no reason to think you should be able to use such a copy with the
threads.h functions. POSIX explicitly spells out the fact that you
can't do this for POSIX sync objects, so if C11 doesn't do the same,
this is probably another defect you should file. Obviously it's
intended that mtx_t objects could hold handles to a system resource
_OR_ actually be the in-memory sync object, so there's no way
assignment could be expected to produce an object that well-defined
behavior.
> - initialization is default initialization of an array of int. For the
> POSIX types, initialization expressions are provided. For C thread
> types the only initialization foreseen by the standard are the init
> functions.
I didn't understand what this has to do with the choice of
implementation.
> - any calls to standard functions use pointers, and because pointer
> representations for struct types are the same.
The type compatibility rules would also apply to aggregates passed by
value between TUs.
Rich
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 2/9] additions to src/time and some implied minor changes here and there
2014-08-31 22:45 [PATCH 0/9] C thread patch series, v. 8.6 and 9.7 Jens Gustedt
2014-08-31 22:45 ` [PATCH 1/9] interface additions for the C thread implementation Jens Gustedt
@ 2014-08-31 22:46 ` Jens Gustedt
2014-09-06 17:44 ` Rich Felker
2014-08-31 22:46 ` [PATCH 3/9] use weak symbols for the POSIX functions that will be used by C threads Jens Gustedt
` (6 subsequent siblings)
8 siblings, 1 reply; 28+ messages in thread
From: Jens Gustedt @ 2014-08-31 22:46 UTC (permalink / raw)
To: musl
This refactors clock_gettime and adds two functions, thrd_sleep and
timespec_get.
The refactoring of clock_gettime introduces a function " clock_gettime"
that has the same signature as clock_gettime, but that returns an
eventual error instead of returning -1 and setting errno. All places that
use clock_gettime functionality internally in musl do this
unconditionally, so there is no reason to mess around with errno.
As an additional fallout for kernels that don't implement the
clock_gettime syscall (yet!), the syscall that results in ENOSYS is
effected only once (instead, as previously, for every call) and a
fallback to gettimeofday is used.
The two newly introduced C11 functions have easy functional equivalences
in POSIX, but these have subtle differences in the handling of errors.
thrd_sleep forces concrete numerical values as error return
timespec_get has a call interface that is incompatible with POSIX because
it has a bogus coding for its clock constants, and also this clock
constants must be returned in case of success. For the moment we only
implement one single clock, TIME_UTC, and map this to
CLOCK_REALTIME. This is the clock that we later need to measure time for
the timedlock and timedwait.
Also, the error handling is different than the one for clock_gettime: the
possible reason for failure are returned and not set in errno.
---
src/aio/aio_suspend.c | 4 +++-
src/network/res_mkquery.c | 4 +++-
src/network/res_msend.c | 4 +++-
src/time/__clock_gettime.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
src/time/clock_gettime.c | 41 +++++++----------------------------------
src/time/ftime.c | 4 +++-
src/time/gettimeofday.c | 4 +++-
src/time/thrd_sleep.c | 26 ++++++++++++++++++++++++++
src/time/timespec_get.c | 12 ++++++++++++
9 files changed, 104 insertions(+), 39 deletions(-)
create mode 100644 src/time/__clock_gettime.c
create mode 100644 src/time/thrd_sleep.c
create mode 100644 src/time/timespec_get.c
diff --git a/src/aio/aio_suspend.c b/src/aio/aio_suspend.c
index 39a1d3a..1e3b7d2 100644
--- a/src/aio/aio_suspend.c
+++ b/src/aio/aio_suspend.c
@@ -2,6 +2,8 @@
#include <errno.h>
#include "pthread_impl.h"
+int __clock_gettime(clockid_t clk, struct timespec *ts);
+
/* Due to the requirement that aio_suspend be async-signal-safe, we cannot
* use any locks, wait queues, etc. that would make it more efficient. The
* only obviously-correct algorithm is to generate a wakeup every time any
@@ -35,7 +37,7 @@ int aio_suspend(const struct aiocb *const cbs[], int cnt, const struct timespec
}
if (first && ts) {
- clock_gettime(CLOCK_MONOTONIC, &at);
+ __clock_gettime(CLOCK_MONOTONIC, &at);
at.tv_sec += ts->tv_sec;
if ((at.tv_nsec += ts->tv_nsec) >= 1000000000) {
at.tv_nsec -= 1000000000;
diff --git a/src/network/res_mkquery.c b/src/network/res_mkquery.c
index ec4568a..8bbaf23 100644
--- a/src/network/res_mkquery.c
+++ b/src/network/res_mkquery.c
@@ -3,6 +3,8 @@
#include <time.h>
#include "libc.h"
+int __clock_gettime(clockid_t clk, struct timespec *ts);
+
int __res_mkquery(int op, const char *dname, int class, int type,
const unsigned char *data, int datalen,
const unsigned char *newrr, unsigned char *buf, int buflen)
@@ -32,7 +34,7 @@ int __res_mkquery(int op, const char *dname, int class, int type,
q[i+3] = class;
/* Make a reasonably unpredictable id */
- clock_gettime(CLOCK_REALTIME, &ts);
+ __clock_gettime(CLOCK_REALTIME, &ts);
id = ts.tv_nsec + ts.tv_nsec/65536UL & 0xffff;
q[0] = id/256;
q[1] = id;
diff --git a/src/network/res_msend.c b/src/network/res_msend.c
index 35f106d..eff968a 100644
--- a/src/network/res_msend.c
+++ b/src/network/res_msend.c
@@ -14,6 +14,8 @@
#include "syscall.h"
#include "lookup.h"
+int __clock_gettime(clockid_t clk, struct timespec *ts);
+
static void cleanup(void *p)
{
__syscall(SYS_close, (intptr_t)p);
@@ -22,7 +24,7 @@ static void cleanup(void *p)
static unsigned long mtime()
{
struct timespec ts;
- clock_gettime(CLOCK_REALTIME, &ts);
+ __clock_gettime(CLOCK_REALTIME, &ts);
return (unsigned long)ts.tv_sec * 1000
+ ts.tv_nsec / 1000000;
}
diff --git a/src/time/__clock_gettime.c b/src/time/__clock_gettime.c
new file mode 100644
index 0000000..adbd4e6
--- /dev/null
+++ b/src/time/__clock_gettime.c
@@ -0,0 +1,44 @@
+#include <time.h>
+#include <errno.h>
+#include <stdint.h>
+#include "syscall.h"
+#include "libc.h"
+#include "atomic.h"
+
+static int sc_clock_gettime(clockid_t clk, struct timespec *ts)
+{
+ return __syscall(SYS_clock_gettime, clk, ts);
+}
+
+static int sc_gettimeofday(clockid_t clk, struct timespec *ts)
+{
+ if (clk != CLOCK_REALTIME)
+ return EINVAL;
+ __syscall(SYS_gettimeofday, ts, 0);
+ ts->tv_nsec = (int)ts->tv_nsec * 1000;
+ return 0;
+}
+
+void *__vdsosym(const char *, const char *);
+
+int __clock_gettime(clockid_t clk, struct timespec *ts)
+{
+ static int (*cgt)(clockid_t, struct timespec *);
+ if (!cgt) {
+ int (*f)(clockid_t, struct timespec *) = 0;
+#ifdef VDSO_CGT_SYM
+ f = __vdsosym(VDSO_CGT_VER, VDSO_CGT_SYM);
+#endif
+ if (!f) {
+ int r = sc_clock_gettime(clk, ts);
+ if (r == -ENOSYS) {
+ f = sc_gettimeofday;
+ } else {
+ a_cas_p(&cgt, 0, sc_clock_gettime);
+ return r;
+ }
+ }
+ a_cas_p(&cgt, 0, f);
+ }
+ return cgt(clk, ts);
+}
diff --git a/src/time/clock_gettime.c b/src/time/clock_gettime.c
index 799251d..13ddb89 100644
--- a/src/time/clock_gettime.c
+++ b/src/time/clock_gettime.c
@@ -1,41 +1,14 @@
#include <time.h>
#include <errno.h>
-#include <stdint.h>
-#include "syscall.h"
-#include "libc.h"
-#include "atomic.h"
-static int sc_clock_gettime(clockid_t clk, struct timespec *ts)
+int __clock_gettime(clockid_t clk, struct timespec *ts);
+
+int clock_gettime(clockid_t clk, struct timespec *ts)
{
- int r = __syscall(SYS_clock_gettime, clk, ts);
- if (!r) return r;
- if (r == -ENOSYS) {
- if (clk == CLOCK_REALTIME) {
- __syscall(SYS_gettimeofday, ts, 0);
- ts->tv_nsec = (int)ts->tv_nsec * 1000;
- return 0;
- }
- r = -EINVAL;
- }
+ int r = __clock_gettime(clk, ts);
+ /* In case of -1 lower levels already have already taken care
+ * of setting errno. */
+ if (r >= -1) return r;
errno = -r;
return -1;
}
-
-void *__vdsosym(const char *, const char *);
-
-int __clock_gettime(clockid_t clk, struct timespec *ts)
-{
-#ifdef VDSO_CGT_SYM
- static int (*cgt)(clockid_t, struct timespec *);
- if (!cgt) {
- void *f = __vdsosym(VDSO_CGT_VER, VDSO_CGT_SYM);
- if (!f) f = (void *)sc_clock_gettime;
- a_cas_p(&cgt, 0, f);
- }
- return cgt(clk, ts);
-#else
- return sc_clock_gettime(clk, ts);
-#endif
-}
-
-weak_alias(__clock_gettime, clock_gettime);
diff --git a/src/time/ftime.c b/src/time/ftime.c
index a1734d0..00af1ca 100644
--- a/src/time/ftime.c
+++ b/src/time/ftime.c
@@ -1,10 +1,12 @@
#include <sys/timeb.h>
#include <time.h>
+int __clock_gettime(clockid_t clk, struct timespec *ts);
+
int ftime(struct timeb *tp)
{
struct timespec ts;
- clock_gettime(CLOCK_REALTIME, &ts);
+ __clock_gettime(CLOCK_REALTIME, &ts);
tp->time = ts.tv_sec;
tp->millitm = ts.tv_nsec / 1000000;
tp->timezone = tp->dstflag = 0;
diff --git a/src/time/gettimeofday.c b/src/time/gettimeofday.c
index 691f8e9..375ec0e 100644
--- a/src/time/gettimeofday.c
+++ b/src/time/gettimeofday.c
@@ -2,11 +2,13 @@
#include <sys/time.h>
#include "syscall.h"
+int __clock_gettime(clockid_t clk, struct timespec *ts);
+
int gettimeofday(struct timeval *restrict tv, void *restrict tz)
{
struct timespec ts;
if (!tv) return 0;
- clock_gettime(CLOCK_REALTIME, &ts);
+ __clock_gettime(CLOCK_REALTIME, &ts);
tv->tv_sec = ts.tv_sec;
tv->tv_usec = (int)ts.tv_nsec / 1000;
return 0;
diff --git a/src/time/thrd_sleep.c b/src/time/thrd_sleep.c
new file mode 100644
index 0000000..3dbfe47
--- /dev/null
+++ b/src/time/thrd_sleep.c
@@ -0,0 +1,26 @@
+#include <time.h>
+#include <errno.h>
+#include "syscall.h"
+#include "libc.h"
+#include "threads.h"
+
+/* Roughly __syscall already returns the right thing: 0 if all went
+ well or a negative error indication, otherwise. */
+int thrd_sleep(const struct timespec *req, struct timespec *rem)
+{
+ int ret = __syscall(SYS_nanosleep, req, rem);
+ switch (ret) {
+ case 0:
+ return 0;
+ /* error described by POSIX: */
+ /* EINTR The nanosleep() function was interrupted by a signal. */
+ /* The C11 wording is: */
+ /* -1 if it has been interrupted by a signal */
+ case -EINTR:
+ return -1;
+ /* EINVAL: described by POSIX */
+ /* EFAULT: described for linux */
+ default:
+ return -2;
+ }
+}
diff --git a/src/time/timespec_get.c b/src/time/timespec_get.c
new file mode 100644
index 0000000..601c3cc
--- /dev/null
+++ b/src/time/timespec_get.c
@@ -0,0 +1,12 @@
+#include <time.h>
+
+int __clock_gettime(clockid_t clk, struct timespec *ts);
+
+/* There is no other implemented value than TIME_UTC, all other values
+ are considered erroneous. */
+int timespec_get(struct timespec * ts, int base)
+{
+ if (base != TIME_UTC) return 0;
+ int ret = __clock_gettime(CLOCK_REALTIME, ts);
+ return ret < 0 ? 0 : base;
+}
--
1.7.10.4
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 2/9] additions to src/time and some implied minor changes here and there
2014-08-31 22:46 ` [PATCH 2/9] additions to src/time and some implied minor changes here and there Jens Gustedt
@ 2014-09-06 17:44 ` Rich Felker
0 siblings, 0 replies; 28+ messages in thread
From: Rich Felker @ 2014-09-06 17:44 UTC (permalink / raw)
To: musl
On Mon, Sep 01, 2014 at 12:46:05AM +0200, Jens Gustedt wrote:
> This refactors clock_gettime and adds two functions, thrd_sleep and
> timespec_get.
I'm committing just timespec_get and the time.h changes as the first
commit. These are independent of threads (but needed to use the full
C11 threads API). I've put the header and implementation changes
together in one commit so that there's not a revision that has one but
not the other (in which case it would be a possibly-mildly-broken
revision). I also found that struct timespec needs to be exposed even
without POSIX (since it's now in C11), so I fixed that too.
Rich
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 3/9] use weak symbols for the POSIX functions that will be used by C threads
2014-08-31 22:45 [PATCH 0/9] C thread patch series, v. 8.6 and 9.7 Jens Gustedt
2014-08-31 22:45 ` [PATCH 1/9] interface additions for the C thread implementation Jens Gustedt
2014-08-31 22:46 ` [PATCH 2/9] additions to src/time and some implied minor changes here and there Jens Gustedt
@ 2014-08-31 22:46 ` Jens Gustedt
2014-09-06 18:52 ` Rich Felker
2014-08-31 22:46 ` [PATCH 4/9] add the functions for tss_t and once_flag Jens Gustedt
` (5 subsequent siblings)
8 siblings, 1 reply; 28+ messages in thread
From: Jens Gustedt @ 2014-08-31 22:46 UTC (permalink / raw)
To: musl
The intent of this is to avoid name space pollution of the C threads
implementation.
This has two sides to it. First we have to provide symbols that wouldn't
pollute the name space for the C threads implementation. Second we have
to clean up some internal uses of POSIX functions such that they don't
implicitly drag in such symbols.
---
src/mman/mprotect.c | 4 +++-
src/thread/__timedwait.c | 9 ++++++---
src/thread/pthread_cond_timedwait.c | 12 +++++++++---
src/thread/pthread_create.c | 21 ++++++++++++++-------
src/thread/pthread_detach.c | 8 ++++++--
src/thread/pthread_getspecific.c | 5 ++++-
src/thread/pthread_join.c | 8 ++++++--
src/thread/pthread_key_create.c | 7 +++++--
src/thread/pthread_mutex_lock.c | 8 ++++++--
src/thread/pthread_mutex_timedlock.c | 4 +++-
src/thread/pthread_mutex_trylock.c | 4 +++-
src/thread/pthread_mutex_unlock.c | 4 +++-
src/thread/pthread_once.c | 4 +++-
src/thread/pthread_setcancelstate.c | 4 +++-
src/thread/pthread_testcancel.c | 4 +++-
15 files changed, 77 insertions(+), 29 deletions(-)
diff --git a/src/mman/mprotect.c b/src/mman/mprotect.c
index f488486..535787b 100644
--- a/src/mman/mprotect.c
+++ b/src/mman/mprotect.c
@@ -2,10 +2,12 @@
#include "libc.h"
#include "syscall.h"
-int mprotect(void *addr, size_t len, int prot)
+int __mprotect(void *addr, size_t len, int prot)
{
size_t start, end;
start = (size_t)addr & -PAGE_SIZE;
end = (size_t)((char *)addr + len + PAGE_SIZE-1) & -PAGE_SIZE;
return syscall(SYS_mprotect, start, end-start, prot);
}
+
+weak_alias(__mprotect, mprotect);
diff --git a/src/thread/__timedwait.c b/src/thread/__timedwait.c
index d6f1233..3d62272 100644
--- a/src/thread/__timedwait.c
+++ b/src/thread/__timedwait.c
@@ -4,6 +4,9 @@
#include "futex.h"
#include "syscall.h"
+int __pthread_setcancelstate(int new, int *old);
+int __clock_gettime(clockid_t clk, struct timespec *ts);
+
int __timedwait(volatile int *addr, int val,
clockid_t clk, const struct timespec *at,
void (*cleanup)(void *), void *arg, int priv)
@@ -15,7 +18,7 @@ int __timedwait(volatile int *addr, int val,
if (at) {
if (at->tv_nsec >= 1000000000UL) return EINVAL;
- if (clock_gettime(clk, &to)) return EINVAL;
+ if (__clock_gettime(clk, &to)) return EINVAL;
to.tv_sec = at->tv_sec - to.tv_sec;
if ((to.tv_nsec = at->tv_nsec - to.tv_nsec) < 0) {
to.tv_sec--;
@@ -25,7 +28,7 @@ int __timedwait(volatile int *addr, int val,
top = &to;
}
- if (!cleanup) pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cs);
+ if (!cleanup) __pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cs);
pthread_cleanup_push(cleanup, arg);
r = -__syscall_cp(SYS_futex, addr, FUTEX_WAIT|priv, val, top);
@@ -33,7 +36,7 @@ int __timedwait(volatile int *addr, int val,
if (r != EINTR && r != ETIMEDOUT) r = 0;
pthread_cleanup_pop(0);
- if (!cleanup) pthread_setcancelstate(cs, 0);
+ if (!cleanup) __pthread_setcancelstate(cs, 0);
return r;
}
diff --git a/src/thread/pthread_cond_timedwait.c b/src/thread/pthread_cond_timedwait.c
index 2d192b0..393d60e 100644
--- a/src/thread/pthread_cond_timedwait.c
+++ b/src/thread/pthread_cond_timedwait.c
@@ -1,5 +1,9 @@
#include "pthread_impl.h"
+void __pthread_testcancel(void);
+int __pthread_mutex_lock(pthread_mutex_t *);
+int __pthread_mutex_unlock(pthread_mutex_t *m);
+
/*
* struct waiter
*
@@ -119,7 +123,7 @@ static void unwait(void *arg)
}
}
-int pthread_cond_timedwait(pthread_cond_t *restrict c, pthread_mutex_t *restrict m, const struct timespec *restrict ts)
+int __pthread_cond_timedwait(pthread_cond_t *restrict c, pthread_mutex_t *restrict m, const struct timespec *restrict ts)
{
struct waiter node = { .cond = c, .mutex = m };
int e, seq, *fut, clock = c->_c_clock;
@@ -130,7 +134,7 @@ int pthread_cond_timedwait(pthread_cond_t *restrict c, pthread_mutex_t *restrict
if (ts && ts->tv_nsec >= 1000000000UL)
return EINVAL;
- pthread_testcancel();
+ __pthread_testcancel();
if (c->_c_shared) {
node.shared = 1;
@@ -151,7 +155,7 @@ int pthread_cond_timedwait(pthread_cond_t *restrict c, pthread_mutex_t *restrict
unlock(&c->_c_lock);
}
- pthread_mutex_unlock(m);
+ __pthread_mutex_unlock(m);
do e = __timedwait(fut, seq, clock, ts, unwait, &node, !node.shared);
while (*fut==seq && (!e || e==EINTR));
@@ -197,3 +201,5 @@ int __private_cond_signal(pthread_cond_t *c, int n)
return 0;
}
+
+weak_alias(__pthread_cond_timedwait, pthread_cond_timedwait);
diff --git a/src/thread/pthread_create.c b/src/thread/pthread_create.c
index e441bda..ed265fb 100644
--- a/src/thread/pthread_create.c
+++ b/src/thread/pthread_create.c
@@ -5,6 +5,10 @@
#include <sys/mman.h>
#include <string.h>
+void *__mmap(void *, size_t, int, int, int, off_t);
+int __munmap(void *, size_t);
+int __mprotect(void *, size_t, int);
+
static void dummy_0()
{
}
@@ -14,7 +18,7 @@ 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)
+_Noreturn void __pthread_exit(void *result)
{
pthread_t self = __pthread_self();
sigset_t set;
@@ -139,7 +143,7 @@ static void init_file_lock(FILE *f)
void *__copy_tls(unsigned char *);
-int pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict attrp, void *(*entry)(void *), void *restrict arg)
+static int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict attrp, void *(*entry)(void *), void *restrict arg)
{
int ret;
size_t size, guard;
@@ -191,14 +195,14 @@ int pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict attrp
if (!tsd) {
if (guard) {
- map = mmap(0, size, PROT_NONE, MAP_PRIVATE|MAP_ANON, -1, 0);
+ map = __mmap(0, size, PROT_NONE, MAP_PRIVATE|MAP_ANON, -1, 0);
if (map == MAP_FAILED) goto fail;
- if (mprotect(map+guard, size-guard, PROT_READ|PROT_WRITE)) {
- munmap(map, size);
+ if (__mprotect(map+guard, size-guard, PROT_READ|PROT_WRITE)) {
+ __munmap(map, size);
goto fail;
}
} else {
- map = mmap(0, size, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANON, -1, 0);
+ map = __mmap(0, size, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANON, -1, 0);
if (map == MAP_FAILED) goto fail;
}
tsd = map + size - __pthread_tsd_size;
@@ -240,7 +244,7 @@ int pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict attrp
if (ret < 0) {
a_dec(&libc.threads_minus_1);
- if (map) munmap(map, size);
+ if (map) __munmap(map, size);
return EAGAIN;
}
@@ -258,3 +262,6 @@ fail:
__release_ptc();
return EAGAIN;
}
+
+weak_alias(__pthread_exit, pthread_exit);
+weak_alias(__pthread_create, pthread_create);
diff --git a/src/thread/pthread_detach.c b/src/thread/pthread_detach.c
index 651c38e..5ca7855 100644
--- a/src/thread/pthread_detach.c
+++ b/src/thread/pthread_detach.c
@@ -1,11 +1,15 @@
#include "pthread_impl.h"
-int pthread_detach(pthread_t t)
+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_getspecific.c b/src/thread/pthread_getspecific.c
index b2a282c..bfc4294 100644
--- a/src/thread/pthread_getspecific.c
+++ b/src/thread/pthread_getspecific.c
@@ -1,7 +1,10 @@
#include "pthread_impl.h"
-void *pthread_getspecific(pthread_key_t k)
+static void *__pthread_getspecific(pthread_key_t k)
{
struct pthread *self = __pthread_self();
return self->tsd[k];
}
+
+weak_alias(__pthread_getspecific, pthread_getspecific);
+weak_alias(__pthread_getspecific, tss_get);
diff --git a/src/thread/pthread_join.c b/src/thread/pthread_join.c
index 719c91c..bca89aa 100644
--- a/src/thread/pthread_join.c
+++ b/src/thread/pthread_join.c
@@ -1,15 +1,19 @@
#include "pthread_impl.h"
#include <sys/mman.h>
+int __munmap(void *, size_t);
+
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);
if (res) *res = t->result;
- if (t->map_base) munmap(t->map_base, t->map_size);
+ if (t->map_base) __munmap(t->map_base, t->map_size);
return 0;
}
+
+weak_alias(__pthread_join, pthread_join);
diff --git a/src/thread/pthread_key_create.c b/src/thread/pthread_key_create.c
index a9187f7..bfcd597 100644
--- a/src/thread/pthread_key_create.c
+++ b/src/thread/pthread_key_create.c
@@ -9,7 +9,7 @@ static void nodtor(void *dummy)
{
}
-int pthread_key_create(pthread_key_t *k, void (*dtor)(void *))
+int __pthread_key_create(pthread_key_t *k, void (*dtor)(void *))
{
unsigned i = (uintptr_t)&k / 16 % PTHREAD_KEYS_MAX;
unsigned j = i;
@@ -31,7 +31,7 @@ int pthread_key_create(pthread_key_t *k, void (*dtor)(void *))
return EAGAIN;
}
-int pthread_key_delete(pthread_key_t k)
+int __pthread_key_delete(pthread_key_t k)
{
keys[k] = 0;
return 0;
@@ -53,3 +53,6 @@ void __pthread_tsd_run_dtors()
}
}
}
+
+weak_alias(__pthread_key_delete, pthread_key_delete);
+weak_alias(__pthread_key_create, pthread_key_create);
diff --git a/src/thread/pthread_mutex_lock.c b/src/thread/pthread_mutex_lock.c
index 2a9a3aa..993f54c 100644
--- a/src/thread/pthread_mutex_lock.c
+++ b/src/thread/pthread_mutex_lock.c
@@ -1,10 +1,14 @@
#include "pthread_impl.h"
-int pthread_mutex_lock(pthread_mutex_t *m)
+int __pthread_mutex_timedlock(pthread_mutex_t *restrict m, const struct timespec *restrict at);
+
+int __pthread_mutex_lock(pthread_mutex_t *m)
{
if ((m->_m_type&15) == PTHREAD_MUTEX_NORMAL
&& !a_cas(&m->_m_lock, 0, EBUSY))
return 0;
- return pthread_mutex_timedlock(m, 0);
+ return __pthread_mutex_timedlock(m, 0);
}
+
+weak_alias(__pthread_mutex_lock, pthread_mutex_lock);
diff --git a/src/thread/pthread_mutex_timedlock.c b/src/thread/pthread_mutex_timedlock.c
index ae883f9..16241ee 100644
--- a/src/thread/pthread_mutex_timedlock.c
+++ b/src/thread/pthread_mutex_timedlock.c
@@ -1,6 +1,6 @@
#include "pthread_impl.h"
-int pthread_mutex_timedlock(pthread_mutex_t *restrict m, const struct timespec *restrict at)
+int __pthread_mutex_timedlock(pthread_mutex_t *restrict m, const struct timespec *restrict at)
{
if ((m->_m_type&15) == PTHREAD_MUTEX_NORMAL
&& !a_cas(&m->_m_lock, 0, EBUSY))
@@ -30,3 +30,5 @@ int pthread_mutex_timedlock(pthread_mutex_t *restrict m, const struct timespec *
}
return r;
}
+
+weak_alias(__pthread_mutex_timedlock, pthread_mutex_timedlock);
diff --git a/src/thread/pthread_mutex_trylock.c b/src/thread/pthread_mutex_trylock.c
index e851517..9be7930 100644
--- a/src/thread/pthread_mutex_trylock.c
+++ b/src/thread/pthread_mutex_trylock.c
@@ -50,9 +50,11 @@ int __pthread_mutex_trylock_owner(pthread_mutex_t *m)
return 0;
}
-int pthread_mutex_trylock(pthread_mutex_t *m)
+static int __pthread_mutex_trylock(pthread_mutex_t *m)
{
if ((m->_m_type&15) == PTHREAD_MUTEX_NORMAL)
return a_cas(&m->_m_lock, 0, EBUSY) & EBUSY;
return __pthread_mutex_trylock_owner(m);
}
+
+weak_alias(__pthread_mutex_trylock, pthread_mutex_trylock);
diff --git a/src/thread/pthread_mutex_unlock.c b/src/thread/pthread_mutex_unlock.c
index 46761d9..a7f39c7 100644
--- a/src/thread/pthread_mutex_unlock.c
+++ b/src/thread/pthread_mutex_unlock.c
@@ -3,7 +3,7 @@
void __vm_lock_impl(int);
void __vm_unlock_impl(void);
-int pthread_mutex_unlock(pthread_mutex_t *m)
+int __pthread_mutex_unlock(pthread_mutex_t *m)
{
pthread_t self;
int waiters = m->_m_waiters;
@@ -36,3 +36,5 @@ int pthread_mutex_unlock(pthread_mutex_t *m)
__wake(&m->_m_lock, 1, priv);
return 0;
}
+
+weak_alias(__pthread_mutex_unlock, pthread_mutex_unlock);
diff --git a/src/thread/pthread_once.c b/src/thread/pthread_once.c
index 2eb0f93..05ebe69 100644
--- a/src/thread/pthread_once.c
+++ b/src/thread/pthread_once.c
@@ -6,7 +6,7 @@ static void undo(void *control)
__wake(control, 1, 1);
}
-int pthread_once(pthread_once_t *control, void (*init)(void))
+int __pthread_once(pthread_once_t *control, void (*init)(void))
{
static int waiters;
@@ -34,3 +34,5 @@ int pthread_once(pthread_once_t *control, void (*init)(void))
return 0;
}
}
+
+weak_alias(__pthread_once, pthread_once);
diff --git a/src/thread/pthread_setcancelstate.c b/src/thread/pthread_setcancelstate.c
index 060bcdc..2268217 100644
--- a/src/thread/pthread_setcancelstate.c
+++ b/src/thread/pthread_setcancelstate.c
@@ -1,6 +1,6 @@
#include "pthread_impl.h"
-int pthread_setcancelstate(int new, int *old)
+int __pthread_setcancelstate(int new, int *old)
{
if (new > 1U) return EINVAL;
if (!libc.has_thread_pointer) return ENOSYS;
@@ -9,3 +9,5 @@ int pthread_setcancelstate(int new, int *old)
self->canceldisable = new;
return 0;
}
+
+weak_alias(__pthread_setcancelstate, pthread_setcancelstate);
diff --git a/src/thread/pthread_testcancel.c b/src/thread/pthread_testcancel.c
index ba5f7c6..ee48e6d 100644
--- a/src/thread/pthread_testcancel.c
+++ b/src/thread/pthread_testcancel.c
@@ -7,7 +7,9 @@ static void dummy()
weak_alias(dummy, __testcancel);
-void pthread_testcancel()
+void __pthread_testcancel()
{
__testcancel();
}
+
+weak_alias(__pthread_testcancel, pthread_testcancel);
--
1.7.10.4
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 3/9] use weak symbols for the POSIX functions that will be used by C threads
2014-08-31 22:46 ` [PATCH 3/9] use weak symbols for the POSIX functions that will be used by C threads Jens Gustedt
@ 2014-09-06 18:52 ` Rich Felker
0 siblings, 0 replies; 28+ messages in thread
From: Rich Felker @ 2014-09-06 18:52 UTC (permalink / raw)
To: musl
On Mon, Sep 01, 2014 at 12:46:23AM +0200, Jens Gustedt wrote:
> The intent of this is to avoid name space pollution of the C threads
> implementation.
I found a few bugs in this which I'm fixing and about to commit if all
goes well:
> diff --git a/src/mman/mprotect.c b/src/mman/mprotect.c
> index f488486..535787b 100644
> --- a/src/mman/mprotect.c
> +++ b/src/mman/mprotect.c
> @@ -2,10 +2,12 @@
> #include "libc.h"
> #include "syscall.h"
>
> -int mprotect(void *addr, size_t len, int prot)
> +int __mprotect(void *addr, size_t len, int prot)
> {
> size_t start, end;
> start = (size_t)addr & -PAGE_SIZE;
> end = (size_t)((char *)addr + len + PAGE_SIZE-1) & -PAGE_SIZE;
> return syscall(SYS_mprotect, start, end-start, prot);
> }
> +
> +weak_alias(__mprotect, mprotect);
I tested and inlining the syscall does not make pthread_create.o
smaller. It would eliminate a dependency on one extra file, so I might
make the change later, but for now I'm just doing it your way in the
interest of getting things committed without major edits to the
patches.
> diff --git a/src/thread/__timedwait.c b/src/thread/__timedwait.c
> index d6f1233..3d62272 100644
> --- a/src/thread/__timedwait.c
> +++ b/src/thread/__timedwait.c
> @@ -4,6 +4,9 @@
> #include "futex.h"
> #include "syscall.h"
>
> +int __pthread_setcancelstate(int new, int *old);
> +int __clock_gettime(clockid_t clk, struct timespec *ts);
I've removed arg names from prototypes like this just for consistency
with how it's done elsewhere.
> @@ -139,7 +143,7 @@ static void init_file_lock(FILE *f)
>
> void *__copy_tls(unsigned char *);
>
> -int pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict attrp, void *(*entry)(void *), void *restrict arg)
> +static int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict attrp, void *(*entry)(void *), void *restrict arg)
This static is wrong if the goal is to go ahead and get the files in
the form where they're "ready to use" for C11. I saw it was reverted
in the later patch, but I'm just taking it out here.
> diff --git a/src/thread/pthread_getspecific.c b/src/thread/pthread_getspecific.c
> index b2a282c..bfc4294 100644
> --- a/src/thread/pthread_getspecific.c
> +++ b/src/thread/pthread_getspecific.c
> @@ -1,7 +1,10 @@
> #include "pthread_impl.h"
>
> -void *pthread_getspecific(pthread_key_t k)
> +static void *__pthread_getspecific(pthread_key_t k)
> {
> struct pthread *self = __pthread_self();
> return self->tsd[k];
> }
> +
> +weak_alias(__pthread_getspecific, pthread_getspecific);
> +weak_alias(__pthread_getspecific, tss_get);
This actually goes ahead and implements one C11 function, which is not
horrible, but mildly inconsistent. I think I'll move it to the later
patch that's actually supposed to implement these functions.
> diff --git a/src/thread/pthread_mutex_trylock.c b/src/thread/pthread_mutex_trylock.c
> index e851517..9be7930 100644
> --- a/src/thread/pthread_mutex_trylock.c
> +++ b/src/thread/pthread_mutex_trylock.c
> @@ -50,9 +50,11 @@ int __pthread_mutex_trylock_owner(pthread_mutex_t *m)
> return 0;
> }
>
> -int pthread_mutex_trylock(pthread_mutex_t *m)
> +static int __pthread_mutex_trylock(pthread_mutex_t *m)
> {
> if ((m->_m_type&15) == PTHREAD_MUTEX_NORMAL)
> return a_cas(&m->_m_lock, 0, EBUSY) & EBUSY;
> return __pthread_mutex_trylock_owner(m);
> }
> +
> +weak_alias(__pthread_mutex_trylock, pthread_mutex_trylock);
This static looks like an actual bug that would break the patch series
as a whole; I can't see where it's made visible for mtx_trylock to use
later. Removing static here.
Rich
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 4/9] add the functions for tss_t and once_flag
2014-08-31 22:45 [PATCH 0/9] C thread patch series, v. 8.6 and 9.7 Jens Gustedt
` (2 preceding siblings ...)
2014-08-31 22:46 ` [PATCH 3/9] use weak symbols for the POSIX functions that will be used by C threads Jens Gustedt
@ 2014-08-31 22:46 ` Jens Gustedt
2014-08-31 22:46 ` [PATCH 5/9] add the functions for mtx_t Jens Gustedt
` (4 subsequent siblings)
8 siblings, 0 replies; 28+ messages in thread
From: Jens Gustedt @ 2014-08-31 22:46 UTC (permalink / raw)
To: musl
These all have POSIX equivalents but with some changes to the interfaces.
---
src/thread/call_once.c | 7 +++++++
src/thread/tss_create.c | 11 +++++++++++
src/thread/tss_delete.c | 7 +++++++
src/thread/tss_set.c | 13 +++++++++++++
4 files changed, 38 insertions(+)
create mode 100644 src/thread/call_once.c
create mode 100644 src/thread/tss_create.c
create mode 100644 src/thread/tss_delete.c
create mode 100644 src/thread/tss_set.c
diff --git a/src/thread/call_once.c b/src/thread/call_once.c
new file mode 100644
index 0000000..bb82d04
--- /dev/null
+++ b/src/thread/call_once.c
@@ -0,0 +1,7 @@
+#include <threads.h>
+
+int __pthread_once(once_flag *, void (*)(void));
+
+void call_once(once_flag *flag, void (*func)(void)) {
+ __pthread_once(flag, func);
+}
diff --git a/src/thread/tss_create.c b/src/thread/tss_create.c
new file mode 100644
index 0000000..af8772a
--- /dev/null
+++ b/src/thread/tss_create.c
@@ -0,0 +1,11 @@
+#include <threads.h>
+
+int __pthread_key_create(tss_t *k, void (*dtor)(void *));
+
+int tss_create(tss_t *tss, tss_dtor_t dtor)
+{
+ /* Different error returns are possible. C glues them together into
+ just failure notification. Can't be optimized to a tail call,
+ unless thrd_error equals EAGAIN. */
+ return __pthread_key_create(tss, dtor) ? thrd_error : thrd_success;
+}
diff --git a/src/thread/tss_delete.c b/src/thread/tss_delete.c
new file mode 100644
index 0000000..7d7aacb
--- /dev/null
+++ b/src/thread/tss_delete.c
@@ -0,0 +1,7 @@
+#include <threads.h>
+
+int __pthread_key_delete(tss_t k);
+
+void (tss_delete)(tss_t key) {
+ (void)__pthread_key_delete(key);
+}
diff --git a/src/thread/tss_set.c b/src/thread/tss_set.c
new file mode 100644
index 0000000..70c4fb7
--- /dev/null
+++ b/src/thread/tss_set.c
@@ -0,0 +1,13 @@
+#include "pthread_impl.h"
+#include <threads.h>
+
+int tss_set(tss_t k, void *x)
+{
+ struct pthread *self = __pthread_self();
+ /* Avoid unnecessary COW */
+ if (self->tsd[k] != x) {
+ self->tsd[k] = x;
+ self->tsd_used = 1;
+ }
+ return thrd_success;
+}
--
1.7.10.4
^ permalink raw reply [flat|nested] 28+ messages in thread* [PATCH 5/9] add the functions for mtx_t
2014-08-31 22:45 [PATCH 0/9] C thread patch series, v. 8.6 and 9.7 Jens Gustedt
` (3 preceding siblings ...)
2014-08-31 22:46 ` [PATCH 4/9] add the functions for tss_t and once_flag Jens Gustedt
@ 2014-08-31 22:46 ` Jens Gustedt
2014-09-07 1:51 ` Rich Felker
2014-09-07 1:54 ` Rich Felker
2014-08-31 22:47 ` [PATCH 6/9] add the functions for cnd_t Jens Gustedt
` (3 subsequent siblings)
8 siblings, 2 replies; 28+ messages in thread
From: Jens Gustedt @ 2014-08-31 22:46 UTC (permalink / raw)
To: musl
This is straightforward encapsulation of the corresponding
pthread_mutex_t functions.
---
src/thread/mtx_destroy.c | 5 +++++
src/thread/mtx_init.c | 10 ++++++++++
src/thread/mtx_lock.c | 13 +++++++++++++
src/thread/mtx_timedlock.c | 18 ++++++++++++++++++
src/thread/mtx_trylock.c | 21 +++++++++++++++++++++
src/thread/mtx_unlock.c | 9 +++++++++
6 files changed, 76 insertions(+)
create mode 100644 src/thread/mtx_destroy.c
create mode 100644 src/thread/mtx_init.c
create mode 100644 src/thread/mtx_lock.c
create mode 100644 src/thread/mtx_timedlock.c
create mode 100644 src/thread/mtx_trylock.c
create mode 100644 src/thread/mtx_unlock.c
diff --git a/src/thread/mtx_destroy.c b/src/thread/mtx_destroy.c
new file mode 100644
index 0000000..8d593dc
--- /dev/null
+++ b/src/thread/mtx_destroy.c
@@ -0,0 +1,5 @@
+#include <threads.h>
+
+void mtx_destroy(mtx_t *mtx) {
+ /* empty */
+}
diff --git a/src/thread/mtx_init.c b/src/thread/mtx_init.c
new file mode 100644
index 0000000..a197898
--- /dev/null
+++ b/src/thread/mtx_init.c
@@ -0,0 +1,10 @@
+#include "pthread_impl.h"
+#include <threads.h>
+
+int mtx_init(mtx_t * m, int type)
+{
+ *m = (mtx_t) {
+ ._m_type = ((type&mtx_recursive) ? PTHREAD_MUTEX_RECURSIVE : PTHREAD_MUTEX_NORMAL),
+ };
+ return thrd_success;
+}
diff --git a/src/thread/mtx_lock.c b/src/thread/mtx_lock.c
new file mode 100644
index 0000000..4d5d75e
--- /dev/null
+++ b/src/thread/mtx_lock.c
@@ -0,0 +1,13 @@
+#include "pthread_impl.h"
+#include <threads.h>
+
+int mtx_lock(mtx_t *m)
+{
+ if (m->_m_type == PTHREAD_MUTEX_NORMAL && !a_cas(&m->_m_lock, 0, EBUSY))
+ return thrd_success;
+ /* Calling mtx_timedlock with a null pointer is an
+ extension. Such a call is convenient, here, since it avoids
+ to repeat the case analysis that is already done for
+ mtx_timedlock. */
+ return mtx_timedlock(m, 0);
+}
diff --git a/src/thread/mtx_timedlock.c b/src/thread/mtx_timedlock.c
new file mode 100644
index 0000000..08359d8
--- /dev/null
+++ b/src/thread/mtx_timedlock.c
@@ -0,0 +1,18 @@
+#include <threads.h>
+#include <errno.h>
+
+int __pthread_mutex_timedlock(mtx_t *restrict m, const struct timespec *restrict ts);
+
+int mtx_timedlock(mtx_t *restrict mutex, const struct timespec *restrict ts) {
+ int ret = __pthread_mutex_timedlock(mutex, ts);
+ switch (ret) {
+ /* May also return EINVAL or EAGAIN. EAGAIN is
+ specially tricky since C11 doesn't define how many recursive
+ calls can be done. (this *isn't* the maximum amount of nested
+ calls!) This implementation here deals this with a counter and
+ detects overflow, so this is definitively UB. */
+ default: return thrd_error;
+ case 0: return thrd_success;
+ case ETIMEDOUT: return thrd_timedout;
+ }
+}
diff --git a/src/thread/mtx_trylock.c b/src/thread/mtx_trylock.c
new file mode 100644
index 0000000..4f55796
--- /dev/null
+++ b/src/thread/mtx_trylock.c
@@ -0,0 +1,21 @@
+#include "pthread_impl.h"
+#include <threads.h>
+
+int __pthread_mutex_trylock(pthread_mutex_t *restrict m);
+
+int mtx_trylock(mtx_t *restrict m) {
+ if (m->_m_type == PTHREAD_MUTEX_NORMAL)
+ return (a_cas(&m->_m_lock, 0, EBUSY) & EBUSY) ? thrd_busy : thrd_success;
+
+ int ret = __pthread_mutex_trylock(m);
+ switch (ret) {
+ /* In case of UB may also return EINVAL or EAGAIN. EAGAIN is
+ specially tricky since C11 doesn't define how many recursive
+ calls can be done. (this *isn't* the maximum amount of nested
+ calls!) This implementation here deals this with a counter and
+ detects overflow, so this is definitively UB. */
+ default: return thrd_error;
+ case 0: return thrd_success;
+ case EBUSY: return thrd_busy;
+ }
+}
diff --git a/src/thread/mtx_unlock.c b/src/thread/mtx_unlock.c
new file mode 100644
index 0000000..e673cd5
--- /dev/null
+++ b/src/thread/mtx_unlock.c
@@ -0,0 +1,9 @@
+#include <threads.h>
+
+int __pthread_mutex_unlock(mtx_t *);
+
+int (mtx_unlock)(mtx_t *mtx) {
+ int ret = __pthread_mutex_unlock(mtx);
+ /* In case of UB may also return EPERM. */
+ return ret ? thrd_error : thrd_success;
+}
--
1.7.10.4
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 5/9] add the functions for mtx_t
2014-08-31 22:46 ` [PATCH 5/9] add the functions for mtx_t Jens Gustedt
@ 2014-09-07 1:51 ` Rich Felker
2014-09-07 1:54 ` Rich Felker
1 sibling, 0 replies; 28+ messages in thread
From: Rich Felker @ 2014-09-07 1:51 UTC (permalink / raw)
To: musl
On Mon, Sep 01, 2014 at 12:46:57AM +0200, Jens Gustedt wrote:
> diff --git a/src/thread/mtx_lock.c b/src/thread/mtx_lock.c
> new file mode 100644
> index 0000000..4d5d75e
> --- /dev/null
> +++ b/src/thread/mtx_lock.c
> @@ -0,0 +1,13 @@
> +#include "pthread_impl.h"
> +#include <threads.h>
> +
> +int mtx_lock(mtx_t *m)
> +{
> + if (m->_m_type == PTHREAD_MUTEX_NORMAL && !a_cas(&m->_m_lock, 0, EBUSY))
> + return thrd_success;
> + /* Calling mtx_timedlock with a null pointer is an
> + extension. Such a call is convenient, here, since it avoids
> + to repeat the case analysis that is already done for
> + mtx_timedlock. */
> + return mtx_timedlock(m, 0);
> +}
> diff --git a/src/thread/mtx_timedlock.c b/src/thread/mtx_timedlock.c
> new file mode 100644
> index 0000000..08359d8
> --- /dev/null
> +++ b/src/thread/mtx_timedlock.c
> @@ -0,0 +1,18 @@
> +#include <threads.h>
> +#include <errno.h>
> +
> +int __pthread_mutex_timedlock(mtx_t *restrict m, const struct timespec *restrict ts);
> +
> +int mtx_timedlock(mtx_t *restrict mutex, const struct timespec *restrict ts) {
> + int ret = __pthread_mutex_timedlock(mutex, ts);
> + switch (ret) {
> + /* May also return EINVAL or EAGAIN. EAGAIN is
> + specially tricky since C11 doesn't define how many recursive
> + calls can be done. (this *isn't* the maximum amount of nested
> + calls!) This implementation here deals this with a counter and
> + detects overflow, so this is definitively UB. */
I'm omitting this comment for now because it sems wrong. EINVAL is not
possible here, and I can't find any evidence that "too many recursive
locks" is UB, so since mtx_timedlock is required to report errors when
the operation can't be performed, I think thrd_error is just the
"right behavior". The spec should be more clear though.
> diff --git a/src/thread/mtx_trylock.c b/src/thread/mtx_trylock.c
> new file mode 100644
> index 0000000..4f55796
> --- /dev/null
> +++ b/src/thread/mtx_trylock.c
> @@ -0,0 +1,21 @@
> +#include "pthread_impl.h"
> +#include <threads.h>
> +
> +int __pthread_mutex_trylock(pthread_mutex_t *restrict m);
> +
> +int mtx_trylock(mtx_t *restrict m) {
> + if (m->_m_type == PTHREAD_MUTEX_NORMAL)
> + return (a_cas(&m->_m_lock, 0, EBUSY) & EBUSY) ? thrd_busy : thrd_success;
> +
> + int ret = __pthread_mutex_trylock(m);
> + switch (ret) {
> + /* In case of UB may also return EINVAL or EAGAIN. EAGAIN is
> + specially tricky since C11 doesn't define how many recursive
> + calls can be done. (this *isn't* the maximum amount of nested
> + calls!) This implementation here deals this with a counter and
> + detects overflow, so this is definitively UB. */
Likewise here. One could even argue thrd_busy is more appropriate
here, but for consistency it should probably be thrd_error too.
> diff --git a/src/thread/mtx_unlock.c b/src/thread/mtx_unlock.c
> new file mode 100644
> index 0000000..e673cd5
> --- /dev/null
> +++ b/src/thread/mtx_unlock.c
> @@ -0,0 +1,9 @@
> +#include <threads.h>
> +
> +int __pthread_mutex_unlock(mtx_t *);
> +
> +int (mtx_unlock)(mtx_t *mtx) {
> + int ret = __pthread_mutex_unlock(mtx);
> + /* In case of UB may also return EPERM. */
> + return ret ? thrd_error : thrd_success;
> +}
We had discussed trimming this down at some point (there is no error
that's not UB) to make it a tail call, so I'll try to do that now.
Rich
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 5/9] add the functions for mtx_t
2014-08-31 22:46 ` [PATCH 5/9] add the functions for mtx_t Jens Gustedt
2014-09-07 1:51 ` Rich Felker
@ 2014-09-07 1:54 ` Rich Felker
1 sibling, 0 replies; 28+ messages in thread
From: Rich Felker @ 2014-09-07 1:54 UTC (permalink / raw)
To: musl
On Mon, Sep 01, 2014 at 12:46:57AM +0200, Jens Gustedt wrote:
> diff --git a/src/thread/mtx_trylock.c b/src/thread/mtx_trylock.c
> new file mode 100644
> index 0000000..4f55796
> --- /dev/null
> +++ b/src/thread/mtx_trylock.c
> @@ -0,0 +1,21 @@
> +#include "pthread_impl.h"
> +#include <threads.h>
> +
> +int __pthread_mutex_trylock(pthread_mutex_t *restrict m);
> +
> +int mtx_trylock(mtx_t *restrict m) {
> + if (m->_m_type == PTHREAD_MUTEX_NORMAL)
> + return (a_cas(&m->_m_lock, 0, EBUSY) & EBUSY) ? thrd_busy : thrd_success;
> +
> + int ret = __pthread_mutex_trylock(m);
One more thing -- the compiler caught a type mismatch here for
pthread_mutex_t vs mtx_t. Casting it away would be valid, but it's
also valid to prototype __pthread_mutex_trylock with mtx_t. The latter
is consistent with what's done elsewhere so I'll do that.
Rich
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 6/9] add the functions for cnd_t
2014-08-31 22:45 [PATCH 0/9] C thread patch series, v. 8.6 and 9.7 Jens Gustedt
` (4 preceding siblings ...)
2014-08-31 22:46 ` [PATCH 5/9] add the functions for mtx_t Jens Gustedt
@ 2014-08-31 22:47 ` Jens Gustedt
2014-08-31 22:47 ` [PATCH 7/9] add the thrd_xxxxxx functions Jens Gustedt
` (2 subsequent siblings)
8 siblings, 0 replies; 28+ messages in thread
From: Jens Gustedt @ 2014-08-31 22:47 UTC (permalink / raw)
To: musl
Because of the clear separation for private pthread_cond_t these
interfaces are quite simple and direct.
---
src/thread/cnd_broadcast.c | 10 ++++++++++
src/thread/cnd_destroy.c | 5 +++++
src/thread/cnd_init.c | 7 +++++++
src/thread/cnd_signal.c | 10 ++++++++++
src/thread/cnd_timedwait.c | 14 ++++++++++++++
src/thread/cnd_wait.c | 10 ++++++++++
6 files changed, 56 insertions(+)
create mode 100644 src/thread/cnd_broadcast.c
create mode 100644 src/thread/cnd_destroy.c
create mode 100644 src/thread/cnd_init.c
create mode 100644 src/thread/cnd_signal.c
create mode 100644 src/thread/cnd_timedwait.c
create mode 100644 src/thread/cnd_wait.c
diff --git a/src/thread/cnd_broadcast.c b/src/thread/cnd_broadcast.c
new file mode 100644
index 0000000..c8cf311
--- /dev/null
+++ b/src/thread/cnd_broadcast.c
@@ -0,0 +1,10 @@
+#include <threads.h>
+
+int __private_cond_signal(cnd_t *, int);
+
+int cnd_broadcast(cnd_t * cnd) {
+ /* This internal function never fails, so it always returns
+ * 0. Under the assumption that thrd_success is 0 this is a
+ * tail call. */
+ return __private_cond_signal(cnd, -1);
+}
diff --git a/src/thread/cnd_destroy.c b/src/thread/cnd_destroy.c
new file mode 100644
index 0000000..d960db6
--- /dev/null
+++ b/src/thread/cnd_destroy.c
@@ -0,0 +1,5 @@
+#include <threads.h>
+
+void cnd_destroy(cnd_t *cnd) {
+ /* For private cv this is a no-op */
+}
diff --git a/src/thread/cnd_init.c b/src/thread/cnd_init.c
new file mode 100644
index 0000000..618b649
--- /dev/null
+++ b/src/thread/cnd_init.c
@@ -0,0 +1,7 @@
+#include <threads.h>
+
+int cnd_init(cnd_t * c)
+{
+ *c = (cnd_t){ 0 };
+ return thrd_success;
+}
diff --git a/src/thread/cnd_signal.c b/src/thread/cnd_signal.c
new file mode 100644
index 0000000..c31282f
--- /dev/null
+++ b/src/thread/cnd_signal.c
@@ -0,0 +1,10 @@
+#include <threads.h>
+
+int __private_cond_signal(cnd_t *, int);
+
+int cnd_signal(cnd_t * cnd) {
+ /* This internal function never fails, so it always returns
+ * 0. Under the assumption that thrd_success is 0 this is a
+ * tail call. */
+ return __private_cond_signal(cnd, 1);
+}
diff --git a/src/thread/cnd_timedwait.c b/src/thread/cnd_timedwait.c
new file mode 100644
index 0000000..d69a4f1
--- /dev/null
+++ b/src/thread/cnd_timedwait.c
@@ -0,0 +1,14 @@
+#include <threads.h>
+#include <errno.h>
+
+int __pthread_cond_timedwait(cnd_t *restrict c, mtx_t *restrict m, const struct timespec *restrict ts);
+
+int cnd_timedwait(cnd_t *restrict cond, mtx_t *restrict mutex, const struct timespec *restrict ts) {
+ int ret = __pthread_cond_timedwait(cond, mutex, ts);
+ switch (ret) {
+ /* May also return EINVAL or EPERM. */
+ default: return thrd_error;
+ case 0: return thrd_success;
+ case ETIMEDOUT: return thrd_timedout;
+ }
+}
diff --git a/src/thread/cnd_wait.c b/src/thread/cnd_wait.c
new file mode 100644
index 0000000..91e89db
--- /dev/null
+++ b/src/thread/cnd_wait.c
@@ -0,0 +1,10 @@
+#include <threads.h>
+
+int cnd_wait(cnd_t *cond, mtx_t *mutex)
+{
+ /* Calling cnd_timedwait with a null pointer is an
+ extension. Such a call is convenient, here since it avoids to
+ repeat the case analysis that is already done for
+ cnd_timedwait. */
+ return cnd_timedwait(cond, mutex, 0);
+}
--
1.7.10.4
^ permalink raw reply [flat|nested] 28+ messages in thread* [PATCH 7/9] add the thrd_xxxxxx functions
2014-08-31 22:45 [PATCH 0/9] C thread patch series, v. 8.6 and 9.7 Jens Gustedt
` (5 preceding siblings ...)
2014-08-31 22:47 ` [PATCH 6/9] add the functions for cnd_t Jens Gustedt
@ 2014-08-31 22:47 ` Jens Gustedt
2014-09-07 14:24 ` Rich Felker
2014-08-31 22:47 ` [PATCH 8/9] separate pthread_create and pthread_exit in two different TU Jens Gustedt
2014-08-31 22:48 ` [PATCH 9/9] Separate pthread_create and thrd_create Jens Gustedt
8 siblings, 1 reply; 28+ messages in thread
From: Jens Gustedt @ 2014-08-31 22:47 UTC (permalink / raw)
To: musl
The major difficulty with C threads versus pthread is that the thread
start functions have different signatures. Basing the C threads
implementation on pthreads therefore needs to do some ugly cast with
function pointers and function results.
We try to be the least intrusive for the existing pthreads
implementation, to make sure we don't break anything and also to ease
maintenance and simultaneous improvement of the code base later on.
Unfortunately this is not completely trivial. Not only are some calling
conventions different, but also the level of requested detail for error
returns varies in the two models. Where POSIX maps most possible errors
of pthread_create to EAGAIN, C threads require the condition of being out
of memory to be identified.
---
src/internal/pthread_impl.h | 2 ++
src/sched/thrd_yield.c | 7 +++++++
src/thread/pthread_create.c | 32 +++++++++++++++++++++-----------
src/thread/thrd_create.c | 14 ++++++++++++++
src/thread/thrd_current.c | 11 +++++++++++
src/thread/thrd_detach.c | 11 +++++++++++
src/thread/thrd_equal.c | 6 ++++++
src/thread/thrd_exit.c | 10 ++++++++++
src/thread/thrd_join.c | 12 ++++++++++++
src/time/thrd_sleep.c | 14 +++++++-------
10 files changed, 101 insertions(+), 18 deletions(-)
create mode 100644 src/sched/thrd_yield.c
create mode 100644 src/thread/thrd_create.c
create mode 100644 src/thread/thrd_current.c
create mode 100644 src/thread/thrd_detach.c
create mode 100644 src/thread/thrd_equal.c
create mode 100644 src/thread/thrd_exit.c
create mode 100644 src/thread/thrd_join.c
diff --git a/src/internal/pthread_impl.h b/src/internal/pthread_impl.h
index 74c62cc..ae6e60b 100644
--- a/src/internal/pthread_impl.h
+++ b/src/internal/pthread_impl.h
@@ -128,4 +128,6 @@ void __restore_sigs(void *);
#define DEFAULT_STACK_SIZE 81920
#define DEFAULT_GUARD_SIZE PAGE_SIZE
+#define __ATTRP_C11_THREAD ((void*)(uintptr_t)-1)
+
#endif
diff --git a/src/sched/thrd_yield.c b/src/sched/thrd_yield.c
new file mode 100644
index 0000000..09d534b
--- /dev/null
+++ b/src/sched/thrd_yield.c
@@ -0,0 +1,7 @@
+#include <sched.h>
+#include "syscall.h"
+
+void thrd_yield()
+{
+ syscall(SYS_sched_yield);
+}
diff --git a/src/thread/pthread_create.c b/src/thread/pthread_create.c
index ed265fb..c1feb62 100644
--- a/src/thread/pthread_create.c
+++ b/src/thread/pthread_create.c
@@ -4,10 +4,12 @@
#include "libc.h"
#include <sys/mman.h>
#include <string.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);
+_Noreturn void __thrd_exit(int);
static void dummy_0()
{
@@ -123,6 +125,14 @@ 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)
/* pthread_key_create.c overrides this */
@@ -143,10 +153,10 @@ static void init_file_lock(FILE *f)
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 __pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict attrp, void *(*entry)(void *), void *restrict arg)
{
- int ret;
- size_t size, guard;
+ int ret, c11 = (attrp == __ATTRP_C11_THREAD);
+ size_t size, guard = 0;
struct pthread *self, *new;
unsigned char *map = 0, *stack = 0, *tsd = 0, *stack_limit;
unsigned flags = CLONE_VM | CLONE_FS | CLONE_FILES | CLONE_SIGHAND
@@ -167,7 +177,7 @@ static int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restr
self->tsd = (void **)__pthread_tsd_main;
libc.threaded = 1;
}
- if (attrp) attr = *attrp;
+ if (attrp && !c11) attr = *attrp;
__acquire_ptc();
@@ -196,14 +206,14 @@ static int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restr
if (!tsd) {
if (guard) {
map = __mmap(0, size, PROT_NONE, MAP_PRIVATE|MAP_ANON, -1, 0);
- if (map == MAP_FAILED) goto fail;
+ if (map == MAP_FAILED) goto fail_nomem;
if (__mprotect(map+guard, size-guard, PROT_READ|PROT_WRITE)) {
__munmap(map, size);
- goto fail;
+ goto fail_nomem;
}
} else {
map = __mmap(0, size, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANON, -1, 0);
- if (map == MAP_FAILED) goto fail;
+ if (map == MAP_FAILED) goto fail_nomem;
}
tsd = map + size - __pthread_tsd_size;
if (!stack) {
@@ -234,7 +244,7 @@ static int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restr
new->canary = self->canary;
a_inc(&libc.threads_minus_1);
- ret = __clone(start, stack, flags, new, &new->tid, TP_ADJ(new), &new->tid);
+ ret = __clone((c11 ? start_c11 : start), stack, flags, new, &new->tid, TP_ADJ(new), &new->tid);
__release_ptc();
@@ -245,7 +255,7 @@ static int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restr
if (ret < 0) {
a_dec(&libc.threads_minus_1);
if (map) __munmap(map, size);
- return EAGAIN;
+ return (!c11 || ret != -ENOMEM) ? EAGAIN : ENOMEM;
}
if (do_sched) {
@@ -258,9 +268,9 @@ static int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restr
*res = new;
return 0;
-fail:
+fail_nomem:
__release_ptc();
- return EAGAIN;
+ return c11 ? ENOMEM : EAGAIN;
}
weak_alias(__pthread_exit, pthread_exit);
diff --git a/src/thread/thrd_create.c b/src/thread/thrd_create.c
new file mode 100644
index 0000000..09c1d9e
--- /dev/null
+++ b/src/thread/thrd_create.c
@@ -0,0 +1,14 @@
+#include "pthread_impl.h"
+#include <threads.h>
+
+int __pthread_create(pthread_t *restrict, const pthread_attr_t *restrict, void *(*)(void *), void *restrict);
+
+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;
+ }
+}
diff --git a/src/thread/thrd_current.c b/src/thread/thrd_current.c
new file mode 100644
index 0000000..d9dabde
--- /dev/null
+++ b/src/thread/thrd_current.c
@@ -0,0 +1,11 @@
+#include "pthread_impl.h"
+#include <threads.h>
+
+/* Not all arch have __pthread_self as a symbol. On some this results
+ in some magic. So this "call" to __pthread_self is not necessary a
+ tail call. In particular, other than the appearance, thrd_current
+ can not be implemented as a weak symbol. */
+thrd_t thrd_current()
+{
+ return __pthread_self();
+}
diff --git a/src/thread/thrd_detach.c b/src/thread/thrd_detach.c
new file mode 100644
index 0000000..b0b153c
--- /dev/null
+++ b/src/thread/thrd_detach.c
@@ -0,0 +1,11 @@
+#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);
+}
diff --git a/src/thread/thrd_equal.c b/src/thread/thrd_equal.c
new file mode 100644
index 0000000..ac49a44
--- /dev/null
+++ b/src/thread/thrd_equal.c
@@ -0,0 +1,6 @@
+#include <threads.h>
+
+int (thrd_equal)(thrd_t a, thrd_t b)
+{
+ return a==b;
+}
diff --git a/src/thread/thrd_exit.c b/src/thread/thrd_exit.c
new file mode 100644
index 0000000..50952ac
--- /dev/null
+++ b/src/thread/thrd_exit.c
@@ -0,0 +1,10 @@
+#include "pthread_impl.h"
+#include <threads.h>
+
+_Noreturn void __pthread_exit(void *);
+
+_Noreturn void __thrd_exit(int result) {
+ __pthread_exit((void*)(intptr_t)result);
+}
+
+weak_alias(__thrd_exit, thrd_exit);
diff --git a/src/thread/thrd_join.c b/src/thread/thrd_join.c
new file mode 100644
index 0000000..ac66789
--- /dev/null
+++ b/src/thread/thrd_join.c
@@ -0,0 +1,12 @@
+#include <stdint.h>
+#include <threads.h>
+
+int __pthread_join(thrd_t, void**);
+
+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;
+}
diff --git a/src/time/thrd_sleep.c b/src/time/thrd_sleep.c
index 3dbfe47..d8eaafd 100644
--- a/src/time/thrd_sleep.c
+++ b/src/time/thrd_sleep.c
@@ -5,21 +5,21 @@
#include "threads.h"
/* Roughly __syscall already returns the right thing: 0 if all went
- well or a negative error indication, otherwise. */
+ * well or a negative error indication, otherwise. */
int thrd_sleep(const struct timespec *req, struct timespec *rem)
{
int ret = __syscall(SYS_nanosleep, req, rem);
switch (ret) {
case 0:
return 0;
- /* error described by POSIX: */
- /* EINTR The nanosleep() function was interrupted by a signal. */
- /* The C11 wording is: */
- /* -1 if it has been interrupted by a signal */
+ /* error described by POSIX:
+ * EINTR The nanosleep() function was interrupted by a signal.
+ * The C11 wording is:
+ * -1 if it has been interrupted by a signal */
case -EINTR:
return -1;
- /* EINVAL: described by POSIX */
- /* EFAULT: described for linux */
+ /* EINVAL: described by POSIX
+ * EFAULT: described for linux */
default:
return -2;
}
--
1.7.10.4
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 7/9] add the thrd_xxxxxx functions
2014-08-31 22:47 ` [PATCH 7/9] add the thrd_xxxxxx functions Jens Gustedt
@ 2014-09-07 14:24 ` Rich Felker
2014-09-07 14:52 ` Jens Gustedt
0 siblings, 1 reply; 28+ messages in thread
From: Rich Felker @ 2014-09-07 14:24 UTC (permalink / raw)
To: musl
On Mon, Sep 01, 2014 at 12:47:26AM +0200, Jens Gustedt wrote:
> The major difficulty with C threads versus pthread is that the thread
> start functions have different signatures. Basing the C threads
> implementation on pthreads therefore needs to do some ugly cast with
> function pointers and function results.
>
> We try to be the least intrusive for the existing pthreads
> implementation, to make sure we don't break anything and also to ease
> maintenance and simultaneous improvement of the code base later on.
>
> Unfortunately this is not completely trivial. Not only are some calling
> conventions different, but also the level of requested detail for error
> returns varies in the two models. Where POSIX maps most possible errors
> of pthread_create to EAGAIN, C threads require the condition of being out
> of memory to be identified.
> ---
> src/internal/pthread_impl.h | 2 ++
> src/sched/thrd_yield.c | 7 +++++++
> src/thread/pthread_create.c | 32 +++++++++++++++++++++-----------
> src/thread/thrd_create.c | 14 ++++++++++++++
> src/thread/thrd_current.c | 11 +++++++++++
> src/thread/thrd_detach.c | 11 +++++++++++
> src/thread/thrd_equal.c | 6 ++++++
> src/thread/thrd_exit.c | 10 ++++++++++
> src/thread/thrd_join.c | 12 ++++++++++++
> src/time/thrd_sleep.c | 14 +++++++-------
> 10 files changed, 101 insertions(+), 18 deletions(-)
> create mode 100644 src/sched/thrd_yield.c
> create mode 100644 src/thread/thrd_create.c
> create mode 100644 src/thread/thrd_current.c
> create mode 100644 src/thread/thrd_detach.c
> create mode 100644 src/thread/thrd_equal.c
> create mode 100644 src/thread/thrd_exit.c
> create mode 100644 src/thread/thrd_join.c
>
> diff --git a/src/internal/pthread_impl.h b/src/internal/pthread_impl.h
> index 74c62cc..ae6e60b 100644
> --- a/src/internal/pthread_impl.h
> +++ b/src/internal/pthread_impl.h
> @@ -128,4 +128,6 @@ void __restore_sigs(void *);
> #define DEFAULT_STACK_SIZE 81920
> #define DEFAULT_GUARD_SIZE PAGE_SIZE
>
> +#define __ATTRP_C11_THREAD ((void*)(uintptr_t)-1)
> +
I actually had an idea to get rid of this and instead put a
start-wrapper pointer in the attribute structure, so thrd_create could
just pass an attribute with a pointer to a C11 start function in
thrd_create.c. But the savings are sufficiently small that I'm just
sticking with the way we discussed for now, since it's already
working.
> #endif
> diff --git a/src/sched/thrd_yield.c b/src/sched/thrd_yield.c
> new file mode 100644
> index 0000000..09d534b
> --- /dev/null
> +++ b/src/sched/thrd_yield.c
> @@ -0,0 +1,7 @@
> +#include <sched.h>
> +#include "syscall.h"
> +
> +void thrd_yield()
> +{
> + syscall(SYS_sched_yield);
> +}
Using __syscall here to avoid a useless call to __syscall_ret whose
result will be thrown away anyway.
> diff --git a/src/thread/pthread_create.c b/src/thread/pthread_create.c
> index ed265fb..c1feb62 100644
> --- a/src/thread/pthread_create.c
> +++ b/src/thread/pthread_create.c
> @@ -4,10 +4,12 @@
> #include "libc.h"
> #include <sys/mman.h>
> #include <string.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);
> +_Noreturn void __thrd_exit(int);
>
> static void dummy_0()
> {
> @@ -123,6 +125,14 @@ static int start(void *p)
> return 0;
> }
Not visible here, but start was still calling pthread_exit rather than
__pthread_exit. Fixed it. Before release we should do at least one
more check to make sure no other similar mistakes were missed.
> +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;
> +}
I changed this to call __pthread_exit with the right casts to avoid
pulling in an extra TU and needing the extra namespace-safe name for
thrd_exit.
> -static 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;
> - size_t size, guard;
> + int ret, c11 = (attrp == __ATTRP_C11_THREAD);
> + size_t size, guard = 0;
Removed initialization of guard.
> map = __mmap(0, size, PROT_NONE, MAP_PRIVATE|MAP_ANON, -1, 0);
> - if (map == MAP_FAILED) goto fail;
> + if (map == MAP_FAILED) goto fail_nomem;
> [...]
> @@ -245,7 +255,7 @@ static int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restr
> if (ret < 0) {
> a_dec(&libc.threads_minus_1);
> if (map) __munmap(map, size);
> - return EAGAIN;
> + return (!c11 || ret != -ENOMEM) ? EAGAIN : ENOMEM;
> [...]
> -fail:
> +fail_nomem:
> __release_ptc();
> - return EAGAIN;
> + return c11 ? ENOMEM : EAGAIN;
I took these out for now because (1) I still don't see any indication
that the standard imposes a definition on what "no memory could be
allocated" means in the context of creating a thread, or any way it
could, since the types of allocation involved in creating a thread
will be highly implementation-specific, and (2) it's incomplete
anyway, because mmap and mprotect can fail with ENOMEM even in cases
where the cause is not failure to allocate memory, but arbitrary
limitations (much like the arbitrary limit on number of task slots) on
_how_ virtual memory space can be laid out into areas with distinct
permissions.
My feeling is still that returning thrd_nomem is the most useful error
for applications regardless of what specific memory resource (e.g. vm
areas, address space, commit charge, space in process table, etc.) was
unable to be allocated. If there's a definitive statement to the
contrary from the committee or serious objection from users, we can
discuss it further at a later time.
> weak_alias(__pthread_exit, pthread_exit);
> diff --git a/src/thread/thrd_create.c b/src/thread/thrd_create.c
> new file mode 100644
> index 0000000..09c1d9e
> --- /dev/null
> +++ b/src/thread/thrd_create.c
> @@ -0,0 +1,14 @@
> +#include "pthread_impl.h"
> +#include <threads.h>
> +
> +int __pthread_create(pthread_t *restrict, const pthread_attr_t *restrict, void *(*)(void *), void *restrict);
> +
> +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;
> + }
> +}
Changed this to match.
> diff --git a/src/thread/thrd_current.c b/src/thread/thrd_current.c
> new file mode 100644
> index 0000000..d9dabde
> --- /dev/null
> +++ b/src/thread/thrd_current.c
> @@ -0,0 +1,11 @@
> +#include "pthread_impl.h"
> +#include <threads.h>
> +
> +/* Not all arch have __pthread_self as a symbol. On some this results
> + in some magic. So this "call" to __pthread_self is not necessary a
> + tail call. In particular, other than the appearance, thrd_current
> + can not be implemented as a weak symbol. */
> +thrd_t thrd_current()
> +{
> + return __pthread_self();
> +}
Moved this to an alias.
> diff --git a/src/thread/thrd_detach.c b/src/thread/thrd_detach.c
> new file mode 100644
> index 0000000..b0b153c
> --- /dev/null
> +++ b/src/thread/thrd_detach.c
> @@ -0,0 +1,11 @@
> +#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);
> +}
Moved this to an alias.
> diff --git a/src/thread/thrd_equal.c b/src/thread/thrd_equal.c
> new file mode 100644
> index 0000000..ac49a44
> --- /dev/null
> +++ b/src/thread/thrd_equal.c
> @@ -0,0 +1,6 @@
> +#include <threads.h>
> +
> +int (thrd_equal)(thrd_t a, thrd_t b)
> +{
> + return a==b;
> +}
And this.
> diff --git a/src/time/thrd_sleep.c b/src/time/thrd_sleep.c
> index 3dbfe47..d8eaafd 100644
> --- a/src/time/thrd_sleep.c
> +++ b/src/time/thrd_sleep.c
> @@ -5,21 +5,21 @@
> #include "threads.h"
>
> /* Roughly __syscall already returns the right thing: 0 if all went
> - well or a negative error indication, otherwise. */
> + * well or a negative error indication, otherwise. */
> int thrd_sleep(const struct timespec *req, struct timespec *rem)
> {
> int ret = __syscall(SYS_nanosleep, req, rem);
> switch (ret) {
> case 0:
> return 0;
> - /* error described by POSIX: */
> - /* EINTR The nanosleep() function was interrupted by a signal. */
> - /* The C11 wording is: */
> - /* -1 if it has been interrupted by a signal */
> + /* error described by POSIX:
> + * EINTR The nanosleep() function was interrupted by a signal.
> + * The C11 wording is:
> + * -1 if it has been interrupted by a signal */
> case -EINTR:
> return -1;
> - /* EINVAL: described by POSIX */
> - /* EFAULT: described for linux */
> + /* EINVAL: described by POSIX
> + * EFAULT: described for linux */
> default:
> return -2;
> }
> --
> 1.7.10.4
This diff didn't apply since I didn't add the original version, but I
think leaving it as two commits like you did was unintended anyway.
I'm applying roughly the new version, but with some fixups to include
directives, and less detail in comments.
BTW looking at the spec again, I see that C11 does not seem to require
the negative return value for other errors to be distinct from -1.
Obviously it's desirable to be distinct though. Perhaps this should be
filed as another bug against C11, if you don't have it on your list
already...
Rich
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 7/9] add the thrd_xxxxxx functions
2014-09-07 14:24 ` Rich Felker
@ 2014-09-07 14:52 ` Jens Gustedt
2014-09-07 15:17 ` Rich Felker
0 siblings, 1 reply; 28+ messages in thread
From: Jens Gustedt @ 2014-09-07 14:52 UTC (permalink / raw)
To: musl
[-- Attachment #1: Type: text/plain, Size: 3200 bytes --]
Am Sonntag, den 07.09.2014, 10:24 -0400 schrieb Rich Felker:
> > diff --git a/src/thread/thrd_current.c b/src/thread/thrd_current.c
> > new file mode 100644
> > index 0000000..d9dabde
> > --- /dev/null
> > +++ b/src/thread/thrd_current.c
> > @@ -0,0 +1,11 @@
> > +#include "pthread_impl.h"
> > +#include <threads.h>
> > +
> > +/* Not all arch have __pthread_self as a symbol. On some this results
> > + in some magic. So this "call" to __pthread_self is not necessary a
> > + tail call. In particular, other than the appearance, thrd_current
> > + can not be implemented as a weak symbol. */
> > +thrd_t thrd_current()
> > +{
> > + return __pthread_self();
> > +}
>
> Moved this to an alias.
Did you read the comment? I think that this would be a bug. As the
comment indicates, there is at least one arch variant that hasn't a
symbol __pthread_self so you can't use that as an alias.
> > diff --git a/src/time/thrd_sleep.c b/src/time/thrd_sleep.c
> > index 3dbfe47..d8eaafd 100644
> > --- a/src/time/thrd_sleep.c
> > +++ b/src/time/thrd_sleep.c
> > @@ -5,21 +5,21 @@
> > #include "threads.h"
> >
> > /* Roughly __syscall already returns the right thing: 0 if all went
> > - well or a negative error indication, otherwise. */
> > + * well or a negative error indication, otherwise. */
> > int thrd_sleep(const struct timespec *req, struct timespec *rem)
> > {
> > int ret = __syscall(SYS_nanosleep, req, rem);
> > switch (ret) {
> > case 0:
> > return 0;
> > - /* error described by POSIX: */
> > - /* EINTR The nanosleep() function was interrupted by a signal. */
> > - /* The C11 wording is: */
> > - /* -1 if it has been interrupted by a signal */
> > + /* error described by POSIX:
> > + * EINTR The nanosleep() function was interrupted by a signal.
> > + * The C11 wording is:
> > + * -1 if it has been interrupted by a signal */
> > case -EINTR:
> > return -1;
> > - /* EINVAL: described by POSIX */
> > - /* EFAULT: described for linux */
> > + /* EINVAL: described by POSIX
> > + * EFAULT: described for linux */
> > default:
> > return -2;
> > }
> > --
> > 1.7.10.4
>
> This diff didn't apply since I didn't add the original version, but I
> think leaving it as two commits like you did was unintended anyway.
> I'm applying roughly the new version, but with some fixups to include
> directives, and less detail in comments.
>
> BTW looking at the spec again, I see that C11 does not seem to require
> the negative return value for other errors to be distinct from -1.
oh, yes
> Obviously it's desirable to be distinct though. Perhaps this should be
> filed as another bug against C11, if you don't have it on your list
> already...
what a mess
Jens
--
:: INRIA Nancy Grand Est ::: AlGorille ::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536 ::
:: :::::::::::::::::::::: gsm France : +33 651400183 ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH 7/9] add the thrd_xxxxxx functions
2014-09-07 14:52 ` Jens Gustedt
@ 2014-09-07 15:17 ` Rich Felker
0 siblings, 0 replies; 28+ messages in thread
From: Rich Felker @ 2014-09-07 15:17 UTC (permalink / raw)
To: musl
On Sun, Sep 07, 2014 at 04:52:07PM +0200, Jens Gustedt wrote:
> Am Sonntag, den 07.09.2014, 10:24 -0400 schrieb Rich Felker:
> > > diff --git a/src/thread/thrd_current.c b/src/thread/thrd_current.c
> > > new file mode 100644
> > > index 0000000..d9dabde
> > > --- /dev/null
> > > +++ b/src/thread/thrd_current.c
> > > @@ -0,0 +1,11 @@
> > > +#include "pthread_impl.h"
> > > +#include <threads.h>
> > > +
> > > +/* Not all arch have __pthread_self as a symbol. On some this results
> > > + in some magic. So this "call" to __pthread_self is not necessary a
> > > + tail call. In particular, other than the appearance, thrd_current
> > > + can not be implemented as a weak symbol. */
> > > +thrd_t thrd_current()
> > > +{
> > > + return __pthread_self();
> > > +}
> >
> > Moved this to an alias.
>
> Did you read the comment? I think that this would be a bug. As the
> comment indicates, there is at least one arch variant that hasn't a
> symbol __pthread_self so you can't use that as an alias.
Not an alias of __pthread_self; that's not a function (or if it is an
inline one, that's an implementation detail). Rather I meant an alias
of the function that's in pthread_self.c (but with namespace
protection applied).
Rich
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 8/9] separate pthread_create and pthread_exit in two different TU
2014-08-31 22:45 [PATCH 0/9] C thread patch series, v. 8.6 and 9.7 Jens Gustedt
` (6 preceding siblings ...)
2014-08-31 22:47 ` [PATCH 7/9] add the thrd_xxxxxx functions Jens Gustedt
@ 2014-08-31 22:47 ` Jens Gustedt
2014-08-31 22:48 ` [PATCH 9/9] Separate pthread_create and thrd_create Jens Gustedt
8 siblings, 0 replies; 28+ messages in thread
From: Jens Gustedt @ 2014-08-31 22:47 UTC (permalink / raw)
To: musl
---
src/thread/pthread_create.c | 124 ++---------------------------------------
src/thread/pthread_exit.c | 130 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 135 insertions(+), 119 deletions(-)
create mode 100644 src/thread/pthread_exit.c
diff --git a/src/thread/pthread_create.c b/src/thread/pthread_create.c
index c1feb62..7bc7b0f 100644
--- a/src/thread/pthread_create.c
+++ b/src/thread/pthread_create.c
@@ -10,102 +10,16 @@ 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 *);
+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)
{
@@ -135,24 +49,6 @@ static int start_c11(void *p)
#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)
-{
- if (f && f->lock<0) f->lock = 0;
-}
-
-void *__copy_tls(unsigned char *);
-
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);
@@ -167,16 +63,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) {
- 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 (!libc.threaded) __thread_enable();
if (attrp && !c11) attr = *attrp;
__acquire_ptc();
@@ -273,5 +160,4 @@ fail_nomem:
return c11 ? ENOMEM : EAGAIN;
}
-weak_alias(__pthread_exit, pthread_exit);
weak_alias(__pthread_create, pthread_create);
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);
--
1.7.10.4
^ permalink raw reply [flat|nested] 28+ messages in thread* [PATCH 9/9] Separate pthread_create and thrd_create
2014-08-31 22:45 [PATCH 0/9] C thread patch series, v. 8.6 and 9.7 Jens Gustedt
` (7 preceding siblings ...)
2014-08-31 22:47 ` [PATCH 8/9] separate pthread_create and pthread_exit in two different TU Jens Gustedt
@ 2014-08-31 22:48 ` Jens Gustedt
8 siblings, 0 replies; 28+ messages in thread
From: Jens Gustedt @ 2014-08-31 22:48 UTC (permalink / raw)
To: musl
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
^ permalink raw reply [flat|nested] 28+ messages in thread