Subject: bug#7928: mktime test in configure: UB resulting in infinite loop

From: Rich Felker <[email-masked]>
Date: Thu, 27 Jan 2011 12:28:50 -0500

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