[backport from gcc-4.7/trunk r171556 ] From: Eric Botcazou Subject: Fix nasty bug in reg-stack.c Date: Sat, 26 Mar 2011 13:59:25 +0100 List-Archive: The bug was introduced 6 years ago but its occurrences apparently are quite rare since we detected it only very recently in our 4.3-based Ada compiler. The end of convert_regs reads: inserted |= compensate_edges (); clear_aux_for_blocks (); fixup_abnormal_edges (); if (inserted) commit_edge_insertions (); There is a nasty ordering problem in these lines. fixup_abnormal_edges is intended to fix up the CFG by moving insns that were inserted past the last insn in a BB before compensate_edges is run onto the fallthrough edge. And compensate_edges also inserts insns onto (fallthrough) edges. Now inserting insns on edges is a FIFO process so, if an insn was inserted past the last insn in a BB and a compensation insn is inserted afterward on the fallthrough edge, then they end up being swapped after the call to fixup_abnormal_edges. A possible fix would be to use in the earlier phases the same mechanism used in compensate_edges: inserting insns on edges directly (except for the case of a single successor). This seems inappropriate at this point of the lifetime of the reg-stack.c code. The attached fix moves the call to fixup_abnormal_edges up to before the call to compensate_edges instead; this in turn requires some changes to fixup_abnormal_edges itself, mostly the uncoupling of the fixup and the finalization codes. Tested on i586-suse-linux, applied on the mainline. gcc/ 2011-03-26 Eric Botcazou * basic-block.h (fixup_abnormal_edges): Adjust prototype. * reload1.c (reload): Adjust call to fixup_abnormal_edges. Rediscover basic blocks and call commit_edge_insertions directly. (fixup_abnormal_edges): Move from here to... * cfgrtl.c (fixup_abnormal_edges): ...here. Only insert instructions on the edges and return whether some have actually been inserted. * reg-stack.c (convert_regs): Fix up abnormal edges before inserting compensation code. --- gcc-4.6.0/gcc/basic-block.h.~1~ 2010-12-14 01:23:40.000000000 +0100 +++ gcc-4.6.0/gcc/basic-block.h 2011-05-14 15:19:56.000000000 +0200 @@ -798,6 +798,7 @@ extern basic_block force_nonfallthru (ed extern rtx block_label (basic_block); extern bool purge_all_dead_edges (void); extern bool purge_dead_edges (basic_block); +extern bool fixup_abnormal_edges (void); /* In cfgbuild.c. */ extern void find_many_sub_basic_blocks (sbitmap); @@ -814,7 +815,6 @@ extern bool delete_unreachable_blocks (v extern bool mark_dfs_back_edges (void); extern void set_edge_can_fallthru_flag (void); extern void update_br_prob_note (basic_block); -extern void fixup_abnormal_edges (void); extern bool inside_basic_block_p (const_rtx); extern bool control_flow_insn_p (const_rtx); extern rtx get_last_bb_insn (basic_block); --- gcc-4.6.0/gcc/cfgrtl.c.~1~ 2010-11-19 19:56:01.000000000 +0100 +++ gcc-4.6.0/gcc/cfgrtl.c 2011-05-14 15:19:56.000000000 +0200 @@ -30,6 +30,8 @@ along with GCC; see the file COPYING3. insert_insn_on_edge, commit_edge_insertions - CFG updating after insn simplification purge_dead_edges, purge_all_dead_edges + - CFG fixing after coarse manipulation + fixup_abnormal_edges Functions not supposed for generic use: - Infrastructure to determine quickly basic block for insn @@ -2471,6 +2473,95 @@ purge_all_dead_edges (void) return purged; } +/* This is used by a few passes that emit some instructions after abnormal + calls, moving the basic block's end, while they in fact do want to emit + them on the fallthru edge. Look for abnormal call edges, find backward + the call in the block and insert the instructions on the edge instead. + + Similarly, handle instructions throwing exceptions internally. + + Return true when instructions have been found and inserted on edges. */ + +bool +fixup_abnormal_edges (void) +{ + bool inserted = false; + basic_block bb; + + FOR_EACH_BB (bb) + { + edge e; + edge_iterator ei; + + /* Look for cases we are interested in - calls or instructions causing + exceptions. */ + FOR_EACH_EDGE (e, ei, bb->succs) + if ((e->flags & EDGE_ABNORMAL_CALL) + || ((e->flags & (EDGE_ABNORMAL | EDGE_EH)) + == (EDGE_ABNORMAL | EDGE_EH))) + break; + + if (e && !CALL_P (BB_END (bb)) && !can_throw_internal (BB_END (bb))) + { + rtx insn; + + /* Get past the new insns generated. Allow notes, as the insns + may be already deleted. */ + insn = BB_END (bb); + while ((NONJUMP_INSN_P (insn) || NOTE_P (insn)) + && !can_throw_internal (insn) + && insn != BB_HEAD (bb)) + insn = PREV_INSN (insn); + + if (CALL_P (insn) || can_throw_internal (insn)) + { + rtx stop, next; + + e = find_fallthru_edge (bb->succs); + + stop = NEXT_INSN (BB_END (bb)); + BB_END (bb) = insn; + + for (insn = NEXT_INSN (insn); insn != stop; insn = next) + { + next = NEXT_INSN (insn); + if (INSN_P (insn)) + { + delete_insn (insn); + + /* Sometimes there's still the return value USE. + If it's placed after a trapping call (i.e. that + call is the last insn anyway), we have no fallthru + edge. Simply delete this use and don't try to insert + on the non-existent edge. */ + if (GET_CODE (PATTERN (insn)) != USE) + { + /* We're not deleting it, we're moving it. */ + INSN_DELETED_P (insn) = 0; + PREV_INSN (insn) = NULL_RTX; + NEXT_INSN (insn) = NULL_RTX; + + insert_insn_on_edge (insn, e); + inserted = true; + } + } + else if (!BARRIER_P (insn)) + set_block_for_insn (insn, NULL); + } + } + + /* It may be that we don't find any trapping insn. In this + case we discovered quite late that the insn that had been + marked as can_throw_internal in fact couldn't trap at all. + So we should in fact delete the EH edges out of the block. */ + else + purge_dead_edges (bb); + } + } + + return inserted; +} + /* Same as split_block but update cfg_layout structures. */ static basic_block --- gcc-4.6.0/gcc/reg-stack.c.~1~ 2011-01-03 21:52:22.000000000 +0100 +++ gcc-4.6.0/gcc/reg-stack.c 2011-05-14 15:19:56.000000000 +0200 @@ -3150,11 +3150,14 @@ convert_regs (void) cfg_altered |= convert_regs_2 (b); } + /* We must fix up abnormal edges before inserting compensation code + because both mechanisms insert insns on edges. */ + inserted |= fixup_abnormal_edges (); + inserted |= compensate_edges (); clear_aux_for_blocks (); - fixup_abnormal_edges (); if (inserted) commit_edge_insertions (); --- gcc-4.6.0/gcc/reload1.c.~1~ 2011-01-23 22:11:24.000000000 +0100 +++ gcc-4.6.0/gcc/reload1.c 2011-05-14 15:19:56.000000000 +0200 @@ -721,6 +721,7 @@ reload (rtx first, int global) rtx insn; struct elim_table *ep; basic_block bb; + bool inserted; /* Make sure even insns with volatile mem refs are recognizable. */ init_recog (); @@ -1299,7 +1300,21 @@ reload (rtx first, int global) /* Free all the insn_chain structures at once. */ obstack_free (&reload_obstack, reload_startobj); unused_insn_chains = 0; - fixup_abnormal_edges (); + + inserted = fixup_abnormal_edges (); + + /* We've possibly turned single trapping insn into multiple ones. */ + if (cfun->can_throw_non_call_exceptions) + { + sbitmap blocks; + blocks = sbitmap_alloc (last_basic_block); + sbitmap_ones (blocks); + find_many_sub_basic_blocks (blocks); + sbitmap_free (blocks); + } + + if (inserted) + commit_edge_insertions (); /* Replacing pseudos with their memory equivalents might have created shared rtx. Subsequent passes would get confused @@ -9113,112 +9128,3 @@ add_auto_inc_notes (rtx insn, rtx x) } } #endif - -/* This is used by reload pass, that does emit some instructions after - abnormal calls moving basic block end, but in fact it wants to emit - them on the edge. Looks for abnormal call edges, find backward the - proper call and fix the damage. - - Similar handle instructions throwing exceptions internally. */ -void -fixup_abnormal_edges (void) -{ - bool inserted = false; - basic_block bb; - - FOR_EACH_BB (bb) - { - edge e; - edge_iterator ei; - - /* Look for cases we are interested in - calls or instructions causing - exceptions. */ - FOR_EACH_EDGE (e, ei, bb->succs) - { - if (e->flags & EDGE_ABNORMAL_CALL) - break; - if ((e->flags & (EDGE_ABNORMAL | EDGE_EH)) - == (EDGE_ABNORMAL | EDGE_EH)) - break; - } - if (e && !CALL_P (BB_END (bb)) - && !can_throw_internal (BB_END (bb))) - { - rtx insn; - - /* Get past the new insns generated. Allow notes, as the insns - may be already deleted. */ - insn = BB_END (bb); - while ((NONJUMP_INSN_P (insn) || NOTE_P (insn)) - && !can_throw_internal (insn) - && insn != BB_HEAD (bb)) - insn = PREV_INSN (insn); - - if (CALL_P (insn) || can_throw_internal (insn)) - { - rtx stop, next; - - stop = NEXT_INSN (BB_END (bb)); - BB_END (bb) = insn; - insn = NEXT_INSN (insn); - - e = find_fallthru_edge (bb->succs); - - while (insn && insn != stop) - { - next = NEXT_INSN (insn); - if (INSN_P (insn)) - { - delete_insn (insn); - - /* Sometimes there's still the return value USE. - If it's placed after a trapping call (i.e. that - call is the last insn anyway), we have no fallthru - edge. Simply delete this use and don't try to insert - on the non-existent edge. */ - if (GET_CODE (PATTERN (insn)) != USE) - { - /* We're not deleting it, we're moving it. */ - INSN_DELETED_P (insn) = 0; - PREV_INSN (insn) = NULL_RTX; - NEXT_INSN (insn) = NULL_RTX; - - insert_insn_on_edge (insn, e); - inserted = true; - } - } - else if (!BARRIER_P (insn)) - set_block_for_insn (insn, NULL); - insn = next; - } - } - - /* It may be that we don't find any such trapping insn. In this - case we discovered quite late that the insn that had been - marked as can_throw_internal in fact couldn't trap at all. - So we should in fact delete the EH edges out of the block. */ - else - purge_dead_edges (bb); - } - } - - /* We've possibly turned single trapping insn into multiple ones. */ - if (cfun->can_throw_non_call_exceptions) - { - sbitmap blocks; - blocks = sbitmap_alloc (last_basic_block); - sbitmap_ones (blocks); - find_many_sub_basic_blocks (blocks); - sbitmap_free (blocks); - } - - if (inserted) - commit_edge_insertions (); - -#ifdef ENABLE_CHECKING - /* Verify that we didn't turn one trapping insn into many, and that - we found and corrected all of the problems wrt fixups on the - fallthru edge. */ - verify_flow_info (); -#endif -}