nptl: Reinstate pthread_timedjoin_np as a cancellation point (BZ#24215)

Patch ce7eb0e90315 ("nptl: Cleanup cancellation macros") changed the
join sequence for internal common __pthread_timedjoin_ex to use the
new macro lll_wait_tid.  The idea was this macro would issue the
cancellable futex operation depending whether the timeout is used or
not.  However if a timeout is used, __lll_timedwait_tid is called and
it is not a cancellable entrypoint.

This patch fixes it by simplifying the code in various ways:

  - Instead of adding the cancellation handling on __lll_timedwait_tid,
    it moves the generic implementation to pthread_join_common.c (called
    now timedwait_tid with some fixes to use the correct type for pid).

  - The llvm_wait_tid macro is removed, along with its replication on
    x86_64, i686, and sparc arch-specific lowlevellock.h.

  - sparc32 __lll_timedwait_tid is also removed, since the code is similar
    to generic one.

  - x86_64 and i386 provides arch-specific __lll_timedwait_tid which is
    also removed since they are similar in functionality to generic C code
    and there is no indication it is better than compiler generated code.

New tests, tst-join8 and tst-join9, are provided to check if
pthread_timedjoin_np acts as a cancellation point.

Checked on x86_64-linux-gnu, i686-linux-gnu, sparcv9-linux-gnu, and
aarch64-linux-gnu.

	[BZ #24215]
	* nptl/Makefile (lpthread-routines): Remove lll_timedwait_tid.
	(tests): Add tst-join8 tst-join9.
	* nptl/lll_timedwait_tid.c: Remove file.
	* sysdeps/sparc/sparc32/lll_timedwait_tid.c: Likewise.
	* sysdeps/unix/sysv/linux/i386/lll_timedwait_tid.c: Likewise.
	* sysdeps/sysv/linux/x86_64/lll_timedwait_tid.c: Likewise.
	* nptl/pthread_join_common.c (timedwait_tid): New function.
	(__pthread_timedjoin_ex): Act as cancellation entrypoint is block
	is set.
	* nptl/tst-join5.c (thread_join): New function.
	(tf1, tf2, do_test): Use libsupport and add pthread_timedjoin_np
	check.
	* nptl/tst-join8.c: New file.
	* nptl/tst-join9.c: Likewise.
	* sysdeps/nptl/lowlevellock-futex.h (lll_futex_wait_cancel,
	lll_futex_timed_wait_cancel): Add generic macros.
	* sysdeps/nptl/lowlevellock.h (__lll_timedwait_tid, lll_wait_tid):
	Remove definitions.
	* sysdeps/unix/sysv/linux/i386/lowlevellock.h: Likewise.
	* sysdeps/unix/sysv/linux/sparc/lowlevellock.h: Likewise.
	* sysdeps/unix/sysv/linux/x86_64/lowlevellock.h: Likewise.
	* sysdeps/sparc/sparc32/lowlevellock.c (__lll_timedwait_tid):
	Remove function.
	* sysdeps/unix/sysv/linux/i386/lowlevellock.S (__lll_timedwait_tid):
	Likewise.
	* sysdeps/unix/sysv/linux/x86_64/lowlevellock.S: Likewise.
	* sysdeps/unix/sysv/linux/lowlevellock-futex.h
	(lll_futex_timed_wait_cancel): New macro.

6 files changed, 137 insertions, 130 deletions

diff --git a/nptl/Makefile b/nptl/Makefile
index 8719f4e..5acfdcc 100644
--- a/nptl/Makefile
+++ b/nptl/Makefile
@@ -120,7 +120,7 @@ libpthread-routines = nptl-init nptlfreeres vars events version pt-interp \
 		      pt-longjmp pt-cleanup\
 		      cancellation \
 		      lowlevellock \
-		      lll_timedlock_wait lll_timedwait_tid \
+		      lll_timedlock_wait \
 		      pt-fork pt-vfork pt-fcntl \
 		      $(pthread-compat-wrappers) \
 		      pt-raise pt-system \
@@ -271,6 +271,7 @@ tests = tst-attr1 tst-attr2 tst-attr3 tst-default-attr \
 	tst-kill1 tst-kill2 tst-kill3 tst-kill4 tst-kill5 tst-kill6 \
 	tst-raise1 \
 	tst-join1 tst-join2 tst-join3 tst-join4 tst-join5 tst-join6 tst-join7 \
+	tst-join8 tst-join9 \
 	tst-detach1 \
 	tst-eintr1 tst-eintr2 tst-eintr3 tst-eintr4 tst-eintr5 \
 	tst-tsd1 tst-tsd2 tst-tsd3 tst-tsd4 tst-tsd5 tst-tsd6 \
diff --git a/nptl/lll_timedwait_tid.c b/nptl/lll_timedwait_tid.c
deleted file mode 100644
index 59ecbb4..0000000
--- a/nptl/lll_timedwait_tid.c
+++ /dev/null
@@ -1,70 +0,0 @@
-/* Timed waiting for thread death.  Generic futex-using version.
-   Copyright (C) 2003-2019 Free Software Foundation, Inc.
-   This file is part of the GNU C Library.
-   Contributed by Paul Mackerras <paulus@au.ibm.com>, 2003.
-
-   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 <atomic.h>
-#include <errno.h>
-#include <lowlevellock.h>
-#include <sys/time.h>
-
-
-/* The kernel notifies a process which uses CLONE_CHILD_CLEARTID via futex
-   wake-up when the clone terminates.  The memory location contains the
-   thread ID while the clone is running and is reset to zero by the kernel
-   afterwards.  The kernel up to version 3.16.3 does not use the private futex
-   operations for futex wake-up when the clone terminates.  */
-int
-__lll_timedwait_tid (int *tidp, const struct timespec *abstime)
-{
-  int tid;
-
-  if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000)
-    return EINVAL;
-
-  /* Repeat until thread terminated.  */
-  while ((tid = *tidp) != 0)
-    {
-      struct timeval tv;
-      struct timespec rt;
-
-      /* Get the current time.  */
-      (void) __gettimeofday (&tv, NULL);
-
-      /* Compute relative timeout.  */
-      rt.tv_sec = abstime->tv_sec - tv.tv_sec;
-      rt.tv_nsec = abstime->tv_nsec - tv.tv_usec * 1000;
-      if (rt.tv_nsec < 0)
-        {
-          rt.tv_nsec += 1000000000;
-          --rt.tv_sec;
-        }
-
-      /* Already timed out?  */
-      if (rt.tv_sec < 0)
-        return ETIMEDOUT;
-
-      /* If *tidp == tid, wait until thread terminates or the wait times out.
-         The kernel up to version 3.16.3 does not use the private futex
-         operations for futex wake-up when the clone terminates.
-      */
-      if (lll_futex_timed_wait (tidp, tid, &rt, LLL_SHARED) == -ETIMEDOUT)
-        return ETIMEDOUT;
-    }
-
-  return 0;
-}
diff --git a/nptl/pthread_join_common.c b/nptl/pthread_join_common.c
index ecb78ff..6efe8ef 100644
--- a/nptl/pthread_join_common.c
+++ b/nptl/pthread_join_common.c
@@ -30,6 +30,52 @@ cleanup (void *arg)
   atomic_compare_exchange_weak_acquire (&arg, &self, NULL);
 }
 
+/* The kernel notifies a process which uses CLONE_CHILD_CLEARTID via futex
+   wake-up when the clone terminates.  The memory location contains the
+   thread ID while the clone is running and is reset to zero by the kernel
+   afterwards.  The kernel up to version 3.16.3 does not use the private futex
+   operations for futex wake-up when the clone terminates.  */
+static int
+timedwait_tid (pid_t *tidp, const struct timespec *abstime)
+{
+  pid_t tid;
+
+  if (abstime->tv_nsec < 0 || abstime->tv_nsec >= 1000000000)
+    return EINVAL;
+
+  /* Repeat until thread terminated.  */
+  while ((tid = *tidp) != 0)
+    {
+      struct timeval tv;
+      struct timespec rt;
+
+      /* Get the current time.  */
+      __gettimeofday (&tv, NULL);
+
+      /* Compute relative timeout.  */
+      rt.tv_sec = abstime->tv_sec - tv.tv_sec;
+      rt.tv_nsec = abstime->tv_nsec - tv.tv_usec * 1000;
+      if (rt.tv_nsec < 0)
+        {
+          rt.tv_nsec += 1000000000;
+          --rt.tv_sec;
+        }
+
+      /* Already timed out?  */
+      if (rt.tv_sec < 0)
+        return ETIMEDOUT;
+
+      /* If *tidp == tid, wait until thread terminates or the wait times out.
+         The kernel up to version 3.16.3 does not use the private futex
+         operations for futex wake-up when the clone terminates.  */
+      if (lll_futex_timed_wait_cancel (tidp, tid, &rt, LLL_SHARED)
+	  == -ETIMEDOUT)
+        return ETIMEDOUT;
+    }
+
+  return 0;
+}
+
 int
 __pthread_timedjoin_ex (pthread_t threadid, void **thread_return,
 			const struct timespec *abstime, bool block)
@@ -74,6 +120,10 @@ __pthread_timedjoin_ex (pthread_t threadid, void **thread_return,
     /* There is already somebody waiting for the thread.  */
     return EINVAL;
 
+  /* BLOCK waits either indefinitely or based on an absolute time.  POSIX also
+     states a cancellation point shall occur for pthread_join, and we use the
+     same rationale for posix_timedjoin_np.  Both timedwait_tid and the futex
+     call use the cancellable variant.  */
   if (block)
     {
       /* During the wait we change to asynchronous cancellation.  If we
@@ -81,7 +131,16 @@ __pthread_timedjoin_ex (pthread_t threadid, void **thread_return,
 	 un-wait-ed for again.  */
       pthread_cleanup_push (cleanup, &pd->joinid);
 
-      result = lll_wait_tid (pd->tid, abstime);
+      if (abstime != NULL)
+	result = timedwait_tid (&pd->tid, abstime);
+      else
+	{
+	  pid_t tid;
+	  /* We need acquire MO here so that we synchronize with the
+	     kernel's store to 0 when the clone terminates. (see above)  */
+	  while ((tid = atomic_load_acquire (&pd->tid)) != 0)
+	    lll_futex_wait_cancel (&pd->tid, tid, LLL_SHARED);
+	}
 
       pthread_cleanup_pop (0);
     }
diff --git a/nptl/tst-join5.c b/nptl/tst-join5.c
index f60a024..39f74dc 100644
--- a/nptl/tst-join5.c
+++ b/nptl/tst-join5.c
@@ -23,6 +23,8 @@
 #include <time.h>
 #include <unistd.h>
 
+#include <support/check.h>
+#include <support/xthread.h>
 
 static void
 wait_code (void)
@@ -37,22 +39,31 @@ wait_code (void)
 static pthread_barrier_t b;
 #endif
 
+static int
+thread_join (pthread_t thread, void **retval)
+{
+#ifdef USE_PTHREAD_TIMEDJOIN_NP
+  struct timespec tv;
+  TEST_COMPARE (clock_gettime (CLOCK_REALTIME, &tv), 0);
+  /* Arbitrary large timeout to make it act as pthread_join.  */
+  tv.tv_sec += 1000;
+  return pthread_timedjoin_np ((pthread_t) thread, retval, &tv);
+#else
+  return pthread_join ((pthread_t) thread, retval);
+#endif
+}
+
 
 static void *
 tf1 (void *arg)
 {
 #ifdef WAIT_IN_CHILD
-  int e = pthread_barrier_wait (&b);
-  if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
-    {
-      printf ("%s: barrier_wait failed\n", __func__);
-      exit (1);
-    }
+  xpthread_barrier_wait (&b);
 
   wait_code ();
 #endif
 
-  pthread_join ((pthread_t) arg, NULL);
+  thread_join ((pthread_t) arg, NULL);
 
   exit (42);
 }
@@ -62,16 +73,12 @@ static void *
 tf2 (void *arg)
 {
 #ifdef WAIT_IN_CHILD
-  int e = pthread_barrier_wait (&b);
-  if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
-    {
-      printf ("%s: barrier_wait failed\n", __func__);
-      exit (1);
-    }
+  xpthread_barrier_wait (&b);
 
   wait_code ();
 #endif
-  pthread_join ((pthread_t) arg, NULL);
+
+  thread_join ((pthread_t) arg, NULL);
 
   exit (43);
 }
@@ -81,16 +88,12 @@ static int
 do_test (void)
 {
 #ifdef WAIT_IN_CHILD
-  if (pthread_barrier_init (&b, NULL, 2) != 0)
-    {
-      puts ("barrier_init failed");
-      return 1;
-    }
+  xpthread_barrier_init (&b, NULL, 2);
 #endif
 
   pthread_t th;
 
-  int err = pthread_join (pthread_self (), NULL);
+  int err = thread_join (pthread_self (), NULL);
   if (err == 0)
     {
       puts ("1st circular join succeeded");
@@ -102,33 +105,20 @@ do_test (void)
       return 1;
     }
 
-  if (pthread_create (&th, NULL, tf1, (void *) pthread_self ()) != 0)
-    {
-      puts ("1st create failed");
-      return 1;
-    }
+  th = xpthread_create (NULL, tf1, (void *) pthread_self ());
 
 #ifndef WAIT_IN_CHILD
   wait_code ();
 #endif
 
-  if (pthread_cancel (th) != 0)
-    {
-      puts ("cannot cancel 1st thread");
-      return 1;
-    }
+  xpthread_cancel (th);
 
 #ifdef WAIT_IN_CHILD
-  int e = pthread_barrier_wait (&b);
-  if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
-    {
-      printf ("%s: barrier_wait failed\n", __func__);
-      return 1;
-    }
+  xpthread_barrier_wait (&b);
 #endif
 
   void *r;
-  err = pthread_join (th, &r);
+  err = thread_join (th, &r);
   if (err != 0)
     {
       printf ("cannot join 1st thread: %d\n", err);
@@ -140,7 +130,7 @@ do_test (void)
       return 1;
     }
 
-  err = pthread_join (pthread_self (), NULL);
+  err = thread_join (pthread_self (), NULL);
   if (err == 0)
     {
       puts ("2nd circular join succeeded");
@@ -152,32 +142,19 @@ do_test (void)
       return 1;
     }
 
-  if (pthread_create (&th, NULL, tf2, (void *) pthread_self ()) != 0)
-    {
-      puts ("2nd create failed");
-      return 1;
-    }
+  th = xpthread_create (NULL, tf2, (void *) pthread_self ());
 
 #ifndef WAIT_IN_CHILD
   wait_code ();
 #endif
 
-  if (pthread_cancel (th) != 0)
-    {
-      puts ("cannot cancel 2nd thread");
-      return 1;
-    }
+  xpthread_cancel (th);
 
 #ifdef WAIT_IN_CHILD
-  e = pthread_barrier_wait (&b);
-  if (e != 0 && e != PTHREAD_BARRIER_SERIAL_THREAD)
-    {
-      printf ("%s: barrier_wait failed\n", __func__);
-      return 1;
-    }
+  xpthread_barrier_wait (&b);
 #endif
 
-  if (pthread_join (th, &r) != 0)
+  if (thread_join (th, &r) != 0)
     {
       puts ("cannot join 2nd thread");
       return 1;
@@ -188,7 +165,7 @@ do_test (void)
       return 1;
     }
 
-  err = pthread_join (pthread_self (), NULL);
+  err = thread_join (pthread_self (), NULL);
   if (err == 0)
     {
       puts ("3rd circular join succeeded");
@@ -203,5 +180,4 @@ do_test (void)
   return 0;
 }
 
-#define TEST_FUNCTION do_test ()
-#include "../test-skeleton.c"
+#include <support/test-driver.c>
diff --git a/nptl/tst-join8.c b/nptl/tst-join8.c
new file mode 100644
index 0000000..f3241fa
--- /dev/null
+++ b/nptl/tst-join8.c
@@ -0,0 +1,20 @@
+/* Check if pthread_timedjoin_np is a cancellation entrypoint.
+   Copyright (C) 2019 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/>.  */
+
+#define USE_PTHREAD_TIMEDJOIN_NP 1
+#include <nptl/tst-join5.c>
diff --git a/nptl/tst-join9.c b/nptl/tst-join9.c
new file mode 100644
index 0000000..7a47f58
--- /dev/null
+++ b/nptl/tst-join9.c
@@ -0,0 +1,21 @@
+/* Check if pthread_timedjoin_np is a cancellation entrypoint.
+   Copyright (C) 2019 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/>.  */
+
+#define USE_PTHREAD_TIMEDJOIN_NP 1
+#define WAIT_IN_CHILD 1
+#include <nptl/tst-join5.c>