* allow EH to escape from GIMPLE_EH_ELSE ELSE block @ 2019-06-27 8:17 Alexandre Oliva 2019-06-27 9:36 ` Richard Biener 0 siblings, 1 reply; 12+ messages in thread From: Alexandre Oliva @ 2019-06-27 8:17 UTC (permalink / raw) To: gcc-patches The only preexisting use of GIMPLE_EH_ELSE, for transactional memory commits, did not allow exceptions to escape from the ELSE path. The trick it uses to allow the ELSE path to see the propagating exception does not work very well if the exception cleanup raises further exceptions: the ELSE block is configured to handle exceptions in itself. This confuses the heck out of CFG and EH cleanups. Basing the lowering context for the ELSE block on outer_state, rather than this_state, gets us the expected enclosing handler. Regstrapped on x86_64-linux-gnu. Ok to install? for gcc/ChangeLog * tree-eh.c (honor_protect_cleanup_actions): Use outer_ rather than this_state as the lowering context for the ELSE seq in a GIMPLE_EH_ELSE. --- gcc/tree-eh.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c index 23c56b5661a1..4de09d1bf7b5 100644 --- a/gcc/tree-eh.c +++ b/gcc/tree-eh.c @@ -1001,11 +1001,14 @@ honor_protect_cleanup_actions (struct leh_state *outer_state, gimple_try_set_cleanup (tf->top_p, gimple_eh_else_n_body (eh_else)); finally = gimple_eh_else_e_body (eh_else); - /* Let the ELSE see the exception that's being processed. */ - eh_region save_ehp = this_state->ehp_region; - this_state->ehp_region = this_state->cur_region; - lower_eh_constructs_1 (this_state, &finally); - this_state->ehp_region = save_ehp; + /* Let the ELSE see the exception that's being processed, but + since the cleanup is outside the try block, process it with + outer_state, otherwise it may be used as a cleanup for + itself, and Bad Things (TM) ensue. */ + eh_region save_ehp = outer_state->ehp_region; + outer_state->ehp_region = this_state->cur_region; + lower_eh_constructs_1 (outer_state, &finally); + outer_state->ehp_region = save_ehp; } else { -- Alexandre Oliva, freedom fighter he/him https://FSFLA.org/blogs/lxo Be the change, be Free! FSF Latin America board member GNU Toolchain Engineer Free Software Evangelist Hay que enGNUrecerse, pero sin perder la terGNUra jamás - Che GNUevara ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: allow EH to escape from GIMPLE_EH_ELSE ELSE block 2019-06-27 8:17 allow EH to escape from GIMPLE_EH_ELSE ELSE block Alexandre Oliva @ 2019-06-27 9:36 ` Richard Biener 2019-06-28 9:43 ` Alexandre Oliva 0 siblings, 1 reply; 12+ messages in thread From: Richard Biener @ 2019-06-27 9:36 UTC (permalink / raw) To: Alexandre Oliva; +Cc: GCC Patches On Thu, Jun 27, 2019 at 10:18 AM Alexandre Oliva <oliva@adacore.com> wrote: > > The only preexisting use of GIMPLE_EH_ELSE, for transactional memory > commits, did not allow exceptions to escape from the ELSE path. The > trick it uses to allow the ELSE path to see the propagating exception > does not work very well if the exception cleanup raises further > exceptions: the ELSE block is configured to handle exceptions in > itself. This confuses the heck out of CFG and EH cleanups. > > Basing the lowering context for the ELSE block on outer_state, rather > than this_state, gets us the expected enclosing handler. > > Regstrapped on x86_64-linux-gnu. Ok to install? Testcase? > > for gcc/ChangeLog > > * tree-eh.c (honor_protect_cleanup_actions): Use outer_ > rather than this_state as the lowering context for the ELSE > seq in a GIMPLE_EH_ELSE. > --- > gcc/tree-eh.c | 13 ++++++++----- > 1 file changed, 8 insertions(+), 5 deletions(-) > > diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c > index 23c56b5661a1..4de09d1bf7b5 100644 > --- a/gcc/tree-eh.c > +++ b/gcc/tree-eh.c > @@ -1001,11 +1001,14 @@ honor_protect_cleanup_actions (struct leh_state *outer_state, > gimple_try_set_cleanup (tf->top_p, gimple_eh_else_n_body (eh_else)); > finally = gimple_eh_else_e_body (eh_else); > > - /* Let the ELSE see the exception that's being processed. */ > - eh_region save_ehp = this_state->ehp_region; > - this_state->ehp_region = this_state->cur_region; > - lower_eh_constructs_1 (this_state, &finally); > - this_state->ehp_region = save_ehp; > + /* Let the ELSE see the exception that's being processed, but > + since the cleanup is outside the try block, process it with > + outer_state, otherwise it may be used as a cleanup for > + itself, and Bad Things (TM) ensue. */ > + eh_region save_ehp = outer_state->ehp_region; > + outer_state->ehp_region = this_state->cur_region; > + lower_eh_constructs_1 (outer_state, &finally); > + outer_state->ehp_region = save_ehp; > } > else > { > > -- > Alexandre Oliva, freedom fighter he/him https://FSFLA.org/blogs/lxo > Be the change, be Free! FSF Latin America board member > GNU Toolchain Engineer Free Software Evangelist > Hay que enGNUrecerse, pero sin perder la terGNUra jamás - Che GNUevara ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: allow EH to escape from GIMPLE_EH_ELSE ELSE block 2019-06-27 9:36 ` Richard Biener @ 2019-06-28 9:43 ` Alexandre Oliva 2019-07-01 11:19 ` Richard Biener 0 siblings, 1 reply; 12+ messages in thread From: Alexandre Oliva @ 2019-06-28 9:43 UTC (permalink / raw) To: Richard Biener; +Cc: GCC Patches On Jun 27, 2019, Richard Biener <richard.guenther@gmail.com> wrote: > On Thu, Jun 27, 2019 at 10:18 AM Alexandre Oliva <oliva@adacore.com> wrote: >> >> The only preexisting use of GIMPLE_EH_ELSE, for transactional memory >> commits, did not allow exceptions to escape from the ELSE path. The >> trick it uses to allow the ELSE path to see the propagating exception >> does not work very well if the exception cleanup raises further >> exceptions: the ELSE block is configured to handle exceptions in >> itself. This confuses the heck out of CFG and EH cleanups. >> >> Basing the lowering context for the ELSE block on outer_state, rather >> than this_state, gets us the expected enclosing handler. >> >> Regstrapped on x86_64-linux-gnu. Ok to install? > Testcase? None possible with the current codebase, I'm afraid. Nothing creates EH_ELSE yet, and nothing creates GIMPLE_EH_ELSE with exception-generating cleanups. The following patch, an extract of a larger patch that is still under internal review (and pending approval of the EH_ELSE-related changes ;-), is minimized into pointlessness so as to just exercise the GIMPLE_EH_ELSE lowering issue. IIRC the problem is NOT immediately triggered in a bootstrap, it requires more elaborate EH scenarios to trigger it: without the GIMPLE_EH_ELSE lowering patch, a few libadalang units failed to compile within delete_unreachable_blocks, using a compiler built with the patch below and the patch that introduced EH_ELSE that I posted yesterday. At first, I suspected GIMPLE_EH_ELSE had bit-rotted, as it doesn't seem to get much use, but the problem turned out to be caused by the nonsensical CFG resulting from the GIMPLE_EH_ELSE lowering, that breaks EH CFG optimizations and end up corrupting the CFG. IIRC it was unsplit_eh that mishandled self edges that arise after a bunch of other valid transformations. I cannot observe the crash with trunk any more, but the EH tree is visibly wrong in tree dumps with eh and blocks, compiling such a simple testcase as this (with a patched compiler as described above): -- compile with -Ofast -g -c with GNAT.Semaphores; use GNAT.Semaphores; package T is subtype Mutual_Exclusion is Binary_Semaphore (Initially_Available => True, Ceiling => Default_Ceiling); Lock : aliased Mutual_Exclusion; end T; Self edges end up arising because of the way GIMPLE_EH_ELSE was lowered: the exceptional cleanup was lowered as if within the TRY_FINALLY_EXPR TRY block, whose exceptions get handled by the exceptional cleanup, but both cleanup paths should be lowered as if after the TRY_FINALLY_EXPR, so that an enclosing EH region is used should they throw exceptions. The current lowering made sense for cleanups that couldn't throw: no EH edge would come out of the lowered exceptional cleanup block. However, EH_ELSE-using code below breaks that assumption. Fortunately, the assumption was not essential for GIMPLE_EH_ELSE: the lowering code just needed this small adjustment to make room for relaxing the requirement that the cleanups couldn't throw and restoring CFG EH edges that match what we get without the patch below. --- gcc/ada/gcc-interface/trans.c +++ gcc/ada/gcc-interface/trans.c @@ -6249,7 +6249,7 @@ Exception_Handler_to_gnu_gcc (Node_Id gnat_node) if (stmt_list_cannot_alter_control_flow_p (Statements (gnat_node))) add_stmt_with_node (stmt, gnat_node); else - add_cleanup (stmt, gnat_node); + add_cleanup (build2 (EH_ELSE, void_type_node, stmt, stmt), gnat_node); gnat_poplevel (); @@ -9066,7 +9081,29 @@ add_cleanup (tree gnu_cleanup, Node_Id g { if (Present (gnat_node)) set_expr_location_from_node (gnu_cleanup, gnat_node, true); - append_to_statement_list (gnu_cleanup, ¤t_stmt_group->cleanups); + /* An EH_ELSE must be by itself, and that's all we need when we use + it. The assert below makes sure that is so. Should we ever need + more than that, we could combine EH_ELSEs, and copy non-EH_ELSE + stmts into both cleanup paths of an EH_ELSE, being careful to + make sure the exceptional path doesn't throw. */ + if (TREE_CODE (gnu_cleanup) == EH_ELSE) + { + gcc_assert (!current_stmt_group->cleanups); + if (Present (gnat_node)) + { + set_expr_location_from_node (TREE_OPERAND (gnu_cleanup, 0), + gnat_node, true); + set_expr_location_from_node (TREE_OPERAND (gnu_cleanup, 1), + gnat_node, true); + } + current_stmt_group->cleanups = gnu_cleanup; + } + else + { + gcc_assert (!current_stmt_group->cleanups + || TREE_CODE (current_stmt_group->cleanups) != EH_ELSE); + append_to_statement_list (gnu_cleanup, ¤t_stmt_group->cleanups); + } } /* Set the BLOCK node corresponding to the current code group to GNU_BLOCK. */ -- Alexandre Oliva, freedom fighter he/him https://FSFLA.org/blogs/lxo Be the change, be Free! FSF Latin America board member GNU Toolchain Engineer Free Software Evangelist Hay que enGNUrecerse, pero sin perder la terGNUra jamás - Che GNUevara ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: allow EH to escape from GIMPLE_EH_ELSE ELSE block 2019-06-28 9:43 ` Alexandre Oliva @ 2019-07-01 11:19 ` Richard Biener 2019-07-02 8:51 ` Alexandre Oliva 0 siblings, 1 reply; 12+ messages in thread From: Richard Biener @ 2019-07-01 11:19 UTC (permalink / raw) To: Alexandre Oliva; +Cc: GCC Patches On Fri, Jun 28, 2019 at 11:43 AM Alexandre Oliva <oliva@adacore.com> wrote: > > On Jun 27, 2019, Richard Biener <richard.guenther@gmail.com> wrote: > > > On Thu, Jun 27, 2019 at 10:18 AM Alexandre Oliva <oliva@adacore.com> wrote: > >> > >> The only preexisting use of GIMPLE_EH_ELSE, for transactional memory > >> commits, did not allow exceptions to escape from the ELSE path. The > >> trick it uses to allow the ELSE path to see the propagating exception > >> does not work very well if the exception cleanup raises further > >> exceptions: the ELSE block is configured to handle exceptions in > >> itself. This confuses the heck out of CFG and EH cleanups. > >> > >> Basing the lowering context for the ELSE block on outer_state, rather > >> than this_state, gets us the expected enclosing handler. > >> > >> Regstrapped on x86_64-linux-gnu. Ok to install? > > > Testcase? > > None possible with the current codebase, I'm afraid. Nothing creates > EH_ELSE yet, and nothing creates GIMPLE_EH_ELSE with > exception-generating cleanups. Oh, I see. The GIMPLE frontend is also missing parsing support for the EH stmt kinds, it might have been possible to build a testcase with that I guess (yeah, only a very slight hint ... ;)). Can you share a .gimple dump that has the issue? So the testcase "survives" CFG construction but then crashes during the first CFG cleanup or only after some EH optimization? Thanks, Richard. > The following patch, an extract of a larger patch that is still under > internal review (and pending approval of the EH_ELSE-related changes > ;-), is minimized into pointlessness so as to just exercise the > GIMPLE_EH_ELSE lowering issue. > > IIRC the problem is NOT immediately triggered in a bootstrap, it > requires more elaborate EH scenarios to trigger it: without the > GIMPLE_EH_ELSE lowering patch, a few libadalang units failed to compile > within delete_unreachable_blocks, using a compiler built with the patch > below and the patch that introduced EH_ELSE that I posted yesterday. > > At first, I suspected GIMPLE_EH_ELSE had bit-rotted, as it doesn't seem > to get much use, but the problem turned out to be caused by the > nonsensical CFG resulting from the GIMPLE_EH_ELSE lowering, that breaks > EH CFG optimizations and end up corrupting the CFG. IIRC it was > unsplit_eh that mishandled self edges that arise after a bunch of other > valid transformations. I cannot observe the crash with trunk any more, > but the EH tree is visibly wrong in tree dumps with eh and blocks, > compiling such a simple testcase as this (with a patched compiler as > described above): > > -- compile with -Ofast -g -c > with GNAT.Semaphores; use GNAT.Semaphores; > package T is > subtype Mutual_Exclusion is Binary_Semaphore > (Initially_Available => True, > Ceiling => Default_Ceiling); > Lock : aliased Mutual_Exclusion; > end T; > > > Self edges end up arising because of the way GIMPLE_EH_ELSE was lowered: > the exceptional cleanup was lowered as if within the TRY_FINALLY_EXPR > TRY block, whose exceptions get handled by the exceptional cleanup, but > both cleanup paths should be lowered as if after the TRY_FINALLY_EXPR, > so that an enclosing EH region is used should they throw exceptions. > > The current lowering made sense for cleanups that couldn't throw: no EH > edge would come out of the lowered exceptional cleanup block. However, > EH_ELSE-using code below breaks that assumption. Fortunately, the > assumption was not essential for GIMPLE_EH_ELSE: the lowering code just > needed this small adjustment to make room for relaxing the requirement > that the cleanups couldn't throw and restoring CFG EH edges that match > what we get without the patch below. > > > --- gcc/ada/gcc-interface/trans.c > +++ gcc/ada/gcc-interface/trans.c > @@ -6249,7 +6249,7 @@ Exception_Handler_to_gnu_gcc (Node_Id gnat_node) > if (stmt_list_cannot_alter_control_flow_p (Statements (gnat_node))) > add_stmt_with_node (stmt, gnat_node); > else > - add_cleanup (stmt, gnat_node); > + add_cleanup (build2 (EH_ELSE, void_type_node, stmt, stmt), gnat_node); > > gnat_poplevel (); > > @@ -9066,7 +9081,29 @@ add_cleanup (tree gnu_cleanup, Node_Id g > { > if (Present (gnat_node)) > set_expr_location_from_node (gnu_cleanup, gnat_node, true); > - append_to_statement_list (gnu_cleanup, ¤t_stmt_group->cleanups); > + /* An EH_ELSE must be by itself, and that's all we need when we use > + it. The assert below makes sure that is so. Should we ever need > + more than that, we could combine EH_ELSEs, and copy non-EH_ELSE > + stmts into both cleanup paths of an EH_ELSE, being careful to > + make sure the exceptional path doesn't throw. */ > + if (TREE_CODE (gnu_cleanup) == EH_ELSE) > + { > + gcc_assert (!current_stmt_group->cleanups); > + if (Present (gnat_node)) > + { > + set_expr_location_from_node (TREE_OPERAND (gnu_cleanup, 0), > + gnat_node, true); > + set_expr_location_from_node (TREE_OPERAND (gnu_cleanup, 1), > + gnat_node, true); > + } > + current_stmt_group->cleanups = gnu_cleanup; > + } > + else > + { > + gcc_assert (!current_stmt_group->cleanups > + || TREE_CODE (current_stmt_group->cleanups) != EH_ELSE); > + append_to_statement_list (gnu_cleanup, ¤t_stmt_group->cleanups); > + } > } > > /* Set the BLOCK node corresponding to the current code group to GNU_BLOCK. */ > > > -- > Alexandre Oliva, freedom fighter he/him https://FSFLA.org/blogs/lxo > Be the change, be Free! FSF Latin America board member > GNU Toolchain Engineer Free Software Evangelist > Hay que enGNUrecerse, pero sin perder la terGNUra jamás - Che GNUevara ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: allow EH to escape from GIMPLE_EH_ELSE ELSE block 2019-07-01 11:19 ` Richard Biener @ 2019-07-02 8:51 ` Alexandre Oliva 2019-07-02 10:11 ` Richard Biener 0 siblings, 1 reply; 12+ messages in thread From: Alexandre Oliva @ 2019-07-02 8:51 UTC (permalink / raw) To: Richard Biener; +Cc: GCC Patches [-- Attachment #1: Type: text/plain, Size: 3742 bytes --] On Jul 1, 2019, Richard Biener <richard.guenther@gmail.com> wrote: > Oh, I see. The GIMPLE frontend is also missing parsing support > for the EH stmt kinds, it might have been possible to build a testcase > with that I guess (yeah, only a very slight hint ... ;)) Ooh, beautiful! Is that in the trunk already? I couldn't immediately find it, but... it's almost early again ;-) and I haven't tried really hard. Pointers to files to change and testcases to modify are welcome either way. There's a bit of a catch (no pun intended) that TRY_FINALLY (without GIMPLE_EH_ELSE) lowering involves lang_hooks.eh_protect_cleanup_actions; propagating exceptions out of cleanups, in either EH_ELSE branch, should probably pay attention to that hook as well, but I haven't touched it because the current code suits us. I mention it because I don't know how the gimple frontend sets up this hook, even though it probably wouldn't matter since you'd be taking the EH_ELSE branch rather than the eh_protect_cleanup_actions one in honor_protect_cleanup_actions. > Can you share a .gimple dump that has the issue? Attached. Look for t.finalize_spec () and t (), both have <<<else_eh_exit>>>s. (ugh, else_eh* vs *EH_ELSE; could we make it ehlse? ;-) / 2 > So the testcase "survives" CFG construction but then crashes during > the first CFG cleanup or only after some EH optimization? I dug up and found out what enabled the crash was a patch that Eric Botcazou tells me he will contribute soon. The bit that brought the crash about was the following patchlet extract. Without this patchlet, the CFG is wrong, but I haven't observed any crash. The reason it's wrong is that the block has an abnormal edge to itself, which can be observed in the optimized dump. Exceptions in the cleanup portion of the TRY_FINALLY should propagate to enclosing handlers, and they do when EH_ELSE is not used. With EH_ELSE, however, we get the unintended EH loop as soon as EH lowering. t.finalize_spec () Eh tree: 2 cleanup land:{2,<L9>} 1 try land:{1,<L4>} catch:{&OTHERS} [...] ;; basic block 10, loop depth 1 ;; pred: 8 ;; 10 <L9>: [LP 2] [LP 2] .gnat_end_handler (EXPTR_9); ;; succ: 10 ;; 11 diff --git a/gcc/gimple-ssa-store-merging.c b/gcc/gimple-ssa-store-merging.c index 5a93830..597a488 100644 --- a/gcc/gimple-ssa-store-merging.c +++ b/gcc/gimple-ssa-store-merging.c @@ -159,6 +159,7 @@ #include "gimple-fold.h" #include "stor-layout.h" #include "timevar.h" +#include "cfgcleanup.h" #include "tree-cfg.h" #include "tree-eh.h" #include "target.h" @@ -4671,6 +4672,14 @@ pass_store_merging::execute (function *fun) basic_block bb; hash_set<gimple *> orig_stmts; + if (cfun->can_throw_non_call_exceptions && cfun->eh) + { + maybe_remove_unreachable_handlers (); + bool changed = unsplit_all_eh (); + if (changed) + delete_unreachable_blocks (); + } + calculate_dominance_info (CDI_DOMINATORS); FOR_EACH_BB_FN (bb, fun) diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c index 23c56b5..f547d98 100644 --- a/gcc/tree-eh.c +++ b/gcc/tree-eh.c @@ -4185,7 +4185,7 @@ unsplit_eh (eh_landing_pad lp) /* Examine each landing pad block and see if it matches unsplit_eh. */ -static bool +bool unsplit_all_eh (void) { bool changed = false; diff --git a/gcc/tree-eh.h b/gcc/tree-eh.h index a588c10..3998a97 100644 --- a/gcc/tree-eh.h +++ b/gcc/tree-eh.h @@ -52,5 +52,6 @@ extern bool maybe_duplicate_eh_stmt (gimple *, gimple *); extern void maybe_remove_unreachable_handlers (void); extern bool verify_eh_edges (gimple *); extern bool verify_eh_dispatch_edge (geh_dispatch *); +extern bool unsplit_all_eh (void); #endif /* GCC_TREE_EH_H */ [-- Attachment #2: t.ads.005t.gimple --] [-- Type: text/plain, Size: 17547 bytes --] _GLOBAL.SZ0_t (positive___XDLU_1__2147483647 p0, positive___XDLU_1__2147483647 p1) { bitsizetype D.5454; bitsizetype iftmp.0; if (p1 <= p0) goto <D.5456>; else goto <D.5457>; <D.5456>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; _4 = _3 + 1; _5 = (bitsizetype) _4; iftmp.0 = _5 * 8; goto <D.5458>; <D.5457>: iftmp.0 = 0; <D.5458>: D.5454 = iftmp.0; return D.5454; } _GLOBAL.SZ1_t (positive___XDLU_1__2147483647 p0, positive___XDLU_1__2147483647 p1) { sizetype D.5459; sizetype iftmp.1; if (p1 <= p0) goto <D.5461>; else goto <D.5462>; <D.5461>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; iftmp.1 = _3 + 1; goto <D.5463>; <D.5462>: iftmp.1 = 0; <D.5463>: D.5459 = iftmp.1; return D.5459; } _GLOBAL.SZ2_t (interfaces__c__size_t p0, interfaces__c__size_t p1) { bitsizetype D.5464; bitsizetype iftmp.2; if (p1 <= p0) goto <D.5466>; else goto <D.5467>; <D.5466>: _1 = p0 - p1; _2 = _1 + 1; _3 = (bitsizetype) _2; iftmp.2 = _3 * 8; goto <D.5468>; <D.5467>: iftmp.2 = 0; <D.5468>: D.5464 = iftmp.2; return D.5464; } _GLOBAL.SZ3_t (interfaces__c__size_t p0, interfaces__c__size_t p1) { sizetype D.5469; sizetype iftmp.3; if (p1 <= p0) goto <D.5471>; else goto <D.5472>; <D.5471>: _1 = p0 - p1; iftmp.3 = _1 + 1; goto <D.5473>; <D.5472>: iftmp.3 = 0; <D.5473>: D.5469 = iftmp.3; return D.5469; } _GLOBAL.SZ4_t (system__tasking__protected_objects__protected_entry_index___XDLU_0__2147483647 p0, system__tasking__protected_objects__protected_entry_index___XDLU_0__2147483647 p1) { bitsizetype D.5474; bitsizetype iftmp.4; if (p1 <= p0) goto <D.5476>; else goto <D.5477>; <D.5476>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; _4 = _3 + 1; _5 = (bitsizetype) _4; iftmp.4 = _5 * 128; goto <D.5478>; <D.5477>: iftmp.4 = 0; <D.5478>: D.5474 = iftmp.4; return D.5474; } _GLOBAL.SZ5_t (system__tasking__protected_objects__protected_entry_index___XDLU_0__2147483647 p0, system__tasking__protected_objects__protected_entry_index___XDLU_0__2147483647 p1) { sizetype D.5479; sizetype iftmp.5; if (p1 <= p0) goto <D.5481>; else goto <D.5482>; <D.5481>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; _4 = _3 + 1; iftmp.5 = _4 * 16; goto <D.5483>; <D.5482>: iftmp.5 = 0; <D.5483>: D.5479 = iftmp.5; return D.5479; } _GLOBAL.SZ6_t (system__tasking__protected_objects__entries__positive_protected_entry_index___XDLU_1__2147483647 p0, system__tasking__protected_objects__entries__positive_protected_entry_index___XDLU_1__2147483647 p1) { bitsizetype D.5484; bitsizetype iftmp.6; if (p1 <= p0) goto <D.5486>; else goto <D.5487>; <D.5486>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; _4 = _3 + 1; _5 = (bitsizetype) _4; iftmp.6 = _5 * 32; goto <D.5488>; <D.5487>: iftmp.6 = 0; <D.5488>: D.5484 = iftmp.6; return D.5484; } _GLOBAL.SZ7_t (system__tasking__protected_objects__entries__positive_protected_entry_index___XDLU_1__2147483647 p0, system__tasking__protected_objects__entries__positive_protected_entry_index___XDLU_1__2147483647 p1) { sizetype D.5489; sizetype iftmp.7; if (p1 <= p0) goto <D.5491>; else goto <D.5492>; <D.5491>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; _4 = _3 + 1; iftmp.7 = _4 * 4; goto <D.5493>; <D.5492>: iftmp.7 = 0; <D.5493>: D.5489 = iftmp.7; return D.5489; } _GLOBAL.SZ8_t (system__tasking__protected_objects__entries__positive_protected_entry_index___XDLU_1__2147483647 p0, system__tasking__protected_objects__entries__positive_protected_entry_index___XDLU_1__2147483647 p1) { bitsizetype D.5494; bitsizetype iftmp.8; if (p1 <= p0) goto <D.5496>; else goto <D.5497>; <D.5496>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; _4 = _3 + 1; _5 = (bitsizetype) _4; iftmp.8 = _5 * 128; goto <D.5498>; <D.5497>: iftmp.8 = 0; <D.5498>: D.5494 = iftmp.8; return D.5494; } _GLOBAL.SZ9_t (system__tasking__protected_objects__entries__positive_protected_entry_index___XDLU_1__2147483647 p0, system__tasking__protected_objects__entries__positive_protected_entry_index___XDLU_1__2147483647 p1) { sizetype D.5499; sizetype iftmp.9; if (p1 <= p0) goto <D.5501>; else goto <D.5502>; <D.5501>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; _4 = _3 + 1; iftmp.9 = _4 * 16; goto <D.5503>; <D.5502>: iftmp.9 = 0; <D.5503>: D.5499 = iftmp.9; return D.5499; } _GLOBAL.SZ10_t (positive___XDLU_1__2147483647 p0, positive___XDLU_1__2147483647 p1) { bitsizetype D.5504; bitsizetype iftmp.10; if (p1 <= p0) goto <D.5506>; else goto <D.5507>; <D.5506>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; _4 = _3 + 1; _5 = (bitsizetype) _4; iftmp.10 = _5 * 64; goto <D.5508>; <D.5507>: iftmp.10 = 0; <D.5508>: D.5504 = iftmp.10; return D.5504; } _GLOBAL.SZ11_t (positive___XDLU_1__2147483647 p0, positive___XDLU_1__2147483647 p1) { sizetype D.5509; sizetype iftmp.11; if (p1 <= p0) goto <D.5511>; else goto <D.5512>; <D.5511>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; _4 = _3 + 1; iftmp.11 = _4 * 8; goto <D.5513>; <D.5512>: iftmp.11 = 0; <D.5513>: D.5509 = iftmp.11; return D.5509; } _GLOBAL.SZ12_t (integer p0, integer p1) { bitsizetype D.5514; bitsizetype iftmp.12; if (p1 <= p0) goto <D.5516>; else goto <D.5517>; <D.5516>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; _4 = _3 + 1; _5 = (bitsizetype) _4; iftmp.12 = _5 * 8; goto <D.5518>; <D.5517>: iftmp.12 = 0; <D.5518>: D.5514 = iftmp.12; return D.5514; } _GLOBAL.SZ13_t (integer p0, integer p1) { sizetype D.5519; sizetype iftmp.13; if (p1 <= p0) goto <D.5521>; else goto <D.5522>; <D.5521>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; iftmp.13 = _3 + 1; goto <D.5523>; <D.5522>: iftmp.13 = 0; <D.5523>: D.5519 = iftmp.13; return D.5519; } _GLOBAL.SZ14_t (system__tasking__atc_level_index___XDLU_1__19 p0, system__tasking__atc_level_index___XDLU_1__19 p1) { bitsizetype D.5524; bitsizetype iftmp.14; if (p1 <= p0) goto <D.5526>; else goto <D.5527>; <D.5526>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; _4 = _3 + 1; _5 = (bitsizetype) _4; iftmp.14 = _5 * 768; goto <D.5528>; <D.5527>: iftmp.14 = 0; <D.5528>: D.5524 = iftmp.14; return D.5524; } _GLOBAL.SZ15_t (system__tasking__atc_level_index___XDLU_1__19 p0, system__tasking__atc_level_index___XDLU_1__19 p1) { sizetype D.5529; sizetype iftmp.15; if (p1 <= p0) goto <D.5531>; else goto <D.5532>; <D.5531>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; _4 = _3 + 1; iftmp.15 = _4 * 96; goto <D.5533>; <D.5532>: iftmp.15 = 0; <D.5533>: D.5529 = iftmp.15; return D.5529; } _GLOBAL.SZ16_t (system__tasking__Tattribute_arrayD1___XDLU_1__32 p0, system__tasking__Tattribute_arrayD1___XDLU_1__32 p1) { bitsizetype D.5534; bitsizetype iftmp.16; if (p1 <= p0) goto <D.5536>; else goto <D.5537>; <D.5536>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; _4 = _3 + 1; _5 = (bitsizetype) _4; iftmp.16 = _5 * 64; goto <D.5538>; <D.5537>: iftmp.16 = 0; <D.5538>: D.5534 = iftmp.16; return D.5534; } _GLOBAL.SZ17_t (system__tasking__Tattribute_arrayD1___XDLU_1__32 p0, system__tasking__Tattribute_arrayD1___XDLU_1__32 p1) { sizetype D.5539; sizetype iftmp.17; if (p1 <= p0) goto <D.5541>; else goto <D.5542>; <D.5541>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; _4 = _3 + 1; iftmp.17 = _4 * 8; goto <D.5543>; <D.5542>: iftmp.17 = 0; <D.5543>: D.5539 = iftmp.17; return D.5539; } _GLOBAL.SZ18_t (system__tasking__task_entry_index___XDLU_0__2147483647 p0, system__tasking__task_entry_index___XDLU_0__2147483647 p1) { bitsizetype D.5544; bitsizetype iftmp.18; if (p1 <= p0) goto <D.5546>; else goto <D.5547>; <D.5546>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; _4 = _3 + 1; _5 = (bitsizetype) _4; iftmp.18 = _5 * 128; goto <D.5548>; <D.5547>: iftmp.18 = 0; <D.5548>: D.5544 = iftmp.18; return D.5544; } _GLOBAL.SZ19_t (system__tasking__task_entry_index___XDLU_0__2147483647 p0, system__tasking__task_entry_index___XDLU_0__2147483647 p1) { sizetype D.5549; sizetype iftmp.19; if (p1 <= p0) goto <D.5551>; else goto <D.5552>; <D.5551>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; _4 = _3 + 1; iftmp.19 = _4 * 16; goto <D.5553>; <D.5552>: iftmp.19 = 0; <D.5553>: D.5549 = iftmp.19; return D.5549; } _GLOBAL.SZ20_t (system__tasking__positive_select_index___XDLU_1__2147483647 p0, system__tasking__positive_select_index___XDLU_1__2147483647 p1) { bitsizetype D.5554; bitsizetype iftmp.20; if (p1 <= p0) goto <D.5556>; else goto <D.5557>; <D.5556>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; _4 = _3 + 1; _5 = (bitsizetype) _4; iftmp.20 = _5 * 64; goto <D.5558>; <D.5557>: iftmp.20 = 0; <D.5558>: D.5554 = iftmp.20; return D.5554; } _GLOBAL.SZ21_t (system__tasking__positive_select_index___XDLU_1__2147483647 p0, system__tasking__positive_select_index___XDLU_1__2147483647 p1) { sizetype D.5559; sizetype iftmp.21; if (p1 <= p0) goto <D.5561>; else goto <D.5562>; <D.5561>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; _4 = _3 + 1; iftmp.21 = _4 * 8; goto <D.5563>; <D.5562>: iftmp.21 = 0; <D.5563>: D.5559 = iftmp.21; return D.5559; } _GLOBAL.SZ22_t (system__multiprocessors__cpu___XDLU_1__65535 p0, system__multiprocessors__cpu___XDLU_1__65535 p1) { bitsizetype D.5564; bitsizetype iftmp.22; if (p1 <= p0) goto <D.5566>; else goto <D.5567>; <D.5566>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; _4 = _3 + 1; _5 = (bitsizetype) _4; iftmp.22 = _5 * 8; goto <D.5568>; <D.5567>: iftmp.22 = 0; <D.5568>: D.5564 = iftmp.22; return D.5564; } _GLOBAL.SZ23_t (system__multiprocessors__cpu___XDLU_1__65535 p0, system__multiprocessors__cpu___XDLU_1__65535 p1) { sizetype D.5569; sizetype iftmp.23; if (p1 <= p0) goto <D.5571>; else goto <D.5572>; <D.5571>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; iftmp.23 = _3 + 1; goto <D.5573>; <D.5572>: iftmp.23 = 0; <D.5573>: D.5569 = iftmp.23; return D.5569; } _GLOBAL.SZ24_t (system__os_interface__Tbit_fieldD1___XDLU_1__1024 p0, system__os_interface__Tbit_fieldD1___XDLU_1__1024 p1) { bitsizetype D.5574; bitsizetype iftmp.24; if (p1 <= p0) goto <D.5576>; else goto <D.5577>; <D.5576>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; _4 = _3 + 1; _5 = (bitsizetype) _4; iftmp.24 = _5 * 8; goto <D.5578>; <D.5577>: iftmp.24 = 0; <D.5578>: D.5574 = iftmp.24; return D.5574; } _GLOBAL.SZ25_t (system__os_interface__Tbit_fieldD1___XDLU_1__1024 p0, system__os_interface__Tbit_fieldD1___XDLU_1__1024 p1) { sizetype D.5579; sizetype iftmp.25; if (p1 <= p0) goto <D.5581>; else goto <D.5582>; <D.5581>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; iftmp.25 = _3 + 1; goto <D.5583>; <D.5582>: iftmp.25 = 0; <D.5583>: D.5579 = iftmp.25; return D.5579; } _GLOBAL.SZ26_t (natural___XDLU_0__2147483647 p0, natural___XDLU_0__2147483647 p1) { bitsizetype D.5584; bitsizetype iftmp.26; if (p1 <= p0) goto <D.5586>; else goto <D.5587>; <D.5586>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; _4 = _3 + 1; _5 = (bitsizetype) _4; iftmp.26 = _5 * 8; goto <D.5588>; <D.5587>: iftmp.26 = 0; <D.5588>: D.5584 = iftmp.26; return D.5584; } _GLOBAL.SZ27_t (natural___XDLU_0__2147483647 p0, natural___XDLU_0__2147483647 p1) { sizetype D.5589; sizetype iftmp.27; if (p1 <= p0) goto <D.5591>; else goto <D.5592>; <D.5591>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; iftmp.27 = _3 + 1; goto <D.5593>; <D.5592>: iftmp.27 = 0; <D.5593>: D.5589 = iftmp.27; return D.5589; } _GLOBAL.SZ28_t (system__secondary_stack__memory_size___XDLU_0__9223372036854775807 p0, system__secondary_stack__memory_size___XDLU_0__9223372036854775807 p1) { bitsizetype D.5594; bitsizetype iftmp.28; if (p1 <= p0) goto <D.5596>; else goto <D.5597>; <D.5596>: p0.29_1 = (sizetype) p0; p1.30_2 = (sizetype) p1; _3 = p0.29_1 - p1.30_2; _4 = _3 + 1; _5 = (bitsizetype) _4; iftmp.28 = _5 * 8; goto <D.5598>; <D.5597>: iftmp.28 = 0; <D.5598>: D.5594 = iftmp.28; return D.5594; } _GLOBAL.SZ29_t (system__secondary_stack__memory_size___XDLU_0__9223372036854775807 p0, system__secondary_stack__memory_size___XDLU_0__9223372036854775807 p1) { sizetype D.5599; sizetype iftmp.31; if (p1 <= p0) goto <D.5601>; else goto <D.5602>; <D.5601>: p0.32_1 = (sizetype) p0; p1.33_2 = (sizetype) p1; _3 = p0.32_1 - p1.33_2; iftmp.31 = _3 + 1; goto <D.5603>; <D.5602>: iftmp.31 = 0; <D.5603>: D.5599 = iftmp.31; return D.5599; } _GLOBAL.SZ30_t (positive___XDLU_1__2147483647 p0, positive___XDLU_1__2147483647 p1) { bitsizetype D.5604; bitsizetype iftmp.34; if (p1 <= p0) goto <D.5606>; else goto <D.5607>; <D.5606>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; _4 = _3 + 1; _5 = (bitsizetype) _4; iftmp.34 = _5 * 64; goto <D.5608>; <D.5607>: iftmp.34 = 0; <D.5608>: D.5604 = iftmp.34; return D.5604; } _GLOBAL.SZ31_t (positive___XDLU_1__2147483647 p0, positive___XDLU_1__2147483647 p1) { sizetype D.5609; sizetype iftmp.35; if (p1 <= p0) goto <D.5611>; else goto <D.5612>; <D.5611>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; _4 = _3 + 1; iftmp.35 = _4 * 8; goto <D.5613>; <D.5612>: iftmp.35 = 0; <D.5613>: D.5609 = iftmp.35; return D.5609; } _GLOBAL.SZ32_t (positive___XDLU_1__2147483647 p0, positive___XDLU_1__2147483647 p1) { bitsizetype D.5614; bitsizetype iftmp.36; if (p1 <= p0) goto <D.5616>; else goto <D.5617>; <D.5616>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; _4 = _3 + 1; _5 = (bitsizetype) _4; iftmp.36 = _5 * 64; goto <D.5618>; <D.5617>: iftmp.36 = 0; <D.5618>: D.5614 = iftmp.36; return D.5614; } _GLOBAL.SZ33_t (positive___XDLU_1__2147483647 p0, positive___XDLU_1__2147483647 p1) { sizetype D.5619; sizetype iftmp.37; if (p1 <= p0) goto <D.5621>; else goto <D.5622>; <D.5621>: _1 = (sizetype) p0; _2 = (sizetype) p1; _3 = _1 - _2; _4 = _3 + 1; iftmp.37 = _4 * 8; goto <D.5623>; <D.5622>: iftmp.37 = 0; <D.5623>: D.5619 = iftmp.37; return D.5619; } t.finalize_spec () { void L0; void L1; boolean R6s; R6s = 0; system__soft_links__abort_defer.38_1 = system__soft_links__abort_defer; system__soft_links__abort_defer.38_1 (); t__C4s.39_2 = t__C4s; if (t__C4s.39_2 == 1) goto L1; else goto L0; L1: try { gnat.semaphores.binary_semaphoreVDF (&t__lock, 1); } catch { catch (&OTHERS) { { void * EXPTR; try { EXPTR = .builtin_eh_pointer (0); .gnat_begin_handler (EXPTR); _3 = R6s ^ 1; if (_3 != 0) goto <D.5624>; else goto <D.5625>; <D.5624>: R6s = 1; system.soft_links.save_library_occurrence (0B); goto <D.5626>; <D.5625>: <D.5626>: } finally { <<<if_normal_exit>>> { .gnat_end_handler (EXPTR); } <<<else_eh_exit>>> { .gnat_end_handler (EXPTR); } } } } } L0: system__soft_links__abort_undefer.40_4 = system__soft_links__abort_undefer; system__soft_links__abort_undefer.40_4 (); return; } t () { try { system__soft_links__abort_defer.41_1 = system__soft_links__abort_defer; system__soft_links__abort_defer.41_1 (); gnat.semaphores.binary_semaphoreVIP (&t__lock, 1, 48); try { gnat.semaphores.binary_semaphoreVDI (&t__lock); t__C4s = 1; } catch { catch (&OTHERS) { { void * EXPTR; try { EXPTR = .builtin_eh_pointer (0); .gnat_begin_handler (EXPTR); gnat.semaphores.binary_semaphoreVDF (&t__lock, 0); { void * SAVED_EXPTR; SAVED_EXPTR = EXPTR; EXPTR = 0B; .gnat_reraise_zcx (SAVED_EXPTR); } } finally { <<<if_normal_exit>>> { .gnat_end_handler (EXPTR); } <<<else_eh_exit>>> { .gnat_end_handler (EXPTR); } } } } } } finally { system.standard_library.abort_undefer_direct (); } } system.standard_library.abort_undefer_direct () { system__soft_links__abort_undefer.42_1 = system__soft_links__abort_undefer; system__soft_links__abort_undefer.42_1 (); return; } [-- Attachment #3: Type: text/plain, Size: 298 bytes --] -- Alexandre Oliva, freedom fighter he/him https://FSFLA.org/blogs/lxo Be the change, be Free! FSF Latin America board member GNU Toolchain Engineer Free Software Evangelist Hay que enGNUrecerse, pero sin perder la terGNUra jamás - Che GNUevara ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: allow EH to escape from GIMPLE_EH_ELSE ELSE block 2019-07-02 8:51 ` Alexandre Oliva @ 2019-07-02 10:11 ` Richard Biener 2019-07-04 8:50 ` Alexandre Oliva 0 siblings, 1 reply; 12+ messages in thread From: Richard Biener @ 2019-07-02 10:11 UTC (permalink / raw) To: Alexandre Oliva; +Cc: GCC Patches On Tue, Jul 2, 2019 at 10:51 AM Alexandre Oliva <oliva@adacore.com> wrote: > > On Jul 1, 2019, Richard Biener <richard.guenther@gmail.com> wrote: > > > Oh, I see. The GIMPLE frontend is also missing parsing support > > for the EH stmt kinds, it might have been possible to build a testcase > > with that I guess (yeah, only a very slight hint ... ;)) > > Ooh, beautiful! Is that in the trunk already? I couldn't immediately > find it, but... it's almost early again ;-) and I haven't tried really > hard. Pointers to files to change and testcases to modify are welcome > either way. Yeah, it's on trunk. The parser is ontop of the C frontend and resides in gcc/c/gimple-parser.c while testcases are in gcc.dg/gimplefe-*.c There are also quite a few unit tests for individual passes now, for example the just contributed gcc.dg/tree-ssa/ssa-dom-cse-8.c The parsing is incomplete, there's no support for parsing try/catch/finally but the FE should be able to handle both high and low GIMPLE as well as GIMPLE with a CFG and SSA. Not sure if there are any/many high-gimple testcases. Also populating on-the-side info (like EH tables or associating stmts with it) isn't implemented but if you do high GIMPLE that doesn't exist besides stuff created by the frontends (so specific catches would need quite some support I guess). > > > There's a bit of a catch (no pun intended) that TRY_FINALLY (without > GIMPLE_EH_ELSE) lowering involves lang_hooks.eh_protect_cleanup_actions; > propagating exceptions out of cleanups, in either EH_ELSE branch, should > probably pay attention to that hook as well, but I haven't touched it > because the current code suits us. I mention it because I don't know > how the gimple frontend sets up this hook, even though it probably > wouldn't matter since you'd be taking the EH_ELSE branch rather than the > eh_protect_cleanup_actions one in honor_protect_cleanup_actions. > > > > Can you share a .gimple dump that has the issue? > > Attached. Look for t.finalize_spec () and t (), both have > <<<else_eh_exit>>>s. > > (ugh, else_eh* vs *EH_ELSE; could we make it ehlse? ;-) / 2 I guess dumping it as finally { if (normal exit) ... else if (eh exit) ... } would work as well? Or even try { } eh_finally { } finally { } which I would find more natural (even in the trees...). > > > > So the testcase "survives" CFG construction but then crashes during > > the first CFG cleanup or only after some EH optimization? > > I dug up and found out what enabled the crash was a patch that Eric > Botcazou tells me he will contribute soon. The bit that brought the > crash about was the following patchlet extract. Without this patchlet, > the CFG is wrong, but I haven't observed any crash. The reason it's > wrong is that the block has an abnormal edge to itself, which can be > observed in the optimized dump. Exceptions in the cleanup portion of > the TRY_FINALLY should propagate to enclosing handlers, and they do when > EH_ELSE is not used. With EH_ELSE, however, we get the unintended EH > loop as soon as EH lowering. Ah, that's of course bad. Thanks, Richard. > > t.finalize_spec () > Eh tree: > 2 cleanup land:{2,<L9>} > 1 try land:{1,<L4>} catch:{&OTHERS} > [...] > ;; basic block 10, loop depth 1 > ;; pred: 8 > ;; 10 > <L9>: [LP 2] > [LP 2] .gnat_end_handler (EXPTR_9); > ;; succ: 10 > ;; 11 > > > > diff --git a/gcc/gimple-ssa-store-merging.c b/gcc/gimple-ssa-store-merging.c > index 5a93830..597a488 100644 > --- a/gcc/gimple-ssa-store-merging.c > +++ b/gcc/gimple-ssa-store-merging.c > @@ -159,6 +159,7 @@ > #include "gimple-fold.h" > #include "stor-layout.h" > #include "timevar.h" > +#include "cfgcleanup.h" > #include "tree-cfg.h" > #include "tree-eh.h" > #include "target.h" > @@ -4671,6 +4672,14 @@ pass_store_merging::execute (function *fun) > basic_block bb; > hash_set<gimple *> orig_stmts; > > + if (cfun->can_throw_non_call_exceptions && cfun->eh) > + { > + maybe_remove_unreachable_handlers (); > + bool changed = unsplit_all_eh (); > + if (changed) > + delete_unreachable_blocks (); > + } > + > calculate_dominance_info (CDI_DOMINATORS); > > FOR_EACH_BB_FN (bb, fun) > diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c > index 23c56b5..f547d98 100644 > --- a/gcc/tree-eh.c > +++ b/gcc/tree-eh.c > @@ -4185,7 +4185,7 @@ unsplit_eh (eh_landing_pad lp) > > /* Examine each landing pad block and see if it matches unsplit_eh. */ > > -static bool > +bool > unsplit_all_eh (void) > { > bool changed = false; > diff --git a/gcc/tree-eh.h b/gcc/tree-eh.h > index a588c10..3998a97 100644 > --- a/gcc/tree-eh.h > +++ b/gcc/tree-eh.h > @@ -52,5 +52,6 @@ extern bool maybe_duplicate_eh_stmt (gimple *, gimple *); > extern void maybe_remove_unreachable_handlers (void); > extern bool verify_eh_edges (gimple *); > extern bool verify_eh_dispatch_edge (geh_dispatch *); > +extern bool unsplit_all_eh (void); > > #endif /* GCC_TREE_EH_H */ > > > > > > > -- > Alexandre Oliva, freedom fighter he/him https://FSFLA.org/blogs/lxo > Be the change, be Free! FSF Latin America board member > GNU Toolchain Engineer Free Software Evangelist > Hay que enGNUrecerse, pero sin perder la terGNUra jamás - Che GNUevara ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: allow EH to escape from GIMPLE_EH_ELSE ELSE block 2019-07-02 10:11 ` Richard Biener @ 2019-07-04 8:50 ` Alexandre Oliva 2019-07-04 10:54 ` Richard Biener 0 siblings, 1 reply; 12+ messages in thread From: Alexandre Oliva @ 2019-07-04 8:50 UTC (permalink / raw) To: Richard Biener; +Cc: GCC Patches On Jul 2, 2019, Richard Biener <richard.guenther@gmail.com> wrote: > Yeah, it's on trunk. The parser is ontop of the C frontend and resides > in gcc/c/gimple-parser.c while testcases are in gcc.dg/gimplefe-*.c > The parsing is incomplete, there's no support for parsing try/catch/finally I'm afraid I haven't got very far, but I tried. It didn't recognize try and finally as keywords, and since the parser is integrated with the C parser IIUC, I wasn't sure how to enable the keywords only within gimple functions. As mentioned in another message, I chose try/finally/else as the notation for TRY_FINALLY_EXPR <..., EH_ELSE_EXPR <..., ...> >, to avoid introducing yet another keyword such as eh_finally. I also considered try/noexcept/finally, or try/noexcept finally/finally, but... else seems to be a lot more closely related with EH_ELSE_EXPR, and at least in gimple it's non-ambiguous. introduce try/finally/else in gimplefe (WIP FTR) From: Alexandre Oliva <oliva@adacore.com> --- gcc/c/gimple-parser.c | 49 ++++++++++++++++++++++++++++++++++++ gcc/testsuite/gcc.dg/gimplefe-43.c | 13 ++++++++++ 2 files changed, 62 insertions(+) create mode 100644 gcc/testsuite/gcc.dg/gimplefe-43.c diff --git a/gcc/c/gimple-parser.c b/gcc/c/gimple-parser.c index b2b364cc41a3..91f2499bb1cc 100644 --- a/gcc/c/gimple-parser.c +++ b/gcc/c/gimple-parser.c @@ -115,6 +115,7 @@ static struct c_expr c_parser_gimple_postfix_expression_after_primary static void c_parser_gimple_declaration (gimple_parser &); static void c_parser_gimple_goto_stmt (gimple_parser &, location_t, tree, gimple_seq *); +static void c_parser_gimple_try_stmt (gimple_parser &, gimple_seq *); static void c_parser_gimple_if_stmt (gimple_parser &, gimple_seq *); static void c_parser_gimple_switch_stmt (gimple_parser &, gimple_seq *); static void c_parser_gimple_return_stmt (gimple_parser &, gimple_seq *); @@ -405,6 +406,9 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) case CPP_KEYWORD: switch (c_parser_peek_token (parser)->keyword) { + case RID_AT_TRY: + c_parser_gimple_try_stmt (parser, seq); + break; case RID_IF: c_parser_gimple_if_stmt (parser, seq); break; @@ -2088,6 +2092,51 @@ c_parser_gimple_paren_condition (gimple_parser &parser) return cond; } +/* Parse gimple try statement. + + try-statement: + try { ... } finally { ... } + try { ... } finally { ... } else { ... } + + This could support try/catch as well, but it's not implemented yet. + */ + +static void +c_parser_gimple_try_stmt (gimple_parser &parser, gimple_seq *seq) +{ + gimple_seq tryseq = NULL; + c_parser_consume_token (parser); + c_parser_gimple_compound_statement (parser, &tryseq); + + if (c_parser_next_token_is (parser, CPP_KEYWORD) + && c_parser_peek_token (parser)->keyword == RID_AT_FINALLY) + { + gimple_seq finseq = NULL; + c_parser_consume_token (parser); + c_parser_gimple_compound_statement (parser, &finseq); + + if (c_parser_next_token_is (parser, CPP_KEYWORD) + && c_parser_peek_token (parser)->keyword == RID_ELSE) + { + gimple_seq elsseq = NULL; + c_parser_consume_token (parser); + c_parser_gimple_compound_statement (parser, &finseq); + + geh_else *stmt = gimple_build_eh_else (finseq, elsseq); + finseq = NULL; + gimple_seq_add_stmt_without_update (&finseq, stmt); + } + + gtry *stmt = gimple_build_try (tryseq, finseq, GIMPLE_TRY_FINALLY); + gimple_seq_add_stmt_without_update (seq, stmt); + } + else if (c_parser_next_token_is (parser, CPP_KEYWORD) + && c_parser_peek_token (parser)->keyword == RID_AT_CATCH) + c_parser_error (parser, "%<catch%> is not supported"); + else + c_parser_error (parser, "expected %<finally%> or %<catch%>"); +} + /* Parse gimple if-else statement. if-statement: diff --git a/gcc/testsuite/gcc.dg/gimplefe-43.c b/gcc/testsuite/gcc.dg/gimplefe-43.c new file mode 100644 index 000000000000..c740e06a78e1 --- /dev/null +++ b/gcc/testsuite/gcc.dg/gimplefe-43.c @@ -0,0 +1,13 @@ +/* { dg-do compile } */ +/* { dg-options "-fgimple" } */ + +void __GIMPLE foo() +{ + try { + ; + } finally { + ; + } else { + ; + } +} -- Alexandre Oliva, freedom fighter he/him https://FSFLA.org/blogs/lxo Be the change, be Free! FSF Latin America board member GNU Toolchain Engineer Free Software Evangelist Hay que enGNUrecerse, pero sin perder la terGNUra jamás - Che GNUevara ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: allow EH to escape from GIMPLE_EH_ELSE ELSE block 2019-07-04 8:50 ` Alexandre Oliva @ 2019-07-04 10:54 ` Richard Biener 2019-07-11 10:44 ` Alexandre Oliva 2019-07-11 10:52 ` Alexandre Oliva 0 siblings, 2 replies; 12+ messages in thread From: Richard Biener @ 2019-07-04 10:54 UTC (permalink / raw) To: Alexandre Oliva; +Cc: GCC Patches On Thu, Jul 4, 2019 at 10:29 AM Alexandre Oliva <oliva@adacore.com> wrote: > > On Jul 2, 2019, Richard Biener <richard.guenther@gmail.com> wrote: > > > Yeah, it's on trunk. The parser is ontop of the C frontend and resides > > in gcc/c/gimple-parser.c while testcases are in gcc.dg/gimplefe-*.c > > > The parsing is incomplete, there's no support for parsing try/catch/finally > > I'm afraid I haven't got very far, but I tried. It didn't recognize try > and finally as keywords, and since the parser is integrated with the C > parser IIUC, I wasn't sure how to enable the keywords only within gimple > functions. Yeah. For other stuff we're simply looking at CPP_NAME and string-matching, see c_parser_gimple_compound_statement where you'd probably hook this into. > As mentioned in another message, I chose try/finally/else as the > notation for TRY_FINALLY_EXPR <..., EH_ELSE_EXPR <..., ...> >, to avoid > introducing yet another keyword such as eh_finally. > > I also considered try/noexcept/finally, or try/noexcept finally/finally, > but... else seems to be a lot more closely related with EH_ELSE_EXPR, > and at least in gimple it's non-ambiguous. > > > introduce try/finally/else in gimplefe (WIP FTR) > > From: Alexandre Oliva <oliva@adacore.com> > > > --- > gcc/c/gimple-parser.c | 49 ++++++++++++++++++++++++++++++++++++ > gcc/testsuite/gcc.dg/gimplefe-43.c | 13 ++++++++++ > 2 files changed, 62 insertions(+) > create mode 100644 gcc/testsuite/gcc.dg/gimplefe-43.c > > diff --git a/gcc/c/gimple-parser.c b/gcc/c/gimple-parser.c > index b2b364cc41a3..91f2499bb1cc 100644 > --- a/gcc/c/gimple-parser.c > +++ b/gcc/c/gimple-parser.c > @@ -115,6 +115,7 @@ static struct c_expr c_parser_gimple_postfix_expression_after_primary > static void c_parser_gimple_declaration (gimple_parser &); > static void c_parser_gimple_goto_stmt (gimple_parser &, location_t, > tree, gimple_seq *); > +static void c_parser_gimple_try_stmt (gimple_parser &, gimple_seq *); > static void c_parser_gimple_if_stmt (gimple_parser &, gimple_seq *); > static void c_parser_gimple_switch_stmt (gimple_parser &, gimple_seq *); > static void c_parser_gimple_return_stmt (gimple_parser &, gimple_seq *); > @@ -405,6 +406,9 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) > case CPP_KEYWORD: > switch (c_parser_peek_token (parser)->keyword) > { > + case RID_AT_TRY: > + c_parser_gimple_try_stmt (parser, seq); > + break; > case RID_IF: > c_parser_gimple_if_stmt (parser, seq); > break; > @@ -2088,6 +2092,51 @@ c_parser_gimple_paren_condition (gimple_parser &parser) > return cond; > } > > +/* Parse gimple try statement. > + > + try-statement: > + try { ... } finally { ... } > + try { ... } finally { ... } else { ... } > + > + This could support try/catch as well, but it's not implemented yet. > + */ > + > +static void > +c_parser_gimple_try_stmt (gimple_parser &parser, gimple_seq *seq) > +{ > + gimple_seq tryseq = NULL; > + c_parser_consume_token (parser); > + c_parser_gimple_compound_statement (parser, &tryseq); > + > + if (c_parser_next_token_is (parser, CPP_KEYWORD) > + && c_parser_peek_token (parser)->keyword == RID_AT_FINALLY) > + { > + gimple_seq finseq = NULL; > + c_parser_consume_token (parser); > + c_parser_gimple_compound_statement (parser, &finseq); > + > + if (c_parser_next_token_is (parser, CPP_KEYWORD) > + && c_parser_peek_token (parser)->keyword == RID_ELSE) > + { > + gimple_seq elsseq = NULL; > + c_parser_consume_token (parser); > + c_parser_gimple_compound_statement (parser, &finseq); > + > + geh_else *stmt = gimple_build_eh_else (finseq, elsseq); > + finseq = NULL; > + gimple_seq_add_stmt_without_update (&finseq, stmt); > + } > + > + gtry *stmt = gimple_build_try (tryseq, finseq, GIMPLE_TRY_FINALLY); > + gimple_seq_add_stmt_without_update (seq, stmt); > + } > + else if (c_parser_next_token_is (parser, CPP_KEYWORD) > + && c_parser_peek_token (parser)->keyword == RID_AT_CATCH) > + c_parser_error (parser, "%<catch%> is not supported"); > + else > + c_parser_error (parser, "expected %<finally%> or %<catch%>"); > +} > + > /* Parse gimple if-else statement. > > if-statement: > diff --git a/gcc/testsuite/gcc.dg/gimplefe-43.c b/gcc/testsuite/gcc.dg/gimplefe-43.c > new file mode 100644 > index 000000000000..c740e06a78e1 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/gimplefe-43.c > @@ -0,0 +1,13 @@ > +/* { dg-do compile } */ > +/* { dg-options "-fgimple" } */ > + > +void __GIMPLE foo() > +{ > + try { > + ; > + } finally { > + ; > + } else { > + ; > + } > +} > > > -- > Alexandre Oliva, freedom fighter he/him https://FSFLA.org/blogs/lxo > Be the change, be Free! FSF Latin America board member > GNU Toolchain Engineer Free Software Evangelist > Hay que enGNUrecerse, pero sin perder la terGNUra jamás - Che GNUevara ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: allow EH to escape from GIMPLE_EH_ELSE ELSE block 2019-07-04 10:54 ` Richard Biener @ 2019-07-11 10:44 ` Alexandre Oliva 2019-07-12 10:49 ` Richard Biener 2019-07-11 10:52 ` Alexandre Oliva 1 sibling, 1 reply; 12+ messages in thread From: Alexandre Oliva @ 2019-07-11 10:44 UTC (permalink / raw) To: Richard Biener; +Cc: GCC Patches On Jul 4, 2019, Richard Biener <richard.guenther@gmail.com> wrote: > Yeah. For other stuff we're simply looking at CPP_NAME and > string-matching, see c_parser_gimple_compound_statement > where you'd probably hook this into. Here's a working patch that introduces try/finally[/else] in gimplefe. Regstrapped on x86_64-linux-gnu. Ok to install? introduce try/finally/else in gimplefe for gcc/c/ChangeLog * gimple-parser.c (c_parser_gimple_try_stmt): New. (c_parser_compound_statement): Call it. for gcc/testsuite/ChangeLog * gcc.dg/gimplefe-43.c: New. --- gcc/c/gimple-parser.c | 61 ++++++++++++++++++++++++++++++++++++ gcc/testsuite/gcc.dg/gimplefe-43.c | 25 +++++++++++++++ 2 files changed, 86 insertions(+) create mode 100644 gcc/testsuite/gcc.dg/gimplefe-43.c diff --git a/gcc/c/gimple-parser.c b/gcc/c/gimple-parser.c index a0ea7215984a..4970ae1e9e08 100644 --- a/gcc/c/gimple-parser.c +++ b/gcc/c/gimple-parser.c @@ -117,6 +117,7 @@ static struct c_expr c_parser_gimple_postfix_expression_after_primary static void c_parser_gimple_declaration (gimple_parser &); static void c_parser_gimple_goto_stmt (gimple_parser &, location_t, tree, gimple_seq *); +static void c_parser_gimple_try_stmt (gimple_parser &, gimple_seq *); static void c_parser_gimple_if_stmt (gimple_parser &, gimple_seq *); static void c_parser_gimple_switch_stmt (gimple_parser &, gimple_seq *); static void c_parser_gimple_return_stmt (gimple_parser &, gimple_seq *); @@ -407,6 +408,9 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) case CPP_KEYWORD: switch (c_parser_peek_token (parser)->keyword) { + case RID_AT_TRY: + c_parser_gimple_try_stmt (parser, seq); + break; case RID_IF: c_parser_gimple_if_stmt (parser, seq); break; @@ -448,6 +452,14 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) c_parser_gimple_label (parser, seq); break; } + if (c_parser_next_token_is (parser, CPP_NAME) + && c_parser_peek_token (parser)->id_kind == C_ID_ID + && strcmp (IDENTIFIER_POINTER (c_parser_peek_token (parser)->value), + "try") == 0) + { + c_parser_gimple_try_stmt (parser, seq); + break; + } /* Basic block specification. __BB (index, ...) */ if ((cfun->curr_properties & PROP_cfg) @@ -2092,6 +2104,55 @@ c_parser_gimple_paren_condition (gimple_parser &parser) return cond; } +/* Parse gimple try statement. + + try-statement: + try { ... } finally { ... } + try { ... } finally { ... } else { ... } + + This could support try/catch as well, but it's not implemented yet. + */ + +static void +c_parser_gimple_try_stmt (gimple_parser &parser, gimple_seq *seq) +{ + gimple_seq tryseq = NULL; + c_parser_consume_token (parser); + c_parser_gimple_compound_statement (parser, &tryseq); + + if ((c_parser_next_token_is (parser, CPP_KEYWORD) + && c_parser_peek_token (parser)->keyword == RID_AT_FINALLY) + || (c_parser_next_token_is (parser, CPP_NAME) + && c_parser_peek_token (parser)->id_kind == C_ID_ID + && strcmp (IDENTIFIER_POINTER (c_parser_peek_token (parser)->value), + "finally") == 0)) + { + gimple_seq finseq = NULL; + c_parser_consume_token (parser); + c_parser_gimple_compound_statement (parser, &finseq); + + if (c_parser_next_token_is (parser, CPP_KEYWORD) + && c_parser_peek_token (parser)->keyword == RID_ELSE) + { + gimple_seq elsseq = NULL; + c_parser_consume_token (parser); + c_parser_gimple_compound_statement (parser, &elsseq); + + geh_else *stmt = gimple_build_eh_else (finseq, elsseq); + finseq = NULL; + gimple_seq_add_stmt_without_update (&finseq, stmt); + } + + gtry *stmt = gimple_build_try (tryseq, finseq, GIMPLE_TRY_FINALLY); + gimple_seq_add_stmt_without_update (seq, stmt); + } + else if (c_parser_next_token_is (parser, CPP_KEYWORD) + && c_parser_peek_token (parser)->keyword == RID_AT_CATCH) + c_parser_error (parser, "%<catch%> is not supported"); + else + c_parser_error (parser, "expected %<finally%> or %<catch%>"); +} + /* Parse gimple if-else statement. if-statement: diff --git a/gcc/testsuite/gcc.dg/gimplefe-43.c b/gcc/testsuite/gcc.dg/gimplefe-43.c new file mode 100644 index 000000000000..5fd66e6dfa5c --- /dev/null +++ b/gcc/testsuite/gcc.dg/gimplefe-43.c @@ -0,0 +1,25 @@ +/* { dg-do compile } */ +/* { dg-options "-fgimple" } */ + +void __GIMPLE foo() +{ + try + { + try + { + ; + } + finally + { + ; + } + else + { + ; + } + } + finally + { + ; + } +} -- Alexandre Oliva, freedom fighter he/him https://FSFLA.org/blogs/lxo Be the change, be Free! FSF Latin America board member GNU Toolchain Engineer Free Software Evangelist Hay que enGNUrecerse, pero sin perder la terGNUra jamás - Che GNUevara ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: allow EH to escape from GIMPLE_EH_ELSE ELSE block 2019-07-11 10:44 ` Alexandre Oliva @ 2019-07-12 10:49 ` Richard Biener 0 siblings, 0 replies; 12+ messages in thread From: Richard Biener @ 2019-07-12 10:49 UTC (permalink / raw) To: Alexandre Oliva; +Cc: GCC Patches On Thu, Jul 11, 2019 at 12:41 PM Alexandre Oliva <oliva@adacore.com> wrote: > > On Jul 4, 2019, Richard Biener <richard.guenther@gmail.com> wrote: > > > Yeah. For other stuff we're simply looking at CPP_NAME and > > string-matching, see c_parser_gimple_compound_statement > > where you'd probably hook this into. > > Here's a working patch that introduces try/finally[/else] in gimplefe. > Regstrapped on x86_64-linux-gnu. Ok to install? OK. Thanks a lot! Richard. > introduce try/finally/else in gimplefe > > for gcc/c/ChangeLog > > * gimple-parser.c (c_parser_gimple_try_stmt): New. > (c_parser_compound_statement): Call it. > > for gcc/testsuite/ChangeLog > > * gcc.dg/gimplefe-43.c: New. > --- > gcc/c/gimple-parser.c | 61 ++++++++++++++++++++++++++++++++++++ > gcc/testsuite/gcc.dg/gimplefe-43.c | 25 +++++++++++++++ > 2 files changed, 86 insertions(+) > create mode 100644 gcc/testsuite/gcc.dg/gimplefe-43.c > > diff --git a/gcc/c/gimple-parser.c b/gcc/c/gimple-parser.c > index a0ea7215984a..4970ae1e9e08 100644 > --- a/gcc/c/gimple-parser.c > +++ b/gcc/c/gimple-parser.c > @@ -117,6 +117,7 @@ static struct c_expr c_parser_gimple_postfix_expression_after_primary > static void c_parser_gimple_declaration (gimple_parser &); > static void c_parser_gimple_goto_stmt (gimple_parser &, location_t, > tree, gimple_seq *); > +static void c_parser_gimple_try_stmt (gimple_parser &, gimple_seq *); > static void c_parser_gimple_if_stmt (gimple_parser &, gimple_seq *); > static void c_parser_gimple_switch_stmt (gimple_parser &, gimple_seq *); > static void c_parser_gimple_return_stmt (gimple_parser &, gimple_seq *); > @@ -407,6 +408,9 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) > case CPP_KEYWORD: > switch (c_parser_peek_token (parser)->keyword) > { > + case RID_AT_TRY: > + c_parser_gimple_try_stmt (parser, seq); > + break; > case RID_IF: > c_parser_gimple_if_stmt (parser, seq); > break; > @@ -448,6 +452,14 @@ c_parser_gimple_compound_statement (gimple_parser &parser, gimple_seq *seq) > c_parser_gimple_label (parser, seq); > break; > } > + if (c_parser_next_token_is (parser, CPP_NAME) > + && c_parser_peek_token (parser)->id_kind == C_ID_ID > + && strcmp (IDENTIFIER_POINTER (c_parser_peek_token (parser)->value), > + "try") == 0) > + { > + c_parser_gimple_try_stmt (parser, seq); > + break; > + } > /* Basic block specification. > __BB (index, ...) */ > if ((cfun->curr_properties & PROP_cfg) > @@ -2092,6 +2104,55 @@ c_parser_gimple_paren_condition (gimple_parser &parser) > return cond; > } > > +/* Parse gimple try statement. > + > + try-statement: > + try { ... } finally { ... } > + try { ... } finally { ... } else { ... } > + > + This could support try/catch as well, but it's not implemented yet. > + */ > + > +static void > +c_parser_gimple_try_stmt (gimple_parser &parser, gimple_seq *seq) > +{ > + gimple_seq tryseq = NULL; > + c_parser_consume_token (parser); > + c_parser_gimple_compound_statement (parser, &tryseq); > + > + if ((c_parser_next_token_is (parser, CPP_KEYWORD) > + && c_parser_peek_token (parser)->keyword == RID_AT_FINALLY) > + || (c_parser_next_token_is (parser, CPP_NAME) > + && c_parser_peek_token (parser)->id_kind == C_ID_ID > + && strcmp (IDENTIFIER_POINTER (c_parser_peek_token (parser)->value), > + "finally") == 0)) > + { > + gimple_seq finseq = NULL; > + c_parser_consume_token (parser); > + c_parser_gimple_compound_statement (parser, &finseq); > + > + if (c_parser_next_token_is (parser, CPP_KEYWORD) > + && c_parser_peek_token (parser)->keyword == RID_ELSE) > + { > + gimple_seq elsseq = NULL; > + c_parser_consume_token (parser); > + c_parser_gimple_compound_statement (parser, &elsseq); > + > + geh_else *stmt = gimple_build_eh_else (finseq, elsseq); > + finseq = NULL; > + gimple_seq_add_stmt_without_update (&finseq, stmt); > + } > + > + gtry *stmt = gimple_build_try (tryseq, finseq, GIMPLE_TRY_FINALLY); > + gimple_seq_add_stmt_without_update (seq, stmt); > + } > + else if (c_parser_next_token_is (parser, CPP_KEYWORD) > + && c_parser_peek_token (parser)->keyword == RID_AT_CATCH) > + c_parser_error (parser, "%<catch%> is not supported"); > + else > + c_parser_error (parser, "expected %<finally%> or %<catch%>"); > +} > + > /* Parse gimple if-else statement. > > if-statement: > diff --git a/gcc/testsuite/gcc.dg/gimplefe-43.c b/gcc/testsuite/gcc.dg/gimplefe-43.c > new file mode 100644 > index 000000000000..5fd66e6dfa5c > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/gimplefe-43.c > @@ -0,0 +1,25 @@ > +/* { dg-do compile } */ > +/* { dg-options "-fgimple" } */ > + > +void __GIMPLE foo() > +{ > + try > + { > + try > + { > + ; > + } > + finally > + { > + ; > + } > + else > + { > + ; > + } > + } > + finally > + { > + ; > + } > +} > > > > -- > Alexandre Oliva, freedom fighter he/him https://FSFLA.org/blogs/lxo > Be the change, be Free! FSF Latin America board member > GNU Toolchain Engineer Free Software Evangelist > Hay que enGNUrecerse, pero sin perder la terGNUra jamás - Che GNUevara ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: allow EH to escape from GIMPLE_EH_ELSE ELSE block 2019-07-04 10:54 ` Richard Biener 2019-07-11 10:44 ` Alexandre Oliva @ 2019-07-11 10:52 ` Alexandre Oliva 2019-07-12 11:02 ` Richard Biener 1 sibling, 1 reply; 12+ messages in thread From: Alexandre Oliva @ 2019-07-11 10:52 UTC (permalink / raw) To: Richard Biener; +Cc: GCC Patches ... and here's a patch that uses a try/finally/else gimplefe test to demonstrate the GIMPLE_EH_ELSE lowering problem (might_throw3 is tagged as [LP 1] rather than [LP 2]), and fixes it. Regstrapped on x86_64-linux-gnu. Ok to install? allow EH to escape from GIMPLE_EH_ELSE ELSE block The only preexisting use of GIMPLE_EH_ELSE, for transactional memory commits, did not allow exceptions to escape from the ELSE path. The trick it uses to allow the ELSE path to see the propagating exception does not work very well if the exception cleanup raises further exceptions: the ELSE block is configured to handle exceptions in itself. This confuses the heck out of CFG and EH cleanups. Basing the lowering context for the ELSE block on outer_state, rather than this_state, gets us the expected enclosing handler. for gcc/ChangeLog * tree-eh.c (honor_protect_cleanup_actions): Use outer_ rather than this_state as the lowering context for the ELSE seq in a GIMPLE_EH_ELSE. for gcc/testsuite/ChangeLog * gcc.dg/gimplefe-44.c: New. --- gcc/testsuite/gcc.dg/gimplefe-44.c | 33 +++++++++++++++++++++++++++++++++ gcc/tree-eh.c | 13 ++++++++----- 2 files changed, 41 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/gimplefe-44.c diff --git a/gcc/testsuite/gcc.dg/gimplefe-44.c b/gcc/testsuite/gcc.dg/gimplefe-44.c new file mode 100644 index 000000000000..a9a92b1701ec --- /dev/null +++ b/gcc/testsuite/gcc.dg/gimplefe-44.c @@ -0,0 +1,33 @@ +/* { dg-do compile } */ +/* { dg-options "-fexceptions -fgimple -fdump-tree-eh-eh" } */ + +void __GIMPLE foo() +{ + try + { + try + { + extern void might_throw1 (); + might_throw1 (); + } + finally + { + extern void might_throw2 (); + might_throw2 (); + } + else + { + extern void might_throw3 (); + might_throw3 (); + } + } + finally + { + extern void might_throw4 (); + might_throw4 (); + } +} + +/* { dg-final { scan-tree-dump ".LP 1. might_throw1" "eh" } } */ +/* { dg-final { scan-tree-dump ".LP 2. might_throw2" "eh" } } */ +/* { dg-final { scan-tree-dump ".LP 2. might_throw3" "eh" } } */ diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c index fb7d202fc6f9..5bb07e49d285 100644 --- a/gcc/tree-eh.c +++ b/gcc/tree-eh.c @@ -996,11 +996,14 @@ honor_protect_cleanup_actions (struct leh_state *outer_state, gimple_try_set_cleanup (tf->top_p, gimple_eh_else_n_body (eh_else)); finally = gimple_eh_else_e_body (eh_else); - /* Let the ELSE see the exception that's being processed. */ - eh_region save_ehp = this_state->ehp_region; - this_state->ehp_region = this_state->cur_region; - lower_eh_constructs_1 (this_state, &finally); - this_state->ehp_region = save_ehp; + /* Let the ELSE see the exception that's being processed, but + since the cleanup is outside the try block, process it with + outer_state, otherwise it may be used as a cleanup for + itself, and Bad Things (TM) ensue. */ + eh_region save_ehp = outer_state->ehp_region; + outer_state->ehp_region = this_state->cur_region; + lower_eh_constructs_1 (outer_state, &finally); + outer_state->ehp_region = save_ehp; } else { -- Alexandre Oliva, freedom fighter he/him https://FSFLA.org/blogs/lxo Be the change, be Free! FSF Latin America board member GNU Toolchain Engineer Free Software Evangelist Hay que enGNUrecerse, pero sin perder la terGNUra jamás - Che GNUevara ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: allow EH to escape from GIMPLE_EH_ELSE ELSE block 2019-07-11 10:52 ` Alexandre Oliva @ 2019-07-12 11:02 ` Richard Biener 0 siblings, 0 replies; 12+ messages in thread From: Richard Biener @ 2019-07-12 11:02 UTC (permalink / raw) To: Alexandre Oliva; +Cc: GCC Patches On Thu, Jul 11, 2019 at 12:44 PM Alexandre Oliva <oliva@adacore.com> wrote: > > ... and here's a patch that uses a try/finally/else gimplefe test to > demonstrate the GIMPLE_EH_ELSE lowering problem (might_throw3 is tagged > as [LP 1] rather than [LP 2]), and fixes it. > > Regstrapped on x86_64-linux-gnu. Ok to install? OK. Richard. > > allow EH to escape from GIMPLE_EH_ELSE ELSE block > > The only preexisting use of GIMPLE_EH_ELSE, for transactional memory > commits, did not allow exceptions to escape from the ELSE path. The > trick it uses to allow the ELSE path to see the propagating exception > does not work very well if the exception cleanup raises further > exceptions: the ELSE block is configured to handle exceptions in > itself. This confuses the heck out of CFG and EH cleanups. > > Basing the lowering context for the ELSE block on outer_state, rather > than this_state, gets us the expected enclosing handler. > > > for gcc/ChangeLog > > * tree-eh.c (honor_protect_cleanup_actions): Use outer_ > rather than this_state as the lowering context for the ELSE > seq in a GIMPLE_EH_ELSE. > > for gcc/testsuite/ChangeLog > > * gcc.dg/gimplefe-44.c: New. > --- > gcc/testsuite/gcc.dg/gimplefe-44.c | 33 +++++++++++++++++++++++++++++++++ > gcc/tree-eh.c | 13 ++++++++----- > 2 files changed, 41 insertions(+), 5 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/gimplefe-44.c > > diff --git a/gcc/testsuite/gcc.dg/gimplefe-44.c b/gcc/testsuite/gcc.dg/gimplefe-44.c > new file mode 100644 > index 000000000000..a9a92b1701ec > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/gimplefe-44.c > @@ -0,0 +1,33 @@ > +/* { dg-do compile } */ > +/* { dg-options "-fexceptions -fgimple -fdump-tree-eh-eh" } */ > + > +void __GIMPLE foo() > +{ > + try > + { > + try > + { > + extern void might_throw1 (); > + might_throw1 (); > + } > + finally > + { > + extern void might_throw2 (); > + might_throw2 (); > + } > + else > + { > + extern void might_throw3 (); > + might_throw3 (); > + } > + } > + finally > + { > + extern void might_throw4 (); > + might_throw4 (); > + } > +} > + > +/* { dg-final { scan-tree-dump ".LP 1. might_throw1" "eh" } } */ > +/* { dg-final { scan-tree-dump ".LP 2. might_throw2" "eh" } } */ > +/* { dg-final { scan-tree-dump ".LP 2. might_throw3" "eh" } } */ > diff --git a/gcc/tree-eh.c b/gcc/tree-eh.c > index fb7d202fc6f9..5bb07e49d285 100644 > --- a/gcc/tree-eh.c > +++ b/gcc/tree-eh.c > @@ -996,11 +996,14 @@ honor_protect_cleanup_actions (struct leh_state *outer_state, > gimple_try_set_cleanup (tf->top_p, gimple_eh_else_n_body (eh_else)); > finally = gimple_eh_else_e_body (eh_else); > > - /* Let the ELSE see the exception that's being processed. */ > - eh_region save_ehp = this_state->ehp_region; > - this_state->ehp_region = this_state->cur_region; > - lower_eh_constructs_1 (this_state, &finally); > - this_state->ehp_region = save_ehp; > + /* Let the ELSE see the exception that's being processed, but > + since the cleanup is outside the try block, process it with > + outer_state, otherwise it may be used as a cleanup for > + itself, and Bad Things (TM) ensue. */ > + eh_region save_ehp = outer_state->ehp_region; > + outer_state->ehp_region = this_state->cur_region; > + lower_eh_constructs_1 (outer_state, &finally); > + outer_state->ehp_region = save_ehp; > } > else > { > > > -- > Alexandre Oliva, freedom fighter he/him https://FSFLA.org/blogs/lxo > Be the change, be Free! FSF Latin America board member > GNU Toolchain Engineer Free Software Evangelist > Hay que enGNUrecerse, pero sin perder la terGNUra jamás - Che GNUevara ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2019-07-12 10:49 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-06-27 8:17 allow EH to escape from GIMPLE_EH_ELSE ELSE block Alexandre Oliva 2019-06-27 9:36 ` Richard Biener 2019-06-28 9:43 ` Alexandre Oliva 2019-07-01 11:19 ` Richard Biener 2019-07-02 8:51 ` Alexandre Oliva 2019-07-02 10:11 ` Richard Biener 2019-07-04 8:50 ` Alexandre Oliva 2019-07-04 10:54 ` Richard Biener 2019-07-11 10:44 ` Alexandre Oliva 2019-07-12 10:49 ` Richard Biener 2019-07-11 10:52 ` Alexandre Oliva 2019-07-12 11:02 ` Richard Biener
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).