This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Implement -fsanitize=float-cast-overflow
- From: Jakub Jelinek <jakub at redhat dot com>
- To: "Joseph S. Myers" <joseph at codesourcery dot com>
- Cc: Marek Polacek <polacek at redhat dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 14 May 2014 13:38:39 +0200
- Subject: Re: [PATCH] Implement -fsanitize=float-cast-overflow
- Authentication-results: sourceware.org; auth=none
- References: <20140513170801 dot GG2663 at redhat dot com> <Pine dot LNX dot 4 dot 64 dot 1405131745570 dot 23277 at digraph dot polyomino dot org dot uk>
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
On Tue, May 13, 2014 at 06:11:15PM +0000, Joseph S. Myers wrote:
> > + tree min = TYPE_MIN_VALUE (type);
> > + tree max = TYPE_MAX_VALUE (type);
> > + /* Add/subtract 1.0 so we can avoid truncating the value of EXPR. */
> > + min = fold_build2 (MINUS_EXPR, expr_type,
> > + build_real_from_int_cst (expr_type, min),
> > + build_one_cst (expr_type));
> > + max = fold_build2 (PLUS_EXPR, expr_type,
> > + build_real_from_int_cst (expr_type, max),
> > + build_one_cst (expr_type));
>
> It looks to me like this will first round the max value to the
> floating-point type, then add 1 to the rounded value and round again.
> Which I think is in fact safe at least for IEEE binary floating-point
> types, but that isn't immediately obvious.
>
> Possible issues:
>
> * Does the folding of the addition occur in all cases for IBM long double?
>
> * Is this correct for decimal floating point? There, the overflow
> condition (value >= max+1) should be using a value of (max+1) rounded
> upward rather than to-nearest, if max+1 isn't exactly representable (and
> in general it isn't - powers of two 0x1p24 and above aren't representable
> in decimal32, 0x1p54 and above in decimal64, 0x1p113 and above in
> decimal128, so you just need to find a case where the double-rounding
> computation you have produces the wrong value).
>
> * Likewise, (value <= min-1) for both binary and decimal floating point -
> you need to round once, away from 0. For float converted to signed int,
> the relevant condition is values < -0x1p31 - 1, i.e. <= 0x1.000002p31f
> once you allow for which values are representable as float, which is not
> min-1 (min-1 rounds to -0x1p31, but a conversion of that to signed int is
> fully defined with no exceptions).
So what do you see as the way to handle this properly?
I mean, for REAL_MODE_FORMAT (TYPE_MODE (expr_type))->b == 2 supposedly to avoid issues with
rounding of the max we could just
REAL_VALUE_TYPE maxval = dconst1;
SET_REAL_EXP (&maxval, REAL_EXP (&maxval) + TYPE_PRECISION (type) - !TYPE_UNSIGNED (type));
real_convert (&maxval, TYPE_MODE (expr_type), &maxval);
max = build_real (expr_type, maxval);
or so, then supposedly max is always the smallest representable binary
floating point value above or equal to TYPE_MAX_VALUE + 1.0.
For the min value, if it is unsigned, then -1.0 is ok for all binary or
decimal floats, if it is signed, then supposedly we could do the above
with s/max/min/;s/dconst1/dconstm1/; and, after the real_convert
do inexact = real_arithmetic (&newminval, MINUS_EXPR, &minval, &dconst1);
if !inexact just min = build_real (expr_type, newminval); and be done with
it (the question is if for IBM double double this will DTRT for
LONG_LONG_MIN, which I think should be that the high double will contain
(double) LONG_LONG_MIN and the low double -1.0). For inexact
(which should be the same thing as if result of real_arithmetic + real_convert
is the same as original minval) we need to subtract more than one, dunno if
we should just compute it from the REAL_EXP and precision, or just keep
subtracing powers of two until after real_convert it is no longer bitwise
identical to original minval. We don't have anything close to
real_nextafter nor real_convert variant that can round for arbitrary
rounding modes.
Any preferences how to implement this?
For _Decimal*, no idea unfortunately, perhaps for the first iteration
ubsan should ignore decimal to int conversions.
Jakub