This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Don't perform A - (-B) -> A + B when sanitizing
- From: Richard Biener <rguenther at suse dot de>
- To: Marek Polacek <polacek at redhat dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Jakub Jelinek <jakub at redhat dot com>
- Date: Wed, 12 Nov 2014 10:49:38 +0100 (CET)
- Subject: Re: [PATCH] Don't perform A - (-B) -> A + B when sanitizing
- Authentication-results: sourceware.org; auth=none
- References: <20141112084819 dot GG29791 at redhat dot com>
On Wed, 12 Nov 2014, Marek Polacek wrote:
> In the following testcase generic_simplify folded A - (-B) -> A + B,
> which means we didn't detect an overflow. So I've tweaked match.pd.
>
> But fold-const.c still can do such a transformation as well, so I had
> to tweak it there as well in the same way.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2014-11-12 Marek Polacek <polacek@redhat.com>
>
> * match.pd (A - (-B) -> A + B): Check for TYPE_OVERFLOW_WRAPS
> and SANITIZE_SI_OVERFLOW.
> * fold-const.c (fold_binary_loc): Likewise.
>
> * c-c++-common/ubsan/overflow-sub-4.c: New test.
> * c-c++-common/ubsan/overflow-sub-2.c: Adjust dg-output.
> * c-c++-common/ubsan/overflow-int128.c: Likewise.
>
> diff --git gcc/fold-const.c gcc/fold-const.c
> index 756f469..504e1b6 100644
> --- gcc/fold-const.c
> +++ gcc/fold-const.c
> @@ -10544,6 +10544,8 @@ fold_binary_loc (location_t loc,
>
> /* A - B -> A + (-B) if B is easily negatable. */
> if (negate_expr_p (arg1)
> + && (TYPE_OVERFLOW_WRAPS (type)
> + || (flag_sanitize & SANITIZE_SI_OVERFLOW) == 0)
TYPE_OVERFLOW_WRAPS returns nonsense for !integral types and thus
this check likely restricts the transform to non-floats for example.
The macro documentation states "given an integral type ..." thus
it should be sth like
&& (!INTEGRAL_TYPE_P (type)
|| TYPE_OVERFLOW_WRAPS (type)
|| (flag_sanitize ...
and the TYPE_OVERFLOW_* macros should probably be guarded with
tree checking against use on non-integral types (you might want
to do that as a followup).
> && ((FLOAT_TYPE_P (type)
> /* Avoid this transformation if B is a positive REAL_CST. */
> && (TREE_CODE (arg1) != REAL_CST
> diff --git gcc/match.pd gcc/match.pd
> index 29b5ab2..5a485f3 100644
> --- gcc/match.pd
> +++ gcc/match.pd
> @@ -291,7 +291,9 @@ along with GCC; see the file COPYING3. If not see
> (simplify
> (minus (convert1? @0) (convert2? (negate @1)))
> (if (tree_nop_conversion_p (type, TREE_TYPE (@0))
> - && tree_nop_conversion_p (type, TREE_TYPE (@1)))
> + && tree_nop_conversion_p (type, TREE_TYPE (@1))
> + && (TYPE_OVERFLOW_WRAPS (type)
> + || (flag_sanitize & SANITIZE_SI_OVERFLOW) == 0))
This has the same issue with unnecessarily restricting FLOAT types, so
can you fix that here and in the two other patterns already guarded
with flag_sanitize?
Ok with that changes.
Thanks,
RIchard.
> (plus (convert @0) (convert @1))))
> /* -(-A) -> A */
> (simplify
> diff --git gcc/testsuite/c-c++-common/ubsan/overflow-sub-2.c gcc/testsuite/c-c++-common/ubsan/overflow-sub-2.c
> index daf6a54..88c4762 100644
> --- gcc/testsuite/c-c++-common/ubsan/overflow-sub-2.c
> +++ gcc/testsuite/c-c++-common/ubsan/overflow-sub-2.c
> @@ -43,12 +43,12 @@ main (void)
> }
>
> /* { dg-output "signed integer overflow: -2147483648 - 1 cannot be represented in type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
> -/* { dg-output "\[^\n\r]*signed integer overflow: -2147483648 \\+ -1 cannot be represented in type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
> -/* { dg-output "\[^\n\r]*signed integer overflow: -2147483548 \\+ -1024 cannot be represented in type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
> -/* { dg-output "\[^\n\r]*signed integer overflow: -2147483648 \\+ -1 cannot be represented in type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
> -/* { dg-output "\[^\n\r]*signed integer overflow: -2147482648 \\+ -1048576 cannot be represented in type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
> +/* { dg-output "\[^\n\r]*signed integer overflow: -2147483648 - 1 cannot be represented in type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
> +/* { dg-output "\[^\n\r]*signed integer overflow: -2147483548 - 1024 cannot be represented in type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
> +/* { dg-output "\[^\n\r]*signed integer overflow: -2147483648 - 1 cannot be represented in type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
> +/* { dg-output "\[^\n\r]*signed integer overflow: -2147482648 - 1048576 cannot be represented in type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
> /* { dg-output "\[^\n\r]*signed integer overflow: -\[^\n\r]* - 1 cannot be represented in type 'long int'\[^\n\r]*(\n|\r\n|\r)" } */
> -/* { dg-output "\[^\n\r]*signed integer overflow: -\[^\n\r]* \\+ -1 cannot be represented in type 'long int'\[^\n\r]*(\n|\r\n|\r)" } */
> -/* { dg-output "\[^\n\r]*signed integer overflow: -\[^\n\r]* \\+ -1024 cannot be represented in type 'long int'\[^\n\r]*(\n|\r\n|\r)" } */
> -/* { dg-output "\[^\n\r]*signed integer overflow: -\[^\n\r]* \\+ -1 cannot be represented in type 'long int'\[^\n\r]*(\n|\r\n|\r)" } */
> -/* { dg-output "\[^\n\r]*signed integer overflow: -\[^\n\r]* \\+ -1048576 cannot be represented in type 'long int'\[^\n\r]*" } */
> +/* { dg-output "\[^\n\r]*signed integer overflow: -\[^\n\r]* - 1 cannot be represented in type 'long int'\[^\n\r]*(\n|\r\n|\r)" } */
> +/* { dg-output "\[^\n\r]*signed integer overflow: -\[^\n\r]* - 1024 cannot be represented in type 'long int'\[^\n\r]*(\n|\r\n|\r)" } */
> +/* { dg-output "\[^\n\r]*signed integer overflow: -\[^\n\r]* - 1 cannot be represented in type 'long int'\[^\n\r]*(\n|\r\n|\r)" } */
> +/* { dg-output "\[^\n\r]*signed integer overflow: -\[^\n\r]* - 1048576 cannot be represented in type 'long int'\[^\n\r]*" } */
> diff --git gcc/testsuite/c-c++-common/ubsan/overflow-int128.c gcc/testsuite/c-c++-common/ubsan/overflow-int128.c
> index 125d6bf..4384d7c 100644
> --- gcc/testsuite/c-c++-common/ubsan/overflow-int128.c
> +++ gcc/testsuite/c-c++-common/ubsan/overflow-int128.c
> @@ -41,7 +41,7 @@ main (void)
> /* { dg-output "\[^\n\r]*signed integer overflow: 0x7fffffffffffffffffffffffffffff9b \\+ 1024 cannot be represented in type '__int128'(\n|\r\n|\r)" } */
> /* { dg-output "\[^\n\r]*signed integer overflow: -1 \\+ 0x80000000000000000000000000000000 cannot be represented in type '__int128'(\n|\r\n|\r)" } */
> /* { dg-output "\[^\n\r]*signed integer overflow: 0x80000000000000000000000000000000 \\+ -1 cannot be represented in type '__int128'(\n|\r\n|\r)" } */
> -/* { dg-output "\[^\n\r]*signed integer overflow: 0x80000000000000000000000000000000 \\+ -1 cannot be represented in type '__int128'(\n|\r\n|\r)" } */
> +/* { dg-output "\[^\n\r]*signed integer overflow: 0x80000000000000000000000000000000 - 1 cannot be represented in type '__int128'(\n|\r\n|\r)" } */
> /* { dg-output "\[^\n\r]*signed integer overflow: 0x80000000000000000000000000000064 \\+ -1024 cannot be represented in type '__int128'(\n|\r\n|\r)" } */
> /* { dg-output "\[^\n\r]*signed integer overflow: 0x7fffffffffffffffffffffffffffffff \\* 2 cannot be represented in type '__int128'(\n|\r\n|\r)" } */
> /* { dg-output "\[^\n\r]*negation of 0x80000000000000000000000000000000 cannot be represented in type '__int128'; cast to an unsigned type to negate this value to itself\[^\n\r]*(\n|\r\n|\r)" } */
> diff --git gcc/testsuite/c-c++-common/ubsan/overflow-sub-4.c gcc/testsuite/c-c++-common/ubsan/overflow-sub-4.c
> index e69de29..519b7ba 100644
> --- gcc/testsuite/c-c++-common/ubsan/overflow-sub-4.c
> +++ gcc/testsuite/c-c++-common/ubsan/overflow-sub-4.c
> @@ -0,0 +1,19 @@
> +/* { dg-do run } */
> +/* { dg-options "-fsanitize=signed-integer-overflow" } */
> +
> +#define INT_MIN (-__INT_MAX__ - 1)
> +
> +int
> +main ()
> +{
> + int x = INT_MIN;
> + int y = 0;
> + int z;
> + asm ("" : "+g" (y));
> + asm ("" : "+g" (x));
> + z = y - (-x);
> + asm ("" : "+g" (z));
> +}
> +
> +/* { 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]*signed integer overflow: 0 - -2147483648 cannot be represented in type 'int'\[^\n\r]*(\n|\r\n|\r)" } */
>
> Marek
>
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Jeff Hawn, Jennifer Guild, Felix Imendoerffer, HRB 21284
(AG Nuernberg)
Maxfeldstrasse 5, 90409 Nuernberg, Germany