On Sun, 19 Jan 2014, Jakub Jelinek wrote:
> 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 <rguent...@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.
> >
Ok, the following simpler patch also fixes the issue and causes
no testsuite fallout - we restrict the folding in builtins.c to
the case with known src strlen.
Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk
and branch.
Richard.
2014-01-17 Richard Biener <rguent...@suse.de>
PR middle-end/59860
* builtins.c (fold_builtin_strcat): Remove case better handled
by tree-ssa-strlen.c.
* gcc.dg/pr59860.c: New testcase.
Index: gcc/testsuite/gcc.dg/pr59860.c
===================================================================
*** gcc/testsuite/gcc.dg/pr59860.c (revision 0)
--- gcc/testsuite/gcc.dg/pr59860.c (working copy)
***************
*** 0 ****
--- 1,15 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O" } */
+
+ extern __inline __attribute__ ((__always_inline__)) __attribute__
((__gnu_inline__)) __attribute__ ((__artificial__)) char * __attribute__
((__nothrow__ , __leaf__))
+ strcat (char *__restrict __dest, const char *__restrict __src)
+ {
+ return __builtin___strcat_chk (__dest, __src, __builtin_object_size
(__dest, 2 > 1));
+ }
+ static char raw_decode;
+ void foo (char **argv, char *outfilename)
+ {
+ if (**argv == 'r')
+ raw_decode = 1;
+ strcat (outfilename, raw_decode ? ".raw" : ".wav");
+ }
Index: gcc/builtins.c
===================================================================
*** gcc/builtins.c (revision 206772)
--- gcc/builtins.c (working copy)
*************** fold_builtin_strcat (location_t loc ATTR
*** 11759,11775 ****
if (!strlen_fn || !strcpy_fn)
return NULL_TREE;
! /* If we don't have a movstr we don't want to emit an strcpy
! call. We have to do that if the length of the source string
! isn't computable (in that case we can use memcpy probably
! later expanding to a sequence of mov instructions). If we
! have movstr instructions we can emit strcpy calls. */
! if (!HAVE_movstr)
! {
! tree len = c_strlen (src, 1);
! if (! len || TREE_SIDE_EFFECTS (len))
! return NULL_TREE;
! }
/* Stabilize the argument list. */
dst = builtin_save_expr (dst);
--- 11759,11769 ----
if (!strlen_fn || !strcpy_fn)
return NULL_TREE;
! /* If the length of the source string isn't computable don't
! split strcat into strlen and strcpy. */
! tree len = c_strlen (src, 1);
! if (! len || TREE_SIDE_EFFECTS (len))
! return NULL_TREE;
/* Stabilize the argument list. */
dst = builtin_save_expr (dst);
