summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
authorRich Felker <dalias@aerifal.cx>2018-05-02 12:13:43 -0400
committerRich Felker <dalias@aerifal.cx>2018-05-02 12:33:29 -0400
commit9e2d820a555e150df462f88c901fcbe25d692a8b (patch)
tree104c68bdbeb371bcb9e12c23b31dfc3f96fd57e6 /src
parent941bd884cc0221d051840ce6d21650339e711863 (diff)
downloadmusl-9e2d820a555e150df462f88c901fcbe25d692a8b.tar.gz
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.
Diffstat (limited to 'src')
-rw-r--r--src/env/__init_tls.c3
-rw-r--r--src/internal/pthread_impl.h1
-rw-r--r--src/thread/pthread_create.c3
-rw-r--r--src/thread/pthread_join.c6
4 files changed, 8 insertions, 5 deletions
diff --git a/src/env/__init_tls.c b/src/env/__init_tls.c
index b125eb1f..80044960 100644
--- 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 f6a4f2c2..f2805b89 100644
--- 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 439ee363..ac06d7a7 100644
--- 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 b7175c09..67eaf9d8 100644
--- 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);