FW: [casting; overflow detection]
Paul Eggert
eggert at cs.ucla.edu
Thu Jan 13 18:52:44 UTC 2011
The business about casting calloc is more a matter of style than
anything else (do we want to program as if it were the 1980s? then
use a cast. otherwise don't) but the overflow issue is more important.
On 01/13/11 06:30, christos at rebar.astron.com (Christos Zoulas) wrote:
> + return number0 < 0 && (-*number + INT_MIN) > delta;
That won't work either, since the '-' could overflow.
The tz code is littered with assumptions that integer overflow wraps
around. You've just found the tip of the iceberg here. It would take
a great deal of effort to remove all those assumptions. I suggest
that, if you compile with GCC, that you compile with the -fwrapv
option, to avoid problems in this area.
That being said, in areas where we know about overflow problems we
might as well be more portable. Here's a patch to catch the instances
of overflow issues that I found. I gave up after a while, because
there are lots more where these came from, but these are pretty
easy fixes so we might as well put them in.
This patch also alters Makefile to warn about the problem and suggest
-fwrapv to GCC users.
===================================================================
RCS file: RCS/Makefile,v
retrieving revision 2010.14
diff -pu -r2010.14 Makefile
--- Makefile 2010/10/12 16:36:50 2010.14
+++ Makefile 2011/01/13 18:25:22
@@ -110,6 +110,10 @@ LDLIBS=
# -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
+# $(GCC_OVERFLOW_FLAGS) if you are using GCC 3.4 or later.
+# If you are using a compiler other than GCC, and if it defaults to
+# undefined behavior on integer overflow, then you need to specify a flag
+# saying that integer arithmetic is supposed to wrap around on overflow.
# -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
@@ -120,6 +124,7 @@ GCC_DEBUG_FLAGS = -Dlint -g -O -fno-comm
-Wall -Wcast-qual -Wconversion -Wmissing-prototypes \
-Wnested-externs -Wpointer-arith -Wshadow \
-Wtraditional # -Wstrict-prototypes -Wwrite-strings
+GCC_OVERFLOW_FLAGS = -fwrapv
#
# If you want to use System V compatibility code, add
# -DUSG_COMPAT
@@ -386,7 +391,7 @@ names:
public: $(ENCHILADA)
make maintainer-clean
- make "CFLAGS=$(GCC_DEBUG_FLAGS)"
+ make "CFLAGS=$(GCC_DEBUG_FLAGS) $(GCC_OVERFLOW_FLAGS)"
-mkdir /tmp/,tzpublic
-for i in $(TDATA) ; do zic -v -d /tmp/,tzpublic $$i 2>&1 | grep -v "starting year" ; done
for i in $(TDATA) ; do zic -d /tmp/,tzpublic $$i || exit; done
===================================================================
RCS file: RCS/date.c,v
retrieving revision 2007.10
diff -pu -r2007.10 date.c
--- date.c 2007/12/03 14:42:13 2007.10
+++ date.c 2011/01/13 18:36:59
@@ -598,15 +598,12 @@ comptm(atmp, btmp)
register const struct tm * const atmp;
register const struct tm * const btmp;
{
- register int result;
-
- if ((result = (atmp->tm_year - btmp->tm_year)) == 0 &&
- (result = (atmp->tm_mon - btmp->tm_mon)) == 0 &&
- (result = (atmp->tm_mday - btmp->tm_mday)) == 0 &&
- (result = (atmp->tm_hour - btmp->tm_hour)) == 0 &&
- (result = (atmp->tm_min - btmp->tm_min)) == 0)
- result = atmp->tm_sec - btmp->tm_sec;
- return result;
+ return atmp->tm_year != btmp->tm_year
+ || atmp->tm_mon != btmp->tm_mon
+ || atmp->tm_mday != btmp->tm_mday
+ || atmp->tm_hour != btmp->tm_hour
+ || atmp->tm_min != btmp->tm_min
+ || atmp->tm_sec != btmp->tm_sec;
}
/*
===================================================================
RCS file: RCS/localtime.c,v
retrieving revision 2010.14
diff -pu -r2010.14 localtime.c
--- localtime.c 2010/10/12 16:36:51 2010.14
+++ localtime.c 2011/01/13 17:58:57
@@ -1606,8 +1606,10 @@ int delta;
int number0;
number0 = *number;
+ if (delta < 0 ? number0 < delta - INT_MIN : INT_MAX - delta < number0)
+ return 1;
*number += delta;
- return (*number < number0) != (delta < 0);
+ return 0;
}
static int
@@ -1618,8 +1620,10 @@ int delta;
long number0;
number0 = *number;
+ if (delta < 0 ? number0 < delta - LONG_MIN : LONG_MAX - delta < number0)
+ return 1;
*number += delta;
- return (*number < number0) != (delta < 0);
+ return 0;
}
static int
===================================================================
RCS file: RCS/zdump.c,v
retrieving revision 2010.14
diff -pu -r2010.14 zdump.c
--- zdump.c 2010/10/12 16:36:51 2010.14
+++ zdump.c 2011/01/13 18:32:27
@@ -17,6 +17,7 @@ static char elsieid[] = "@(#)zdump.c 8.1
#include "time.h" /* for struct tm */
#include "stdlib.h" /* for exit, malloc, atoi */
#include "float.h" /* for FLT_MAX and DBL_MAX */
+#include "limits.h" /* for INT_MAX, LONG_MAX, etc. */
#include "ctype.h" /* for isalpha et al. */
#ifndef isascii
#define isascii(x) 1
@@ -426,21 +427,25 @@ _("%s: use of -v on system with floating
}
} else if (0 > (time_t) -1) {
/*
- ** time_t is signed. Assume overflow wraps around.
+ ** time_t is signed.
*/
- time_t t = 0;
- time_t t1 = 1;
-
- while (t < t1) {
- t = t1;
- t1 = 2 * t1 + 1;
+ if (sizeof (time_t) == sizeof (int)) {
+ absolute_min_time = INT_MIN;
+ absolute_max_time = INT_MAX;
+ } else if (sizeof (time_t) == sizeof (long)) {
+ absolute_min_time = LONG_MIN;
+ absolute_max_time = LONG_MAX;
+#if defined LLONG_MIN && defined LLONG_MAX
+ } else if (sizeof (time_t) == sizeof (long long)) {
+ absolute_min_time = LLONG_MIN;
+ absolute_max_time = LLONG_MAX;
+#endif
+ } else {
+ (void) fprintf(stderr,
+_("%s: use of -v on system with signed time_t whose extrema are unknown\n"),
+ progname);
+ exit(EXIT_FAILURE);
}
-
- absolute_max_time = t;
- t = -t;
- absolute_min_time = t - 1;
- if (t < absolute_min_time)
- absolute_min_time = t;
} else {
/*
** time_t is unsigned.
===================================================================
RCS file: RCS/zic.c,v
retrieving revision 2010.14
diff -pu -r2010.14 zic.c
--- zic.c 2010/10/12 17:37:29 2010.14
+++ zic.c 2011/01/13 18:18:35
@@ -2533,14 +2533,11 @@ oadd(t1, t2)
const long t1;
const long t2;
{
- register long t;
-
- t = t1 + t2;
- if ((t2 > 0 && t <= t1) || (t2 < 0 && t >= t1)) {
+ if (t2 < 0 ? t1 < t2 - LONG_MIN : LONG_MAX - t2 < t1) {
error(_("time overflow"));
exit(EXIT_FAILURE);
}
- return t;
+ return t1 + t2;
}
static zic_t
@@ -2548,18 +2545,15 @@ tadd(t1, t2)
const zic_t t1;
const long t2;
{
- register zic_t t;
-
if (t1 == max_time && t2 > 0)
return max_time;
if (t1 == min_time && t2 < 0)
return min_time;
- t = t1 + t2;
- if ((t2 > 0 && t <= t1) || (t2 < 0 && t >= t1)) {
+ if (t2 < 0 ? t1 < t2 - LONG_MIN : LONG_MAX - t2 < t1) {
error(_("time overflow"));
exit(EXIT_FAILURE);
}
- return t;
+ return t1 + t2;
}
/*
More information about the tz
mailing list