[v9,1/2] Y2038: Add 64-bit time for all architectures

Message ID 20181010121242.13902-1-albert.aribaud@3adev.fr
State New, archived
Headers

Commit Message

Albert ARIBAUD Oct. 10, 2018, 12:12 p.m. UTC
  glibc support for 64-bit time_t on 32-bit architectures
will involve:

- Using 64-bit times inside glibc, with conversions
  to and from 32-bit times taking place as necessary
  for interfaces using such times.

- Adding 64-bit-time support in the glibc public API.
  This support should be dynamic, i.e. glibc should
  provide both 32-bit and 64-bit implementations and
   let user code choose at compile time whether to use
   the 32-bit or 64-bit interfaces.

This requires a glibc-internal name for a type for times
that are always 64-bit.

To determine whether the default time_t interfaces are 32-bit
and so need conversions, or are 64-bit and so are compatible
with the internal 64-bit type without conversions, a macro
giving the size of the  default time_t is also required.
This macro is called __TIMESIZE.

Based on __TIMESIZE, a new macro is defined, __TIME64_T_TYPE,
 which is always the right __*_T_TYPE to hold a 64-bit-time.
__TIME64_T_TYPE equals __TIME_T_TYPE if __TIMESIZE equals 64
and equals __SQUAD_T_TYPE otherwise.

__time64_t can then replace uses of internal_time_t.

* bit/time64.h: New file.
* bits/timesize: (__TIMESIZE): New macro.
* include/time.h: replace internal_time_t with __time64_t.
* posix/bits/types (__time64_t): Add.
* stdlib/Makefile: Add bits/time64.h to includes.
* stdlib/Makefile: Add bits/timesize.h to includes.
* sysdeps/unix/sysv/linux/x86/bits/time64.h: New file.
* sysdeps/unix/sysv/linux/x86/bits/timesize.h (__TIMESIZE): New macro.
---
 bits/time64.h                               | 36 +++++++++++++++++++
 bits/timesize.h                             | 22 ++++++++++++
 include/time.h                              |  7 +---
 posix/bits/types.h                          |  8 +++++
 stdlib/Makefile                             |  2 +-
 sysdeps/unix/sysv/linux/x86/bits/time64.h   | 39 +++++++++++++++++++++
 sysdeps/unix/sysv/linux/x86/bits/timesize.h | 25 +++++++++++++
 time/tzfile.c                               | 18 +++++-----
 8 files changed, 141 insertions(+), 16 deletions(-)
 create mode 100644 bits/time64.h
 create mode 100644 bits/timesize.h
 create mode 100644 sysdeps/unix/sysv/linux/x86/bits/time64.h
 create mode 100644 sysdeps/unix/sysv/linux/x86/bits/timesize.h
  

Comments

Joseph Myers Oct. 10, 2018, 4:22 p.m. UTC | #1
On Wed, 10 Oct 2018, Albert ARIBAUD (3ADEV) wrote:

>  bits/time64.h                               | 36 +++++++++++++++++++
>  bits/timesize.h                             | 22 ++++++++++++
>  include/time.h                              |  7 +---
>  posix/bits/types.h                          |  8 +++++
>  stdlib/Makefile                             |  2 +-
>  sysdeps/unix/sysv/linux/x86/bits/time64.h   | 39 +++++++++++++++++++++
>  sysdeps/unix/sysv/linux/x86/bits/timesize.h | 25 +++++++++++++

My understanding from review comments on previous versions of this patch 
was that the generic bits/time64.h was in fact fully correct for x32, so 
no x86-specific version of that header should be needed - only the 
x86-specific version of bits/timesize.h.
  
Albert ARIBAUD Oct. 11, 2018, 1:57 p.m. UTC | #2
Hi Joseph,

On Wed, 10 Oct 2018 16:22:49 +0000, Joseph Myers
<joseph@codesourcery.com> wrote :

> On Wed, 10 Oct 2018, Albert ARIBAUD (3ADEV) wrote:
> 
> >  bits/time64.h                               | 36 +++++++++++++++++++
> >  bits/timesize.h                             | 22 ++++++++++++
> >  include/time.h                              |  7 +---
> >  posix/bits/types.h                          |  8 +++++
> >  stdlib/Makefile                             |  2 +-
> >  sysdeps/unix/sysv/linux/x86/bits/time64.h   | 39 +++++++++++++++++++++
> >  sysdeps/unix/sysv/linux/x86/bits/timesize.h | 25 +++++++++++++  
> 
> My understanding from review comments on previous versions of this patch 
> was that the generic bits/time64.h was in fact fully correct for x32, so 
> no x86-specific version of that header should be needed - only the 
> x86-specific version of bits/timesize.h.

That was not my understanding, which was as follows:

- We need to determine the size of the target architecture time_t type
  in order to decide whether we need to define a 64-bit time type.
- we do not have the time_t size readily available, so we must infer it
  based on some other type size.
- The best candidate is the word size, which matches the time_t size for
  all architectures ; that is the most general rule.
- However, for x32 the word size is 32 yet the time_t size is 64, so
  x32 needs an exception.

Cordialement,
Albert ARIBAUD
3ADEV
  
Joseph Myers Oct. 11, 2018, 2:15 p.m. UTC | #3
On Thu, 11 Oct 2018, Albert ARIBAUD wrote:

> - We need to determine the size of the target architecture time_t type
>   in order to decide whether we need to define a 64-bit time type.

Yes.

> - we do not have the time_t size readily available, so we must infer it
>   based on some other type size.

That happens in bits/timesize.h.

Once bits/timesize.h exists, the value from there can be used in 
bits/time64.h, so only one bits/time64.h should be needed.

This patch could be split in two.  The first would add bits/timesize.h 
(two variants), and maybe also use __TIMESIZE in bits/msq-pad.h and remove 
the x86-specific version of that header.  The second would add 
bits/time64.h (one variant).
  
Albert ARIBAUD Oct. 11, 2018, 2:55 p.m. UTC | #4
Hi Joseph,

On Thu, 11 Oct 2018 14:15:33 +0000, Joseph Myers
<joseph@codesourcery.com> wrote :

> On Thu, 11 Oct 2018, Albert ARIBAUD wrote:
> 
> > - We need to determine the size of the target architecture time_t type
> >   in order to decide whether we need to define a 64-bit time type.  
> 
> Yes.
> 
> > - we do not have the time_t size readily available, so we must infer it
> >   based on some other type size.  
> 
> That happens in bits/timesize.h.
> 
> Once bits/timesize.h exists, the value from there can be used in 
> bits/time64.h, so only one bits/time64.h should be needed.

Ok.

> This patch could be split in two.  The first would add bits/timesize.h 
> (two variants), and maybe also use __TIMESIZE in bits/msq-pad.h and remove 
> the x86-specific version of that header.  The second would add 
> bits/time64.h (one variant).

I assume you mean bits/msq.h, not bits/msq-pad.h.

But shouldn't modifications to bits/msq.h be put in a separate,
dedicated patch?

Cordialement,
Albert ARIBAUD
3ADEV
  
Joseph Myers Oct. 11, 2018, 3:22 p.m. UTC | #5
On Thu, 11 Oct 2018, Albert ARIBAUD wrote:

> > This patch could be split in two.  The first would add bits/timesize.h 
> > (two variants), and maybe also use __TIMESIZE in bits/msq-pad.h and remove 
> > the x86-specific version of that header.  The second would add 
> > bits/time64.h (one variant).
> 
> I assume you mean bits/msq.h, not bits/msq-pad.h.

No, I mean bits/msq-pad.h.  As of today there is only one bits/msq.h for 
systems using the Linux kernel, and the only reason for the x86-specific 
variant of bits/msq-pad.h to exist is the lack of __TIMESIZE to use in the 
generic bits/msq-pad.h.
  
Albert ARIBAUD Oct. 11, 2018, 3:28 p.m. UTC | #6
Hi Joseph,

Le Thu, 11 Oct 2018 15:22:57 +0000, Joseph Myers
<joseph@codesourcery.com> a écrit :

> On Thu, 11 Oct 2018, Albert ARIBAUD wrote:
> 
> > > This patch could be split in two.  The first would add bits/timesize.h 
> > > (two variants), and maybe also use __TIMESIZE in bits/msq-pad.h and remove 
> > > the x86-specific version of that header.  The second would add 
> > > bits/time64.h (one variant).  
> > 
> > I assume you mean bits/msq.h, not bits/msq-pad.h.  
> 
> No, I mean bits/msq-pad.h.  As of today there is only one bits/msq.h for 
> systems using the Linux kernel, and the only reason for the x86-specific 
> variant of bits/msq-pad.h to exist is the lack of __TIMESIZE to use in the 
> generic bits/msq-pad.h.

Sorry, my repo was lagging one commit behind origin/master.

Cordialement,
Albert ARIBAUD
3ADEV
  
Albert ARIBAUD Oct. 11, 2018, 3:51 p.m. UTC | #7
Hi Joseph,

On Thu, 11 Oct 2018 15:22:57 +0000, Joseph Myers
<joseph@codesourcery.com> wrote :

> On Thu, 11 Oct 2018, Albert ARIBAUD wrote:
> 
> > > This patch could be split in two.  The first would add bits/timesize.h 
> > > (two variants), and maybe also use __TIMESIZE in bits/msq-pad.h and remove 
> > > the x86-specific version of that header.  The second would add 
> > > bits/time64.h (one variant).  
> > 
> > I assume you mean bits/msq.h, not bits/msq-pad.h.  
> 
> No, I mean bits/msq-pad.h.  As of today there is only one bits/msq.h for 
> systems using the Linux kernel, and the only reason for the x86-specific 
> variant of bits/msq-pad.h to exist is the lack of __TIMESIZE to use in the 
> generic bits/msq-pad.h.

For consistency, should we not replace __WORDSIZE with __TIMESIZE in all
msq-pad.h files rather than just sysdeps/unix/sysv/linux/bits/msq-pad.h?

Cordialement,
Albert ARIBAUD
3ADEV
  
Joseph Myers Oct. 11, 2018, 4:40 p.m. UTC | #8
On Thu, 11 Oct 2018, Albert ARIBAUD wrote:

> > No, I mean bits/msq-pad.h.  As of today there is only one bits/msq.h for 
> > systems using the Linux kernel, and the only reason for the x86-specific 
> > variant of bits/msq-pad.h to exist is the lack of __TIMESIZE to use in the 
> > generic bits/msq-pad.h.
> 
> For consistency, should we not replace __WORDSIZE with __TIMESIZE in all
> msq-pad.h files rather than just sysdeps/unix/sysv/linux/bits/msq-pad.h?

Sure, that would be reasonable.
  

Patch

diff --git a/bits/time64.h b/bits/time64.h
new file mode 100644
index 0000000000..c8a9c7db5f
--- /dev/null
+++ b/bits/time64.h
@@ -0,0 +1,36 @@ 
+/* bits/time64.h -- underlying types for __time64_t.  Generic version.
+   Copyright (C) 2018 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef _BITS_TYPES_H
+# error "Never include <bits/time64.h> directly; use <sys/types.h> instead."
+#endif
+
+#ifndef	_BITS_TIME64_H
+#define	_BITS_TIME64_H	1
+
+/* Define __TIME64_T_TYPE so that it is always a 64-bit type.  */
+
+#if __TIMESIZE == 64
+/* If we already have 64-bit time type then use it.  */
+# define __TIME64_T_TYPE		__TIME_T_TYPE
+#else
+/* Define a 64-bit time type alongsize the 32-bit one.  */
+# define __TIME64_T_TYPE		__SQUAD_TYPE
+#endif
+
+#endif /* bits/time64.h */
diff --git a/bits/timesize.h b/bits/timesize.h
new file mode 100644
index 0000000000..cc47ff165f
--- /dev/null
+++ b/bits/timesize.h
@@ -0,0 +1,22 @@ 
+/* Bit size of the time_t type at glibc build time, general case.
+   Copyright (C) 2018 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#include <bits/wordsize.h>
+
+/* Size in bits of the 'time_t' type of the default ABI.  */
+#define __TIMESIZE	__WORDSIZE
diff --git a/include/time.h b/include/time.h
index e30c5fc3b1..e99711a556 100644
--- a/include/time.h
+++ b/include/time.h
@@ -26,10 +26,6 @@  extern __typeof (clock_getcpuclockid) __clock_getcpuclockid;
 /* Now define the internal interfaces.  */
 struct tm;
 
-/* time_t variant for representing time zone data, independent of
-   time_t.  */
-typedef __int64_t internal_time_t;
-
 /* Defined in mktime.c.  */
 extern const unsigned short int __mon_yday[2][13] attribute_hidden;
 
@@ -43,7 +39,7 @@  extern int __use_tzfile attribute_hidden;
 
 extern void __tzfile_read (const char *file, size_t extra,
 			   char **extrap) attribute_hidden;
-extern void __tzfile_compute (internal_time_t timer, int use_localtime,
+extern void __tzfile_compute (__time64_t timer, int use_localtime,
 			      long int *leap_correct, int *leap_hit,
 			      struct tm *tp) attribute_hidden;
 extern void __tzfile_default (const char *std, const char *dst,
@@ -101,7 +97,6 @@  extern char * __strptime_internal (const char *rp, const char *fmt,
 
 extern double __difftime (time_t time1, time_t time0);
 
-
 /* Use in the clock_* functions.  Size of the field representing the
    actual clock ID.  */
 #define CLOCK_IDFIELD_SIZE	3
diff --git a/posix/bits/types.h b/posix/bits/types.h
index 5e22ce41bf..cda0a70dd8 100644
--- a/posix/bits/types.h
+++ b/posix/bits/types.h
@@ -25,6 +25,7 @@ 
 
 #include <features.h>
 #include <bits/wordsize.h>
+#include <bits/timesize.h>
 
 /* Convenience types.  */
 typedef unsigned char __u_char;
@@ -138,6 +139,7 @@  __extension__ typedef unsigned long long int __uintmax_t;
 # error
 #endif
 #include <bits/typesizes.h>	/* Defines __*_T_TYPE macros.  */
+#include <bits/time64.h>	/* Defines __TIME*_T_TYPE macros.  */
 
 
 __STD_TYPE __DEV_T_TYPE __dev_t;	/* Type of device numbers.  */
@@ -211,6 +213,12 @@  __STD_TYPE __U32_TYPE __socklen_t;
    It is not currently necessary for this to be machine-specific.  */
 typedef int __sig_atomic_t;
 
+#if __TIMESIZE == 64
+# define __time64_t __time_t
+#else
+__STD_TYPE __TIME64_T_TYPE __time64_t;	/* Seconds since the Epoch.  */
+#endif
+
 #undef __STD_TYPE
 
 #endif /* bits/types.h */
diff --git a/stdlib/Makefile b/stdlib/Makefile
index 01194bbf7c..bdb0a18295 100644
--- a/stdlib/Makefile
+++ b/stdlib/Makefile
@@ -29,7 +29,7 @@  headers	:= stdlib.h bits/stdlib.h bits/stdlib-ldbl.h bits/stdlib-float.h      \
 	   ucontext.h sys/ucontext.h bits/indirect-return.h		      \
 	   alloca.h fmtmsg.h						      \
 	   bits/stdlib-bsearch.h sys/random.h bits/stdint-intn.h	      \
-	   bits/stdint-uintn.h
+	   bits/stdint-uintn.h bits/time64.h bits/timesize.h		      \
 
 routines	:=							      \
 	atof atoi atol atoll						      \
diff --git a/sysdeps/unix/sysv/linux/x86/bits/time64.h b/sysdeps/unix/sysv/linux/x86/bits/time64.h
new file mode 100644
index 0000000000..d4b1766b6a
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/x86/bits/time64.h
@@ -0,0 +1,39 @@ 
+/* bits/time64.h -- underlying types for __time64_t.  Linux/x86-64 version.
+   Copyright (C) 2018 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#ifndef _BITS_TYPES_H
+# error "Never include <bits/time64.h> directly; use <sys/types.h> instead."
+#endif
+
+#ifndef	_BITS_TIME64_H
+#define	_BITS_TIME64_H	1
+
+/* Define __TIME64_T_TYPE so that it is always a 64-bit type.  */
+
+#if defined __x86_64__ && defined __ILP32__
+/* For x32, time is 64-bit even though word size is 32-bit.  */
+# define __TIME64_T_TYPE		__SQUAD_TYPE
+#elif __TIMESIZE == 64
+/* If we already have 64-bit time type then use it.  */
+# define __TIME64_T_TYPE		__TIME_T_TYPE
+#else
+/* Define a 64-bit time type alongsize the 32-bit one.  */
+# define __TIME64_T_TYPE		__SQUAD_TYPE
+#endif
+
+#endif /* bits/time64.h */
diff --git a/sysdeps/unix/sysv/linux/x86/bits/timesize.h b/sysdeps/unix/sysv/linux/x86/bits/timesize.h
new file mode 100644
index 0000000000..8b88ab84b0
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/x86/bits/timesize.h
@@ -0,0 +1,25 @@ 
+/* Bit size of the time_t type at glibc build time, x86-64 and x32 case.
+   Copyright (C) 2018 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <http://www.gnu.org/licenses/>.  */
+
+#if defined __x86_64__ && defined __ILP32__
+/* For x32, time is 64-bit even though word size is 32-bit.  */
+# define __TIMESIZE	64
+#else
+/* For others, time size is word size.  */
+# define __TIMESIZE	__WORDSIZE
+#endif
diff --git a/time/tzfile.c b/time/tzfile.c
index 72ef75f074..844a68de8c 100644
--- a/time/tzfile.c
+++ b/time/tzfile.c
@@ -44,12 +44,12 @@  struct ttinfo
 
 struct leap
   {
-    internal_time_t transition;	/* Time the transition takes effect.  */
+    __time64_t transition;	/* Time the transition takes effect.  */
     long int change;		/* Seconds of correction to apply.  */
   };
 
 static size_t num_transitions;
-libc_freeres_ptr (static internal_time_t *transitions);
+libc_freeres_ptr (static __time64_t *transitions);
 static unsigned char *type_idxs;
 static size_t num_types;
 static struct ttinfo *types;
@@ -113,8 +113,8 @@  __tzfile_read (const char *file, size_t extra, char **extrap)
   size_t tzspec_len;
   char *new = NULL;
 
-  _Static_assert (sizeof (internal_time_t) == 8,
-		  "internal_time_t must be eight bytes");
+  _Static_assert (sizeof (__time64_t) == 8,
+		  "__time64_t must be eight bytes");
 
   __use_tzfile = 0;
 
@@ -217,9 +217,9 @@  __tzfile_read (const char *file, size_t extra, char **extrap)
 
   if (__builtin_expect (num_transitions
 			> ((SIZE_MAX - (__alignof__ (struct ttinfo) - 1))
-			   / (sizeof (internal_time_t) + 1)), 0))
+			   / (sizeof (__time64_t) + 1)), 0))
     goto lose;
-  total_size = num_transitions * (sizeof (internal_time_t) + 1);
+  total_size = num_transitions * (sizeof (__time64_t) + 1);
   total_size = ((total_size + __alignof__ (struct ttinfo) - 1)
 		& ~(__alignof__ (struct ttinfo) - 1));
   types_idx = total_size;
@@ -276,7 +276,7 @@  __tzfile_read (const char *file, size_t extra, char **extrap)
     goto lose;
 
   type_idxs = (unsigned char *) transitions + (num_transitions
-					       * sizeof (internal_time_t));
+					       * sizeof (__time64_t));
   types = (struct ttinfo *) ((char *) transitions + types_idx);
   zone_names = (char *) types + num_types * sizeof (struct ttinfo);
   leaps = (struct leap *) ((char *) transitions + leaps_idx);
@@ -578,7 +578,7 @@  __tzfile_default (const char *std, const char *dst,
 }
 
 void
-__tzfile_compute (internal_time_t timer, int use_localtime,
+__tzfile_compute (__time64_t timer, int use_localtime,
 		  long int *leap_correct, int *leap_hit,
 		  struct tm *tp)
 {
@@ -667,7 +667,7 @@  __tzfile_compute (internal_time_t timer, int use_localtime,
 	     initial search spot from it.  Half of a gregorian year
 	     has on average 365.2425 * 86400 / 2 = 15778476 seconds.
 	     The value i can be truncated if size_t is smaller than
-	     internal_time_t, but this is harmless because it is just
+	     __time64_t, but this is harmless because it is just
 	     a guess.  */
 	  i = (transitions[num_transitions - 1] - timer) / 15778476;
 	  if (i < num_transitions)