Message ID | 87sf02guiz.fsf@oldenburg.str.redhat.com |
---|---|
State | New |
Headers | show |
Series | login: Use unsigned 32-bit types for seconds-since-epoch | expand |
On 4/3/24 11:39, Florian Weimer wrote: > For consistency, > if there is a 64-bit architecture that is coinstallable, define > __WORDSIZE_TIME64_COMPAT32 to 1 on the 32-bit architectyre as well. Could you explain the advantage of consistency here? User code almost invariably assignes ut_tv.tv_sec to time_t (this is true of every instance I found of ut_tv in Gnulib source code, for example). So changing this field's type on platforms where time_t is 32 bits will likely be ineffective in practice, and might cause more problems than it cures. Since the _TIME_BITS==32 world is going away anyway by 2038, it might be better to leave the field types unchanged in that world, to lessen compatibility glitches before that world gives up its ghost.
* Paul Eggert: > On 4/3/24 11:39, Florian Weimer wrote: >> For consistency, >> if there is a 64-bit architecture that is coinstallable, define >> __WORDSIZE_TIME64_COMPAT32 to 1 on the 32-bit architectyre as well. > > Could you explain the advantage of consistency here? User code almost > invariably assignes ut_tv.tv_sec to time_t (this is true of every > instance I found of ut_tv in Gnulib source code, for example). So > changing this field's type on platforms where time_t is 32 bits will > likely be ineffective in practice, and might cause more problems than > it cures. Few applications with a 32-bit time_t will work once there is a value in this field with the MSB set. So the relevant case is applications that were built with -D_TIME_BITS=64, and there the consistent behavior with the 64-bit architecture helps. > Since the _TIME_BITS==32 world is going away anyway by 2038, it might > be better to leave the field types unchanged in that world, to lessen > compatibility glitches before that world gives up its ghost. This is an on-disk format that doesn't change with -D_TIME_BITS=64. Thanks, Florian
On 04/04/24 02:09, Florian Weimer wrote: > * Paul Eggert: > >> On 4/3/24 11:39, Florian Weimer wrote: >>> For consistency, >>> if there is a 64-bit architecture that is coinstallable, define >>> __WORDSIZE_TIME64_COMPAT32 to 1 on the 32-bit architectyre as well. >> >> Could you explain the advantage of consistency here? User code almost >> invariably assignes ut_tv.tv_sec to time_t (this is true of every >> instance I found of ut_tv in Gnulib source code, for example). So >> changing this field's type on platforms where time_t is 32 bits will >> likely be ineffective in practice, and might cause more problems than >> it cures. > > Few applications with a 32-bit time_t will work once there is a value in > this field with the MSB set. So the relevant case is applications that > were built with -D_TIME_BITS=64, and there the consistent behavior with > the 64-bit architecture helps. This helps only architectures with __WORDSIZE_TIME64_COMPAT32, so it does not really solve the issue for all legacy platforms. I still prefer if we just deprecate this whole interface, since from other legacy ABI history (non-LFS interface) programs will keep using it until something breaks. > >> Since the _TIME_BITS==32 world is going away anyway by 2038, it might >> be better to leave the field types unchanged in that world, to lessen >> compatibility glitches before that world gives up its ghost. > > This is an on-disk format that doesn't change with -D_TIME_BITS=64. > > Thanks, > Florian >
* Adhemerval Zanella Netto: > On 04/04/24 02:09, Florian Weimer wrote: >> * Paul Eggert: >> >>> On 4/3/24 11:39, Florian Weimer wrote: >>>> For consistency, >>>> if there is a 64-bit architecture that is coinstallable, define >>>> __WORDSIZE_TIME64_COMPAT32 to 1 on the 32-bit architectyre as well. >>> >>> Could you explain the advantage of consistency here? User code almost >>> invariably assignes ut_tv.tv_sec to time_t (this is true of every >>> instance I found of ut_tv in Gnulib source code, for example). So >>> changing this field's type on platforms where time_t is 32 bits will >>> likely be ineffective in practice, and might cause more problems than >>> it cures. >> >> Few applications with a 32-bit time_t will work once there is a value in >> this field with the MSB set. So the relevant case is applications that >> were built with -D_TIME_BITS=64, and there the consistent behavior with >> the 64-bit architecture helps. > > This helps only architectures with __WORDSIZE_TIME64_COMPAT32, so it does > not really solve the issue for all legacy platforms. We can add more __WORDSIZE_TIME64_COMPAT32=1 definitions. > I still prefer if we just deprecate this whole interface, since from > other legacy ABI history (non-LFS interface) programs will keep using > it until something breaks. I don't disagree, I just want to give distributions the option to backport a reviewed patch with a workaround that appears not to require much application porting. I believe many of us are preparing toolchains for distributions that are running close to the 2038 cliff. Thanks, Florian
On 05/04/24 09:52, Florian Weimer wrote: > * Adhemerval Zanella Netto: > >> On 04/04/24 02:09, Florian Weimer wrote: >>> * Paul Eggert: >>> >>>> On 4/3/24 11:39, Florian Weimer wrote: >>>>> For consistency, >>>>> if there is a 64-bit architecture that is coinstallable, define >>>>> __WORDSIZE_TIME64_COMPAT32 to 1 on the 32-bit architectyre as well. >>>> >>>> Could you explain the advantage of consistency here? User code almost >>>> invariably assignes ut_tv.tv_sec to time_t (this is true of every >>>> instance I found of ut_tv in Gnulib source code, for example). So >>>> changing this field's type on platforms where time_t is 32 bits will >>>> likely be ineffective in practice, and might cause more problems than >>>> it cures. >>> >>> Few applications with a 32-bit time_t will work once there is a value in >>> this field with the MSB set. So the relevant case is applications that >>> were built with -D_TIME_BITS=64, and there the consistent behavior with >>> the 64-bit architecture helps. >> >> This helps only architectures with __WORDSIZE_TIME64_COMPAT32, so it does >> not really solve the issue for all legacy platforms. > > We can add more __WORDSIZE_TIME64_COMPAT32=1 definitions. > >> I still prefer if we just deprecate this whole interface, since from >> other legacy ABI history (non-LFS interface) programs will keep using >> it until something breaks. > > I don't disagree, I just want to give distributions the option to > backport a reviewed patch with a workaround that appears not to require > much application porting. I believe many of us are preparing toolchains > for distributions that are running close to the 2038 cliff. So maybe add deprecated on utmp/utmpx functions, along with a NEWS entry stating these function will make noop in future glibc release?
* Adhemerval Zanella Netto: >>> I still prefer if we just deprecate this whole interface, since from >>> other legacy ABI history (non-LFS interface) programs will keep using >>> it until something breaks. >> >> I don't disagree, I just want to give distributions the option to >> backport a reviewed patch with a workaround that appears not to require >> much application porting. I believe many of us are preparing toolchains >> for distributions that are running close to the 2038 cliff. > > So maybe add deprecated on utmp/utmpx functions, along with a NEWS entry > stating these function will make noop in future glibc release? I added a second NEWS entry in v2. Thanks, Florian
diff --git a/NEWS b/NEWS index da4b2223e9..2b2484b983 100644 --- a/NEWS +++ b/NEWS @@ -28,7 +28,11 @@ Major new features: Deprecated and removed features, and other changes affecting compatibility: - [Add deprecations, removals and changes affecting compatibility here] +* Architectures which use a 32-bit seconds-since-epoch field in struct + lastlog, struct utmp, struct utmpx (such as i386, powerpc64le, rv32, + rv64, x86-64) switched from a signed to an unsigned type for that + field. This allows these fields to store timestamps beyond the + year 2038. Changes to build and runtime requirements: diff --git a/bits/utmp.h b/bits/utmp.h index f2d1c13d8c..27cb536800 100644 --- a/bits/utmp.h +++ b/bits/utmp.h @@ -36,7 +36,7 @@ struct lastlog { #if __WORDSIZE_TIME64_COMPAT32 - int32_t ll_time; + __uint32_t ll_time; #else __time_t ll_time; #endif @@ -76,7 +76,7 @@ struct utmp int32_t ut_session; /* Session ID, used for windowing. */ struct { - int32_t tv_sec; /* Seconds. */ + __uint32_t tv_sec; /* Seconds. */ int32_t tv_usec; /* Microseconds. */ } ut_tv; /* Time entry was made. */ #else diff --git a/sysdeps/gnu/bits/utmpx.h b/sysdeps/gnu/bits/utmpx.h index 34b4afbc6a..ed0df9bd81 100644 --- a/sysdeps/gnu/bits/utmpx.h +++ b/sysdeps/gnu/bits/utmpx.h @@ -74,7 +74,7 @@ struct utmpx __int32_t ut_session; /* Session ID, used for windowing. */ struct { - __int32_t tv_sec; /* Seconds. */ + __uint32_t tv_sec; /* Seconds. */ __int32_t tv_usec; /* Microseconds. */ } ut_tv; /* Time entry was made. */ #else diff --git a/sysdeps/mips/bits/wordsize.h b/sysdeps/mips/bits/wordsize.h index 57f0f2a22f..30dd3fd85d 100644 --- a/sysdeps/mips/bits/wordsize.h +++ b/sysdeps/mips/bits/wordsize.h @@ -19,11 +19,7 @@ #define __WORDSIZE _MIPS_SZPTR -#if _MIPS_SIM == _ABI64 -# define __WORDSIZE_TIME64_COMPAT32 1 -#else -# define __WORDSIZE_TIME64_COMPAT32 0 -#endif +#define __WORDSIZE_TIME64_COMPAT32 1 #if __WORDSIZE == 32 #define __WORDSIZE32_SIZE_ULONG 0 diff --git a/sysdeps/powerpc/powerpc32/bits/wordsize.h b/sysdeps/powerpc/powerpc32/bits/wordsize.h index 04ca9debf0..6993fb6b29 100644 --- a/sysdeps/powerpc/powerpc32/bits/wordsize.h +++ b/sysdeps/powerpc/powerpc32/bits/wordsize.h @@ -2,10 +2,9 @@ #if defined __powerpc64__ # define __WORDSIZE 64 -# define __WORDSIZE_TIME64_COMPAT32 1 #else # define __WORDSIZE 32 -# define __WORDSIZE_TIME64_COMPAT32 0 # define __WORDSIZE32_SIZE_ULONG 0 # define __WORDSIZE32_PTRDIFF_LONG 0 #endif +#define __WORDSIZE_TIME64_COMPAT32 1 diff --git a/sysdeps/sparc/sparc32/bits/wordsize.h b/sysdeps/sparc/sparc32/bits/wordsize.h index 4bbd2e63b4..a2e79e0fa9 100644 --- a/sysdeps/sparc/sparc32/bits/wordsize.h +++ b/sysdeps/sparc/sparc32/bits/wordsize.h @@ -1,6 +1,6 @@ /* Determine the wordsize from the preprocessor defines. */ #define __WORDSIZE 32 -#define __WORDSIZE_TIME64_COMPAT32 0 +#define __WORDSIZE_TIME64_COMPAT32 1 #define __WORDSIZE32_SIZE_ULONG 0 #define __WORDSIZE32_PTRDIFF_LONG 0 diff --git a/sysdeps/sparc/sparc64/bits/wordsize.h b/sysdeps/sparc/sparc64/bits/wordsize.h index 2f66f10d72..ea103e5970 100644 --- a/sysdeps/sparc/sparc64/bits/wordsize.h +++ b/sysdeps/sparc/sparc64/bits/wordsize.h @@ -2,10 +2,9 @@ #if defined __arch64__ || defined __sparcv9 # define __WORDSIZE 64 -# define __WORDSIZE_TIME64_COMPAT32 1 #else # define __WORDSIZE 32 -# define __WORDSIZE_TIME64_COMPAT32 0 # define __WORDSIZE32_SIZE_ULONG 0 # define __WORDSIZE32_PTRDIFF_LONG 0 #endif +#define __WORDSIZE_TIME64_COMPAT32 1 diff --git a/sysdeps/unix/sysv/linux/sparc/bits/wordsize.h b/sysdeps/unix/sysv/linux/sparc/bits/wordsize.h index 7562875ee2..ea103e5970 100644 --- a/sysdeps/unix/sysv/linux/sparc/bits/wordsize.h +++ b/sysdeps/unix/sysv/linux/sparc/bits/wordsize.h @@ -2,10 +2,9 @@ #if defined __arch64__ || defined __sparcv9 # define __WORDSIZE 64 -# define __WORDSIZE_TIME64_COMPAT32 1 #else # define __WORDSIZE 32 # define __WORDSIZE32_SIZE_ULONG 0 # define __WORDSIZE32_PTRDIFF_LONG 0 -# define __WORDSIZE_TIME64_COMPAT32 0 #endif +#define __WORDSIZE_TIME64_COMPAT32 1 diff --git a/sysdeps/x86/bits/wordsize.h b/sysdeps/x86/bits/wordsize.h index 70f652bca1..b1442406b4 100644 --- a/sysdeps/x86/bits/wordsize.h +++ b/sysdeps/x86/bits/wordsize.h @@ -8,10 +8,9 @@ #define __WORDSIZE32_PTRDIFF_LONG 0 #endif +#define __WORDSIZE_TIME64_COMPAT32 1 + #ifdef __x86_64__ -# define __WORDSIZE_TIME64_COMPAT32 1 /* Both x86-64 and x32 use the 64-bit system call interface. */ # define __SYSCALL_WORDSIZE 64 -#else -# define __WORDSIZE_TIME64_COMPAT32 0 #endif