* [PATCH] Fix infinite recursion with div-by-zero (PR middle-end/70992)
@ 2017-07-18 16:05 Marek Polacek
2017-07-19 10:46 ` Richard Biener
0 siblings, 1 reply; 12+ messages in thread
From: Marek Polacek @ 2017-07-18 16:05 UTC (permalink / raw)
To: GCC Patches
We ended up in infinite recursion between extract_muldiv_1 and
fold_plusminus_mult_expr, because one turns this expression into the other
and the other does the reverse:
((2147483648 / 0) * 2) + 2 <-> 2 * (2147483648 / 0 + 1)
I tried (unsuccessfully) to fix it in either extract_muldiv_1 or
fold_plusminus_mult_expr, but in the end I went with just turning (x / 0) + A
to x / 0 (and similarly for %), because with that undefined division we can do
anything and this fixes the issue. Any better ideas?
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2017-07-18 Marek Polacek <polacek@redhat.com>
PR middle-end/70992
* fold-const.c (fold_binary_loc): Fold (x / 0) + A to x / 0,
and (x % 0) + A to x % 0.
* gcc.dg/torture/pr70992.c: New test.
* gcc.dg/torture/pr70992-2.c: New test.
diff --git gcc/fold-const.c gcc/fold-const.c
index 1bcbbb58154..9abdc9a8c20 100644
--- gcc/fold-const.c
+++ gcc/fold-const.c
@@ -9387,6 +9387,12 @@ fold_binary_loc (location_t loc,
TREE_TYPE (arg0), arg0,
cst0));
}
+ /* Adding anything to a division-by-zero makes no sense and
+ can confuse extract_muldiv and fold_plusminus_mult_expr. */
+ else if ((TREE_CODE (arg0) == TRUNC_DIV_EXPR
+ || TREE_CODE (arg0) == TRUNC_MOD_EXPR)
+ && integer_zerop (TREE_OPERAND (arg0, 1)))
+ return fold_convert_loc (loc, type, arg0);
}
/* Handle (A1 * C1) + (A2 * C2) with A1, A2 or C1, C2 being the same or
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" } */
+}
Marek
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Fix infinite recursion with div-by-zero (PR middle-end/70992)
2017-07-18 16:05 [PATCH] Fix infinite recursion with div-by-zero (PR middle-end/70992) Marek Polacek
@ 2017-07-19 10:46 ` Richard Biener
2017-07-19 13:56 ` Marek Polacek
0 siblings, 1 reply; 12+ messages in thread
From: Richard Biener @ 2017-07-19 10:46 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches
On Tue, Jul 18, 2017 at 6:05 PM, Marek Polacek <polacek@redhat.com> wrote:
> We ended up in infinite recursion between extract_muldiv_1 and
> fold_plusminus_mult_expr, because one turns this expression into the other
> and the other does the reverse:
>
> ((2147483648 / 0) * 2) + 2 <-> 2 * (2147483648 / 0 + 1)
>
> I tried (unsuccessfully) to fix it in either extract_muldiv_1 or
> fold_plusminus_mult_expr, but in the end I went with just turning (x / 0) + A
> to x / 0 (and similarly for %), because with that undefined division we can do
> anything and this fixes the issue. Any better ideas?
Heh - I looked at this at least twice as well with no conclusive fix...
My final thought was to fold division/modulo by zero to __builtin_trap () but
I didn't get to implement that. I'm not sure if we need to preserve
the behavior
of raising SIGFPE as I think at least the C standard makes it undefined.
OTOH other languages with non-call-exceptions might want to catch division
by zero.
Did you see why the oscillation doesn't happen for
((2147483648 / A) * 2) + 2 <-> 2 * (2147483648 / A + 1)
? What's special for the zero constant as divisor?
Richard.
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2017-07-18 Marek Polacek <polacek@redhat.com>
>
> PR middle-end/70992
> * fold-const.c (fold_binary_loc): Fold (x / 0) + A to x / 0,
> and (x % 0) + A to x % 0.
>
> * gcc.dg/torture/pr70992.c: New test.
> * gcc.dg/torture/pr70992-2.c: New test.
>
> diff --git gcc/fold-const.c gcc/fold-const.c
> index 1bcbbb58154..9abdc9a8c20 100644
> --- gcc/fold-const.c
> +++ gcc/fold-const.c
> @@ -9387,6 +9387,12 @@ fold_binary_loc (location_t loc,
> TREE_TYPE (arg0), arg0,
> cst0));
> }
> + /* Adding anything to a division-by-zero makes no sense and
> + can confuse extract_muldiv and fold_plusminus_mult_expr. */
> + else if ((TREE_CODE (arg0) == TRUNC_DIV_EXPR
> + || TREE_CODE (arg0) == TRUNC_MOD_EXPR)
> + && integer_zerop (TREE_OPERAND (arg0, 1)))
> + return fold_convert_loc (loc, type, arg0);
> }
>
> /* Handle (A1 * C1) + (A2 * C2) with A1, A2 or C1, C2 being the same or
> 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" } */
> +}
>
> Marek
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Fix infinite recursion with div-by-zero (PR middle-end/70992)
2017-07-19 10:46 ` Richard Biener
@ 2017-07-19 13:56 ` Marek Polacek
2017-07-20 10:55 ` Richard Biener
0 siblings, 1 reply; 12+ messages in thread
From: Marek Polacek @ 2017-07-19 13:56 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches
On Wed, Jul 19, 2017 at 12:45:12PM +0200, Richard Biener wrote:
> On Tue, Jul 18, 2017 at 6:05 PM, Marek Polacek <polacek@redhat.com> wrote:
> > We ended up in infinite recursion between extract_muldiv_1 and
> > fold_plusminus_mult_expr, because one turns this expression into the other
> > and the other does the reverse:
> >
> > ((2147483648 / 0) * 2) + 2 <-> 2 * (2147483648 / 0 + 1)
> >
> > I tried (unsuccessfully) to fix it in either extract_muldiv_1 or
> > fold_plusminus_mult_expr, but in the end I went with just turning (x / 0) + A
> > to x / 0 (and similarly for %), because with that undefined division we can do
> > anything and this fixes the issue. Any better ideas?
>
> Heh - I looked at this at least twice as well with no conclusive fix...
>
> My final thought was to fold division/modulo by zero to __builtin_trap () but
> I didn't get to implement that. I'm not sure if we need to preserve
> the behavior
> of raising SIGFPE as I think at least the C standard makes it undefined.
> OTOH other languages with non-call-exceptions might want to catch division
> by zero.
It's definitely undefined in C, so there we can do anything we see fit, but not
sure about the rest
> Did you see why the oscillation doesn't happen for
>
> ((2147483648 / A) * 2) + 2 <-> 2 * (2147483648 / A + 1)
>
> ? What's special for the zero constant as divisor?
I think it comes down to how split_tree splits the expression. For the above
we never call associate_trees, i.e., this condition is never true:
9647 if (ok
9648 && (2 < ((var0 != 0) + (var1 != 0)
9649 + (con0 != 0) + (con1 != 0)
9650 + (lit0 != 0) + (lit1 != 0)
9651 + (minus_lit0 != 0) + (minus_lit1 != 0))))
because var0 = so, lit1 = 2, and the rest is null.
We also don't go into infinite recursion with x / 0 instead of 2147483648 / 0,
because split_tree will put "x / 0 + 1" into var0, whereas it will put
2147483648 / 0 into con1, because it's TREE_CONSTANT - and so we have more than
2 exprs that are non-null and we end up looping.
One thing I pondered was to set TREE_OVERFLOW for division-by-zero, and avoid
folding such expressions (e.g. in extract_muldiv), but that probably wouldn't
help if the division-by-zero was nested in an expression.
Also, a funny thing, the original testcase
uint32_t
ls (uint32_t so)
{
return (so + so) * (0x80000000 / 0 + 1);
}
will compile just fine if we change the parameter type to unsigned int. Even
though uint32_t _is_ unsigned int!
Marek
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Fix infinite recursion with div-by-zero (PR middle-end/70992)
2017-07-19 13:56 ` Marek Polacek
@ 2017-07-20 10:55 ` Richard Biener
2017-07-20 14:20 ` Marek Polacek
0 siblings, 1 reply; 12+ messages in thread
From: Richard Biener @ 2017-07-20 10:55 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches
On Wed, Jul 19, 2017 at 3:55 PM, Marek Polacek <polacek@redhat.com> wrote:
> On Wed, Jul 19, 2017 at 12:45:12PM +0200, Richard Biener wrote:
>> On Tue, Jul 18, 2017 at 6:05 PM, Marek Polacek <polacek@redhat.com> wrote:
>> > We ended up in infinite recursion between extract_muldiv_1 and
>> > fold_plusminus_mult_expr, because one turns this expression into the other
>> > and the other does the reverse:
>> >
>> > ((2147483648 / 0) * 2) + 2 <-> 2 * (2147483648 / 0 + 1)
>> >
>> > I tried (unsuccessfully) to fix it in either extract_muldiv_1 or
>> > fold_plusminus_mult_expr, but in the end I went with just turning (x / 0) + A
>> > to x / 0 (and similarly for %), because with that undefined division we can do
>> > anything and this fixes the issue. Any better ideas?
>>
>> Heh - I looked at this at least twice as well with no conclusive fix...
>>
>> My final thought was to fold division/modulo by zero to __builtin_trap () but
>> I didn't get to implement that. I'm not sure if we need to preserve
>> the behavior
>> of raising SIGFPE as I think at least the C standard makes it undefined.
>> OTOH other languages with non-call-exceptions might want to catch division
>> by zero.
>
> It's definitely undefined in C, so there we can do anything we see fit, but not
> sure about the rest
>
>> Did you see why the oscillation doesn't happen for
>>
>> ((2147483648 / A) * 2) + 2 <-> 2 * (2147483648 / A + 1)
>>
>> ? What's special for the zero constant as divisor?
>
> I think it comes down to how split_tree splits the expression. For the above
> we never call associate_trees, i.e., this condition is never true:
>
> 9647 if (ok
> 9648 && (2 < ((var0 != 0) + (var1 != 0)
> 9649 + (con0 != 0) + (con1 != 0)
> 9650 + (lit0 != 0) + (lit1 != 0)
> 9651 + (minus_lit0 != 0) + (minus_lit1 != 0))))
>
> because var0 = so, lit1 = 2, and the rest is null.
>
> We also don't go into infinite recursion with x / 0 instead of 2147483648 / 0,
> because split_tree will put "x / 0 + 1" into var0, whereas it will put
> 2147483648 / 0 into con1, because it's TREE_CONSTANT - and so we have more than
> 2 exprs that are non-null and we end up looping.
Ah yeah - now I remeber. Stupid associate relying on TREE_CONSTANT ...
Maybe sth like
Index: gcc/fold-const.c
===================================================================
--- gcc/fold-const.c (revision 250379)
+++ gcc/fold-const.c (working copy)
@@ -816,9 +816,9 @@ split_tree (location_t loc, tree in, tre
|| TREE_CODE (op1) == FIXED_CST)
*litp = op1, neg_litp_p = neg1_p, op1 = 0;
- if (op0 != 0 && TREE_CONSTANT (op0))
+ if (op0 != 0 && TREE_CONSTANT (op0) && ! tree_could_trap_p (op0))
*conp = op0, op0 = 0;
- else if (op1 != 0 && TREE_CONSTANT (op1))
+ else if (op1 != 0 && TREE_CONSTANT (op1) && ! tree_could_trap_p (op1))
*conp = op1, neg_conp_p = neg1_p, op1 = 0;
/* If we haven't dealt with either operand, this is not a case we can
@@ -846,7 +846,7 @@ split_tree (location_t loc, tree in, tre
var = negate_expr (var);
}
}
- else if (TREE_CONSTANT (in))
+ else if (TREE_CONSTANT (in) && ! tree_could_trap_p (in))
*conp = in;
else if (TREE_CODE (in) == BIT_NOT_EXPR
&& code == PLUS_EXPR)
would help that?
> One thing I pondered was to set TREE_OVERFLOW for division-by-zero, and avoid
> folding such expressions (e.g. in extract_muldiv), but that probably wouldn't
> help if the division-by-zero was nested in an expression.
>
> Also, a funny thing, the original testcase
> uint32_t
> ls (uint32_t so)
> {
> return (so + so) * (0x80000000 / 0 + 1);
> }
> will compile just fine if we change the parameter type to unsigned int. Even
> though uint32_t _is_ unsigned int!
;)
> Marek
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Fix infinite recursion with div-by-zero (PR middle-end/70992)
2017-07-20 10:55 ` Richard Biener
@ 2017-07-20 14:20 ` Marek Polacek
2017-07-20 18:23 ` Richard Biener
0 siblings, 1 reply; 12+ messages in thread
From: Marek Polacek @ 2017-07-20 14:20 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches
On Thu, Jul 20, 2017 at 12:55:10PM +0200, Richard Biener wrote:
> On Wed, Jul 19, 2017 at 3:55 PM, Marek Polacek <polacek@redhat.com> wrote:
> > On Wed, Jul 19, 2017 at 12:45:12PM +0200, Richard Biener wrote:
> >> On Tue, Jul 18, 2017 at 6:05 PM, Marek Polacek <polacek@redhat.com> wrote:
> >> > We ended up in infinite recursion between extract_muldiv_1 and
> >> > fold_plusminus_mult_expr, because one turns this expression into the other
> >> > and the other does the reverse:
> >> >
> >> > ((2147483648 / 0) * 2) + 2 <-> 2 * (2147483648 / 0 + 1)
> >> >
> >> > I tried (unsuccessfully) to fix it in either extract_muldiv_1 or
> >> > fold_plusminus_mult_expr, but in the end I went with just turning (x / 0) + A
> >> > to x / 0 (and similarly for %), because with that undefined division we can do
> >> > anything and this fixes the issue. Any better ideas?
> >>
> >> Heh - I looked at this at least twice as well with no conclusive fix...
> >>
> >> My final thought was to fold division/modulo by zero to __builtin_trap () but
> >> I didn't get to implement that. I'm not sure if we need to preserve
> >> the behavior
> >> of raising SIGFPE as I think at least the C standard makes it undefined.
> >> OTOH other languages with non-call-exceptions might want to catch division
> >> by zero.
> >
> > It's definitely undefined in C, so there we can do anything we see fit, but not
> > sure about the rest
> >
> >> Did you see why the oscillation doesn't happen for
> >>
> >> ((2147483648 / A) * 2) + 2 <-> 2 * (2147483648 / A + 1)
> >>
> >> ? What's special for the zero constant as divisor?
> >
> > I think it comes down to how split_tree splits the expression. For the above
> > we never call associate_trees, i.e., this condition is never true:
> >
> > 9647 if (ok
> > 9648 && (2 < ((var0 != 0) + (var1 != 0)
> > 9649 + (con0 != 0) + (con1 != 0)
> > 9650 + (lit0 != 0) + (lit1 != 0)
> > 9651 + (minus_lit0 != 0) + (minus_lit1 != 0))))
> >
> > because var0 = so, lit1 = 2, and the rest is null.
> >
> > We also don't go into infinite recursion with x / 0 instead of 2147483648 / 0,
> > because split_tree will put "x / 0 + 1" into var0, whereas it will put
> > 2147483648 / 0 into con1, because it's TREE_CONSTANT - and so we have more than
> > 2 exprs that are non-null and we end up looping.
>
> Ah yeah - now I remeber. Stupid associate relying on TREE_CONSTANT ...
>
> Maybe sth like
>
> Index: gcc/fold-const.c
> ===================================================================
> --- gcc/fold-const.c (revision 250379)
> +++ gcc/fold-const.c (working copy)
> @@ -816,9 +816,9 @@ split_tree (location_t loc, tree in, tre
> || TREE_CODE (op1) == FIXED_CST)
> *litp = op1, neg_litp_p = neg1_p, op1 = 0;
>
> - if (op0 != 0 && TREE_CONSTANT (op0))
> + if (op0 != 0 && TREE_CONSTANT (op0) && ! tree_could_trap_p (op0))
> *conp = op0, op0 = 0;
> - else if (op1 != 0 && TREE_CONSTANT (op1))
> + else if (op1 != 0 && TREE_CONSTANT (op1) && ! tree_could_trap_p (op1))
> *conp = op1, neg_conp_p = neg1_p, op1 = 0;
>
> /* If we haven't dealt with either operand, this is not a case we can
> @@ -846,7 +846,7 @@ split_tree (location_t loc, tree in, tre
> var = negate_expr (var);
> }
> }
> - else if (TREE_CONSTANT (in))
> + else if (TREE_CONSTANT (in) && ! tree_could_trap_p (in))
> *conp = in;
> else if (TREE_CODE (in) == BIT_NOT_EXPR
> && code == PLUS_EXPR)
>
> would help that?
That works for one testcase, but not for another, e.g. this one:
unsigned int *od;
int
fn (void)
{
return (0 % 0 + 1) * *od * 2; /* { dg-warning "division by zero" } */
}
because tree_could_trap_p says "no" for "0 % 0 + 1" -- it just sees the outer
PLUS_MINUS and never checks the subexpression with %. Should we change that?
Guess we'd have to call tree_could_trap_p recursively...
Marek
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Fix infinite recursion with div-by-zero (PR middle-end/70992)
2017-07-20 14:20 ` Marek Polacek
@ 2017-07-20 18:23 ` Richard Biener
2017-07-25 12:19 ` Marek Polacek
0 siblings, 1 reply; 12+ messages in thread
From: Richard Biener @ 2017-07-20 18:23 UTC (permalink / raw)
To: Marek Polacek; +Cc: GCC Patches
On July 20, 2017 4:20:00 PM GMT+02:00, Marek Polacek <polacek@redhat.com> wrote:
>On Thu, Jul 20, 2017 at 12:55:10PM +0200, Richard Biener wrote:
>> On Wed, Jul 19, 2017 at 3:55 PM, Marek Polacek <polacek@redhat.com>
>wrote:
>> > On Wed, Jul 19, 2017 at 12:45:12PM +0200, Richard Biener wrote:
>> >> On Tue, Jul 18, 2017 at 6:05 PM, Marek Polacek
><polacek@redhat.com> wrote:
>> >> > We ended up in infinite recursion between extract_muldiv_1 and
>> >> > fold_plusminus_mult_expr, because one turns this expression into
>the other
>> >> > and the other does the reverse:
>> >> >
>> >> > ((2147483648 / 0) * 2) + 2 <-> 2 * (2147483648 / 0 + 1)
>> >> >
>> >> > I tried (unsuccessfully) to fix it in either extract_muldiv_1 or
>> >> > fold_plusminus_mult_expr, but in the end I went with just
>turning (x / 0) + A
>> >> > to x / 0 (and similarly for %), because with that undefined
>division we can do
>> >> > anything and this fixes the issue. Any better ideas?
>> >>
>> >> Heh - I looked at this at least twice as well with no conclusive
>fix...
>> >>
>> >> My final thought was to fold division/modulo by zero to
>__builtin_trap () but
>> >> I didn't get to implement that. I'm not sure if we need to
>preserve
>> >> the behavior
>> >> of raising SIGFPE as I think at least the C standard makes it
>undefined.
>> >> OTOH other languages with non-call-exceptions might want to catch
>division
>> >> by zero.
>> >
>> > It's definitely undefined in C, so there we can do anything we see
>fit, but not
>> > sure about the rest
>> >
>> >> Did you see why the oscillation doesn't happen for
>> >>
>> >> ((2147483648 / A) * 2) + 2 <-> 2 * (2147483648 / A + 1)
>> >>
>> >> ? What's special for the zero constant as divisor?
>> >
>> > I think it comes down to how split_tree splits the expression. For
>the above
>> > we never call associate_trees, i.e., this condition is never true:
>> >
>> > 9647 if (ok
>> > 9648 && (2 < ((var0 != 0) + (var1 != 0)
>> > 9649 + (con0 != 0) + (con1 != 0)
>> > 9650 + (lit0 != 0) + (lit1 != 0)
>> > 9651 + (minus_lit0 != 0) + (minus_lit1 !=
>0))))
>> >
>> > because var0 = so, lit1 = 2, and the rest is null.
>> >
>> > We also don't go into infinite recursion with x / 0 instead of
>2147483648 / 0,
>> > because split_tree will put "x / 0 + 1" into var0, whereas it will
>put
>> > 2147483648 / 0 into con1, because it's TREE_CONSTANT - and so we
>have more than
>> > 2 exprs that are non-null and we end up looping.
>>
>> Ah yeah - now I remeber. Stupid associate relying on TREE_CONSTANT
>...
>>
>> Maybe sth like
>>
>> Index: gcc/fold-const.c
>> ===================================================================
>> --- gcc/fold-const.c (revision 250379)
>> +++ gcc/fold-const.c (working copy)
>> @@ -816,9 +816,9 @@ split_tree (location_t loc, tree in, tre
>> || TREE_CODE (op1) == FIXED_CST)
>> *litp = op1, neg_litp_p = neg1_p, op1 = 0;
>>
>> - if (op0 != 0 && TREE_CONSTANT (op0))
>> + if (op0 != 0 && TREE_CONSTANT (op0) && ! tree_could_trap_p
>(op0))
>> *conp = op0, op0 = 0;
>> - else if (op1 != 0 && TREE_CONSTANT (op1))
>> + else if (op1 != 0 && TREE_CONSTANT (op1) && !
>tree_could_trap_p (op1))
>> *conp = op1, neg_conp_p = neg1_p, op1 = 0;
>>
>> /* If we haven't dealt with either operand, this is not a case
>we can
>> @@ -846,7 +846,7 @@ split_tree (location_t loc, tree in, tre
>> var = negate_expr (var);
>> }
>> }
>> - else if (TREE_CONSTANT (in))
>> + else if (TREE_CONSTANT (in) && ! tree_could_trap_p (in))
>> *conp = in;
>> else if (TREE_CODE (in) == BIT_NOT_EXPR
>> && code == PLUS_EXPR)
>>
>> would help that?
>
>That works for one testcase, but not for another, e.g. this one:
>
>unsigned int *od;
>int
>fn (void)
>{
> return (0 % 0 + 1) * *od * 2; /* { dg-warning "division by zero" } */
>}
>
>because tree_could_trap_p says "no" for "0 % 0 + 1" -- it just sees the
>outer
>PLUS_MINUS and never checks the subexpression with %. Should we change
>that?
>Guess we'd have to call tree_could_trap_p recursively...
No, we shouldn't. Maybe trapping ops shouldn't be marked TREE_CONSTANT?
(Make sure to test with Ada...)
Richard.
> Marek
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Fix infinite recursion with div-by-zero (PR middle-end/70992)
2017-07-20 18:23 ` Richard Biener
@ 2017-07-25 12:19 ` Marek Polacek
2017-07-25 13:05 ` Richard Biener
0 siblings, 1 reply; 12+ messages in thread
From: Marek Polacek @ 2017-07-25 12:19 UTC (permalink / raw)
To: Richard Biener; +Cc: GCC Patches
On Thu, Jul 20, 2017 at 08:22:59PM +0200, Richard Biener wrote:
> No, we shouldn't. Maybe trapping ops shouldn't be marked TREE_CONSTANT?
> (Make sure to test with Ada...)
That works for both testcases, but I can't say I really like the idea of
modifying build2... but it's where the TREE_CONSTANT comes from.
The tree_could_trap_p bit is because the C++ FE can pass "< x" around, e.g.
a LT_EXPR with the first operand null.
Bootstrapped/regtested on x86_64-linux (Ada included) and powerpc64le-unknown-linux-gnu,
ok for trunk?
2017-07-25 Marek Polacek <polacek@redhat.com>
PR c/70992
* tree-eh.c (tree_could_trap_p): Check if the first operand is null.
* tree.c: Include "tree-eh.h".
(build2_stat): Only set TREE_CONSTANT if the tree couldn't trap.
* 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-eh.c gcc/tree-eh.c
index c68d71a5e54..cde4ef3f387 100644
--- gcc/tree-eh.c
+++ gcc/tree-eh.c
@@ -2599,7 +2599,7 @@ tree_could_trap_p (tree expr)
if (t)
{
- if (COMPARISON_CLASS_P (expr))
+ if (COMPARISON_CLASS_P (expr) && TREE_OPERAND (expr, 0))
fp_operation = FLOAT_TYPE_P (TREE_TYPE (TREE_OPERAND (expr, 0)));
else
fp_operation = FLOAT_TYPE_P (t);
diff --git gcc/tree.c gcc/tree.c
index b7de2840ac6..205ba7fd79d 100644
--- gcc/tree.c
+++ gcc/tree.c
@@ -62,6 +62,7 @@ along with GCC; see the file COPYING3. If not see
#include "print-tree.h"
#include "ipa-utils.h"
#include "selftest.h"
+#include "tree-eh.h"
/* Tree code classes. */
@@ -4505,7 +4506,7 @@ 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;
+ TREE_CONSTANT (t) = constant && !tree_could_trap_p (t);
TREE_THIS_VOLATILE (t)
= (TREE_CODE_CLASS (code) == tcc_reference
&& arg0 && TREE_THIS_VOLATILE (arg0));
Marek
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Fix infinite recursion with div-by-zero (PR middle-end/70992)
2017-07-25 12:19 ` Marek Polacek
@ 2017-07-25 13:05 ` Richard Biener
2017-07-25 13:27 ` Eric Botcazou
0 siblings, 1 reply; 12+ messages in thread
From: Richard Biener @ 2017-07-25 13:05 UTC (permalink / raw)
To: Marek Polacek, Eric Botcazou; +Cc: GCC Patches
On Tue, Jul 25, 2017 at 2:19 PM, Marek Polacek <polacek@redhat.com> wrote:
> On Thu, Jul 20, 2017 at 08:22:59PM +0200, Richard Biener wrote:
>> No, we shouldn't. Maybe trapping ops shouldn't be marked TREE_CONSTANT?
>> (Make sure to test with Ada...)
>
> That works for both testcases, but I can't say I really like the idea of
> modifying build2... but it's where the TREE_CONSTANT comes from.
Yes, I know.
> The tree_could_trap_p bit is because the C++ FE can pass "< x" around, e.g.
> a LT_EXPR with the first operand null.
Eh... :/
> Bootstrapped/regtested on x86_64-linux (Ada included) and powerpc64le-unknown-linux-gnu,
> ok for trunk?
Eric, any comments?
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.
Thanks,
Richard.
> 2017-07-25 Marek Polacek <polacek@redhat.com>
>
> PR c/70992
> * tree-eh.c (tree_could_trap_p): Check if the first operand is null.
> * tree.c: Include "tree-eh.h".
> (build2_stat): Only set TREE_CONSTANT if the tree couldn't trap.
>
> * 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-eh.c gcc/tree-eh.c
> index c68d71a5e54..cde4ef3f387 100644
> --- gcc/tree-eh.c
> +++ gcc/tree-eh.c
> @@ -2599,7 +2599,7 @@ tree_could_trap_p (tree expr)
>
> if (t)
> {
> - if (COMPARISON_CLASS_P (expr))
> + if (COMPARISON_CLASS_P (expr) && TREE_OPERAND (expr, 0))
> fp_operation = FLOAT_TYPE_P (TREE_TYPE (TREE_OPERAND (expr, 0)));
> else
> fp_operation = FLOAT_TYPE_P (t);
> diff --git gcc/tree.c gcc/tree.c
> index b7de2840ac6..205ba7fd79d 100644
> --- gcc/tree.c
> +++ gcc/tree.c
> @@ -62,6 +62,7 @@ along with GCC; see the file COPYING3. If not see
> #include "print-tree.h"
> #include "ipa-utils.h"
> #include "selftest.h"
> +#include "tree-eh.h"
>
> /* Tree code classes. */
>
> @@ -4505,7 +4506,7 @@ 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;
> + TREE_CONSTANT (t) = constant && !tree_could_trap_p (t);
> TREE_THIS_VOLATILE (t)
> = (TREE_CODE_CLASS (code) == tcc_reference
> && arg0 && TREE_THIS_VOLATILE (arg0));
>
> Marek
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Fix infinite recursion with div-by-zero (PR middle-end/70992)
2017-07-25 13:05 ` Richard Biener
@ 2017-07-25 13:27 ` Eric Botcazou
2017-07-25 13:47 ` Richard Biener
0 siblings, 1 reply; 12+ messages in thread
From: Eric Botcazou @ 2017-07-25 13:27 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches, Marek Polacek
> 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.
--
Eric Botcazou
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Fix infinite recursion with div-by-zero (PR middle-end/70992)
2017-07-25 13:27 ` Eric Botcazou
@ 2017-07-25 13:47 ` Richard Biener
2017-07-26 11:35 ` Marek Polacek
0 siblings, 1 reply; 12+ messages in thread
From: Richard Biener @ 2017-07-25 13:47 UTC (permalink / raw)
To: Eric Botcazou; +Cc: GCC Patches, Marek Polacek
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 ;)
Thanks,
Richard.
> --
> Eric Botcazou
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Fix infinite recursion with div-by-zero (PR middle-end/70992)
2017-07-25 13:47 ` Richard Biener
@ 2017-07-26 11:35 ` Marek Polacek
2017-07-26 11:36 ` Richard Biener
0 siblings, 1 reply; 12+ messages in thread
From: Marek Polacek @ 2017-07-26 11:35 UTC (permalink / raw)
To: Richard Biener; +Cc: Eric Botcazou, GCC Patches
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?
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] Fix infinite recursion with div-by-zero (PR middle-end/70992)
2017-07-26 11:35 ` Marek Polacek
@ 2017-07-26 11:36 ` Richard Biener
0 siblings, 0 replies; 12+ messages in thread
From: Richard Biener @ 2017-07-26 11:36 UTC (permalink / raw)
To: Marek Polacek; +Cc: Eric Botcazou, GCC Patches
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
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-07-26 11:36 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-18 16:05 [PATCH] Fix infinite recursion with div-by-zero (PR middle-end/70992) Marek Polacek
2017-07-19 10:46 ` Richard Biener
2017-07-19 13:56 ` Marek Polacek
2017-07-20 10:55 ` Richard Biener
2017-07-20 14:20 ` Marek Polacek
2017-07-20 18:23 ` Richard Biener
2017-07-25 12:19 ` Marek Polacek
2017-07-25 13:05 ` Richard Biener
2017-07-25 13:27 ` Eric Botcazou
2017-07-25 13:47 ` Richard Biener
2017-07-26 11:35 ` Marek Polacek
2017-07-26 11:36 ` Richard Biener
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).