Re: [PATCH] fix a couple of bugs in const string folding (PR 86532)

[Martin Sebor <msebor at gmail dot com>]

On 07/17/2018 09:38 AM, Jeff Law wrote:
> On 07/17/2018 09:19 AM, Martin Sebor wrote:
> > My enhancement to extract constant strings out of complex
> > aggregates committed last week introduced a couple of bugs in
> > dealing with non-constant indices and offsets.  One of the bugs
> > was fixed earlier today (PR 86528) but another one remains.  It
> > causes strlen (among other functions) to incorrectly fold
> > expressions involving a non-constant index into an array of
> > strings by treating the index the same as a non-consatnt
> > offset into it.
> >
> > The non-constant index should either prevent the folding, or it
> > needs to handle it differently from an offset.
> >
> > The attached patch takes the conservative approach of avoiding
> > the folding in this case.  The remaining changes deal with
> > the fallout from the fix.
> >
> > Tested on x86_64-linux.
> >
> > Martin
> >
> >
> > gcc-86532.diff
> >
> >
> > PR tree-optimization/86532 - Wrong code due to a wrong strlen folding starting with r262522
> >
> > gcc/ChangeLog:
> >
> > 	PR tree-optimization/86532
> > 	* builtins.c (c_strlen): Correct handling of non-constant offsets.	
> > 	(check_access): Be prepared for non-constant length ranges.
> > 	* expr.c (string_constant): Only handle the minor non-constant
> > 	array index.
> >
> > gcc/testsuite/ChangeLog:
> >
> > 	PR tree-optimization/86532
> > 	* gcc.c-torture/execute/strlen-2.c: New test.
>
> [ ... ]
>
> > Index: gcc/expr.c
> > ===================================================================
> > --- gcc/expr.c	(revision 262764)
> > +++ gcc/expr.c	(working copy)
>
> > @@ -11343,16 +11356,15 @@ string_constant (tree arg, tree *ptr_offset)
> >      {
> >        if (TREE_CODE (TREE_TYPE (array)) != ARRAY_TYPE)
> >  	return NULL_TREE;
> > -      if (tree eltsize = TYPE_SIZE_UNIT (TREE_TYPE (TREE_TYPE (array))))
> > -	{
> > -	  /* Add the scaled variable index to the constant offset.  */
> > -	  tree eltoff = fold_build2 (MULT_EXPR, TREE_TYPE (offset),
> > -				     fold_convert (sizetype, varidx),
> > -				     eltsize);
> > -	  offset = fold_build2 (PLUS_EXPR, TREE_TYPE (offset), offset, eltoff);
> > -	}
> > -      else
> > -	return NULL_TREE;
> > +
> > +      while (TREE_CODE (chartype) != INTEGER_TYPE)
> > +	chartype = TREE_TYPE (chartype);
> This is a bit concerning.  First under what conditions is chartype not
> going to be an INTEGER_TYPE?  And under what conditions will extracting
> its type ultimately lead to something that is an INTEGER_TYPE?

chartype is usually (maybe even always) pointer type here:

  const char a[] = "123";
  extern int i;
  n = strlen (&a[i]);

Martin