This sounds like a problem we encountered too. zdump.c uses behavior of integer overflow on L.358 to detect when newt has reached or exceeded the maximum time_t value. When newt exceeds the maximum time_t value from L.356, it overflows to a negative number, so the L.358 check for newt less than t succeeds. However according to the C Standard, the behavior of integer overflow is undefined (see below). The Sun Studio 11 compiler with -O flag optimizes L.358 out, because it detects the check is not needed, because for the valid range of time_t, newt will never be > t, since from L.355 newt is t plus a positive number. With L.358 optimized out, the for loop at L.352 executes infinitely. zdump.c: ... 249 time_t t; 250 time_t newt; ... 352 for ( ; ; ) { 353 if (t >= cuthitime) 354 break; 355 newt = t + SECSPERHOUR * 12; <-- 356 if (newt >= cuthitime) 357 break; 358 if (newt <= t) <-- 359 break; 360 newtmp = localtime(&newt); ... 376 t = newt; 377 tm = newtm; 378 tmp = newtmp; 379 } A possible fix is to instead check whether t plus delta (delta = SECSPERHOUR * 12) is greater than the maximum time_t: (time + delta) > max_time_t or: time > max_time_t - delta /* avoids time+delta overflow */ ------- zdump.c ------- @@ -314,11 +314,12 @@ for (;;) { if (t >= cuthitime) break; + /* check if newt will overrun maximum time_t value */ + if (t > LONG_MAX - (SECSPERHOUR * 12)) + break; newt = t + SECSPERHOUR * 12; if (newt >= cuthitime) break; - if (newt <= t) - break; newtmp = localtime(&newt); if (newtmp != NULL) newtm = *newtmp; The C Standard says (for signed integer): ISO/IEC 9899:1999: 6.5 p5: If an exceptional condition occurs during the evaluation of an expression (that is, if the result is not mathematically defined or not in the range of representable values for its type), the behavior is undefined. 3.4.3 p3: EXAMPLE: An example of undefined behavior is the behavior on integer overflow.
Date: Mon, 24 Nov 2008 09:09:13 -0500 From: "Olson, Arthur David (NIH/NCI) [E]" <olsona@dc37a.nci.nih.gov> Subject: FW: mail to tz list bounced To: tz@lecserver.nci.nih.gov Cc: Andreas Radke <a.radke@arcor.de>
I'm forwarding this message from Andreas Radke; while Andreas is on the time zone mailing list, the message bounced when he tried to send it directly.
--ado
Datum: Wed, 19 Nov 2008 21:47:05 +0100 Von: Andreas Radke <a.radke@arcor.de> An: tz@lecserver.nci.nih.gov Betreff: optimization -O2 broken?
I'm the ArchLinux package maintainer for tzdata. We are using gcc 4.3.2/glibc2.8- We have run into a small problem with compiler optimizations.
Using our default CFLAGS="-march=i686 -mtune=generic -O2 -pipe" the zdump binary is built in broken way on i686 architecture. I have to apply -O1 to make zdump work. See our bugreport: http://bugs.archlinux.org/task/11947
I could confirm this when running zdump -v EUROPE/BERLIN it stuck after two printing first two lines.
x86_64 architecture doesn't seem to be affected. Any idea why this happens and how to fix it?
-Andy
Robbin Kawabata <Robbin.Kawabata@sun.com> writes:
if (t >= cuthitime) break; + /* check if newt will overrun maximum time_t value */ + if (t > LONG_MAX - (SECSPERHOUR * 12)) + break; newt = t + SECSPERHOUR * 12; if (newt >= cuthitime) break; - if (newt <= t) - break;
Thanks for the diagnosis and patch. I see one problem with that code, though: it assumes that time_t == long, which is not a portable assumption. Also, the code is overly complicated. I suggest using something like this instead: if (t >= cuthitime || t >= cuthitime - SECSPERHOUR * 12) break; newt = t + SECSPERHOUR * 12; The seemingly redundant "t >= cuthitime" test prevents arithmetic overflow in the subtraction (admittedly highly unlikely, but I couldn't prove to my own satisfaction that it could never occur). If we are really worried about cycles, the seemingly-redundant test could be hoisted out of the loop, as it applies only to the first iteration of the loop.
Paul Eggert said:
Also, the code is overly complicated. I suggest using something like this instead:
if (t >= cuthitime || t >= cuthitime - SECSPERHOUR * 12) break; newt = t + SECSPERHOUR * 12;
The seemingly redundant "t >= cuthitime" test prevents arithmetic overflow in the subtraction (admittedly highly unlikely, but I couldn't prove to my own satisfaction that it could never occur).
On a system with 2-byte integers, SECSPERHOUR * 12 will overflow. This can be fixed by changing the 12 to 12L. Apart from that, the overflow can only occur if cuthitime is extremely negative (within 43,200 of the most negative value). I don't have the code to hand and Google only points me at a Hungarian version, so I can't check if that can ever happen. Incidentally, does setabsolutes() have the same problem in it? -- Clive D.W. Feather | Work: <clive@demon.net> | Tel: +44 20 8495 6138 Internet Expert | Home: <clive@davros.org> | Fax: +44 870 051 9937 Demon Internet | WWW: http://www.davros.org | Mobile: +44 7973 377646 THUS - a Cable and Wireless business
"Clive D.W. Feather" <clive@demon.net> writes:
Apart from that, the overflow can only occur if cuthitime is extremely negative (within 43,200 of the most negative value). I don't have the code to hand and Google only points me at a Hungarian version, so I can't check if that can ever happen.
It can also happen if time_t is unsigned and cuthitme is less than 43200, i.e., cuthitime is close to 1970-01-01. This was the scenario I was more worried about. Unsigned time_t is rare but not unheard of. (These days, I expect it's more common than 16-bit int, for zdump ports anyway....)
Incidentally, does setabsolutes() have the same problem in it?
Could be; I haven't looked into it.
Paul Eggert said:
Apart from that, the overflow can only occur if cuthitime is extremely negative (within 43,200 of the most negative value). I don't have the code to hand and Google only points me at a Hungarian version, so I can't check if that can ever happen.
It can also happen if time_t is unsigned and cuthitme is less than 43200, i.e., cuthitime is close to 1970-01-01.
True. -- Clive D.W. Feather | Work: <clive@demon.net> | Tel: +44 20 8495 6138 Internet Expert | Home: <clive@davros.org> | Fax: +44 870 051 9937 Demon Internet | WWW: http://www.davros.org | Mobile: +44 7973 377646 THUS - a Cable and Wireless business
Date: Tue, 25 Nov 2008 07:37:36 -0800 From: Paul Eggert <eggert@cs.ucla.edu> Message-ID: <87hc5vhl4v.fsf@penguin.cs.ucla.edu> | It can also happen if time_t is unsigned I thought that was not permitted these days? I tried that as an experiment in the lead up to 4.2BSD (or some similar version, might have been earlier) and it didn't survive even early beta test versions, way too much assumes that time_t must be signed (including a conversion of (time_t)0 to a struct tm if you're anywhere west of Greenwich - I've always been (almost always been) east, so that problem never revealed itself to me...) All the existing problems could probably be fixed, if they could be found, but new ones would just keep occurring. I haven't seen anyone attempt that again in ages I think, and I really doubt that the issue is likely to arise (with time_t only rationally possible at 32 bits it made sense to attempt to get the extra 70 years by using the extra bit, with 64 bit time_t that's irrelevant, but being able to do ordinary subtraction of time_t values isn't). kre
Robert Elz <kre@munnari.oz.au> writes:
Date: Tue, 25 Nov 2008 07:37:36 -0800 From: Paul Eggert <eggert@cs.ucla.edu> Message-ID: <87hc5vhl4v.fsf@penguin.cs.ucla.edu>
| It can also happen if time_t is unsigned
I thought that was not permitted these days?
If all we go by is the standard, even a floating-point time_t is permitted (though our code won't work with that...).
I tried that as an experiment in the lead up to 4.2BSD
Yes, it breaks a lot of software and is pretty rare. But I think it's used in at least some versions of DJGPP, a modern 32-bit platform; see <http://www.delorie.com/djgpp/doc/incs/sys/djtypes.h>. Maybe DJGPP plans to still be with us after 2038?
participants (4)
-
Clive D.W. Feather -
Paul Eggert -
Robbin Kawabata -
Robert Elz