diff mbox series

[v2,2/3] gimple-fold: Use ranges to simplify _chk calls

Message ID 20211115173315.1857598-3-siddhesh@gotplt.org
State New
Headers show
Series gimple-fold improvements | expand

Commit Message

Siddhesh Poyarekar Nov. 15, 2021, 5:33 p.m. UTC 
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_lower): New function.
	(gimple_fold_builtin_strncat_chk,
	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>
---
 gcc/gimple-fold.c                       | 216 +++++++++---------------
 gcc/testsuite/gcc.dg/Wobjsize-1.c       |   5 +-
 gcc/testsuite/gcc.dg/fold-stringops-2.c |  49 ++++++
 3 files changed, 132 insertions(+), 138 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/fold-stringops-2.c

Comments

Jeff Law Nov. 15, 2021, 8:25 p.m. UTC | #1 
On 11/15/2021 10:33 AM, 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_lower): New function.
> 	(gimple_fold_builtin_strncat_chk,
> 	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.
>
>

> @@ -3024,39 +3040,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_lower (stmt, len, size) && !known_lower (stmt, maxlen, size))
Formatting it.  Move the trailing && !known_lower (...) to its own line.

OK with the formatting nit fixed.

jeff
Siddhesh Poyarekar Nov. 15, 2021, 10:53 p.m. UTC | #2 
On 11/16/21 01:55, Jeff Law wrote:
> 
> 
> On 11/15/2021 10:33 AM, 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_lower): New function.
>>     (gimple_fold_builtin_strncat_chk,
>>     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.
>>
>>
> 
>> @@ -3024,39 +3040,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_lower (stmt, len, size) && !known_lower (stmt, 
>> maxlen, size))
> Formatting it.  Move the trailing && !known_lower (...) to its own line.
> 
> OK with the formatting nit fixed.

Thanks, I fixed the nit and pushed the series.

Siddhesh
diff mbox series

Patch

diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index 2e92efa7f61..f3362287c0d 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -2031,6 +2031,28 @@  get_maxval_strlen (tree arg, strlen_range_kind rkind, tree *nonstr = NULL)
   return lendata.decl ? NULL_TREE : lendata.maxlen;
 }
 
+/* Return true if LEN is known to be less than or equal to (or if STRICT is
+   true, strictly less than) the lower bound of SIZE at compile time and false
+   otherwise.  */
+
+static bool
+known_lower (gimple *stmt, tree len, tree size, bool strict = false)
+{
+  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))
+    {
+      if (strict)
+	return wi::ltu_p (len_range[1], size_range[0]);
+      else
+       return wi::leu_p (len_range[1], size_range[0]);
+    }
+
+  return false;
+}
 
 /* Fold function call to builtin strcpy with arguments DEST and SRC.
    If LEN is not NULL, it represents the length of the string to be
@@ -2566,16 +2588,10 @@  gimple_fold_builtin_strncat_chk (gimple_stmt_iterator *gsi)
       return true;
     }
 
-  if (! tree_fits_uhwi_p (size))
-    return false;
-
   if (! integer_all_onesp (size))
     {
       tree src_len = c_strlen (src, 1);
-      if (src_len
-	  && tree_fits_uhwi_p (src_len)
-	  && tree_fits_uhwi_p (len)
-	  && ! tree_int_cst_lt (len, src_len))
+      if (known_lower (stmt, src_len, len))
 	{
 	  /* If LEN >= strlen (SRC), optimize into __strcat_chk.  */
 	  fn = builtin_decl_explicit (BUILT_IN_STRCAT_CHK);
@@ -3024,39 +3040,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_lower (stmt, len, size) && !known_lower (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;
@@ -3136,61 +3137,48 @@  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_lower (stmt, len, size, true)
+	  && !known_lower (stmt, maxlen, size, true))
 	{
-	  /* 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 provably less than size,
+	     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.  */
@@ -3220,41 +3208,23 @@  gimple_fold_builtin_stxncpy_chk (gimple_stmt_iterator *gsi,
   bool ignore = gimple_call_lhs (stmt) == NULL_TREE;
   tree fn;
 
-  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_lower (stmt, len, size) && !known_lower (stmt, maxlen, size))
     {
-      if (! tree_fits_uhwi_p (len))
+      if (fcode == BUILT_IN_STPNCPY_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 return value of __stpncpy_chk is ignored,
+	     optimize into __strncpy_chk.  */
+	  fn = builtin_decl_explicit (BUILT_IN_STRNCPY_CHK);
+	  if (fn)
 	    {
-	      if (fcode == BUILT_IN_STPNCPY_CHK && ignore)
-		{
-		  /* If return value of __stpncpy_chk is ignored,
-		     optimize into __strncpy_chk.  */
-		  fn = builtin_decl_explicit (BUILT_IN_STRNCPY_CHK);
-		  if (fn)
-		    {
-		      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;
     }
 
   /* If __builtin_st{r,p}ncpy_chk is used, assume st{r,p}ncpy is available.  */
@@ -3374,27 +3344,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_lower (stmt, len, size) && !known_lower (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;
 
@@ -3453,9 +3407,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 ())
@@ -3482,20 +3433,13 @@  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;
-		}
+		len = c_strlen (arg, 1);
 	    }
 	}
     }
 
-  if (! integer_all_onesp (size))
-    {
-      if (! len || ! tree_int_cst_lt (len, size))
-	return false;
-    }
+  if (! integer_all_onesp (size) && !known_lower (stmt, len, size, true))
+    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/fold-stringops-2.c b/gcc/testsuite/gcc.dg/fold-stringops-2.c
new file mode 100644
index 00000000000..0b415dfaf57
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/fold-stringops-2.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" } } */