[PROPOSED] Fix integer overflow when adjusting posixrules
Commit Message
Florian Weimer wrote:
> On 04/10/2016 09:42 PM, Paul Eggert wrote:
>> +/* intprops.h -- properties of integer types
>
> I'm afraid I have serious doubts about the maintainability of this file.
The file has been used in core GNU utilities for a decade or so, so there is
some evidence for maintainability. It can always be improved of course.
> In this case, the argument for the outer cast is not within the range of
> type t, so the result of the cast is implementation-defined. For ones'
> complement, ~0 can be undefined, it does not have to be 0.
Good catch. That code dates back to 2005, when ones' complement platforms were
still arguably plausible porting targets for GNU code. Unisys stopped selling
ones' complement mainframes late last year, though. ~0 should work on Unisys
mainframes; but really, it's time to remove that gingerbread, if only to make
the code easier to review.
There's a similar issue in time/mktime.c in glibc (also shared with gnulib).
Revised patches attached. The first is an updated version of the tzfile fix, and
the second fixes mktime. The proposed intprops.h and mktime.c files match what's
now in gnulib (which I've already fixed).
From 83be322adc0d4ac94aaa46b157aff21447f0755d Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Wed, 13 Apr 2016 00:26:06 -0700
Subject: [PATCH 2/2] mktime assumes two's complement now
The code attempted to verify that the current platform is two's
complement, but in theory the verification would not have
correctly rejected some non-two's-complement hosts, and anyway the
verification effort was not worth the hassle.
Reported by Florian Weimer in:
https://sourceware.org/ml/libc-alpha/2016-04/msg00295.html
* time/mktime.c (TYPE_TWOS_COMPLEMENT):
Remove. All uses rewritten to assume two's complement, which is
all we can reasonably test nowadays anyway.
---
ChangeLog | 11 +++++++++++
time/mktime.c | 31 +++++++++++--------------------
2 files changed, 22 insertions(+), 20 deletions(-)
@@ -1,5 +1,16 @@
2016-04-13 Paul Eggert <eggert@cs.ucla.edu>
+ mktime assumes two's complement now
+ The code attempted to verify that the current platform is two's
+ complement, but in theory the verification would not have
+ correctly rejected some non-two's-complement hosts, and anyway the
+ verification effort was not worth the hassle.
+ Reported by Florian Weimer in:
+ https://sourceware.org/ml/libc-alpha/2016-04/msg00295.html
+ * time/mktime.c (TYPE_TWOS_COMPLEMENT):
+ Remove. All uses rewritten to assume two's complement, which is
+ all we can reasonably test nowadays anyway.
+
Fix integer overflow when adjusting posixrules
[BZ #19738]
This bug can occur on hosts with 32-bit time_t when reading tz files
@@ -103,25 +103,20 @@ verify (long_int_is_wide_enough, INT_MAX == INT_MAX * (long_int) 2 / 2);
an integer. */
#define TYPE_IS_INTEGER(t) ((t) 1.5 == 1)
-/* True if negative values of the signed integer type T use two's
- complement, or if T is an unsigned integer type. */
-#define TYPE_TWOS_COMPLEMENT(t) ((t) ~ (t) 0 == (t) -1)
-
/* True if the arithmetic type T is signed. */
#define TYPE_SIGNED(t) (! ((t) 0 < (t) -1))
-/* The maximum and minimum values for the integer type T. These
- macros have undefined behavior if T is signed and has padding bits.
- If this is a problem for you, please let us know how to fix it for
- your host. */
-#define TYPE_MINIMUM(t) \
- ((t) (! TYPE_SIGNED (t) \
- ? (t) 0 \
- : ~ TYPE_MAXIMUM (t)))
-#define TYPE_MAXIMUM(t) \
- ((t) (! TYPE_SIGNED (t) \
- ? (t) -1 \
- : ((((t) 1 << (sizeof (t) * CHAR_BIT - 2)) - 1) * 2 + 1)))
+/* Minimum and maximum values for integer types.
+ These macros have undefined behavior for signed types that either
+ have padding bits or do not use two's complement. If this is a
+ problem for you, please let us know how to fix it for your host. */
+
+/* The maximum and minimum values for the integer type T. */
+#define TYPE_MINIMUM(t) ((t) ~ TYPE_MAXIMUM (t))
+#define TYPE_MAXIMUM(t) \
+ ((t) (! TYPE_SIGNED (t) \
+ ? (t) -1 \
+ : ((((t) 1 << (sizeof (t) * CHAR_BIT - 2)) - 1) * 2 + 1)))
#ifndef TIME_T_MIN
# define TIME_T_MIN TYPE_MINIMUM (time_t)
@@ -132,10 +127,6 @@ verify (long_int_is_wide_enough, INT_MAX == INT_MAX * (long_int) 2 / 2);
#define TIME_T_MIDPOINT (SHR (TIME_T_MIN + TIME_T_MAX, 1) + 1)
verify (time_t_is_integer, TYPE_IS_INTEGER (time_t));
-verify (twos_complement_arithmetic,
- (TYPE_TWOS_COMPLEMENT (int)
- && TYPE_TWOS_COMPLEMENT (long_int)
- && TYPE_TWOS_COMPLEMENT (time_t)));
#define EPOCH_YEAR 1970
#define TM_YEAR_BASE 1900
--
2.5.5