Re: [PATCH]: Fix use of __builtin_eh_pointer in EH_ELSE

[Richard Henderson <rth at redhat dot com>]

On 09/03/2013 07:08 AM, Tristan Gingold wrote:
> Hi,
> 
> The field state->ehp_region wasn't updated before lowering constructs in the eh
> path of EH_ELSE.  As a consequence, __builtin_eh_pointer is lowered to 0 (or
> possibly to a wrong region number) in this path.
> 
> The only user of EH_ELSE looks to be trans-mem.c:lower_transaction, and the
> consequence of that is a memory leak.
> 
> Furthermore, according to calls.c:flags_from_decl_or_type, the "transaction_pure"
> attribute must be set on the function type, not on the function declaration.
> Hence the change to declare __builtin_eh_pointer.
> (I don't understand the guard condition to set the attribute for it in tree.c.
>  Why is 'builtin_decl_explicit_p (BUILT_IN_TM_LOAD_1)' needed in addition to
>  flag_tm ?)

Clearly these are totally unrelated and should not be in the same patch.

The BUILT_IN_TM_LOAD_1 thing looks like it might have something to do with
non-C front ends, which don't all create the builtins.

> diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c
> index 6ffbd26..86ee77e 100644
> --- a/gcc/tree-eh.c
> +++ b/gcc/tree-eh.c
> @@ -1093,8 +1093,12 @@ lower_try_finally_nofallthru (struct leh_state *state,
>  
>        if (tf->may_throw)
>  	{
> +	  eh_region prev_ehp_region = state->ehp_region;
> +
>  	  finally = gimple_eh_else_e_body (eh_else);
> +	  state->ehp_region = tf->region;
>  	  lower_eh_constructs_1 (state, &finally);
> +	  state->ehp_region = prev_ehp_region;

This doesn't look right.

Does it work to save and restore ehp_region in the two places that
we currently set it instead?


r~