This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix bb-reorder asm goto handling (PR sanitizer/81262)
- From: Richard Biener <rguenther at suse dot de>
- To: Jakub Jelinek <jakub at redhat dot com>,Jeff Law <law at redhat dot com>,Jan Hubicka <jh at suse dot cz>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Sat, 01 Jul 2017 09:03:47 +0200
- Subject: Re: [PATCH] Fix bb-reorder asm goto handling (PR sanitizer/81262)
- Authentication-results: sourceware.org; auth=none
- References: <20170630173334.GP2123@tucnak>
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