This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] avoid using strnlen result for late calls to strlen (PR 82604)
- From: Martin Sebor <msebor at gmail dot com>
- To: Gcc Patch List <gcc-patches at gcc dot gnu dot org>, Jeff Law <law at redhat dot com>
- Date: Mon, 18 Jun 2018 13:15:49 -0600
- Subject: [PATCH] avoid using strnlen result for late calls to strlen (PR 82604)
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));
}
}