On Thu, Jan 27, 2011 at 08:14:56AM -0700, Eric Blake wrote: > # define TYPE_MINIMUM(t) \ > ((t) (! TYPE_SIGNED (t) \ > ? (t) 0 \ > : TYPE_SIGNED_MAGNITUDE (t) \ > ? ~ (t) 0 \ > : ~ (t) 0 << (sizeof (t) * CHAR_BIT - 1))) > # define TYPE_MAXIMUM(t) \ > ((t) (! TYPE_SIGNED (t) \ > ? (t) -1 \ > : ~ (~ (t) 0 << (sizeof (t) * CHAR_BIT - 1))))
The last line of this macro has UB due to signed integer overflow in the << operation. Replace it with (( (t)1 << CHAR_BIT*sizeof(time_t)-2 ) - 1) * 2 + 1 Which for a 32-bit type would expand as: (0x40000000 - 1) * 2 + 1 0x3fffffff *2 + 1 0x7ffffffe + 1 0x7fffffff With no overflows. > and no one has complained yet, so we might as well just use this same > logic in m4/mktime.m4. Well apparently no one complained about the overflow in coreutils either. Perhaps later gcc versions are more forgiving; I found it building on a system where I have to use gcc 3.2.3, which, being one of the earlier versions to utilize the UB of signed overflow, might be less diplomatic about it. Anyway to avoid future trouble, I would strive to remove all signed overflow UB from the tests even if it doesn't presently hurt anyone. > 8 is a magic number; it would be better to use CHAR_BIT, as was done in > intprops.h. As you wish. This code (coreutils) is sufficiently POSIX-like-system dependent that I thought using POSIX's requirement CHAR_BIT==8 was reasonable. > > If this test comes from higher-up (gnulib?) please forward my bug > > report to the relevant upstream. > > Forwarded; and the patch should be applied shortly. Thanks! Rich
