allow EH to escape from GIMPLE_EH_ELSE ELSE block

allow EH to escape from GIMPLE_EH_ELSE ELSE block

[Alexandre Oliva]

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

Re: allow EH to escape from GIMPLE_EH_ELSE ELSE block

[Richard Biener]

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

Re: allow EH to escape from GIMPLE_EH_ELSE ELSE block

[Alexandre Oliva]

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, &current_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, &current_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

Re: allow EH to escape from GIMPLE_EH_ELSE ELSE block

[Richard Biener]

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, &current_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, &current_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

Re: allow EH to escape from GIMPLE_EH_ELSE ELSE block

[Alexandre Oliva]

[-- 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

Re: allow EH to escape from GIMPLE_EH_ELSE ELSE block

[Richard Biener]

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

Re: allow EH to escape from GIMPLE_EH_ELSE ELSE block

[Alexandre Oliva]

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

Re: allow EH to escape from GIMPLE_EH_ELSE ELSE block

[Richard Biener]

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

Re: allow EH to escape from GIMPLE_EH_ELSE ELSE block

[Alexandre Oliva]

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

Re: allow EH to escape from GIMPLE_EH_ELSE ELSE block

[Alexandre Oliva]

... 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

Re: allow EH to escape from GIMPLE_EH_ELSE ELSE block

[Richard Biener]

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

Re: allow EH to escape from GIMPLE_EH_ELSE ELSE block

[Richard Biener]

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