public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] Fix bb-reorder asm goto handling (PR sanitizer/81262)
@ 2017-06-30 17:33 Jakub Jelinek
  2017-07-01  7:03 ` Richard Biener
  0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2017-06-30 17:33 UTC (permalink / raw)
  To: Richard Biener, Jeff Law, Jan Hubicka; +Cc: gcc-patches

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.

--- 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

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] Fix bb-reorder asm goto handling (PR sanitizer/81262)
  2017-06-30 17:33 [PATCH] Fix bb-reorder asm goto handling (PR sanitizer/81262) Jakub Jelinek
@ 2017-07-01  7:03 ` Richard Biener
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2017-07-01  7:03 UTC (permalink / raw)
  To: Jakub Jelinek, Jeff Law, Jan Hubicka; +Cc: gcc-patches

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2017-07-01  7:03 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-30 17:33 [PATCH] Fix bb-reorder asm goto handling (PR sanitizer/81262) Jakub Jelinek
2017-07-01  7:03 ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).