Re: RFA: Backport fix for PR80769

[Richard Biener <richard dot guenther at gmail dot com>]

On Thu, Jul 27, 2017 at 11:36 PM, Richard Sandiford
<richard.sandiford@linaro.org> wrote:
> This is a minimal-ish backport of the fix for PR80769.  The trunk version
> also replaced open-coded instances of get_next_strinfo with calls to the
> new function.  It also added asserts in various other places to try to
> ensure that related strinfos were consistently delayed or not delayed.
>
> Tested on aarch64-linux-gnu and x86_64-linux-gnu.  OK for gcc-7-branch?
> (And OK for gcc-6-branch if the same patch passes testing there?)

Ok.

Thanks,
Richard.

> Richard
>
>
> 2017-07-27  Richard Sandiford  <richard.sandiford@linaro.org>
>
> gcc/
>         PR tree-optimization/80769
>         * tree-ssa-strlen.c (strinfo): Document that "stmt" is also used
>         for malloc and calloc.  Document the new invariant that all related
>         strinfos have delayed lengths or none do.
>         (get_next_strinfo): New function.
>         (verify_related_strinfos): Move earlier in file.
>         (set_endptr_and_length): New function, split out from...
>         (get_string_length): ...here.  Also set the lengths of related
>         strinfos.
>
> gcc/testsuite/
>         PR tree-optimization/80769
>         * gcc.dg/strlenopt-31.c: New test.
>         * gcc.dg/strlenopt-31g.c: Likewise.
>
> Index: gcc/tree-ssa-strlen.c
> ===================================================================
> --- gcc/tree-ssa-strlen.c       2017-05-10 18:04:24.514775477 +0100
> +++ gcc/tree-ssa-strlen.c       2017-07-27 18:21:20.308966958 +0100
> @@ -61,7 +61,13 @@ struct strinfo
>    tree length;
>    /* Any of the corresponding pointers for querying alias oracle.  */
>    tree ptr;
> -  /* Statement for delayed length computation.  */
> +  /* This is used for two things:
> +
> +     - To record the statement that should be used for delayed length
> +       computations.  We maintain the invariant that all related strinfos
> +       have delayed lengths or none do.
> +
> +     - To record the malloc or calloc call that produced this result.  */
>    gimple *stmt;
>    /* Pointer to '\0' if known, if NULL, it can be computed as
>       ptr + length.  */
> @@ -156,6 +162,19 @@ get_strinfo (int idx)
>    return (*stridx_to_strinfo)[idx];
>  }
>
> +/* Get the next strinfo in the chain after SI, or null if none.  */
> +
> +static inline strinfo *
> +get_next_strinfo (strinfo *si)
> +{
> +  if (si->next == 0)
> +    return NULL;
> +  strinfo *nextsi = get_strinfo (si->next);
> +  if (nextsi == NULL || nextsi->first != si->first || nextsi->prev != si->idx)
> +    return NULL;
> +  return nextsi;
> +}
> +
>  /* Helper function for get_stridx.  */
>
>  static int
> @@ -438,6 +457,45 @@ set_strinfo (int idx, strinfo *si)
>    (*stridx_to_strinfo)[idx] = si;
>  }
>
> +/* Return the first strinfo in the related strinfo chain
> +   if all strinfos in between belong to the chain, otherwise NULL.  */
> +
> +static strinfo *
> +verify_related_strinfos (strinfo *origsi)
> +{
> +  strinfo *si = origsi, *psi;
> +
> +  if (origsi->first == 0)
> +    return NULL;
> +  for (; si->prev; si = psi)
> +    {
> +      if (si->first != origsi->first)
> +       return NULL;
> +      psi = get_strinfo (si->prev);
> +      if (psi == NULL)
> +       return NULL;
> +      if (psi->next != si->idx)
> +       return NULL;
> +    }
> +  if (si->idx != si->first)
> +    return NULL;
> +  return si;
> +}
> +
> +/* Set SI's endptr to ENDPTR and compute its length based on SI->ptr.
> +   Use LOC for folding.  */
> +
> +static void
> +set_endptr_and_length (location_t loc, strinfo *si, tree endptr)
> +{
> +  si->endptr = endptr;
> +  si->stmt = NULL;
> +  tree start_as_size = fold_convert_loc (loc, size_type_node, si->ptr);
> +  tree end_as_size = fold_convert_loc (loc, size_type_node, endptr);
> +  si->length = fold_build2_loc (loc, MINUS_EXPR, size_type_node,
> +                               end_as_size, start_as_size);
> +}
> +
>  /* Return string length, or NULL if it can't be computed.  */
>
>  static tree
> @@ -533,12 +591,12 @@ get_string_length (strinfo *si)
>         case BUILT_IN_STPCPY_CHK_CHKP:
>           gcc_assert (lhs != NULL_TREE);
>           loc = gimple_location (stmt);
> -         si->endptr = lhs;
> -         si->stmt = NULL;
> -         lhs = fold_convert_loc (loc, size_type_node, lhs);
> -         si->length = fold_convert_loc (loc, size_type_node, si->ptr);
> -         si->length = fold_build2_loc (loc, MINUS_EXPR, size_type_node,
> -                                       lhs, si->length);
> +         set_endptr_and_length (loc, si, lhs);
> +         for (strinfo *chainsi = verify_related_strinfos (si);
> +              chainsi != NULL;
> +              chainsi = get_next_strinfo (chainsi))
> +           if (chainsi->length == NULL)
> +             set_endptr_and_length (loc, chainsi, lhs);
>           break;
>         case BUILT_IN_MALLOC:
>           break;
> @@ -607,32 +665,6 @@ unshare_strinfo (strinfo *si)
>    return nsi;
>  }
>
> -/* Return first strinfo in the related strinfo chain
> -   if all strinfos in between belong to the chain, otherwise
> -   NULL.  */
> -
> -static strinfo *
> -verify_related_strinfos (strinfo *origsi)
> -{
> -  strinfo *si = origsi, *psi;
> -
> -  if (origsi->first == 0)
> -    return NULL;
> -  for (; si->prev; si = psi)
> -    {
> -      if (si->first != origsi->first)
> -       return NULL;
> -      psi = get_strinfo (si->prev);
> -      if (psi == NULL)
> -       return NULL;
> -      if (psi->next != si->idx)
> -       return NULL;
> -    }
> -  if (si->idx != si->first)
> -    return NULL;
> -  return si;
> -}
> -
>  /* Attempt to create a new strinfo for BASESI + OFF, or find existing
>     strinfo if there is any.  Return it's idx, or 0 if no strinfo has
>     been created.  */
> Index: gcc/testsuite/gcc.dg/strlenopt-31.c
> ===================================================================
> --- /dev/null   2017-07-27 17:06:14.642985039 +0100
> +++ gcc/testsuite/gcc.dg/strlenopt-31.c 2017-07-27 18:11:20.158804210 +0100
> @@ -0,0 +1,25 @@
> +/* { dg-do run } */
> +/* { dg-options "-O2" } */
> +
> +#include "strlenopt.h"
> +
> +__attribute__((noinline, noclone)) int
> +bar (char *p1, const char *q)
> +{
> +  strcpy (p1, "abcde");
> +  char *p2 = strchr (p1, '\0');
> +  strcpy (p2, q);
> +  char *p3 = strchr (p2, '\0');
> +  memcpy (p3, "x", 2);
> +  return strlen (p1);
> +}
> +
> +int
> +main (void)
> +{
> +  char buffer[10];
> +  int res = bar (buffer, "foo");
> +  if (strcmp (buffer, "abcdefoox") != 0 || res != 9)
> +    abort ();
> +  return 0;
> +}
> Index: gcc/testsuite/gcc.dg/strlenopt-31g.c
> ===================================================================
> --- /dev/null   2017-07-27 17:06:14.642985039 +0100
> +++ gcc/testsuite/gcc.dg/strlenopt-31g.c        2017-07-27 18:11:20.158804210 +0100
> @@ -0,0 +1,9 @@
> +/* { dg-do run { target *-*-linux* *-*-gnu* } } */
> +/* { dg-options "-O2 -fdump-tree-strlen" } */
> +
> +#define USE_GNU
> +#include "strlenopt-31.c"
> +
> +/* { dg-final { scan-tree-dump-times "stpcpy \\(" 1 "strlen" } } */
> +/* { dg-final { scan-tree-dump-times "memcpy \\(" 2 "strlen" } } */
> +/* { dg-final { scan-tree-dump-not "strlen \\(" "strlen" } } */