Anyone for adding -fwrapv to our standard CFLAGS?

It seems that gcc is up to some creative reinterpretation of basic C
semantics again; specifically, you can no longer trust that traditional
C semantics of integer overflow hold:
https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=175462

While I don't think we are anywhere using exactly the same trick that
the referenced mysql code is using, it certainly seems likely to me that
a compiler that is willing to replace "x < 0 && -x < 0" with "false"
might be able to break some of the integer overflow checks we do use.

I think we need to add -fwrapv to CFLAGS anytime the compiler will take
it, same as we recently started doing with -fno-strict-aliasing.

Comments?

regards, tom lane

On Mon, 2005-12-12 at 16:19 -0500, Tom Lane wrote:
> It seems that gcc is up to some creative reinterpretation of basic C
> semantics again; specifically, you can no longer trust that traditional
> C semantics of integer overflow hold:
> https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=175462
>
> While I don't think we are anywhere using exactly the same trick that
> the referenced mysql code is using, it certainly seems likely to me that
> a compiler that is willing to replace "x < 0 && -x < 0" with "false"
> might be able to break some of the integer overflow checks we do use.

IMHO code that makes assumptions about overflow behavior beyond what is
defined by the standard is asking for trouble, whether those assumptions
are "traditional C semantics" or not. Given that -fwrapv apparently
hurts performance *and* you've presented no evidence that we actually
need the flag in the first place, I'm not sold on this idea...

-Neil

Neil Conway <neilc(at)samurai(dot)com> writes:
> IMHO code that makes assumptions about overflow behavior beyond what is
> defined by the standard is asking for trouble, whether those assumptions
> are "traditional C semantics" or not. Given that -fwrapv apparently
> hurts performance *and* you've presented no evidence that we actually
> need the flag in the first place, I'm not sold on this idea...

Can you present a reasonably efficient way to test for integer overflow,
as is needed in int4pl and friends (not to mention a lot of security
checks that you yourself had a hand in adding), without making any
assumptions that the new gcc may think are illegal? ISTM that what the
spec is doing (according to Jakub's interpretation of it anyway) is
denying the existence of overflow for signed values, which is (a) silly
and (b) unhelpful.

The larger problem here, however, is exactly the same as it was with
-fno-strict-aliasing: the compiler provides no useful tools for finding
the places where your code may be broken by the new assumptions.
I'd be willing to have a go at fixing the problems if I could be certain
that we'd found them all, but what gcc is actually offering us is a game
of Russian roulette. I have better things to do with my time than to
try to find all the places we'd need to fix to have any confidence that
our code is safe without -fwrapv.

regards, tom lane

Tom Lane wrote:

> It seems that gcc is up to some creative reinterpretation of basic C
> semantics again; specifically, you can no longer trust that traditional
> C semantics of integer overflow hold:
> https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=175462
>
> While I don't think we are anywhere using exactly the same trick that
> the referenced mysql code is using, it certainly seems likely to me that
> a compiler that is willing to replace "x < 0 && -x < 0" with "false"
> might be able to break some of the integer overflow checks we do use.
>
> I think we need to add -fwrapv to CFLAGS anytime the compiler will take
> it, same as we recently started doing with -fno-strict-aliasing.

What about this one from the bug (by Jakub Jelinek):

> Now, -fwrapv can be an answer if you are unwilling to fix the broken code,
> but be prepared that the performance will be terrible, as GCC will not be
> able to optimize many loops in a way that it is allowed by the standard.

"Performance will be terrible" does not sound that good.

Is there any other GCC guy you could talk about this? I don't think
GCC==Jakub Jelinek? What do others suggest? There should be a portable way
to detect overflow, no?

Best Regards,
Michael Paesold

[Tom, I removed you from CC: because your spam filter tends to eat my mail;
you should get it through the lists, though.]

"Michael Paesold" <mpaesold(at)gmx(dot)at> writes:
> What about this one from the bug (by Jakub Jelinek):
> ...
> "Performance will be terrible" does not sound that good.

That's the overstatement of the week, though. Jakub is merely unhappy
because some new optimizations won't get applied. At worst -fwrapv will
leave us with the same performance we had before gcc 4.1.

As I said to Jakub, in the end correctness trumps performance every
time. The risk that gcc 4.1 without -fwrapv will silently break corner
cases in our code is simply not acceptable, and the cost of making sure
that it doesn't is IMHO not worth the (undefined, undocumented)
performance gains that might or might not ensue.

If anyone really wants to argue the point, a useful first step would
be to grab a copy of gcc 4.1 and see if you can detect any overall
performance difference in Postgres compiled with and without -fwrapv.

regards, tom lane