On 05/08/2015 09:59 AM, Joseph Myers wrote:
Paul, although glibc's copy of parts of tzcode is a bit out of date, it
looks like the currenthttps://github.com/eggert/tz.git still has the
problematic code in private.h, relying on left-shifting -1 which has
undefined behavior in C99/C11 (implementation-defined in C90, as per
DR#081).
Thanks for reporting that. I installed the attached patch into the
experimental tz version on github <https://github.com/eggert/tz>, with
the intent that this fix propagate into the next tz release and thus
into glibc etc.
>From b8f4f998104e74fc2c4a3759317b5153e95db16e Mon Sep 17 00:00:00 2001
From: Paul Eggert <egg...@cs.ucla.edu>
Date: Fri, 8 May 2015 14:47:40 -0700
Subject: [PATCH] Avoid left-shift-into-sign-bit undefined behavior
Problem reported by Joseph Myers in:
https://gcc.gnu.org/ml/gcc-patches/2015-05/msg00704.html
* localtime.c (detzcode, detzcode64): Don't rely on
undefined behavior with left shift into sign bit.
Port better to non-2's-complement machines.
* private.h (TWOS_COMPLEMENT, MAXVAL, MINVAL): New macros.
* private.h (time_t_min, time_t_max):
* zic.c (min_time, max_time): Use them to avoid undefined behavior.
* zdump.c (atime_shift): New constant.
(absolute_min_time, absolute_max_time):
Use it to avoid undefined behavior.
---
localtime.c | 32 +++++++++++++++++++++++++++-----
private.h | 23 ++++++++++++++---------
zdump.c | 6 ++++--
zic.c | 4 ++--
4 files changed, 47 insertions(+), 18 deletions(-)
diff --git a/localtime.c b/localtime.c
index a2d9aa8..423e13e 100644
--- a/localtime.c
+++ b/localtime.c
@@ -216,22 +216,44 @@ detzcode(const char *const codep)
{
register int_fast32_t result;
register int i;
+ int_fast32_t one = 1;
+ int_fast32_t halfmaxval = one << (32 - 2);
+ int_fast32_t maxval = halfmaxval - 1 + halfmaxval;
+ int_fast32_t minval = -1 - maxval;
- result = (codep[0] & 0x80) ? -1 : 0;
- for (i = 0; i < 4; ++i)
+ result = codep[0] & 0x7f;
+ for (i = 1; i < 4; ++i)
result = (result << 8) | (codep[i] & 0xff);
+
+ if (codep[0] & 0x80) {
+ /* Do two's-complement negation even on non-two's-complement machines.
+ If the result would be minval - 1, return minval. */
+ result -= !TWOS_COMPLEMENT(int_fast32_t) && result != 0;
+ result += minval;
+ }
return result;
}
static int_fast64_t
detzcode64(const char *const codep)
{
- register int_fast64_t result;
+ register uint_fast64_t result;
register int i;
+ int_fast64_t one = 1;
+ int_fast64_t halfmaxval = one << (64 - 2);
+ int_fast64_t maxval = halfmaxval - 1 + halfmaxval;
+ int_fast64_t minval = -TWOS_COMPLEMENT(int_fast64_t) - maxval;
- result = (codep[0] & 0x80) ? -1 : 0;
- for (i = 0; i < 8; ++i)
+ result = codep[0] & 0x7f;
+ for (i = 1; i < 8; ++i)
result = (result << 8) | (codep[i] & 0xff);
+
+ if (codep[0] & 0x80) {
+ /* Do two's-complement negation even on non-two's-complement machines.
+ If the result would be minval - 1, return minval. */
+ result -= !TWOS_COMPLEMENT(int_fast64_t) && result != 0;
+ result += minval;
+ }
return result;
}
diff --git a/private.h b/private.h
index 61cf922..f277e7a 100644
--- a/private.h
+++ b/private.h
@@ -472,15 +472,20 @@ time_t time2posix_z(timezone_t, time_t) ATTRIBUTE_PURE;
#define TYPE_SIGNED(type) (((type) -1) < 0)
#endif /* !defined TYPE_SIGNED */
-/* The minimum and maximum finite time values. */
-static time_t const time_t_min =
- (TYPE_SIGNED(time_t)
- ? (time_t) -1 << (CHAR_BIT * sizeof (time_t) - 1)
- : 0);
-static time_t const time_t_max =
- (TYPE_SIGNED(time_t)
- ? - (~ 0 < 0) - ((time_t) -1 << (CHAR_BIT * sizeof (time_t) - 1))
- : -1);
+#define TWOS_COMPLEMENT(t) ((t) ~ (t) 0 < 0)
+
+/* Max and min values of the integer type T, of which only the bottom
+ B bits are used, and where the highest-order used bit is considered
+ to be a sign bit if T is signed. */
+#define MAXVAL(t, b) \
+ ((t) (((t) 1 << ((b) - 1 - TYPE_SIGNED(t))) \
+ - 1 + ((t) 1 << ((b) - 1 - TYPE_SIGNED(t)))))
+#define MINVAL(t, b) \
+ ((t) (TYPE_SIGNED(t) ? - TWOS_COMPLEMENT(t) - MAXVAL(t, b) : 0))
+
+/* The minimum and maximum finite time values. This assumes no padding. */
+static time_t const time_t_min = MINVAL(time_t, TYPE_BIT(time_t));
+static time_t const time_t_max = MAXVAL(time_t, TYPE_BIT(time_t));
#ifndef INT_STRLEN_MAXIMUM
/*
diff --git a/zdump.c b/zdump.c
index 13bbb0e..adb806c 100644
--- a/zdump.c
+++ b/zdump.c
@@ -246,13 +246,15 @@ extern int optind;
extern char * tzname[2];
/* The minimum and maximum finite time values. */
+enum { atime_shift = CHAR_BIT * sizeof (time_t) - 2 };
static time_t const absolute_min_time =
((time_t) -1 < 0
- ? (time_t) -1 << (CHAR_BIT * sizeof (time_t) - 1)
+ ? (- ((time_t) ~ (time_t) 0 < 0)
+ - (((time_t) 1 << atime_shift) - 1 + ((time_t) 1 << atime_shift)))
: 0);
static time_t const absolute_max_time =
((time_t) -1 < 0
- ? - (~ 0 < 0) - ((time_t) -1 << (CHAR_BIT * sizeof (time_t) - 1))
+ ? (((time_t) 1 << atime_shift) - 1 + ((time_t) 1 << atime_shift))
: -1);
static int longest;
static char * progname;
diff --git a/zic.c b/zic.c
index 636649b..52b45ad 100644
--- a/zic.c
+++ b/zic.c
@@ -813,8 +813,8 @@ warning(_("hard link failed, symbolic link used"));
#define TIME_T_BITS_IN_FILE 64
-static const zic_t min_time = (zic_t) -1 << (TIME_T_BITS_IN_FILE - 1);
-static const zic_t max_time = -1 - ((zic_t) -1 << (TIME_T_BITS_IN_FILE - 1));
+static zic_t const min_time = MINVAL (zic_t, TIME_T_BITS_IN_FILE);
+static zic_t const max_time = MAXVAL (zic_t, TIME_T_BITS_IN_FILE);
/* Estimated time of the Big Bang, in seconds since the POSIX epoch.
rounded downward to the negation of a power of two that is
--
2.1.0
