[prev in list] [next in list] [prev in thread] [next in thread] 

List:       glibc-cvs
Subject:    [glibc/zack/y2038-preliminaries] Warn when gettimeofday is called with non-null tzp argument.
From:       Zack Weinberg <zack () sourceware ! org>
Date:       2019-08-28 13:05:14
Message-ID: 20190828130514.20044.qmail () sourceware ! org
[Download RAW message or body]

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=c50ec7ce062883cbb3d1820b2c1ea262ca5fff5b

commit c50ec7ce062883cbb3d1820b2c1ea262ca5fff5b
Author: Zack Weinberg <zackw@panix.com>
Date:   Mon Aug 19 13:51:25 2019 -0400

    Warn when gettimeofday is called with non-null tzp argument.
    
    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.
    
    Also make the public prototype of gettimeofday declare its second
    argument with type "void *" unconditionally, consistent with POSIX.
    
    	* 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.

Diff:
---
 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 8eeea39..3eab53b 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 c4f6426..5bc91fc 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 5dbc7fc..a4e7fd2 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;
[prev in list] [next in list] [prev in thread] [next in thread]