| Message ID | 3126157.y62fDbuCNQ@polaris |
|---|---|
| State | New |
| Headers | show |
On Thu, Nov 6, 2014 at 2:58 PM, Eric Botcazou <ebotcazou@adacore.com> wrote: > Hi, > > the compiler started to segfault on this ACATS test very recently, probably > when the large Pointer Bounds Checker patch went in, although the issue had > been latent. For the attached reduced testcase, the backtrace is: > > Starting program: /home/eric/build/gcc/native/stage1-gcc/gnat1 -quiet > opt43.adb -O > opt43.adb:8:05: warning: "return" statement missing following this statement > opt43.adb:8:05: warning: Program_Error may be raised at run time > > Program received signal SIGSEGV, Segmentation fault. > stmt_could_throw_1_p (stmt=0x7ffff6d2e460) > at /home/eric/svn/gcc/gcc/tree-eh.c:2722 > warning: Source file is more recent than executable. > 2722 fp_operation = FLOAT_TYPE_P (t); > (gdb) bt > #0 stmt_could_throw_1_p (stmt=0x7ffff6d2e460) > at /home/eric/svn/gcc/gcc/tree-eh.c:2722 > #1 stmt_could_throw_p (stmt=0x7ffff6d2e460) > at /home/eric/svn/gcc/gcc/tree-eh.c:2772 > #2 0x0000000000ec81ad in maybe_clean_eh_stmt_fn (ifun=0x7ffff69b45e8, > stmt=0x7ffff6d2e460) at /home/eric/svn/gcc/gcc/tree-eh.c:2846 > #3 0x0000000000ec81e7 in maybe_clean_eh_stmt (stmt=<optimized out>) > at /home/eric/svn/gcc/gcc/tree-eh.c:2856 > #4 0x0000000000ea9160 in execute_fixup_cfg () > > The problem is in fixup_noreturn_call; it contains this block of code: > > /* We need to remove SSA name to avoid checking errors. > All uses are dominated by the noreturn and thus will > be removed afterwards. > We proactively remove affected non-PHI statements to avoid > fixup_cfg from trying to update them and crashing. */ > if (TREE_CODE (op) == SSA_NAME) > { > use_operand_p use_p; > imm_use_iterator iter; > gimple use_stmt; > bitmap_iterator bi; > unsigned int bb_index; > > bitmap blocks = BITMAP_ALLOC (NULL); > > FOR_EACH_IMM_USE_STMT (use_stmt, iter, op) > { > if (gimple_code (use_stmt) != GIMPLE_PHI) > bitmap_set_bit (blocks, gimple_bb (use_stmt)->index); > else > FOR_EACH_IMM_USE_ON_STMT (use_p, iter) > SET_USE (use_p, error_mark_node); > } > EXECUTE_IF_SET_IN_BITMAP (blocks, 0, bb_index, bi) > delete_basic_block (BASIC_BLOCK_FOR_FN (cfun, bb_index)); > BITMAP_FREE (blocks); > release_ssa_name (op); > } > > The .ssa dump of the affected function starts with: > > ;; Function Opt43 (_ada_opt43, funcdef_no=1, decl_uid=4096, cgraph_uid=1, > symbol_order=1) > > Opt43 () > { > void * EXPTR; > integer _5; > integer _6; > integer _8; > integer _9; > > <bb 2>: > _5 = opt43.func (); > > <bb 3>: > _6 = _5; > _8 = opt43.func (); > > <bb 4>: > _9 = _8; > if (_6 == _9) > goto <bb 5>; > else > goto <bb 6>; > > When the above code processes the first call to opt43.func, it frees _5 and > deletes bb 3, which in turn frees _6. But _6 is still referenced in the > comparison statement in bb 4 and stmt_could_throw_1_p chokes because of that. > The proposed fix is to avoid doing such a poor DCE in fixup_noreturn_call. > > Tested on x86_64-suse-linux, OK for the mainline? Can you instead of initializing it with 0, turn it into a default definition? Thus do sth like SSA_NAME_VAR (lhs) = create_tmp_reg (TREE_TYPE (lhs), NULL); set_ssa_default_def (cfun, SSA_NAME_VAR (lhs), lhs); Ok with that change. Thanks, Richard. > > 2014-11-06 Eric Botcazou <ebotcazou@adacore.com> > > * tree-cfgcleanup. (fixup_noreturn_call): Do not perform DCE here. > > > 2014-11-06 Eric Botcazou <ebotcazou@adacore.com> > > * gnat.dg/opt43.adb: New test. > > > -- > Eric Botcazou
> Can you instead of initializing it with 0, turn it into a default > definition? > > Thus do sth like > > SSA_NAME_VAR (lhs) = create_tmp_reg (TREE_TYPE (lhs), NULL); > set_ssa_default_def (cfun, SSA_NAME_VAR (lhs), lhs); This occurred to me, but I didn't feel comfortable about fiddling with the SSA_NAME_VAR of LHS. Now this appears to work fine, so I'll install the modified version if a full testing cycle successfully finishes. > Ok with that change. Thanks.
Index: tree-cfgcleanup.c =================================================================== --- tree-cfgcleanup.c (revision 217148) +++ tree-cfgcleanup.c (working copy) @@ -568,7 +568,6 @@ bool fixup_noreturn_call (gimple stmt) { basic_block bb = gimple_bb (stmt); - bool changed = false; if (gimple_call_builtin_p (stmt, BUILT_IN_RETURN)) return false; @@ -590,46 +589,29 @@ fixup_noreturn_call (gimple stmt) split_block (bb, stmt); } - changed |= remove_fallthru_edge (bb->succs); - - /* If there is LHS, remove it. */ - if (gimple_call_lhs (stmt)) + /* If there is an LHS, remove it. */ + tree lhs = gimple_call_lhs (stmt); + if (lhs) { - tree op = gimple_call_lhs (stmt); gimple_call_set_lhs (stmt, NULL_TREE); - /* We need to remove SSA name to avoid checking errors. - All uses are dominated by the noreturn and thus will - be removed afterwards. - We proactively remove affected non-PHI statements to avoid - fixup_cfg from trying to update them and crashing. */ - if (TREE_CODE (op) == SSA_NAME) + /* We need to initialize the SSA name to avoid checking errors. */ + if (TREE_CODE (lhs) == SSA_NAME) { - use_operand_p use_p; - imm_use_iterator iter; - gimple use_stmt; - bitmap_iterator bi; - unsigned int bb_index; - - bitmap blocks = BITMAP_ALLOC (NULL); - - FOR_EACH_IMM_USE_STMT (use_stmt, iter, op) + edge e = find_fallthru_edge (bb->succs); + if (e) { - if (gimple_code (use_stmt) != GIMPLE_PHI) - bitmap_set_bit (blocks, gimple_bb (use_stmt)->index); - else - FOR_EACH_IMM_USE_ON_STMT (use_p, iter) - SET_USE (use_p, error_mark_node); + tree zero = build_zero_cst (TREE_TYPE (lhs)); + gimple init_stmt = gimple_build_assign (lhs, zero); + gsi_insert_on_edge (e, init_stmt); + gsi_commit_one_edge_insert (e, NULL); } - EXECUTE_IF_SET_IN_BITMAP (blocks, 0, bb_index, bi) - delete_basic_block (BASIC_BLOCK_FOR_FN (cfun, bb_index)); - BITMAP_FREE (blocks); - release_ssa_name (op); } + update_stmt (stmt); - changed = true; } - return changed; + + return remove_fallthru_edge (bb->succs); }
Hi, the compiler started to segfault on this ACATS test very recently, probably when the large Pointer Bounds Checker patch went in, although the issue had been latent. For the attached reduced testcase, the backtrace is: Starting program: /home/eric/build/gcc/native/stage1-gcc/gnat1 -quiet opt43.adb -O opt43.adb:8:05: warning: "return" statement missing following this statement opt43.adb:8:05: warning: Program_Error may be raised at run time Program received signal SIGSEGV, Segmentation fault. stmt_could_throw_1_p (stmt=0x7ffff6d2e460) at /home/eric/svn/gcc/gcc/tree-eh.c:2722 warning: Source file is more recent than executable. 2722 fp_operation = FLOAT_TYPE_P (t); (gdb) bt #0 stmt_could_throw_1_p (stmt=0x7ffff6d2e460) at /home/eric/svn/gcc/gcc/tree-eh.c:2722 #1 stmt_could_throw_p (stmt=0x7ffff6d2e460) at /home/eric/svn/gcc/gcc/tree-eh.c:2772 #2 0x0000000000ec81ad in maybe_clean_eh_stmt_fn (ifun=0x7ffff69b45e8, stmt=0x7ffff6d2e460) at /home/eric/svn/gcc/gcc/tree-eh.c:2846 #3 0x0000000000ec81e7 in maybe_clean_eh_stmt (stmt=<optimized out>) at /home/eric/svn/gcc/gcc/tree-eh.c:2856 #4 0x0000000000ea9160 in execute_fixup_cfg () The problem is in fixup_noreturn_call; it contains this block of code: /* We need to remove SSA name to avoid checking errors. All uses are dominated by the noreturn and thus will be removed afterwards. We proactively remove affected non-PHI statements to avoid fixup_cfg from trying to update them and crashing. */ if (TREE_CODE (op) == SSA_NAME) { use_operand_p use_p; imm_use_iterator iter; gimple use_stmt; bitmap_iterator bi; unsigned int bb_index; bitmap blocks = BITMAP_ALLOC (NULL); FOR_EACH_IMM_USE_STMT (use_stmt, iter, op) { if (gimple_code (use_stmt) != GIMPLE_PHI) bitmap_set_bit (blocks, gimple_bb (use_stmt)->index); else FOR_EACH_IMM_USE_ON_STMT (use_p, iter) SET_USE (use_p, error_mark_node); } EXECUTE_IF_SET_IN_BITMAP (blocks, 0, bb_index, bi) delete_basic_block (BASIC_BLOCK_FOR_FN (cfun, bb_index)); BITMAP_FREE (blocks); release_ssa_name (op); } The .ssa dump of the affected function starts with: ;; Function Opt43 (_ada_opt43, funcdef_no=1, decl_uid=4096, cgraph_uid=1, symbol_order=1) Opt43 () { void * EXPTR; integer _5; integer _6; integer _8; integer _9; <bb 2>: _5 = opt43.func (); <bb 3>: _6 = _5; _8 = opt43.func (); <bb 4>: _9 = _8; if (_6 == _9) goto <bb 5>; else goto <bb 6>; When the above code processes the first call to opt43.func, it frees _5 and deletes bb 3, which in turn frees _6. But _6 is still referenced in the comparison statement in bb 4 and stmt_could_throw_1_p chokes because of that. The proposed fix is to avoid doing such a poor DCE in fixup_noreturn_call. Tested on x86_64-suse-linux, OK for the mainline? 2014-11-06 Eric Botcazou <ebotcazou@adacore.com> * tree-cfgcleanup. (fixup_noreturn_call): Do not perform DCE here. 2014-11-06 Eric Botcazou <ebotcazou@adacore.com> * gnat.dg/opt43.adb: New test.