[v3] y2038: nptl: Convert pthread_cond_{clock|timed}wait to support 64 bit time

  The pthread_cond_clockwait and pthread_cond_timedwait have been converted
to support 64 bit time.

This change introduces new futex_abstimed_wait_cancelable64 function in
./sysdeps/nptl/futex-helpers.c, which uses futex_time64 where possible
and tries to replace low-level preprocessor macros from
lowlevellock-futex.h
The pthread_cond_{clock|timed}wait only accepts absolute time. Moreover,
there is no need to check for NULL passed as *abstime pointer as
__pthread_cond_wait_common() always passes non-NULL struct __timespec64
pointer to futex_abstimed_wait_cancellable64().

For systems with __TIMESIZE != 64 && __WORDSIZE == 32:
- Conversions between 64 bit time to 32 bit are necessary
- Redirection to __pthread_cond_{clock|timed}wait64 will provide support
  for 64 bit time

The futex_abstimed_wait_cancelable64 function has been put into a separate
file on the purpose - to avoid issues apparent on the m68k architecture
related to small number of available registers (there is not enough
registers to put all necessary arguments in them if the above function
would be added to futex-internal.h with __always_inline attribute).

In fact - new function - namely __futex_abstimed_wait_cancellable32 is
used to reduce number of needed registers (as some in-register values are
stored on the stack when function call is made).

Build tests:
./src/scripts/build-many-glibcs.py glibcs

Run-time tests:
- Run specific tests on ARM/x86 32bit systems (qemu):
  https://github.com/lmajewski/meta-y2038 and run tests:
  https://github.com/lmajewski/y2038-tests/commits/master

Above tests were performed with Y2038 redirection applied as well as without
to test the proper usage of both __pthread_cond_{clock|timed}wait64 and
__pthread_cond_{clock|timed}wait.

---
Changes for v3:

- Change the file name from futex-helpers.c to futex-internal.c
- Use INTERNAL_SYSCALL_CANCEL() instead of INTERNAL_SYSCALL_CALL and remove not
  necessary __pthread_{disable|enable}_asynccancel ()
- Remove not needed blank lines
- Do not change code formatting not related to this patch
- Add attribute_hidden instead of libpthread_hidden_def
- Rewrite the __futex_abstimed_wait_cancelable64 to have a separate function
  call for 32 bit time syscall fallback
- Remove sysdeps/unix/sysv/linux/m68k/futex-helpers.c as it is not needed
  anymore
---
 nptl/pthreadP.h               | 11 ++++
 nptl/pthread_cond_wait.c      | 43 +++++++++++++---
 sysdeps/nptl/Makefile         |  2 +-
 sysdeps/nptl/futex-internal.c | 96 +++++++++++++++++++++++++++++++++++
 sysdeps/nptl/futex-internal.h | 11 ++++
 5 files changed, 154 insertions(+), 9 deletions(-)
 create mode 100644 sysdeps/nptl/futex-internal.c
  

On 30/08/2020 19:44, Lukasz Majewski wrote:
> The pthread_cond_clockwait and pthread_cond_timedwait have been converted
> to support 64 bit time.
> 
> This change introduces new futex_abstimed_wait_cancelable64 function in
> ./sysdeps/nptl/futex-helpers.c, which uses futex_time64 where possible
> and tries to replace low-level preprocessor macros from
> lowlevellock-futex.h
> The pthread_cond_{clock|timed}wait only accepts absolute time. Moreover,
> there is no need to check for NULL passed as *abstime pointer as
> __pthread_cond_wait_common() always passes non-NULL struct __timespec64
> pointer to futex_abstimed_wait_cancellable64().
> 
> For systems with __TIMESIZE != 64 && __WORDSIZE == 32:
> - Conversions between 64 bit time to 32 bit are necessary
> - Redirection to __pthread_cond_{clock|timed}wait64 will provide support
>   for 64 bit time
> 
> The futex_abstimed_wait_cancelable64 function has been put into a separate
> file on the purpose - to avoid issues apparent on the m68k architecture
> related to small number of available registers (there is not enough
> registers to put all necessary arguments in them if the above function
> would be added to futex-internal.h with __always_inline attribute).
> 
> In fact - new function - namely __futex_abstimed_wait_cancellable32 is
> used to reduce number of needed registers (as some in-register values are
> stored on the stack when function call is made).
> 
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs
> 
> Run-time tests:
> - Run specific tests on ARM/x86 32bit systems (qemu):
>   https://github.com/lmajewski/meta-y2038 and run tests:
>   https://github.com/lmajewski/y2038-tests/commits/master
> 
> Above tests were performed with Y2038 redirection applied as well as without
> to test the proper usage of both __pthread_cond_{clock|timed}wait64 and
> __pthread_cond_{clock|timed}wait.

LGTM with attribute_hidden fixes below.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> 
> ---
> Changes for v3:
> 
> - Change the file name from futex-helpers.c to futex-internal.c
> - Use INTERNAL_SYSCALL_CANCEL() instead of INTERNAL_SYSCALL_CALL and remove not
>   necessary __pthread_{disable|enable}_asynccancel ()
> - Remove not needed blank lines
> - Do not change code formatting not related to this patch
> - Add attribute_hidden instead of libpthread_hidden_def
> - Rewrite the __futex_abstimed_wait_cancelable64 to have a separate function
>   call for 32 bit time syscall fallback
> - Remove sysdeps/unix/sysv/linux/m68k/futex-helpers.c as it is not needed
>   anymore
> ---
>  nptl/pthreadP.h               | 11 ++++
>  nptl/pthread_cond_wait.c      | 43 +++++++++++++---
>  sysdeps/nptl/Makefile         |  2 +-
>  sysdeps/nptl/futex-internal.c | 96 +++++++++++++++++++++++++++++++++++
>  sysdeps/nptl/futex-internal.h | 11 ++++
>  5 files changed, 154 insertions(+), 9 deletions(-)
>  create mode 100644 sysdeps/nptl/futex-internal.c
> 
> diff --git a/nptl/pthreadP.h b/nptl/pthreadP.h
> index 99713c8447..9bb44c8535 100644
> --- a/nptl/pthreadP.h
> +++ b/nptl/pthreadP.h
> @@ -462,6 +462,8 @@ extern int __pthread_cond_wait (pthread_cond_t *cond, pthread_mutex_t *mutex);
>  #if __TIMESIZE == 64
>  # define __pthread_clockjoin_np64 __pthread_clockjoin_np
>  # define __pthread_timedjoin_np64 __pthread_timedjoin_np
> +# define __pthread_cond_timedwait64 __pthread_cond_timedwait
> +# define __pthread_cond_clockwait64 __pthread_cond_clockwait
>  #else
>  extern int __pthread_clockjoin_np64 (pthread_t threadid, void **thread_return,
>                                       clockid_t clockid,
> @@ -470,6 +472,15 @@ libc_hidden_proto (__pthread_clockjoin_np64)
>  extern int __pthread_timedjoin_np64 (pthread_t threadid, void **thread_return,
>                                       const struct __timespec64 *abstime);
>  libc_hidden_proto (__pthread_timedjoin_np64)
> +extern int __pthread_cond_timedwait64 (pthread_cond_t *cond,
> +                                       pthread_mutex_t *mutex,
> +                                       const struct __timespec64 *abstime);
> +libpthread_hidden_proto (__pthread_cond_timedwait64)
> +extern int __pthread_cond_clockwait64 (pthread_cond_t *cond,
> +                                       pthread_mutex_t *mutex,
> +                                       clockid_t clockid,
> +                                       const struct __timespec64 *abstime);
> +libpthread_hidden_proto (__pthread_cond_clockwait64)
>  #endif
>  
>  extern int __pthread_cond_timedwait (pthread_cond_t *cond,

Ok.

> diff --git a/nptl/pthread_cond_wait.c b/nptl/pthread_cond_wait.c
> index 85ddbc1011..7d158d553f 100644
> --- a/nptl/pthread_cond_wait.c
> +++ b/nptl/pthread_cond_wait.c
> @@ -378,8 +378,7 @@ __condvar_cleanup_waiting (void *arg)
>  */
>  static __always_inline int
>  __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex,
> -    clockid_t clockid,
> -    const struct timespec *abstime)
> +    clockid_t clockid, const struct __timespec64 *abstime)
>  {
>    const int maxspin = 0;
>    int err;
> @@ -517,7 +516,7 @@ __pthread_cond_wait_common (pthread_cond_t *cond, pthread_mutex_t *mutex,
>  	        err = ETIMEDOUT;
>  	      else
>  		{
> -		  err = futex_abstimed_wait_cancelable
> +		  err = __futex_abstimed_wait_cancelable64
>                      (cond->__data.__g_signals + g, 0, clockid, abstime,
>                       private);
>  		}

Ok.

> @@ -640,8 +639,8 @@ __pthread_cond_wait (pthread_cond_t *cond, pthread_mutex_t *mutex)
>  
>  /* See __pthread_cond_wait_common.  */
>  int
> -__pthread_cond_timedwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
> -    const struct timespec *abstime)
> +__pthread_cond_timedwait64 (pthread_cond_t *cond, pthread_mutex_t *mutex,
> +                            const struct __timespec64 *abstime)
>  {
>    /* Check parameter validity.  This should also tell the compiler that
>       it can assume that abstime is not NULL.  */

Ok.

> @@ -655,6 +654,20 @@ __pthread_cond_timedwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
>                      ? CLOCK_MONOTONIC : CLOCK_REALTIME;
>    return __pthread_cond_wait_common (cond, mutex, clockid, abstime);
>  }
> +
> +#if __TIMESIZE != 64
> +libpthread_hidden_def (__pthread_cond_timedwait64)
> +
> +int
> +__pthread_cond_timedwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
> +                          const struct timespec *abstime)
> +{
> +  struct __timespec64 ts64 = valid_timespec_to_timespec64 (*abstime);
> +
> +  return __pthread_cond_timedwait64 (cond, mutex, &ts64);
> +}
> +#endif
> +
>  versioned_symbol (libpthread, __pthread_cond_wait, pthread_cond_wait,
>  		  GLIBC_2_3_2);
>  versioned_symbol (libpthread, __pthread_cond_timedwait, pthread_cond_timedwait,

Ok.

> @@ -662,9 +675,9 @@ versioned_symbol (libpthread, __pthread_cond_timedwait, pthread_cond_timedwait,
>  
>  /* See __pthread_cond_wait_common.  */
>  int
> -__pthread_cond_clockwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
> -			  clockid_t clockid,
> -			  const struct timespec *abstime)
> +__pthread_cond_clockwait64 (pthread_cond_t *cond, pthread_mutex_t *mutex,
> +                            clockid_t clockid,
> +                            const struct __timespec64 *abstime)
>  {
>    /* Check parameter validity.  This should also tell the compiler that
>       it can assume that abstime is not NULL.  */

Ok.

> @@ -676,4 +689,18 @@ __pthread_cond_clockwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
>  
>    return __pthread_cond_wait_common (cond, mutex, clockid, abstime);
>  }
> +
> +#if __TIMESIZE != 64
> +libpthread_hidden_def (__pthread_cond_clockwait64)
> +
> +int
> +__pthread_cond_clockwait (pthread_cond_t *cond, pthread_mutex_t *mutex,
> +                          clockid_t clockid,
> +                          const struct timespec *abstime)
> +{
> +  struct __timespec64 ts64 = valid_timespec_to_timespec64 (*abstime);
> +
> +  return __pthread_cond_clockwait64 (cond, mutex, clockid, &ts64);
> +}
> +#endif

Ok.

>  weak_alias (__pthread_cond_clockwait, pthread_cond_clockwait);
> diff --git a/sysdeps/nptl/Makefile b/sysdeps/nptl/Makefile
> index 0631a870c8..a65be3b7ea 100644
> --- a/sysdeps/nptl/Makefile
> +++ b/sysdeps/nptl/Makefile
> @@ -17,7 +17,7 @@
>  # <https://www.gnu.org/licenses/>.
>  
>  ifeq ($(subdir),nptl)
> -libpthread-sysdep_routines += errno-loc
> +libpthread-sysdep_routines += errno-loc futex-internal
>  endif
>  
>  ifeq ($(subdir),rt)

Ok.

> diff --git a/sysdeps/nptl/futex-internal.c b/sysdeps/nptl/futex-internal.c
> new file mode 100644
> index 0000000000..692820b0b0
> --- /dev/null
> +++ b/sysdeps/nptl/futex-internal.c
> @@ -0,0 +1,96 @@
> +/* futex helper functions for glibc-internal use.
> +   Copyright (C) 2020 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
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#include <errno.h>
> +#include <sysdep.h>
> +#include <time.h>
> +#include <futex-internal.h>
> +#include <kernel-features.h>
> +
> +#ifndef __ASSUME_TIME64_SYSCALLS
> +static int
> +__futex_abstimed_wait_cancellable32 (unsigned int* futex_word,
> +                                     unsigned int expected, clockid_t clockid,
> +                                     const struct __timespec64* abstime,
> +                                     int private)
> +{
> +  if (! in_time_t_range (abstime->tv_sec))
> +    return -EOVERFLOW;
> +
> +  unsigned int clockbit = (clockid == CLOCK_REALTIME)
> +	  ? FUTEX_CLOCK_REALTIME : 0;
> +  int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, private);
> +
> +  struct timespec ts32 = valid_timespec64_to_timespec (*abstime);
> +  return INTERNAL_SYSCALL_CANCEL (futex, futex_word, op, expected,
> +                                  &ts32, NULL /* Unused.  */,
> +                                  FUTEX_BITSET_MATCH_ANY);
> +}
> +#endif
> +
> +int
> +attribute_hidden

There is no need to add this, the function prototype should handle it.

> +__futex_abstimed_wait_cancelable64 (unsigned int* futex_word,
> +                                    unsigned int expected, clockid_t clockid,
> +                                    const struct __timespec64* abstime,
> +                                    int private)
> +{
> +  unsigned int clockbit;
> +  int err;
> +
> +  /* Work around the fact that the kernel rejects negative timeout values
> +     despite them being valid.  */
> +  if (__glibc_unlikely ((abstime != NULL) && (abstime->tv_sec < 0)))
> +    return ETIMEDOUT;
> +
> +  if (! lll_futex_supported_clockid (clockid))
> +    return EINVAL;
> +
> +  clockbit = (clockid == CLOCK_REALTIME) ? FUTEX_CLOCK_REALTIME : 0;
> +  int op = __lll_private_flag (FUTEX_WAIT_BITSET | clockbit, private);
> +
> +  err = INTERNAL_SYSCALL_CANCEL (futex_time64, futex_word, op, expected,
> +                                 abstime, NULL /* Unused.  */,
> +                                 FUTEX_BITSET_MATCH_ANY);
> +#ifndef __ASSUME_TIME64_SYSCALLS
> +if (err == -ENOSYS)
> +	err = __futex_abstimed_wait_cancellable32 (futex_word, expected,
> +	                                           clockid, abstime, private);
> +#endif
> +
> +  switch (err)
> +    {
> +    case 0:
> +    case -EAGAIN:
> +    case -EINTR:
> +    case -ETIMEDOUT:
> +    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:
> +      futex_fatal_error ();
> +    }
> +}

Ok.

> diff --git a/sysdeps/nptl/futex-internal.h b/sysdeps/nptl/futex-internal.h
> index 159aae82dc..3c4a3edb55 100644
> --- a/sysdeps/nptl/futex-internal.h
> +++ b/sysdeps/nptl/futex-internal.h
> @@ -519,4 +519,15 @@ futex_timed_wait_cancel64 (pid_t *tidp,  pid_t tid,
>        futex_fatal_error ();
>      }
>  }
> +
> +/* The futex_abstimed_wait_cancelable64 has been moved to a separate file
> +   to avoid problems with exhausting available registers on some architectures
> +   - e.g. on m68k architecture.  */
> +int
> +attribute_hidden

Move the attribute_hidden to the end of the function prototype (commit 30e5069c7d4
fix a issue where the __MALLOC_DEPRECATED is empty on some configurations, it
should not happen here since attribute_hidden will be always defined with supported
configurations though).

> +__futex_abstimed_wait_cancelable64 (unsigned int* futex_word,
> +                                    unsigned int expected, clockid_t clockid,
> +                                    const struct __timespec64* abstime,
> +                                    int private);
> +
>  #endif  /* futex-internal.h */
>