[PATCH 0/3] gimple-fold improvements

[PATCH 0/3] gimple-fold improvements

[Siddhesh Poyarekar]

This patchset improves folding in cases where input lengths
and/or destination sizes may not be constant but are range bound.

Tested on x86_64 with a full bootstrap build and verified that there are
no regressions resulting from this patchset.

Also tested a build of bash, which results in 3 __memcpy_chk calls being
optimized away completely in addition to a couple of memmove and strcpy
chk variants being transformed into regular calls.

Siddhesh Poyarekar (3):
  gimple-fold: Transform stp*cpy_chk to str*cpy directly
  gimple-fold: Use ranges to simplify _chk calls
  gimple-fold: Use ranges to simplify strncat and snprintf

 gcc/gimple-fold.c                       | 312 +++++++++---------------
 gcc/testsuite/gcc.dg/Wobjsize-1.c       |   5 +-
 gcc/testsuite/gcc.dg/fold-stringops-1.c |  23 ++
 gcc/testsuite/gcc.dg/fold-stringops-2.c |  63 +++++
 gcc/testsuite/gcc.dg/fold-stringops-3.c |  18 ++
 5 files changed, 220 insertions(+), 201 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/fold-stringops-1.c
 create mode 100644 gcc/testsuite/gcc.dg/fold-stringops-2.c
 create mode 100644 gcc/testsuite/gcc.dg/fold-stringops-3.c

-- 
2.31.1

[PATCH 1/3] gimple-fold: Transform stp*cpy_chk to str*cpy directly

[Siddhesh Poyarekar]

Avoid going through another folding cycle and use the ignore flag to
directly transform BUILT_IN_STPCPY_CHK to BUILT_IN_STRCPY when set,
likewise for BUILT_IN_STPNCPY_CHK to BUILT_IN_STPNCPY.

Dump the transformation in dump_file so that we can verify in tests that
the direct transformation actually happened.

gcc/ChangeLog:

	* gimple-fold.c (gimple_fold_builtin_stxcpy_chk,
	gimple_fold_builtin_stxncpy_chk): Use BUILT_IN_STRNCPY if return
	value is not used.

gcc/testsuite/ChangeLog:

	* gcc.dg/fold-stringops.c: New test.

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

diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index 6e25a7c05db..92e15784803 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -3088,6 +3088,19 @@ gimple_fold_builtin_memory_chk (gimple_stmt_iterator *gsi,
   return true;
 }
 
+static void
+dump_transformation (gimple *from, gimple *to)
+{
+  if (dump_file && (dump_flags & TDF_DETAILS))
+    {
+      fprintf (dump_file, "transformed ");
+      print_generic_expr (dump_file, gimple_call_fn (from), dump_flags);
+      fprintf (dump_file, " to ");
+      print_generic_expr (dump_file, gimple_call_fn (to), dump_flags);
+      fprintf (dump_file, "\n");
+    }
+}
+
 /* Fold a call to the __st[rp]cpy_chk builtin.
    DEST, SRC, and SIZE are the arguments to the call.
    IGNORE is true if return value can be ignored.  FCODE is the BUILT_IN_*
@@ -3184,12 +3197,13 @@ gimple_fold_builtin_stxcpy_chk (gimple_stmt_iterator *gsi,
     }
 
   /* If __builtin_st{r,p}cpy_chk is used, assume st{r,p}cpy is available.  */
-  fn = builtin_decl_explicit (fcode == BUILT_IN_STPCPY_CHK
+  fn = builtin_decl_explicit (fcode == BUILT_IN_STPCPY_CHK && !ignore
 			      ? BUILT_IN_STPCPY : BUILT_IN_STRCPY);
   if (!fn)
     return false;
 
   gimple *repl = gimple_build_call (fn, 2, dest, src);
+  dump_transformation (stmt, repl);
   replace_call_with_call_and_fold (gsi, repl);
   return true;
 }
@@ -3209,19 +3223,6 @@ gimple_fold_builtin_stxncpy_chk (gimple_stmt_iterator *gsi,
   bool ignore = gimple_call_lhs (stmt) == NULL_TREE;
   tree 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;
-	 }
-    }
-
   if (! tree_fits_uhwi_p (size))
     return false;
 
@@ -3234,7 +3235,23 @@ gimple_fold_builtin_stxncpy_chk (gimple_stmt_iterator *gsi,
 	     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;
+	    {
+	      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;
+	    }
 	}
       else
 	maxlen = len;
@@ -3244,12 +3261,13 @@ gimple_fold_builtin_stxncpy_chk (gimple_stmt_iterator *gsi,
     }
 
   /* 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
+  fn = builtin_decl_explicit (fcode == BUILT_IN_STPNCPY_CHK && !ignore
 			      ? BUILT_IN_STPNCPY : BUILT_IN_STRNCPY);
   if (!fn)
     return false;
 
   gimple *repl = gimple_build_call (fn, 3, dest, src, len);
+  dump_transformation (stmt, repl);
   replace_call_with_call_and_fold (gsi, repl);
   return true;
 }
diff --git a/gcc/testsuite/gcc.dg/fold-stringops-1.c b/gcc/testsuite/gcc.dg/fold-stringops-1.c
new file mode 100644
index 00000000000..712dd369e7c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/fold-stringops-1.c
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-lower-details" } */
+
+char dst[2048];
+
+char *
+copy1 (const char *src, int cond)
+{
+  __builtin___stpncpy_chk (dst, src, 42, __builtin_object_size (dst, 0));
+
+  return dst;
+}
+
+char *
+copy2 (void)
+{
+  __builtin___stpcpy_chk (dst, "Hello world", __builtin_object_size (dst, 0));
+
+  return dst;
+}
+/* { dg-final { scan-tree-dump "transformed __builtin___stpncpy_chk to __builtin_strncpy" "lower" } } */
+/* { dg-final { scan-tree-dump "transformed __builtin___stpcpy_chk to __builtin_strcpy" "lower" } } */
+
-- 
2.31.1

Re: [PATCH 1/3] gimple-fold: Transform stp*cpy_chk to str*cpy directly

[Prathamesh Kulkarni]

On Fri, 12 Nov 2021 at 01:12, Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
>
> Avoid going through another folding cycle and use the ignore flag to
> directly transform BUILT_IN_STPCPY_CHK to BUILT_IN_STRCPY when set,
> likewise for BUILT_IN_STPNCPY_CHK to BUILT_IN_STPNCPY.
>
> Dump the transformation in dump_file so that we can verify in tests that
> the direct transformation actually happened.
>
> gcc/ChangeLog:
>
>         * gimple-fold.c (gimple_fold_builtin_stxcpy_chk,
>         gimple_fold_builtin_stxncpy_chk): Use BUILT_IN_STRNCPY if return
>         value is not used.
>
> gcc/testsuite/ChangeLog:
>
>         * gcc.dg/fold-stringops.c: New test.
>
> Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
> ---
>  gcc/gimple-fold.c                       | 50 +++++++++++++++++--------
>  gcc/testsuite/gcc.dg/fold-stringops-1.c | 23 ++++++++++++
>  2 files changed, 57 insertions(+), 16 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/fold-stringops-1.c
>
> diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
> index 6e25a7c05db..92e15784803 100644
> --- a/gcc/gimple-fold.c
> +++ b/gcc/gimple-fold.c
> @@ -3088,6 +3088,19 @@ gimple_fold_builtin_memory_chk (gimple_stmt_iterator *gsi,
>    return true;
>  }
>
> +static void
> +dump_transformation (gimple *from, gimple *to)
I assume that both from and to will always be builtin calls ?
In that case, perhaps better to use gcall * here (and in rest of patch).
Also, needs a top-level comment describing the function.
> +{
> +  if (dump_file && (dump_flags & TDF_DETAILS))
Perhaps better to use dump_enabled_p ?
> +    {
> +      fprintf (dump_file, "transformed ");
Perhaps use dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, ...) ?
I think you can use gimple_location to get the location.

Thanks,
Prathamesh
> +      print_generic_expr (dump_file, gimple_call_fn (from), dump_flags);
> +      fprintf (dump_file, " to ");
> +      print_generic_expr (dump_file, gimple_call_fn (to), dump_flags);
> +      fprintf (dump_file, "\n");
> +    }
> +}
> +
>  /* Fold a call to the __st[rp]cpy_chk builtin.
>     DEST, SRC, and SIZE are the arguments to the call.
>     IGNORE is true if return value can be ignored.  FCODE is the BUILT_IN_*
> @@ -3184,12 +3197,13 @@ gimple_fold_builtin_stxcpy_chk (gimple_stmt_iterator *gsi,
>      }
>
>    /* If __builtin_st{r,p}cpy_chk is used, assume st{r,p}cpy is available.  */
> -  fn = builtin_decl_explicit (fcode == BUILT_IN_STPCPY_CHK
> +  fn = builtin_decl_explicit (fcode == BUILT_IN_STPCPY_CHK && !ignore
>                               ? BUILT_IN_STPCPY : BUILT_IN_STRCPY);
>    if (!fn)
>      return false;
>
>    gimple *repl = gimple_build_call (fn, 2, dest, src);
> +  dump_transformation (stmt, repl);
>    replace_call_with_call_and_fold (gsi, repl);
>    return true;
>  }
> @@ -3209,19 +3223,6 @@ gimple_fold_builtin_stxncpy_chk (gimple_stmt_iterator *gsi,
>    bool ignore = gimple_call_lhs (stmt) == NULL_TREE;
>    tree 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;
> -        }
> -    }
> -
>    if (! tree_fits_uhwi_p (size))
>      return false;
>
> @@ -3234,7 +3235,23 @@ gimple_fold_builtin_stxncpy_chk (gimple_stmt_iterator *gsi,
>              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;
> +           {
> +             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;
> +           }
>         }
>        else
>         maxlen = len;
> @@ -3244,12 +3261,13 @@ gimple_fold_builtin_stxncpy_chk (gimple_stmt_iterator *gsi,
>      }
>
>    /* 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
> +  fn = builtin_decl_explicit (fcode == BUILT_IN_STPNCPY_CHK && !ignore
>                               ? BUILT_IN_STPNCPY : BUILT_IN_STRNCPY);
>    if (!fn)
>      return false;
>
>    gimple *repl = gimple_build_call (fn, 3, dest, src, len);
> +  dump_transformation (stmt, repl);
>    replace_call_with_call_and_fold (gsi, repl);
>    return true;
>  }
> diff --git a/gcc/testsuite/gcc.dg/fold-stringops-1.c b/gcc/testsuite/gcc.dg/fold-stringops-1.c
> new file mode 100644
> index 00000000000..712dd369e7c
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/fold-stringops-1.c
> @@ -0,0 +1,23 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-lower-details" } */
> +
> +char dst[2048];
> +
> +char *
> +copy1 (const char *src, int cond)
> +{
> +  __builtin___stpncpy_chk (dst, src, 42, __builtin_object_size (dst, 0));
> +
> +  return dst;
> +}
> +
> +char *
> +copy2 (void)
> +{
> +  __builtin___stpcpy_chk (dst, "Hello world", __builtin_object_size (dst, 0));
> +
> +  return dst;
> +}
> +/* { dg-final { scan-tree-dump "transformed __builtin___stpncpy_chk to __builtin_strncpy" "lower" } } */
> +/* { dg-final { scan-tree-dump "transformed __builtin___stpcpy_chk to __builtin_strcpy" "lower" } } */
> +
> --
> 2.31.1
>

Re: [PATCH 1/3] gimple-fold: Transform stp*cpy_chk to str*cpy directly

[Siddhesh Poyarekar]

On 11/12/21 22:46, Prathamesh Kulkarni wrote:
> On Fri, 12 Nov 2021 at 01:12, Siddhesh Poyarekar <siddhesh@gotplt.org> wrote:
>>
>> Avoid going through another folding cycle and use the ignore flag to
>> directly transform BUILT_IN_STPCPY_CHK to BUILT_IN_STRCPY when set,
>> likewise for BUILT_IN_STPNCPY_CHK to BUILT_IN_STPNCPY.
>>
>> Dump the transformation in dump_file so that we can verify in tests that
>> the direct transformation actually happened.
>>
>> gcc/ChangeLog:
>>
>>          * gimple-fold.c (gimple_fold_builtin_stxcpy_chk,
>>          gimple_fold_builtin_stxncpy_chk): Use BUILT_IN_STRNCPY if return
>>          value is not used.
>>
>> gcc/testsuite/ChangeLog:
>>
>>          * gcc.dg/fold-stringops.c: New test.
>>
>> Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
>> ---
>>   gcc/gimple-fold.c                       | 50 +++++++++++++++++--------
>>   gcc/testsuite/gcc.dg/fold-stringops-1.c | 23 ++++++++++++
>>   2 files changed, 57 insertions(+), 16 deletions(-)
>>   create mode 100644 gcc/testsuite/gcc.dg/fold-stringops-1.c
>>
>> diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
>> index 6e25a7c05db..92e15784803 100644
>> --- a/gcc/gimple-fold.c
>> +++ b/gcc/gimple-fold.c
>> @@ -3088,6 +3088,19 @@ gimple_fold_builtin_memory_chk (gimple_stmt_iterator *gsi,
>>     return true;
>>   }
>>
>> +static void
>> +dump_transformation (gimple *from, gimple *to)
> I assume that both from and to will always be builtin calls ?
> In that case, perhaps better to use gcall * here (and in rest of patch).
> Also, needs a top-level comment describing the function.
>> +{
>> +  if (dump_file && (dump_flags & TDF_DETAILS))
> Perhaps better to use dump_enabled_p ?
>> +    {
>> +      fprintf (dump_file, "transformed ");
> Perhaps use dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, ...) ?
> I think you can use gimple_location to get the location.
> 

Thanks, I'll fix these up.

Siddhesh

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

[Siddhesh Poyarekar]

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_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

diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index 92e15784803..bcfd5d97feb 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 SIZE at compile time
+   and false otherwise.  EXTRABYTE is true if there needs to be room for an
+   extra NUL byte at the end.  */
+
+static bool
+known_safe (gimple *stmt, tree len, tree size, bool extrabyte = 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 (extrabyte)
+	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_safe (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_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;
@@ -3139,61 +3140,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_safe (stmt, len, size, true)
+	  && !known_safe (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.  */
@@ -3223,41 +3211,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_safe (stmt, len, size) && !known_safe (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.  */
@@ -3377,27 +3347,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;
 
@@ -3456,9 +3410,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 ())
@@ -3485,20 +3436,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_safe (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" } } */
-- 
2.31.1

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

[Siddhesh Poyarekar]

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

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

[Siddhesh Poyarekar]

On 11/12/21 01:11, Siddhesh Poyarekar wrote:
> 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.
> 

Sorry, it looks like my last test run was stale and didn't catch the 
regressions dropping warnings caused.  I'll bring back the warnings in 
v2 with ranges.

Siddhesh

[PATCH v2 0/3] gimple-fold improvements

[Siddhesh Poyarekar]

This patchset improves folding in cases where input lengths
and/or destination sizes may not be constant but are range bound.

Tested on x86_64 with a full bootstrap build and verified that there are
no regressions resulting from this patchset.  I double-checked that the
run was current and I wasn't checking logs of a stale build.

I tested builds of bash and wpa_supplicant with this patchset.
wpa_supplicant ends up with 30 fewer __memcpy_chk calls, of which 14 are
completely optimized away and the rest transformed to __memcpy. 

In bash, 3 __memcpy_chk calls are optimized away completely in addition
to a couple of memmove and strcpy chk variants being transformed into
regular calls.

Changes from v1:

- Use dump_* functions instead of directly using dump_file
- Bring back warnings and reduce scope of changes to strncat to only
  using ranges to determine if call can be simplified to strcat.
- Renamed known_safe to known_lower

Siddhesh Poyarekar (3):
  gimple-fold: Transform stp*cpy_chk to str*cpy directly
  gimple-fold: Use ranges to simplify _chk calls
  gimple-fold: Use ranges to simplify strncat and snprintf

 gcc/gimple-fold.c                       | 343 ++++++++++--------------
 gcc/testsuite/gcc.dg/Wobjsize-1.c       |   5 +-
 gcc/testsuite/gcc.dg/fold-stringops-1.c |  23 ++
 gcc/testsuite/gcc.dg/fold-stringops-2.c |  63 +++++
 gcc/testsuite/gcc.dg/fold-stringops-3.c |  18 ++
 5 files changed, 256 insertions(+), 196 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/fold-stringops-1.c
 create mode 100644 gcc/testsuite/gcc.dg/fold-stringops-2.c
 create mode 100644 gcc/testsuite/gcc.dg/fold-stringops-3.c

-- 
2.31.1

[PATCH v2 1/3] gimple-fold: Transform stp*cpy_chk to str*cpy directly

[Siddhesh Poyarekar]

Avoid going through another folding cycle and use the ignore flag to
directly transform BUILT_IN_STPCPY_CHK to BUILT_IN_STRCPY when set,
likewise for BUILT_IN_STPNCPY_CHK to BUILT_IN_STPNCPY.

Dump the transformation in dump_file so that we can verify in tests that
the direct transformation actually happened.

gcc/ChangeLog:

	* gimple-fold.c (dump_transformation): New function.
	(gimple_fold_builtin_stxcpy_chk,
	gimple_fold_builtin_stxncpy_chk): Use it.  Simplify to
	BUILT_IN_STRNCPY if return value is not used.

gcc/testsuite/ChangeLog:

	* gcc.dg/fold-stringops.c: New test.

Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
---
 gcc/gimple-fold.c                       | 55 ++++++++++++++++---------
 gcc/testsuite/gcc.dg/fold-stringops-1.c | 23 +++++++++++
 2 files changed, 58 insertions(+), 20 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/fold-stringops-1.c

diff --git a/gcc/gimple-fold.c b/gcc/gimple-fold.c
index 6e25a7c05db..2e92efa7f61 100644
--- a/gcc/gimple-fold.c
+++ b/gcc/gimple-fold.c
@@ -3088,6 +3088,16 @@ gimple_fold_builtin_memory_chk (gimple_stmt_iterator *gsi,
   return true;
 }
 
+/* Print a message in the dump file recording transformation of FROM to TO.  */
+
+static void
+dump_transformation (gcall *from, gcall *to)
+{
+  if (dump_enabled_p ())
+    dump_printf_loc (MSG_OPTIMIZED_LOCATIONS, from, "simplified %T to %T\n",
+		     gimple_call_fn (from), gimple_call_fn (to));
+}
+
 /* Fold a call to the __st[rp]cpy_chk builtin.
    DEST, SRC, and SIZE are the arguments to the call.
    IGNORE is true if return value can be ignored.  FCODE is the BUILT_IN_*
@@ -3100,7 +3110,7 @@ gimple_fold_builtin_stxcpy_chk (gimple_stmt_iterator *gsi,
 				tree src, tree size,
 				enum built_in_function fcode)
 {
-  gimple *stmt = gsi_stmt (*gsi);
+  gcall *stmt = as_a <gcall *> (gsi_stmt (*gsi));
   location_t loc = gimple_location (stmt);
   bool ignore = gimple_call_lhs (stmt) == NULL_TREE;
   tree len, fn;
@@ -3184,12 +3194,13 @@ gimple_fold_builtin_stxcpy_chk (gimple_stmt_iterator *gsi,
     }
 
   /* If __builtin_st{r,p}cpy_chk is used, assume st{r,p}cpy is available.  */
-  fn = builtin_decl_explicit (fcode == BUILT_IN_STPCPY_CHK
+  fn = builtin_decl_explicit (fcode == BUILT_IN_STPCPY_CHK && !ignore
 			      ? BUILT_IN_STPCPY : BUILT_IN_STRCPY);
   if (!fn)
     return false;
 
-  gimple *repl = gimple_build_call (fn, 2, dest, src);
+  gcall *repl = gimple_build_call (fn, 2, dest, src);
+  dump_transformation (stmt, repl);
   replace_call_with_call_and_fold (gsi, repl);
   return true;
 }
@@ -3205,23 +3216,10 @@ gimple_fold_builtin_stxncpy_chk (gimple_stmt_iterator *gsi,
 				 tree len, tree size,
 				 enum built_in_function fcode)
 {
-  gimple *stmt = gsi_stmt (*gsi);
+  gcall *stmt = as_a <gcall *> (gsi_stmt (*gsi));
   bool ignore = gimple_call_lhs (stmt) == NULL_TREE;
   tree 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;
-	 }
-    }
-
   if (! tree_fits_uhwi_p (size))
     return false;
 
@@ -3234,7 +3232,23 @@ gimple_fold_builtin_stxncpy_chk (gimple_stmt_iterator *gsi,
 	     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;
+	    {
+	      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;
+	    }
 	}
       else
 	maxlen = len;
@@ -3244,12 +3258,13 @@ gimple_fold_builtin_stxncpy_chk (gimple_stmt_iterator *gsi,
     }
 
   /* 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
+  fn = builtin_decl_explicit (fcode == BUILT_IN_STPNCPY_CHK && !ignore
 			      ? BUILT_IN_STPNCPY : BUILT_IN_STRNCPY);
   if (!fn)
     return false;
 
-  gimple *repl = gimple_build_call (fn, 3, dest, src, len);
+  gcall *repl = gimple_build_call (fn, 3, dest, src, len);
+  dump_transformation (stmt, repl);
   replace_call_with_call_and_fold (gsi, repl);
   return true;
 }
diff --git a/gcc/testsuite/gcc.dg/fold-stringops-1.c b/gcc/testsuite/gcc.dg/fold-stringops-1.c
new file mode 100644
index 00000000000..e26632afd4e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/fold-stringops-1.c
@@ -0,0 +1,23 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-lower-details" } */
+
+char dst[2048];
+
+char *
+copy1 (const char *src, int cond)
+{
+  __builtin___stpncpy_chk (dst, src, 42, __builtin_object_size (dst, 0));
+
+  return dst;
+}
+
+char *
+copy2 (void)
+{
+  __builtin___stpcpy_chk (dst, "Hello world", __builtin_object_size (dst, 0));
+
+  return dst;
+}
+/* { dg-final { scan-tree-dump "simplified __builtin___stpncpy_chk to __builtin_strncpy" "lower" } } */
+/* { dg-final { scan-tree-dump "simplified __builtin___stpcpy_chk to __builtin_strcpy" "lower" } } */
+
-- 
2.31.1

Re: [PATCH v2 1/3] gimple-fold: Transform stp*cpy_chk to str*cpy directly

[Jeff Law]

On 11/15/2021 10:33 AM, Siddhesh Poyarekar wrote:
> Avoid going through another folding cycle and use the ignore flag to
> directly transform BUILT_IN_STPCPY_CHK to BUILT_IN_STRCPY when set,
> likewise for BUILT_IN_STPNCPY_CHK to BUILT_IN_STPNCPY.
>
> Dump the transformation in dump_file so that we can verify in tests that
> the direct transformation actually happened.
>
> gcc/ChangeLog:
>
> 	* gimple-fold.c (dump_transformation): New function.
> 	(gimple_fold_builtin_stxcpy_chk,
> 	gimple_fold_builtin_stxncpy_chk): Use it.  Simplify to
> 	BUILT_IN_STRNCPY if return value is not used.
>
> gcc/testsuite/ChangeLog:
>
> 	* gcc.dg/fold-stringops.c: New test.
OK
jeff

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

[Siddhesh Poyarekar]

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

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" } } */
-- 
2.31.1

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

[Jeff Law]

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

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

[Siddhesh Poyarekar]

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

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

[Siddhesh Poyarekar]

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

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" } } */
-- 
2.31.1

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

[Jeff Law]

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