On Tue, Mar 25, 2014 at 10:09:26AM +0100, Richard Biener wrote:
> On Tue, 25 Mar 2014, Jakub Jelinek wrote:
>
> > Hi!
> >
> > While Marek has been debugging while some perl test fails when perl is built
> > with GCC 4.9, we've discovered that it is because of undefined behavior in
> > it:
> > ...
> > && (((UV)1 << NV_PRESERVES_UV_BITS) >
> > (UV)(SvIVX(sv) > 0 ? SvIVX(sv) : -SvIVX(sv)))
> > where SvIVX(sv) can be LONG_MIN, at which point there is undefined behavior
> > on the negation, but -fsanitize=undefined did detect only other issues in
> > the same source file and not this one, because fold-const.c folded it into
> > ABS_EXPR early.
> >
> > This patch disables such folding, because all the A op 0 ? A : -A
> > operations this if is trying to optimize will need instrumentation with
> > -fsanitize=signed-integer-overflow.
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> Can't one form a valid constant expression using this? Also isn't
> ABS_EXPR undefined for LONG_MIN as well? So why is ubsan not
> instrumenting that instead?
Well, for a constant expression A would need to fold to a constant, but then
A op 0 would fold to a constant too and so would the whole A op 0 ? A : -A.
Instrumenting ABS_EXPR is certainly possible, but definitely more code than
this patch. I could revert this patch and implement it for 5.0 as ABS_EXPR
instrumentation. Of course, if you think we should do the instrumentation
of ABS_EXPR, I can hack it up now.
I was afraid of also the other transformations in:
A == 0? A : -A same as -A
A != 0? A : -A same as A
A >= 0? A : -A same as abs (A)
A > 0? A : -A same as abs (A)
A <= 0? A : -A same as -abs (A)
A < 0? A : -A same as -abs (A)
but only the second one doesn't add ABS_EXPR and removes the negation, but
in that case the negation would happen only for 0, thus it would be fine.
Jakub
