Message ID | 20190828153236.18229-10-zackw@panix.com |
---|---|
State | New |
Headers | show |
Series | Y2038 preparation: use clock_[gs]ettime to implement the other time-getting and -setting functions. | expand |
On 28/08/2019 12:32, Zack Weinberg wrote: > Since there are no known uses of gettimeofday's vestigial "get time > zone" feature that are not bugs, add a fortify-style wrapper inline to > sys/time.h that issues a warning whenever gettimeofday is called with > a second argument that is not a compile-time null pointer > constant. > > At present this is only possible with GCC; clang does not implement > attribute((warning)). The wrapper is only activated when __OPTIMIZE__ > is defined because it throws false positives when optimization is off, > even though it's an always-inline function. > > An oversight in the implementation of __builtin_constant_p causes it > to fail to detect compile-time *pointer* constants unless they are > cast to an integer of a different size. (Loss of data in this cast is > harmless; the overall expression is still constant if and only if the > original pointer was.) This is GCC bug 95514. Thanks to > Kamil Cukrowski <kamilcukrowski@gmail.com> for the workaround. > As a precaution, I added a static assertion to debug/warning-nop.c to > make sure that the cast _is_ casting to an integer of a different > size; this is too unlikely a scenario to be worth checking in the > public header, but if someone ever adds a port where short is the > same size as intptr_t, we'll still catch it. The conditionals of making this work without false-positive seems quite specific and fragile (gcc-only, optimized build) and the cast hack make even more suspicious it won't break in some arcane build environment (since it is an exported header, not internally used). I would prefer to just wait gcc to correctly fix it and set its minimum version to enable it with the expected semantic. > > Also make the public prototype of gettimeofday declare its second > argument with type "void *" unconditionally, consistent with POSIX. This is ok though. > > * time/sys/time.h (__timezone_ptr_t): Delete. > (gettimeofday): Always declare second argument with type "void *". > When possible, wrap with a fortify-style inline function that > detects non-null or non-constant second argument and issues a > warning. Improve commentary. > (settimeofday): Improve commentary. > > * time/gettimeofday.c (gettimeofday): Declare second argument with > type "void *". > * debug/warning-nop.c: Include sys/time.h and stdint.h. > Add static_assert to verify the requirements of the workaround > for GCC bug 95514. > --- > debug/warning-nop.c | 9 +++++++++ > time/gettimeofday.c | 4 ++-- > time/sys/time.h | 47 ++++++++++++++++++++++++++++++++++----------- > 3 files changed, 47 insertions(+), 13 deletions(-) > > diff --git a/debug/warning-nop.c b/debug/warning-nop.c > index 8eeea396c3..3eab53b78f 100644 > --- a/debug/warning-nop.c > +++ b/debug/warning-nop.c > @@ -67,4 +67,13 @@ nop (void) > #define __builtin___strncpy_chk(dest, src, len, bos) NULL > #define __builtin_object_size(bos, level) 0 > > +/* The code in sys/time.h that uses __warndecl has to work around GCC > + bug 91554. The work-around is only effective if intptr_t is not > + the same size as short. */ > +#include <stdint.h> > +_Static_assert (sizeof (intptr_t) != sizeof (short), > + "workaround for GCC bug 91554 in sys/time.h" > + " is only effective when short is smaller than a pointer"); > + > #include <string.h> > +#include <sys/time.h> > diff --git a/time/gettimeofday.c b/time/gettimeofday.c > index c4f642631f..5bc91fc214 100644 > --- a/time/gettimeofday.c > +++ b/time/gettimeofday.c > @@ -23,10 +23,10 @@ > If *TZ is not NULL, clear it. > Returns 0 on success, -1 on errors. */ > int > -___gettimeofday (struct timeval *tv, struct timezone *tz) > +___gettimeofday (struct timeval *restrict tv, void *restrict tz) > { > if (__glibc_unlikely (tz != 0)) > - memset (tz, 0, sizeof *tz); > + memset (tz, 0, sizeof (struct timezone)); > > struct timespec ts; > if (__clock_gettime (CLOCK_REALTIME, &ts)) > diff --git a/time/sys/time.h b/time/sys/time.h > index 5dbc7fc627..a4e7fd20d1 100644 > --- a/time/sys/time.h > +++ b/time/sys/time.h > @@ -54,23 +54,48 @@ struct timezone > int tz_minuteswest; /* Minutes west of GMT. */ > int tz_dsttime; /* Nonzero if DST is ever in effect. */ > }; > - > -typedef struct timezone *__restrict __timezone_ptr_t; > -#else > -typedef void *__restrict __timezone_ptr_t; > #endif > > -/* Get the current time of day and timezone information, > - putting it into *TV and *TZ. If TZ is NULL, *TZ is not filled. > - Returns 0 on success, -1 on errors. > - NOTE: This form of timezone information is obsolete. > - Use the functions and variables declared in <time.h> instead. */ > +/* Get the current time of day, putting it into *TV. > + If TZ is not null, *TZ must be a struct timezone, and both fields > + will be set to zero. > + Calling this function with a non-null TZ is obsolete; > + use localtime etc. instead. > + This function itself is semi-obsolete; > + most callers should use time or clock_gettime instead. */ > extern int gettimeofday (struct timeval *__restrict __tv, > - __timezone_ptr_t __tz) __THROW __nonnull ((1)); > + void *__restrict __tz) __THROW __nonnull ((1)); > + > +#if __GNUC_PREREQ (4,3) && defined __REDIRECT && defined __OPTIMIZE__ > +/* Issue a warning for use of gettimeofday with a non-null __tz argument. */ > +__warndecl (__warn_gettimeofday_nonnull_timezone, > + "gettimeofday with non-null or non-constant timezone parameter;" > + " this is obsolete and inaccurate, use localtime instead"); > + > +extern int __REDIRECT_NTH (__gettimeofday_alias, > + (struct timeval *__restrict __tv, > + void *__restrict __tz), gettimeofday) > + __nonnull ((1)); > + > +/* The double cast below works around a limitation in __builtin_constant_p > + in all released versions of GCC (as of August 2019). > + See <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91554>. */ > +__fortify_function int > +__NTH (gettimeofday (struct timeval *__restrict __tv, void *__restrict __tz)) > +{ > + if (! (__builtin_constant_p ((short) (__intptr_t) __tz) && __tz == 0)) > + __warn_gettimeofday_nonnull_timezone (); > + > + return __gettimeofday_alias (__tv, __tz); > +} > +#endif > > #ifdef __USE_MISC > /* Set the current time of day and timezone information. > - This call is restricted to the super-user. */ > + This call is restricted to the super-user. > + Setting the timezone in this way is obsolete, but we don't yet > + warn about it because it still has some uses for which there is > + no alternative. */ > extern int settimeofday (const struct timeval *__tv, > const struct timezone *__tz) > __THROW; >
diff --git a/debug/warning-nop.c b/debug/warning-nop.c index 8eeea396c3..3eab53b78f 100644 --- a/debug/warning-nop.c +++ b/debug/warning-nop.c @@ -67,4 +67,13 @@ nop (void) #define __builtin___strncpy_chk(dest, src, len, bos) NULL #define __builtin_object_size(bos, level) 0 +/* The code in sys/time.h that uses __warndecl has to work around GCC + bug 91554. The work-around is only effective if intptr_t is not + the same size as short. */ +#include <stdint.h> +_Static_assert (sizeof (intptr_t) != sizeof (short), + "workaround for GCC bug 91554 in sys/time.h" + " is only effective when short is smaller than a pointer"); + #include <string.h> +#include <sys/time.h> diff --git a/time/gettimeofday.c b/time/gettimeofday.c index c4f642631f..5bc91fc214 100644 --- a/time/gettimeofday.c +++ b/time/gettimeofday.c @@ -23,10 +23,10 @@ If *TZ is not NULL, clear it. Returns 0 on success, -1 on errors. */ int -___gettimeofday (struct timeval *tv, struct timezone *tz) +___gettimeofday (struct timeval *restrict tv, void *restrict tz) { if (__glibc_unlikely (tz != 0)) - memset (tz, 0, sizeof *tz); + memset (tz, 0, sizeof (struct timezone)); struct timespec ts; if (__clock_gettime (CLOCK_REALTIME, &ts)) diff --git a/time/sys/time.h b/time/sys/time.h index 5dbc7fc627..a4e7fd20d1 100644 --- a/time/sys/time.h +++ b/time/sys/time.h @@ -54,23 +54,48 @@ struct timezone int tz_minuteswest; /* Minutes west of GMT. */ int tz_dsttime; /* Nonzero if DST is ever in effect. */ }; - -typedef struct timezone *__restrict __timezone_ptr_t; -#else -typedef void *__restrict __timezone_ptr_t; #endif -/* Get the current time of day and timezone information, - putting it into *TV and *TZ. If TZ is NULL, *TZ is not filled. - Returns 0 on success, -1 on errors. - NOTE: This form of timezone information is obsolete. - Use the functions and variables declared in <time.h> instead. */ +/* Get the current time of day, putting it into *TV. + If TZ is not null, *TZ must be a struct timezone, and both fields + will be set to zero. + Calling this function with a non-null TZ is obsolete; + use localtime etc. instead. + This function itself is semi-obsolete; + most callers should use time or clock_gettime instead. */ extern int gettimeofday (struct timeval *__restrict __tv, - __timezone_ptr_t __tz) __THROW __nonnull ((1)); + void *__restrict __tz) __THROW __nonnull ((1)); + +#if __GNUC_PREREQ (4,3) && defined __REDIRECT && defined __OPTIMIZE__ +/* Issue a warning for use of gettimeofday with a non-null __tz argument. */ +__warndecl (__warn_gettimeofday_nonnull_timezone, + "gettimeofday with non-null or non-constant timezone parameter;" + " this is obsolete and inaccurate, use localtime instead"); + +extern int __REDIRECT_NTH (__gettimeofday_alias, + (struct timeval *__restrict __tv, + void *__restrict __tz), gettimeofday) + __nonnull ((1)); + +/* The double cast below works around a limitation in __builtin_constant_p + in all released versions of GCC (as of August 2019). + See <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91554>. */ +__fortify_function int +__NTH (gettimeofday (struct timeval *__restrict __tv, void *__restrict __tz)) +{ + if (! (__builtin_constant_p ((short) (__intptr_t) __tz) && __tz == 0)) + __warn_gettimeofday_nonnull_timezone (); + + return __gettimeofday_alias (__tv, __tz); +} +#endif #ifdef __USE_MISC /* Set the current time of day and timezone information. - This call is restricted to the super-user. */ + This call is restricted to the super-user. + Setting the timezone in this way is obsolete, but we don't yet + warn about it because it still has some uses for which there is + no alternative. */ extern int settimeofday (const struct timeval *__tv, const struct timezone *__tz) __THROW;