On 04/26/2017 01:59 AM, Richard Biener wrote:
On Tue, 25 Apr 2017, Martin Sebor wrote:
On 04/24/2017 05:25 AM, Richard Biener wrote:
The following makes signed overflow undefined for all (non-)optimization
levels. The intent is to remove -fno-strict-overflow signed overflow
behavior as that is not a sensible option to the user (it ends up
with the worst of both -fwrapv and -fno-wrapv). The implementation
details need to be preserved for the forseeable future to not wreck
UBSAN with either associating (-fwrapv behavior) or optimizing
(-fno-wrapv behavior).
The other choice would be to make -fwrapv the default for -O[01].
A second patch in this series would unify -f[no-]wrapv, -f[no-]trapv
and -f[no-]strict-overflow with a
-fsigned-integer-overflow={undefined,wrapping,trapping[,sanitized]}
option, making conflicts amongst the options explicit (and reduce
the number of flag_ variables). 'sanitized' would essentially map
to todays flag_strict_overflow = 0. There's another sole user
of flag_strict_overflow, POINTER_TYPE_OVERFLOW_UNDEFINED - not sure
what to do about that, apart from exposing it as different flag
alltogether.
Further patches in the series would remove -Wstrict-overflow (and
cleanup VRP for example).
Minimizing the differences between the guarantees provided at
different optimization levels is a good thing. It will help
uncover bugs that would go undetected during development (with
-O0) and only manifest when building with optimization (which
may be less frequent).
I find the -Wstrict-overflow warning with all its levels over-
engineered but I'm not sure I'm in favor of completely eliminating
it. It has helped illuminate the signed integer overflow problem
for many users who were otherwise completely unaware of it. I'd
be concerned that by getting rid of it users might be lulled back
into assuming that it has the same wrapping semantics as common
hardware (or simply doesn't happen). It sounds like you'd like
to get rid of it to simplify GCC code. Would it make sense to
preserve it for at least the most egregious instances of overflow
(like in 'if (i + 1 < i)' and similar)?
Such cases can (and should!) be certainly warned for but in the
frontend. The current implementation which warns at the point
of simplification isn't too useful given it warns even when the
above doesn't appear literally but through complex optimization.
The most complaint about the warning is that it warns about
perfectly valid code -- there's nothing invalid in a program doing
if (i + 1 < i)
and the compiler optimizing this is good.
The warning was supposed to be a hint that maybe the programmer
wasn't aware of undefined signed integer overflow and thus the
code didn't do what he expected.
Exactly. The example above clearly indicates an error on the part
of its author: an assumption that signed overflow has wrapping
semantics. It's a common mistake that people make based on their
experience with popular hardware.
I agree with optimizing such code, but it shouldn't be done
silently, especially not on targets where it does have the
expected semantics. Warning only in the front end will miss
the more subtle instances of the problem.
That said, I certainly share your concern about false positives
for code that results from prior transformations. But I'm hopeful
there is a solution to the problem other than limiting warnings
to little more than pattern matchers in the front end.
We do have -fsanitize=undefined now to better catch all these
cases.
The sanitizers are very helpful as a complementary tool to
warnings, but not as a substitute for them. Unlike warnings,
they're not available to all projects, depend on every code
path being exercised, have a relatively high runtime overhead,
and tend to catch bugs late in the development cycle (after
they have been injected into the code base).
Btw, the above should warn under one of the various -W...
that warn about expressions always evaluating to true/false
(didn't spot one that's right on, so maybe we need to add a new one -
or re-use -Wstrict-overflow).
That would make sense. -Wtautological-compare seems close.
But to catch more than just the trivial cases it would need
to be extended to the middle-end. For instance:
void bar (int i, int j)
{
if (j < 1 || 2 < j)
j = 1;
if (i + j < i) // could be optimized (with a warning)
foo ();
}