use a dedicated futex object for pthread_join instead of tid field

the tid field in the pthread structure is not volatile, and really
shouldn't be, so as not to limit the compiler's ability to reorder,
merge, or split loads in code paths that may be relevant to
performance (like controlling lock ownership).

however, use of objects which are not volatile or atomic with futex
wait is inherently broken, since the compiler is free to transform a
single load into multiple loads, thereby using a different value for
the controlling expression of the loop and the value passed to the
futex syscall, leading the syscall to block instead of returning.

reportedly glibc's pthread_join was actually affected by an equivalent
issue in glibc on s390.

add a separate, dedicated join_futex object for pthread_join to use.

diff --git a/src/env/__init_tls.c b/src/env/__init_tls.c
index b125eb1f543abd34b28524019bd7b8043977b1ca..80044960d454220420d5376749f36b18dce6d07a 100644 (file)
--- a/src/env/__init_tls.c
+++ b/src/env/__init_tls.c
@@ -15,7 +15,8 @@ int __init_tp(void *p)
        int r = __set_thread_area(TP_ADJ(p));
        if (r < 0) return -1;
        if (!r) libc.can_do_threads = 1;
-       td->tid = __syscall(SYS_set_tid_address, &td->tid);
+       td->join_futex = -1;
+       td->tid = __syscall(SYS_set_tid_address, &td->join_futex);
        td->locale = &libc.global_locale;
        td->robust_list.head = &td->robust_list.head;
        return 0;

diff --git a/src/internal/pthread_impl.h b/src/internal/pthread_impl.h
index f6a4f2c28959a17d602fdbfae6e9b000cb4cf72a..f2805b893724e23f25437cdc644e7167f1048982 100644 (file)
--- a/src/internal/pthread_impl.h
+++ b/src/internal/pthread_impl.h
@@ -43,6 +43,7 @@ struct pthread {
        int unblock_cancel;
        volatile int timer_id;
        locale_t locale;
+       volatile int join_futex;
        volatile int killlock[1];
        volatile int exitlock[1];
        volatile int startlock[2];

diff --git a/src/thread/pthread_create.c b/src/thread/pthread_create.c
index 439ee3633ebaaa7b7f7b4cc964b63a9dafa3012a..ac06d7a709d89973855fecf1c52ec3abdc2b0476 100644 (file)
--- a/src/thread/pthread_create.c
+++ b/src/thread/pthread_create.c
@@ -282,9 +282,10 @@ int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict att
        new->robust_list.head = &new->robust_list.head;
        new->unblock_cancel = self->cancel;
        new->CANARY = self->CANARY;
+       new->join_futex = -1;
 
        a_inc(&libc.threads_minus_1);
-       ret = __clone((c11 ? start_c11 : 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->join_futex);
 
        __release_ptc();
 

diff --git a/src/thread/pthread_join.c b/src/thread/pthread_join.c
index b7175c0937c15177d56b4a48b401db4d0d163092..67eaf9d81de134bdd8c131bb015114ea5b1b4e4a 100644 (file)
--- a/src/thread/pthread_join.c
+++ b/src/thread/pthread_join.c
@@ -12,8 +12,8 @@ int __pthread_timedjoin_np(pthread_t t, void **res, const struct timespec *at)
        __pthread_setcancelstate(PTHREAD_CANCEL_DISABLE, &cs);
        if (cs == PTHREAD_CANCEL_ENABLE) __pthread_setcancelstate(cs, 0);
        if (t->detached) a_crash();
-       while ((tmp = t->tid) && r != ETIMEDOUT && r != EINVAL)
-               r = __timedwait_cp(&t->tid, tmp, CLOCK_REALTIME, at, 0);
+       while ((tmp = t->join_futex) && r != ETIMEDOUT && r != EINVAL)
+               r = __timedwait_cp(&t->join_futex, tmp, CLOCK_REALTIME, at, 0);
        __pthread_setcancelstate(cs, 0);
        if (r == ETIMEDOUT || r == EINVAL) return r;
        a_barrier();
@@ -29,7 +29,7 @@ int __pthread_join(pthread_t t, void **res)
 
 int __pthread_tryjoin_np(pthread_t t, void **res)
 {
-       return t->tid ? EBUSY : __pthread_join(t, res);
+       return t->join_futex ? EBUSY : __pthread_join(t, res);
 }
 
 weak_alias(__pthread_tryjoin_np, pthread_tryjoin_np);