While looking into opportunities to detect strnlen/strlen coding
mistakes (pr86199) I noticed a bug in the strnlen implementation
I committed earlier today that lets a strnlen() result be saved
and used in subsequent calls to strlen() with the same argument.
The attached patch changes the handle_builtin_strlen() function
to discard the strnlen() result unless its bound is greater than
the length of the string.

Martin
PR tree-optimization/86204 -  wrong strlen result after prior strnlen

gcc/ChangeLog:

	PR tree-optimization/86204
	* tree-ssa-strlen.c (handle_builtin_strlen): Avoid storing
	a strnlen result if it's less than the length of the string.

gcc/testsuite/ChangeLog:

	PR tree-optimization/86204
	* gcc.dg/strlenopt-46.c: New test.

Index: gcc/testsuite/gcc.dg/strlenopt-46.c
===================================================================
--- gcc/testsuite/gcc.dg/strlenopt-46.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/strlenopt-46.c	(working copy)
@@ -0,0 +1,131 @@
+/* PR tree-optimization/86204 - wrong strlen result after prior strnlen
+   { dg-do compile }
+   { dg-options "-O2 -Wall" } */
+
+#include "strlenopt.h"
+
+#define NOIPA   __attribute__ ((noipa))
+
+char a[] = "12345";
+
+NOIPA void f0 (void)
+{
+  unsigned n0 = strnlen (a, 0);
+  unsigned n1 = strlen (a);
+
+  if (n0 != 0 || n1 != 5)
+    abort ();
+}
+
+NOIPA void f1 (void)
+{
+  unsigned n0 = strnlen (a, 1);
+  unsigned n1 = strlen (a);
+
+  if (n0 != 1 || n1 != 5)
+    abort ();
+}
+
+NOIPA void f2 (void)
+{
+  unsigned n0 = strnlen (a, 2);
+  unsigned n1 = strlen (a);
+
+  if (n0 != 2 || n1 != 5)
+    abort ();
+}
+
+NOIPA void f3 (void)
+{
+  unsigned n0 = strnlen (a, 3);
+  unsigned n1 = strlen (a);
+
+  if (n0 != 3 || n1 != 5)
+    abort ();
+}
+
+NOIPA void f4 (void)
+{
+  unsigned n0 = strnlen (a, 4);
+  unsigned n1 = strlen (a);
+
+  if (n0 != 4 || n1 != 5)
+    abort ();
+}
+
+NOIPA void f5 (void)
+{
+  unsigned n0 = strnlen (a, 5);
+  unsigned n1 = strlen (a);
+
+  if (n0 != 5 || n1 != 5)
+    abort ();
+}
+
+NOIPA void f6 (void)
+{
+  unsigned n0 = strnlen (a, 6);
+  unsigned n1 = strlen (a);
+
+  if (n0 != 5 || n1 != 5)
+    abort ();
+}
+
+NOIPA void fx (unsigned n)
+{
+  unsigned n0 = strnlen (a, n);
+  unsigned n1 = strlen (a);
+
+  unsigned min = n < 5 ? n : 5;
+  if (n0 != min || n1 != 5)
+    abort ();
+}
+
+NOIPA void g2 (void)
+{
+  strcpy (a, "123");
+  unsigned n0 = strnlen (a, 2);
+  unsigned n1 = strlen (a);
+
+  if (n0 != 2 || n1 != 3)
+    abort ();
+}
+
+NOIPA void g7 (void)
+{
+  strcpy (a, "123");
+  unsigned n0 = strnlen (a, 7);
+  unsigned n1 = strlen (a);
+
+  if (n0 != 3 || n1 != 3)
+    abort ();
+}
+
+NOIPA void gx (unsigned n)
+{
+  strcpy (a, "123");
+  unsigned n0 = strnlen (a, n);
+  unsigned n1 = strlen (a);
+
+  unsigned min = n < 3 ? n : 3;
+  if (n0 != min || n1 != 3)
+    abort ();
+}
+
+int main (void)
+{
+  f0 ();
+  f1 ();
+  f2 ();
+  f3 ();
+  f4 ();
+  f5 ();
+  f6 ();
+  fx (2);
+  fx (7);
+
+  g2 ();
+  g7 ();
+  gx (2);
+  gx (7);
+}
Index: gcc/tree-ssa-strlen.c
===================================================================
--- gcc/tree-ssa-strlen.c	(revision 261705)
+++ gcc/tree-ssa-strlen.c	(working copy)
@@ -1270,8 +1270,19 @@ handle_builtin_strlen (gimple_stmt_iterator *gsi)
 	  rhs = unshare_expr (rhs);
 	  if (!useless_type_conversion_p (TREE_TYPE (lhs), TREE_TYPE (rhs)))
 	    rhs = fold_convert_loc (loc, TREE_TYPE (lhs), rhs);
+
+	  bool bounded = false;
 	  if (bound)
-	    rhs = fold_build2_loc (loc, MIN_EXPR, TREE_TYPE (rhs), rhs, bound);
+	    {
+	      tree new_rhs
+		= fold_build2_loc (loc, MIN_EXPR, TREE_TYPE (rhs), rhs, bound);
+
+	      bounded = (TREE_CODE (new_rhs) != INTEGER_CST
+			 || tree_int_cst_lt (new_rhs, rhs));
+
+	      rhs = new_rhs;
+	    }
+
 	  if (!update_call_from_tree (gsi, rhs))
 	    gimplify_and_update_call_from_tree (gsi, rhs);
 	  stmt = gsi_stmt (*gsi);
@@ -1281,6 +1292,11 @@ handle_builtin_strlen (gimple_stmt_iterator *gsi)
 	      fprintf (dump_file, "into: ");
 	      print_gimple_stmt (dump_file, stmt, 0, TDF_SLIM);
 	    }
+
+	  /* Avoid storing the length for calls to strnlen().  */
+	  if (bounded)
+	    return;
+
 	  if (si != NULL
 	      && TREE_CODE (si->nonzero_chars) != SSA_NAME
 	      && TREE_CODE (si->nonzero_chars) != INTEGER_CST
@@ -1299,6 +1315,7 @@ handle_builtin_strlen (gimple_stmt_iterator *gsi)
     }
   if (SSA_NAME_OCCURS_IN_ABNORMAL_PHI (lhs))
     return;
+
   if (idx == 0)
     idx = new_stridx (src);
   else
@@ -1333,9 +1350,14 @@ handle_builtin_strlen (gimple_stmt_iterator *gsi)
     }
   if (idx)
     {
-      strinfo *si = new_strinfo (src, idx, lhs, true);
-      set_strinfo (idx, si);
-      find_equal_ptrs (src, idx);
+      if (!bound)
+	{
+	  /* Only store the new length information for calls to strlen(),
+	     not for those to strnlen().  */
+	  strinfo *si = new_strinfo (src, idx, lhs, true);
+	  set_strinfo (idx, si);
+	  find_equal_ptrs (src, idx);
+	}
 
       /* For SRC that is an array of N elements, set LHS's range
 	 to [0, min (N, BOUND)].  A constant return value means
@@ -1362,7 +1384,7 @@ handle_builtin_strlen (gimple_stmt_iterator *gsi)
 	      }
 	  }
 
-      if (strlen_to_stridx)
+      if (strlen_to_stridx && !bound)
 	strlen_to_stridx->put (lhs, stridx_strlenloc (idx, loc));
     }
 }

Reply via email to