[v6] y2038: linux: Provide __gettimeofday64 implementation
diff mbox series

Message ID 20200212084525.7046-1-lukma@denx.de
State New
Headers show
Series
  • [v6] y2038: linux: Provide __gettimeofday64 implementation
Related show

Commit Message

Lukasz Majewski Feb. 12, 2020, 8:45 a.m. UTC
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(-)

Comments

Adhemerval Zanella Feb. 18, 2020, 1:24 p.m. UTC | #1
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.

Patch
diff mbox series

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,
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
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>
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>