Re: [PATCH][RFC] Fix PR59860

[Jakub Jelinek <jakub at redhat dot com>]

On Sun, Jan 19, 2014 at 07:05:12PM +0100, Richard Biener wrote:
> The following patch fixes PR59860 by removing the only folding
> builtins.c does that can end up recursing to GIMPLE call stmt
> folding.  It does that via strcat -> strlen + strcpy folding
> and then folding the strlen gimple stmt via gimplify which
> then can use the SSA web to fold that to a constant and then
> the strcpy call to memcpy.  This confuses the virtual operand
> updating code - not that it ends up creating wrong virtual SSA
> form but by bogously marking virtual operands for renaming
> through the operand scanner as the folding on the just gimplified
> sequence doesn't see any VOPs yet.
> 
> Bootstrapped on x86_64-unknown-linux-gnu, testing reveals that I
> have to adjust gcc.c-torture/execute/builtins/strcat.c expectations
> and gcc.dg/strlenopt-* likely because of different input.
> 
> I still think this patch is better than the second option, avoiding
> to call fold_stmt from the gimplifier in this case (either by
> a new ctx flag or looking at ctx->into_ssa).  I also think that
> "fixing" this by scheduling update-ssa after objsz is wrong.
> 
> Any opinions?  Maybe any different preferences for branch / trunk?

If you verify that strlen pass does the right thing here (I hope it does,
but haven't verified), then I think this is the way for the trunk, not
sure about the branch, I'd prefer there something less intrusive at this
point, even if it is say just through setting some global flag that will
disable the strcat folding at the problematic spot, or folds it on the tree
before gimplification, or gimplifies operands and builds the call in gimple
by hand for the strcat case.

> 2014-01-17  Richard Biener  <rguenther@suse.de>
> 
> 	PR middle-end/59860
> 	* builtins.c (fold_builtin_strcat): Remove case better handled
> 	by tree-ssa-strlen.c

Missing dot at the end ;)
> 
> 	* gcc.dg/pr59860.c: New testcase.
> 

	Jakub