diff mbox

[cfgloop] PR middle-end/68375: Restructure get_loop_body_in_bfs_order to handle loops with only a header

Message ID 56543E55.7040907@arm.com
State New
Headers show

Commit Message

Kyrylo Tkachov Nov. 24, 2015, 10:39 a.m. UTC 
Hi all,

The ICE in this PR occurs when trying to dump the CFG to a .dot file.
The i > vc assert at the bottom of the loop in get_loop_body_in_bfs_order
gets triggered when i == 1 and vc == 1.  This is not a problem for that loop
because it contains only a header and it's not going to go around the loop again
to access uninitialized memory in the blocks array.

Richi suggested restructuring the loop to initialise the blocks array with the header
early on and moving the assert to the beginning of the loop.
This fix works as far as I can test.

Bootstrapped and tested on arm, aarch64, x86_64.

Ok for trunk?

Thanks,
Kyrill

2015-11-24  Richard Biener  <rguenther@suse.de>
             Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     PR middle-end/68375
     * cfgloop.c (get_loop_body_in_bfs_order): Restructure loop to avoid
     bogus assertion.

2015-11-24  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

     PR middle-end/68375
     * gcc.dg/pr68375.c: New test.

Comments

Richard Biener Nov. 24, 2015, 10:58 a.m. UTC | #1 
On Tue, Nov 24, 2015 at 11:39 AM, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
> Hi all,
>
> The ICE in this PR occurs when trying to dump the CFG to a .dot file.
> The i > vc assert at the bottom of the loop in get_loop_body_in_bfs_order
> gets triggered when i == 1 and vc == 1.  This is not a problem for that loop
> because it contains only a header and it's not going to go around the loop
> again
> to access uninitialized memory in the blocks array.
>
> Richi suggested restructuring the loop to initialise the blocks array with
> the header
> early on and moving the assert to the beginning of the loop.
> This fix works as far as I can test.
>
> Bootstrapped and tested on arm, aarch64, x86_64.
>
> Ok for trunk?

Ok.

Thanks,
Richard.

> Thanks,
> Kyrill
>
> 2015-11-24  Richard Biener  <rguenther@suse.de>
>             Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>     PR middle-end/68375
>     * cfgloop.c (get_loop_body_in_bfs_order): Restructure loop to avoid
>     bogus assertion.
>
> 2015-11-24  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>
>
>     PR middle-end/68375
>     * gcc.dg/pr68375.c: New test.
diff mbox

Patch

commit 0e5c74a36e7368e0f55196f5e0cf5d640a051e63
Author: Kyrylo Tkachov <kyrylo.tkachov@arm.com>
Date:   Wed Nov 18 16:49:21 2015 +0000

    PR middle-end/68375: Restructure get_loop_body_in_bfs_order to handle loops with only a header

diff --git a/gcc/cfgloop.c b/gcc/cfgloop.c
index bf00e0e..83a5262 100644
--- a/gcc/cfgloop.c
+++ b/gcc/cfgloop.c
@@ -913,37 +913,32 @@  get_loop_body_in_bfs_order (const struct loop *loop)
   basic_block *blocks;
   basic_block bb;
   bitmap visited;
-  unsigned int i = 0;
-  unsigned int vc = 1;
+  unsigned int i = 1;
+  unsigned int vc = 0;
 
   gcc_assert (loop->num_nodes);
   gcc_assert (loop->latch != EXIT_BLOCK_PTR_FOR_FN (cfun));
 
   blocks = XNEWVEC (basic_block, loop->num_nodes);
   visited = BITMAP_ALLOC (NULL);
-
-  bb = loop->header;
+  blocks[0] = loop->header;
+  bitmap_set_bit (visited, loop->header->index);
   while (i < loop->num_nodes)
     {
       edge e;
       edge_iterator ei;
-
-      if (bitmap_set_bit (visited, bb->index))
-	/* This basic block is now visited */
-	blocks[i++] = bb;
+      gcc_assert (i > vc);
+      bb = blocks[vc++];
 
       FOR_EACH_EDGE (e, ei, bb->succs)
 	{
 	  if (flow_bb_inside_loop_p (loop, e->dest))
 	    {
+	      /* This bb is now visited.  */
 	      if (bitmap_set_bit (visited, e->dest->index))
 		blocks[i++] = e->dest;
 	    }
 	}
-
-      gcc_assert (i > vc);
-
-      bb = blocks[vc++];
     }
 
   BITMAP_FREE (visited);
diff --git a/gcc/testsuite/gcc.dg/pr68375.c b/gcc/testsuite/gcc.dg/pr68375.c
new file mode 100644
index 0000000..bbbdd91
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr68375.c
@@ -0,0 +1,39 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-optimized-graph" } */
+
+int a, c, d, e, g, h;
+short f;
+
+int
+foo ()
+{
+}
+
+short
+fn1 (void)
+{
+  int j[2];
+  for (; e; e++)
+    if (j[0])
+      for (;;)
+	;
+  if (!g)
+    return f;
+}
+
+int
+main (void)
+{
+  for (; a < 1; a++)
+    {
+      for (c = 0; c < 2; c++)
+	{
+	  d && (f = 0);
+	  h = fn1 ();
+	}
+      __builtin_printf ("%d\n", (char) f);
+   }
+
+ return 0;
+}
+