From: Neil Conway on
On Sun, Aug 1, 2010 at 9:40 PM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote:
> I'm still wondering about the bleats I saw for -fwrapv though.
> configure already is set up to install that switch only conditionally:
>
> �# Disable optimizations that assume no overflow; needed for gcc 4.3+
> �PGAC_PROG_CC_CFLAGS_OPT([-fwrapv])
>
> but apparently the test used for this does not notice warning messages.
> Can we improve that?

I think this is a non-issue at least with respect to clang, since they
added support for -fwrapv recently. However, I wonder if the logic
should be the reverse: unless we have evidence to suggest that the
compiler provides the integer overflow behavior we require (e.g., it
supports -fwrapv, sufficiently old GCC, etc.), then we should emit a
warning to suggest that the resulting binary might be buggy.

Neil

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Greg Stark on
On Mon, Aug 2, 2010 at 4:27 PM, Tom Lane <tgl(a)sss.pgh.pa.us> wrote:
> Here's the problem: if the compiler is allowed to assume that overflow
> cannot happen, it is always going to be able to "prove" that the
> if-test is constant false. �This is inherent. �Anybody claiming to
> exhibit a safe way to code the overflow test is really only telling
> you that today's version of their compiler isn't smart enough to make
> the proof.

So I'll do the next two parts of the dialogue myself:

Questioner: So why not write that as:

if ((arg1 > 0 && arg2 > 0 && arg1 > MAXINT - arg2) ||
(arg1 < 0 && arg2 < 0 && arg1 < MININT + arg2))
elog("overflow")
else
return arg1+arg2;

Tom: Because that code is much more complex and prone to errors
especially when you start getting into multiplication and other
operations and it's also much slower than the code which allows
overflow to happen and then checks that the result makes sense.

I'm not entirely sure I agree. At least I haven't actually gone
through all the arithmetic operations and I'm not sure how much more
complex they get. If they were all at that level of complexity I think
I would say we should go ahead and bite the performance bullet and do
it the ultra-safe way.

--
greg

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Tom Lane on
Neil Conway <neil.conway(a)gmail.com> writes:
> FWIW, I think we should aim to eventually remove the dependency on
> -fwrapv, and instead make the code correct under the semantics
> guaranteed by the C spec.

[ shrug... ] We've gone around on that before. I can't get excited
about it, and the reason is that 99% of the places where we actually
need -fwrapv behavior is in places where we are trying to test for
overflow. The need for that can't be legislated away. As an example,
in int4pl we have code like

result = arg1 + arg2;
if (some-test-on-result-and-arg1-and-arg2)
elog(ERROR, "overflow");

Here's the problem: if the compiler is allowed to assume that overflow
cannot happen, it is always going to be able to "prove" that the
if-test is constant false. This is inherent. Anybody claiming to
exhibit a safe way to code the overflow test is really only telling
you that today's version of their compiler isn't smart enough to make
the proof. Next year, who knows? I'm uninterested in getting into
that kind of arms race with the compiler authors. I prefer to keep
the goalposts exactly where they've been for the past forty years,
namely -fwrapv semantics.

If the compiler guys actually were serious about helping people write
code that didn't need -fwrapv, they would provide primitives for
testing for arithmetic overflow. I would be ecstatic if we could write
int4pl like this:

if (sum_overflows(arg1, arg2))
elog(ERROR, "overflow");
else
return arg1 + arg2;

especially since with a halfway decent compiler this sort of
construction wouldn't even require doing the addition twice ---
presumably the compiler could see that it had a common subexpression
there, and generate code that consists of one addition plus a
branch-on-overflow test. This'd be clean, readable, and far more
efficient than the hacks we have to resort to now.

Short of that, though, -fwrapv is what it's gonna be.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Martijn van Oosterhout on
On Mon, Aug 02, 2010 at 05:34:57PM +0100, Greg Stark wrote:
> Tom: Because that code is much more complex and prone to errors
> especially when you start getting into multiplication and other
> operations and it's also much slower than the code which allows
> overflow to happen and then checks that the result makes sense.

> I'm not entirely sure I agree. At least I haven't actually gone
> through all the arithmetic operations and I'm not sure how much more
> complex they get. If they were all at that level of complexity I think
> I would say we should go ahead and bite the performance bullet and do
> it the ultra-safe way.

FWIW, here's a site with some gcc magic which will allow you to detect
overflows during addition. Ofcourse, the fact that it's gcc specific
makes it a lot less useful.

http://www.fefe.de/intof.html

Have a nice day,
--
Martijn van Oosterhout <kleptog(a)svana.org> http://svana.org/kleptog/
> Patriotism is when love of your own people comes first; nationalism,
> when hate for people other than your own comes first.
> - Charles de Gaulle