diff mbox

Fix bb-reorder asm goto handling (PR sanitizer/81262)

Message ID 20170630173334.GP2123@tucnak
State New
Headers show

Commit Message

Jakub Jelinek June 30, 2017, 5:33 p.m. UTC
Hi!

The following testcases now ICE on the trunk.  The problem is that
fix_up_fall_thru_edges doesn't notice asm goto does have a fallthru edge
when it has 3 edges and the EDGE_FALLTHRU is only 3rd.  Fixed by using
find_fallthru_edge if we didn't find it among the first 2 edges no matter
what the branch kind is.

Another bug is that the cond_jump variable is not really cleared and thus
once it is set to something on one of the bbs, it could be used later on
completely different bb.  This got fixed by moving the vars into the scopes
where they IMHO belong.

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

2017-06-30  Jakub Jelinek  <jakub@redhat.com>

	PR sanitizer/81262
	* bb-reorder.c (fix_up_fall_thru_edges): Move variable declarations to
	the right scopes, make sure cond_jump isn't preserved between multiple
	iterations.  Search for fallthru edge whenever there are 3+ edges and
	use find_fallthru_edge for it.

	* gcc.c-torture/compile/pr81262.c: New test.
	* g++.dg/ubsan/pr81262.C: New test.


	Jakub

Comments

Richard Biener July 1, 2017, 7:03 a.m. UTC | #1
On June 30, 2017 7:33:34 PM GMT+02:00, Jakub Jelinek <jakub@redhat.com> wrote:
>Hi!
>
>The following testcases now ICE on the trunk.  The problem is that
>fix_up_fall_thru_edges doesn't notice asm goto does have a fallthru
>edge
>when it has 3 edges and the EDGE_FALLTHRU is only 3rd.  Fixed by using
>find_fallthru_edge if we didn't find it among the first 2 edges no
>matter
>what the branch kind is.
>
>Another bug is that the cond_jump variable is not really cleared and
>thus
>once it is set to something on one of the bbs, it could be used later
>on
>completely different bb.  This got fixed by moving the vars into the
>scopes
>where they IMHO belong.
>
>Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

OK.

Richard.

>2017-06-30  Jakub Jelinek  <jakub@redhat.com>
>
>	PR sanitizer/81262
>	* bb-reorder.c (fix_up_fall_thru_edges): Move variable declarations to
>	the right scopes, make sure cond_jump isn't preserved between multiple
>	iterations.  Search for fallthru edge whenever there are 3+ edges and
>	use find_fallthru_edge for it.
>
>	* gcc.c-torture/compile/pr81262.c: New test.
>	* g++.dg/ubsan/pr81262.C: New test.
>
>--- gcc/bb-reorder.c.jj	2017-06-30 09:49:32.000000000 +0200
>+++ gcc/bb-reorder.c	2017-06-30 13:31:06.709898101 +0200
>@@ -1812,18 +1812,15 @@ static void
> fix_up_fall_thru_edges (void)
> {
>   basic_block cur_bb;
>-  basic_block new_bb;
>-  edge succ1;
>-  edge succ2;
>-  edge fall_thru;
>-  edge cond_jump = NULL;
>-  bool cond_jump_crosses;
>-  int invert_worked;
>-  rtx_insn *old_jump;
>-  rtx_code_label *fall_thru_label;
> 
>   FOR_EACH_BB_FN (cur_bb, cfun)
>     {
>+      edge succ1;
>+      edge succ2;
>+      edge fall_thru = NULL;
>+      edge cond_jump = NULL;
>+      rtx_code_label *fall_thru_label;
>+
>       fall_thru = NULL;
>       if (EDGE_COUNT (cur_bb->succs) > 0)
> 	succ1 = EDGE_SUCC (cur_bb, 0);
>@@ -1849,20 +1846,8 @@ fix_up_fall_thru_edges (void)
> 	  fall_thru = succ2;
> 	  cond_jump = succ1;
> 	}
>-      else if (succ1
>-	       && (block_ends_with_call_p (cur_bb)
>-		   || can_throw_internal (BB_END (cur_bb))))
>-	{
>-	  edge e;
>-	  edge_iterator ei;
>-
>-	  FOR_EACH_EDGE (e, ei, cur_bb->succs)
>-	    if (e->flags & EDGE_FALLTHRU)
>-	      {
>-		fall_thru = e;
>-		break;
>-	      }
>-	}
>+      else if (succ2 && EDGE_COUNT (cur_bb->succs) > 2)
>+	fall_thru = find_fallthru_edge (cur_bb->succs);
> 
>    if (fall_thru && (fall_thru->dest != EXIT_BLOCK_PTR_FOR_FN (cfun)))
> 	{
>@@ -1873,9 +1858,9 @@ fix_up_fall_thru_edges (void)
> 	      /* The fall_thru edge crosses; now check the cond jump edge, if
> 		 it exists.  */
> 
>-	      cond_jump_crosses = true;
>-	      invert_worked  = 0;
>-	      old_jump = BB_END (cur_bb);
>+	      bool cond_jump_crosses = true;
>+	      int invert_worked = 0;
>+	      rtx_insn *old_jump = BB_END (cur_bb);
> 
> 	      /* Find the jump instruction, if there is one.  */
> 
>@@ -1895,12 +1880,13 @@ fix_up_fall_thru_edges (void)
> 		      /* Find label in fall_thru block. We've already added
> 			 any missing labels, so there must be one.  */
> 
>-		      fall_thru_label = block_label (fall_thru->dest);
>+		      rtx_code_label *fall_thru_label
>+			= block_label (fall_thru->dest);
> 
> 		      if (old_jump && fall_thru_label)
> 			{
>-			  rtx_jump_insn *old_jump_insn =
>-				dyn_cast <rtx_jump_insn *> (old_jump);
>+			  rtx_jump_insn *old_jump_insn
>+			    = dyn_cast <rtx_jump_insn *> (old_jump);
> 			  if (old_jump_insn)
> 			    invert_worked = invert_jump (old_jump_insn,
> 							 fall_thru_label, 0);
>@@ -1931,7 +1917,7 @@ fix_up_fall_thru_edges (void)
> 		     becomes EDGE_CROSSING.  */
> 
> 		  fall_thru->flags &= ~EDGE_CROSSING;
>-		  new_bb = force_nonfallthru (fall_thru);
>+		  basic_block new_bb = force_nonfallthru (fall_thru);
> 
> 		  if (new_bb)
> 		    {
>--- gcc/testsuite/gcc.c-torture/compile/pr81262.c.jj	2017-06-30
>13:30:06.493624559 +0200
>+++ gcc/testsuite/gcc.c-torture/compile/pr81262.c	2017-06-30
>13:30:15.000521931 +0200
>@@ -0,0 +1,14 @@
>+/* PR sanitizer/81262 */
>+
>+void bar (void) __attribute__((cold, noreturn));
>+
>+int
>+foo (void)
>+{
>+  asm goto ("" : : : : l1, l2);
>+  bar ();
>+ l1:
>+  return 1;
>+ l2:
>+  return 0;
>+}
>--- gcc/testsuite/g++.dg/ubsan/pr81262.C.jj	2017-06-30
>13:25:59.339606262 +0200
>+++ gcc/testsuite/g++.dg/ubsan/pr81262.C	2017-06-30 13:26:08.563494984
>+0200
>@@ -0,0 +1,14 @@
>+// PR sanitizer/81262
>+// { dg-do compile }
>+// { dg-options "-O2 -fsanitize=unreachable" }
>+
>+int
>+foo ()
>+{
>+  asm goto ("" : : : : l1, l2);
>+  __builtin_unreachable ();
>+ l1:
>+  return 1;
>+ l2:
>+  return 0;
>+}
>
>	Jakub
diff mbox

Patch

--- gcc/bb-reorder.c.jj	2017-06-30 09:49:32.000000000 +0200
+++ gcc/bb-reorder.c	2017-06-30 13:31:06.709898101 +0200
@@ -1812,18 +1812,15 @@  static void
 fix_up_fall_thru_edges (void)
 {
   basic_block cur_bb;
-  basic_block new_bb;
-  edge succ1;
-  edge succ2;
-  edge fall_thru;
-  edge cond_jump = NULL;
-  bool cond_jump_crosses;
-  int invert_worked;
-  rtx_insn *old_jump;
-  rtx_code_label *fall_thru_label;
 
   FOR_EACH_BB_FN (cur_bb, cfun)
     {
+      edge succ1;
+      edge succ2;
+      edge fall_thru = NULL;
+      edge cond_jump = NULL;
+      rtx_code_label *fall_thru_label;
+
       fall_thru = NULL;
       if (EDGE_COUNT (cur_bb->succs) > 0)
 	succ1 = EDGE_SUCC (cur_bb, 0);
@@ -1849,20 +1846,8 @@  fix_up_fall_thru_edges (void)
 	  fall_thru = succ2;
 	  cond_jump = succ1;
 	}
-      else if (succ1
-	       && (block_ends_with_call_p (cur_bb)
-		   || can_throw_internal (BB_END (cur_bb))))
-	{
-	  edge e;
-	  edge_iterator ei;
-
-	  FOR_EACH_EDGE (e, ei, cur_bb->succs)
-	    if (e->flags & EDGE_FALLTHRU)
-	      {
-		fall_thru = e;
-		break;
-	      }
-	}
+      else if (succ2 && EDGE_COUNT (cur_bb->succs) > 2)
+	fall_thru = find_fallthru_edge (cur_bb->succs);
 
       if (fall_thru && (fall_thru->dest != EXIT_BLOCK_PTR_FOR_FN (cfun)))
 	{
@@ -1873,9 +1858,9 @@  fix_up_fall_thru_edges (void)
 	      /* The fall_thru edge crosses; now check the cond jump edge, if
 		 it exists.  */
 
-	      cond_jump_crosses = true;
-	      invert_worked  = 0;
-	      old_jump = BB_END (cur_bb);
+	      bool cond_jump_crosses = true;
+	      int invert_worked = 0;
+	      rtx_insn *old_jump = BB_END (cur_bb);
 
 	      /* Find the jump instruction, if there is one.  */
 
@@ -1895,12 +1880,13 @@  fix_up_fall_thru_edges (void)
 		      /* Find label in fall_thru block. We've already added
 			 any missing labels, so there must be one.  */
 
-		      fall_thru_label = block_label (fall_thru->dest);
+		      rtx_code_label *fall_thru_label
+			= block_label (fall_thru->dest);
 
 		      if (old_jump && fall_thru_label)
 			{
-			  rtx_jump_insn *old_jump_insn =
-				dyn_cast <rtx_jump_insn *> (old_jump);
+			  rtx_jump_insn *old_jump_insn
+			    = dyn_cast <rtx_jump_insn *> (old_jump);
 			  if (old_jump_insn)
 			    invert_worked = invert_jump (old_jump_insn,
 							 fall_thru_label, 0);
@@ -1931,7 +1917,7 @@  fix_up_fall_thru_edges (void)
 		     becomes EDGE_CROSSING.  */
 
 		  fall_thru->flags &= ~EDGE_CROSSING;
-		  new_bb = force_nonfallthru (fall_thru);
+		  basic_block new_bb = force_nonfallthru (fall_thru);
 
 		  if (new_bb)
 		    {
--- gcc/testsuite/gcc.c-torture/compile/pr81262.c.jj	2017-06-30 13:30:06.493624559 +0200
+++ gcc/testsuite/gcc.c-torture/compile/pr81262.c	2017-06-30 13:30:15.000521931 +0200
@@ -0,0 +1,14 @@ 
+/* PR sanitizer/81262 */
+
+void bar (void) __attribute__((cold, noreturn));
+
+int
+foo (void)
+{
+  asm goto ("" : : : : l1, l2);
+  bar ();
+ l1:
+  return 1;
+ l2:
+  return 0;
+}
--- gcc/testsuite/g++.dg/ubsan/pr81262.C.jj	2017-06-30 13:25:59.339606262 +0200
+++ gcc/testsuite/g++.dg/ubsan/pr81262.C	2017-06-30 13:26:08.563494984 +0200
@@ -0,0 +1,14 @@ 
+// PR sanitizer/81262
+// { dg-do compile }
+// { dg-options "-O2 -fsanitize=unreachable" }
+
+int
+foo ()
+{
+  asm goto ("" : : : : l1, l2);
+  __builtin_unreachable ();
+ l1:
+  return 1;
+ l2:
+  return 0;
+}