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)
