On Tue, 26 Feb 2019, Jakub Jelinek wrote: > On Mon, Feb 25, 2019 at 04:55:56PM -0700, Martin Sebor wrote: > > Storing the strnlen result is intentional when it equals strlen. > > For the *si update, you never want to do that for strnlen. > And for the strlen_to_stridx, while I don't think it is that important, > here is an updated patch that handles it, plus it handles strnlen (x, 0) > even if we don't know anything about x and handles strnlen (p, cst) > even if we know the first cst bytes are non-zero, but don't know anything > about zero termination. > > Ok for trunk if it passes bootstrap/regtest?
OK. Richard. > 2019-02-26 Jakub Jelinek <ja...@redhat.com> > > PR tree-optimization/89500 > * tree-ssa-strlen.c (stridx_strlenloc): Adjust comment. > (handle_builtin_strlen): Remove noncst_bound variable. Always > optimize strnlen (x, 0) to 0. Optimize strnlen (x, cst) to > cst if the first cst bytes starting at x are known to be non-zero, > even if the string is not zero terminated. Don't try to modify > *si for strnlen. Update strlen_to_stridx only for strlen or if > we can prove strnlen returns the same value as strlen would. > > * gcc.dg/pr89500.c: New test. > * gcc.dg/Wstringop-overflow-10.c: New test. > * gcc.dg/strlenopt-60.c: New test. > > --- gcc/tree-ssa-strlen.c.jj 2019-02-25 23:56:55.033106702 +0100 > +++ gcc/tree-ssa-strlen.c 2019-02-26 13:53:35.163161757 +0100 > @@ -156,7 +156,8 @@ struct decl_stridxlist_map > mappings. */ > static hash_map<tree_decl_hash, stridxlist> *decl_to_stridxlist_htab; > > -/* Hash table mapping strlen calls to stridx instances describing > +/* Hash table mapping strlen (or strnlen with constant bound and return > + smaller than bound) calls to stridx instances describing > the calls' arguments. Non-null only when warn_stringop_truncation > is non-zero. */ > typedef std::pair<int, location_t> stridx_strlenloc; > @@ -1269,19 +1270,33 @@ handle_builtin_strlen (gimple_stmt_itera > tree bound = (DECL_FUNCTION_CODE (callee) == BUILT_IN_STRNLEN > ? gimple_call_arg (stmt, 1) : NULL_TREE); > int idx = get_stridx (src); > - if (idx) > + if (idx || (bound && integer_zerop (bound))) > { > strinfo *si = NULL; > tree rhs; > > if (idx < 0) > rhs = build_int_cst (TREE_TYPE (lhs), ~idx); > + else if (idx == 0) > + rhs = bound; > else > { > rhs = NULL_TREE; > si = get_strinfo (idx); > if (si != NULL) > - rhs = get_string_length (si); > + { > + rhs = get_string_length (si); > + /* For strnlen, if bound is constant, even if si is not known > + to be zero terminated, if we know at least bound bytes are > + not zero, the return value will be bound. */ > + if (rhs == NULL_TREE > + && bound != NULL_TREE > + && TREE_CODE (bound) == INTEGER_CST > + && si->nonzero_chars != NULL_TREE > + && TREE_CODE (si->nonzero_chars) == INTEGER_CST > + && tree_int_cst_le (bound, si->nonzero_chars)) > + rhs = bound; > + } > } > if (rhs != NULL_TREE) > { > @@ -1294,18 +1309,8 @@ handle_builtin_strlen (gimple_stmt_itera > if (!useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (rhs))) > rhs = fold_convert_loc (loc, TREE_TYPE (lhs), rhs); > > - /* Set for strnlen() calls with a non-constant bound. */ > - bool noncst_bound = false; > if (bound) > - { > - tree new_rhs > - = fold_build2_loc (loc, MIN_EXPR, TREE_TYPE (rhs), rhs, bound); > - > - noncst_bound = (TREE_CODE (new_rhs) != INTEGER_CST > - || tree_int_cst_lt (new_rhs, rhs)); > - > - rhs = new_rhs; > - } > + rhs = fold_build2_loc (loc, MIN_EXPR, TREE_TYPE (rhs), rhs, bound); > > if (!update_call_from_tree (gsi, rhs)) > gimplify_and_update_call_from_tree (gsi, rhs); > @@ -1317,12 +1322,9 @@ handle_builtin_strlen (gimple_stmt_itera > print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM); > } > > - /* Avoid storing the length for calls to strnlen() with > - a non-constant bound. */ > - if (noncst_bound) > - return; > - > if (si != NULL > + /* Don't update anything for strnlen. */ > + && bound == NULL_TREE > && TREE_CODE (si->nonzero_chars) != SSA_NAME > && TREE_CODE (si->nonzero_chars) != INTEGER_CST > && !SSA_NAME_OCCURS_IN_ABNORMAL_PHI (lhs)) > @@ -1332,7 +1334,13 @@ handle_builtin_strlen (gimple_stmt_itera > gcc_assert (si->full_string_p); > } > > - if (strlen_to_stridx) > + if (strlen_to_stridx > + && (bound == NULL_TREE > + /* For strnlen record this only if the call is proven > + to return the same value as strlen would. */ > + || (TREE_CODE (bound) == INTEGER_CST > + && TREE_CODE (rhs) == INTEGER_CST > + && tree_int_cst_lt (rhs, bound)))) > strlen_to_stridx->put (lhs, stridx_strlenloc (idx, loc)); > > return; > --- gcc/testsuite/gcc.dg/pr89500.c.jj 2019-02-26 11:37:28.946217272 +0100 > +++ gcc/testsuite/gcc.dg/pr89500.c 2019-02-26 11:37:28.946217272 +0100 > @@ -0,0 +1,17 @@ > +/* PR tree-optimization/89500 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2" } */ > + > +typedef __SIZE_TYPE__ size_t; > +extern size_t strlen (const char *); > +extern size_t strnlen (const char *, size_t); > +extern void bar (char *); > + > +void > +foo (int *a) > +{ > + char c[64]; > + bar (c); > + a[0] = strlen (c); > + a[1] = strnlen (c, 0); > +} > --- gcc/testsuite/gcc.dg/Wstringop-overflow-10.c.jj 2019-02-26 > 13:48:27.846299194 +0100 > +++ gcc/testsuite/gcc.dg/Wstringop-overflow-10.c 2019-02-26 > 13:48:21.902399033 +0100 > @@ -0,0 +1,34 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -Wstringop-overflow" } */ > + > +void > +foo (char *a) > +{ > + char b[16] = "abcdefg"; > + __builtin_strncpy (a, b, __builtin_strlen (b)); /* { dg-warning > "specified bound depends on the length of the source argument" } */ > +} > + > +void > +bar (char *a) > +{ > + char b[16] = "abcdefg"; > + __builtin_strncpy (a, b, __builtin_strnlen (b, 8)); /* { dg-warning > "specified bound depends on the length of the source argument" } */ > +} > + > +void > +baz (char *a) > +{ > + char b[16] = "abcdefg"; > + __builtin_strncpy (a, b, __builtin_strnlen (b, 7)); /* { dg-bogus > "specified bound depends on the length of the source argument" } */ > +} > + > +void fill (char *); > + > +void > +qux (char *a) > +{ > + char b[16]; > + fill (b); > + __builtin_memcpy (b, "abcdefg", 7); > + __builtin_strncpy (a, b, __builtin_strnlen (b, 8)); /* { dg-bogus > "specified bound depends on the length of the source argument" } */ > +} > --- gcc/testsuite/gcc.dg/strlenopt-60.c.jj 2019-02-26 12:09:03.151213821 > +0100 > +++ gcc/testsuite/gcc.dg/strlenopt-60.c 2019-02-26 13:54:27.612299315 > +0100 > @@ -0,0 +1,58 @@ > +/* PR tree-optimization/89500 */ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -fdump-tree-optimized" } */ > +/* { dg-final { scan-tree-dump-times "return 10;" 2 "optimized" } } */ > +/* { dg-final { scan-tree-dump-times "return 5;" 1 "optimized" } } */ > +/* { dg-final { scan-tree-dump-times "return 0;" 2 "optimized" } } */ > +/* { dg-final { scan-tree-dump-times "strnlen " 1 "optimized" } } */ > + > +#include "strlenopt.h" > + > +void foo (char *); > + > +size_t > +f1 (void) > +{ > + char a[10] = "0123456789"; > + return strnlen (a, 10); > +} > + > +size_t > +f2 (void) > +{ > + char a[10] = "0123456789"; > + return strnlen (a, 5); > +} > + > +size_t > +f3 (void) > +{ > + char a[10] = "0123456789"; > + return strnlen (a, 0); > +} > + > +size_t > +f4 (void) > +{ > + char a[20]; > + foo (a); > + memcpy (a, "0123456789", 10); > + return strnlen (a, 10); > +} > + > +size_t > +f5 (void) > +{ > + char a[20]; > + foo (a); > + memcpy (a, "0123456789", 10); > + return strnlen (a, 14); > +} > + > +size_t > +f6 (void) > +{ > + char a[20]; > + foo (a); > + return strnlen (a, 0); > +} > > > Jakub > > -- Richard Biener <rguent...@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)