On Mon, Dec 12, 2011 at 6:01 PM, Jakub Jelinek <ja...@redhat.com> wrote:
> Hi!
>
> In gimple_fold_call (called from fold_stmt from replace_uses_by from cfg
> cleanup) we weren't calling maybe_clean_or_replace_eh_stmt, so when
> we've replaced a printf call (which can throw) with puts (which can throw
> too), nothing would update EH stmts. It would be problematic calling
> gimple_purge_dead_eh_edges, because callers might be surprised by that,
> especially when this happens during cfg cleanup, so instead I just assert
> it is not needed and don't try to fold if a throwing stmt would be replaced
> by non-throwing. FAB pass can handle that instead. No folding
> has been actually disabled because of that check during bootstrap/regtest,
> so it is there just in case.
I think that all callers of fold_stmt are supposed to handle EH
updating themselves, similar to how they are supposed to
call update_stmt. I see that replace_uses_by does call
maybe_clean_or_replace_eh_stmt - are you sure it is this path
the issue triggers on?
Richard.
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2011-12-12 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/51481
> * gimple-fold.c (gimple_fold_call): Call
> maybe_clean_or_replace_eh_stmt. Avoid optimization if stmt has EH
> edges, but gimple_fold_builtin result can't throw.
>
> * gcc.dg/pr51481.c: New test.
>
> --- gcc/gimple-fold.c.jj 2011-12-11 22:02:37.000000000 +0100
> +++ gcc/gimple-fold.c 2011-12-12 11:42:34.740168390 +0100
> @@ -1117,10 +1117,21 @@ gimple_fold_call (gimple_stmt_iterator *
> if (callee && DECL_BUILT_IN (callee))
> {
> tree result = gimple_fold_builtin (stmt);
> - if (result)
> + if (result
> + /* Disallow EH edge removal here. We can't call
> + gimple_purge_dead_eh_edges here. */
> + && (lookup_stmt_eh_lp (stmt) == 0
> + || tree_could_throw_p (result)))
> {
> if (!update_call_from_tree (gsi, result))
> gimplify_and_update_call_from_tree (gsi, result);
> + if (!gsi_end_p (*gsi))
> + {
> + gimple new_stmt = gsi_stmt (*gsi);
> + bool update_eh ATTRIBUTE_UNUSED
> + = maybe_clean_or_replace_eh_stmt (stmt, new_stmt);
> + gcc_assert (!update_eh);
> + }
> changed = true;
> }
> }
> --- gcc/testsuite/gcc.dg/pr51481.c.jj 2011-12-12 11:18:27.304678207 +0100
> +++ gcc/testsuite/gcc.dg/pr51481.c 2011-12-12 11:18:02.000000000 +0100
> @@ -0,0 +1,33 @@
> +/* PR tree-optimization/51481 */
> +/* { dg-do compile } */
> +/* { dg-options "-O -fexceptions -fipa-cp -fipa-cp-clone" } */
> +
> +extern const unsigned short int **foo (void)
> + __attribute__ ((__nothrow__, __const__));
> +struct S { unsigned short s1; int s2; };
> +extern struct S *s[26];
> +
> +void
> +bar (int x, struct S *y, ...)
> +{
> + static struct S *t;
> + __builtin_va_list ap;
> + __builtin_va_start (ap, y);
> + if (t != s[7])
> + {
> + const char *p = "aAbBc";
> + t = s[7];
> + while ((*foo ())[(unsigned char) *p])
> + p++;
> + }
> + __builtin_printf (x == 0 ? "abc\n" : "def\n");
> + if (y != 0)
> + __builtin_printf ("ghi %d %d", y->s2, y->s1);
> + __builtin_va_end (ap);
> +}
> +
> +void
> +baz (char *x)
> +{
> + bar (1, 0, x);
> +}
>
> Jakub
