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~