[tz] source code question regarding localtime.c PS

[Paul Eggert]

Thanks for mentioning the problem.  The code relies on undefined
behavior when a time_t value is outside int_fast64_t range.
It's better to avoid undefined behavior, and the code would be
clearer if we put in some comments about this issue.  Here's
a proposed patch for this, which I've pushed to the experimental
github repository.

>From b0bf6d8fee915393274deb7df0ba2aacc8756d99 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert at cs.ucla.edu>
Date: Wed, 7 Aug 2013 09:03:09 -0700
Subject: [PATCH] Avoid undefined behavior on integer overflow.

Problem reported by Alois Treindl in
<http://mm.icann.org/pipermail/tz/2013-August/019493.html>.
* localtime.c (truncate_time): New function.
(localsub): Use it to avoid undefined behavior on integer overflow.
* private.h (INTMAX_MAX, INTMAX_MIN, UINTMAX_MAX):
New macros, for older platforms that lack them.
---
 localtime.c | 50 ++++++++++++++++++++++++++++++++++++++------------
 private.h   | 15 +++++++++++++++
 2 files changed, 53 insertions(+), 12 deletions(-)

diff --git a/localtime.c b/localtime.c
index 23bc635..353c73b 100644
--- a/localtime.c
+++ b/localtime.c
@@ -1254,6 +1254,38 @@ tzset(void)
 	settzname();
 }
 
+/* Return T with any fractional part discarded.  */
+static time_t
+truncate_time(time_t t)
+{
+	/*
+	** If time_t is floating-point, convert it to integer and
+	** back; this discards the fraction.  Avoid using <math.h>
+	** functions, as we don't want to depend on <math.h>.  Use <,
+	** not <=, when comparing against *_MIN and *_MAX values, as
+	** this avoids undefined behavior when, for example,
+	** INTMAX_MAX is converted to a larger time_t value before it
+	** is compared.
+	**
+	** On all platforms that we know of, if a time value is
+	** outside intmax_t/uintmax_t range, then it is an integer so we can
+	** simply return it.  There's no simple, portable way to check this.
+	** If you know of a counterexample platform, please report a bug.
+	*/
+	if (!TYPE_INTEGRAL(time_t)) {
+		if (INTMAX_MIN < t && t < INTMAX_MAX) {
+			intmax_t i = t;
+			return i;
+		}
+		if (0 <= t && t < UINTMAX_MAX) {
+			uintmax_t i = t;
+			return i;
+		}
+	}
+
+	return t;
+}
+
 /*
 ** The easy way to behave "as if no library function calls" localtime
 ** is to not call it--so we drop its guts into "localsub", which can be
@@ -1283,21 +1315,15 @@ localsub(const time_t *const timep, const int_fast32_t offset,
 		(sp->goahead && t > sp->ats[sp->timecnt - 1])) {
 			time_t			newt = t;
 			register time_t		seconds;
-			register time_t		tcycles;
-			register int_fast64_t	icycles;
+			register time_t		years;
 
 			if (t < sp->ats[0])
 				seconds = sp->ats[0] - t;
 			else	seconds = t - sp->ats[sp->timecnt - 1];
 			--seconds;
-			tcycles = seconds / YEARSPERREPEAT / AVGSECSPERYEAR;
-			++tcycles;
-			icycles = tcycles;
-			if (tcycles - icycles >= 1 || icycles - tcycles >= 1)
-				return NULL;
-			seconds = icycles;
-			seconds *= YEARSPERREPEAT;
-			seconds *= AVGSECSPERYEAR;
+			years = (truncate_time (seconds / SECSPERREPEAT + 1)
+				 * YEARSPERREPEAT);
+			seconds = years * AVGSECSPERYEAR;
 			if (t < sp->ats[0])
 				newt += seconds;
 			else	newt -= seconds;
@@ -1310,8 +1336,8 @@ localsub(const time_t *const timep, const int_fast32_t offset,
 
 				newy = tmp->tm_year;
 				if (t < sp->ats[0])
-					newy -= icycles * YEARSPERREPEAT;
-				else	newy += icycles * YEARSPERREPEAT;
+					newy -= years;
+				else	newy += years;
 				tmp->tm_year = newy;
 				if (tmp->tm_year != newy)
 					return NULL;
diff --git a/private.h b/private.h
index a31a26e..f00a926 100644
--- a/private.h
+++ b/private.h
@@ -167,9 +167,18 @@ typedef int int_fast32_t;
 # if defined LLONG_MAX || defined __LONG_LONG_MAX__
 typedef long long intmax_t;
 #  define PRIdMAX "lld"
+#  ifdef LLONG_MAX
+#   define INTMAX_MAX LLONG_MAX
+#   define INTMAX_MIN LLONG_MIN
+#  else
+#   define INTMAX_MAX __LONG_LONG_MAX__
+#   define INTMAX_MIN __LONG_LONG_MIN__
+#  endif
 # else
 typedef long intmax_t;
 #  define PRIdMAX "ld"
+#  define INTMAX_MAX LONG_MAX
+#  define INTMAX_MIN LONG_MIN
 # endif
 #endif
 
@@ -177,9 +186,15 @@ typedef long intmax_t;
 # if defined ULLONG_MAX || defined __LONG_LONG_MAX__
 typedef unsigned long long uintmax_t;
 #  define PRIuMAX "llu"
+#  ifdef ULLONG_MAX
+#   define UINTMAX_MAX ULLONG_MAX
+#  else
+#   define UINTMAX_MAX (__LONG_LONG_MAX__ * 2ULL + 1)
+#  endif
 # else
 typedef unsigned long uintmax_t;
 #  define PRIuMAX "lu"
+#  define UINTMAX_MAX ULONG_MAX
 # endif
 #endif
 
-- 
1.8.1.2