public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] gimple-fold: Smarter optimization of _chk variants
@ 2021-11-10  9:38 Siddhesh Poyarekar
  2021-11-10 10:45 ` [PATCH v2] " Siddhesh Poyarekar
  0 siblings, 1 reply; 3+ messages in thread
From: Siddhesh Poyarekar @ 2021-11-10  9:38 UTC (permalink / raw)
  To: gcc-patches; +Cc: jakub, msebor

Instead of comparing LEN and SIZE only if they are constants, use their
ranges to decide if LEN will always be lower than or same as SIZE.

This change ends up putting the stringop-overflow warning line number
against the strcpy implementation, so adjust the warning check to be
line number agnostic.

gcc/ChangeLog:

	* gimple-fold.c (known_safe): New function.
	(gimple_fold_builtin_memory_chk, gimple_fold_builtin_stxcpy_chk,
	gimple_fold_builtin_stxncpy_chk,
	gimple_fold_builtin_snprintf_chk,
	gimple_fold_builtin_sprintf_chk): Use it.

gcc/testsuite/ChangeLog:

	* gcc.dg/Wobjsize-1.c: Make warning change line agnostic.
	* gcc.dg/builtin-chk-fold.c: New test.

Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
---
Testing:

- gcc.dg shows no new regressions on x86_64, a full bootstrap and test
  run are in progress.

 gcc/gimple-fold.c                       | 186 +++++++++---------------
 gcc/testsuite/gcc.dg/Wobjsize-1.c       |   5 +-
 gcc/testsuite/gcc.dg/builtin-chk-fold.c |  21 +++
 3 files changed, 94 insertions(+), 118 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/builtin-chk-fold.c

diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index 6e25a7c05db..7399b49d00f 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -2987,6 +2987,25 @@ gimple_fold_builtin_fputs (gimple_stmt_iterator *gsi,
   return false;
 }
 
+/* Return true if LEN is known to be less than or equal to SIZE at compile time
+   and false otherwise.  Emit a warning if LEN is known to be greater than SIZE
+   at compile time.  */
+
+static bool
+known_safe (gimple *stmt, tree len, tree size)
+{
+  if (len == NULL_TREE)
+    return false;
+
+  wide_int size_range[2];
+  wide_int len_range[2];
+  if (get_range (len, stmt, len_range) && get_range (size, stmt, size_range)
+      && wi::leu_p (len_range[1], size_range[0]))
+    return true;
+
+  return false;
+}
+
 /* Fold a call to the __mem{cpy,pcpy,move,set}_chk builtin.
    DEST, SRC, LEN, and SIZE are the arguments to the call.
    IGNORE is true, if return value can be ignored.  FCODE is the BUILT_IN_*
@@ -3024,39 +3043,24 @@ gimple_fold_builtin_memory_chk (gimple_stmt_iterator *gsi,
 	}
     }
 
-  if (! tree_fits_uhwi_p (size))
-    return false;
-
   tree maxlen = get_maxval_strlen (len, SRK_INT_VALUE);
-  if (! integer_all_onesp (size))
+  if (! integer_all_onesp (size)
+      && !known_safe (stmt, len, size) && !known_safe (stmt, maxlen, size))
     {
-      if (! tree_fits_uhwi_p (len))
+      /* MAXLEN and LEN both cannot be proved to be less than SIZE, at
+	 least try to optimize (void) __mempcpy_chk () into
+	 (void) __memcpy_chk () */
+      if (fcode == BUILT_IN_MEMPCPY_CHK && ignore)
 	{
-	  /* If LEN is not constant, try MAXLEN too.
-	     For MAXLEN only allow optimizing into non-_ocs function
-	     if SIZE is >= MAXLEN, never convert to __ocs_fail ().  */
-	  if (maxlen == NULL_TREE || ! tree_fits_uhwi_p (maxlen))
-	    {
-	      if (fcode == BUILT_IN_MEMPCPY_CHK && ignore)
-		{
-		  /* (void) __mempcpy_chk () can be optimized into
-		     (void) __memcpy_chk ().  */
-		  fn = builtin_decl_explicit (BUILT_IN_MEMCPY_CHK);
-		  if (!fn)
-		    return false;
+	  fn = builtin_decl_explicit (BUILT_IN_MEMCPY_CHK);
+	  if (!fn)
+	    return false;
 
-		  gimple *repl = gimple_build_call (fn, 4, dest, src, len, size);
-		  replace_call_with_call_and_fold (gsi, repl);
-		  return true;
-		}
-	      return false;
-	    }
+	  gimple *repl = gimple_build_call (fn, 4, dest, src, len, size);
+	  replace_call_with_call_and_fold (gsi, repl);
+	  return true;
 	}
-      else
-	maxlen = len;
-
-      if (tree_int_cst_lt (size, maxlen))
-	return false;
+      return false;
     }
 
   fn = NULL_TREE;
@@ -3126,61 +3130,47 @@ gimple_fold_builtin_stxcpy_chk (gimple_stmt_iterator *gsi,
       return true;
     }
 
-  if (! tree_fits_uhwi_p (size))
-    return false;
-
   tree maxlen = get_maxval_strlen (src, SRK_STRLENMAX);
   if (! integer_all_onesp (size))
     {
       len = c_strlen (src, 1);
-      if (! len || ! tree_fits_uhwi_p (len))
+      if (!known_safe (stmt, len, size) && !known_safe (stmt, maxlen, size))
 	{
-	  /* If LEN is not constant, try MAXLEN too.
-	     For MAXLEN only allow optimizing into non-_ocs function
-	     if SIZE is >= MAXLEN, never convert to __ocs_fail ().  */
-	  if (maxlen == NULL_TREE || ! tree_fits_uhwi_p (maxlen))
+	  if (fcode == BUILT_IN_STPCPY_CHK)
 	    {
-	      if (fcode == BUILT_IN_STPCPY_CHK)
-		{
-		  if (! ignore)
-		    return false;
-
-		  /* If return value of __stpcpy_chk is ignored,
-		     optimize into __strcpy_chk.  */
-		  fn = builtin_decl_explicit (BUILT_IN_STRCPY_CHK);
-		  if (!fn)
-		    return false;
-
-		  gimple *repl = gimple_build_call (fn, 3, dest, src, size);
-		  replace_call_with_call_and_fold (gsi, repl);
-		  return true;
-		}
-
-	      if (! len || TREE_SIDE_EFFECTS (len))
+	      if (! ignore)
 		return false;
 
-	      /* If c_strlen returned something, but not a constant,
-		 transform __strcpy_chk into __memcpy_chk.  */
-	      fn = builtin_decl_explicit (BUILT_IN_MEMCPY_CHK);
+	      /* If return value of __stpcpy_chk is ignored,
+		 optimize into __strcpy_chk.  */
+	      fn = builtin_decl_explicit (BUILT_IN_STRCPY_CHK);
 	      if (!fn)
 		return false;
 
-	      gimple_seq stmts = NULL;
-	      len = force_gimple_operand (len, &stmts, true, NULL_TREE);
-	      len = gimple_convert (&stmts, loc, size_type_node, len);
-	      len = gimple_build (&stmts, loc, PLUS_EXPR, size_type_node, len,
-				  build_int_cst (size_type_node, 1));
-	      gsi_insert_seq_before (gsi, stmts, GSI_SAME_STMT);
-	      gimple *repl = gimple_build_call (fn, 4, dest, src, len, size);
+	      gimple *repl = gimple_build_call (fn, 3, dest, src, size);
 	      replace_call_with_call_and_fold (gsi, repl);
 	      return true;
 	    }
-	}
-      else
-	maxlen = len;
 
-      if (! tree_int_cst_lt (maxlen, size))
-	return false;
+	  if (! len || TREE_SIDE_EFFECTS (len))
+	    return false;
+
+	  /* If c_strlen returned something, but not a constant,
+	     transform __strcpy_chk into __memcpy_chk.  */
+	  fn = builtin_decl_explicit (BUILT_IN_MEMCPY_CHK);
+	  if (!fn)
+	    return false;
+
+	  gimple_seq stmts = NULL;
+	  len = force_gimple_operand (len, &stmts, true, NULL_TREE);
+	  len = gimple_convert (&stmts, loc, size_type_node, len);
+	  len = gimple_build (&stmts, loc, PLUS_EXPR, size_type_node, len,
+			      build_int_cst (size_type_node, 1));
+	  gsi_insert_seq_before (gsi, stmts, GSI_SAME_STMT);
+	  gimple *repl = gimple_build_call (fn, 4, dest, src, len, size);
+	  replace_call_with_call_and_fold (gsi, repl);
+	  return true;
+	}
     }
 
   /* If __builtin_st{r,p}cpy_chk is used, assume st{r,p}cpy is available.  */
@@ -3222,26 +3212,10 @@ gimple_fold_builtin_stxncpy_chk (gimple_stmt_iterator *gsi,
 	 }
     }
 
-  if (! tree_fits_uhwi_p (size))
-    return false;
-
   tree maxlen = get_maxval_strlen (len, SRK_INT_VALUE);
-  if (! integer_all_onesp (size))
-    {
-      if (! tree_fits_uhwi_p (len))
-	{
-	  /* If LEN is not constant, try MAXLEN too.
-	     For MAXLEN only allow optimizing into non-_ocs function
-	     if SIZE is >= MAXLEN, never convert to __ocs_fail ().  */
-	  if (maxlen == NULL_TREE || ! tree_fits_uhwi_p (maxlen))
-	    return false;
-	}
-      else
-	maxlen = len;
-
-      if (tree_int_cst_lt (size, maxlen))
-	return false;
-    }
+  if (! integer_all_onesp (size)
+      && !known_safe (stmt, len, size) && !known_safe (stmt, maxlen, size))
+    return false;
 
   /* If __builtin_st{r,p}ncpy_chk is used, assume st{r,p}ncpy is available.  */
   fn = builtin_decl_explicit (fcode == BUILT_IN_STPNCPY_CHK
@@ -3359,27 +3333,11 @@ gimple_fold_builtin_snprintf_chk (gimple_stmt_iterator *gsi,
   size = gimple_call_arg (stmt, 3);
   fmt = gimple_call_arg (stmt, 4);
 
-  if (! tree_fits_uhwi_p (size))
+  tree maxlen = get_maxval_strlen (len, SRK_INT_VALUE);
+  if (! integer_all_onesp (size)
+      && !known_safe (stmt, len, size) && !known_safe (stmt, maxlen, size))
     return false;
 
-  if (! integer_all_onesp (size))
-    {
-      tree maxlen = get_maxval_strlen (len, SRK_INT_VALUE);
-      if (! tree_fits_uhwi_p (len))
-	{
-	  /* If LEN is not constant, try MAXLEN too.
-	     For MAXLEN only allow optimizing into non-_ocs function
-	     if SIZE is >= MAXLEN, never convert to __ocs_fail ().  */
-	  if (maxlen == NULL_TREE || ! tree_fits_uhwi_p (maxlen))
-	    return false;
-	}
-      else
-	maxlen = len;
-
-      if (tree_int_cst_lt (size, maxlen))
-	return false;
-    }
-
   if (!init_target_chars ())
     return false;
 
@@ -3438,9 +3396,6 @@ gimple_fold_builtin_sprintf_chk (gimple_stmt_iterator *gsi,
   size = gimple_call_arg (stmt, 2);
   fmt = gimple_call_arg (stmt, 3);
 
-  if (! tree_fits_uhwi_p (size))
-    return false;
-
   len = NULL_TREE;
 
   if (!init_target_chars ())
@@ -3454,7 +3409,7 @@ gimple_fold_builtin_sprintf_chk (gimple_stmt_iterator *gsi,
       if (strchr (fmt_str, target_percent) == 0)
 	{
 	  if (fcode != BUILT_IN_SPRINTF_CHK || nargs == 4)
-	    len = build_int_cstu (size_type_node, strlen (fmt_str));
+	    len = build_int_cstu (size_type_node, strlen (fmt_str) + 1);
 	}
       /* If the format is "%s" and first ... argument is a string literal,
 	 we know the size too.  */
@@ -3468,19 +3423,18 @@ gimple_fold_builtin_sprintf_chk (gimple_stmt_iterator *gsi,
 	      arg = gimple_call_arg (stmt, 4);
 	      if (POINTER_TYPE_P (TREE_TYPE (arg)))
 		{
-		  len = c_strlen (arg, 1);
 		  if (! len || ! tree_fits_uhwi_p (len))
 		    len = NULL_TREE;
+		  else
+		    len = fold_build2 (PLUS_EXPR, ssizetype, c_strlen (arg, 1),
+				       ssize_int (1));
 		}
 	    }
 	}
     }
 
-  if (! integer_all_onesp (size))
-    {
-      if (! len || ! tree_int_cst_lt (len, size))
-	return false;
-    }
+  if (! integer_all_onesp (size) && !known_safe (stmt, len, size))
+    return false;
 
   /* Only convert __{,v}sprintf_chk to {,v}sprintf if flag is 0
      or if format doesn't contain % chars or is "%s".  */
diff --git a/gcc/testsuite/gcc.dg/Wobjsize-1.c b/gcc/testsuite/gcc.dg/Wobjsize-1.c
index 2bd2f93897b..988b8bcbf35 100644
--- a/gcc/testsuite/gcc.dg/Wobjsize-1.c
+++ b/gcc/testsuite/gcc.dg/Wobjsize-1.c
@@ -7,11 +7,12 @@ char buf[6];
 
 int main(int argc, char **argv)
 {
-  strcpy (buf,"hello ");    /* { dg-warning "\\\[-Wstringop-overflow" } */
+  strcpy (buf,"hello ");
   return 0;
 }
 
-/* { dg-message "file included" "included" { target *-*-* } 0 }
+/* { dg-warning "\\\[-Wstringop-overflow" "warning" { target *-*-* } 0 }
+   { dg-message "file included" "included" { target *-*-* } 0 }
    { dg-message "inlined from" "inlined" { target *-*-* } 0 }
 
    The test might emit two warnings, one for the strcpy call and
diff --git a/gcc/testsuite/gcc.dg/builtin-chk-fold.c b/gcc/testsuite/gcc.dg/builtin-chk-fold.c
new file mode 100644
index 00000000000..517b146b50f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/builtin-chk-fold.c
@@ -0,0 +1,21 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+#define bos(__d) __builtin_object_size ((__d), 0)
+
+char *
+safe (const char *src, int cond, __SIZE_TYPE__ len)
+{
+  char *dst;
+
+  if (cond)
+    dst = __builtin_malloc (1024);
+  else
+    dst = __builtin_malloc (2048);
+
+  len = len > 2048 ? 2048 : len;
+
+  return __builtin___memcpy_chk (dst, src, len, bos (dst));
+}
+
+/* { dg-final { scan-assembler-not "__memcpy_chk" } } */
-- 
2.31.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH v2] gimple-fold: Smarter optimization of _chk variants
  2021-11-10  9:38 [PATCH] gimple-fold: Smarter optimization of _chk variants Siddhesh Poyarekar
@ 2021-11-10 10:45 ` Siddhesh Poyarekar
  2021-11-10 12:20   ` Siddhesh Poyarekar
  0 siblings, 1 reply; 3+ messages in thread
From: Siddhesh Poyarekar @ 2021-11-10 10:45 UTC (permalink / raw)
  To: gcc-patches; +Cc: jakub, msebor

Instead of comparing LEN and SIZE only if they are constants, use their
ranges to decide if LEN will always be lower than or same as SIZE.

This change ends up putting the stringop-overflow warning line number
against the strcpy implementation, so adjust the warning check to be
line number agnostic.

gcc/ChangeLog:

	* gimple-fold.c (known_safe): New function.
	(gimple_fold_builtin_memory_chk, gimple_fold_builtin_stxcpy_chk,
	gimple_fold_builtin_stxncpy_chk,
	gimple_fold_builtin_snprintf_chk,
	gimple_fold_builtin_sprintf_chk): Use it.

gcc/testsuite/ChangeLog:

	* gcc.dg/Wobjsize-1.c: Make warning change line agnostic.
	* gcc.dg/builtin-chk-fold.c: New test.

Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
---
Changes from v1:
- Update comment that incorrectly said that known_safe emits a warning.
- Add tests for strncpy and snprintf too.

 gcc/gimple-fold.c                       | 185 +++++++++---------------
 gcc/testsuite/gcc.dg/Wobjsize-1.c       |   5 +-
 gcc/testsuite/gcc.dg/builtin-chk-fold.c |  49 +++++++
 3 files changed, 121 insertions(+), 118 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/builtin-chk-fold.c

diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index 6e25a7c05db..36b06218c88 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -2987,6 +2987,24 @@ gimple_fold_builtin_fputs (gimple_stmt_iterator *gsi,
   return false;
 }
 
+/* Return true if LEN is known to be less than or equal to SIZE at compile time
+   and false otherwise.  */
+
+static bool
+known_safe (gimple *stmt, tree len, tree size)
+{
+  if (len == NULL_TREE)
+    return false;
+
+  wide_int size_range[2];
+  wide_int len_range[2];
+  if (get_range (len, stmt, len_range) && get_range (size, stmt, size_range)
+      && wi::leu_p (len_range[1], size_range[0]))
+    return true;
+
+  return false;
+}
+
 /* Fold a call to the __mem{cpy,pcpy,move,set}_chk builtin.
    DEST, SRC, LEN, and SIZE are the arguments to the call.
    IGNORE is true, if return value can be ignored.  FCODE is the BUILT_IN_*
@@ -3024,39 +3042,24 @@ gimple_fold_builtin_memory_chk (gimple_stmt_iterator *gsi,
 	}
     }
 
-  if (! tree_fits_uhwi_p (size))
-    return false;
-
   tree maxlen = get_maxval_strlen (len, SRK_INT_VALUE);
-  if (! integer_all_onesp (size))
+  if (! integer_all_onesp (size)
+      && !known_safe (stmt, len, size) && !known_safe (stmt, maxlen, size))
     {
-      if (! tree_fits_uhwi_p (len))
+      /* MAXLEN and LEN both cannot be proved to be less than SIZE, at
+	 least try to optimize (void) __mempcpy_chk () into
+	 (void) __memcpy_chk () */
+      if (fcode == BUILT_IN_MEMPCPY_CHK && ignore)
 	{
-	  /* If LEN is not constant, try MAXLEN too.
-	     For MAXLEN only allow optimizing into non-_ocs function
-	     if SIZE is >= MAXLEN, never convert to __ocs_fail ().  */
-	  if (maxlen == NULL_TREE || ! tree_fits_uhwi_p (maxlen))
-	    {
-	      if (fcode == BUILT_IN_MEMPCPY_CHK && ignore)
-		{
-		  /* (void) __mempcpy_chk () can be optimized into
-		     (void) __memcpy_chk ().  */
-		  fn = builtin_decl_explicit (BUILT_IN_MEMCPY_CHK);
-		  if (!fn)
-		    return false;
+	  fn = builtin_decl_explicit (BUILT_IN_MEMCPY_CHK);
+	  if (!fn)
+	    return false;
 
-		  gimple *repl = gimple_build_call (fn, 4, dest, src, len, size);
-		  replace_call_with_call_and_fold (gsi, repl);
-		  return true;
-		}
-	      return false;
-	    }
+	  gimple *repl = gimple_build_call (fn, 4, dest, src, len, size);
+	  replace_call_with_call_and_fold (gsi, repl);
+	  return true;
 	}
-      else
-	maxlen = len;
-
-      if (tree_int_cst_lt (size, maxlen))
-	return false;
+      return false;
     }
 
   fn = NULL_TREE;
@@ -3126,61 +3129,47 @@ gimple_fold_builtin_stxcpy_chk (gimple_stmt_iterator *gsi,
       return true;
     }
 
-  if (! tree_fits_uhwi_p (size))
-    return false;
-
   tree maxlen = get_maxval_strlen (src, SRK_STRLENMAX);
   if (! integer_all_onesp (size))
     {
       len = c_strlen (src, 1);
-      if (! len || ! tree_fits_uhwi_p (len))
+      if (!known_safe (stmt, len, size) && !known_safe (stmt, maxlen, size))
 	{
-	  /* If LEN is not constant, try MAXLEN too.
-	     For MAXLEN only allow optimizing into non-_ocs function
-	     if SIZE is >= MAXLEN, never convert to __ocs_fail ().  */
-	  if (maxlen == NULL_TREE || ! tree_fits_uhwi_p (maxlen))
+	  if (fcode == BUILT_IN_STPCPY_CHK)
 	    {
-	      if (fcode == BUILT_IN_STPCPY_CHK)
-		{
-		  if (! ignore)
-		    return false;
-
-		  /* If return value of __stpcpy_chk is ignored,
-		     optimize into __strcpy_chk.  */
-		  fn = builtin_decl_explicit (BUILT_IN_STRCPY_CHK);
-		  if (!fn)
-		    return false;
-
-		  gimple *repl = gimple_build_call (fn, 3, dest, src, size);
-		  replace_call_with_call_and_fold (gsi, repl);
-		  return true;
-		}
-
-	      if (! len || TREE_SIDE_EFFECTS (len))
+	      if (! ignore)
 		return false;
 
-	      /* If c_strlen returned something, but not a constant,
-		 transform __strcpy_chk into __memcpy_chk.  */
-	      fn = builtin_decl_explicit (BUILT_IN_MEMCPY_CHK);
+	      /* If return value of __stpcpy_chk is ignored,
+		 optimize into __strcpy_chk.  */
+	      fn = builtin_decl_explicit (BUILT_IN_STRCPY_CHK);
 	      if (!fn)
 		return false;
 
-	      gimple_seq stmts = NULL;
-	      len = force_gimple_operand (len, &stmts, true, NULL_TREE);
-	      len = gimple_convert (&stmts, loc, size_type_node, len);
-	      len = gimple_build (&stmts, loc, PLUS_EXPR, size_type_node, len,
-				  build_int_cst (size_type_node, 1));
-	      gsi_insert_seq_before (gsi, stmts, GSI_SAME_STMT);
-	      gimple *repl = gimple_build_call (fn, 4, dest, src, len, size);
+	      gimple *repl = gimple_build_call (fn, 3, dest, src, size);
 	      replace_call_with_call_and_fold (gsi, repl);
 	      return true;
 	    }
-	}
-      else
-	maxlen = len;
 
-      if (! tree_int_cst_lt (maxlen, size))
-	return false;
+	  if (! len || TREE_SIDE_EFFECTS (len))
+	    return false;
+
+	  /* If c_strlen returned something, but not a constant,
+	     transform __strcpy_chk into __memcpy_chk.  */
+	  fn = builtin_decl_explicit (BUILT_IN_MEMCPY_CHK);
+	  if (!fn)
+	    return false;
+
+	  gimple_seq stmts = NULL;
+	  len = force_gimple_operand (len, &stmts, true, NULL_TREE);
+	  len = gimple_convert (&stmts, loc, size_type_node, len);
+	  len = gimple_build (&stmts, loc, PLUS_EXPR, size_type_node, len,
+			      build_int_cst (size_type_node, 1));
+	  gsi_insert_seq_before (gsi, stmts, GSI_SAME_STMT);
+	  gimple *repl = gimple_build_call (fn, 4, dest, src, len, size);
+	  replace_call_with_call_and_fold (gsi, repl);
+	  return true;
+	}
     }
 
   /* If __builtin_st{r,p}cpy_chk is used, assume st{r,p}cpy is available.  */
@@ -3222,26 +3211,10 @@ gimple_fold_builtin_stxncpy_chk (gimple_stmt_iterator *gsi,
 	 }
     }
 
-  if (! tree_fits_uhwi_p (size))
-    return false;
-
   tree maxlen = get_maxval_strlen (len, SRK_INT_VALUE);
-  if (! integer_all_onesp (size))
-    {
-      if (! tree_fits_uhwi_p (len))
-	{
-	  /* If LEN is not constant, try MAXLEN too.
-	     For MAXLEN only allow optimizing into non-_ocs function
-	     if SIZE is >= MAXLEN, never convert to __ocs_fail ().  */
-	  if (maxlen == NULL_TREE || ! tree_fits_uhwi_p (maxlen))
-	    return false;
-	}
-      else
-	maxlen = len;
-
-      if (tree_int_cst_lt (size, maxlen))
-	return false;
-    }
+  if (! integer_all_onesp (size)
+      && !known_safe (stmt, len, size) && !known_safe (stmt, maxlen, size))
+    return false;
 
   /* If __builtin_st{r,p}ncpy_chk is used, assume st{r,p}ncpy is available.  */
   fn = builtin_decl_explicit (fcode == BUILT_IN_STPNCPY_CHK
@@ -3359,27 +3332,11 @@ gimple_fold_builtin_snprintf_chk (gimple_stmt_iterator *gsi,
   size = gimple_call_arg (stmt, 3);
   fmt = gimple_call_arg (stmt, 4);
 
-  if (! tree_fits_uhwi_p (size))
+  tree maxlen = get_maxval_strlen (len, SRK_INT_VALUE);
+  if (! integer_all_onesp (size)
+      && !known_safe (stmt, len, size) && !known_safe (stmt, maxlen, size))
     return false;
 
-  if (! integer_all_onesp (size))
-    {
-      tree maxlen = get_maxval_strlen (len, SRK_INT_VALUE);
-      if (! tree_fits_uhwi_p (len))
-	{
-	  /* If LEN is not constant, try MAXLEN too.
-	     For MAXLEN only allow optimizing into non-_ocs function
-	     if SIZE is >= MAXLEN, never convert to __ocs_fail ().  */
-	  if (maxlen == NULL_TREE || ! tree_fits_uhwi_p (maxlen))
-	    return false;
-	}
-      else
-	maxlen = len;
-
-      if (tree_int_cst_lt (size, maxlen))
-	return false;
-    }
-
   if (!init_target_chars ())
     return false;
 
@@ -3438,9 +3395,6 @@ gimple_fold_builtin_sprintf_chk (gimple_stmt_iterator *gsi,
   size = gimple_call_arg (stmt, 2);
   fmt = gimple_call_arg (stmt, 3);
 
-  if (! tree_fits_uhwi_p (size))
-    return false;
-
   len = NULL_TREE;
 
   if (!init_target_chars ())
@@ -3454,7 +3408,7 @@ gimple_fold_builtin_sprintf_chk (gimple_stmt_iterator *gsi,
       if (strchr (fmt_str, target_percent) == 0)
 	{
 	  if (fcode != BUILT_IN_SPRINTF_CHK || nargs == 4)
-	    len = build_int_cstu (size_type_node, strlen (fmt_str));
+	    len = build_int_cstu (size_type_node, strlen (fmt_str) + 1);
 	}
       /* If the format is "%s" and first ... argument is a string literal,
 	 we know the size too.  */
@@ -3468,19 +3422,18 @@ gimple_fold_builtin_sprintf_chk (gimple_stmt_iterator *gsi,
 	      arg = gimple_call_arg (stmt, 4);
 	      if (POINTER_TYPE_P (TREE_TYPE (arg)))
 		{
-		  len = c_strlen (arg, 1);
 		  if (! len || ! tree_fits_uhwi_p (len))
 		    len = NULL_TREE;
+		  else
+		    len = fold_build2 (PLUS_EXPR, ssizetype, c_strlen (arg, 1),
+				       ssize_int (1));
 		}
 	    }
 	}
     }
 
-  if (! integer_all_onesp (size))
-    {
-      if (! len || ! tree_int_cst_lt (len, size))
-	return false;
-    }
+  if (! integer_all_onesp (size) && !known_safe (stmt, len, size))
+    return false;
 
   /* Only convert __{,v}sprintf_chk to {,v}sprintf if flag is 0
      or if format doesn't contain % chars or is "%s".  */
diff --git a/gcc/testsuite/gcc.dg/Wobjsize-1.c b/gcc/testsuite/gcc.dg/Wobjsize-1.c
index 2bd2f93897b..988b8bcbf35 100644
--- a/gcc/testsuite/gcc.dg/Wobjsize-1.c
+++ b/gcc/testsuite/gcc.dg/Wobjsize-1.c
@@ -7,11 +7,12 @@ char buf[6];
 
 int main(int argc, char **argv)
 {
-  strcpy (buf,"hello ");    /* { dg-warning "\\\[-Wstringop-overflow" } */
+  strcpy (buf,"hello ");
   return 0;
 }
 
-/* { dg-message "file included" "included" { target *-*-* } 0 }
+/* { dg-warning "\\\[-Wstringop-overflow" "warning" { target *-*-* } 0 }
+   { dg-message "file included" "included" { target *-*-* } 0 }
    { dg-message "inlined from" "inlined" { target *-*-* } 0 }
 
    The test might emit two warnings, one for the strcpy call and
diff --git a/gcc/testsuite/gcc.dg/builtin-chk-fold.c b/gcc/testsuite/gcc.dg/builtin-chk-fold.c
new file mode 100644
index 00000000000..0b415dfaf57
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/builtin-chk-fold.c
@@ -0,0 +1,49 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+#define bos(__d) __builtin_object_size ((__d), 0)
+
+char *
+safe1 (const char *src, int cond, __SIZE_TYPE__ len)
+{
+  char *dst;
+
+  if (cond)
+    dst = __builtin_malloc (1024);
+  else
+    dst = __builtin_malloc (2048);
+
+  len = len > 2048 ? 2048 : len;
+
+  return __builtin___memcpy_chk (dst, src, len, bos (dst));
+}
+
+char *
+safe2 (const char *src, int cond, unsigned char len)
+{
+  char *dst;
+
+  if (cond)
+    dst = __builtin_malloc (1024);
+  else
+    dst = __builtin_malloc (2048);
+
+  return __builtin___strncpy_chk (dst, src, len, bos (dst));
+}
+
+int
+safe3 (const char *src, int cond, unsigned char len)
+{
+  char *dst;
+
+  if (cond)
+    dst = __builtin_malloc (1024);
+  else
+    dst = __builtin_malloc (2048);
+
+  return __builtin___snprintf_chk (dst, len, 0, bos (dst), "%s", src);
+}
+
+/* { dg-final { scan-assembler-not "__memcpy_chk" } } */
+/* { dg-final { scan-assembler-not "__strncpy_chk" } } */
+/* { dg-final { scan-assembler-not "__snprintf_chk" } } */
-- 
2.31.1


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] gimple-fold: Smarter optimization of _chk variants
  2021-11-10 10:45 ` [PATCH v2] " Siddhesh Poyarekar
@ 2021-11-10 12:20   ` Siddhesh Poyarekar
  0 siblings, 0 replies; 3+ messages in thread
From: Siddhesh Poyarekar @ 2021-11-10 12:20 UTC (permalink / raw)
  To: gcc-patches; +Cc: jakub, msebor

On 11/10/21 16:15, Siddhesh Poyarekar wrote:
> Instead of comparing LEN and SIZE only if they are constants, use their
> ranges to decide if LEN will always be lower than or same as SIZE.
> 
> This change ends up putting the stringop-overflow warning line number
> against the strcpy implementation, so adjust the warning check to be
> line number agnostic.
> 
> gcc/ChangeLog:
> 
> 	* gimple-fold.c (known_safe): New function.
> 	(gimple_fold_builtin_memory_chk, gimple_fold_builtin_stxcpy_chk,
> 	gimple_fold_builtin_stxncpy_chk,
> 	gimple_fold_builtin_snprintf_chk,
> 	gimple_fold_builtin_sprintf_chk): Use it.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* gcc.dg/Wobjsize-1.c: Make warning change line agnostic.
> 	* gcc.dg/builtin-chk-fold.c: New test.
> 
> Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
> ---
> Changes from v1:
> - Update comment that incorrectly said that known_safe emits a warning.
> - Add tests for strncpy and snprintf too.

Sorry, this is failing some torture tests.  I'll fix up and send another 
version.

Siddhesh

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-11-10 12:20 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-10  9:38 [PATCH] gimple-fold: Smarter optimization of _chk variants Siddhesh Poyarekar
2021-11-10 10:45 ` [PATCH v2] " Siddhesh Poyarekar
2021-11-10 12:20   ` Siddhesh Poyarekar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).