[PATCH] reassoc vs uninitialized variable {PR112581]

[PATCH] reassoc vs uninitialized variable {PR112581]

[Andrew Pinski]

Like r14-2293-g11350734240dba and r14-2289-gb083203f053f16,
reassociation can combine across a few bb and one of the usage
can be an uninitializated variable and if going from an conditional
usage to an unconditional usage can cause wrong code.
This uses maybe_undef_p like other passes where this can happen.

Note if-to-switch uses the function (init_range_entry) provided
by ressociation so we need to call mark_ssa_maybe_undefs there;
otherwise we assume almost all ssa names are uninitialized.

Bootstrapped and tested on x86_64-linux-gnu.

gcc/ChangeLog:

	PR tree-optimization/112581
	* gimple-if-to-switch.cc (pass_if_to_switch::execute): Call
	mark_ssa_maybe_undefs.
	* tree-ssa-reassoc.cc (can_reassociate_op_p): Uninitialized
	variables can not be reassociated.
	(init_range_entry): Check for uninitialized variables too.
	(init_reassoc): Call mark_ssa_maybe_undefs.

gcc/testsuite/ChangeLog:

	PR tree-optimization/112581
	* gcc.c-torture/execute/pr112581-1.c: New test.

Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
---
 gcc/gimple-if-to-switch.cc                    |  3 ++
 .../gcc.c-torture/execute/pr112581-1.c        | 37 +++++++++++++++++++
 gcc/tree-ssa-reassoc.cc                       |  7 +++-
 3 files changed, 46 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr112581-1.c

diff --git a/gcc/gimple-if-to-switch.cc b/gcc/gimple-if-to-switch.cc
index 7792a6024cd..af8d6684d32 100644
--- a/gcc/gimple-if-to-switch.cc
+++ b/gcc/gimple-if-to-switch.cc
@@ -54,6 +54,7 @@ along with GCC; see the file COPYING3.  If not see
 #include "alloc-pool.h"
 #include "tree-switch-conversion.h"
 #include "tree-ssa-reassoc.h"
+#include "tree-ssa.h"
 
 using namespace tree_switch_conversion;
 
@@ -494,6 +495,8 @@ pass_if_to_switch::execute (function *fun)
   auto_vec<if_chain *> all_candidates;
   hash_map<basic_block, condition_info *> conditions_in_bbs;
 
+  mark_ssa_maybe_undefs ();
+
   basic_block bb;
   FOR_EACH_BB_FN (bb, fun)
     find_conditions (bb, &conditions_in_bbs);
diff --git a/gcc/testsuite/gcc.c-torture/execute/pr112581-1.c b/gcc/testsuite/gcc.c-torture/execute/pr112581-1.c
new file mode 100644
index 00000000000..14081c96d58
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/execute/pr112581-1.c
@@ -0,0 +1,37 @@
+/* { dg-require-effective-target int32plus } */
+/* PR tree-optimization/112581 */
+/* reassociation, used to combine 2 bb to together,
+   that made an unitialized variable unconditional used
+   which then at runtime would cause an infinite loop.  */
+int a = -1, b = 2501896061, c, d, e, f = 3, g;
+int main() {
+  unsigned h;
+  int i;
+  d = 0;
+  for (; d < 1; d++) {
+    int j = ~-((6UL ^ a) / b);
+    if (b)
+    L:
+      if (!f)
+        continue;
+    if (c)
+      i = 1;
+    if (j) {
+      i = 0;
+      while (e)
+        ;
+    }
+    g = -1 % b;
+    h = ~(b || h);
+    f = g || 0;
+    a = a || 0;
+    if (!a)
+      h = 0;
+    while (h > 4294967294)
+      if (i)
+        break;
+    if (c)
+      goto L;
+  }
+  return 0;
+}
diff --git a/gcc/tree-ssa-reassoc.cc b/gcc/tree-ssa-reassoc.cc
index cdef9f7cdc3..94873745928 100644
--- a/gcc/tree-ssa-reassoc.cc
+++ b/gcc/tree-ssa-reassoc.cc
@@ -647,6 +647,9 @@ can_reassociate_op_p (tree op)
 {
   if (TREE_CODE (op) == SSA_NAME && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (op))
     return false;
+  /* Uninitialized variables can't participate in reassociation. */
+  if (TREE_CODE (op) == SSA_NAME && ssa_name_maybe_undef_p (op))
+    return false;
   /* Make sure asm goto outputs do not participate in reassociation since
      we have no way to find an insertion place after asm goto.  */
   if (TREE_CODE (op) == SSA_NAME
@@ -2600,7 +2603,8 @@ init_range_entry (struct range_entry *r, tree exp, gimple *stmt)
 	}
 
       if (TREE_CODE (arg0) != SSA_NAME
-	  || SSA_NAME_OCCURS_IN_ABNORMAL_PHI (arg0))
+	  || SSA_NAME_OCCURS_IN_ABNORMAL_PHI (arg0)
+	  || ssa_name_maybe_undef_p (arg0))
 	break;
       loc = gimple_location (stmt);
       switch (code)
@@ -7418,6 +7422,7 @@ init_reassoc (void)
   free (bbs);
   calculate_dominance_info (CDI_POST_DOMINATORS);
   plus_negates = vNULL;
+  mark_ssa_maybe_undefs ();
 }
 
 /* Cleanup after the reassociation pass, and print stats if
-- 
2.39.3

Re: [PATCH] reassoc vs uninitialized variable {PR112581]

[Richard Biener]

On Sat, Dec 23, 2023 at 7:35 PM Andrew Pinski <quic_apinski@quicinc.com> wrote:
>
> Like r14-2293-g11350734240dba and r14-2289-gb083203f053f16,
> reassociation can combine across a few bb and one of the usage
> can be an uninitializated variable and if going from an conditional
> usage to an unconditional usage can cause wrong code.
> This uses maybe_undef_p like other passes where this can happen.
>
> Note if-to-switch uses the function (init_range_entry) provided
> by ressociation so we need to call mark_ssa_maybe_undefs there;
> otherwise we assume almost all ssa names are uninitialized.
>
> Bootstrapped and tested on x86_64-linux-gnu.

OK.

Thanks,
Richard.

> gcc/ChangeLog:
>
>         PR tree-optimization/112581
>         * gimple-if-to-switch.cc (pass_if_to_switch::execute): Call
>         mark_ssa_maybe_undefs.
>         * tree-ssa-reassoc.cc (can_reassociate_op_p): Uninitialized
>         variables can not be reassociated.
>         (init_range_entry): Check for uninitialized variables too.
>         (init_reassoc): Call mark_ssa_maybe_undefs.
>
> gcc/testsuite/ChangeLog:
>
>         PR tree-optimization/112581
>         * gcc.c-torture/execute/pr112581-1.c: New test.
>
> Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>
> ---
>  gcc/gimple-if-to-switch.cc                    |  3 ++
>  .../gcc.c-torture/execute/pr112581-1.c        | 37 +++++++++++++++++++
>  gcc/tree-ssa-reassoc.cc                       |  7 +++-
>  3 files changed, 46 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/gcc.c-torture/execute/pr112581-1.c
>
> diff --git a/gcc/gimple-if-to-switch.cc b/gcc/gimple-if-to-switch.cc
> index 7792a6024cd..af8d6684d32 100644
> --- a/gcc/gimple-if-to-switch.cc
> +++ b/gcc/gimple-if-to-switch.cc
> @@ -54,6 +54,7 @@ along with GCC; see the file COPYING3.  If not see
>  #include "alloc-pool.h"
>  #include "tree-switch-conversion.h"
>  #include "tree-ssa-reassoc.h"
> +#include "tree-ssa.h"
>
>  using namespace tree_switch_conversion;
>
> @@ -494,6 +495,8 @@ pass_if_to_switch::execute (function *fun)
>    auto_vec<if_chain *> all_candidates;
>    hash_map<basic_block, condition_info *> conditions_in_bbs;
>
> +  mark_ssa_maybe_undefs ();
> +
>    basic_block bb;
>    FOR_EACH_BB_FN (bb, fun)
>      find_conditions (bb, &conditions_in_bbs);
> diff --git a/gcc/testsuite/gcc.c-torture/execute/pr112581-1.c b/gcc/testsuite/gcc.c-torture/execute/pr112581-1.c
> new file mode 100644
> index 00000000000..14081c96d58
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/execute/pr112581-1.c
> @@ -0,0 +1,37 @@
> +/* { dg-require-effective-target int32plus } */
> +/* PR tree-optimization/112581 */
> +/* reassociation, used to combine 2 bb to together,
> +   that made an unitialized variable unconditional used
> +   which then at runtime would cause an infinite loop.  */
> +int a = -1, b = 2501896061, c, d, e, f = 3, g;
> +int main() {
> +  unsigned h;
> +  int i;
> +  d = 0;
> +  for (; d < 1; d++) {
> +    int j = ~-((6UL ^ a) / b);
> +    if (b)
> +    L:
> +      if (!f)
> +        continue;
> +    if (c)
> +      i = 1;
> +    if (j) {
> +      i = 0;
> +      while (e)
> +        ;
> +    }
> +    g = -1 % b;
> +    h = ~(b || h);
> +    f = g || 0;
> +    a = a || 0;
> +    if (!a)
> +      h = 0;
> +    while (h > 4294967294)
> +      if (i)
> +        break;
> +    if (c)
> +      goto L;
> +  }
> +  return 0;
> +}
> diff --git a/gcc/tree-ssa-reassoc.cc b/gcc/tree-ssa-reassoc.cc
> index cdef9f7cdc3..94873745928 100644
> --- a/gcc/tree-ssa-reassoc.cc
> +++ b/gcc/tree-ssa-reassoc.cc
> @@ -647,6 +647,9 @@ can_reassociate_op_p (tree op)
>  {
>    if (TREE_CODE (op) == SSA_NAME && SSA_NAME_OCCURS_IN_ABNORMAL_PHI (op))
>      return false;
> +  /* Uninitialized variables can't participate in reassociation. */
> +  if (TREE_CODE (op) == SSA_NAME && ssa_name_maybe_undef_p (op))
> +    return false;
>    /* Make sure asm goto outputs do not participate in reassociation since
>       we have no way to find an insertion place after asm goto.  */
>    if (TREE_CODE (op) == SSA_NAME
> @@ -2600,7 +2603,8 @@ init_range_entry (struct range_entry *r, tree exp, gimple *stmt)
>         }
>
>        if (TREE_CODE (arg0) != SSA_NAME
> -         || SSA_NAME_OCCURS_IN_ABNORMAL_PHI (arg0))
> +         || SSA_NAME_OCCURS_IN_ABNORMAL_PHI (arg0)
> +         || ssa_name_maybe_undef_p (arg0))
>         break;
>        loc = gimple_location (stmt);
>        switch (code)
> @@ -7418,6 +7422,7 @@ init_reassoc (void)
>    free (bbs);
>    calculate_dominance_info (CDI_POST_DOMINATORS);
>    plus_negates = vNULL;
> +  mark_ssa_maybe_undefs ();
>  }
>
>  /* Cleanup after the reassociation pass, and print stats if
> --
> 2.39.3
>