This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix infinite recursion with div-by-zero (PR middle-end/70992)
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Marek Polacek <polacek at redhat dot com>
- Cc: Eric Botcazou <ebotcazou at adacore dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 26 Jul 2017 13:36:24 +0200
- Subject: Re: [PATCH] Fix infinite recursion with div-by-zero (PR middle-end/70992)
- Authentication-results: sourceware.org; auth=none
- References: <20170718160511.GP2890@redhat.com> <20170725121937.GF3397@redhat.com> <CAFiYyc2Nm8j914KkpGGS0mnqMU=MxT5YagOz0pjq8GC1=t1yfg@mail.gmail.com> <1771717.3h9EB9PvgT@arcturus.home> <CAFiYyc2a8DDA4Gkd=KgTWKdJWLs3dyO02sg253d5t8OBHdW5iQ@mail.gmail.com> <20170726113503.GI3397@redhat.com>
On Wed, Jul 26, 2017 at 1:35 PM, Marek Polacek <polacek@redhat.com> wrote:
> On Tue, Jul 25, 2017 at 03:47:31PM +0200, Richard Biener wrote:
>> On Tue, Jul 25, 2017 at 3:30 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
>> >> Eric, any comments?
>> >
>> > No objection for the build2_stat hunk, I think it's in keeping with the Ada
>> > semantics. But the tree_could_trap_p hunk is certainly an abomination...
>> >
>> >> We could also avoid tree_could_trap_p by special-casing only
>> >> *_DIV_EXPR and *_MOD_EXPR
>> >> with integer zero 2nd operand explicitely in build2 given there's no
>> >> "constant" value for this. That is,
>> >> for FP 1./0. is NaN (a "constant" value) even if the operation might trap.
>> >
>> > Yes, that would be faster & simpler and avoids the abomination.
>>
>> Ok, then let's go with that slightly uglier but less abominal variant ;)
>
> Like this then?
>
> Bootstrapped/regtested on x86_64-linux (including Ada) and powerpc64le-unknown-linux-gnu,
> ok for trunk?
Ok.
Thanks,
Richard.
> 2017-07-26 Marek Polacek <polacek@redhat.com>
>
> PR middle-end/70992
> * tree.c (build2_stat): Don't set TREE_CONSTANT on divisions by zero.
>
> * gcc.dg/overflow-warn-1.c: Adjust dg-error.
> * gcc.dg/overflow-warn-2.c: Likewise.
> * gcc.dg/overflow-warn-3.c: Likewise.
> * gcc.dg/overflow-warn-4.c: Likewise.
> * gcc.dg/torture/pr70992-2.c: New test.
> * gcc.dg/torture/pr70992.c: New test.
>
> diff --git gcc/testsuite/gcc.dg/overflow-warn-1.c gcc/testsuite/gcc.dg/overflow-warn-1.c
> index 8eb322579cf..a5cd5738636 100644
> --- gcc/testsuite/gcc.dg/overflow-warn-1.c
> +++ gcc/testsuite/gcc.dg/overflow-warn-1.c
> @@ -49,7 +49,7 @@ static int sc = INT_MAX + 1; /* { dg-warning "25:integer overflow in expression"
> void *p = 0 * (INT_MAX + 1); /* { dg-warning "integer overflow in expression" } */
> /* { dg-warning "initialization makes pointer from integer without a cast" "null" { target *-*-* } .-1 } */
> void *q = 0 * (1 / 0); /* { dg-warning "division by zero" } */
> -/* { dg-error "initializer element is not computable at load time" "constant" { target *-*-* } .-1 } */
> +/* { dg-error "initializer element is not constant" "constant" { target *-*-* } .-1 } */
> /* { dg-warning "initialization makes pointer from integer without a cast" "null" { target *-*-* } .-2 } */
> void *r = (1 ? 0 : INT_MAX+1);
>
> diff --git gcc/testsuite/gcc.dg/overflow-warn-2.c gcc/testsuite/gcc.dg/overflow-warn-2.c
> index f048d6dae2a..05ab104fa4a 100644
> --- gcc/testsuite/gcc.dg/overflow-warn-2.c
> +++ gcc/testsuite/gcc.dg/overflow-warn-2.c
> @@ -49,7 +49,7 @@ static int sc = INT_MAX + 1; /* { dg-warning "integer overflow in expression" }
> void *p = 0 * (INT_MAX + 1); /* { dg-warning "integer overflow in expression" } */
> /* { dg-warning "initialization makes pointer from integer without a cast" "null" { target *-*-* } .-1 } */
> void *q = 0 * (1 / 0); /* { dg-warning "division by zero" } */
> -/* { dg-error "initializer element is not computable at load time" "constant" { target *-*-* } .-1 } */
> +/* { dg-error "initializer element is not constant" "constant" { target *-*-* } .-1 } */
> /* { dg-warning "initialization makes pointer from integer without a cast" "null" { target *-*-* } .-2 } */
> void *r = (1 ? 0 : INT_MAX+1);
>
> diff --git gcc/testsuite/gcc.dg/overflow-warn-3.c gcc/testsuite/gcc.dg/overflow-warn-3.c
> index 664011e401d..fd4a34f67e2 100644
> --- gcc/testsuite/gcc.dg/overflow-warn-3.c
> +++ gcc/testsuite/gcc.dg/overflow-warn-3.c
> @@ -55,7 +55,7 @@ void *p = 0 * (INT_MAX + 1); /* { dg-warning "integer overflow in expression" }
> /* { dg-warning "overflow in constant expression" "constant" { target *-*-* } .-1 } */
> /* { dg-warning "initialization makes pointer from integer without a cast" "null" { target *-*-* } .-2 } */
> void *q = 0 * (1 / 0); /* { dg-warning "division by zero" } */
> -/* { dg-error "initializer element is not computable at load time" "constant" { target *-*-* } .-1 } */
> +/* { dg-error "initializer element is not constant" "constant" { target *-*-* } .-1 } */
> /* { dg-warning "initialization makes pointer from integer without a cast" "null" { target *-*-* } .-2 } */
> void *r = (1 ? 0 : INT_MAX+1);
>
> diff --git gcc/testsuite/gcc.dg/overflow-warn-4.c gcc/testsuite/gcc.dg/overflow-warn-4.c
> index 52677ce897a..018e3e1e4cd 100644
> --- gcc/testsuite/gcc.dg/overflow-warn-4.c
> +++ gcc/testsuite/gcc.dg/overflow-warn-4.c
> @@ -55,7 +55,7 @@ void *p = 0 * (INT_MAX + 1); /* { dg-warning "integer overflow in expression" }
> /* { dg-error "overflow in constant expression" "constant" { target *-*-* } .-1 } */
> /* { dg-error "initialization makes pointer from integer without a cast" "null" { target *-*-* } .-2 } */
> void *q = 0 * (1 / 0); /* { dg-warning "division by zero" } */
> -/* { dg-error "initializer element is not computable at load time" "constant" { target *-*-* } .-1 } */
> +/* { dg-error "initializer element is not constant" "constant" { target *-*-* } .-1 } */
> /* { dg-error "initialization makes pointer from integer without a cast" "null" { target *-*-* } .-2 } */
> void *r = (1 ? 0 : INT_MAX+1);
>
> diff --git gcc/testsuite/gcc.dg/torture/pr70992-2.c gcc/testsuite/gcc.dg/torture/pr70992-2.c
> index e69de29bb2d..c5d2c5f2683 100644
> --- gcc/testsuite/gcc.dg/torture/pr70992-2.c
> +++ gcc/testsuite/gcc.dg/torture/pr70992-2.c
> @@ -0,0 +1,9 @@
> +/* PR middle-end/70992 */
> +/* { dg-do compile } */
> +
> +unsigned int *od;
> +int
> +fn (void)
> +{
> + return (0 % 0 + 1) * *od * 2; /* { dg-warning "division by zero" } */
> +}
> diff --git gcc/testsuite/gcc.dg/torture/pr70992.c gcc/testsuite/gcc.dg/torture/pr70992.c
> index e69de29bb2d..56728e09d1b 100644
> --- gcc/testsuite/gcc.dg/torture/pr70992.c
> +++ gcc/testsuite/gcc.dg/torture/pr70992.c
> @@ -0,0 +1,41 @@
> +/* PR middle-end/70992 */
> +/* { dg-do compile } */
> +
> +typedef unsigned int uint32_t;
> +typedef int int32_t;
> +
> +uint32_t
> +fn (uint32_t so)
> +{
> + return (so + so) * (0x80000000 / 0 + 1); /* { dg-warning "division by zero" } */
> +}
> +
> +uint32_t
> +fn5 (uint32_t so)
> +{
> + return (0x80000000 / 0 + 1) * (so + so); /* { dg-warning "division by zero" } */
> +}
> +
> +uint32_t
> +fn6 (uint32_t so)
> +{
> + return (0x80000000 / 0 - 1) * (so + so); /* { dg-warning "division by zero" } */
> +}
> +
> +uint32_t
> +fn2 (uint32_t so)
> +{
> + return (so + so) * (0x80000000 / 0 - 1); /* { dg-warning "division by zero" } */
> +}
> +
> +int32_t
> +fn3 (int32_t so)
> +{
> + return (so + so) * (0x80000000 / 0 + 1); /* { dg-warning "division by zero" } */
> +}
> +
> +int32_t
> +fn4 (int32_t so)
> +{
> + return (so + so) * (0x80000000 / 0 - 1); /* { dg-warning "division by zero" } */
> +}
> diff --git gcc/tree.c gcc/tree.c
> index b7de2840ac6..48fb2ce0651 100644
> --- gcc/tree.c
> +++ gcc/tree.c
> @@ -4456,7 +4456,7 @@ build1_stat (enum tree_code code, tree type, tree node MEM_STAT_DECL)
> tree
> build2_stat (enum tree_code code, tree tt, tree arg0, tree arg1 MEM_STAT_DECL)
> {
> - bool constant, read_only, side_effects;
> + bool constant, read_only, side_effects, div_by_zero;
> tree t;
>
> gcc_assert (TREE_CODE_LENGTH (code) == 2);
> @@ -4489,6 +4489,23 @@ build2_stat (enum tree_code code, tree tt, tree arg0, tree arg1 MEM_STAT_DECL)
> read_only = 1;
> side_effects = TREE_SIDE_EFFECTS (t);
>
> + switch (code)
> + {
> + case TRUNC_DIV_EXPR:
> + case CEIL_DIV_EXPR:
> + case FLOOR_DIV_EXPR:
> + case ROUND_DIV_EXPR:
> + case EXACT_DIV_EXPR:
> + case CEIL_MOD_EXPR:
> + case FLOOR_MOD_EXPR:
> + case ROUND_MOD_EXPR:
> + case TRUNC_MOD_EXPR:
> + div_by_zero = integer_zerop (arg1);
> + break;
> + default:
> + div_by_zero = false;
> + }
> +
> PROCESS_ARG (0);
> PROCESS_ARG (1);
>
> @@ -4505,7 +4522,8 @@ build2_stat (enum tree_code code, tree tt, tree arg0, tree arg1 MEM_STAT_DECL)
> else
> {
> TREE_READONLY (t) = read_only;
> - TREE_CONSTANT (t) = constant;
> + /* Don't mark X / 0 as constant. */
> + TREE_CONSTANT (t) = constant && !div_by_zero;
> TREE_THIS_VOLATILE (t)
> = (TREE_CODE_CLASS (code) == tcc_reference
> && arg0 && TREE_THIS_VOLATILE (arg0));
>
> Marek