This looks like a latent problem in the -Wmaybe-uninitialized code unrelated to my work.

The problem here is a sequence of simplify_preds() followed by normalize_preds() that I added, but is based on prior art all over the file:


+      simplify_preds (&uninit_preds, NULL, false);
+      uninit_preds = normalize_preds (uninit_preds, NULL, false);

The problem is that in this particular testcase we trigger simplify_preds_4 which makes a copy of a chain, frees the chain, and then tries to use the chain later (in normalize_preds). The normalize_preds() function tries to free again the chain and we blow up:

This is the main problem in simplify_preds_4:

  /* Now clean up the chain.  */
  if (simplified)
    {
      for (i = 0; i < n; i++)
        {
          if ((*preds)[i].is_empty ())
            continue;
          s_preds.safe_push ((*preds)[i]);
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
// Makes a copy of the pred_chain.
        }

      destroy_predicate_vecs (preds);
// ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
// free() all the pred_chain's.

      (*preds) = s_preds;
// ^^^^^^^^^^^^^^^^^^^^^^
// Wait a minute, we still keep a copy of the pred_chains.
      s_preds = vNULL;
    }

I have no idea how this worked even before my patch. Perhaps we never had a simplify_preds() followed by a normalize_preds() where the simplification involved a call to simplify_preds_4.

Interestingly enough, simplify_preds_2() has the exact same code, but with the fix I am proposing. So this seems like an oversight. Also, the fact that the simplification in simplify_preds_2 is more common than the simplification performed in simplify_preds_4 would suggest that simplify_preds_4 was uncommon enough and probably wasn't been used in a simplify_preds/normalize_preds combo.

Anyways... I've made some other cleanups to the code, but the main gist of the entire patch is:

-      destroy_predicate_vecs (preds);
+      preds->release ();

That is, release preds, but don't free the associated memory with the pred_chain's therein.

This patch is on top of my pending patch here:

        https://gcc.gnu.org/ml/gcc-patches/2016-11/msg02900.html

Tested on x86-64 Linux.

OK for trunk?
Aldy
commit ec4443b8dcf89465cb8d9735a3e0a27b181c975c
Author: Aldy Hernandez <al...@redhat.com>
Date:   Thu Dec 1 04:53:38 2016 -0500

            PR middle-end/78548
            * tree-ssa-uninit.c (simplify_preds_4): Call release() instead of
            destroy_predicate_vecs.
            (uninit_uses_cannot_happen): Make uninit_preds a scalar.

diff --git a/gcc/testsuite/gcc.dg/uninit-pr78548.c 
b/gcc/testsuite/gcc.dg/uninit-pr78548.c
new file mode 100644
index 0000000..12e06dd
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/uninit-pr78548.c
@@ -0,0 +1,24 @@
+/* { dg-do compile } */
+/* { dg-options "-Wall -w -O2" } */
+
+char a;
+int b;
+unsigned c, d;
+short e;
+int main_f;
+int main (  ) {
+L0:
+    if ( e )     goto L1;
+    b = c & d || a;
+    if ( !c )     printf ( "", ( long long ) main_f );
+    if ( d || !c )     {
+        printf ( "%llu\n", ( long long ) main );
+        goto L2;
+    }
+    unsigned g = b;
+L1:
+    b = g;
+L2:
+    if ( b )     goto L0;
+  return 0;
+}
diff --git a/gcc/tree-ssa-uninit.c b/gcc/tree-ssa-uninit.c
index a648995..b4892c7 100644
--- a/gcc/tree-ssa-uninit.c
+++ b/gcc/tree-ssa-uninit.c
@@ -1774,7 +1774,7 @@ simplify_preds_4 (pred_chain_union *preds)
          s_preds.safe_push ((*preds)[i]);
        }
 
-      destroy_predicate_vecs (preds);
+      preds->release ();
       (*preds) = s_preds;
       s_preds = vNULL;
     }
@@ -2211,10 +2211,9 @@ uninit_uses_cannot_happen (gphi *phi, unsigned 
uninit_opnds,
 
   /* Look for the control dependencies of all the uninitialized
      operands and build guard predicates describing them.  */
-  unsigned i;
-  pred_chain_union uninit_preds[max_phi_args];
-  memset (uninit_preds, 0, sizeof (pred_chain_union) * phi_args);
-  for (i = 0; i < phi_args; ++i)
+  pred_chain_union uninit_preds;
+  bool ret = true;
+  for (unsigned i = 0; i < phi_args; ++i)
     {
       if (!MASK_TEST_BIT (uninit_opnds, i))
        continue;
@@ -2226,26 +2225,32 @@ uninit_uses_cannot_happen (gphi *phi, unsigned 
uninit_opnds,
       int num_calls = 0;
 
       /* Build the control dependency chain for uninit operand `i'...  */
+      uninit_preds = vNULL;
       if (!compute_control_dep_chain (find_dom (e->src),
                                      e->src, dep_chains, &num_chains,
                                      &cur_chain, &num_calls))
-       return false;
+       {
+         ret = false;
+         break;
+       }
       /* ...and convert it into a set of predicates.  */
       convert_control_dep_chain_into_preds (dep_chains, num_chains,
-                                           &uninit_preds[i]);
+                                           &uninit_preds);
       for (size_t j = 0; j < num_chains; ++j)
        dep_chains[j].release ();
-      simplify_preds (&uninit_preds[i], NULL, false);
-      uninit_preds[i]
-       = normalize_preds (uninit_preds[i], NULL, false);
+      simplify_preds (&uninit_preds, NULL, false);
+      uninit_preds = normalize_preds (uninit_preds, NULL, false);
 
       /* Can the guard for this uninitialized operand be invalidated
         by the PHI use?  */
-      if (!can_chain_union_be_invalidated_p (uninit_preds[i],
-                                            phi_use_guards[0]))
-       return false;
+      if (!can_chain_union_be_invalidated_p (uninit_preds, phi_use_guards[0]))
+       {
+         ret = false;
+         break;
+       }
     }
-  return true;
+  destroy_predicate_vecs (&uninit_preds);
+  return ret;
 }
 
 /* Computes the predicates that guard the use and checks

Reply via email to