[PATCH] Fix up strnlen handling in tree-ssa-strlen.c (PR tree-optimization/89500)

[Jakub Jelinek]

Hi!

The following testcase ICEs, because rhs (previously known string length)
is SSA_NAME (return value from the earlier strlen), but bound is 0 and so
MIN_EXPR yields also size_zero_node.  The noncst_bound initialization
checks if new_rhs is INTEGER_CST and if it is, assumes rhs must be too
and uses tree_int_cst_lt.  While we could just add
|| TREE_CODE (rhs) != INTEGER_CST
before the || tree_int_cst_lt (new_rhs, rhs), I fail to see what
noncst_bound (which would be misnamed anyway) is good for.
For further optimizations in the strlen pass we care about string lengths
only, and we can prove strnlen returns that length only if all of rhs,
new_rhs and bound are actually INTEGER_CSTs for which we can prove that
new_rhs is equal to rhs.  The
          if (si != NULL
              && TREE_CODE (si->nonzero_chars) != SSA_NAME
              && TREE_CODE (si->nonzero_chars) != INTEGER_CST
              && !SSA_NAME_OCCURS_IN_ABNORMAL_PHI (lhs))
            {
              si = unshare_strinfo (si);
              si->nonzero_chars = lhs;
              gcc_assert (si->full_string_p);
            }
hunk is though about trying to improve the case where we could compute
the length through some extra code (e.g. through strchr transformations,
stpcpy etc.) and for the next time we don't want to repeat that computation,
but just use the lhs of the strlen call, i.e. SSA_NAME.  As written above,
we explicitly don't do anything if it is INTEGER_CST, we already know very
well the length.  So the only case where we can prove something is not
useful here.
The only remaining code in the function before returning is:
          if (strlen_to_stridx)
            strlen_to_stridx->put (lhs, stridx_strlenloc (idx, loc));
but I believe doing that for strnlen wasn't really intentional, in the
case we don't know the length before we don't store it either:
      if (strlen_to_stridx && !bound)
        strlen_to_stridx->put (lhs, stridx_strlenloc (idx, loc));
it is for the warnings and I believe it is desirable to do that for strlen
only.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

2019-02-25  Jakub Jelinek  <jakub@redhat.com>

	PR tree-optimization/89500
	* tree-ssa-strlen.c (handle_builtin_strlen): Remove noncst_bound
	variable, return early after optimizing stmt if bound is non-NULL.

	* gcc.dg/pr89500.c: New test.

--- gcc/tree-ssa-strlen.c.jj	2019-01-18 00:33:19.466003372 +0100
+++ gcc/tree-ssa-strlen.c	2019-02-25 22:13:27.165521677 +0100
@@ -1294,18 +1294,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,9 +1307,12 @@ 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)
+	  /* Don't record anything below for strnlen, it is to simplify
+	     length computation if si->nonzero_chars is complex, but
+	     the only case where we could prove strnlen return value is
+	     the string length is if si->nonzero_chars is constant and
+	     bound is too and si->nonzero_chars <= bound.  */
+	  if (bound)
 	    return;
 
 	  if (si != NULL
--- gcc/testsuite/gcc.dg/pr89500.c.jj	2019-02-25 22:14:46.468222667 +0100
+++ gcc/testsuite/gcc.dg/pr89500.c	2019-02-25 22:14:26.948542413 +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);
+}

	Jakub