diff mbox

Fix dom ICE (PR tree-optimization/55329)

Message ID 20121115201415.GD1886@tucnak.redhat.com
State New
Headers show

Commit Message

Jakub Jelinek Nov. 15, 2012, 8:14 p.m. UTC 
Hi!

On the following testcase we ICE, because tree_ssa_dominator_optimize
modifies the bitmap while iterating it.  In particular, it contained just
a single bit in it, bitmap_clear_bit turned the bitmap into empty bitmap and
bitmap_set_bit on a lower bit reused the same bitmap_element with lower
index, but higher bit set in it, therefore EXECUTE_IF_SET_IN_BITMAP in next
iteration gave bit above highest basic block.

Fixed by never clearing any bits from the bitmap while traversing it,
it isn't IMHO a big deal, gimple_purge_all_dead_eh_edges will in a few
insns find out the forwarder block doesn't have any EDGE_EH edges and will
do nothing for it, on the other side to handle even chained forwarder blocks
it loops to find the last one.  Insertion of bits looks safe to me, we don't
care if those inserted bits are processed again or not (nothing will be done
for them anymore).

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2012-11-15  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/55329
	* tree-ssa-dom.c (tree_ssa_dominator_optimize): Never clear
	bits in needed_eh_cleanup while iterating over the bitmap.
	Look through all forwarder blocks at once.

	* g++.dg/opt/pr55329.C: New test.


	Jakub

Comments

Jeff Law Nov. 16, 2012, 7:37 p.m. UTC | #1 
On 11/15/2012 01:14 PM, Jakub Jelinek wrote:
> Hi!
>
> On the following testcase we ICE, because tree_ssa_dominator_optimize
> modifies the bitmap while iterating it.  In particular, it contained just
> a single bit in it, bitmap_clear_bit turned the bitmap into empty bitmap and
> bitmap_set_bit on a lower bit reused the same bitmap_element with lower
> index, but higher bit set in it, therefore EXECUTE_IF_SET_IN_BITMAP in next
> iteration gave bit above highest basic block.
>
> Fixed by never clearing any bits from the bitmap while traversing it,
> it isn't IMHO a big deal, gimple_purge_all_dead_eh_edges will in a few
> insns find out the forwarder block doesn't have any EDGE_EH edges and will
> do nothing for it, on the other side to handle even chained forwarder blocks
> it loops to find the last one.  Insertion of bits looks safe to me, we don't
> care if those inserted bits are processed again or not (nothing will be done
> for them anymore).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2012-11-15  Jakub Jelinek  <jakub@redhat.com>
>
> 	PR tree-optimization/55329
> 	* tree-ssa-dom.c (tree_ssa_dominator_optimize): Never clear
> 	bits in needed_eh_cleanup while iterating over the bitmap.
> 	Look through all forwarder blocks at once.
>
> 	* g++.dg/opt/pr55329.C: New test.
This is fine.

I've still got a todo on my list from 2009 to have checking code to 
detect when a bitmap is changing while we're iterating on it.

jeff
diff mbox

Patch

--- gcc/tree-ssa-dom.c.jj	2012-11-01 09:33:28.000000000 +0100
+++ gcc/tree-ssa-dom.c	2012-11-15 17:06:59.024545244 +0100
@@ -801,17 +801,21 @@  tree_ssa_dominator_optimize (void)
 
       /* Jump threading may have created forwarder blocks from blocks
 	 needing EH cleanup; the new successor of these blocks, which
-	 has inherited from the original block, needs the cleanup.  */
+	 has inherited from the original block, needs the cleanup.
+	 Don't clear bits in the bitmap, as that can break the bitmap
+	 iterator.  */
       EXECUTE_IF_SET_IN_BITMAP (need_eh_cleanup, 0, i, bi)
 	{
 	  basic_block bb = BASIC_BLOCK (i);
-	  if (bb
-	      && single_succ_p (bb)
-	      && (single_succ_edge (bb)->flags & EDGE_EH) == 0)
-	    {
-	      bitmap_clear_bit (need_eh_cleanup, i);
-	      bitmap_set_bit (need_eh_cleanup, single_succ (bb)->index);
-	    }
+	  if (bb == NULL)
+	    continue;
+	  while (single_succ_p (bb)
+		 && (single_succ_edge (bb)->flags & EDGE_EH) == 0)
+	    bb = single_succ (bb);
+	  if (bb == EXIT_BLOCK_PTR)
+	    continue;
+	  if ((unsigned) bb->index != i)
+	    bitmap_set_bit (need_eh_cleanup, bb->index);
 	}
 
       gimple_purge_all_dead_eh_edges (need_eh_cleanup);
--- gcc/testsuite/g++.dg/opt/pr55329.C.jj	2012-11-15 17:08:30.659899476 +0100
+++ gcc/testsuite/g++.dg/opt/pr55329.C	2012-11-15 17:09:13.529606803 +0100
@@ -0,0 +1,73 @@ 
+// PR tree-optimization/55329
+// { dg-do compile }
+// { dg-options "-O -fno-guess-branch-probability -fnon-call-exceptions --param=early-inlining-insns=111" }
+
+void *f1 ();
+void f2 (void *);
+void f3 ();
+static inline void *
+f4 ()
+{
+  void *p = f1 ();
+  if (!p)
+    f3 ();
+  return p;
+}
+
+struct A
+{
+  int *a;
+  A ();
+  ~A () { a3 (); }
+  int a1 (int * p) { if (!p) f3 (); f2 (p); }
+  int *a2 ();
+  void a3 () { if (*a) a1 (a); }
+  int a4 (int x) { if (*a) f4 (); *a2 () += x; }
+};
+
+struct B : A
+{
+  ~B () { a3 (); }
+};
+
+template <class T>
+struct C
+{
+  T *c;
+  C ();
+  int c1 () { return *(int *) f4 (); }
+  ~C () { if (c1 ()) for (T *t = c + c2 (); t != c; t--) T (); }
+  int c2 ();
+};
+
+class D
+{
+  C <C <int> > c;
+};
+
+struct E
+{
+  int *e;
+  ~E () { delete e; }
+};
+
+struct F
+{
+  int *f1 ();
+  D f2;
+  E f3;
+  F () { f4 (); }
+};
+
+struct G : F
+{
+  B g;
+  G () { g.a4 (*g1 ()->f1 ()); g1 ()->f1 (); }
+  F *g1 ();
+};
+
+void
+foo ()
+{
+  G g;
+}