[v2] sysdeps/nanosleep: Use clock_nanosleep_time64 if avaliable

Message ID 20191024234106.27081-1-alistair.francis@wdc.com
State New
Headers show
Series
  • [v2] sysdeps/nanosleep: Use clock_nanosleep_time64 if avaliable
Related show

Commit Message

Alistair Francis Oct. 24, 2019, 11:41 p.m.
The nanosleep syscall is not supported on newer 32-bit platforms (such
as RV32). To fix this issue let's use clock_nanosleep_time64 if it is
avaliable.

Let's use CLOCK_REALTIME when calling clock_nanosleep_time64 as the
Linux specification says:
  "POSIX.1 specifies that nanosleep() should measure time against the
   CLOCK_REALTIME clock. However, Linux measures the time using the
   CLOCK_MONOTONIC clock. This probably does not matter, since the POSIX.1
   specification for clock_settime(2) says that discontinuous changes in
   CLOCK_REALTIME should not affect nanosleep()"
---
This was patch was runtime tested with RV32 and RV64
It was build tested using the ./scripts/build-many-glibcs.py script.

I am currently running:
$ make ; make install ; make check
tests on native ARM (32-bit) with the following three confiugrations:
 - 4.19 Kernel and 4.19 Headers
 - 5.2 Kernel and 4.19 Headers
 - 5.2 Kernel and 5.2 Headers
I should have the results tomorrow.

v2:
 - Explicitly include `#include <kernel-features.h>`

 include/time.h                               | 20 ++++++
 nptl/thrd_sleep.c                            | 70 +++++++++++++++++--
 sysdeps/unix/sysv/linux/clock_nanosleep.c    | 73 ++++++++++++++++++--
 sysdeps/unix/sysv/linux/nanosleep.c          | 65 ++++++++++++++++-
 sysdeps/unix/sysv/linux/nanosleep_nocancel.c | 65 ++++++++++++++++-
 5 files changed, 280 insertions(+), 13 deletions(-)

-- 
2.23.0

Comments

Joseph Myers Oct. 25, 2019, 9:44 p.m. | #1
On Thu, 24 Oct 2019, Alistair Francis wrote:

> +# ifdef __NR_clock_nanosleep_time64

> +  long int ret_64;

> +

> +  ret_64 = INTERNAL_SYSCALL_CANCEL (clock_nanosleep_time64, err, CLOCK_REALTIME,

> +                                    0, time_point, remaining);

> +

> +  if (ret_64 == 0 || errno != ENOSYS)

> +    ret = ret_64;

> +# endif /* __NR_clock_nanosleep_time64 */

> +  if (ret < 0)

> +    {

> +      struct timespec tp32, tr32;

> +

> +      if (! in_time_t_range (time_point->tv_sec))

> +        {

> +          __set_errno (EOVERFLOW);

> +          return -1;

> +        }

> +

> +      tp32 = valid_timespec64_to_timespec (*time_point);

> +      ret =  INTERNAL_SYSCALL_CANCEL (nanosleep, err, &tp32, &tr32);


This looks like it would call the nanosleep syscall if 
clock_nanosleep_time64 failed for some non-ENOSYS reason.  That doesn't 
seem right.

> +      if ((ret == 0 || errno != ENOSYS) && remaining)

> +        *remaining = valid_timespec_to_timespec64 (tr32);


And if nanosleep fails for a non-EINTR reason, it won't have filled in 
tr32, so this could be copying uninitialized data.

> +#if __TIMESIZE != 64

> +int

> +thrd_sleep (const struct timespec* time_point, struct timespec* remaining)

> +{

> +  int ret;

> +  struct __timespec64 tp64, tr64;

> +

> +  tp64 = valid_timespec_to_timespec64 (*time_point);

> +  ret = __thrd_sleep_time64 (&tp64, &tr64);

> +

> +  if (ret == 0 || errno != ENOSYS)

> +    {

> +      if (! in_time_t_range (tr64.tv_sec))

> +        {

> +          __set_errno (EOVERFLOW);

> +          return -1;


This could not only be copying uninitialized data for such a failure (e.g. 
a failure for an invalid nanoseconds value), it could actually produce a 
spurious EOVERFLOW error if that uninitialized data is out of range.

As far as I know the remaining amount can't exceed the amount passed in, 
so when it's copied it shouldn't actually be necessary to check for 
overflow.

I think similar issues apply for other functions in the patch.

Do we have test coverage in the testsuite for invalid nanoseconds 
arguments to all these functions, making sure they produce proper EINVAL 
errors?  That might not cover many of these bugs (it wouldn't detect 
simply reading uninitialized memory, or calling more syscalls than 
necessary, and detecting the spurious EOVERFLOW would depend on what 
values were in that memory).  But it would seem a good idea to make sure 
we do have such test coverage for functions using time arguments.

-- 
Joseph S. Myers
joseph@codesourcery.com
Adhemerval Zanella Oct. 28, 2019, 7:51 p.m. | #2
On 24/10/2019 20:41, Alistair Francis wrote:
> The nanosleep syscall is not supported on newer 32-bit platforms (such

> as RV32). To fix this issue let's use clock_nanosleep_time64 if it is

> avaliable.

> 

> Let's use CLOCK_REALTIME when calling clock_nanosleep_time64 as the

> Linux specification says:

>   "POSIX.1 specifies that nanosleep() should measure time against the

>    CLOCK_REALTIME clock. However, Linux measures the time using the

>    CLOCK_MONOTONIC clock. This probably does not matter, since the POSIX.1

>    specification for clock_settime(2) says that discontinuous changes in

>    CLOCK_REALTIME should not affect nanosleep()"

> ---

> This was patch was runtime tested with RV32 and RV64

> It was build tested using the ./scripts/build-many-glibcs.py script.

> 

> I am currently running:

> $ make ; make install ; make check

> tests on native ARM (32-bit) with the following three confiugrations:

>  - 4.19 Kernel and 4.19 Headers

>  - 5.2 Kernel and 4.19 Headers

>  - 5.2 Kernel and 5.2 Headers

> I should have the results tomorrow.

> 

> v2:

>  - Explicitly include `#include <kernel-features.h>`

> 

>  include/time.h                               | 20 ++++++

>  nptl/thrd_sleep.c                            | 70 +++++++++++++++++--

>  sysdeps/unix/sysv/linux/clock_nanosleep.c    | 73 ++++++++++++++++++--

>  sysdeps/unix/sysv/linux/nanosleep.c          | 65 ++++++++++++++++-

>  sysdeps/unix/sysv/linux/nanosleep_nocancel.c | 65 ++++++++++++++++-

>  5 files changed, 280 insertions(+), 13 deletions(-)

> 

> diff --git a/include/time.h b/include/time.h

> index d93b16a7810..0e703f87cef 100644

> --- a/include/time.h

> +++ b/include/time.h

> @@ -174,6 +174,26 @@ libc_hidden_proto (__difftime64)

>  

>  extern double __difftime (time_t time1, time_t time0);

>  

> +#if __TIMESIZE == 64

> +# define __thrd_sleep_time64 thrd_sleep

> +# define __clock_nanosleep_time64 __clock_nanosleep

> +# define __nanosleep_time64 __nanosleep

> +# define __nanosleep_nocancel_time64 __nanosleep_nocancel

> +#else

> +extern int __thrd_sleep_time64 (const struct __timespec64* time_point,

> +                                struct __timespec64* remaining);

> +libc_hidden_proto (__thrd_sleep_time64)

> +extern int __clock_nanosleep_time64 (clockid_t clock_id,

> +                                     int flags, const struct __timespec64 *req,

> +                                     struct __timespec64 *rem);

> +libc_hidden_proto (__clock_nanosleep_time64)

> +extern  int __nanosleep_time64 (const struct __timespec64 *requested_time,

> +                                struct __timespec64 *remaining);

> +libc_hidden_proto (__nanosleep_time64)

> +extern int __nanosleep_nocancel_time64 (const struct __timespec64 *requested_time,

> +                                        struct __timespec64 *remaining);

> +libc_hidden_proto (__nanosleep_nocancel_time64)

> +#endif

>  

>  /* Use in the clock_* functions.  Size of the field representing the

>     actual clock ID.  */

> diff --git a/nptl/thrd_sleep.c b/nptl/thrd_sleep.c

> index 2e185dd748e..2a6bb1f9e3f 100644

> --- a/nptl/thrd_sleep.c

> +++ b/nptl/thrd_sleep.c

> @@ -17,23 +17,85 @@

>     <https://www.gnu.org/licenses/>.  */

>  

>  #include <time.h>

> +#include <kernel-features.h>

>  #include <sysdep-cancel.h>

>  

>  #include "thrd_priv.h"

>  

>  int

> -thrd_sleep (const struct timespec* time_point, struct timespec* remaining)

> +__thrd_sleep_time64 (const struct __timespec64* time_point, struct __timespec64* remaining)

>  {

>    INTERNAL_SYSCALL_DECL (err);

> -  int ret = INTERNAL_SYSCALL_CANCEL (nanosleep, err, time_point, remaining);

> +  int ret = -1;

> +

> +#ifdef __ASSUME_TIME64_SYSCALLS

> +# ifndef __NR_clock_nanosleep_time64

> +#  define __NR_clock_nanosleep_time64 __NR_clock_nanosleep

> +# endif

> +  ret = INTERNAL_SYSCALL_CANCEL (clock_nanosleep_time64, err, CLOCK_REALTIME,

> +                                 0, time_point, remaining);

> +#else

> +# ifdef __NR_clock_nanosleep_time64

> +  long int ret_64;

> +

> +  ret_64 = INTERNAL_SYSCALL_CANCEL (clock_nanosleep_time64, err, CLOCK_REALTIME,

> +                                    0, time_point, remaining);

> +

> +  if (ret_64 == 0 || errno != ENOSYS)

> +    ret = ret_64;

> +# endif /* __NR_clock_nanosleep_time64 */

> +  if (ret < 0)

> +    {

> +      struct timespec tp32, tr32;

> +

> +      if (! in_time_t_range (time_point->tv_sec))

> +        {

> +          __set_errno (EOVERFLOW);

> +          return -1;

> +        }

> +

> +      tp32 = valid_timespec64_to_timespec (*time_point);

> +      ret =  INTERNAL_SYSCALL_CANCEL (nanosleep, err, &tp32, &tr32);

> +

> +      if ((ret == 0 || errno != ENOSYS) && remaining)

> +        *remaining = valid_timespec_to_timespec64 (tr32);

> +    }

> +#endif /* __ASSUME_TIME64_SYSCALLS */

> +

>    if (INTERNAL_SYSCALL_ERROR_P (ret, err))

>      {

>        /* C11 states thrd_sleep function returns -1 if it has been interrupted

> -	 by a signal, or a negative value if it fails.  */

> +         by a signal, or a negative value if it fails.  */

>        ret = INTERNAL_SYSCALL_ERRNO (ret, err);

>        if (ret == EINTR)

> -	return -1;

> +        return -1;

>        return -2;

>      }

>    return 0;

>  }


Wouldn't be simple to just call __clock_nanosleep_time64 instead of duplicate
the syscall handling and the required error code for c11 thread?  Something
like:

int
__thrd_sleep_time64 (const struct __timespec64* time_point,
		     struct __timespec64* remaining)
{
  int ret = __clock_nanosleep (CLOCK_REALTIME, 0, time_point, remaining);
  /* C11 states thrd_sleep function returns -1 if it has been interrupted
     by a signal, or a negative value if it fails.  */
  switch (ret)
  {
     case 0:      return 0;
     case EINTR:  return -1;
     default:     return -2;
  }
}

> +

> +#if __TIMESIZE != 64

> +int

> +thrd_sleep (const struct timespec* time_point, struct timespec* remaining)

> +{

> +  int ret;

> +  struct __timespec64 tp64, tr64;

> +

> +  tp64 = valid_timespec_to_timespec64 (*time_point);

> +  ret = __thrd_sleep_time64 (&tp64, &tr64);

> +

> +  if (ret == 0 || errno != ENOSYS)

> +    {

> +      if (! in_time_t_range (tr64.tv_sec))

> +        {

> +          __set_errno (EOVERFLOW);

> +          return -1;

> +        }

> +

> +      if (remaining)

> +        *remaining = valid_timespec64_to_timespec (tr64);

> +    }

> +

> +  return ret;

> +}

> +#endif

> diff --git a/sysdeps/unix/sysv/linux/clock_nanosleep.c b/sysdeps/unix/sysv/linux/clock_nanosleep.c

> index 1f240b8720a..d257ea57fb0 100644

> --- a/sysdeps/unix/sysv/linux/clock_nanosleep.c

> +++ b/sysdeps/unix/sysv/linux/clock_nanosleep.c

> @@ -16,6 +16,7 @@

>     <https://www.gnu.org/licenses/>.  */

>  

>  #include <time.h>

> +#include <kernel-features.h>

>  #include <errno.h>

>  

>  #include <sysdep-cancel.h>

> @@ -26,9 +27,11 @@

>  /* We can simply use the syscall.  The CPU clocks are not supported

>     with this function.  */

>  int

> -__clock_nanosleep (clockid_t clock_id, int flags, const struct timespec *req,

> -		   struct timespec *rem)

> +__clock_nanosleep_time64 (clockid_t clock_id, int flags, const struct __timespec64 *req,

> +                          struct __timespec64 *rem)

>  {

> +  int r = -1;

> +

>    if (clock_id == CLOCK_THREAD_CPUTIME_ID)

>      return EINVAL;

>    if (clock_id == CLOCK_PROCESS_CPUTIME_ID)

> @@ -37,12 +40,72 @@ __clock_nanosleep (clockid_t clock_id, int flags, const struct timespec *req,

>    /* If the call is interrupted by a signal handler or encounters an error,

>       it returns a positive value similar to errno.  */

>    INTERNAL_SYSCALL_DECL (err);

> -  int r = INTERNAL_SYSCALL_CANCEL (clock_nanosleep, err, clock_id, flags,

> -				   req, rem);

> +

> +#ifdef __ASSUME_TIME64_SYSCALLS

> +# ifndef __NR_clock_nanosleep_time64

> +#  define __NR_clock_nanosleep_time64 __NR_clock_nanosleep

> +# endif

> +  r = INTERNAL_SYSCALL_CANCEL (clock_nanosleep_time64, err, clock_id,

> +                               flags, req, rem);

> +#else

> +# ifdef __NR_clock_nanosleep_time64

> +  long int ret_64;

> +

> +  ret_64 = INTERNAL_SYSCALL_CANCEL (clock_nanosleep_time64, err, clock_id,

> +                                    flags, req, rem);

> +

> +  if (ret_64 == 0 || errno != ENOSYS)

> +    r = ret_64;

> +# endif /* __NR_clock_nanosleep_time64 */

> +  if (r < 0)

> +    {

> +      struct timespec ts32, tr32;

> +

> +      if (! in_time_t_range (req->tv_sec))

> +        {

> +          __set_errno (EOVERFLOW);

> +          return -1;

> +        }

> +

> +      ts32 = valid_timespec64_to_timespec (*req);

> +      r =  INTERNAL_SYSCALL_CANCEL (clock_nanosleep, err, &ts32, &tr32);

> +

> +      if ((r == 0 || errno != ENOSYS) && rem)

> +        *rem = valid_timespec_to_timespec64 (tr32);

> +    }

> +#endif /* __ASSUME_TIME64_SYSCALLS */

> +

>    return (INTERNAL_SYSCALL_ERROR_P (r, err)

> -	  ? INTERNAL_SYSCALL_ERRNO (r, err) : 0);

> +          ? INTERNAL_SYSCALL_ERRNO (r, err) : 0);

>  }

>  

> +#if __TIMESIZE != 64

> +int

> +__clock_nanosleep (clockid_t clock_id, int flags, const struct timespec *req,

> +                   struct timespec *rem)

> +{

> +  int r;

> +  struct __timespec64 treq64, trem64;

> +

> +  treq64 = valid_timespec_to_timespec64 (*req);

> +  r = __clock_nanosleep_time64 (clock_id, flags, &treq64, &trem64);

> +

> +  if (r == 0 || errno != ENOSYS)

> +    {

> +      if (! in_time_t_range (trem64.tv_sec))

> +        {

> +          __set_errno (EOVERFLOW);

> +          return -1;

> +        }

> +

> +      if (rem)

> +        *rem = valid_timespec64_to_timespec (trem64);

> +    }

> +

> +  return r;

> +}

> +#endif

> +

>  versioned_symbol (libc, __clock_nanosleep, clock_nanosleep, GLIBC_2_17);

>  /* clock_nanosleep moved to libc in version 2.17;

>     old binaries may expect the symbol version it had in librt.  */

> diff --git a/sysdeps/unix/sysv/linux/nanosleep.c b/sysdeps/unix/sysv/linux/nanosleep.c

> index 6787909248f..10a6d57a1de 100644

> --- a/sysdeps/unix/sysv/linux/nanosleep.c

> +++ b/sysdeps/unix/sysv/linux/nanosleep.c

> @@ -17,15 +17,76 @@

>     <https://www.gnu.org/licenses/>.  */

>  

>  #include <time.h>

> +#include <kernel-features.h>

>  #include <sysdep-cancel.h>

>  #include <not-cancel.h>

>  

>  /* Pause execution for a number of nanoseconds.  */

>  int

> +__nanosleep_time64 (const struct __timespec64 *requested_time,

> +                    struct __timespec64 *remaining)

> +{

> +#if defined(__ASSUME_TIME64_SYSCALLS)

> +# ifndef __NR_clock_nanosleep_time64

> +#  define __NR_clock_nanosleep_time64 __NR_clock_nanosleep

> +# endif

> +  return SYSCALL_CANCEL (clock_nanosleep_time64, CLOCK_REALTIME, 0,

> +                         requested_time, remaining);

> +#else

> +# ifdef __NR_clock_nanosleep_time64

> +  long int ret_64;

> +

> +  ret_64 = SYSCALL_CANCEL (clock_nanosleep_time64, CLOCK_REALTIME, 0,

> +                           requested_time, remaining);

> +

> +  if (ret_64 == 0 || errno != ENOSYS)

> +    return ret_64;

> +# endif /* __NR_clock_nanosleep_time64 */

> +  int ret;

> +  struct timespec ts32, tr32;

> +

> +  if (! in_time_t_range (requested_time->tv_sec))

> +    {

> +      __set_errno (EOVERFLOW);

> +      return -1;

> +    }

> +

> +  ts32 = valid_timespec64_to_timespec (*requested_time);

> +  ret = SYSCALL_CANCEL (nanosleep, &ts32, &tr32);

> +

> +  if ((ret == 0 || errno != ENOSYS) && remaining)

> +    *remaining = valid_timespec_to_timespec64 (tr32);

> +

> +  return ret;

> +#endif /* __ASSUME_TIME64_SYSCALLS */

> +}

> +

> +#if __TIMESIZE != 64

> +int

>  __nanosleep (const struct timespec *requested_time,

> -	     struct timespec *remaining)

> +             struct timespec *remaining)

>  {

> -  return SYSCALL_CANCEL (nanosleep, requested_time, remaining);

> +  int r;

> +  struct __timespec64 treq64, trem64;

> +

> +  treq64 = valid_timespec_to_timespec64 (*requested_time);

> +  r = __nanosleep_time64 (&treq64, &trem64);

> +

> +  if (r == 0 || errno != ENOSYS)

> +    {

> +      if (! in_time_t_range (trem64.tv_sec))

> +        {

> +          __set_errno (EOVERFLOW);

> +          return -1;

> +        }

> +

> +      if (remaining)

> +        *remaining = valid_timespec64_to_timespec (trem64);

> +    }

> +

> +  return r;

>  }

> +#endif

> +

>  hidden_def (__nanosleep)

>  weak_alias (__nanosleep, nanosleep)

> diff --git a/sysdeps/unix/sysv/linux/nanosleep_nocancel.c b/sysdeps/unix/sysv/linux/nanosleep_nocancel.c

> index d6442bf4f7f..1a6c6c0a48a 100644

> --- a/sysdeps/unix/sysv/linux/nanosleep_nocancel.c

> +++ b/sysdeps/unix/sysv/linux/nanosleep_nocancel.c

> @@ -17,13 +17,74 @@

>     <https://www.gnu.org/licenses/>.  */

>  

>  #include <time.h>

> +#include <kernel-features.h>

>  #include <sysdep-cancel.h>

>  #include <not-cancel.h>

>  

> +int

> +__nanosleep_nocancel_time64 (const struct __timespec64 *requested_time,

> +                             struct __timespec64 *remaining)

> +{

> +#ifdef __ASSUME_TIME64_SYSCALLS

> +# ifndef __NR_clock_nanosleep_time64

> +#  define __NR_clock_nanosleep_time64 __NR_clock_nanosleep

> +# endif

> +  return INLINE_SYSCALL_CALL (clock_nanosleep_time64, CLOCK_REALTIME, 0,

> +                              requested_time, remaining);

> +#else

> +# ifdef __NR_clock_nanosleep_time64

> +  long int ret_64;

> +

> +  ret_64 = INLINE_SYSCALL_CALL (clock_nanosleep_time64, CLOCK_REALTIME, 0,

> +                                requested_time, remaining);

> +

> +  if (ret_64 == 0 || errno != ENOSYS)

> +    return ret_64;

> +# endif /* __NR_clock_nanosleep_time64 */

> +  int ret;

> +  struct timespec ts32, tr32;

> +

> +  if (! in_time_t_range (requested_time->tv_sec))

> +    {

> +      __set_errno (EOVERFLOW);

> +      return -1;

> +    }

> +

> +  ts32 = valid_timespec64_to_timespec (*requested_time);

> +  ret = INLINE_SYSCALL_CALL (nanosleep, &ts32, &tr32);

> +

> +  if (ret == 0 || errno != ENOSYS)

> +    *remaining = valid_timespec_to_timespec64 (tr32);

> +

> +  return ret;

> +#endif /* __ASSUME_TIME64_SYSCALLS */

> +}

> +

> +#if __TIMESIZE != 64

>  int

>  __nanosleep_nocancel (const struct timespec *requested_time,

> -		      struct timespec *remaining)

> +                      struct timespec *remaining)

>  {

> -  return INLINE_SYSCALL_CALL (nanosleep, requested_time, remaining);

> +  int ret;

> +  struct __timespec64 treq64, trem64;

> +

> +  treq64 = valid_timespec_to_timespec64 (*requested_time);

> +  ret = __nanosleep_nocancel_time64 (&treq64, &trem64);

> +

> +  if (ret == 0 || errno != ENOSYS)

> +    {

> +      if (! in_time_t_range (trem64.tv_sec))

> +        {

> +          __set_errno (EOVERFLOW);

> +          return -1;

> +        }

> +

> +      if (remaining)

> +        *remaining = valid_timespec64_to_timespec (trem64);

> +    }

> +

> +  return ret;

>  }

> +#endif

> +

>  hidden_def (__nanosleep_nocancel)

>
Alistair Francis Oct. 29, 2019, 1:45 p.m. | #3
On Fri, Oct 25, 2019 at 11:44 PM Joseph Myers <joseph@codesourcery.com> wrote:
>

> On Thu, 24 Oct 2019, Alistair Francis wrote:

>

> > +# ifdef __NR_clock_nanosleep_time64

> > +  long int ret_64;

> > +

> > +  ret_64 = INTERNAL_SYSCALL_CANCEL (clock_nanosleep_time64, err, CLOCK_REALTIME,

> > +                                    0, time_point, remaining);

> > +

> > +  if (ret_64 == 0 || errno != ENOSYS)

> > +    ret = ret_64;

> > +# endif /* __NR_clock_nanosleep_time64 */

> > +  if (ret < 0)

> > +    {

> > +      struct timespec tp32, tr32;

> > +

> > +      if (! in_time_t_range (time_point->tv_sec))

> > +        {

> > +          __set_errno (EOVERFLOW);

> > +          return -1;

> > +        }

> > +

> > +      tp32 = valid_timespec64_to_timespec (*time_point);

> > +      ret =  INTERNAL_SYSCALL_CANCEL (nanosleep, err, &tp32, &tr32);

>

> This looks like it would call the nanosleep syscall if

> clock_nanosleep_time64 failed for some non-ENOSYS reason.  That doesn't

> seem right.


Yes, you are right. I have fixed this in clock_nanosleep and the
conversion in thrd_sleep() pointed out by Adhemerval fixes it there.


>

> > +      if ((ret == 0 || errno != ENOSYS) && remaining)

> > +        *remaining = valid_timespec_to_timespec64 (tr32);

>

> And if nanosleep fails for a non-EINTR reason, it won't have filled in

> tr32, so this could be copying uninitialized data.


Yes, fixed.

>

> > +#if __TIMESIZE != 64

> > +int

> > +thrd_sleep (const struct timespec* time_point, struct timespec* remaining)

> > +{

> > +  int ret;

> > +  struct __timespec64 tp64, tr64;

> > +

> > +  tp64 = valid_timespec_to_timespec64 (*time_point);

> > +  ret = __thrd_sleep_time64 (&tp64, &tr64);

> > +

> > +  if (ret == 0 || errno != ENOSYS)

> > +    {

> > +      if (! in_time_t_range (tr64.tv_sec))

> > +        {

> > +          __set_errno (EOVERFLOW);

> > +          return -1;

>

> This could not only be copying uninitialized data for such a failure (e.g.

> a failure for an invalid nanoseconds value), it could actually produce a

> spurious EOVERFLOW error if that uninitialized data is out of range.

>

> As far as I know the remaining amount can't exceed the amount passed in,

> so when it's copied it shouldn't actually be necessary to check for

> overflow.


I have removed the check when converting the remaining time from
64-bit to 32-bit for all files.

>

> I think similar issues apply for other functions in the patch.

>

> Do we have test coverage in the testsuite for invalid nanoseconds

> arguments to all these functions, making sure they produce proper EINVAL

> errors?  That might not cover many of these bugs (it wouldn't detect

> simply reading uninitialized memory, or calling more syscalls than

> necessary, and detecting the spurious EOVERFLOW would depend on what

> values were in that memory).  But it would seem a good idea to make sure

> we do have such test coverage for functions using time arguments.


I did see test failures (they were running as I sent this patch) so I
think this is covered, although I haven't investigated the failures.

Alistair

>

> --

> Joseph S. Myers

> joseph@codesourcery.com
Alistair Francis Oct. 29, 2019, 1:45 p.m. | #4
On Mon, Oct 28, 2019 at 8:51 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>

>

>

> On 24/10/2019 20:41, Alistair Francis wrote:

> > The nanosleep syscall is not supported on newer 32-bit platforms (such

> > as RV32). To fix this issue let's use clock_nanosleep_time64 if it is

> > avaliable.

> >

> > Let's use CLOCK_REALTIME when calling clock_nanosleep_time64 as the

> > Linux specification says:

> >   "POSIX.1 specifies that nanosleep() should measure time against the

> >    CLOCK_REALTIME clock. However, Linux measures the time using the

> >    CLOCK_MONOTONIC clock. This probably does not matter, since the POSIX.1

> >    specification for clock_settime(2) says that discontinuous changes in

> >    CLOCK_REALTIME should not affect nanosleep()"

> > ---

> > This was patch was runtime tested with RV32 and RV64

> > It was build tested using the ./scripts/build-many-glibcs.py script.

> >

> > I am currently running:

> > $ make ; make install ; make check

> > tests on native ARM (32-bit) with the following three confiugrations:

> >  - 4.19 Kernel and 4.19 Headers

> >  - 5.2 Kernel and 4.19 Headers

> >  - 5.2 Kernel and 5.2 Headers

> > I should have the results tomorrow.

> >

> > v2:

> >  - Explicitly include `#include <kernel-features.h>`

> >

> >  include/time.h                               | 20 ++++++

> >  nptl/thrd_sleep.c                            | 70 +++++++++++++++++--

> >  sysdeps/unix/sysv/linux/clock_nanosleep.c    | 73 ++++++++++++++++++--

> >  sysdeps/unix/sysv/linux/nanosleep.c          | 65 ++++++++++++++++-

> >  sysdeps/unix/sysv/linux/nanosleep_nocancel.c | 65 ++++++++++++++++-

> >  5 files changed, 280 insertions(+), 13 deletions(-)

> >

> > diff --git a/include/time.h b/include/time.h

> > index d93b16a7810..0e703f87cef 100644

> > --- a/include/time.h

> > +++ b/include/time.h

> > @@ -174,6 +174,26 @@ libc_hidden_proto (__difftime64)

> >

> >  extern double __difftime (time_t time1, time_t time0);

> >

> > +#if __TIMESIZE == 64

> > +# define __thrd_sleep_time64 thrd_sleep

> > +# define __clock_nanosleep_time64 __clock_nanosleep

> > +# define __nanosleep_time64 __nanosleep

> > +# define __nanosleep_nocancel_time64 __nanosleep_nocancel

> > +#else

> > +extern int __thrd_sleep_time64 (const struct __timespec64* time_point,

> > +                                struct __timespec64* remaining);

> > +libc_hidden_proto (__thrd_sleep_time64)

> > +extern int __clock_nanosleep_time64 (clockid_t clock_id,

> > +                                     int flags, const struct __timespec64 *req,

> > +                                     struct __timespec64 *rem);

> > +libc_hidden_proto (__clock_nanosleep_time64)

> > +extern  int __nanosleep_time64 (const struct __timespec64 *requested_time,

> > +                                struct __timespec64 *remaining);

> > +libc_hidden_proto (__nanosleep_time64)

> > +extern int __nanosleep_nocancel_time64 (const struct __timespec64 *requested_time,

> > +                                        struct __timespec64 *remaining);

> > +libc_hidden_proto (__nanosleep_nocancel_time64)

> > +#endif

> >

> >  /* Use in the clock_* functions.  Size of the field representing the

> >     actual clock ID.  */

> > diff --git a/nptl/thrd_sleep.c b/nptl/thrd_sleep.c

> > index 2e185dd748e..2a6bb1f9e3f 100644

> > --- a/nptl/thrd_sleep.c

> > +++ b/nptl/thrd_sleep.c

> > @@ -17,23 +17,85 @@

> >     <https://www.gnu.org/licenses/>.  */

> >

> >  #include <time.h>

> > +#include <kernel-features.h>

> >  #include <sysdep-cancel.h>

> >

> >  #include "thrd_priv.h"

> >

> >  int

> > -thrd_sleep (const struct timespec* time_point, struct timespec* remaining)

> > +__thrd_sleep_time64 (const struct __timespec64* time_point, struct __timespec64* remaining)

> >  {

> >    INTERNAL_SYSCALL_DECL (err);

> > -  int ret = INTERNAL_SYSCALL_CANCEL (nanosleep, err, time_point, remaining);

> > +  int ret = -1;

> > +

> > +#ifdef __ASSUME_TIME64_SYSCALLS

> > +# ifndef __NR_clock_nanosleep_time64

> > +#  define __NR_clock_nanosleep_time64 __NR_clock_nanosleep

> > +# endif

> > +  ret = INTERNAL_SYSCALL_CANCEL (clock_nanosleep_time64, err, CLOCK_REALTIME,

> > +                                 0, time_point, remaining);

> > +#else

> > +# ifdef __NR_clock_nanosleep_time64

> > +  long int ret_64;

> > +

> > +  ret_64 = INTERNAL_SYSCALL_CANCEL (clock_nanosleep_time64, err, CLOCK_REALTIME,

> > +                                    0, time_point, remaining);

> > +

> > +  if (ret_64 == 0 || errno != ENOSYS)

> > +    ret = ret_64;

> > +# endif /* __NR_clock_nanosleep_time64 */

> > +  if (ret < 0)

> > +    {

> > +      struct timespec tp32, tr32;

> > +

> > +      if (! in_time_t_range (time_point->tv_sec))

> > +        {

> > +          __set_errno (EOVERFLOW);

> > +          return -1;

> > +        }

> > +

> > +      tp32 = valid_timespec64_to_timespec (*time_point);

> > +      ret =  INTERNAL_SYSCALL_CANCEL (nanosleep, err, &tp32, &tr32);

> > +

> > +      if ((ret == 0 || errno != ENOSYS) && remaining)

> > +        *remaining = valid_timespec_to_timespec64 (tr32);

> > +    }

> > +#endif /* __ASSUME_TIME64_SYSCALLS */

> > +

> >    if (INTERNAL_SYSCALL_ERROR_P (ret, err))

> >      {

> >        /* C11 states thrd_sleep function returns -1 if it has been interrupted

> > -      by a signal, or a negative value if it fails.  */

> > +         by a signal, or a negative value if it fails.  */

> >        ret = INTERNAL_SYSCALL_ERRNO (ret, err);

> >        if (ret == EINTR)

> > -     return -1;

> > +        return -1;

> >        return -2;

> >      }

> >    return 0;

> >  }

>

> Wouldn't be simple to just call __clock_nanosleep_time64 instead of duplicate

> the syscall handling and the required error code for c11 thread?  Something

> like:

>

> int

> __thrd_sleep_time64 (const struct __timespec64* time_point,

>                      struct __timespec64* remaining)

> {

>   int ret = __clock_nanosleep (CLOCK_REALTIME, 0, time_point, remaining);

>   /* C11 states thrd_sleep function returns -1 if it has been interrupted

>      by a signal, or a negative value if it fails.  */

>   switch (ret)

>   {

>      case 0:      return 0;

>      case EINTR:  return -1;

>      default:     return -2;

>   }

> }


This looks good to me, I'll try that.

Alistair

>

> > +

> > +#if __TIMESIZE != 64

> > +int

> > +thrd_sleep (const struct timespec* time_point, struct timespec* remaining)

> > +{

> > +  int ret;

> > +  struct __timespec64 tp64, tr64;

> > +

> > +  tp64 = valid_timespec_to_timespec64 (*time_point);

> > +  ret = __thrd_sleep_time64 (&tp64, &tr64);

> > +

> > +  if (ret == 0 || errno != ENOSYS)

> > +    {

> > +      if (! in_time_t_range (tr64.tv_sec))

> > +        {

> > +          __set_errno (EOVERFLOW);

> > +          return -1;

> > +        }

> > +

> > +      if (remaining)

> > +        *remaining = valid_timespec64_to_timespec (tr64);

> > +    }

> > +

> > +  return ret;

> > +}

> > +#endif

> > diff --git a/sysdeps/unix/sysv/linux/clock_nanosleep.c b/sysdeps/unix/sysv/linux/clock_nanosleep.c

> > index 1f240b8720a..d257ea57fb0 100644

> > --- a/sysdeps/unix/sysv/linux/clock_nanosleep.c

> > +++ b/sysdeps/unix/sysv/linux/clock_nanosleep.c

> > @@ -16,6 +16,7 @@

> >     <https://www.gnu.org/licenses/>.  */

> >

> >  #include <time.h>

> > +#include <kernel-features.h>

> >  #include <errno.h>

> >

> >  #include <sysdep-cancel.h>

> > @@ -26,9 +27,11 @@

> >  /* We can simply use the syscall.  The CPU clocks are not supported

> >     with this function.  */

> >  int

> > -__clock_nanosleep (clockid_t clock_id, int flags, const struct timespec *req,

> > -                struct timespec *rem)

> > +__clock_nanosleep_time64 (clockid_t clock_id, int flags, const struct __timespec64 *req,

> > +                          struct __timespec64 *rem)

> >  {

> > +  int r = -1;

> > +

> >    if (clock_id == CLOCK_THREAD_CPUTIME_ID)

> >      return EINVAL;

> >    if (clock_id == CLOCK_PROCESS_CPUTIME_ID)

> > @@ -37,12 +40,72 @@ __clock_nanosleep (clockid_t clock_id, int flags, const struct timespec *req,

> >    /* If the call is interrupted by a signal handler or encounters an error,

> >       it returns a positive value similar to errno.  */

> >    INTERNAL_SYSCALL_DECL (err);

> > -  int r = INTERNAL_SYSCALL_CANCEL (clock_nanosleep, err, clock_id, flags,

> > -                                req, rem);

> > +

> > +#ifdef __ASSUME_TIME64_SYSCALLS

> > +# ifndef __NR_clock_nanosleep_time64

> > +#  define __NR_clock_nanosleep_time64 __NR_clock_nanosleep

> > +# endif

> > +  r = INTERNAL_SYSCALL_CANCEL (clock_nanosleep_time64, err, clock_id,

> > +                               flags, req, rem);

> > +#else

> > +# ifdef __NR_clock_nanosleep_time64

> > +  long int ret_64;

> > +

> > +  ret_64 = INTERNAL_SYSCALL_CANCEL (clock_nanosleep_time64, err, clock_id,

> > +                                    flags, req, rem);

> > +

> > +  if (ret_64 == 0 || errno != ENOSYS)

> > +    r = ret_64;

> > +# endif /* __NR_clock_nanosleep_time64 */

> > +  if (r < 0)

> > +    {

> > +      struct timespec ts32, tr32;

> > +

> > +      if (! in_time_t_range (req->tv_sec))

> > +        {

> > +          __set_errno (EOVERFLOW);

> > +          return -1;

> > +        }

> > +

> > +      ts32 = valid_timespec64_to_timespec (*req);

> > +      r =  INTERNAL_SYSCALL_CANCEL (clock_nanosleep, err, &ts32, &tr32);

> > +

> > +      if ((r == 0 || errno != ENOSYS) && rem)

> > +        *rem = valid_timespec_to_timespec64 (tr32);

> > +    }

> > +#endif /* __ASSUME_TIME64_SYSCALLS */

> > +

> >    return (INTERNAL_SYSCALL_ERROR_P (r, err)

> > -       ? INTERNAL_SYSCALL_ERRNO (r, err) : 0);

> > +          ? INTERNAL_SYSCALL_ERRNO (r, err) : 0);

> >  }

> >

> > +#if __TIMESIZE != 64

> > +int

> > +__clock_nanosleep (clockid_t clock_id, int flags, const struct timespec *req,

> > +                   struct timespec *rem)

> > +{

> > +  int r;

> > +  struct __timespec64 treq64, trem64;

> > +

> > +  treq64 = valid_timespec_to_timespec64 (*req);

> > +  r = __clock_nanosleep_time64 (clock_id, flags, &treq64, &trem64);

> > +

> > +  if (r == 0 || errno != ENOSYS)

> > +    {

> > +      if (! in_time_t_range (trem64.tv_sec))

> > +        {

> > +          __set_errno (EOVERFLOW);

> > +          return -1;

> > +        }

> > +

> > +      if (rem)

> > +        *rem = valid_timespec64_to_timespec (trem64);

> > +    }

> > +

> > +  return r;

> > +}

> > +#endif

> > +

> >  versioned_symbol (libc, __clock_nanosleep, clock_nanosleep, GLIBC_2_17);

> >  /* clock_nanosleep moved to libc in version 2.17;

> >     old binaries may expect the symbol version it had in librt.  */

> > diff --git a/sysdeps/unix/sysv/linux/nanosleep.c b/sysdeps/unix/sysv/linux/nanosleep.c

> > index 6787909248f..10a6d57a1de 100644

> > --- a/sysdeps/unix/sysv/linux/nanosleep.c

> > +++ b/sysdeps/unix/sysv/linux/nanosleep.c

> > @@ -17,15 +17,76 @@

> >     <https://www.gnu.org/licenses/>.  */

> >

> >  #include <time.h>

> > +#include <kernel-features.h>

> >  #include <sysdep-cancel.h>

> >  #include <not-cancel.h>

> >

> >  /* Pause execution for a number of nanoseconds.  */

> >  int

> > +__nanosleep_time64 (const struct __timespec64 *requested_time,

> > +                    struct __timespec64 *remaining)

> > +{

> > +#if defined(__ASSUME_TIME64_SYSCALLS)

> > +# ifndef __NR_clock_nanosleep_time64

> > +#  define __NR_clock_nanosleep_time64 __NR_clock_nanosleep

> > +# endif

> > +  return SYSCALL_CANCEL (clock_nanosleep_time64, CLOCK_REALTIME, 0,

> > +                         requested_time, remaining);

> > +#else

> > +# ifdef __NR_clock_nanosleep_time64

> > +  long int ret_64;

> > +

> > +  ret_64 = SYSCALL_CANCEL (clock_nanosleep_time64, CLOCK_REALTIME, 0,

> > +                           requested_time, remaining);

> > +

> > +  if (ret_64 == 0 || errno != ENOSYS)

> > +    return ret_64;

> > +# endif /* __NR_clock_nanosleep_time64 */

> > +  int ret;

> > +  struct timespec ts32, tr32;

> > +

> > +  if (! in_time_t_range (requested_time->tv_sec))

> > +    {

> > +      __set_errno (EOVERFLOW);

> > +      return -1;

> > +    }

> > +

> > +  ts32 = valid_timespec64_to_timespec (*requested_time);

> > +  ret = SYSCALL_CANCEL (nanosleep, &ts32, &tr32);

> > +

> > +  if ((ret == 0 || errno != ENOSYS) && remaining)

> > +    *remaining = valid_timespec_to_timespec64 (tr32);

> > +

> > +  return ret;

> > +#endif /* __ASSUME_TIME64_SYSCALLS */

> > +}

> > +

> > +#if __TIMESIZE != 64

> > +int

> >  __nanosleep (const struct timespec *requested_time,

> > -          struct timespec *remaining)

> > +             struct timespec *remaining)

> >  {

> > -  return SYSCALL_CANCEL (nanosleep, requested_time, remaining);

> > +  int r;

> > +  struct __timespec64 treq64, trem64;

> > +

> > +  treq64 = valid_timespec_to_timespec64 (*requested_time);

> > +  r = __nanosleep_time64 (&treq64, &trem64);

> > +

> > +  if (r == 0 || errno != ENOSYS)

> > +    {

> > +      if (! in_time_t_range (trem64.tv_sec))

> > +        {

> > +          __set_errno (EOVERFLOW);

> > +          return -1;

> > +        }

> > +

> > +      if (remaining)

> > +        *remaining = valid_timespec64_to_timespec (trem64);

> > +    }

> > +

> > +  return r;

> >  }

> > +#endif

> > +

> >  hidden_def (__nanosleep)

> >  weak_alias (__nanosleep, nanosleep)

> > diff --git a/sysdeps/unix/sysv/linux/nanosleep_nocancel.c b/sysdeps/unix/sysv/linux/nanosleep_nocancel.c

> > index d6442bf4f7f..1a6c6c0a48a 100644

> > --- a/sysdeps/unix/sysv/linux/nanosleep_nocancel.c

> > +++ b/sysdeps/unix/sysv/linux/nanosleep_nocancel.c

> > @@ -17,13 +17,74 @@

> >     <https://www.gnu.org/licenses/>.  */

> >

> >  #include <time.h>

> > +#include <kernel-features.h>

> >  #include <sysdep-cancel.h>

> >  #include <not-cancel.h>

> >

> > +int

> > +__nanosleep_nocancel_time64 (const struct __timespec64 *requested_time,

> > +                             struct __timespec64 *remaining)

> > +{

> > +#ifdef __ASSUME_TIME64_SYSCALLS

> > +# ifndef __NR_clock_nanosleep_time64

> > +#  define __NR_clock_nanosleep_time64 __NR_clock_nanosleep

> > +# endif

> > +  return INLINE_SYSCALL_CALL (clock_nanosleep_time64, CLOCK_REALTIME, 0,

> > +                              requested_time, remaining);

> > +#else

> > +# ifdef __NR_clock_nanosleep_time64

> > +  long int ret_64;

> > +

> > +  ret_64 = INLINE_SYSCALL_CALL (clock_nanosleep_time64, CLOCK_REALTIME, 0,

> > +                                requested_time, remaining);

> > +

> > +  if (ret_64 == 0 || errno != ENOSYS)

> > +    return ret_64;

> > +# endif /* __NR_clock_nanosleep_time64 */

> > +  int ret;

> > +  struct timespec ts32, tr32;

> > +

> > +  if (! in_time_t_range (requested_time->tv_sec))

> > +    {

> > +      __set_errno (EOVERFLOW);

> > +      return -1;

> > +    }

> > +

> > +  ts32 = valid_timespec64_to_timespec (*requested_time);

> > +  ret = INLINE_SYSCALL_CALL (nanosleep, &ts32, &tr32);

> > +

> > +  if (ret == 0 || errno != ENOSYS)

> > +    *remaining = valid_timespec_to_timespec64 (tr32);

> > +

> > +  return ret;

> > +#endif /* __ASSUME_TIME64_SYSCALLS */

> > +}

> > +

> > +#if __TIMESIZE != 64

> >  int

> >  __nanosleep_nocancel (const struct timespec *requested_time,

> > -                   struct timespec *remaining)

> > +                      struct timespec *remaining)

> >  {

> > -  return INLINE_SYSCALL_CALL (nanosleep, requested_time, remaining);

> > +  int ret;

> > +  struct __timespec64 treq64, trem64;

> > +

> > +  treq64 = valid_timespec_to_timespec64 (*requested_time);

> > +  ret = __nanosleep_nocancel_time64 (&treq64, &trem64);

> > +

> > +  if (ret == 0 || errno != ENOSYS)

> > +    {

> > +      if (! in_time_t_range (trem64.tv_sec))

> > +        {

> > +          __set_errno (EOVERFLOW);

> > +          return -1;

> > +        }

> > +

> > +      if (remaining)

> > +        *remaining = valid_timespec64_to_timespec (trem64);

> > +    }

> > +

> > +  return ret;

> >  }

> > +#endif

> > +

> >  hidden_def (__nanosleep_nocancel)

> >
Adhemerval Zanella Oct. 29, 2019, 8:15 p.m. | #5
On 24/10/2019 20:41, Alistair Francis wrote:
> diff --git a/sysdeps/unix/sysv/linux/nanosleep.c b/sysdeps/unix/sysv/linux/nanosleep.c

> index 6787909248f..10a6d57a1de 100644

> --- a/sysdeps/unix/sysv/linux/nanosleep.c

> +++ b/sysdeps/unix/sysv/linux/nanosleep.c

> @@ -17,15 +17,76 @@

>     <https://www.gnu.org/licenses/>.  */

>  

>  #include <time.h>

> +#include <kernel-features.h>

>  #include <sysdep-cancel.h>

>  #include <not-cancel.h>

>  

>  /* Pause execution for a number of nanoseconds.  */

>  int

> +__nanosleep_time64 (const struct __timespec64 *requested_time,

> +                    struct __timespec64 *remaining)

> +{

> +#if defined(__ASSUME_TIME64_SYSCALLS)

> +# ifndef __NR_clock_nanosleep_time64

> +#  define __NR_clock_nanosleep_time64 __NR_clock_nanosleep

> +# endif

> +  return SYSCALL_CANCEL (clock_nanosleep_time64, CLOCK_REALTIME, 0,

> +                         requested_time, remaining);

> +#else

> +# ifdef __NR_clock_nanosleep_time64

> +  long int ret_64;

> +

> +  ret_64 = SYSCALL_CANCEL (clock_nanosleep_time64, CLOCK_REALTIME, 0,

> +                           requested_time, remaining);

> +

> +  if (ret_64 == 0 || errno != ENOSYS)

> +    return ret_64;

> +# endif /* __NR_clock_nanosleep_time64 */

> +  int ret;

> +  struct timespec ts32, tr32;

> +

> +  if (! in_time_t_range (requested_time->tv_sec))

> +    {

> +      __set_errno (EOVERFLOW);

> +      return -1;

> +    }

> +

> +  ts32 = valid_timespec64_to_timespec (*requested_time);

> +  ret = SYSCALL_CANCEL (nanosleep, &ts32, &tr32);

> +

> +  if ((ret == 0 || errno != ENOSYS) && remaining)

> +    *remaining = valid_timespec_to_timespec64 (tr32);

> +

> +  return ret;

> +#endif /* __ASSUME_TIME64_SYSCALLS */

> +}

> +

> +#if __TIMESIZE != 64

> +int

>  __nanosleep (const struct timespec *requested_time,

> -	     struct timespec *remaining)

> +             struct timespec *remaining)

>  {

> -  return SYSCALL_CANCEL (nanosleep, requested_time, remaining);

> +  int r;

> +  struct __timespec64 treq64, trem64;

> +

> +  treq64 = valid_timespec_to_timespec64 (*requested_time);

> +  r = __nanosleep_time64 (&treq64, &trem64);

> +

> +  if (r == 0 || errno != ENOSYS)

> +    {

> +      if (! in_time_t_range (trem64.tv_sec))

> +        {

> +          __set_errno (EOVERFLOW);

> +          return -1;

> +        }

> +

> +      if (remaining)

> +        *remaining = valid_timespec64_to_timespec (trem64);

> +    }

> +

> +  return r;

>  }

> +#endif

> +

>  hidden_def (__nanosleep)

>  weak_alias (__nanosleep, nanosleep)


There is no need to replicate all the syscall logic, nanosleep can be implemented
with __clock_nanosleep.  You can do:

int
__nanosleep (const struct timespec *requested_time,
             struct timespec *remaining)
{
  int ret = __clock_nanosleep (CLOCK_REALTIME, 0, requested_time, remaining);
  if (ret != 0)
    {
      __set_errno (-ret);
      return -1;
    }
  return ret;
}

I think we can even make it the default version, thus removing the linux
specific file.


> diff --git a/sysdeps/unix/sysv/linux/nanosleep_nocancel.c b/sysdeps/unix/sysv/linux/nanosleep_nocancel.c

> index d6442bf4f7f..1a6c6c0a48a 100644

> --- a/sysdeps/unix/sysv/linux/nanosleep_nocancel.c

> +++ b/sysdeps/unix/sysv/linux/nanosleep_nocancel.c

> @@ -17,13 +17,74 @@

>     <https://www.gnu.org/licenses/>.  */

>  

>  #include <time.h>

> +#include <kernel-features.h>

>  #include <sysdep-cancel.h>

>  #include <not-cancel.h>

>  

> +int

> +__nanosleep_nocancel_time64 (const struct __timespec64 *requested_time,

> +                             struct __timespec64 *remaining)

> +{

> +#ifdef __ASSUME_TIME64_SYSCALLS

> +# ifndef __NR_clock_nanosleep_time64

> +#  define __NR_clock_nanosleep_time64 __NR_clock_nanosleep

> +# endif

> +  return INLINE_SYSCALL_CALL (clock_nanosleep_time64, CLOCK_REALTIME, 0,

> +                              requested_time, remaining);

> +#else

> +# ifdef __NR_clock_nanosleep_time64

> +  long int ret_64;

> +

> +  ret_64 = INLINE_SYSCALL_CALL (clock_nanosleep_time64, CLOCK_REALTIME, 0,

> +                                requested_time, remaining);

> +

> +  if (ret_64 == 0 || errno != ENOSYS)

> +    return ret_64;

> +# endif /* __NR_clock_nanosleep_time64 */

> +  int ret;

> +  struct timespec ts32, tr32;

> +

> +  if (! in_time_t_range (requested_time->tv_sec))

> +    {

> +      __set_errno (EOVERFLOW);

> +      return -1;

> +    }

> +

> +  ts32 = valid_timespec64_to_timespec (*requested_time);

> +  ret = INLINE_SYSCALL_CALL (nanosleep, &ts32, &tr32);

> +

> +  if (ret == 0 || errno != ENOSYS)

> +    *remaining = valid_timespec_to_timespec64 (tr32);

> +

> +  return ret;

> +#endif /* __ASSUME_TIME64_SYSCALLS */

> +}

> +

> +#if __TIMESIZE != 64

>  int

>  __nanosleep_nocancel (const struct timespec *requested_time,

> -		      struct timespec *remaining)

> +                      struct timespec *remaining)

>  {

> -  return INLINE_SYSCALL_CALL (nanosleep, requested_time, remaining);

> +  int ret;

> +  struct __timespec64 treq64, trem64;

> +

> +  treq64 = valid_timespec_to_timespec64 (*requested_time);

> +  ret = __nanosleep_nocancel_time64 (&treq64, &trem64);

> +

> +  if (ret == 0 || errno != ENOSYS)

> +    {

> +      if (! in_time_t_range (trem64.tv_sec))

> +        {

> +          __set_errno (EOVERFLOW);

> +          return -1;

> +        }

> +

> +      if (remaining)

> +        *remaining = valid_timespec64_to_timespec (trem64);

> +    }

> +

> +  return ret;

>  }

> +#endif

> +

>  hidden_def (__nanosleep_nocancel)

> 


The __nanosleep_nocancel is just used priority mutex for pthread_mutex_timedlock,
and I am almost sure that we can replace the code path with __lll_clocklock_wait.
Something like the below.

If you like I can sent it as a cleanup patch, since it would require some more
work to remove the __nanosleep_nocancel internal definitions as well.

--

diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c
index d6f0d9e31a..b9536ddb19 100644
--- a/nptl/pthread_mutex_timedlock.c
+++ b/nptl/pthread_mutex_timedlock.c
@@ -25,6 +25,7 @@
 #include <atomic.h>
 #include <lowlevellock.h>
 #include <not-cancel.h>
+#include <futex-internal.h>
 
 #include <stap-probe.h>
 
@@ -377,50 +378,29 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,
 	    int private = (robust
 			   ? PTHREAD_ROBUST_MUTEX_PSHARED (mutex)
 			   : PTHREAD_MUTEX_PSHARED (mutex));
-	    INTERNAL_SYSCALL_DECL (__err);
-
-	    int e = INTERNAL_SYSCALL (futex, __err, 4, &mutex->__data.__lock,
-				      __lll_private_flag (FUTEX_LOCK_PI,
-							  private), 1,
-				      abstime);
-	    if (INTERNAL_SYSCALL_ERROR_P (e, __err))
+	    int e = futex_lock_pi ((unsigned int *) &mutex->__data.__lock, 1,
+				   abstime, private);
+	    if (e == ETIMEDOUT)
 	      {
-		if (INTERNAL_SYSCALL_ERRNO (e, __err) == ETIMEDOUT)
-		  return ETIMEDOUT;
-
-		if (INTERNAL_SYSCALL_ERRNO (e, __err) == ESRCH
-		    || INTERNAL_SYSCALL_ERRNO (e, __err) == EDEADLK)
+	        return ETIMEDOUT;
+	      }
+	    else if (e == ESRCH || e == EDEADLK)
+	      {
+		assert (e != EDEADLK
+			|| (kind != PTHREAD_MUTEX_ERRORCHECK_NP
+			   && kind != PTHREAD_MUTEX_RECURSIVE_NP));
+		/* ESRCH can happen only for non-robust PI mutexes where
+		   the owner of the lock died.  */
+		assert (e != ESRCH || !robust);
+
+		/* Delay the thread until the timeout is reached. Then return
+		   ETIMEDOUT.  */
+		do
 		  {
-		    assert (INTERNAL_SYSCALL_ERRNO (e, __err) != EDEADLK
-			    || (kind != PTHREAD_MUTEX_ERRORCHECK_NP
-				&& kind != PTHREAD_MUTEX_RECURSIVE_NP));
-		    /* ESRCH can happen only for non-robust PI mutexes where
-		       the owner of the lock died.  */
-		    assert (INTERNAL_SYSCALL_ERRNO (e, __err) != ESRCH
-			    || !robust);
-
-		    /* Delay the thread until the timeout is reached.
-		       Then return ETIMEDOUT.  */
-		    struct timespec reltime;
-		    struct timespec now;
-
-		    INTERNAL_SYSCALL (clock_gettime, __err, 2, clockid,
-				      &now);
-		    reltime.tv_sec = abstime->tv_sec - now.tv_sec;
-		    reltime.tv_nsec = abstime->tv_nsec - now.tv_nsec;
-		    if (reltime.tv_nsec < 0)
-		      {
-			reltime.tv_nsec += 1000000000;
-			--reltime.tv_sec;
-		      }
-		    if (reltime.tv_sec >= 0)
-		      while (__nanosleep_nocancel (&reltime, &reltime) != 0)
-			continue;
-
-		    return ETIMEDOUT;
-		  }
-
-		return INTERNAL_SYSCALL_ERRNO (e, __err);
+		    e = __lll_clocklock_wait (&(int){0}, CLOCK_REALTIME,
+					      abstime, private);
+		  } while (e != ETIMEDOUT);
+		return e;
 	      }
 
 	    oldval = mutex->__data.__lock;
diff --git a/sysdeps/unix/sysv/linux/futex-internal.h b/sysdeps/unix/sysv/linux/futex-internal.h
index 5a4f4ff818..1442df63a1 100644
--- a/sysdeps/unix/sysv/linux/futex-internal.h
+++ b/sysdeps/unix/sysv/linux/futex-internal.h
@@ -252,4 +252,30 @@ futex_wake (unsigned int *futex_word, int processes_to_wake, int private)
     }
 }
 
+static __always_inline int
+futex_lock_pi (unsigned int *futex_word, unsigned int expected,
+	       const struct timespec *abstime, int private)
+{
+  int err = lll_futex_timed_lock_pi (futex_word, expected, abstime, private);
+  switch (err)
+    {
+    case 0:
+    case -EAGAIN:
+    case -EINTR:
+    case -ETIMEDOUT:
+    case -ESRCH:
+    case -EDEADLK:
+      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 ();
+    }
+}
+
 #endif  /* futex-internal.h */
diff --git a/sysdeps/unix/sysv/linux/lowlevellock-futex.h b/sysdeps/unix/sysv/linux/lowlevellock-futex.h
index b423673ed4..5730eb2ba2 100644
--- a/sysdeps/unix/sysv/linux/lowlevellock-futex.h
+++ b/sysdeps/unix/sysv/linux/lowlevellock-futex.h
@@ -127,6 +127,11 @@
 		     FUTEX_OP_CLEAR_WAKE_IF_GT_ONE)
 
 /* Priority Inheritance support.  */
+#define lll_futex_timed_lock_pi(futexp, val, reltime, private) 		\
+  lll_futex_syscall (4, futexp,						\
+		     __lll_private_flag (FUTEX_LOCK_PI, private),	\
+		     val, reltime)
+
 #define lll_futex_wait_requeue_pi(futexp, val, mutex, private) \
   lll_futex_timed_wait_requeue_pi (futexp, val, NULL, 0, mutex, private)
Florian Weimer Oct. 30, 2019, 2:06 p.m. | #6
* Adhemerval Zanella:

> There is no need to replicate all the syscall logic, nanosleep can be implemented

> with __clock_nanosleep.  You can do:

>

> int

> __nanosleep (const struct timespec *requested_time,

>              struct timespec *remaining)

> {

>   int ret = __clock_nanosleep (CLOCK_REALTIME, 0, requested_time, remaining);

>   if (ret != 0)

>     {

>       __set_errno (-ret);

>       return -1;

>     }

>   return ret;

> }


Why the -ret?  I think it should be a plain ret.

Thanks,
Florian
Adhemerval Zanella Oct. 30, 2019, 2:34 p.m. | #7
On 30/10/2019 11:06, Florian Weimer wrote:
> * Adhemerval Zanella:

> 

>> There is no need to replicate all the syscall logic, nanosleep can be implemented

>> with __clock_nanosleep.  You can do:

>>

>> int

>> __nanosleep (const struct timespec *requested_time,

>>              struct timespec *remaining)

>> {

>>   int ret = __clock_nanosleep (CLOCK_REALTIME, 0, requested_time, remaining);

>>   if (ret != 0)

>>     {

>>       __set_errno (-ret);

>>       return -1;

>>     }

>>   return ret;

>> }

> 

> Why the -ret?  I think it should be a plain ret.


Indeed, my mistake here.
Alistair Francis Oct. 31, 2019, 10:21 a.m. | #8
On Tue, Oct 29, 2019 at 9:16 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>

>

>

> On 24/10/2019 20:41, Alistair Francis wrote:

> > diff --git a/sysdeps/unix/sysv/linux/nanosleep.c b/sysdeps/unix/sysv/linux/nanosleep.c

> > index 6787909248f..10a6d57a1de 100644

> > --- a/sysdeps/unix/sysv/linux/nanosleep.c

> > +++ b/sysdeps/unix/sysv/linux/nanosleep.c

> > @@ -17,15 +17,76 @@

> >     <https://www.gnu.org/licenses/>.  */

> >

> >  #include <time.h>

> > +#include <kernel-features.h>

> >  #include <sysdep-cancel.h>

> >  #include <not-cancel.h>

> >

> >  /* Pause execution for a number of nanoseconds.  */

> >  int

> > +__nanosleep_time64 (const struct __timespec64 *requested_time,

> > +                    struct __timespec64 *remaining)

> > +{

> > +#if defined(__ASSUME_TIME64_SYSCALLS)

> > +# ifndef __NR_clock_nanosleep_time64

> > +#  define __NR_clock_nanosleep_time64 __NR_clock_nanosleep

> > +# endif

> > +  return SYSCALL_CANCEL (clock_nanosleep_time64, CLOCK_REALTIME, 0,

> > +                         requested_time, remaining);

> > +#else

> > +# ifdef __NR_clock_nanosleep_time64

> > +  long int ret_64;

> > +

> > +  ret_64 = SYSCALL_CANCEL (clock_nanosleep_time64, CLOCK_REALTIME, 0,

> > +                           requested_time, remaining);

> > +

> > +  if (ret_64 == 0 || errno != ENOSYS)

> > +    return ret_64;

> > +# endif /* __NR_clock_nanosleep_time64 */

> > +  int ret;

> > +  struct timespec ts32, tr32;

> > +

> > +  if (! in_time_t_range (requested_time->tv_sec))

> > +    {

> > +      __set_errno (EOVERFLOW);

> > +      return -1;

> > +    }

> > +

> > +  ts32 = valid_timespec64_to_timespec (*requested_time);

> > +  ret = SYSCALL_CANCEL (nanosleep, &ts32, &tr32);

> > +

> > +  if ((ret == 0 || errno != ENOSYS) && remaining)

> > +    *remaining = valid_timespec_to_timespec64 (tr32);

> > +

> > +  return ret;

> > +#endif /* __ASSUME_TIME64_SYSCALLS */

> > +}

> > +

> > +#if __TIMESIZE != 64

> > +int

> >  __nanosleep (const struct timespec *requested_time,

> > -          struct timespec *remaining)

> > +             struct timespec *remaining)

> >  {

> > -  return SYSCALL_CANCEL (nanosleep, requested_time, remaining);

> > +  int r;

> > +  struct __timespec64 treq64, trem64;

> > +

> > +  treq64 = valid_timespec_to_timespec64 (*requested_time);

> > +  r = __nanosleep_time64 (&treq64, &trem64);

> > +

> > +  if (r == 0 || errno != ENOSYS)

> > +    {

> > +      if (! in_time_t_range (trem64.tv_sec))

> > +        {

> > +          __set_errno (EOVERFLOW);

> > +          return -1;

> > +        }

> > +

> > +      if (remaining)

> > +        *remaining = valid_timespec64_to_timespec (trem64);

> > +    }

> > +

> > +  return r;

> >  }

> > +#endif

> > +

> >  hidden_def (__nanosleep)

> >  weak_alias (__nanosleep, nanosleep)

>

> There is no need to replicate all the syscall logic, nanosleep can be implemented

> with __clock_nanosleep.  You can do:

>

> int

> __nanosleep (const struct timespec *requested_time,

>              struct timespec *remaining)

> {

>   int ret = __clock_nanosleep (CLOCK_REALTIME, 0, requested_time, remaining);

>   if (ret != 0)

>     {

>       __set_errno (-ret);

>       return -1;

>     }

>   return ret;

> }

>

> I think we can even make it the default version, thus removing the linux

> specific file.


Ok, testing now then I'll send the patch.

>

>

> > diff --git a/sysdeps/unix/sysv/linux/nanosleep_nocancel.c b/sysdeps/unix/sysv/linux/nanosleep_nocancel.c

> > index d6442bf4f7f..1a6c6c0a48a 100644

> > --- a/sysdeps/unix/sysv/linux/nanosleep_nocancel.c

> > +++ b/sysdeps/unix/sysv/linux/nanosleep_nocancel.c

> > @@ -17,13 +17,74 @@

> >     <https://www.gnu.org/licenses/>.  */

> >

> >  #include <time.h>

> > +#include <kernel-features.h>

> >  #include <sysdep-cancel.h>

> >  #include <not-cancel.h>

> >

> > +int

> > +__nanosleep_nocancel_time64 (const struct __timespec64 *requested_time,

> > +                             struct __timespec64 *remaining)

> > +{

> > +#ifdef __ASSUME_TIME64_SYSCALLS

> > +# ifndef __NR_clock_nanosleep_time64

> > +#  define __NR_clock_nanosleep_time64 __NR_clock_nanosleep

> > +# endif

> > +  return INLINE_SYSCALL_CALL (clock_nanosleep_time64, CLOCK_REALTIME, 0,

> > +                              requested_time, remaining);

> > +#else

> > +# ifdef __NR_clock_nanosleep_time64

> > +  long int ret_64;

> > +

> > +  ret_64 = INLINE_SYSCALL_CALL (clock_nanosleep_time64, CLOCK_REALTIME, 0,

> > +                                requested_time, remaining);

> > +

> > +  if (ret_64 == 0 || errno != ENOSYS)

> > +    return ret_64;

> > +# endif /* __NR_clock_nanosleep_time64 */

> > +  int ret;

> > +  struct timespec ts32, tr32;

> > +

> > +  if (! in_time_t_range (requested_time->tv_sec))

> > +    {

> > +      __set_errno (EOVERFLOW);

> > +      return -1;

> > +    }

> > +

> > +  ts32 = valid_timespec64_to_timespec (*requested_time);

> > +  ret = INLINE_SYSCALL_CALL (nanosleep, &ts32, &tr32);

> > +

> > +  if (ret == 0 || errno != ENOSYS)

> > +    *remaining = valid_timespec_to_timespec64 (tr32);

> > +

> > +  return ret;

> > +#endif /* __ASSUME_TIME64_SYSCALLS */

> > +}

> > +

> > +#if __TIMESIZE != 64

> >  int

> >  __nanosleep_nocancel (const struct timespec *requested_time,

> > -                   struct timespec *remaining)

> > +                      struct timespec *remaining)

> >  {

> > -  return INLINE_SYSCALL_CALL (nanosleep, requested_time, remaining);

> > +  int ret;

> > +  struct __timespec64 treq64, trem64;

> > +

> > +  treq64 = valid_timespec_to_timespec64 (*requested_time);

> > +  ret = __nanosleep_nocancel_time64 (&treq64, &trem64);

> > +

> > +  if (ret == 0 || errno != ENOSYS)

> > +    {

> > +      if (! in_time_t_range (trem64.tv_sec))

> > +        {

> > +          __set_errno (EOVERFLOW);

> > +          return -1;

> > +        }

> > +

> > +      if (remaining)

> > +        *remaining = valid_timespec64_to_timespec (trem64);

> > +    }

> > +

> > +  return ret;

> >  }

> > +#endif

> > +

> >  hidden_def (__nanosleep_nocancel)

> >

>

> The __nanosleep_nocancel is just used priority mutex for pthread_mutex_timedlock,

> and I am almost sure that we can replace the code path with __lll_clocklock_wait.

> Something like the below.

>

> If you like I can sent it as a cleanup patch, since it would require some more

> work to remove the __nanosleep_nocancel internal definitions as well.


I would prefer that going in seperatley via a cleanup patch.

Alistair

>

> --

>

> diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c

> index d6f0d9e31a..b9536ddb19 100644

> --- a/nptl/pthread_mutex_timedlock.c

> +++ b/nptl/pthread_mutex_timedlock.c

> @@ -25,6 +25,7 @@

>  #include <atomic.h>

>  #include <lowlevellock.h>

>  #include <not-cancel.h>

> +#include <futex-internal.h>

>

>  #include <stap-probe.h>

>

> @@ -377,50 +378,29 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,

>             int private = (robust

>                            ? PTHREAD_ROBUST_MUTEX_PSHARED (mutex)

>                            : PTHREAD_MUTEX_PSHARED (mutex));

> -           INTERNAL_SYSCALL_DECL (__err);

> -

> -           int e = INTERNAL_SYSCALL (futex, __err, 4, &mutex->__data.__lock,

> -                                     __lll_private_flag (FUTEX_LOCK_PI,

> -                                                         private), 1,

> -                                     abstime);

> -           if (INTERNAL_SYSCALL_ERROR_P (e, __err))

> +           int e = futex_lock_pi ((unsigned int *) &mutex->__data.__lock, 1,

> +                                  abstime, private);

> +           if (e == ETIMEDOUT)

>               {

> -               if (INTERNAL_SYSCALL_ERRNO (e, __err) == ETIMEDOUT)

> -                 return ETIMEDOUT;

> -

> -               if (INTERNAL_SYSCALL_ERRNO (e, __err) == ESRCH

> -                   || INTERNAL_SYSCALL_ERRNO (e, __err) == EDEADLK)

> +               return ETIMEDOUT;

> +             }

> +           else if (e == ESRCH || e == EDEADLK)

> +             {

> +               assert (e != EDEADLK

> +                       || (kind != PTHREAD_MUTEX_ERRORCHECK_NP

> +                          && kind != PTHREAD_MUTEX_RECURSIVE_NP));

> +               /* ESRCH can happen only for non-robust PI mutexes where

> +                  the owner of the lock died.  */

> +               assert (e != ESRCH || !robust);

> +

> +               /* Delay the thread until the timeout is reached. Then return

> +                  ETIMEDOUT.  */

> +               do

>                   {

> -                   assert (INTERNAL_SYSCALL_ERRNO (e, __err) != EDEADLK

> -                           || (kind != PTHREAD_MUTEX_ERRORCHECK_NP

> -                               && kind != PTHREAD_MUTEX_RECURSIVE_NP));

> -                   /* ESRCH can happen only for non-robust PI mutexes where

> -                      the owner of the lock died.  */

> -                   assert (INTERNAL_SYSCALL_ERRNO (e, __err) != ESRCH

> -                           || !robust);

> -

> -                   /* Delay the thread until the timeout is reached.

> -                      Then return ETIMEDOUT.  */

> -                   struct timespec reltime;

> -                   struct timespec now;

> -

> -                   INTERNAL_SYSCALL (clock_gettime, __err, 2, clockid,

> -                                     &now);

> -                   reltime.tv_sec = abstime->tv_sec - now.tv_sec;

> -                   reltime.tv_nsec = abstime->tv_nsec - now.tv_nsec;

> -                   if (reltime.tv_nsec < 0)

> -                     {

> -                       reltime.tv_nsec += 1000000000;

> -                       --reltime.tv_sec;

> -                     }

> -                   if (reltime.tv_sec >= 0)

> -                     while (__nanosleep_nocancel (&reltime, &reltime) != 0)

> -                       continue;

> -

> -                   return ETIMEDOUT;

> -                 }

> -

> -               return INTERNAL_SYSCALL_ERRNO (e, __err);

> +                   e = __lll_clocklock_wait (&(int){0}, CLOCK_REALTIME,

> +                                             abstime, private);

> +                 } while (e != ETIMEDOUT);

> +               return e;

>               }

>

>             oldval = mutex->__data.__lock;

> diff --git a/sysdeps/unix/sysv/linux/futex-internal.h b/sysdeps/unix/sysv/linux/futex-internal.h

> index 5a4f4ff818..1442df63a1 100644

> --- a/sysdeps/unix/sysv/linux/futex-internal.h

> +++ b/sysdeps/unix/sysv/linux/futex-internal.h

> @@ -252,4 +252,30 @@ futex_wake (unsigned int *futex_word, int processes_to_wake, int private)

>      }

>  }

>

> +static __always_inline int

> +futex_lock_pi (unsigned int *futex_word, unsigned int expected,

> +              const struct timespec *abstime, int private)

> +{

> +  int err = lll_futex_timed_lock_pi (futex_word, expected, abstime, private);

> +  switch (err)

> +    {

> +    case 0:

> +    case -EAGAIN:

> +    case -EINTR:

> +    case -ETIMEDOUT:

> +    case -ESRCH:

> +    case -EDEADLK:

> +      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 ();

> +    }

> +}

> +

>  #endif  /* futex-internal.h */

> diff --git a/sysdeps/unix/sysv/linux/lowlevellock-futex.h b/sysdeps/unix/sysv/linux/lowlevellock-futex.h

> index b423673ed4..5730eb2ba2 100644

> --- a/sysdeps/unix/sysv/linux/lowlevellock-futex.h

> +++ b/sysdeps/unix/sysv/linux/lowlevellock-futex.h

> @@ -127,6 +127,11 @@

>                      FUTEX_OP_CLEAR_WAKE_IF_GT_ONE)

>

>  /* Priority Inheritance support.  */

> +#define lll_futex_timed_lock_pi(futexp, val, reltime, private)                 \

> +  lll_futex_syscall (4, futexp,                                                \

> +                    __lll_private_flag (FUTEX_LOCK_PI, private),       \

> +                    val, reltime)

> +

>  #define lll_futex_wait_requeue_pi(futexp, val, mutex, private) \

>    lll_futex_timed_wait_requeue_pi (futexp, val, NULL, 0, mutex, private)

>
Alistair Francis Nov. 4, 2019, 6:16 p.m. | #9
On Tue, Oct 29, 2019 at 1:16 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>

>

>

> On 24/10/2019 20:41, Alistair Francis wrote:

> > diff --git a/sysdeps/unix/sysv/linux/nanosleep.c b/sysdeps/unix/sysv/linux/nanosleep.c

> > index 6787909248f..10a6d57a1de 100644

> > --- a/sysdeps/unix/sysv/linux/nanosleep.c

> > +++ b/sysdeps/unix/sysv/linux/nanosleep.c

> > @@ -17,15 +17,76 @@

> >     <https://www.gnu.org/licenses/>.  */

> >

> >  #include <time.h>

> > +#include <kernel-features.h>

> >  #include <sysdep-cancel.h>

> >  #include <not-cancel.h>

> >

> >  /* Pause execution for a number of nanoseconds.  */

> >  int

> > +__nanosleep_time64 (const struct __timespec64 *requested_time,

> > +                    struct __timespec64 *remaining)

> > +{

> > +#if defined(__ASSUME_TIME64_SYSCALLS)

> > +# ifndef __NR_clock_nanosleep_time64

> > +#  define __NR_clock_nanosleep_time64 __NR_clock_nanosleep

> > +# endif

> > +  return SYSCALL_CANCEL (clock_nanosleep_time64, CLOCK_REALTIME, 0,

> > +                         requested_time, remaining);

> > +#else

> > +# ifdef __NR_clock_nanosleep_time64

> > +  long int ret_64;

> > +

> > +  ret_64 = SYSCALL_CANCEL (clock_nanosleep_time64, CLOCK_REALTIME, 0,

> > +                           requested_time, remaining);

> > +

> > +  if (ret_64 == 0 || errno != ENOSYS)

> > +    return ret_64;

> > +# endif /* __NR_clock_nanosleep_time64 */

> > +  int ret;

> > +  struct timespec ts32, tr32;

> > +

> > +  if (! in_time_t_range (requested_time->tv_sec))

> > +    {

> > +      __set_errno (EOVERFLOW);

> > +      return -1;

> > +    }

> > +

> > +  ts32 = valid_timespec64_to_timespec (*requested_time);

> > +  ret = SYSCALL_CANCEL (nanosleep, &ts32, &tr32);

> > +

> > +  if ((ret == 0 || errno != ENOSYS) && remaining)

> > +    *remaining = valid_timespec_to_timespec64 (tr32);

> > +

> > +  return ret;

> > +#endif /* __ASSUME_TIME64_SYSCALLS */

> > +}

> > +

> > +#if __TIMESIZE != 64

> > +int

> >  __nanosleep (const struct timespec *requested_time,

> > -          struct timespec *remaining)

> > +             struct timespec *remaining)

> >  {

> > -  return SYSCALL_CANCEL (nanosleep, requested_time, remaining);

> > +  int r;

> > +  struct __timespec64 treq64, trem64;

> > +

> > +  treq64 = valid_timespec_to_timespec64 (*requested_time);

> > +  r = __nanosleep_time64 (&treq64, &trem64);

> > +

> > +  if (r == 0 || errno != ENOSYS)

> > +    {

> > +      if (! in_time_t_range (trem64.tv_sec))

> > +        {

> > +          __set_errno (EOVERFLOW);

> > +          return -1;

> > +        }

> > +

> > +      if (remaining)

> > +        *remaining = valid_timespec64_to_timespec (trem64);

> > +    }

> > +

> > +  return r;

> >  }

> > +#endif

> > +

> >  hidden_def (__nanosleep)

> >  weak_alias (__nanosleep, nanosleep)

>

> There is no need to replicate all the syscall logic, nanosleep can be implemented

> with __clock_nanosleep.  You can do:

>

> int

> __nanosleep (const struct timespec *requested_time,

>              struct timespec *remaining)

> {

>   int ret = __clock_nanosleep (CLOCK_REALTIME, 0, requested_time, remaining);

>   if (ret != 0)

>     {

>       __set_errno (-ret);

>       return -1;

>     }

>   return ret;

> }


This doesn't work as __clock_nanosleep() isn't avaliable in nptl so it
fails to compile. My v3 patch attempted to fix this, but that also
doesn't work and ends up squishing some patches together.

Alistair

>

> I think we can even make it the default version, thus removing the linux

> specific file.

>

>

> > diff --git a/sysdeps/unix/sysv/linux/nanosleep_nocancel.c b/sysdeps/unix/sysv/linux/nanosleep_nocancel.c

> > index d6442bf4f7f..1a6c6c0a48a 100644

> > --- a/sysdeps/unix/sysv/linux/nanosleep_nocancel.c

> > +++ b/sysdeps/unix/sysv/linux/nanosleep_nocancel.c

> > @@ -17,13 +17,74 @@

> >     <https://www.gnu.org/licenses/>.  */

> >

> >  #include <time.h>

> > +#include <kernel-features.h>

> >  #include <sysdep-cancel.h>

> >  #include <not-cancel.h>

> >

> > +int

> > +__nanosleep_nocancel_time64 (const struct __timespec64 *requested_time,

> > +                             struct __timespec64 *remaining)

> > +{

> > +#ifdef __ASSUME_TIME64_SYSCALLS

> > +# ifndef __NR_clock_nanosleep_time64

> > +#  define __NR_clock_nanosleep_time64 __NR_clock_nanosleep

> > +# endif

> > +  return INLINE_SYSCALL_CALL (clock_nanosleep_time64, CLOCK_REALTIME, 0,

> > +                              requested_time, remaining);

> > +#else

> > +# ifdef __NR_clock_nanosleep_time64

> > +  long int ret_64;

> > +

> > +  ret_64 = INLINE_SYSCALL_CALL (clock_nanosleep_time64, CLOCK_REALTIME, 0,

> > +                                requested_time, remaining);

> > +

> > +  if (ret_64 == 0 || errno != ENOSYS)

> > +    return ret_64;

> > +# endif /* __NR_clock_nanosleep_time64 */

> > +  int ret;

> > +  struct timespec ts32, tr32;

> > +

> > +  if (! in_time_t_range (requested_time->tv_sec))

> > +    {

> > +      __set_errno (EOVERFLOW);

> > +      return -1;

> > +    }

> > +

> > +  ts32 = valid_timespec64_to_timespec (*requested_time);

> > +  ret = INLINE_SYSCALL_CALL (nanosleep, &ts32, &tr32);

> > +

> > +  if (ret == 0 || errno != ENOSYS)

> > +    *remaining = valid_timespec_to_timespec64 (tr32);

> > +

> > +  return ret;

> > +#endif /* __ASSUME_TIME64_SYSCALLS */

> > +}

> > +

> > +#if __TIMESIZE != 64

> >  int

> >  __nanosleep_nocancel (const struct timespec *requested_time,

> > -                   struct timespec *remaining)

> > +                      struct timespec *remaining)

> >  {

> > -  return INLINE_SYSCALL_CALL (nanosleep, requested_time, remaining);

> > +  int ret;

> > +  struct __timespec64 treq64, trem64;

> > +

> > +  treq64 = valid_timespec_to_timespec64 (*requested_time);

> > +  ret = __nanosleep_nocancel_time64 (&treq64, &trem64);

> > +

> > +  if (ret == 0 || errno != ENOSYS)

> > +    {

> > +      if (! in_time_t_range (trem64.tv_sec))

> > +        {

> > +          __set_errno (EOVERFLOW);

> > +          return -1;

> > +        }

> > +

> > +      if (remaining)

> > +        *remaining = valid_timespec64_to_timespec (trem64);

> > +    }

> > +

> > +  return ret;

> >  }

> > +#endif

> > +

> >  hidden_def (__nanosleep_nocancel)

> >

>

> The __nanosleep_nocancel is just used priority mutex for pthread_mutex_timedlock,

> and I am almost sure that we can replace the code path with __lll_clocklock_wait.

> Something like the below.

>

> If you like I can sent it as a cleanup patch, since it would require some more

> work to remove the __nanosleep_nocancel internal definitions as well.

>

> --

>

> diff --git a/nptl/pthread_mutex_timedlock.c b/nptl/pthread_mutex_timedlock.c

> index d6f0d9e31a..b9536ddb19 100644

> --- a/nptl/pthread_mutex_timedlock.c

> +++ b/nptl/pthread_mutex_timedlock.c

> @@ -25,6 +25,7 @@

>  #include <atomic.h>

>  #include <lowlevellock.h>

>  #include <not-cancel.h>

> +#include <futex-internal.h>

>

>  #include <stap-probe.h>

>

> @@ -377,50 +378,29 @@ __pthread_mutex_clocklock_common (pthread_mutex_t *mutex,

>             int private = (robust

>                            ? PTHREAD_ROBUST_MUTEX_PSHARED (mutex)

>                            : PTHREAD_MUTEX_PSHARED (mutex));

> -           INTERNAL_SYSCALL_DECL (__err);

> -

> -           int e = INTERNAL_SYSCALL (futex, __err, 4, &mutex->__data.__lock,

> -                                     __lll_private_flag (FUTEX_LOCK_PI,

> -                                                         private), 1,

> -                                     abstime);

> -           if (INTERNAL_SYSCALL_ERROR_P (e, __err))

> +           int e = futex_lock_pi ((unsigned int *) &mutex->__data.__lock, 1,

> +                                  abstime, private);

> +           if (e == ETIMEDOUT)

>               {

> -               if (INTERNAL_SYSCALL_ERRNO (e, __err) == ETIMEDOUT)

> -                 return ETIMEDOUT;

> -

> -               if (INTERNAL_SYSCALL_ERRNO (e, __err) == ESRCH

> -                   || INTERNAL_SYSCALL_ERRNO (e, __err) == EDEADLK)

> +               return ETIMEDOUT;

> +             }

> +           else if (e == ESRCH || e == EDEADLK)

> +             {

> +               assert (e != EDEADLK

> +                       || (kind != PTHREAD_MUTEX_ERRORCHECK_NP

> +                          && kind != PTHREAD_MUTEX_RECURSIVE_NP));

> +               /* ESRCH can happen only for non-robust PI mutexes where

> +                  the owner of the lock died.  */

> +               assert (e != ESRCH || !robust);

> +

> +               /* Delay the thread until the timeout is reached. Then return

> +                  ETIMEDOUT.  */

> +               do

>                   {

> -                   assert (INTERNAL_SYSCALL_ERRNO (e, __err) != EDEADLK

> -                           || (kind != PTHREAD_MUTEX_ERRORCHECK_NP

> -                               && kind != PTHREAD_MUTEX_RECURSIVE_NP));

> -                   /* ESRCH can happen only for non-robust PI mutexes where

> -                      the owner of the lock died.  */

> -                   assert (INTERNAL_SYSCALL_ERRNO (e, __err) != ESRCH

> -                           || !robust);

> -

> -                   /* Delay the thread until the timeout is reached.

> -                      Then return ETIMEDOUT.  */

> -                   struct timespec reltime;

> -                   struct timespec now;

> -

> -                   INTERNAL_SYSCALL (clock_gettime, __err, 2, clockid,

> -                                     &now);

> -                   reltime.tv_sec = abstime->tv_sec - now.tv_sec;

> -                   reltime.tv_nsec = abstime->tv_nsec - now.tv_nsec;

> -                   if (reltime.tv_nsec < 0)

> -                     {

> -                       reltime.tv_nsec += 1000000000;

> -                       --reltime.tv_sec;

> -                     }

> -                   if (reltime.tv_sec >= 0)

> -                     while (__nanosleep_nocancel (&reltime, &reltime) != 0)

> -                       continue;

> -

> -                   return ETIMEDOUT;

> -                 }

> -

> -               return INTERNAL_SYSCALL_ERRNO (e, __err);

> +                   e = __lll_clocklock_wait (&(int){0}, CLOCK_REALTIME,

> +                                             abstime, private);

> +                 } while (e != ETIMEDOUT);

> +               return e;

>               }

>

>             oldval = mutex->__data.__lock;

> diff --git a/sysdeps/unix/sysv/linux/futex-internal.h b/sysdeps/unix/sysv/linux/futex-internal.h

> index 5a4f4ff818..1442df63a1 100644

> --- a/sysdeps/unix/sysv/linux/futex-internal.h

> +++ b/sysdeps/unix/sysv/linux/futex-internal.h

> @@ -252,4 +252,30 @@ futex_wake (unsigned int *futex_word, int processes_to_wake, int private)

>      }

>  }

>

> +static __always_inline int

> +futex_lock_pi (unsigned int *futex_word, unsigned int expected,

> +              const struct timespec *abstime, int private)

> +{

> +  int err = lll_futex_timed_lock_pi (futex_word, expected, abstime, private);

> +  switch (err)

> +    {

> +    case 0:

> +    case -EAGAIN:

> +    case -EINTR:

> +    case -ETIMEDOUT:

> +    case -ESRCH:

> +    case -EDEADLK:

> +      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 ();

> +    }

> +}

> +

>  #endif  /* futex-internal.h */

> diff --git a/sysdeps/unix/sysv/linux/lowlevellock-futex.h b/sysdeps/unix/sysv/linux/lowlevellock-futex.h

> index b423673ed4..5730eb2ba2 100644

> --- a/sysdeps/unix/sysv/linux/lowlevellock-futex.h

> +++ b/sysdeps/unix/sysv/linux/lowlevellock-futex.h

> @@ -127,6 +127,11 @@

>                      FUTEX_OP_CLEAR_WAKE_IF_GT_ONE)

>

>  /* Priority Inheritance support.  */

> +#define lll_futex_timed_lock_pi(futexp, val, reltime, private)                 \

> +  lll_futex_syscall (4, futexp,                                                \

> +                    __lll_private_flag (FUTEX_LOCK_PI, private),       \

> +                    val, reltime)

> +

>  #define lll_futex_wait_requeue_pi(futexp, val, mutex, private) \

>    lll_futex_timed_wait_requeue_pi (futexp, val, NULL, 0, mutex, private)

>
Adhemerval Zanella Nov. 4, 2019, 7:30 p.m. | #10
On 04/11/2019 15:16, Alistair Francis wrote:
> On Tue, Oct 29, 2019 at 1:16 PM Adhemerval Zanella

> <adhemerval.zanella@linaro.org> wrote:

>>

>>

>>

>> On 24/10/2019 20:41, Alistair Francis wrote:


>>

>> There is no need to replicate all the syscall logic, nanosleep can be implemented

>> with __clock_nanosleep.  You can do:

>>

>> int

>> __nanosleep (const struct timespec *requested_time,

>>              struct timespec *remaining)

>> {

>>   int ret = __clock_nanosleep (CLOCK_REALTIME, 0, requested_time, remaining);

>>   if (ret != 0)

>>     {

>>       __set_errno (-ret);

>>       return -1;

>>     }

>>   return ret;

>> }

> 

> This doesn't work as __clock_nanosleep() isn't avaliable in nptl so it

> fails to compile. My v3 patch attempted to fix this, but that also

> doesn't work and ends up squishing some patches together.


For this case you will need to add __clock_nanosleep as a GLIBC_PRIVATE
exported symbol from libc and add a hidden_proto/hidden_def for libc 
internal plt avoidance (as below).

However, I agree with Joseph and Florian that for this case it would be
better to move the symbol from libpthread to libc. I will send a patch
to refactor it.

diff --git a/sysdeps/unix/sysv/linux/Versions b/sysdeps/unix/sysv/linux/Versions
index d385085c61..475abb5004 100644
--- a/sysdeps/unix/sysv/linux/Versions
+++ b/sysdeps/unix/sysv/linux/Versions
@@ -187,5 +187,6 @@ libc {
     __sigtimedwait;
     # functions used by nscd
     __netlink_assert_response;
+    __clock_nanosleep;
   }
 }
diff --git a/sysdeps/unix/sysv/linux/clock_nanosleep.c b/sysdeps/unix/sysv/linux/clock_nanosleep.c
index 1f240b8720..f3c6fd2d5f 100644
--- a/sysdeps/unix/sysv/linux/clock_nanosleep.c
+++ b/sysdeps/unix/sysv/linux/clock_nanosleep.c
@@ -42,7 +42,7 @@ __clock_nanosleep (clockid_t clock_id, int flags, const struct timespec *req,
   return (INTERNAL_SYSCALL_ERROR_P (r, err)
          ? INTERNAL_SYSCALL_ERRNO (r, err) : 0);
 }
-
+libc_hidden_def (__clock_nanosleep)
 versioned_symbol (libc, __clock_nanosleep, clock_nanosleep, GLIBC_2_17);
 /* clock_nanosleep moved to libc in version 2.17;
    old binaries may expect the symbol version it had in librt.  */
diff --git a/sysdeps/unix/sysv/linux/nanosleep.c b/sysdeps/unix/sysv/linux/nanosleep.c
index 6787909248..1e2481847b 100644
--- a/sysdeps/unix/sysv/linux/nanosleep.c
+++ b/sysdeps/unix/sysv/linux/nanosleep.c
@@ -25,7 +25,14 @@ int
 __nanosleep (const struct timespec *requested_time,
             struct timespec *remaining)
 {
-  return SYSCALL_CANCEL (nanosleep, requested_time, remaining);
+  //return SYSCALL_CANCEL (nanosleep, requested_time, remaining);
+  int ret = __clock_nanosleep (CLOCK_REALTIME, 0, requested_time, remaining);
+  if (ret != 0)
+    {
+      __set_errno (-ret);
+      return -1;
+    }
+  return ret;
 }
 hidden_def (__nanosleep)
 weak_alias (__nanosleep, nanosleep)
Alistair Francis Nov. 4, 2019, 10:57 p.m. | #11
On Mon, Nov 4, 2019 at 11:30 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>

>

>

> On 04/11/2019 15:16, Alistair Francis wrote:

> > On Tue, Oct 29, 2019 at 1:16 PM Adhemerval Zanella

> > <adhemerval.zanella@linaro.org> wrote:

> >>

> >>

> >>

> >> On 24/10/2019 20:41, Alistair Francis wrote:

>

> >>

> >> There is no need to replicate all the syscall logic, nanosleep can be implemented

> >> with __clock_nanosleep.  You can do:

> >>

> >> int

> >> __nanosleep (const struct timespec *requested_time,

> >>              struct timespec *remaining)

> >> {

> >>   int ret = __clock_nanosleep (CLOCK_REALTIME, 0, requested_time, remaining);

> >>   if (ret != 0)

> >>     {

> >>       __set_errno (-ret);

> >>       return -1;

> >>     }

> >>   return ret;

> >> }

> >

> > This doesn't work as __clock_nanosleep() isn't avaliable in nptl so it

> > fails to compile. My v3 patch attempted to fix this, but that also

> > doesn't work and ends up squishing some patches together.

>

> For this case you will need to add __clock_nanosleep as a GLIBC_PRIVATE

> exported symbol from libc and add a hidden_proto/hidden_def for libc

> internal plt avoidance (as below).


Thanks for explaining this.

>

> However, I agree with Joseph and Florian that for this case it would be

> better to move the symbol from libpthread to libc. I will send a patch

> to refactor it.


I am going to include the diff below and convert __nanosleep to call
__clock_nanosleep(), but I won't change the file structure or move
anything else around. I'll leave that to you in a follow up patch.

Alistair

>

> diff --git a/sysdeps/unix/sysv/linux/Versions b/sysdeps/unix/sysv/linux/Versions

> index d385085c61..475abb5004 100644

> --- a/sysdeps/unix/sysv/linux/Versions

> +++ b/sysdeps/unix/sysv/linux/Versions

> @@ -187,5 +187,6 @@ libc {

>      __sigtimedwait;

>      # functions used by nscd

>      __netlink_assert_response;

> +    __clock_nanosleep;

>    }

>  }

> diff --git a/sysdeps/unix/sysv/linux/clock_nanosleep.c b/sysdeps/unix/sysv/linux/clock_nanosleep.c

> index 1f240b8720..f3c6fd2d5f 100644

> --- a/sysdeps/unix/sysv/linux/clock_nanosleep.c

> +++ b/sysdeps/unix/sysv/linux/clock_nanosleep.c

> @@ -42,7 +42,7 @@ __clock_nanosleep (clockid_t clock_id, int flags, const struct timespec *req,

>    return (INTERNAL_SYSCALL_ERROR_P (r, err)

>           ? INTERNAL_SYSCALL_ERRNO (r, err) : 0);

>  }

> -

> +libc_hidden_def (__clock_nanosleep)

>  versioned_symbol (libc, __clock_nanosleep, clock_nanosleep, GLIBC_2_17);

>  /* clock_nanosleep moved to libc in version 2.17;

>     old binaries may expect the symbol version it had in librt.  */

> diff --git a/sysdeps/unix/sysv/linux/nanosleep.c b/sysdeps/unix/sysv/linux/nanosleep.c

> index 6787909248..1e2481847b 100644

> --- a/sysdeps/unix/sysv/linux/nanosleep.c

> +++ b/sysdeps/unix/sysv/linux/nanosleep.c

> @@ -25,7 +25,14 @@ int

>  __nanosleep (const struct timespec *requested_time,

>              struct timespec *remaining)

>  {

> -  return SYSCALL_CANCEL (nanosleep, requested_time, remaining);

> +  //return SYSCALL_CANCEL (nanosleep, requested_time, remaining);

> +  int ret = __clock_nanosleep (CLOCK_REALTIME, 0, requested_time, remaining);

> +  if (ret != 0)

> +    {

> +      __set_errno (-ret);

> +      return -1;

> +    }

> +  return ret;

>  }

>  hidden_def (__nanosleep)

>  weak_alias (__nanosleep, nanosleep)

Patch

diff --git a/include/time.h b/include/time.h
index d93b16a7810..0e703f87cef 100644
--- a/include/time.h
+++ b/include/time.h
@@ -174,6 +174,26 @@  libc_hidden_proto (__difftime64)
 
 extern double __difftime (time_t time1, time_t time0);
 
+#if __TIMESIZE == 64
+# define __thrd_sleep_time64 thrd_sleep
+# define __clock_nanosleep_time64 __clock_nanosleep
+# define __nanosleep_time64 __nanosleep
+# define __nanosleep_nocancel_time64 __nanosleep_nocancel
+#else
+extern int __thrd_sleep_time64 (const struct __timespec64* time_point,
+                                struct __timespec64* remaining);
+libc_hidden_proto (__thrd_sleep_time64)
+extern int __clock_nanosleep_time64 (clockid_t clock_id,
+                                     int flags, const struct __timespec64 *req,
+                                     struct __timespec64 *rem);
+libc_hidden_proto (__clock_nanosleep_time64)
+extern  int __nanosleep_time64 (const struct __timespec64 *requested_time,
+                                struct __timespec64 *remaining);
+libc_hidden_proto (__nanosleep_time64)
+extern int __nanosleep_nocancel_time64 (const struct __timespec64 *requested_time,
+                                        struct __timespec64 *remaining);
+libc_hidden_proto (__nanosleep_nocancel_time64)
+#endif
 
 /* Use in the clock_* functions.  Size of the field representing the
    actual clock ID.  */
diff --git a/nptl/thrd_sleep.c b/nptl/thrd_sleep.c
index 2e185dd748e..2a6bb1f9e3f 100644
--- a/nptl/thrd_sleep.c
+++ b/nptl/thrd_sleep.c
@@ -17,23 +17,85 @@ 
    <https://www.gnu.org/licenses/>.  */
 
 #include <time.h>
+#include <kernel-features.h>
 #include <sysdep-cancel.h>
 
 #include "thrd_priv.h"
 
 int
-thrd_sleep (const struct timespec* time_point, struct timespec* remaining)
+__thrd_sleep_time64 (const struct __timespec64* time_point, struct __timespec64* remaining)
 {
   INTERNAL_SYSCALL_DECL (err);
-  int ret = INTERNAL_SYSCALL_CANCEL (nanosleep, err, time_point, remaining);
+  int ret = -1;
+
+#ifdef __ASSUME_TIME64_SYSCALLS
+# ifndef __NR_clock_nanosleep_time64
+#  define __NR_clock_nanosleep_time64 __NR_clock_nanosleep
+# endif
+  ret = INTERNAL_SYSCALL_CANCEL (clock_nanosleep_time64, err, CLOCK_REALTIME,
+                                 0, time_point, remaining);
+#else
+# ifdef __NR_clock_nanosleep_time64
+  long int ret_64;
+
+  ret_64 = INTERNAL_SYSCALL_CANCEL (clock_nanosleep_time64, err, CLOCK_REALTIME,
+                                    0, time_point, remaining);
+
+  if (ret_64 == 0 || errno != ENOSYS)
+    ret = ret_64;
+# endif /* __NR_clock_nanosleep_time64 */
+  if (ret < 0)
+    {
+      struct timespec tp32, tr32;
+
+      if (! in_time_t_range (time_point->tv_sec))
+        {
+          __set_errno (EOVERFLOW);
+          return -1;
+        }
+
+      tp32 = valid_timespec64_to_timespec (*time_point);
+      ret =  INTERNAL_SYSCALL_CANCEL (nanosleep, err, &tp32, &tr32);
+
+      if ((ret == 0 || errno != ENOSYS) && remaining)
+        *remaining = valid_timespec_to_timespec64 (tr32);
+    }
+#endif /* __ASSUME_TIME64_SYSCALLS */
+
   if (INTERNAL_SYSCALL_ERROR_P (ret, err))
     {
       /* C11 states thrd_sleep function returns -1 if it has been interrupted
-	 by a signal, or a negative value if it fails.  */
+         by a signal, or a negative value if it fails.  */
       ret = INTERNAL_SYSCALL_ERRNO (ret, err);
       if (ret == EINTR)
-	return -1;
+        return -1;
       return -2;
     }
   return 0;
 }
+
+#if __TIMESIZE != 64
+int
+thrd_sleep (const struct timespec* time_point, struct timespec* remaining)
+{
+  int ret;
+  struct __timespec64 tp64, tr64;
+
+  tp64 = valid_timespec_to_timespec64 (*time_point);
+  ret = __thrd_sleep_time64 (&tp64, &tr64);
+
+  if (ret == 0 || errno != ENOSYS)
+    {
+      if (! in_time_t_range (tr64.tv_sec))
+        {
+          __set_errno (EOVERFLOW);
+          return -1;
+        }
+
+      if (remaining)
+        *remaining = valid_timespec64_to_timespec (tr64);
+    }
+
+  return ret;
+}
+#endif
diff --git a/sysdeps/unix/sysv/linux/clock_nanosleep.c b/sysdeps/unix/sysv/linux/clock_nanosleep.c
index 1f240b8720a..d257ea57fb0 100644
--- a/sysdeps/unix/sysv/linux/clock_nanosleep.c
+++ b/sysdeps/unix/sysv/linux/clock_nanosleep.c
@@ -16,6 +16,7 @@ 
    <https://www.gnu.org/licenses/>.  */
 
 #include <time.h>
+#include <kernel-features.h>
 #include <errno.h>
 
 #include <sysdep-cancel.h>
@@ -26,9 +27,11 @@ 
 /* We can simply use the syscall.  The CPU clocks are not supported
    with this function.  */
 int
-__clock_nanosleep (clockid_t clock_id, int flags, const struct timespec *req,
-		   struct timespec *rem)
+__clock_nanosleep_time64 (clockid_t clock_id, int flags, const struct __timespec64 *req,
+                          struct __timespec64 *rem)
 {
+  int r = -1;
+
   if (clock_id == CLOCK_THREAD_CPUTIME_ID)
     return EINVAL;
   if (clock_id == CLOCK_PROCESS_CPUTIME_ID)
@@ -37,12 +40,72 @@  __clock_nanosleep (clockid_t clock_id, int flags, const struct timespec *req,
   /* If the call is interrupted by a signal handler or encounters an error,
      it returns a positive value similar to errno.  */
   INTERNAL_SYSCALL_DECL (err);
-  int r = INTERNAL_SYSCALL_CANCEL (clock_nanosleep, err, clock_id, flags,
-				   req, rem);
+
+#ifdef __ASSUME_TIME64_SYSCALLS
+# ifndef __NR_clock_nanosleep_time64
+#  define __NR_clock_nanosleep_time64 __NR_clock_nanosleep
+# endif
+  r = INTERNAL_SYSCALL_CANCEL (clock_nanosleep_time64, err, clock_id,
+                               flags, req, rem);
+#else
+# ifdef __NR_clock_nanosleep_time64
+  long int ret_64;
+
+  ret_64 = INTERNAL_SYSCALL_CANCEL (clock_nanosleep_time64, err, clock_id,
+                                    flags, req, rem);
+
+  if (ret_64 == 0 || errno != ENOSYS)
+    r = ret_64;
+# endif /* __NR_clock_nanosleep_time64 */
+  if (r < 0)
+    {
+      struct timespec ts32, tr32;
+
+      if (! in_time_t_range (req->tv_sec))
+        {
+          __set_errno (EOVERFLOW);
+          return -1;
+        }
+
+      ts32 = valid_timespec64_to_timespec (*req);
+      r =  INTERNAL_SYSCALL_CANCEL (clock_nanosleep, err, &ts32, &tr32);
+
+      if ((r == 0 || errno != ENOSYS) && rem)
+        *rem = valid_timespec_to_timespec64 (tr32);
+    }
+#endif /* __ASSUME_TIME64_SYSCALLS */
+
   return (INTERNAL_SYSCALL_ERROR_P (r, err)
-	  ? INTERNAL_SYSCALL_ERRNO (r, err) : 0);
+          ? INTERNAL_SYSCALL_ERRNO (r, err) : 0);
 }
 
+#if __TIMESIZE != 64
+int
+__clock_nanosleep (clockid_t clock_id, int flags, const struct timespec *req,
+                   struct timespec *rem)
+{
+  int r;
+  struct __timespec64 treq64, trem64;
+
+  treq64 = valid_timespec_to_timespec64 (*req);
+  r = __clock_nanosleep_time64 (clock_id, flags, &treq64, &trem64);
+
+  if (r == 0 || errno != ENOSYS)
+    {
+      if (! in_time_t_range (trem64.tv_sec))
+        {
+          __set_errno (EOVERFLOW);
+          return -1;
+        }
+
+      if (rem)
+        *rem = valid_timespec64_to_timespec (trem64);
+    }
+
+  return r;
+}
+#endif
+
 versioned_symbol (libc, __clock_nanosleep, clock_nanosleep, GLIBC_2_17);
 /* clock_nanosleep moved to libc in version 2.17;
    old binaries may expect the symbol version it had in librt.  */
diff --git a/sysdeps/unix/sysv/linux/nanosleep.c b/sysdeps/unix/sysv/linux/nanosleep.c
index 6787909248f..10a6d57a1de 100644
--- a/sysdeps/unix/sysv/linux/nanosleep.c
+++ b/sysdeps/unix/sysv/linux/nanosleep.c
@@ -17,15 +17,76 @@ 
    <https://www.gnu.org/licenses/>.  */
 
 #include <time.h>
+#include <kernel-features.h>
 #include <sysdep-cancel.h>
 #include <not-cancel.h>
 
 /* Pause execution for a number of nanoseconds.  */
 int
+__nanosleep_time64 (const struct __timespec64 *requested_time,
+                    struct __timespec64 *remaining)
+{
+#if defined(__ASSUME_TIME64_SYSCALLS)
+# ifndef __NR_clock_nanosleep_time64
+#  define __NR_clock_nanosleep_time64 __NR_clock_nanosleep
+# endif
+  return SYSCALL_CANCEL (clock_nanosleep_time64, CLOCK_REALTIME, 0,
+                         requested_time, remaining);
+#else
+# ifdef __NR_clock_nanosleep_time64
+  long int ret_64;
+
+  ret_64 = SYSCALL_CANCEL (clock_nanosleep_time64, CLOCK_REALTIME, 0,
+                           requested_time, remaining);
+
+  if (ret_64 == 0 || errno != ENOSYS)
+    return ret_64;
+# endif /* __NR_clock_nanosleep_time64 */
+  int ret;
+  struct timespec ts32, tr32;
+
+  if (! in_time_t_range (requested_time->tv_sec))
+    {
+      __set_errno (EOVERFLOW);
+      return -1;
+    }
+
+  ts32 = valid_timespec64_to_timespec (*requested_time);
+  ret = SYSCALL_CANCEL (nanosleep, &ts32, &tr32);
+
+  if ((ret == 0 || errno != ENOSYS) && remaining)
+    *remaining = valid_timespec_to_timespec64 (tr32);
+
+  return ret;
+#endif /* __ASSUME_TIME64_SYSCALLS */
+}
+
+#if __TIMESIZE != 64
+int
 __nanosleep (const struct timespec *requested_time,
-	     struct timespec *remaining)
+             struct timespec *remaining)
 {
-  return SYSCALL_CANCEL (nanosleep, requested_time, remaining);
+  int r;
+  struct __timespec64 treq64, trem64;
+
+  treq64 = valid_timespec_to_timespec64 (*requested_time);
+  r = __nanosleep_time64 (&treq64, &trem64);
+
+  if (r == 0 || errno != ENOSYS)
+    {
+      if (! in_time_t_range (trem64.tv_sec))
+        {
+          __set_errno (EOVERFLOW);
+          return -1;
+        }
+
+      if (remaining)
+        *remaining = valid_timespec64_to_timespec (trem64);
+    }
+
+  return r;
 }
+#endif
+
 hidden_def (__nanosleep)
 weak_alias (__nanosleep, nanosleep)
diff --git a/sysdeps/unix/sysv/linux/nanosleep_nocancel.c b/sysdeps/unix/sysv/linux/nanosleep_nocancel.c
index d6442bf4f7f..1a6c6c0a48a 100644
--- a/sysdeps/unix/sysv/linux/nanosleep_nocancel.c
+++ b/sysdeps/unix/sysv/linux/nanosleep_nocancel.c
@@ -17,13 +17,74 @@ 
    <https://www.gnu.org/licenses/>.  */
 
 #include <time.h>
+#include <kernel-features.h>
 #include <sysdep-cancel.h>
 #include <not-cancel.h>
 
+int
+__nanosleep_nocancel_time64 (const struct __timespec64 *requested_time,
+                             struct __timespec64 *remaining)
+{
+#ifdef __ASSUME_TIME64_SYSCALLS
+# ifndef __NR_clock_nanosleep_time64
+#  define __NR_clock_nanosleep_time64 __NR_clock_nanosleep
+# endif
+  return INLINE_SYSCALL_CALL (clock_nanosleep_time64, CLOCK_REALTIME, 0,
+                              requested_time, remaining);
+#else
+# ifdef __NR_clock_nanosleep_time64
+  long int ret_64;
+
+  ret_64 = INLINE_SYSCALL_CALL (clock_nanosleep_time64, CLOCK_REALTIME, 0,
+                                requested_time, remaining);
+
+  if (ret_64 == 0 || errno != ENOSYS)
+    return ret_64;
+# endif /* __NR_clock_nanosleep_time64 */
+  int ret;
+  struct timespec ts32, tr32;
+
+  if (! in_time_t_range (requested_time->tv_sec))
+    {
+      __set_errno (EOVERFLOW);
+      return -1;
+    }
+
+  ts32 = valid_timespec64_to_timespec (*requested_time);
+  ret = INLINE_SYSCALL_CALL (nanosleep, &ts32, &tr32);
+
+  if (ret == 0 || errno != ENOSYS)
+    *remaining = valid_timespec_to_timespec64 (tr32);
+
+  return ret;
+#endif /* __ASSUME_TIME64_SYSCALLS */
+}
+
+#if __TIMESIZE != 64
 int
 __nanosleep_nocancel (const struct timespec *requested_time,
-		      struct timespec *remaining)
+                      struct timespec *remaining)
 {
-  return INLINE_SYSCALL_CALL (nanosleep, requested_time, remaining);
+  int ret;
+  struct __timespec64 treq64, trem64;
+
+  treq64 = valid_timespec_to_timespec64 (*requested_time);
+  ret = __nanosleep_nocancel_time64 (&treq64, &trem64);
+
+  if (ret == 0 || errno != ENOSYS)
+    {
+      if (! in_time_t_range (trem64.tv_sec))
+        {
+          __set_errno (EOVERFLOW);
+          return -1;
+        }
+
+      if (remaining)
+        *remaining = valid_timespec64_to_timespec (trem64);
+    }
+
+  return ret;
 }
+#endif
+
 hidden_def (__nanosleep_nocancel)