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

Message ID 20211115173315.1857598-4-siddhesh@gotplt.org
State Committed
Headers
Series gimple-fold improvements |

Commit Message

Siddhesh Poyarekar Nov. 15, 2021, 5:33 p.m. UTC
  Use ranges for lengths and object sizes in strncat and snprintf to
determine if they can be transformed into simpler operations.

gcc/ChangeLog:

	* gimple-fold.c (gimple_fold_builtin_strncat): 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                       | 102 ++++++++++++------------
 gcc/testsuite/gcc.dg/fold-stringops-2.c |  16 +++-
 gcc/testsuite/gcc.dg/fold-stringops-3.c |  18 +++++
 3 files changed, 82 insertions(+), 54 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/fold-stringops-3.c
  

Comments

Jeff Law Nov. 15, 2021, 7:38 p.m. UTC | #1
On 11/15/2021 10:33 AM, Siddhesh Poyarekar wrote:
> Use ranges for lengths and object sizes in strncat and snprintf to
> determine if they can be transformed into simpler operations.
>
> gcc/ChangeLog:
>
> 	* gimple-fold.c (gimple_fold_builtin_strncat): 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.
OK
jeff
  

Patch

diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index f3362287c0d..50b9ba8d558 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -2485,72 +2485,73 @@  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 (!src_len || known_lower (stmt, len, src_len, true))
     return false;
 
   unsigned HOST_WIDE_INT dstsize;
+  bool found_dstsize = compute_builtin_object_size (dst, 1, &dstsize);
 
-  bool nowarn = warning_suppressed_p (stmt, OPT_Wstringop_overflow_);
-
-  if (!nowarn && compute_builtin_object_size (dst, 1, &dstsize))
+  /* Warn on constant LEN.  */
+  if (TREE_CODE (len) == INTEGER_CST)
     {
-      int cmpdst = compare_tree_int (len, dstsize);
+      bool nowarn = warning_suppressed_p (stmt, OPT_Wstringop_overflow_);
 
-      if (cmpdst >= 0)
+      if (!nowarn && found_dstsize)
 	{
-	  tree fndecl = gimple_call_fndecl (stmt);
+	  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_);
+	    }
+	}
 
-	  /* 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.  */
+      if (!nowarn && TREE_CODE (src_len) == INTEGER_CST
+	  && tree_int_cst_compare (src_len, len) == 0)
+	{
+	  tree fndecl = gimple_call_fndecl (stmt);
 	  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)
+
+	  /* 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_);
 	}
     }
 
-  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_);
-    }
+  if (!known_lower (stmt, src_len, len))
+    return false;
 
   tree fn = builtin_decl_implicit (BUILT_IN_STRCAT);
 
@@ -3623,10 +3624,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)
@@ -3646,6 +3643,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
@@ -3653,8 +3652,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_lower (stmt, len, destsize, true))
 	return false;
 
       gimple_seq stmts = NULL;
@@ -3663,7 +3661,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
@@ -3694,8 +3692,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';
@@ -3704,7 +3700,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_lower (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" } } */