On Wed, 22 Nov 2023, Jakub Jelinek wrote:
> Hi!
>
> The following testcase ICEs with -std=c++98 since r14-5086 because
> block_may_fallthru is called on a TRY_CATCH_EXPR whose second operand
> is a MODIFY_EXPR rather than STATEMENT_LIST, which try_catch_may_fallthru
> apparently expects.
> I've been wondering whether that isn't some kind of FE bug and whether
> there isn't some unwritten rule that second operand of TRY_CATCH_EXPR
> must be a STATEMENT_LIST, but then I tried
> --- gcc/gimplify.cc 2023-07-19 14:23:42.409875238 +0200
> +++ gcc/gimplify.cc 2023-11-22 11:07:50.511000206 +0100
> @@ -16730,6 +16730,10 @@ gimplify_expr (tree *expr_p, gimple_seq
> Note that this only affects the destructor calls in FINALLY/CATCH
> block, and will automatically reset to its original value by the
> end of gimplify_expr. */
> + if (TREE_CODE (*expr_p) == TRY_CATCH_EXPR
> + && TREE_OPERAND (*expr_p, 1)
> + && TREE_CODE (TREE_OPERAND (*expr_p, 1)) != STATEMENT_LIST)
> + gcc_unreachable ();
> input_location = UNKNOWN_LOCATION;
> eval = cleanup = NULL;
> gimplify_and_add (TREE_OPERAND (*expr_p, 0), &eval);
> hack in gcc 13 and triggered on hundreds of tests there within just 5
> seconds of running make check-g++ -j32 (and in cases I looked at had nothing
> to do with the r14-5086 backports), so I believe this is just bad
> assumption on the try_catch_may_fallthru side, gimplify.cc certainly doesn't
> care, it just calls gimplify_and_add (TREE_OPERAND (*expr_p, 1), &cleanup);
> on it. So, IMHO non-STATEMENT_LIST in the second operand is equivalent to
> a STATEMENT_LIST containing a single statement.
Did you check if there's ever a CATCH_EXPR or EH_FILTER_EXPR not wrapped
inside a STATEMENT_LIST? That is, does
if (TREE_CODE (TREE_OPERAND (stmt, 1)) != STATEMENT_LIST)
{
gcc_checking_assert (code != CATCH_EXPR && code != EH_FILTER_EXPR);
return false;
}
work?
> Unfortunately, I don't see an easy way to create an artificial tree iterator
> from just a single tree statement, so the patch duplicates what the loops
> later do (after all, it is very simple, just didn't want to duplicate
> also the large comments explaning it, so the 3 See below. comments).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> Shall it go to branches as well given that r14-5086 has been backported
> to those branches as well?
>
> 2023-11-22 Jakub Jelinek <ja...@redhat.com>
>
> PR c++/112619
> * tree.cc (try_catch_may_fallthru): If second operand of
> TRY_CATCH_EXPR is not a STATEMENT_LIST, handle it as if it was a
> STATEMENT_LIST containing a single statement.
>
> * g++.dg/eh/pr112619.C: New test.
>
> --- gcc/tree.cc.jj 2023-11-14 09:24:28.436530018 +0100
> +++ gcc/tree.cc 2023-11-21 19:19:19.384347469 +0100
> @@ -12573,6 +12573,24 @@ try_catch_may_fallthru (const_tree stmt)
> if (block_may_fallthru (TREE_OPERAND (stmt, 0)))
> return true;
>
> + switch (TREE_CODE (TREE_OPERAND (stmt, 1)))
> + {
> + case CATCH_EXPR:
> + /* See below. */
> + return block_may_fallthru (CATCH_BODY (TREE_OPERAND (stmt, 1)));
> +
> + case EH_FILTER_EXPR:
> + /* See below. */
> + return block_may_fallthru (EH_FILTER_FAILURE (TREE_OPERAND (stmt, 1)));
> +
> + case STATEMENT_LIST:
> + break;
> +
> + default:
> + /* See below. */
> + return false;
> + }
> +
> i = tsi_start (TREE_OPERAND (stmt, 1));
> switch (TREE_CODE (tsi_stmt (i)))
> {
> --- gcc/testsuite/g++.dg/eh/pr112619.C.jj 2023-11-21 19:22:47.437439283
> +0100
> +++ gcc/testsuite/g++.dg/eh/pr112619.C 2023-11-21 19:22:24.887754376
> +0100
> @@ -0,0 +1,15 @@
> +// PR c++/112619
> +// { dg-do compile }
> +
> +struct S { S (); ~S (); };
> +
> +S
> +foo (int a, int b)
> +{
> + if (a || b)
> + {
> + S s;
> + return s;
> + }
> + return S ();
> +}
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
