Message ID | 20121018125839.GY584@tucnak.redhat.com |
---|---|
State | New |
Headers | show |
On Thu, Oct 18, 2012 at 5:58 AM, Jakub Jelinek <jakub@redhat.com> wrote: > Hi! > > Dodji reported to me an ICE about NOTE_INSN_BASIC_BLOCK in a middle > of a bb. The following patch (cfgexpand.c hunk) fixes that. > I then run asan cc1/cc1plus with -O2 -fasan on a portion of cc1files > and got other ICEs, which is the reason for the two asan.c changes > - one was that base wasn't unshared, so we could hit tree sharing > verification failures, and the other issue is that cgraph_build_static_cdtor > can call ggc_collect, so holding trees around in automatic variables > across it doesn't work very well. I'd have to build too many trees > again, so I think easiest is to add a new GTY root. > > Ok for asan? > > 2012-10-18 Jakub Jelinek <jakub@redhat.com> > > * asan.c (build_check_stmt): Unshare base. > > * asan.c (asan_ctor_statements): New variable. > (asan_finish_file): Use asan_ctor_statements instead > of ctor_statements. > > * cfgexpand.c (gimple_expand_cfg): If return_label is > followed by NOTE_INSN_BASIC_BLOCK, emit var_ret_seq > after the note instead of before it. > > --- gcc/asan.c.jj 2012-10-17 20:43:00.000000000 +0200 > +++ gcc/asan.c 2012-10-18 14:20:23.837423900 +0200 > @@ -459,6 +459,8 @@ build_check_stmt (tree base, > set_immediate_dominator (CDI_DOMINATORS, else_bb, cond_bb); > } > > + base = unshare_expr (base); > + > gsi = gsi_last_bb (cond_bb); > g = gimple_build_assign_with_ops (TREE_CODE (base), > make_ssa_name (TREE_TYPE (base), NULL), > @@ -748,6 +750,10 @@ asan_add_global (tree decl, tree type, V > CONSTRUCTOR_APPEND_ELT (v, NULL_TREE, init); > } > > +/* Needs to be GTY(()), because cgraph_build_static_cdtor may > + invoke ggc_collect. */ > +static GTY(()) tree asan_ctor_statements; > + > /* Module-level instrumentation. > - Insert __asan_init() into the list of CTORs. > - TODO: insert redzones around globals. > @@ -756,12 +762,11 @@ asan_add_global (tree decl, tree type, V > void > asan_finish_file (void) > { > - tree ctor_statements = NULL_TREE; > struct varpool_node *vnode; > unsigned HOST_WIDE_INT gcount = 0; > > append_to_statement_list (build_call_expr (asan_init_func (), 0), > - &ctor_statements); > + &asan_ctor_statements); > FOR_EACH_DEFINED_VARIABLE (vnode) > if (asan_protect_global (vnode->symbol.decl)) > ++gcount; > @@ -799,7 +805,7 @@ asan_finish_file (void) > append_to_statement_list (build_call_expr (decl, 2, > build_fold_addr_expr (var), > build_int_cst (uptr, gcount)), > - &ctor_statements); > + &asan_ctor_statements); > > decl = build_fn_decl ("__asan_unregister_globals", type); > TREE_NOTHROW (decl) = 1; > @@ -810,7 +816,7 @@ asan_finish_file (void) > cgraph_build_static_cdtor ('D', dtor_statements, > MAX_RESERVED_INIT_PRIORITY - 1); > } > - cgraph_build_static_cdtor ('I', ctor_statements, > + cgraph_build_static_cdtor ('I', asan_ctor_statements, > MAX_RESERVED_INIT_PRIORITY - 1); > } > > --- gcc/cfgexpand.c.jj 2012-10-17 20:41:02.000000000 +0200 > +++ gcc/cfgexpand.c 2012-10-18 13:01:06.084479343 +0200 > @@ -4615,7 +4615,12 @@ gimple_expand_cfg (void) > insn_locations_finalize (); > > if (var_ret_seq) > - emit_insn_after (var_ret_seq, return_label); > + { > + rtx after = return_label; > + if (NEXT_INSN (after) && NOTE_INSN_BASIC_BLOCK_P (NEXT_INSN (after))) > + after = NEXT_INSN (after); Just a nit: NEXT_INSN (after) can be commoned. Other than that, looks fine to me. thanks, David > + emit_insn_after (var_ret_seq, after); > + } > > /* Zap the tree EH table. */ > set_eh_throw_stmt_table (cfun, NULL); > > Jakub
--- gcc/asan.c.jj 2012-10-17 20:43:00.000000000 +0200 +++ gcc/asan.c 2012-10-18 14:20:23.837423900 +0200 @@ -459,6 +459,8 @@ build_check_stmt (tree base, set_immediate_dominator (CDI_DOMINATORS, else_bb, cond_bb); } + base = unshare_expr (base); + gsi = gsi_last_bb (cond_bb); g = gimple_build_assign_with_ops (TREE_CODE (base), make_ssa_name (TREE_TYPE (base), NULL), @@ -748,6 +750,10 @@ asan_add_global (tree decl, tree type, V CONSTRUCTOR_APPEND_ELT (v, NULL_TREE, init); } +/* Needs to be GTY(()), because cgraph_build_static_cdtor may + invoke ggc_collect. */ +static GTY(()) tree asan_ctor_statements; + /* Module-level instrumentation. - Insert __asan_init() into the list of CTORs. - TODO: insert redzones around globals. @@ -756,12 +762,11 @@ asan_add_global (tree decl, tree type, V void asan_finish_file (void) { - tree ctor_statements = NULL_TREE; struct varpool_node *vnode; unsigned HOST_WIDE_INT gcount = 0; append_to_statement_list (build_call_expr (asan_init_func (), 0), - &ctor_statements); + &asan_ctor_statements); FOR_EACH_DEFINED_VARIABLE (vnode) if (asan_protect_global (vnode->symbol.decl)) ++gcount; @@ -799,7 +805,7 @@ asan_finish_file (void) append_to_statement_list (build_call_expr (decl, 2, build_fold_addr_expr (var), build_int_cst (uptr, gcount)), - &ctor_statements); + &asan_ctor_statements); decl = build_fn_decl ("__asan_unregister_globals", type); TREE_NOTHROW (decl) = 1; @@ -810,7 +816,7 @@ asan_finish_file (void) cgraph_build_static_cdtor ('D', dtor_statements, MAX_RESERVED_INIT_PRIORITY - 1); } - cgraph_build_static_cdtor ('I', ctor_statements, + cgraph_build_static_cdtor ('I', asan_ctor_statements, MAX_RESERVED_INIT_PRIORITY - 1); } --- gcc/cfgexpand.c.jj 2012-10-17 20:41:02.000000000 +0200 +++ gcc/cfgexpand.c 2012-10-18 13:01:06.084479343 +0200 @@ -4615,7 +4615,12 @@ gimple_expand_cfg (void) insn_locations_finalize (); if (var_ret_seq) - emit_insn_after (var_ret_seq, return_label); + { + rtx after = return_label; + if (NEXT_INSN (after) && NOTE_INSN_BASIC_BLOCK_P (NEXT_INSN (after))) + after = NEXT_INSN (after); + emit_insn_after (var_ret_seq, after); + } /* Zap the tree EH table. */ set_eh_throw_stmt_table (cfun, NULL);