Anyone for adding -fwrapv to our standard CFLAGS?

Started by Tom Laneabout 20 years ago5 messages
#1Tom Lane
tgl@sss.pgh.pa.us

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

#2Neil Conway
neilc@samurai.com
In reply to: Tom Lane (#1)
Re: Anyone for adding -fwrapv to our standard CFLAGS?

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

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Neil Conway (#2)
Re: Anyone for adding -fwrapv to our standard CFLAGS?

Neil Conway <neilc@samurai.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

#4Michael Paesold
mpaesold@gmx.at
In reply to: Tom Lane (#1)
Re: Anyone for adding -fwrapv to our standard CFLAGS?

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.]

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Michael Paesold (#4)
Re: Anyone for adding -fwrapv to our standard CFLAGS?

"Michael Paesold" <mpaesold@gmx.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