nptl: Consolidate pthread_{timed,try}join{_np}

This patch consolidates the pthread_join and gnu extensions to avoid
code duplication.  The function pthread_join, pthread_tryjoin_np, and
pthread_timedjoin_np are now based on pthread_timedjoin_ex.

It also fixes some inconsistencies on ESRCH, EINVAL, EDEADLK handling
(where each implementation differs from each other) and also on
clenup handler (which now always use a CAS).

Checked on i686-linux-gnu and x86_64-linux-gnu.

	* nptl/pthreadP.h (__pthread_timedjoin_np): Define.
	* nptl/pthread_join.c (pthread_join): Use __pthread_timedjoin_np.
	* nptl/pthread_tryjoin.c (pthread_tryjoin): Likewise.
	* nptl/pthread_timedjoin.c (cleanup): Use CAS on argument setting.
	(pthread_timedjoin_np): Define internal symbol and common code from
	pthread_join.
	* sysdeps/unix/sysv/linux/i386/lowlevellock.h (__lll_timedwait_tid):
	Remove superflous checks.
	* sysdeps/unix/sysv/linux/x86_64/lowlevellock.h (__lll_timedwait_tid):
	Likewise.

Signed-off-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Reviewed-by: Carlos O'Donell <carlos@redhat.com>

9 files changed, 145 insertions, 230 deletions

diff --git a/ChangeLog b/ChangeLog
index ad2c9db353..35d1d102f6 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,18 @@
+2017-12-20  Adhemerval Zanella  <adhemerval.zanella@linaro.org>
+
+	* nptl/Makefile (libpthread-routines): Add pthread_join_common.
+	* nptl/pthreadP.h (__pthread_timedjoin_ex): New prototype.
+	* nptl/pthread_join_common.c: New file: common function used on
+	pthread_join, pthread_timedjoin_np, pthread_tryjoin_np.
+	* nptl/pthread_join.c (pthread_join): Use __pthread_timedjoin_ex.
+	* nptl/pthread_tryjoin.c (pthread_tryjoin): Likewise.
+	* nptl/pthread_timedjoin.c (pthread_timedjoin_np): Likewise.
+	(cleanup): Move definition to pthread_join_common.c.
+	* sysdeps/unix/sysv/linux/i386/lowlevellock.h (__lll_timedwait_tid):
+	Remove superflous checks.
+	* sysdeps/unix/sysv/linux/x86_64/lowlevellock.h (__lll_timedwait_tid):
+	Likewise.
+
 2017-12-20  Szabolcs Nagy  <szabolcs.nagy@arm.com>
 
 	* sysdeps/aarch64/libm-test-ulps: Update.
diff --git a/nptl/Makefile b/nptl/Makefile
index ab8cc98728..cf2ba8131b 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -49,6 +49,7 @@ pthread-compat-wrappers = \
 libpthread-routines = nptl-init vars events version pt-interp \
 		      pthread_create pthread_exit pthread_detach \
 		      pthread_join pthread_tryjoin pthread_timedjoin \
+		      pthread_join_common \
 		      compat-pthread_self pthread_equal pthread_yield \
 		      pthread_getconcurrency pthread_setconcurrency \
 		      pthread_getschedparam pthread_setschedparam \
diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
index 713000e32d..2646b8f605 100644
--- a/nptl/pthreadP.h
+++ b/nptl/pthreadP.h
@@ -506,6 +506,8 @@ extern int __pthread_setcanceltype (int type, int *oldtype);
 extern int __pthread_enable_asynccancel (void) attribute_hidden;
 extern void __pthread_disable_asynccancel (int oldtype) attribute_hidden;
 extern void __pthread_testcancel (void);
+extern int __pthread_timedjoin_ex (pthread_t, void **, const struct timespec *,
+				   bool);
 
 #if IS_IN (libpthread)
 hidden_proto (__pthread_mutex_init)
@@ -524,6 +526,7 @@ hidden_proto (__pthread_setcancelstate)
 hidden_proto (__pthread_testcancel)
 hidden_proto (__pthread_mutexattr_init)
 hidden_proto (__pthread_mutexattr_settype)
+hidden_proto (__pthread_timedjoin_ex)
 #endif
 
 extern int __pthread_cond_broadcast_2_0 (pthread_cond_2_0_t *cond);
diff --git a/nptl/pthread_join.c b/nptl/pthread_join.c
index afc8c379ca..47379a67a3 100644
--- a/nptl/pthread_join.c
+++ b/nptl/pthread_join.c
@@ -16,103 +16,11 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#include <errno.h>
-#include <stdlib.h>
-
-#include <atomic.h>
 #include "pthreadP.h"
 
-#include <stap-probe.h>
-
-
-static void
-cleanup (void *arg)
-{
-  /* If we already changed the waiter ID, reset it.  The call cannot
-     fail for any reason but the thread not having done that yet so
-     there is no reason for a loop.  */
-  (void) atomic_compare_and_exchange_bool_acq ((struct pthread **) arg, NULL,
-					       THREAD_SELF);
-}
-
-
 int
 __pthread_join (pthread_t threadid, void **thread_return)
 {
-  struct pthread *pd = (struct pthread *) threadid;
-
-  /* Make sure the descriptor is valid.  */
-  if (INVALID_NOT_TERMINATED_TD_P (pd))
-    /* Not a valid thread handle.  */
-    return ESRCH;
-
-  /* Is the thread joinable?.  */
-  if (IS_DETACHED (pd))
-    /* We cannot wait for the thread.  */
-    return EINVAL;
-
-  struct pthread *self = THREAD_SELF;
-  int result = 0;
-
-  LIBC_PROBE (pthread_join, 1, threadid);
-
-  /* During the wait we change to asynchronous cancellation.  If we
-     are canceled the thread we are waiting for must be marked as
-     un-wait-ed for again.  */
-  pthread_cleanup_push (cleanup, &pd->joinid);
-
-  /* Switch to asynchronous cancellation.  */
-  int oldtype = CANCEL_ASYNC ();
-
-  if ((pd == self
-       || (self->joinid == pd
-	   && (pd->cancelhandling
-	       & (CANCELING_BITMASK | CANCELED_BITMASK | EXITING_BITMASK
-		  | TERMINATED_BITMASK)) == 0))
-      && !CANCEL_ENABLED_AND_CANCELED (self->cancelhandling))
-    /* This is a deadlock situation.  The threads are waiting for each
-       other to finish.  Note that this is a "may" error.  To be 100%
-       sure we catch this error we would have to lock the data
-       structures but it is not necessary.  In the unlikely case that
-       two threads are really caught in this situation they will
-       deadlock.  It is the programmer's problem to figure this
-       out.  */
-    result = EDEADLK;
-  /* Wait for the thread to finish.  If it is already locked something
-     is wrong.  There can only be one waiter.  */
-  else if (__builtin_expect (atomic_compare_and_exchange_bool_acq (&pd->joinid,
-								   self,
-								   NULL), 0))
-    /* There is already somebody waiting for the thread.  */
-    result = EINVAL;
-  else
-    /* Wait for the child.  */
-    lll_wait_tid (pd->tid);
-
-
-  /* Restore cancellation mode.  */
-  CANCEL_RESET (oldtype);
-
-  /* Remove the handler.  */
-  pthread_cleanup_pop (0);
-
-
-  if (__glibc_likely (result == 0))
-    {
-      /* We mark the thread as terminated and as joined.  */
-      pd->tid = -1;
-
-      /* Store the return value if the caller is interested.  */
-      if (thread_return != NULL)
-	*thread_return = pd->result;
-
-
-      /* Free the TCB.  */
-      __free_tcb (pd);
-    }
-
-  LIBC_PROBE (pthread_join_ret, 3, threadid, result, pd->result);
-
-  return result;
+  return __pthread_timedjoin_ex (threadid, thread_return, NULL, true);
 }
 weak_alias (__pthread_join, pthread_join)
diff --git a/nptl/pthread_join_common.c b/nptl/pthread_join_common.c
new file mode 100644
index 0000000000..59c54457b6
--- /dev/null
+++ b/nptl/pthread_join_common.c
@@ -0,0 +1,115 @@
+/* Common definition for pthread_{timed,try}join{_np}.
+   Copyright (C) 2017 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include "pthreadP.h"
+#include <atomic.h>
+#include <stap-probe.h>
+
+static void
+cleanup (void *arg)
+{
+  /* If we already changed the waiter ID, reset it.  The call cannot
+     fail for any reason but the thread not having done that yet so
+     there is no reason for a loop.  */
+  struct pthread *self = THREAD_SELF;
+  atomic_compare_exchange_weak_acquire (&arg, &self, NULL);
+}
+
+int
+__pthread_timedjoin_ex (pthread_t threadid, void **thread_return,
+			const struct timespec *abstime, bool block)
+{
+  struct pthread *pd = (struct pthread *) threadid;
+
+  /* Make sure the descriptor is valid.  */
+  if (INVALID_NOT_TERMINATED_TD_P (pd))
+    /* Not a valid thread handle.  */
+    return ESRCH;
+
+  /* Is the thread joinable?.  */
+  if (IS_DETACHED (pd))
+    /* We cannot wait for the thread.  */
+    return EINVAL;
+
+  struct pthread *self = THREAD_SELF;
+  int result = 0;
+
+  LIBC_PROBE (pthread_join, 1, threadid);
+
+  if ((pd == self
+       || (self->joinid == pd
+	   && (pd->cancelhandling
+	       & (CANCELING_BITMASK | CANCELED_BITMASK | EXITING_BITMASK
+		  | TERMINATED_BITMASK)) == 0))
+      && !CANCEL_ENABLED_AND_CANCELED (self->cancelhandling))
+    /* This is a deadlock situation.  The threads are waiting for each
+       other to finish.  Note that this is a "may" error.  To be 100%
+       sure we catch this error we would have to lock the data
+       structures but it is not necessary.  In the unlikely case that
+       two threads are really caught in this situation they will
+       deadlock.  It is the programmer's problem to figure this
+       out.  */
+    return EDEADLK;
+
+  /* Wait for the thread to finish.  If it is already locked something
+     is wrong.  There can only be one waiter.  */
+  else if (__glibc_unlikely (atomic_compare_exchange_weak_acquire (&pd->joinid,
+								   &self,
+								   NULL)))
+    /* There is already somebody waiting for the thread.  */
+    return EINVAL;
+
+  if (block)
+    {
+      /* During the wait we change to asynchronous cancellation.  If we
+	 are cancelled the thread we are waiting for must be marked as
+	 un-wait-ed for again.  */
+      pthread_cleanup_push (cleanup, &pd->joinid);
+
+      int oldtype = CANCEL_ASYNC ();
+
+      if (abstime != NULL)
+	result = lll_timedwait_tid (pd->tid, abstime);
+      else
+	lll_wait_tid (pd->tid);
+
+      CANCEL_RESET (oldtype);
+
+      pthread_cleanup_pop (0);
+    }
+
+  if (__glibc_likely (result == 0))
+    {
+      /* We mark the thread as terminated and as joined.  */
+      pd->tid = -1;
+
+      /* Store the return value if the caller is interested.  */
+      if (thread_return != NULL)
+	*thread_return = pd->result;
+
+      /* Free the TCB.  */
+      __free_tcb (pd);
+    }
+  else
+    pd->joinid = NULL;
+
+  LIBC_PROBE (pthread_join_ret, 3, threadid, result, pd->result);
+
+  return result;
+}
+hidden_def (__pthread_timedjoin_ex)
diff --git a/nptl/pthread_timedjoin.c b/nptl/pthread_timedjoin.c
index 567c171133..0f74a98c20 100644
--- a/nptl/pthread_timedjoin.c
+++ b/nptl/pthread_timedjoin.c
@@ -16,89 +16,12 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#include <errno.h>
-#include <stdlib.h>
-#include <atomic.h>
 #include "pthreadP.h"
 
-
-static void
-cleanup (void *arg)
-{
-  *(void **) arg = NULL;
-}
-
-
 int
-pthread_timedjoin_np (pthread_t threadid, void **thread_return,
-		      const struct timespec *abstime)
+__pthread_timedjoin_np (pthread_t threadid, void **thread_return,
+			const struct timespec *abstime)
 {
-  struct pthread *self;
-  struct pthread *pd = (struct pthread *) threadid;
-  int result;
-
-  /* Make sure the descriptor is valid.  */
-  if (INVALID_NOT_TERMINATED_TD_P (pd))
-    /* Not a valid thread handle.  */
-    return ESRCH;
-
-  /* Is the thread joinable?.  */
-  if (IS_DETACHED (pd))
-    /* We cannot wait for the thread.  */
-    return EINVAL;
-
-  self = THREAD_SELF;
-  if (pd == self || self->joinid == pd)
-    /* This is a deadlock situation.  The threads are waiting for each
-       other to finish.  Note that this is a "may" error.  To be 100%
-       sure we catch this error we would have to lock the data
-       structures but it is not necessary.  In the unlikely case that
-       two threads are really caught in this situation they will
-       deadlock.  It is the programmer's problem to figure this
-       out.  */
-    return EDEADLK;
-
-  /* Wait for the thread to finish.  If it is already locked something
-     is wrong.  There can only be one waiter.  */
-  if (__builtin_expect (atomic_compare_and_exchange_bool_acq (&pd->joinid,
-							      self, NULL), 0))
-    /* There is already somebody waiting for the thread.  */
-    return EINVAL;
-
-
-  /* During the wait we change to asynchronous cancellation.  If we
-     are cancelled the thread we are waiting for must be marked as
-     un-wait-ed for again.  */
-  pthread_cleanup_push (cleanup, &pd->joinid);
-
-  /* Switch to asynchronous cancellation.  */
-  int oldtype = CANCEL_ASYNC ();
-
-
-  /* Wait for the child.  */
-  result = lll_timedwait_tid (pd->tid, abstime);
-
-
-  /* Restore cancellation mode.  */
-  CANCEL_RESET (oldtype);
-
-  /* Remove the handler.  */
-  pthread_cleanup_pop (0);
-
-
-  /* We might have timed out.  */
-  if (result == 0)
-    {
-      /* Store the return value if the caller is interested.  */
-      if (thread_return != NULL)
-	*thread_return = pd->result;
-
-
-      /* Free the TCB.  */
-      __free_tcb (pd);
-    }
-  else
-    pd->joinid = NULL;
-
-  return result;
+  return __pthread_timedjoin_ex (threadid, thread_return, abstime, true);
 }
+weak_alias (__pthread_timedjoin_np, pthread_timedjoin_np)
diff --git a/nptl/pthread_tryjoin.c b/nptl/pthread_tryjoin.c
index 6a3b62e409..b34df54970 100644
--- a/nptl/pthread_tryjoin.c
+++ b/nptl/pthread_tryjoin.c
@@ -16,57 +16,17 @@
    License along with the GNU C Library; if not, see
    <http://www.gnu.org/licenses/>.  */
 
-#include <errno.h>
-#include <stdlib.h>
-
-#include <atomic.h>
 #include "pthreadP.h"
 
-
 int
 pthread_tryjoin_np (pthread_t threadid, void **thread_return)
 {
-  struct pthread *self;
-  struct pthread *pd = (struct pthread *) threadid;
-
-  /* Make sure the descriptor is valid.  */
-  if (DEBUGGING_P && __find_in_stack_list (pd) == NULL)
-    /* Not a valid thread handle.  */
-    return ESRCH;
-
-  /* Is the thread joinable?.  */
-  if (IS_DETACHED (pd))
-    /* We cannot wait for the thread.  */
-    return EINVAL;
-
-  self = THREAD_SELF;
-  if (pd == self || self->joinid == pd)
-    /* This is a deadlock situation.  The threads are waiting for each
-       other to finish.  Note that this is a "may" error.  To be 100%
-       sure we catch this error we would have to lock the data
-       structures but it is not necessary.  In the unlikely case that
-       two threads are really caught in this situation they will
-       deadlock.  It is the programmer's problem to figure this
-       out.  */
-    return EDEADLK;
-
   /* Return right away if the thread hasn't terminated yet.  */
+  struct pthread *pd = (struct pthread *) threadid;
   if (pd->tid != 0)
     return EBUSY;
 
-  /* Wait for the thread to finish.  If it is already locked something
-     is wrong.  There can only be one waiter.  */
-  if (atomic_compare_and_exchange_bool_acq (&pd->joinid, self, NULL))
-    /* There is already somebody waiting for the thread.  */
-    return EINVAL;
-
-  /* Store the return value if the caller is interested.  */
-  if (thread_return != NULL)
-    *thread_return = pd->result;
-
-
-  /* Free the TCB.  */
-  __free_tcb (pd);
-
-  return 0;
+  /* If pd->tid != 0 then lll_wait_tid will not block on futex
+     operation.  */
+  return __pthread_timedjoin_ex (threadid, thread_return, NULL, false);
 }
diff --git a/sysdeps/unix/sysv/linux/i386/lowlevellock.h b/sysdeps/unix/sysv/linux/i386/lowlevellock.h
index 197bb1fcd9..098c64bf96 100644
--- a/sysdeps/unix/sysv/linux/i386/lowlevellock.h
+++ b/sysdeps/unix/sysv/linux/i386/lowlevellock.h
@@ -237,12 +237,7 @@ extern int __lll_timedwait_tid (int *tid, const struct timespec *abstime)
   ({									      \
     int __result = 0;							      \
     if ((tid) != 0)							      \
-      {									      \
-	if ((abstime)->tv_nsec < 0 || (abstime)->tv_nsec >= 1000000000)	      \
-	  __result = EINVAL;						      \
-	else								      \
-	  __result = __lll_timedwait_tid (&(tid), (abstime));		      \
-      }									      \
+      __result = __lll_timedwait_tid (&(tid), (abstime));		      \
     __result; })
 
 
diff --git a/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h b/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
index cbf6597e00..0b56a2eb1c 100644
--- a/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
+++ b/sysdeps/unix/sysv/linux/x86_64/lowlevellock.h
@@ -249,12 +249,7 @@ extern int __lll_timedwait_tid (int *, const struct timespec *)
   ({									      \
     int __result = 0;							      \
     if ((tid) != 0)							      \
-      {									      \
-	if ((abstime)->tv_nsec < 0 || (abstime)->tv_nsec >= 1000000000)	      \
-	  __result = EINVAL;						      \
-	else								      \
-	  __result = __lll_timedwait_tid (&(tid), (abstime));		      \
-      }									      \
+      __result = __lll_timedwait_tid (&(tid), (abstime));		      \
     __result; })
 
 extern int __lll_lock_elision (int *futex, short *adapt_count, int private)