| Message ID | 87y3r9tvek.fsf@linaro.org |
|---|---|
| State | New |
| Headers | show |
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" } } */
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" } } */