problems with unsigned 32-bit time_t coming up
I vaguely recalled someone reporting trouble with the tz database on NetBSD or some other BSD in the past few months. The gist of it was that this platform has 32-bit 'long' and 64-bit 'time_t', and the tz code didn't play nicely on such a system. To try to catch this sort of thing I'd like to add support for trying different time_t types, even if they disagree with the system's time_t. I'll send a couple of proposed patches to the list to do that. I'll then follow up with a report of a bug in the reference implementation that is discovered by the patches, when configured to run with an unsigned 32-bit time_t. This isn't the NetBSD problem, but hey! at least there's an easy-to-run test case that illustrates the bug.
This makes it easier to test on (say) Debian, even if we're testing the time_t type on (say) NetBSD. NetBSD uses 64-bit time_t on 32-bit hosts, and this lets us test a NetBSD-style implementation (32-bit 'long', 64-bit time_t) on a 32-bit Debian host. * Makefile: Update comments to talk about TIME_T_FLOATING and time_tz. Sort the comments. * private.h (restrict): Define to empty with older compilers. 'restrict' is now needed, to define gmtime_r and localtime_r in standard ways when time_tz is defined. Make the following changes if time_tz is defined: (sys_time, time): New static functions. The former is the system 'time' function that applies to the system time_t, the latter our function that applies to our time_t. (time_t, ctime, ctime_r, difftime, gmtime, gmtime_r, localtime) (localtime_r, mktime): Rename to tz_time_t, tz_ctime, etc., via macros. Declare the renamed versions. * zdump.8: Document new options -V, -t. * zdump.c: Include private.h if time_tz is defined. (INITIALIZE): Remove; no longer needed. (absolute_min_time, absolute_max_time): Work even if time_t is wider than intmax_t, which can be true with GCC and __int128_t. Use the new TIME_T_FLOATING macro for this. (usage): Document new flags. (main): Support them. --- Makefile | 16 +++---- private.h | 56 +++++++++++++++++++++++++ zdump.8 | 17 +++++++- zdump.c | 140 ++++++++++++++++++++++++++++++++++++++------------------------ 4 files changed, 166 insertions(+), 63 deletions(-) diff --git a/Makefile b/Makefile index 0d6df64..cca817c 100644 --- a/Makefile +++ b/Makefile @@ -109,21 +109,23 @@ LDLIBS= # -DHAVE_SYMLINK=0 if your system lacks the symlink function # -DHAVE_SYS_STAT_H=0 if your compiler lacks a "sys/stat.h" # -DHAVE_SYS_WAIT_H=0 if your compiler lacks a "sys/wait.h" -# -DLOCALE_HOME=\"path\" if locales are in "path", not "/usr/lib/locale" # -DHAVE_UNISTD_H=0 if your compiler lacks a "unistd.h" (Microsoft C++ 7?) # -DHAVE_UTMPX_H=1 if your compiler has a "utmpx.h" -# -DTZDEFRULESTRING=\",date/time,date/time\" to default to the specified -# DST transitions if the time zone files cannot be accessed -# -DTZ_DOMAIN=\"foo\" to use "foo" for gettext domain name; default is "tz" -# -TTZ_DOMAINDIR=\"/path\" to use "/path" for gettext directory; -# the default is system-supplied, typically "/usr/lib/locale" -# $(GCC_DEBUG_FLAGS) if you are using GCC and want lots of checking +# -DLOCALE_HOME=\"path\" if locales are in "path", not "/usr/lib/locale" # -DNO_RUN_TIME_WARNINGS_ABOUT_YEAR_2000_PROBLEMS_THANK_YOU=1 # if you do not want run time warnings about formats that may cause # year 2000 grief +# -DTIME_T_FLOATING=1 if your time_t (or time_tz) is floating point +# -Dtime_tz=\"T\" to use T as the time_t type, rather than the system time_t +# -DTZ_DOMAIN=\"foo\" to use "foo" for gettext domain name; default is "tz" +# -TTZ_DOMAINDIR=\"/path\" to use "/path" for gettext directory; +# the default is system-supplied, typically "/usr/lib/locale" +# -DTZDEFRULESTRING=\",date/time,date/time\" to default to the specified +# DST transitions if the time zone files cannot be accessed # -DZIC_MAX_ABBR_LEN_WO_WARN=3 # (or some other number) to set the maximum time zone abbreviation length # that zic will accept without a warning (the default is 6) +# $(GCC_DEBUG_FLAGS) if you are using GCC and want lots of checking GCC_DEBUG_FLAGS = -Dlint -g3 -O3 -fno-common -fstrict-aliasing \ -Wall -Wextra \ -Wbad-function-cast -Wcast-align -Wcast-qual \ diff --git a/private.h b/private.h index 1d1d391..05ddba0 100644 --- a/private.h +++ b/private.h @@ -150,6 +150,10 @@ typedef long int_fast64_t; # define ATTRIBUTE_PURE /* empty */ #endif +#if __STDC_VERSION__ < 199901 && !defined restrict +# define restrict /* empty */ +#endif + /* ** Workarounds for compilers/systems. */ @@ -165,6 +169,58 @@ extern char * asctime_r(struct tm const *, char *); #endif /* +** Compile with -Dtime_tz=T to build the tz package with a private +** time_t type equivalent to T rather than the system-supplied time_t. +** This debugging feature can test unusual design decisions +** (e.g., time_t wider than 'long', or unsigned time_t) even on +** typical platforms. +*/ +#ifdef time_tz +static time_t sys_time(time_t *x) { return time(x); } + +# undef ctime +# define ctime tz_ctime +# undef ctime_r +# define ctime_r tz_ctime_r +# undef difftime +# define difftime tz_difftime +# undef gmtime +# define gmtime tz_gmtime +# undef gmtime_r +# define gmtime_r tz_gmtime_r +# undef localtime +# define localtime tz_localtime +# undef localtime_r +# define localtime_r tz_localtime_r +# undef mktime +# define mktime tz_mktime +# undef time +# define time tz_time +# undef time_t +# define time_t tz_time_t + +typedef time_tz time_t; + +char *ctime(time_t const *); +char *ctime_r(time_t const *, char *); +double difftime(time_t, time_t); +struct tm *gmtime(time_t const *); +struct tm *gmtime_r(time_t const *restrict, struct tm *restrict); +struct tm *localtime(time_t const *); +struct tm *localtime_r(time_t const *restrict, struct tm *restrict); +time_t mktime(struct tm *); + +static time_t +time(time_t *p) +{ + time_t r = sys_time(0); + if (p) + *p = r; + return r; +} +#endif + +/* ** Private function declarations. */ diff --git a/zdump.8 b/zdump.8 index 63349e4..85e9641 100644 --- a/zdump.8 +++ b/zdump.8 @@ -37,15 +37,28 @@ Each line ends with if the given time is Daylight Saving Time or .B isdst=0 otherwise. +.B \-V +Like +.BR \-v , +except omit the times relative to the extreme time values. +This generates output that is easier to compare to that of +implementations with different time representations. .TP .BI "\-c " [loyear,]hiyear Cut off verbose output near the start of the given year(s). +.TP +.BI "\-t " [lotime,]hitime +Cut off verbose output at the start of the given time(s), +given in seconds since 1970-01-01 00:00:00 UTC. By default, -the program cuts off verbose output near the starts of the years -500 and 2500. +the program cuts off verbose output near the starts of the years +\-500 and 2500. .SH LIMITATIONS The .B \-v -option may not be used on systems with floating-point time_t values +and +.B \-V +options may not be used on systems with floating-point time_t values that are neither float nor double. .PP Time discontinuities are found by sampling the results returned by localtime diff --git a/zdump.c b/zdump.c index 9255aff..8118091 100644 --- a/zdump.c +++ b/zdump.c @@ -9,8 +9,15 @@ ** This code has been made independent of the rest of the time ** conversion package to increase confidence in the verification it provides. ** You can use this code to help in verifying other implementations. +** +** However, include private.h when debugging, so that it overrides +** time_t consistently with the rest of the package. */ +#ifdef time_tz +# include "private.h" +#endif + #include "stdio.h" /* for stdout, stderr, perror */ #include "string.h" /* for strcpy */ #include "sys/types.h" /* for time_t */ @@ -112,14 +119,6 @@ #endif /* !defined lint */ #endif /* !defined GNUC_or_lint */ -#ifndef INITIALIZE -#ifdef GNUC_or_lint -#define INITIALIZE(x) ((x) = 0) -#else /* !defined GNUC_or_lint */ -#define INITIALIZE(x) -#endif /* !defined GNUC_or_lint */ -#endif /* !defined INITIALIZE */ - #if 2 < __GNUC__ || (__GNUC__ == 2 && 96 <= __GNUC_MINOR__) # define ATTRIBUTE_PURE __attribute__ ((__pure__)) #else @@ -151,20 +150,16 @@ extern char * optarg; extern int optind; extern char * tzname[2]; -/* The minimum and maximum finite time values. Shift 'long long' or - 'long' instead of 'time_t'; this avoids compile-time errors when - time_t is floating-point. In practice, 'long long' is wide enough. */ +/* The minimum and maximum finite time values. */ static time_t const absolute_min_time = ((time_t) 0.5 == 0.5 ? (sizeof (time_t) == sizeof (float) ? (time_t) -FLT_MAX : sizeof (time_t) == sizeof (double) ? (time_t) -DBL_MAX : sizeof (time_t) == sizeof (long double) ? (time_t) -LDBL_MAX : 0) +#ifndef TIME_T_FLOATING : (time_t) -1 < 0 -#ifdef LLONG_MAX - ? (time_t) ((long long) -1 << (CHAR_BIT * sizeof (time_t) - 1)) -#else - ? (time_t) ((long) -1 << (CHAR_BIT * sizeof (time_t) - 1)) + ? (time_t) -1 << (CHAR_BIT * sizeof (time_t) - 1) #endif : 0); static time_t const absolute_max_time = @@ -173,13 +168,11 @@ static time_t const absolute_max_time = : sizeof (time_t) == sizeof (double) ? (time_t) DBL_MAX : sizeof (time_t) == sizeof (long double) ? (time_t) LDBL_MAX : -1) +#ifndef TIME_T_FLOATING : (time_t) -1 < 0 -#ifdef LLONG_MAX - ? (time_t) (- (~ 0 < 0) - ((long long) -1 << (CHAR_BIT * sizeof (time_t) - 1))) -#else - ? (time_t) (- (~ 0 < 0) - ((long) -1 << (CHAR_BIT * sizeof (time_t) - 1))) + ? - (~ 0 < 0) - ((time_t) -1 << (CHAR_BIT * sizeof (time_t) - 1)) #endif - : (time_t) -1); + : -1); static size_t longest; static char * progname; static int warned; @@ -270,9 +263,9 @@ static void usage(FILE * const stream, const int status) { (void) fprintf(stream, -_("%s: usage is %s [ --version ] [ --help ] [ -v ] [ -c [loyear,]hiyear ] zonename ...\n\ -\n\ -Report bugs to %s.\n"), +_("%s: usage: %s [--version] [--help] [-{vV}] [-{ct} [lo,]hi] zonename ...\n" + "\n" + "Report bugs to %s.\n"), progname, progname, REPORT_BUGS_TO); exit(status); } @@ -281,11 +274,10 @@ int main(int argc, char *argv[]) { register int i; - register int c; register int vflag; + register int Vflag; register char * cutarg; - register long cutloyear = ZDUMP_LO_YEAR; - register long cuthiyear = ZDUMP_HI_YEAR; + register char * cuttimes; register time_t cutlotime; register time_t cuthitime; register char ** fakeenv; @@ -297,8 +289,8 @@ main(int argc, char *argv[]) register struct tm * tmp; register struct tm * newtmp; - INITIALIZE(cutlotime); - INITIALIZE(cuthitime); + cutlotime = absolute_min_time; + cuthitime = absolute_max_time; #if HAVE_GETTEXT (void) setlocale(LC_ALL, ""); #ifdef TZ_DOMAINDIR @@ -314,22 +306,30 @@ main(int argc, char *argv[]) } else if (strcmp(argv[i], "--help") == 0) { usage(stdout, EXIT_SUCCESS); } - vflag = 0; - cutarg = NULL; - while ((c = getopt(argc, argv, "c:v")) == 'c' || c == 'v') - if (c == 'v') - vflag = 1; - else cutarg = optarg; - if ((c != EOF && c != -1) || - (optind == argc - 1 && strcmp(argv[optind], "=") == 0)) { - usage(stderr, EXIT_FAILURE); - } - if (vflag) { + vflag = Vflag = 0; + cutarg = cuttimes = NULL; + for (;;) + switch (getopt(argc, argv, "c:t:vV")) { + case 'c': cutarg = optarg; break; + case 't': cuttimes = optarg; break; + case 'v': vflag = 1; break; + case 'V': Vflag = 1; break; + case -1: + if (! (optind == argc - 1 && strcmp(argv[optind], "=") == 0)) + goto arg_processing_done; + /* Fall through. */ + default: + usage(stderr, EXIT_FAILURE); + } + arg_processing_done:; + + if (vflag | Vflag) { + long lo; + long hi; + char dummy; + register long cutloyear = ZDUMP_LO_YEAR; + register long cuthiyear = ZDUMP_HI_YEAR; if (cutarg != NULL) { - long lo; - long hi; - char dummy; - if (sscanf(cutarg, "%ld%c", &hi, &dummy) == 1) { cuthiyear = hi; } else if (sscanf(cutarg, "%ld,%ld%c", @@ -343,8 +343,36 @@ main(int argc, char *argv[]) } } checkabsolutes(); - cutlotime = yeartot(cutloyear); - cuthitime = yeartot(cuthiyear); + if (cutarg != NULL || cuttimes == NULL) { + cutlotime = yeartot(cutloyear); + cuthitime = yeartot(cuthiyear); + } + if (cuttimes != NULL) { + if (sscanf(cuttimes, "%ld%c", &hi, &dummy) == 1) { + if (hi < cuthitime) { + if (hi < absolute_min_time) + hi = absolute_min_time; + cuthitime = hi; + } + } else if (sscanf(cuttimes, "%ld,%ld%c", + &lo, &hi, &dummy) == 2) { + if (cutlotime < lo) { + if (absolute_max_time < lo) + lo = absolute_max_time; + cutlotime = lo; + } + if (hi < cuthitime) { + if (hi < absolute_min_time) + hi = absolute_min_time; + cuthitime = hi; + } + } else { + (void) fprintf(stderr, + _("%s: wild -t argument %s\n"), + progname, cuttimes); + exit(EXIT_FAILURE); + } + } } (void) time(&now); longest = 0; @@ -375,15 +403,17 @@ main(int argc, char *argv[]) static char buf[MAX_STRING_LENGTH]; (void) strcpy(&fakeenv[0][3], argv[i]); - if (!vflag) { + if (! (vflag | Vflag)) { show(argv[i], now, FALSE); continue; } warned = FALSE; t = absolute_min_time; - show(argv[i], t, TRUE); - t += SECSPERHOUR * HOURSPERDAY; - show(argv[i], t, TRUE); + if (!Vflag) { + show(argv[i], t, TRUE); + t += SECSPERHOUR * HOURSPERDAY; + show(argv[i], t, TRUE); + } if (t < cutlotime) t = cutlotime; tmp = my_localtime(&t); @@ -415,11 +445,13 @@ main(int argc, char *argv[]) tm = newtm; tmp = newtmp; } - t = absolute_max_time; - t -= SECSPERHOUR * HOURSPERDAY; - show(argv[i], t, TRUE); - t += SECSPERHOUR * HOURSPERDAY; - show(argv[i], t, TRUE); + if (!Vflag) { + t = absolute_max_time; + t -= SECSPERHOUR * HOURSPERDAY; + show(argv[i], t, TRUE); + t += SECSPERHOUR * HOURSPERDAY; + show(argv[i], t, TRUE); + } } if (fflush(stdout) || ferror(stdout)) { (void) fprintf(stderr, "%s: ", progname); -- 1.8.1.2
* Makefile (TIME_T_ALTERNATIVES): New macro. (check_time_t_alternatives, clean_misc): New rules. (clean): Split out into clean_misc and removing tzpublic. (public): Add check_time_t. --- Makefile | 38 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index cca817c..912bab6 100644 --- a/Makefile +++ b/Makefile @@ -53,6 +53,9 @@ TOPDIR= /usr/local TZDIR= $(TOPDIR)/etc/zoneinfo +# Types to try, as an alternative to time_t. int64_t should be first. +TIME_T_ALTERNATIVES= int64_t int32_t uint32_t uint64_t + # The "tzselect", "zic", and "zdump" commands get installed in. . . ETCDIR= $(TOPDIR)/etc @@ -427,9 +430,10 @@ check_tables: checktab.awk $(PRIMARY_YDATA) check_web: $(WEB_PAGES) $(VALIDATE_ENV) $(VALIDATE) $(VALIDATE_FLAGS) $(WEB_PAGES) -clean: +clean_misc: rm -f core *.o *.out \ date tzselect version.h zdump zic yearistype +clean: clean_misc rm -f -r tzpublic maintainer-clean: clean @@ -440,7 +444,8 @@ maintainer-clean: clean names: @echo $(ENCHILADA) -public: check check_public set-timestamps tarballs signatures +public: check check_public check_time_t_alternatives \ + set-timestamps tarballs signatures # Set the time stamps to those of the git repository, if available, # and if the files have not changed since then. @@ -471,6 +476,35 @@ check_public: $(ENCHILADA) $(zic) -v -d tzpublic $(TDATA) rm -f -r tzpublic +# Check that the code works under various alternative +# implementations of time_t. +check_time_t_alternatives: + mkdir tzpublic + zones=`$(AWK) '/^[^#]/ { print $$3 }' <zone.tab` && \ + for type in $(TIME_T_ALTERNATIVES); do \ + mkdir tzpublic/$$type && \ + make clean_misc && \ + make TOPDIR=`pwd`/tzpublic/$$type \ + CFLAGS='$(CFLAGS) -Dtime_tz='"'$$type'" \ + install && \ + diff -qr tzpublic/int64_t/etc/zoneinfo tzpublic/$$type/etc/zoneinfo && \ + case $$type in \ + int32_t) range=-2147483648,2147483647;; \ + uint32_t) range=0,4294967296;; \ + int64_t) continue;; \ + *u*) range=0,10000000000;; \ + *) range=-10000000000,10000000000;; \ + esac && \ + echo checking $$type zones ... && \ + tzpublic/int64_t/etc/zdump -V -t $$range $$zones \ + >tzpublic/int64_t.out && \ + tzpublic/$$type/etc/zdump -V -t $$range $$zones \ + >tzpublic/$$type.out && \ + diff -u tzpublic/int64_t.out tzpublic/$$type.out \ + || exit; \ + done + rm -f -r tzpublic + tarballs: tzcode$(VERSION).tar.gz tzdata$(VERSION).tar.gz tzcode$(VERSION).tar.gz: $(COMMON) $(DOCS) $(SOURCES) $(MISC) -- 1.8.1.2
The recently-added tests uncovered problems in the reference implementation on platforms where time_t is an unsigned 32-bit quantity. To reproduce the problem, check out the latest version from Github and run the command 'make check_time_t_alternatives'. On my platform (Ubuntu 13.04 x86-64), this generates about 45,000 lines of output, which I won't bore readers with, but here's one example: -America/New_York Sun Mar 14 06:59:59 2038 UTC = Sun Mar 14 01:59:59 2038 EST isdst=0 -America/New_York Sun Mar 14 07:00:00 2038 UTC = Sun Mar 14 03:00:00 2038 EDT isdst=1 -America/New_York Sun Nov 7 05:59:59 2038 UTC = Sun Nov 7 01:59:59 2038 EDT isdst=1 -America/New_York Sun Nov 7 06:00:00 2038 UTC = Sun Nov 7 01:00:00 2038 EST isdst=0 These transitions should exist with unsigned 32-bit time_t, but they're missing.
On May 27, 2:58pm, eggert@cs.ucla.edu (Paul Eggert) wrote: -- Subject: [tz] problems with unsigned 32-bit time_t coming up | I vaguely recalled someone reporting trouble with the tz database | on NetBSD or some other BSD in the past few months. The gist of it | was that this platform has 32-bit 'long' and 64-bit 'time_t', and | the tz code didn't play nicely on such a system. | | To try to catch this sort of thing I'd like to add support for trying | different time_t types, even if they disagree with the system's | time_t. I'll send a couple of proposed patches to the list to do that. | I'll then follow up with a report of a bug in the reference | implementation that is discovered by the patches, when configured | to run with an unsigned 32-bit time_t. This isn't the NetBSD | problem, but hey! at least there's an easy-to-run test | case that illustrates the bug. There have been two major bugs I can remember relating to NetBSD's 64 bit time_t. 1. Sun's openjdk mercurial repository got corrupted when I committed some code from a 32 bit system with 64 bit time_t. Talking about trusting the binary payload!?!? I believe it was fixed and I hope it was done properly. 2. Postgres assumed the sizeof(time_t) == sizeof(long), and when systems upgraded to NetBSD-6 and recompiled postgres (because the compatibility code just works fine), it exposed that postgres bug and none of the databases would load. To my knowledge both bugs have been fixed. I don't know of any others? Does anyone else? Unfortunately I don't know how to catch those application errors easily, specially when they use casts... christos
On Mon, 27 May 2013, Christos Zoulas wrote:
There have been two major bugs I can remember relating to NetBSD's 64 bit time_t. [...] 2. Postgres assumed the sizeof(time_t) == sizeof(long), and when systems upgraded to NetBSD-6 and recompiled postgres (because the compatibility code just works fine), it exposed that postgres bug and none of the databases would load.
Let me expand a little about the compatibility code. Up to and including NetBSD-5.x, NetBSD on 32-bit platforms used 32-bit time_t (the same size as long). NetBSD-6.0 was the first release in which NetBSD on 32-bit platforms used 64-bit time_t (but 32-bit long). The kernel and libc provide compatibility interfaces that allow applications that were compiled against an older version of NetBSD to continue to work (with 32-bit time_t in the application) when run under a newer kernel (with 64-bit time_t in the kernel). Because of the compatibility code, latent bugs in applications that were compiled on an older version of NetBSD (that had 32-bit time_t) may be hidden until the application is recompiled under the newer version of NetBSD (with 64-bit time_t). --apb (Alan Barrett)
On 2013-05-27 22:58, Paul Eggert wrote:
I vaguely recalled someone reporting trouble with the tz database on NetBSD or some other BSD in the past few months. The gist of it was that this platform has 32-bit 'long' and 64-bit 'time_t', and the tz code didn't play nicely on such a system.
Just FYI, the fairly new "X32" ABI for Linux also has 32-bit 'long' (and 32-bit pointers) and 64-bit 'time_t', so that's another system architecture the code could be tested on. -- -=( Ian Abbott @ MEV Ltd. E-mail: <abbotti@mev.co.uk> )=- -=( Tel: +44 (0)161 477 1898 FAX: +44 (0)161 718 3587 )=-
participants (4)
-
Alan Barrett -
christos＠zoulas.com -
Ian Abbott -
Paul Eggert