On Wed, 12 Nov 2014, Marek Polacek wrote:
> On Tue, Nov 11, 2014 at 07:45:40PM +0100, Richard Biener wrote:
> > On November 11, 2014 6:49:34 PM CET, Jakub Jelinek <ja...@redhat.com> wrote:
> > >On Tue, Nov 11, 2014 at 06:40:25PM +0100, Marek Polacek wrote:
> > >> --- gcc/fold-const.c
> > >> +++ gcc/fold-const.c
> > >> @@ -7862,9 +7862,15 @@ fold_unary_loc (location_t loc, enum tree_code
> > >code, tree type, tree op0)
> > >> return fold_view_convert_expr (type, op0);
> > >>
> > >> case NEGATE_EXPR:
> > >> - tem = fold_negate_expr (loc, arg0);
> > >> - if (tem)
> > >> - return fold_convert_loc (loc, type, tem);
> > >> + if (TREE_CODE (arg0) == INTEGER_CST
> > >> + || TREE_CODE (arg0) == REAL_CST
> > >> + || TYPE_OVERFLOW_WRAPS (type)
> > >> + || (flag_sanitize & SANITIZE_SI_OVERFLOW) == 0)
> > >> + {
> > >> + tem = fold_negate_expr (loc, arg0);
> > >> + if (tem)
> > >> + return fold_convert_loc (loc, type, tem);
> > >> + }
> > >
> > >a) if arg0 is INTEGER_CST, but e.g. INT_MIN, should we
> > > really fold it for -fsanitize=signed-integer-overflow
> > > (I'd say in that case we should fold only if it is not
> > > the minimum value)
>
> Ok. We don't sanitize constant folded expressions (e.g. INT_MAX +
> 1), but this can be easily done.
>
> > Right. I think you should fix VRP instead.
>
> With the following no VRP tweaks are needed (I hope).
>
> > >b) if the argument is not INTEGRAL_TYPE_P (type), shouldn't
> > > we fold no matter what flag_sanitize says? Is the REAL_CST
> > > case needed in that case?
> >
> > Yes and no I guess.
>
> Done.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2014-11-12 Marek Polacek <pola...@redhat.com>
>
> * fold-const.c (fold_unary_loc): Don't call fold_negate_expr
> when the result is undefined.
>
> * c-c++-common/ubsan/overflow-negate-3.c: New test.
>
> diff --git gcc/fold-const.c gcc/fold-const.c
> index 756f469..3688f5a 100644
> --- gcc/fold-const.c
> +++ gcc/fold-const.c
> @@ -7854,9 +7854,16 @@ fold_unary_loc (location_t loc, enum tree_code code,
> tree type, tree op0)
> return fold_view_convert_expr (type, op0);
>
> case NEGATE_EXPR:
> - tem = fold_negate_expr (loc, arg0);
> - if (tem)
> - return fold_convert_loc (loc, type, tem);
> + if (!INTEGRAL_TYPE_P (type)
> + || (TREE_CODE (arg0) == INTEGER_CST
> + && !tree_int_cst_equal (arg0, TYPE_MIN_VALUE (type)))
> + || TYPE_OVERFLOW_WRAPS (type)
> + || (flag_sanitize & SANITIZE_SI_OVERFLOW) == 0)
> + {
> + tem = fold_negate_expr (loc, arg0);
> + if (tem)
> + return fold_convert_loc (loc, type, tem);
> + }
> return NULL_TREE;
Err - please adjust fold_negate_expr instead.
Richard.
> case ABS_EXPR:
> diff --git gcc/testsuite/c-c++-common/ubsan/overflow-negate-3.c
> gcc/testsuite/c-c++-common/ubsan/overflow-negate-3.c
> index e69de29..e6db394 100644
> --- gcc/testsuite/c-c++-common/ubsan/overflow-negate-3.c
> +++ gcc/testsuite/c-c++-common/ubsan/overflow-negate-3.c
> @@ -0,0 +1,21 @@
> +/* { dg-do run } */
> +/* { dg-options "-fsanitize=signed-integer-overflow" } */
> +
> +#define INT_MIN (-__INT_MAX__ - 1)
> +
> +int
> +main ()
> +{
> + int x = INT_MIN;
> + int y;
> + asm ("" : "+g" (x));
> + y = -(-x);
> + asm ("" : "+g" (y));
> + y = -(-INT_MIN);
> + asm ("" : "+g" (y));
> +}
> +
> +/* { dg-output "negation of -2147483648 cannot be represented in type
> 'int'\[^\n\r]*; cast to an unsigned type to negate this value to
> itself\[^\n\r]*(\n|\r\n|\r)" } */
> +/* { dg-output "\[^\n\r]*negation of -2147483648 cannot be represented in
> type 'int'\[^\n\r]*; cast to an unsigned type to negate this value to
> itself\[^\n\r]*(\n|\r\n|\r)" } */
> +/* { dg-output "\[^\n\r]*negation of -2147483648 cannot be represented in
> type 'int'\[^\n\r]*; cast to an unsigned type to negate this value to
> itself\[^\n\r]*(\n|\r\n|\r)" } */
> +/* { dg-output "\[^\n\r]*negation of -2147483648 cannot be represented in
> type 'int'\[^\n\r]*; cast to an unsigned type to negate this value to
> itself\[^\n\r]*(\n|\r\n|\r)" } */
>
> Marek
>
>
--
Richard Biener <rguent...@suse.de>
SUSE LINUX GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendoerffer, HRB 21284
(AG Nuernberg)
Maxfeldstrasse 5, 90409 Nuernberg, Germany
