From patchwork Sat Oct 15 14:21:27 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eric Botcazou X-Patchwork-Id: 119975 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 2694CB70F5 for ; Sun, 16 Oct 2011 01:25:56 +1100 (EST) Received: (qmail 5161 invoked by alias); 15 Oct 2011 14:25:52 -0000 Received: (qmail 5148 invoked by uid 22791); 15 Oct 2011 14:25:51 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from mel.act-europe.fr (HELO mel.act-europe.fr) (194.98.77.210) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Sat, 15 Oct 2011 14:25:37 +0000 Received: from localhost (localhost [127.0.0.1]) by filtered-smtp.eu.adacore.com (Postfix) with ESMTP id 0DB34CB021F; Sat, 15 Oct 2011 16:25:37 +0200 (CEST) Received: from mel.act-europe.fr ([127.0.0.1]) by localhost (smtp.eu.adacore.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id JcTnfFywVDcI; Sat, 15 Oct 2011 16:25:27 +0200 (CEST) Received: from [192.168.1.2] (bon31-9-83-155-120-49.fbx.proxad.net [83.155.120.49]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by mel.act-europe.fr (Postfix) with ESMTP id A3287CB025A; Sat, 15 Oct 2011 16:25:14 +0200 (CEST) From: Eric Botcazou To: Markus Trippelsdorf Subject: Re: resent2 [PATCH] Fix ICE in redirect_jump, at jump.c:1497 PR50496 Date: Sat, 15 Oct 2011 16:21:27 +0200 User-Agent: KMail/1.9.9 Cc: gcc-patches@gcc.gnu.org, Bernd Schmidt References: <20111014085134.GA1662@x4.trippels.de> <201110141310.09537.ebotcazou@adacore.com> <20111014115943.GB1662@x4.trippels.de> In-Reply-To: <20111014115943.GB1662@x4.trippels.de> MIME-Version: 1.0 Content-Disposition: inline Message-Id: <201110151621.27366.ebotcazou@adacore.com> Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org > PR middle-end/50496 > * cfgrtl.c (try_redirect_by_replacing_jump): Treat EXIT_BLOCK_PTR case > separately before call to redirect_jump(). Add assertion. > (patch_jump_insn): Same. This will definitely disable redirections to the exit block. Now... > diff --git a/gcc/cfgrtl.c b/gcc/cfgrtl.c > index b3f045b..57f561f 100644 > --- a/gcc/cfgrtl.c > +++ b/gcc/cfgrtl.c > @@ -846,11 +846,10 @@ try_redirect_by_replacing_jump (edge e, basic_block > target, bool in_cfglayout) if (dump_file) > fprintf (dump_file, "Redirecting jump %i from %i to %i.\n", > INSN_UID (insn), e->dest->index, target->index); > - if (!redirect_jump (insn, block_label (target), 0)) > - { > - gcc_assert (target == EXIT_BLOCK_PTR); > - return NULL; > - } > + if (target == EXIT_BLOCK_PTR) > + return NULL; > + if (! redirect_jump (insn, block_label (target), 0)) > + gcc_unreachable (); > } > > /* Cannot do anything for target exit block. */ > @@ -1030,11 +1029,10 @@ patch_jump_insn (rtx insn, rtx old_label, > basic_block new_bb) /* If the substitution doesn't succeed, die. This can > happen > if the back end emitted unrecognizable instructions or if > target is exit block on some arches. */ > - if (!redirect_jump (insn, block_label (new_bb), 0)) > - { > - gcc_assert (new_bb == EXIT_BLOCK_PTR); > - return false; > - } > + if (new_bb == EXIT_BLOCK_PTR) > + return false; > + if (! redirect_jump (insn, block_label (new_bb), 0)) > + gcc_unreachable (); > } > } > return true; ... the code is pretty clear: attempting to redirect to the exit block is a valid operation (since its potential failure is expected by the assertion). The problem is that the interface to redirect_jump/redirect_jump_1 has recently been changed: 2011-07-28 Bernd Schmidt [...] * jump.c (delete_related_insns): Likewise. (jump_to_label_p): New function. (redirect_target): New static function. (redirect_exp_1): Use it. Adjust to handle ret_rtx in JUMP_LABELS. (redirect_jump_1): Assert that the new label is nonnull. (redirect_jump): Likewise. (redirect_jump_2): Check for ANY_RETURN_P rather than NULL labels. [...] but all the callers haven't apparently been updated. The model seems to be: (dead_or_predicable): Change NEW_DEST arg to DEST_EDGE. All callers changed. Ensure that the right label is passed to redirect_jump. @@ -4134,10 +4137,16 @@ dead_or_predicable (basic_block test_bb, old_dest = JUMP_LABEL (jump); if (other_bb != new_dest) { - new_label = block_label (new_dest); + if (JUMP_P (BB_END (dest_edge->src))) + new_dest_label = JUMP_LABEL (BB_END (dest_edge->src)); + else if (new_dest == EXIT_BLOCK_PTR) + new_dest_label = ret_rtx; + else + new_dest_label = block_label (new_dest); + if (reversep - ? ! invert_jump_1 (jump, new_label) - : ! redirect_jump_1 (jump, new_label)) + ? ! invert_jump_1 (jump, new_dest_label) + : ! redirect_jump_1 (jump, new_dest_label)) goto cancel; } so the correct fix is very likely something like: Bernd, should all the callers of redirect_jump/redirect_jump_1 be audited for this pattern (there are 3 of them in cfgrtl.c for example)? Index: cfgrtl.c =================================================================== --- cfgrtl.c (revision 179844) +++ cfgrtl.c (working copy) @@ -1024,13 +1024,20 @@ patch_jump_insn (rtx insn, rtx old_label if (!currently_expanding_to_rtl || JUMP_LABEL (insn) == old_label) { + rtx new_label; + /* If the insn doesn't go where we think, we're confused. */ gcc_assert (JUMP_LABEL (insn) == old_label); + if (new_bb == EXIT_BLOCK_PTR) + new_label = ret_rtx; + else + new_label = block_label (new_bb); + /* If the substitution doesn't succeed, die. This can happen if the back end emitted unrecognizable instructions or if target is exit block on some arches. */ - if (!redirect_jump (insn, block_label (new_bb), 0)) + if (!redirect_jump (insn, new_label, 0)) { gcc_assert (new_bb == EXIT_BLOCK_PTR); return false;