nptl: Replace __futex_clocklock_wait64 with __futex_abstimed_wait64

For non null timeouts, the __futex_clocklock_wait64 creates an a
relative timeout by subtracting the current time from the input
argument.  The same behavior can be obtained with FUTEX_WAIT_BITSET
without the need to calculate the relative timeout.  Besides consolidate
the code it also avoid the possible relative timeout issues [1].

The __futex_abstimed_wait64 needs also to return EINVAL syscall
errors.

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

[1] https://sourceware.org/pipermail/libc-alpha/2020-November/119881.html

Reviewed-by: Lukasz Majewski <lukma@denx.de>

diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
index 0ec47359be274f81acd67a37415002eb54c5d01e..e643eab258414eff2db7b74a2babc83d5ec9ede4 100644 (file)
--- a/nptl/pthread_mutex_timedlock.c
+++ b/nptl/pthread_mutex_timedlock.c
@@ -391,8 +391,8 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
                /* Delay the thread until the timeout is reached. Then return
                   ETIMEDOUT.  */
                do
-                 e = __futex_clocklock_wait64 (&(int){0}, 0, clockid, abstime,
-                                               private);
+                 e = __futex_abstimed_wait64 (&(unsigned int){0}, 0, clockid,
+                                              abstime, private);
                while (e != ETIMEDOUT);
                return ETIMEDOUT;
              }

diff --git a/sysdeps/nptl/futex-internal.c b/sysdeps/nptl/futex-internal.c
index 30c662547f52bfdc776e985e87be2a3b0e3c2299..11031cc46a91a1c655a0b046b98cfe9e74ccc1e9 100644 (file)
--- a/sysdeps/nptl/futex-internal.c
+++ b/sysdeps/nptl/futex-internal.c
@@ -90,15 +90,13 @@ __futex_abstimed_wait_common64 (unsigned int* futex_word,
     case -EAGAIN:
     case -EINTR:
     case -ETIMEDOUT:
+    case -EINVAL:
     case -EOVERFLOW:  /* Passed absolute timeout uses 64 bit time_t type, but
                          underlying kernel does not support 64 bit time_t futex
                          syscalls.  */
       return -err;
 
     case -EFAULT: /* Must have been caused by a glibc or application bug.  */
-    case -EINVAL: /* Either due to wrong alignment or due to the timeout not
-                    being normalized.  Must have been caused by a glibc or
-                    application bug.  */
     case -ENOSYS: /* Must have been caused by a glibc bug.  */
     /* No other errors are documented at this time.  */
     default:
@@ -124,56 +122,3 @@ __futex_abstimed_wait_cancelable64 (unsigned int* futex_word,
   return __futex_abstimed_wait_common64 (futex_word, expected, clockid,
                                          abstime, private, true);
 }
-
-int
-__futex_clocklock_wait64 (int *futex, int val, clockid_t clockid,
-                          const struct __timespec64 *abstime, int private)
-{
-  struct __timespec64 ts, *tsp = NULL;
-
-  if (abstime != NULL)
-    {
-      /* Reject invalid timeouts.  */
-      if (! valid_nanoseconds (abstime->tv_nsec))
-        return EINVAL;
-
-      /* Get the current time. This can only fail if clockid is not valid.  */
-      if (__glibc_unlikely (__clock_gettime64 (clockid, &ts) != 0))
-        return EINVAL;
-
-      /* Compute relative timeout.  */
-      ts.tv_sec = abstime->tv_sec - ts.tv_sec;
-      ts.tv_nsec = abstime->tv_nsec - ts.tv_nsec;
-      if (ts.tv_nsec < 0)
-        {
-         ts.tv_nsec += 1000000000;
-         --ts.tv_sec;
-        }
-
-      if (ts.tv_sec < 0)
-        return ETIMEDOUT;
-
-      tsp = &ts;
-    }
-
-  int err = INTERNAL_SYSCALL_CALL (futex_time64, futex,
-                                   __lll_private_flag (FUTEX_WAIT, private),
-                                   val, tsp);
-#ifndef __ASSUME_TIME64_SYSCALLS
-  if (err == -ENOSYS)
-    {
-      if (tsp != NULL && ! in_time_t_range (tsp->tv_sec))
-        return EOVERFLOW;
-
-      struct timespec ts32;
-      if (tsp != NULL)
-        ts32 = valid_timespec64_to_timespec (*tsp);
-
-      err = INTERNAL_SYSCALL_CALL (futex, futex,
-                                   __lll_private_flag (FUTEX_WAIT, private),
-                                   val, tsp != NULL ? &ts32 : NULL);
-    }
-#endif
-
-  return -err;
-}

diff --git a/sysdeps/nptl/futex-internal.h b/sysdeps/nptl/futex-internal.h
index 91440637f170cf6f8a7937961a008c3837cbe8c9..e67803952f5892ab72f5d87f9b0073de5d81cb46 100644 (file)
--- a/sysdeps/nptl/futex-internal.h
+++ b/sysdeps/nptl/futex-internal.h
@@ -413,10 +413,6 @@ __futex_abstimed_wait64 (unsigned int* futex_word, unsigned int expected,
                          const struct __timespec64* abstime,
                          int private) attribute_hidden;
 
-int
-__futex_clocklock_wait64 (int *futex, int val, clockid_t clockid,
-                          const struct __timespec64 *abstime,
-                          int private) attribute_hidden;
 
 static __always_inline int
 __futex_clocklock64 (int *futex, clockid_t clockid,
@@ -428,7 +424,8 @@ __futex_clocklock64 (int *futex, clockid_t clockid,
     {
       while (atomic_exchange_acq (futex, 2) != 0)
         {
-          err = __futex_clocklock_wait64 (futex, 2, clockid, abstime, private);
+          err = __futex_abstimed_wait64 ((unsigned int *) futex, 2, clockid,
+                                        abstime, private);
           if (err == EINVAL || err == ETIMEDOUT || err == EOVERFLOW)
             break;
         }