Re: [PATCH] Fix pattern causing C_MAYBE_CONST_EXPRs leak into gimplifier (PR c/68513)

[Jakub Jelinek <jakub at redhat dot com>]

On Wed, Nov 25, 2015 at 03:35:10PM +0100, Marek Polacek wrote:
> Look how 'x' is duplicated in the result of the pattern, and since (because of
> the volatile 'e') it has TREE_SIDE_EFFECTS, we need to wrap it in a SAVE_EXPR,
> which means the convert() produces
> 
> (short int) (((SAVE_EXPR <c_maybe_const_expr <i> != 0 ? (unsigned short)
>                c_maybe_const_expr <*e > : 0>)
>               ^ (unsigned short) i) & (unsigned short) u
>              ^ (SAVE_EXPR <c_maybe_const_expr <i > != 0 ? (unsigned short)
>                 c_maybe_const_expr <*e > : 0>))
> 
> What's important is that we end up with a C_MAYBE_CONST_EXPR in a SAVE_EXPR.
> 
> Down the line, we get into c_process_expr_stmt, where there's
> 
>   expr = c_fully_fold (expr, false, NULL);
> 
> so we try to fully-fold the whole expression.  However, c_fully_fold just
> gives up when it sees a SAVE_EXPR, so it doesn't scrub out C_MAYBE_CONST_EXPR.
> It then leaks into the gimplifier and crashes.

Ran into similar issues many times in the past, the main issue is that the C
FE wants that before c_fully_fold is called on some expression that nobody
wraps anything into a SAVE_EXPR except by calling c_save_expr.
So, unless we get rid of that (severe) limitation, we should make sure
that either match.pd never creates SAVE_EXPRs when called from the "early"
spots in the C FE, or that it uses c_save_expr instead of save_expr.
But finding out if you have an expression on which c_fully_fold has been
already called or not is non-trivial too.

Wonder if we couldn't use some FE specific bit on the SAVE_EXPR to say
whether c_fully_fold_internal has already processed it or not, and just
get rid of c_save_expr, in c_fully_fold* recurse into SAVE_EXPRs too, but
only if that bit is not yet already set, and set it afterwards.

	Jakub