Re: [PATCH] Fix must-not-throw regions with SJLJ exception handling

[Richard Guenther <richard dot guenther at gmail dot com>]

On Thu, Jan 20, 2011 at 8:27 PM, Ulrich Weigand <uweigand@de.ibm.com> wrote:
> Hello,
>
> a couple of exception related tests are failing on SPU:
> FAIL: g++.dg/cpp0x/noexcept04.C execution test
> FAIL: g++.dg/cpp0x/noexcept05.C scan-assembler LSDA
> FAIL: g++.dg/eh/spec10.C execution test
> FAIL: g++.dg/eh/spec11.C scan-assembler LSDA
>
> These are related to must-not-throw regions. ?To implement those,
> we may have to register a personality function with empty LSDA.
> This has been implemented for DWARF CFI exception handling, but
> apparently not (fully?) for SJLJ exception handling.
>
> As far as I can see, there's two problems:
>
> - In sjlj_assign_call_site_values, the code sets the crtl->uses_eh_lsda
> ?to indicate an LSDA is needed. ?However this done by looping over all
> ?*landing pads* in the function -- but must-not-throw regions do not
> ?in fact have a landing pad, so they will be overlooked here.
>
> ?To fix this, the patch below moves setting crtl->uses_eh_lsda into the
> ?sjlj_mark_call_sites routine, which loops over all *insns*, just like
> ?convert_to_eh_region_ranges does for DWARF CFI.
>
> - Even so, if there are no landing pads at all, sjlj_mark_call_sites is
> ?not even called (and no code to install the SJLJ handler is emitted).
> ?But there can still be must-not-throw regions ...
>
> ?The patch below adds an extra check, if there are no landing pads,
> ?whether we still have must-not-throw regions (again by scanning the
> ?insns). ?If so, it generates code to register a simplified version
> ?of the SJLJ data structure, which only contains the personality
> ?function and an (empty) LSDA, but no actual setjmp landing pad.
>
> This seems to fix the problem for me; the tests above now pass.
>
> Tested on spu-elf with no regressions.
>
> OK for mainline?

Is there any primary or secondary target that uses SJLJ exceptions?
If so please make sure we do not regress with your patch there.

Otherwise RTH should probably look at this patch.

Thanks,
Richard.

> Bye,
> Ulrich
>
>
> ChangeLog:
>
> ? ? ? ?* except.c (sjlj_assign_call_site_values): Move setting the
> ? ? ? ?crtl->uses_eh_lsda flag to ...
> ? ? ? ?(sjlj_mark_call_sites): ... here.
> ? ? ? ?(sjlj_have_must_not_throw): New function.
> ? ? ? ?(sjlj_emit_function_enter): Support NULL dispatch label.
> ? ? ? ?(sjlj_build_landing_pads): In a function with no landing pads
> ? ? ? ?that still has must-not-throw regions, generate code to register
> ? ? ? ?a personality function with empty LSDA.
>
>
> Index: gcc/except.c
> ===================================================================
> *** gcc/except.c ? ? ? ?(revision 168909)
> --- gcc/except.c ? ? ? ?(working copy)
> *************** sjlj_assign_call_site_values (void)
> *** 998,1005 ****
>
> ? ? ? ?/* First: build the action table. ?*/
> ? ? ? ?action = collect_one_action_chain (ar_hash, lp->region);
> - ? ? ? if (action != -1)
> - ? ? ? ? crtl->uses_eh_lsda = 1;
>
> ? ? ? ?/* Next: assign call-site values. ?If dwarf2 terms, this would be
> ? ? ? ? ? the region number assigned by convert_to_eh_region_ranges, but
> --- 998,1003 ----
> *************** sjlj_mark_call_sites (void)
> *** 1065,1070 ****
> --- 1063,1071 ----
> ? ? ? ? ?this_call_site = 0;
> ? ? ? ?}
>
> + ? ? ? if (this_call_site != -1)
> + ? ? ? crtl->uses_eh_lsda = 1;
> +
> ? ? ? ?if (this_call_site == last_call_site)
> ? ? ? ?continue;
>
> *************** sjlj_mark_call_sites (void)
> *** 1085,1090 ****
> --- 1086,1121 ----
> ? ? ?}
> ?}
>
> + /* Check whether any must-not-throw regions exist. ?*/
> +
> + static bool
> + sjlj_have_must_not_throw (void)
> + {
> + ? rtx insn;
> +
> + ? for (insn = get_insns (); insn ; insn = NEXT_INSN (insn))
> + ? ? {
> + ? ? ? eh_landing_pad lp;
> + ? ? ? eh_region r;
> + ? ? ? bool nothrow;
> +
> + ? ? ? if (! INSN_P (insn))
> + ? ? ? continue;
> +
> + ? ? ? nothrow = get_eh_region_and_lp_from_rtx (insn, &r, &lp);
> + ? ? ? if (nothrow)
> + ? ? ? continue;
> +
> + ? ? ? if (r && r->type == ERT_MUST_NOT_THROW)
> + ? ? ? {
> + ? ? ? ? crtl->uses_eh_lsda = 1;
> + ? ? ? ? return true;
> + ? ? ? }
> + ? ? }
> +
> + ? return false;
> + }
> +
> ?/* Construct the SjLj_Function_Context. ?*/
>
> ?static void
> *************** sjlj_emit_function_enter (rtx dispatch_l
> *** 1119,1145 ****
> ? ?else
> ? ? ?emit_move_insn (mem, const0_rtx);
>
> ?#ifdef DONT_USE_BUILTIN_SETJMP
> ! ? {
> ! ? ? rtx x, last;
> ! ? ? x = emit_library_call_value (setjmp_libfunc, NULL_RTX, LCT_RETURNS_TWICE,
> ! ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?TYPE_MODE (integer_type_node), 1,
> ! ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?plus_constant (XEXP (fc, 0),
> ! ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? sjlj_fc_jbuf_ofs), Pmode);
> !
> ! ? ? emit_cmp_and_jump_insns (x, const0_rtx, NE, 0,
> ! ? ? ? ? ? ? ? ? ? ? ? ? ? ?TYPE_MODE (integer_type_node), 0, dispatch_label);
> ! ? ? last = get_last_insn ();
> ! ? ? if (JUMP_P (last) && any_condjump_p (last))
> ! ? ? ? {
> ! ? ? ? ? gcc_assert (!find_reg_note (last, REG_BR_PROB, 0));
> ! ? ? ? ? add_reg_note (last, REG_BR_PROB, GEN_INT (REG_BR_PROB_BASE / 100));
> ! ? ? ? }
> ! ? }
> ! #else
> ! ? expand_builtin_setjmp_setup (plus_constant (XEXP (fc, 0), sjlj_fc_jbuf_ofs),
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? dispatch_label);
> ?#endif
>
> ? ?emit_library_call (unwind_sjlj_register_libfunc, LCT_NORMAL, VOIDmode,
> ? ? ? ? ? ? ? ? ? ? 1, XEXP (fc, 0), Pmode);
> --- 1150,1179 ----
> ? ?else
> ? ? ?emit_move_insn (mem, const0_rtx);
>
> + ? if (dispatch_label)
> + ? ? {
> ?#ifdef DONT_USE_BUILTIN_SETJMP
> ! ? ? ? rtx x, last;
> ! ? ? ? x = emit_library_call_value (setjmp_libfunc, NULL_RTX, LCT_RETURNS_TWICE,
> ! ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?TYPE_MODE (integer_type_node), 1,
> ! ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?plus_constant (XEXP (fc, 0),
> ! ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? sjlj_fc_jbuf_ofs), Pmode);
> !
> ! ? ? ? emit_cmp_and_jump_insns (x, const0_rtx, NE, 0,
> ! ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?TYPE_MODE (integer_type_node), 0,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? dispatch_label);
> + ? ? ? last = get_last_insn ();
> + ? ? ? if (JUMP_P (last) && any_condjump_p (last))
> + ? ? ? {
> + ? ? ? ? gcc_assert (!find_reg_note (last, REG_BR_PROB, 0));
> + ? ? ? ? add_reg_note (last, REG_BR_PROB, GEN_INT (REG_BR_PROB_BASE / 100));
> + ? ? ? }
> + #else
> + ? ? ? expand_builtin_setjmp_setup (plus_constant (XEXP (fc, 0),
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? sjlj_fc_jbuf_ofs),
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?dispatch_label);
> ?#endif
> + ? ? }
>
> ? ?emit_library_call (unwind_sjlj_register_libfunc, LCT_NORMAL, VOIDmode,
> ? ? ? ? ? ? ? ? ? ? 1, XEXP (fc, 0), Pmode);
> *************** sjlj_build_landing_pads (void)
> *** 1363,1368 ****
> --- 1397,1418 ----
> ? ? ? ?sjlj_emit_function_exit ();
> ? ? ?}
>
> + ? /* If we do not have any landing pads, we may still need to register a
> + ? ? ?personality routine and (empty) LSDA to handle must-not-throw regions. ?*/
> + ? else if (sjlj_have_must_not_throw ())
> + ? ? {
> + ? ? ? int align = STACK_SLOT_ALIGNMENT (sjlj_fc_type_node,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? TYPE_MODE (sjlj_fc_type_node),
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? TYPE_ALIGN (sjlj_fc_type_node));
> + ? ? ? crtl->eh.sjlj_fc
> + ? ? ? = assign_stack_local (TYPE_MODE (sjlj_fc_type_node),
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? int_size_in_bytes (sjlj_fc_type_node),
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? align);
> +
> + ? ? ? sjlj_emit_function_enter (NULL_RTX);
> + ? ? ? sjlj_emit_function_exit ();
> + ? ? }
> +
> ? ?VEC_free (int, heap, sjlj_lp_call_site_index);
> ?}
>
> --
> ?Dr. Ulrich Weigand
> ?GNU Toolchain for Linux on System z and Cell BE
> ?Ulrich.Weigand@de.ibm.com
>