Re: INTERVAL overflow detection is terribly broken

On Jan26, 2014, at 03:50 , Bruce Momjian <bruce@momjian.us> wrote:
> Patch attached.

> +     if ((float)tm->tm_year * MONTHS_PER_YEAR + tm->tm_mon > INT_MAX)
> +         return -1;

Is this bullet-proof? If float and int are both 32-bit, the float's mantissa
will be less than 32-bit (24 or so, I think), and thus won't be able to
represent INT_MAX accurately. This means there's a value of
tm_year * MONTHS_PER_YEAR which is less than INT_MAX, yet for which
tm_year * MONTHS_PER_YEAR + tm_mon will return tm_year * MONTHS_PER_YEAR
unmodified if tm_mon is small enough (e.g, 1). But if tm_year * MONTHS_PER_YEAR
was close enough to INT_MAX, the actual value of
tm_year * MONTHS_PER_YEAR + tm_mon might still be larger than INT_MAX,
the floating-point computation just won't detect it. In that case, the
check would succeed, yet the actual integer computation would still overflow.

It should be safe with "double" instead of "float".

> +     if (SAMESIGN(span1->month, span2->month) &&
> +         !SAMESIGN(result->month, span1->month))

This assumes wrapping semantics for signed integral types, which isn't
guaranteed by the C standard - it says overflows of signed integral types
produce undefined results. We currently depend on wrapping semantics for
these types in more places, and therefore need GCC's "-fwrapv" anway, but
I still wonder if adding more of these kinds of checks is a good idea.

best regards,
Florian Pflug