diff mbox series

[PUSHED] operator_trunc_mod::wi_fold: Return VARYING for mod by zero.

Message ID 20201012165632.198721-1-aldyh@redhat.com
State New
Headers show
Series [PUSHED] operator_trunc_mod::wi_fold: Return VARYING for mod by zero. | expand

Commit Message

Aldy Hernandez Oct. 12, 2020, 4:56 p.m. UTC
Division by zero should return VARYING, otherwise we propagate undefine all over the
ranger and cause bad things to happen :).  This fixes MOD 0 to also return VARYING.

This is Andrew's patch.  I forgot to use --author for proper patch
attribution.

Tested on x86-64 Linux.

Pushed to trunk.

gcc/ChangeLog:

	PR tree-optimization/97378
	* range-op.cc (operator_trunc_mod::wi_fold): Return VARYING for mod by zero.

gcc/testsuite/ChangeLog:

	* gcc.dg/pr97378.c: New test.
---
 gcc/range-op.cc                |  6 +++---
 gcc/testsuite/gcc.dg/pr97378.c | 15 +++++++++++++++
 2 files changed, 18 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/pr97378.c

Comments

Richard Biener Oct. 13, 2020, 6:10 a.m. UTC | #1
On Mon, Oct 12, 2020 at 6:57 PM Aldy Hernandez via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> Division by zero should return VARYING, otherwise we propagate undefine all over the
> ranger and cause bad things to happen :)

So we never should propagate UNDEFINED?

>.  This fixes MOD 0 to also return VARYING.
>
> This is Andrew's patch.  I forgot to use --author for proper patch
> attribution.
>
> Tested on x86-64 Linux.
>
> Pushed to trunk.
>
> gcc/ChangeLog:
>
>         PR tree-optimization/97378
>         * range-op.cc (operator_trunc_mod::wi_fold): Return VARYING for mod by zero.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.dg/pr97378.c: New test.
> ---
>  gcc/range-op.cc                |  6 +++---
>  gcc/testsuite/gcc.dg/pr97378.c | 15 +++++++++++++++
>  2 files changed, 18 insertions(+), 3 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/pr97378.c
>
> diff --git a/gcc/range-op.cc b/gcc/range-op.cc
> index ce6ae2de20c..6108de367ad 100644
> --- a/gcc/range-op.cc
> +++ b/gcc/range-op.cc
> @@ -1359,7 +1359,7 @@ operator_div::wi_fold (irange &r, tree type,
>    // If we're definitely dividing by zero, there's nothing to do.
>    if (wi_zero_p (type, divisor_min, divisor_max))
>      {
> -      r.set_undefined ();
> +      r.set_varying (type);
>        return;
>      }
>
> @@ -2624,10 +2624,10 @@ operator_trunc_mod::wi_fold (irange &r, tree type,
>    signop sign = TYPE_SIGN (type);
>    unsigned prec = TYPE_PRECISION (type);
>
> -  // Mod 0 is undefined.  Return undefined.
> +  // Mod 0 is undefined.
>    if (wi_zero_p (type, rh_lb, rh_ub))
>      {
> -      r.set_undefined ();
> +      r.set_varying (type);
>        return;
>      }
>
> diff --git a/gcc/testsuite/gcc.dg/pr97378.c b/gcc/testsuite/gcc.dg/pr97378.c
> new file mode 100644
> index 00000000000..27e4a1f4321
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/pr97378.c
> @@ -0,0 +1,15 @@
> +// { dg-do compile }
> +// { dg-options "-O2" }
> +
> +int a, b, c;
> +void d() {
> +e : {
> +  long f;
> +  long *g = &f;
> +  if ((a != 0) - (b = 0))
> +    ;
> +  else
> +    a &= (*g %= a *= c) >= (*g || f);
> +  goto e;
> +}
> +}
> --
> 2.26.2
>
Andrew MacLeod Oct. 14, 2020, 4:43 p.m. UTC | #2
On 10/13/20 2:10 AM, Richard Biener wrote:
> On Mon, Oct 12, 2020 at 6:57 PM Aldy Hernandez via Gcc-patches
> <gcc-patches@gcc.gnu.org> wrote:
>> Division by zero should return VARYING, otherwise we propagate undefine all over the
>> ranger and cause bad things to happen :)
> So we never should propagate UNDEFINED?

I added a comment in the PR.

the problem was that we were feeding an undefined into the branch at the 
bottom, and the old ranger model was that undefined meant unreachable.. 
and it would therefore make BOTH sides of the branch unreachable, and 
that was triggering some unpleasant side effects when interactive with 
the subst-and-fold model and trying to calculate a range feeding into 
the condition.

I had audited rangeops so that when we folded undefined values we used 
varying as their value (as in we don't know what the value is) but 
missed the mod code.  I plan to  make UNDEFINED more consistent, i just 
need to find the time to sit down and audit it.  I suspect there are 
still a couple of lingering dark corners where undefined interactions 
aren't quite right.

Andrew
diff mbox series

Patch

diff --git a/gcc/range-op.cc b/gcc/range-op.cc
index ce6ae2de20c..6108de367ad 100644
--- a/gcc/range-op.cc
+++ b/gcc/range-op.cc
@@ -1359,7 +1359,7 @@  operator_div::wi_fold (irange &r, tree type,
   // If we're definitely dividing by zero, there's nothing to do.
   if (wi_zero_p (type, divisor_min, divisor_max))
     {
-      r.set_undefined ();
+      r.set_varying (type);
       return;
     }
 
@@ -2624,10 +2624,10 @@  operator_trunc_mod::wi_fold (irange &r, tree type,
   signop sign = TYPE_SIGN (type);
   unsigned prec = TYPE_PRECISION (type);
 
-  // Mod 0 is undefined.  Return undefined.
+  // Mod 0 is undefined.
   if (wi_zero_p (type, rh_lb, rh_ub))
     {
-      r.set_undefined ();
+      r.set_varying (type);
       return;
     }
 
diff --git a/gcc/testsuite/gcc.dg/pr97378.c b/gcc/testsuite/gcc.dg/pr97378.c
new file mode 100644
index 00000000000..27e4a1f4321
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr97378.c
@@ -0,0 +1,15 @@ 
+// { dg-do compile }
+// { dg-options "-O2" }
+
+int a, b, c;
+void d() {
+e : {
+  long f;
+  long *g = &f;
+  if ((a != 0) - (b = 0))
+    ;
+  else
+    a &= (*g %= a *= c) >= (*g || f);
+  goto e;
+}
+}