Re: [patch] Fix bug with noreturn call

[Jan Hubicka <hubicka at ucw dot cz>]

> > Overall I think the whole concept of saving statement pointers into on-side
> > array, not clearing them and expecting statements to be sane is very
> > fragile. There are many reasons why the statement can be removed,
> > cgraph_redirect_edge_call_stmt_to_callee is just one of them. Taking this
> > approach we would have to clear BB pointer everywhere (that can probably be
> > done within gsi_replace).
> 
> I didn't invent it, copy_bb does exactly the same thing:
> 
> 	      /* Copy all GIMPLE_CALL flags, location and block, except
> 		 GF_CALL_VA_ARG_PACK.  */
> 	      gimple_call_copy_flags (new_call, stmt);
> 	      gimple_call_set_va_arg_pack (new_call, false);
> 	      gimple_set_location (new_call, gimple_location (stmt));
> 	      gimple_set_block (new_call, gimple_block (stmt));
> 	      gimple_call_set_lhs (new_call, gimple_call_lhs (stmt));
> 
> 	      gsi_replace (&copy_gsi, new_call, false);
> 	      gimple_set_bb (stmt, NULL);
> 	      stmt = new_call;
> 
> without even explaining why. :-)

Yep, I noticed too.  gsi_remove is setting BB to NULL and BB remove use it, 
so I guess gsi_replace is one of relatively few ways the statement can get out of
instruction stream without its BB pointer being cleared.

It might be better to just make gsi_replace to clear BB and test that it works
than visiting all the 30 uses of gsi_replace in compiler to verify that they can't
leak the noreturn call?

Honza
> 
> > This is fine with me for both mainline and branch without the BB clear and
> > if we don't find better way to track changes, with the BB clear too.
> 
> Thanks, but I don't plan to work on the better way in the near future and I 
> don't think it would be appropriate for the branch anyway.
> 
> -- 
> Eric Botcazou