This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix pattern causing C_MAYBE_CONST_EXPRs leak into gimplifier (PR c/68513)
- From: Richard Biener <rguenther at suse dot de>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: Marek Polacek <polacek at redhat dot com>, "Joseph S. Myers" <joseph at codesourcery dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 25 Nov 2015 15:56:39 +0100 (CET)
- Subject: Re: [PATCH] Fix pattern causing C_MAYBE_CONST_EXPRs leak into gimplifier (PR c/68513)
- Authentication-results: sourceware.org; auth=none
- References: <20151125143509 dot GU21807 at redhat dot com> <20151125144620 dot GY5675 at tucnak dot redhat dot com> <alpine dot LSU dot 2 dot 11 dot 1511251555140 dot 4884 at t29 dot fhfr dot qr>
On Wed, 25 Nov 2015, Richard Biener wrote:
> On Wed, 25 Nov 2015, Jakub Jelinek wrote:
>
> > 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.
>
> c_gimplify_expr and SAVE_EXPR_RESOLVED_P would work as well I guess?
Or simply not call fold () from convert () -> convert_to_integer ().
Richard.