[PATCH 3/3] gimple-fold: Use ranges to simplify strncat and snprintf

[Siddhesh Poyarekar <siddhesh@gotplt.org>]

Remove the warnings for strncat since it is already handled (and even
the error messages look identical) in gimple-ssa-warn-access.  Instead,
use len range to determine if it is within bounds of source and
destination and simplify it to strcat if it's safe.

Likewise for snprintf, use ranges to determine if it can be transformed
to strcpy.

gcc/ChangeLog:

	* gimple-fold.c (gimple_fold_builtin_strncat): Remove warnings
	and use ranges to determine if it is safe to transform to
	strcat.
	(gimple_fold_builtin_snprintf): Likewise.

gcc/testsuite/ChangeLog:

	* gcc.dg/fold-stringops-2.c: Define size_t.
	(safe1): Adjust.
	(safe4): New test.
	* gcc.dg/fold-stringops-3.c: New test.

Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
---
 gcc/gimple-fold.c                       | 76 +++++--------------------
 gcc/testsuite/gcc.dg/fold-stringops-2.c | 16 +++++-
 gcc/testsuite/gcc.dg/fold-stringops-3.c | 18 ++++++
 3 files changed, 47 insertions(+), 63 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/fold-stringops-3.c

diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index bcfd5d97feb..3112b86c2f7 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -2485,72 +2485,29 @@ gimple_fold_builtin_strncat (gimple_stmt_iterator *gsi)
   tree dst = gimple_call_arg (stmt, 0);
   tree src = gimple_call_arg (stmt, 1);
   tree len = gimple_call_arg (stmt, 2);
-
-  const char *p = c_getstr (src);
+  tree src_len = c_strlen (src, 1);
 
   /* If the requested length is zero, or the src parameter string
      length is zero, return the dst parameter.  */
-  if (integer_zerop (len) || (p && *p == '\0'))
+  if (integer_zerop (len) || (src_len && integer_zerop (src_len)))
     {
       replace_call_with_value (gsi, dst);
       return true;
     }
 
-  if (TREE_CODE (len) != INTEGER_CST || !p)
-    return false;
-
-  unsigned srclen = strlen (p);
-
-  int cmpsrc = compare_tree_int (len, srclen);
-
   /* Return early if the requested len is less than the string length.
      Warnings will be issued elsewhere later.  */
-  if (cmpsrc < 0)
+  if (!known_safe (stmt, src_len, len))
     return false;
 
   unsigned HOST_WIDE_INT dstsize;
 
-  bool nowarn = warning_suppressed_p (stmt, OPT_Wstringop_overflow_);
-
-  if (!nowarn && compute_builtin_object_size (dst, 1, &dstsize))
-    {
-      int cmpdst = compare_tree_int (len, dstsize);
-
-      if (cmpdst >= 0)
-	{
-	  tree fndecl = gimple_call_fndecl (stmt);
-
-	  /* Strncat copies (at most) LEN bytes and always appends
-	     the terminating NUL so the specified bound should never
-	     be equal to (or greater than) the size of the destination.
-	     If it is, the copy could overflow.  */
-	  location_t loc = gimple_location (stmt);
-	  nowarn = warning_at (loc, OPT_Wstringop_overflow_,
-			       cmpdst == 0
-			       ? G_("%qD specified bound %E equals "
-				    "destination size")
-			       : G_("%qD specified bound %E exceeds "
-				    "destination size %wu"),
-			       fndecl, len, dstsize);
-	  if (nowarn)
-	    suppress_warning (stmt, OPT_Wstringop_overflow_);
-	}
-    }
-
-  if (!nowarn && cmpsrc == 0)
-    {
-      tree fndecl = gimple_call_fndecl (stmt);
-      location_t loc = gimple_location (stmt);
-
-      /* To avoid possible overflow the specified bound should also
-	 not be equal to the length of the source, even when the size
-	 of the destination is unknown (it's not an uncommon mistake
-	 to specify as the bound to strncpy the length of the source).  */
-      if (warning_at (loc, OPT_Wstringop_overflow_,
-		      "%qD specified bound %E equals source length",
-		      fndecl, len))
-	suppress_warning (stmt, OPT_Wstringop_overflow_);
-    }
+  /* Likewise, bail out from the transformation if we're unable to determine
+     the destination size.  Warnings will be issued elsewhere later.  */
+  if (!compute_builtin_object_size (dst, 1, &dstsize)
+      || !known_safe (stmt, len, build_int_cstu (TREE_TYPE (len), dstsize),
+		      true))
+    return false;
 
   tree fn = builtin_decl_implicit (BUILT_IN_STRCAT);
 
@@ -3626,10 +3583,6 @@ gimple_fold_builtin_snprintf (gimple_stmt_iterator *gsi)
   if (gimple_call_num_args (stmt) == 4)
     orig = gimple_call_arg (stmt, 3);
 
-  if (!tree_fits_uhwi_p (destsize))
-    return false;
-  unsigned HOST_WIDE_INT destlen = tree_to_uhwi (destsize);
-
   /* Check whether the format is a literal string constant.  */
   fmt_str = c_getstr (fmt);
   if (fmt_str == NULL)
@@ -3649,6 +3602,8 @@ gimple_fold_builtin_snprintf (gimple_stmt_iterator *gsi)
       if (orig)
 	return false;
 
+      tree len = build_int_cstu (TREE_TYPE (destsize), strlen (fmt_str));
+
       /* We could expand this as
 	 memcpy (str, fmt, cst - 1); str[cst - 1] = '\0';
 	 or to
@@ -3656,8 +3611,7 @@ gimple_fold_builtin_snprintf (gimple_stmt_iterator *gsi)
 	 but in the former case that might increase code size
 	 and in the latter case grow .rodata section too much.
 	 So punt for now.  */
-      size_t len = strlen (fmt_str);
-      if (len >= destlen)
+      if (!known_safe (stmt, len, destsize, true))
 	return false;
 
       gimple_seq stmts = NULL;
@@ -3666,7 +3620,7 @@ gimple_fold_builtin_snprintf (gimple_stmt_iterator *gsi)
       if (tree lhs = gimple_call_lhs (stmt))
 	{
 	  repl = gimple_build_assign (lhs,
-				      build_int_cst (TREE_TYPE (lhs), len));
+				      fold_convert (TREE_TYPE (lhs), len));
 	  gimple_seq_add_stmt_without_update (&stmts, repl);
 	  gsi_replace_with_seq_vops (gsi, stmts);
 	  /* gsi now points at the assignment to the lhs, get a
@@ -3697,8 +3651,6 @@ gimple_fold_builtin_snprintf (gimple_stmt_iterator *gsi)
 	return false;
 
       tree orig_len = get_maxval_strlen (orig, SRK_STRLEN);
-      if (!orig_len || TREE_CODE (orig_len) != INTEGER_CST)
-	return false;
 
       /* We could expand this as
 	 memcpy (str1, str2, cst - 1); str1[cst - 1] = '\0';
@@ -3707,7 +3659,7 @@ gimple_fold_builtin_snprintf (gimple_stmt_iterator *gsi)
 	 but in the former case that might increase code size
 	 and in the latter case grow .rodata section too much.
 	 So punt for now.  */
-      if (compare_tree_int (orig_len, destlen) >= 0)
+      if (!known_safe (stmt, orig_len, destsize, true))
 	return false;
 
       /* Convert snprintf (str1, cst, "%s", str2) into
diff --git a/gcc/testsuite/gcc.dg/fold-stringops-2.c b/gcc/testsuite/gcc.dg/fold-stringops-2.c
index 0b415dfaf57..ac7d29eac50 100644
--- a/gcc/testsuite/gcc.dg/fold-stringops-2.c
+++ b/gcc/testsuite/gcc.dg/fold-stringops-2.c
@@ -1,10 +1,12 @@
 /* { dg-do compile } */
 /* { dg-options "-O2" } */
 
+typedef __SIZE_TYPE__ size_t;
+
 #define bos(__d) __builtin_object_size ((__d), 0)
 
 char *
-safe1 (const char *src, int cond, __SIZE_TYPE__ len)
+safe1 (const char *src, int cond, size_t len)
 {
   char *dst;
 
@@ -44,6 +46,18 @@ safe3 (const char *src, int cond, unsigned char len)
   return __builtin___snprintf_chk (dst, len, 0, bos (dst), "%s", src);
 }
 
+char dst[1024];
+
+void
+safe4 (size_t len)
+{
+  len = len > sizeof (dst) - 1 ? sizeof (dst) - 1 : len;
+  len = len < sizeof (dst) / 2 ? sizeof (dst) / 2 : len;
+
+  __builtin_strncat (dst, "hello", len);
+}
+
 /* { dg-final { scan-assembler-not "__memcpy_chk" } } */
 /* { dg-final { scan-assembler-not "__strncpy_chk" } } */
 /* { dg-final { scan-assembler-not "__snprintf_chk" } } */
+/* { dg-final { scan-assembler-not "strncat" } } */
diff --git a/gcc/testsuite/gcc.dg/fold-stringops-3.c b/gcc/testsuite/gcc.dg/fold-stringops-3.c
new file mode 100644
index 00000000000..ae2efbf9967
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/fold-stringops-3.c
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+typedef __SIZE_TYPE__ size_t;
+
+char dst[1024];
+
+void
+safe1 (size_t len)
+{
+  len = len > sizeof (dst) ? sizeof (dst) : len;
+  len = len < sizeof (dst) / 2 ? sizeof (dst) / 2 : len;
+
+  __builtin_snprintf (dst, len, "hello");
+  __builtin_snprintf (dst + 5, len, "%s", " world");
+}
+
+/* { dg-final { scan-assembler-not "snprintf" } } */
-- 
2.31.1