[v6] y2038: linux: Provide __gettimeofday64 implementation

  In the glibc the gettimeofday can use vDSO (on power and x86 the
USE_IFUNC_GETTIMEOFDAY is defined), gettimeofday syscall or 'default'
___gettimeofday() from ./time/gettime.c (as a fallback).

In this patch the last function (___gettimeofday) has been refactored and
moved to ./sysdeps/unix/sysv/linux/gettimeofday.c to be Linux specific.

The new __gettimeofday64 explicit 64 bit function for getting 64 bit time from
the kernel (by internally calling __clock_gettime64) has been introduced.

Moreover, a 32 bit version - __gettimeofday has been refactored to internally
use __gettimeofday64.

The __gettimeofday is now supposed to be used on systems still supporting 32
bit time (__TIMESIZE != 64) - hence the necessary check for time_t potential
overflow and conversion of struct __timeval64 to 32 bit struct timespec.

The alpha port is a bit problematic for this change - it supports 64 bit time
and can safely use gettimeofday implementation from ./time/gettimeofday.c as it
has ./sysdeps/unix/sysv/linux/alpha/gettimeofday.c, which includes
./time/gettimeofday.c, so the Linux specific one can be avoided.
For that reason the code to set default gettimeofday symbol has not been moved
to ./sysdeps/unix/sysv/linux/gettimeofday.c as only alpha defines
VERSION_gettimeofday.

The USE_IFUNC_GETTIMEOFDAY for powerpc and i686 has been undefined on
purpose. Due to that the support for gettimeofday vDSO on them has been
traded for Y2038 safeness (as this syscall is going to be obsolete).

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

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

Above tests were performed with Y2038 redirection applied as well as without
to test proper usage of both __gettimeofday64 and __gettimeofday.

---
Changes for v6:
- Do not check if tv pointer is NULL as in generic implementation of
  ./time/gettimeofday.c. Moreover, prototype for this function has
  __nonnull GCC attribute for this parameter.

Changes for v5:
- Replace '___gettimeofday64' with '__gettimeofday64' in the subject of the
  patch

Changes for v4:
- Rename ___gettimeofday{64} to __gettimeofday{64} as '___' prefix is not
  needed for our implementation
- Correctly handle the case when tv is NULL (also in __gettimeofday).
- Do not define USE_IFUNC_GETTIMEOFDAY for 32 bit archs - namely powerpc and
  i686.

Changes for v3:
- New patch
---
 include/time.h                                |  4 ++
 sysdeps/unix/sysv/linux/gettimeofday.c        | 40 ++++++++++++++++++-
 .../unix/sysv/linux/powerpc/gettimeofday.c    |  4 +-
 sysdeps/unix/sysv/linux/x86/gettimeofday.c    |  4 +-
 4 files changed, 49 insertions(+), 3 deletions(-)
  

On 12/02/2020 05:45, Lukasz Majewski wrote:
> In the glibc the gettimeofday can use vDSO (on power and x86 the
> USE_IFUNC_GETTIMEOFDAY is defined), gettimeofday syscall or 'default'
> ___gettimeofday() from ./time/gettime.c (as a fallback).
> 
> In this patch the last function (___gettimeofday) has been refactored and
> moved to ./sysdeps/unix/sysv/linux/gettimeofday.c to be Linux specific.

In fact the function implementation is replicated on Linux version. Once this
patch is upstream I plan to make a generic implementation and refactor alpha,
Linux, and generic implementation to avoid code duplication.

> 
> The new __gettimeofday64 explicit 64 bit function for getting 64 bit time from
> the kernel (by internally calling __clock_gettime64) has been introduced.
> 
> Moreover, a 32 bit version - __gettimeofday has been refactored to internally
> use __gettimeofday64.
> 
> The __gettimeofday is now supposed to be used on systems still supporting 32
> bit time (__TIMESIZE != 64) - hence the necessary check for time_t potential
> overflow and conversion of struct __timeval64 to 32 bit struct timespec.
> 
> The alpha port is a bit problematic for this change - it supports 64 bit time
> and can safely use gettimeofday implementation from ./time/gettimeofday.c as it
> has ./sysdeps/unix/sysv/linux/alpha/gettimeofday.c, which includes
> ./time/gettimeofday.c, so the Linux specific one can be avoided.
> For that reason the code to set default gettimeofday symbol has not been moved
> to ./sysdeps/unix/sysv/linux/gettimeofday.c as only alpha defines
> VERSION_gettimeofday.

You can remove this part about alpha from commit message, this
patch does not change anything about alpha behaviour.

> 
> The USE_IFUNC_GETTIMEOFDAY for powerpc and i686 has been undefined on
> purpose. Due to that the support for gettimeofday vDSO on them has been
> traded for Y2038 safeness (as this syscall is going to be obsolete).

I think it would be useful to extend a bit why this optimization was
removed from i686 and powerpc32.  Maybe add:

  The iFUNC vDSO direct call optimization has been removed from both
  i686 and powerpc32 (USE_IFUNC_GETTIMEOFDAY is not defined for the
  architectures anymore).  The Linux kernel does not provide a y2038
  safe implementation of gettimeofday neither it plans to provide it
  in the future, clock_gettime64 should be use instead.  It meant to
  keep support for this optimization it would require to handle another
  build permutation (!__ASSUME_TIME64_SYSCALLS && USE_IFUNC_GETTIMEOFDAY)
  with adds more complexity and has limited use (since the idea is to
  eventually have a y2038 safe glibc build).

> 
> Build tests:
> ./src/scripts/build-many-glibcs.py glibcs
> 
> Run-time tests:
> - Run specific tests on ARM/x86 32bit systems (qemu):
>   https://github.com/lmajewski/meta-y2038 and run tests:
>   https://github.com/lmajewski/y2038-tests/commits/master
> 
> Above tests were performed with Y2038 redirection applied as well as without
> to test proper usage of both __gettimeofday64 and __gettimeofday.

LGTM with the suggested commit message changes.

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

> 
> ---
> Changes for v6:
> - Do not check if tv pointer is NULL as in generic implementation of
>   ./time/gettimeofday.c. Moreover, prototype for this function has
>   __nonnull GCC attribute for this parameter.
> 
> Changes for v5:
> - Replace '___gettimeofday64' with '__gettimeofday64' in the subject of the
>   patch
> 
> Changes for v4:
> - Rename ___gettimeofday{64} to __gettimeofday{64} as '___' prefix is not
>   needed for our implementation
> - Correctly handle the case when tv is NULL (also in __gettimeofday).
> - Do not define USE_IFUNC_GETTIMEOFDAY for 32 bit archs - namely powerpc and
>   i686.
> 
> Changes for v3:
> - New patch
> ---
>  include/time.h                                |  4 ++
>  sysdeps/unix/sysv/linux/gettimeofday.c        | 40 ++++++++++++++++++-
>  .../unix/sysv/linux/powerpc/gettimeofday.c    |  4 +-
>  sysdeps/unix/sysv/linux/x86/gettimeofday.c    |  4 +-
>  4 files changed, 49 insertions(+), 3 deletions(-)
> 
> diff --git a/include/time.h b/include/time.h
> index 73f66277ac..61806658e7 100644
> --- a/include/time.h
> +++ b/include/time.h
> @@ -227,10 +227,14 @@ libc_hidden_proto (__sched_rr_get_interval64);
>  
>  #if __TIMESIZE == 64
>  # define __settimeofday64 __settimeofday
> +# define __gettimeofday64 __gettimeofday
>  #else
>  extern int __settimeofday64 (const struct __timeval64 *tv,
>                               const struct timezone *tz);
>  libc_hidden_proto (__settimeofday64)
> +extern int __gettimeofday64 (struct __timeval64 *restrict tv,
> +                             void *restrict tz);
> +libc_hidden_proto (__gettimeofday64)
>  #endif
>  
>  /* Compute the `struct tm' representation of T,

Ok.

> diff --git a/sysdeps/unix/sysv/linux/gettimeofday.c b/sysdeps/unix/sysv/linux/gettimeofday.c
> index d5cdb22495..cb57bc9cf2 100644
> --- a/sysdeps/unix/sysv/linux/gettimeofday.c
> +++ b/sysdeps/unix/sysv/linux/gettimeofday.c
> @@ -54,5 +54,43 @@ __gettimeofday (struct timeval *restrict tv, void *restrict tz)
>  # endif
>  weak_alias (__gettimeofday, gettimeofday)
>  #else /* USE_IFUNC_GETTIMEOFDAY  */
> -# include <time/gettimeofday.c>
> +/* Conversion of gettimeofday function to support 64 bit time on archs
> +   with __WORDSIZE == 32 and __TIMESIZE == 32/64  */
> +#include <errno.h>
> +
> +int
> +__gettimeofday64 (struct __timeval64 *restrict tv, void *restrict tz)
> +{
> +  if (__glibc_unlikely (tz != 0))
> +    memset (tz, 0, sizeof (struct timezone));
> +
> +  struct __timespec64 ts64;
> +  if (__clock_gettime64 (CLOCK_REALTIME, &ts64))
> +	  return -1;
> +
> +  *tv = timespec64_to_timeval64 (ts64);
> +  return 0;
> +}
> +
> +# if __TIMESIZE != 64
> +libc_hidden_def (__gettimeofday64)
> +
> +int
> +__gettimeofday (struct timeval *restrict tv, void *restrict tz)
> +{
> +  struct __timeval64 tv64;
> +  if (__gettimeofday64 (&tv64, tz))
> +	  return -1;
> +
> +  if (! in_time_t_range (tv64.tv_sec))
> +    {
> +      __set_errno (EOVERFLOW);
> +      return -1;
> +    }
> +
> +  *tv = valid_timeval64_to_timeval (tv64);
> +  return 0;
> +}
> +# endif
> +weak_alias (__gettimeofday, gettimeofday)
>  #endif

Ok.

> diff --git a/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c b/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c
> index 183fb0ac70..2d6978f333 100644
> --- a/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c
> +++ b/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c
> @@ -15,5 +15,7 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> -#define USE_IFUNC_GETTIMEOFDAY
> +#ifdef __powerpc64__
> +# define USE_IFUNC_GETTIMEOFDAY
> +#endif
>  #include <sysdeps/unix/sysv/linux/gettimeofday.c>

Ok.

> diff --git a/sysdeps/unix/sysv/linux/x86/gettimeofday.c b/sysdeps/unix/sysv/linux/x86/gettimeofday.c
> index 1b7aa880a2..0c1779dc83 100644
> --- a/sysdeps/unix/sysv/linux/x86/gettimeofday.c
> +++ b/sysdeps/unix/sysv/linux/x86/gettimeofday.c
> @@ -16,5 +16,7 @@
>     License along with the GNU C Library; if not, see
>     <https://www.gnu.org/licenses/>.  */
>  
> -#define USE_IFUNC_GETTIMEOFDAY
> +#ifdef __x86_64__
> +# define USE_IFUNC_GETTIMEOFDAY
> +#endif
>  #include <sysdeps/unix/sysv/linux/gettimeofday.c>
> 

Ok.
  

Hi Adhemerval,

Thank you for the review.

> On 12/02/2020 05:45, Lukasz Majewski wrote:
> > In the glibc the gettimeofday can use vDSO (on power and x86 the
> > USE_IFUNC_GETTIMEOFDAY is defined), gettimeofday syscall or
> > 'default' ___gettimeofday() from ./time/gettime.c (as a fallback).
> > 
> > In this patch the last function (___gettimeofday) has been
> > refactored and moved to ./sysdeps/unix/sysv/linux/gettimeofday.c to
> > be Linux specific.  
> 
> In fact the function implementation is replicated on Linux version.
> Once this patch is upstream I plan to make a generic implementation
> and refactor alpha, Linux, and generic implementation to avoid code
> duplication.

Ok. I shall apply this patch till the end of the day.

> 
> > 
> > The new __gettimeofday64 explicit 64 bit function for getting 64
> > bit time from the kernel (by internally calling __clock_gettime64)
> > has been introduced.
> > 
> > Moreover, a 32 bit version - __gettimeofday has been refactored to
> > internally use __gettimeofday64.
> > 
> > The __gettimeofday is now supposed to be used on systems still
> > supporting 32 bit time (__TIMESIZE != 64) - hence the necessary
> > check for time_t potential overflow and conversion of struct
> > __timeval64 to 32 bit struct timespec.
> > 
> > The alpha port is a bit problematic for this change - it supports
> > 64 bit time and can safely use gettimeofday implementation from
> > ./time/gettimeofday.c as it has
> > ./sysdeps/unix/sysv/linux/alpha/gettimeofday.c, which includes
> > ./time/gettimeofday.c, so the Linux specific one can be avoided.
> > For that reason the code to set default gettimeofday symbol has not
> > been moved to ./sysdeps/unix/sysv/linux/gettimeofday.c as only
> > alpha defines VERSION_gettimeofday.  
> 
> You can remove this part about alpha from commit message, this
> patch does not change anything about alpha behaviour.

Ok.

> 
> > 
> > The USE_IFUNC_GETTIMEOFDAY for powerpc and i686 has been undefined
> > on purpose. Due to that the support for gettimeofday vDSO on them
> > has been traded for Y2038 safeness (as this syscall is going to be
> > obsolete).  
> 
> I think it would be useful to extend a bit why this optimization was
> removed from i686 and powerpc32.  Maybe add:
> 
>   The iFUNC vDSO direct call optimization has been removed from both
>   i686 and powerpc32 (USE_IFUNC_GETTIMEOFDAY is not defined for the
>   architectures anymore).  The Linux kernel does not provide a y2038
>   safe implementation of gettimeofday neither it plans to provide it
>   in the future, clock_gettime64 should be use instead.  It meant to
>   keep support for this optimization it would require to handle
> another build permutation (!__ASSUME_TIME64_SYSCALLS &&
> USE_IFUNC_GETTIMEOFDAY) with adds more complexity and has limited use
> (since the idea is to eventually have a y2038 safe glibc build).
> 

Ok. I will add this description.

> > 
> > Build tests:
> > ./src/scripts/build-many-glibcs.py glibcs
> > 
> > Run-time tests:
> > - Run specific tests on ARM/x86 32bit systems (qemu):
> >   https://github.com/lmajewski/meta-y2038 and run tests:
> >   https://github.com/lmajewski/y2038-tests/commits/master
> > 
> > Above tests were performed with Y2038 redirection applied as well
> > as without to test proper usage of both __gettimeofday64 and
> > __gettimeofday.  
> 
> LGTM with the suggested commit message changes.
> 
> Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>

Thanks. 

Could you also review following patch set:
https://patchwork.ozlabs.org/cover/1234918/

(It seems to be straightforward and has been posted to the mailing list
some time ago).

Thanks in advance.

> 
> > 
> > ---
> > Changes for v6:
> > - Do not check if tv pointer is NULL as in generic implementation of
> >   ./time/gettimeofday.c. Moreover, prototype for this function has
> >   __nonnull GCC attribute for this parameter.
> > 
> > Changes for v5:
> > - Replace '___gettimeofday64' with '__gettimeofday64' in the
> > subject of the patch
> > 
> > Changes for v4:
> > - Rename ___gettimeofday{64} to __gettimeofday{64} as '___' prefix
> > is not needed for our implementation
> > - Correctly handle the case when tv is NULL (also in
> > __gettimeofday).
> > - Do not define USE_IFUNC_GETTIMEOFDAY for 32 bit archs - namely
> > powerpc and i686.
> > 
> > Changes for v3:
> > - New patch
> > ---
> >  include/time.h                                |  4 ++
> >  sysdeps/unix/sysv/linux/gettimeofday.c        | 40
> > ++++++++++++++++++- .../unix/sysv/linux/powerpc/gettimeofday.c    |
> >  4 +- sysdeps/unix/sysv/linux/x86/gettimeofday.c    |  4 +-
> >  4 files changed, 49 insertions(+), 3 deletions(-)
> > 
> > diff --git a/include/time.h b/include/time.h
> > index 73f66277ac..61806658e7 100644
> > --- a/include/time.h
> > +++ b/include/time.h
> > @@ -227,10 +227,14 @@ libc_hidden_proto (__sched_rr_get_interval64);
> >  
> >  #if __TIMESIZE == 64
> >  # define __settimeofday64 __settimeofday
> > +# define __gettimeofday64 __gettimeofday
> >  #else
> >  extern int __settimeofday64 (const struct __timeval64 *tv,
> >                               const struct timezone *tz);
> >  libc_hidden_proto (__settimeofday64)
> > +extern int __gettimeofday64 (struct __timeval64 *restrict tv,
> > +                             void *restrict tz);
> > +libc_hidden_proto (__gettimeofday64)
> >  #endif
> >  
> >  /* Compute the `struct tm' representation of T,  
> 
> Ok.
> 
> > diff --git a/sysdeps/unix/sysv/linux/gettimeofday.c
> > b/sysdeps/unix/sysv/linux/gettimeofday.c index
> > d5cdb22495..cb57bc9cf2 100644 ---
> > a/sysdeps/unix/sysv/linux/gettimeofday.c +++
> > b/sysdeps/unix/sysv/linux/gettimeofday.c @@ -54,5 +54,43 @@
> > __gettimeofday (struct timeval *restrict tv, void *restrict tz) #
> > endif weak_alias (__gettimeofday, gettimeofday)
> >  #else /* USE_IFUNC_GETTIMEOFDAY  */
> > -# include <time/gettimeofday.c>
> > +/* Conversion of gettimeofday function to support 64 bit time on
> > archs
> > +   with __WORDSIZE == 32 and __TIMESIZE == 32/64  */
> > +#include <errno.h>
> > +
> > +int
> > +__gettimeofday64 (struct __timeval64 *restrict tv, void *restrict
> > tz) +{
> > +  if (__glibc_unlikely (tz != 0))
> > +    memset (tz, 0, sizeof (struct timezone));
> > +
> > +  struct __timespec64 ts64;
> > +  if (__clock_gettime64 (CLOCK_REALTIME, &ts64))
> > +	  return -1;
> > +
> > +  *tv = timespec64_to_timeval64 (ts64);
> > +  return 0;
> > +}
> > +
> > +# if __TIMESIZE != 64
> > +libc_hidden_def (__gettimeofday64)
> > +
> > +int
> > +__gettimeofday (struct timeval *restrict tv, void *restrict tz)
> > +{
> > +  struct __timeval64 tv64;
> > +  if (__gettimeofday64 (&tv64, tz))
> > +	  return -1;
> > +
> > +  if (! in_time_t_range (tv64.tv_sec))
> > +    {
> > +      __set_errno (EOVERFLOW);
> > +      return -1;
> > +    }
> > +
> > +  *tv = valid_timeval64_to_timeval (tv64);
> > +  return 0;
> > +}
> > +# endif
> > +weak_alias (__gettimeofday, gettimeofday)
> >  #endif  
> 
> Ok.
> 
> > diff --git a/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c
> > b/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c index
> > 183fb0ac70..2d6978f333 100644 ---
> > a/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c +++
> > b/sysdeps/unix/sysv/linux/powerpc/gettimeofday.c @@ -15,5 +15,7 @@
> >     License along with the GNU C Library; if not, see
> >     <https://www.gnu.org/licenses/>.  */
> >  
> > -#define USE_IFUNC_GETTIMEOFDAY
> > +#ifdef __powerpc64__
> > +# define USE_IFUNC_GETTIMEOFDAY
> > +#endif
> >  #include <sysdeps/unix/sysv/linux/gettimeofday.c>  
> 
> Ok.
> 
> > diff --git a/sysdeps/unix/sysv/linux/x86/gettimeofday.c
> > b/sysdeps/unix/sysv/linux/x86/gettimeofday.c index
> > 1b7aa880a2..0c1779dc83 100644 ---
> > a/sysdeps/unix/sysv/linux/x86/gettimeofday.c +++
> > b/sysdeps/unix/sysv/linux/x86/gettimeofday.c @@ -16,5 +16,7 @@
> >     License along with the GNU C Library; if not, see
> >     <https://www.gnu.org/licenses/>.  */
> >  
> > -#define USE_IFUNC_GETTIMEOFDAY
> > +#ifdef __x86_64__
> > +# define USE_IFUNC_GETTIMEOFDAY
> > +#endif
> >  #include <sysdeps/unix/sysv/linux/gettimeofday.c>
> >   
> 
> Ok.




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-59 Fax: (+49)-8142-66989-80 Email: lukma@denx.de