From: | Florian Pflug <fgp(at)phlo(dot)org> |
---|---|
To: | Bruce Momjian <bruce(at)momjian(dot)us> |
Cc: | Rok Kralj <rok(dot)kralj(at)gmail(dot)com>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: INTERVAL overflow detection is terribly broken |
Date: | 2014-01-26 13:13:03 |
Message-ID: | 357E3D42-E37D-4365-9359-12DEC1496F08@phlo.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Jan26, 2014, at 03:50 , Bruce Momjian <bruce(at)momjian(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | Magnus Hagander | 2014-01-26 14:53:25 | effective_cache_size calculation overflow |
Previous Message | Andres Freund | 2014-01-26 12:58:00 | Re: Freezing without write I/O |