Message ID | 20180926073053.11710-1-albert.aribaud@3adev.fr |
---|---|
State | New |
Headers | show |
Series | [v8,1/2] Y2038: Add 64-bit time for all architectures | expand |
* Albert ARIBAUD: > 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 > @@ -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 Should the __TIMESIZE == 64 case use typedef as well? > 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..81de09e23f > --- /dev/null > +++ b/sysdeps/unix/sysv/linux/x86/bits/time64.h > +#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 then use it. */ > +# define __TIME64_T_TYPE __TIME_T_TYPE > +#else > +/* Define a 64-bit type alongsize the 32-bit one. */ > +# define __TIME64_T_TYPE __SQUAD_TYPE > +#endif Perhaps I'm missing something, but I think this should be put into a generic header and could be written like this: #if __TIMESIZE == 64 # define __TIME64_T_TYPE __TIME_T_TYPE #else # if __WORDSIZE != 32 # error "32-bit word size expected for non-64-bit time_t" # endif # define __TIME64_T_TYPE __SQUAD_TYPE #endif I don't think there is a 64-bit port with a 32-bit time_t. > 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 > +#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 I think writing this as #ifdef __x86_64__ /* This includes x32, where time_t is 64-bit even though the word size is 32-bit. */ # define __TIMESIZE 64 #else # define __TIMESIZE 32 #endif is much clearer. It's not that there's going to be a different x86-64 API with yet another time_t size any time soon. > index 72ef75f074..844a68de8c 100644 > --- a/time/tzfile.c > +++ b/time/tzfile.c The tzfile.c changes look okay to me. Thanks, Florian
Hi Florian, On Wed, 26 Sep 2018 10:39:59 +0200, Florian Weimer <fweimer@redhat.com> wrote : > * Albert ARIBAUD: > > > 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 > > > @@ -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 > > Should the __TIMESIZE == 64 case use typedef as well? It was requested that the Y038 code cause as little change as possible at the object level in the TIMESIZE == 64 case, hence the #define which ensures that after the preprocessor stage, the compiler can only see 'time_t', not 'time64_t'. > > 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..81de09e23f > > --- /dev/null > > +++ b/sysdeps/unix/sysv/linux/x86/bits/time64.h > > > +#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 then use it. */ > > +# define __TIME64_T_TYPE __TIME_T_TYPE > > +#else > > +/* Define a 64-bit type alongsize the 32-bit one. */ > > +# define __TIME64_T_TYPE __SQUAD_TYPE > > +#endif > > Perhaps I'm missing something, but I think this should be put into a > generic header and could be written like this: > > #if __TIMESIZE == 64 > # define __TIME64_T_TYPE __TIME_T_TYPE > #else > # if __WORDSIZE != 32 > # error "32-bit word size expected for non-64-bit time_t" > # endif > # define __TIME64_T_TYPE __SQUAD_TYPE > #endif > > I don't think there is a 64-bit port with a 32-bit time_t. The patch does provide definitions for __TIMESIZE and __TIME64_T_TYPE in generic headers (bits/timesize and bits/time64.h respectively) which cover all cases for all 64-bit architectures and all 32-bit architectures. Since the only exception is X32, it makes sense that the definitions which take this exception into account be put in the most X32 specific header, which is the x86 one. This organization stems from previous comments by Joseph Myers: https://patchwork.ozlabs.org/patch/930876/#1935670 > > 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 > > > +#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 > > I think writing this as > > #ifdef __x86_64__ > /* This includes x32, where time_t is 64-bit even though the word size > is 32-bit. */ > # define __TIMESIZE 64 > #else > # define __TIMESIZE 32 > #endif > > is much clearer. It's not that there's going to be a different x86-64 > API with yet another time_t size any time soon. I prefer the #if/#else condition to explicitly involve x32 (through the symbol __ILP32__) rather than implicitly. > > index 72ef75f074..844a68de8c 100644 > > --- a/time/tzfile.c > > +++ b/time/tzfile.c > > The tzfile.c changes look okay to me. Thanks! > Thanks, > Florian Cordialement, Albert ARIBAUD 3ADEV
* Albert ARIBAUD: > Hi Florian, > > On Wed, 26 Sep 2018 10:39:59 +0200, Florian Weimer <fweimer@redhat.com> > wrote : > >> * Albert ARIBAUD: >> >> > 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 >> >> > @@ -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 >> >> Should the __TIMESIZE == 64 case use typedef as well? > > It was requested that the Y038 code cause as little change as possible > at the object level in the TIMESIZE == 64 case, hence the #define which > ensures that after the preprocessor stage, the compiler can only see > 'time_t', not 'time64_t'. > >> > 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..81de09e23f >> > --- /dev/null >> > +++ b/sysdeps/unix/sysv/linux/x86/bits/time64.h >> >> > +#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 then use it. */ >> > +# define __TIME64_T_TYPE __TIME_T_TYPE >> > +#else >> > +/* Define a 64-bit type alongsize the 32-bit one. */ >> > +# define __TIME64_T_TYPE __SQUAD_TYPE >> > +#endif >> >> Perhaps I'm missing something, but I think this should be put into a >> generic header and could be written like this: >> >> #if __TIMESIZE == 64 >> # define __TIME64_T_TYPE __TIME_T_TYPE >> #else >> # if __WORDSIZE != 32 >> # error "32-bit word size expected for non-64-bit time_t" >> # endif >> # define __TIME64_T_TYPE __SQUAD_TYPE >> #endif >> >> I don't think there is a 64-bit port with a 32-bit time_t. > > The patch does provide definitions for __TIMESIZE and __TIME64_T_TYPE > in generic headers (bits/timesize and bits/time64.h respectively) > which cover all cases for all 64-bit architectures and all 32-bit > architectures. > > Since the only exception is X32, it makes sense that the definitions > which take this exception into account be put in the most X32 specific > header, which is the x86 one. My concern is that x32 is the only exception today, but future ports will need the logic I sketched above as well. > This organization stems from previous comments by Joseph Myers: > > https://patchwork.ozlabs.org/patch/930876/#1935670 Sorry. 8-( Florian
On Wed, 26 Sep 2018, Albert ARIBAUD (3ADEV) wrote: > - Adding 64-bit-time support in the glibc public API. > This support should be dynamic, i.e. iglibc should s/iglibc/glibc/ > +/* Define __TIME64_T_TYPE so that it is always a 64-bit type. > + * On 64-bit platforms and X32, there is already a 64-bit time. > + * On 32-bit platforms except X32, we need to create one. */ I don't think the comments on the generic header should be referencing specific platforms such as x32 like that.
diff --git a/bits/time64.h b/bits/time64.h new file mode 100644 index 0000000000..4fd60fef7e --- /dev/null +++ b/bits/time64.h @@ -0,0 +1,38 @@ +/* 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. + * On 64-bit platforms and X32, there is already a 64-bit time. + * On 32-bit platforms except X32, we need to create one. */ + +#if __TIMESIZE == 64 +/* If we already have 64-bit time then use it. */ +# define __TIME64_T_TYPE __TIME_T_TYPE +#else +/* Define a 64-bit 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..81de09e23f --- /dev/null +++ b/sysdeps/unix/sysv/linux/x86/bits/time64.h @@ -0,0 +1,41 @@ +/* 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. + * On 64-bit platforms and X32, there is already a 64-bit time. + * On 32-bit platforms except X32, we need to create one. */ + +#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 then use it. */ +# define __TIME64_T_TYPE __TIME_T_TYPE +#else +/* Define a 64-bit 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)