public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] correct range of stpcpy result (PR 101397)
@ 2021-07-15  1:49 Martin Sebor
  2021-07-20 16:08 ` Jeff Law
  0 siblings, 1 reply; 3+ messages in thread
From: Martin Sebor @ 2021-07-15  1:49 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 670 bytes --]

Access warnings look through calls to the subset of built-ins
that return one of their pointer arguments to find the object
the pointer it points to and its offset.  The computation is
wrong for functions like stpcpy, stpncpy and mempcpy that
return a pointer plus some offset, and leads to a false positive
-Warray-bounds in Glibc with the recent refactoring of the warning
to take advantage of this logic.

The attached patch corrects this mistake by accounting for this
property of these functions while at the same time constraining
the offset to the size of the source argument for better
accuracy.

Tested on x86_64-linux and by also building Glibc there.

Martin

[-- Attachment #2: gcc-101397.diff --]
[-- Type: text/x-patch, Size: 35309 bytes --]

PR middle-end/101397 - spurious warning writing to the result of stpcpy minus 1


gcc/ChangeLog:

	PR middle-end/101397
	* builtins.c (gimple_call_return_array): Add argument.  Correct
	offsets for memchr, mempcpy, stpcpy, and stpncpy.
	(compute_objsize_r): Adjust offset computation for argument returning
	built-ins.

gcc/testsuite/ChangeLog:

	PR middle-end/101397
	* gcc.dg/Warray-bounds-80.c: New test.
	* gcc.dg/Warray-bounds-81.c: New test.
	* gcc.dg/Warray-bounds-82.c: New test.
	* gcc.dg/Warray-bounds-83.c: New test.
	* gcc.dg/Warray-bounds-84.c: New test.
	* gcc.dg/Wstringop-overflow-46.c: Adjust expected output.

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 39ab139b7e1..170d776c410 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -5200,12 +5200,19 @@ get_offset_range (tree x, gimple *stmt, offset_int r[2], range_query *rvals)
 /* Return the argument that the call STMT to a built-in function returns
    or null if it doesn't.  On success, set OFFRNG[] to the range of offsets
    from the argument reflected in the value returned by the built-in if it
-   can be determined, otherwise to 0 and HWI_M1U respectively.  */
+   can be determined, otherwise to 0 and HWI_M1U respectively.  Set
+   *PAST_END for functions like mempcpy that might return a past the end
+   pointer (most functions return a dereferenceable pointer to an existing
+   element of an array).  */
 
 static tree
-gimple_call_return_array (gimple *stmt, offset_int offrng[2],
+gimple_call_return_array (gimple *stmt, offset_int offrng[2], bool *past_end,
 			  range_query *rvals)
 {
+  /* Clear and set below for the rare function(s) that might return
+     a past-the-end pointer.  */
+  *past_end = false;
+
   {
     /* Check for attribute fn spec to see if the function returns one
        of its arguments.  */
@@ -5213,6 +5220,7 @@ gimple_call_return_array (gimple *stmt, offset_int offrng[2],
     unsigned int argno;
     if (fnspec.returns_arg (&argno))
       {
+	/* Functions return the first argument (not a range).  */
 	offrng[0] = offrng[1] = 0;
 	return gimple_call_arg (stmt, argno);
       }
@@ -5242,6 +5250,7 @@ gimple_call_return_array (gimple *stmt, offset_int offrng[2],
       if (gimple_call_num_args (stmt) != 2)
 	return NULL_TREE;
 
+      /* Allocation functions return a pointer to the beginning.  */
       offrng[0] = offrng[1] = 0;
       return gimple_call_arg (stmt, 1);
     }
@@ -5253,10 +5262,6 @@ gimple_call_return_array (gimple *stmt, offset_int offrng[2],
     case BUILT_IN_MEMMOVE:
     case BUILT_IN_MEMMOVE_CHK:
     case BUILT_IN_MEMSET:
-    case BUILT_IN_STPCPY:
-    case BUILT_IN_STPCPY_CHK:
-    case BUILT_IN_STPNCPY:
-    case BUILT_IN_STPNCPY_CHK:
     case BUILT_IN_STRCAT:
     case BUILT_IN_STRCAT_CHK:
     case BUILT_IN_STRCPY:
@@ -5265,18 +5270,34 @@ gimple_call_return_array (gimple *stmt, offset_int offrng[2],
     case BUILT_IN_STRNCAT_CHK:
     case BUILT_IN_STRNCPY:
     case BUILT_IN_STRNCPY_CHK:
+      /* Functions return the first argument (not a range).  */
       offrng[0] = offrng[1] = 0;
       return gimple_call_arg (stmt, 0);
 
     case BUILT_IN_MEMPCPY:
     case BUILT_IN_MEMPCPY_CHK:
       {
+	/* The returned pointer is in a range constrained by the smaller
+	   of the upper bound of the size argument and the source object
+	   size.  */
+	offrng[0] = 0;
+	offrng[1] = HOST_WIDE_INT_M1U;
 	tree off = gimple_call_arg (stmt, 2);
-	if (!get_offset_range (off, stmt, offrng, rvals))
+	bool off_valid = get_offset_range (off, stmt, offrng, rvals);
+	if (!off_valid || offrng[0] != offrng[1])
 	  {
-	    offrng[0] = 0;
-	    offrng[1] = HOST_WIDE_INT_M1U;
+	    /* If the offset is either indeterminate or in some range,
+	       try to constrain its upper bound to at most the size
+	       of the source object.  */
+	    access_ref aref;
+	    tree src = gimple_call_arg (stmt, 1);
+	    if (compute_objsize (src, 1, &aref, rvals)
+		&& aref.sizrng[1] < offrng[1])
+	      offrng[1] = aref.sizrng[1];
 	  }
+
+	/* Mempcpy may return a past-the-end pointer.  */
+	*past_end = true;
 	return gimple_call_arg (stmt, 0);
       }
 
@@ -5284,23 +5305,63 @@ gimple_call_return_array (gimple *stmt, offset_int offrng[2],
       {
 	tree off = gimple_call_arg (stmt, 2);
 	if (get_offset_range (off, stmt, offrng, rvals))
-	  offrng[0] = 0;
+	  offrng[1] -= 1;
 	else
-	  {
-	    offrng[0] = 0;
-	    offrng[1] = HOST_WIDE_INT_M1U;
-	  }
+	  offrng[1] = HOST_WIDE_INT_M1U;
+
+	offrng[0] = 0;
 	return gimple_call_arg (stmt, 0);
       }
 
     case BUILT_IN_STRCHR:
     case BUILT_IN_STRRCHR:
     case BUILT_IN_STRSTR:
+      offrng[0] = 0;
+      offrng[1] = HOST_WIDE_INT_M1U;
+      return gimple_call_arg (stmt, 0);
+
+    case BUILT_IN_STPCPY:
+    case BUILT_IN_STPCPY_CHK:
       {
+	access_ref aref;
+	tree src = gimple_call_arg (stmt, 1);
+	if (compute_objsize (src, 1, &aref, rvals))
+	  offrng[1] = aref.sizrng[1] - 1;
+	else
+	  offrng[1] = HOST_WIDE_INT_M1U;
+	
 	offrng[0] = 0;
+	return gimple_call_arg (stmt, 0);
+      }
+
+    case BUILT_IN_STPNCPY:
+    case BUILT_IN_STPNCPY_CHK:
+      {
+	/* The returned pointer is in a range between the first argument
+	   and it plus the smaller of the upper bound of the size argument
+	   and the source object size.  */
 	offrng[1] = HOST_WIDE_INT_M1U;
+	tree off = gimple_call_arg (stmt, 2);
+	if (!get_offset_range (off, stmt, offrng, rvals)
+	    || offrng[0] != offrng[1])
+	  {
+	    /* If the offset is either indeterminate or in some range,
+	       try to constrain its upper bound to at most the size
+	       of the source object.  */
+	    access_ref aref;
+	    tree src = gimple_call_arg (stmt, 1);
+	    if (compute_objsize (src, 1, &aref, rvals)
+		&& aref.sizrng[1] < offrng[1])
+	      offrng[1] = aref.sizrng[1];
+	  }
+
+	/* When the source is the empty string the returned pointer is
+	   a copy of the argument.  Otherwise stpcpy can also return
+	   a past-the-end pointer.  */
+	offrng[0] = 0;
+	*past_end = true;
+	return gimple_call_arg (stmt, 0);
       }
-      return gimple_call_arg (stmt, 0);
 
     default:
       break;
@@ -5753,9 +5814,12 @@ compute_objsize_r (tree ptr, int ostype, access_ref *pref,
 	      /* For functions known to return one of their pointer arguments
 		 try to determine what the returned pointer points to, and on
 		 success add OFFRNG which was set to the offset added by
-		 the function (e.g., memchr) to the overall offset.  */
+		 the function (e.g., memchr or stpcpy) to the overall offset.
+	      */
+	      bool past_end;
 	      offset_int offrng[2];
-	      if (tree ret = gimple_call_return_array (stmt, offrng, rvals))
+	      if (tree ret = gimple_call_return_array (stmt, offrng,
+						       &past_end, rvals))
 		{
 		  if (!compute_objsize_r (ret, ostype, pref, snlim, qry))
 		    return false;
@@ -5764,6 +5828,11 @@ compute_objsize_r (tree ptr, int ostype, access_ref *pref,
 		     the object.  */
 		  offset_int remrng[2];
 		  remrng[1] = pref->size_remaining (remrng);
+		  if (remrng[1] != 0 && !past_end)
+		    /* Decrement the size for functions that never return
+		       a past-the-end pointer.  */
+		    remrng[1] -= 1;
+
 		  if (remrng[1] < offrng[1])
 		    offrng[1] = remrng[1];
 		  pref->add_offset (offrng[0], offrng[1]);
diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-80.c b/gcc/testsuite/gcc.dg/Warray-bounds-80.c
new file mode 100644
index 00000000000..4ef32fbbfc3
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Warray-bounds-80.c
@@ -0,0 +1,96 @@
+/* PR tree-optimization/101397 - spurious warning writing to the result
+   of stpcpy minus 1
+   { dg-do compile }
+   { dg-options "-O2 -Wall" } */
+
+char* stpcpy (char*, const char*);
+
+void sink (int, ...);
+
+extern char ax[], a3[3], a5[5], *s;
+
+volatile int x;
+
+void test_stpcpy (int i)
+{
+  {
+    char *p = stpcpy (ax, s);
+    x = p[-9];                          // { dg-bogus "\\\[-Warray-bounds" }
+    x = p[-1];                          // { dg-bogus "\\\[-Warray-bounds" }
+    x = p[ 0];
+    x = p[+9];
+  }
+
+  {
+    char *p = stpcpy (a3, s);
+    x = p[-2];                          // { dg-bogus "\\\[-Warray-bounds" }
+    x = p[-1];                          // { dg-bogus "\\\[-Warray-bounds" }
+  }
+
+  {
+    char *p = stpcpy (a3, s);
+    x = p[-3];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-2], p[-1], p[0], p[1], p[2]);
+    x = p[ 3];                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    /* Stpcpy always returns a pointer to the copied nul (which must
+       exist) and never a past-the-end pointer.  As a result, P below
+       is in [a5, a5 + 4].  */
+    char *p = stpcpy (a5, s);
+    x = p[-5];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-4], p[-3], p[-2], p[-1], p[0]);
+    sink (p[1], p[2], p[3], p[4]);
+    x = p[ 5];                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    char *p = stpcpy (a5 + 1, s);
+    x = p[-5];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-4], p[-3], p[-2], p[-1], p[0]);
+    sink (p[1], p[2], p[3]);
+    x = p[ 4];                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    char *p = stpcpy (a5 + 2, s);
+    x = p[-5];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-4], p[-3], p[-2], p[-1], p[0]);
+    sink (p[1], p[2]);
+    x = p[ 3];                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    char *p = stpcpy (a5 + 3, s);
+    x = p[-5];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-4], p[-3], p[-2], p[-1], p[0]);
+    sink (p[1]);
+    x = p[ 2];                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    /* Because strlen (a3) is at most 2, the stpcpy call must return
+       a pointer in the range [ax, ax + 2], and so -3 is necessarily
+       out of bounds.  */
+    char *p = stpcpy (ax, a3);
+    p[-3] = 1;                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    if (i >= 0)
+      i = -1;
+
+    char *p = stpcpy (a3, s);
+    x = p[i];                           // { dg-bogus "\\\[-Warray-bounds" }
+  }
+
+  {
+    if (i >= -3)
+      i = -3;
+
+    char *p = stpcpy (a3, s);
+    p[i] = 1;                           // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+}
diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-81.c b/gcc/testsuite/gcc.dg/Warray-bounds-81.c
new file mode 100644
index 00000000000..27e725d298a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Warray-bounds-81.c
@@ -0,0 +1,302 @@
+/* PR tree-optimization/101397 - spurious warning writing to the result
+   of stpcpy minus 1
+   Verify warnings for indexing into a pointer returned from stpncpy.
+   The call stpncpy(S1, S2, N) returns the address of the copy of
+   the first NUL is it exists or &S1[N] otherwise.
+   { dg-do compile }
+   { dg-options "-O2 -Wall -Wno-stringop-truncation" } */
+
+typedef __SIZE_TYPE__ size_t;
+
+void* calloc (size_t, size_t);
+char* stpncpy (char*, const char*, size_t);
+
+void sink (int, ...);
+
+extern char ax[], a3[3], a5[5], a7[7], a9[9], *s;
+
+volatile int x;
+
+/* Verify warnings for indexing into the result of stpncpy with a source
+   pointing to an array of unknown bound.  */
+
+void test_stpncpy_from_ptr (int i, int n)
+{
+  {
+    // P is in [ax, ax + 5].
+    char *p = stpncpy (ax, s, 5);
+    x = p[-6];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-5], p[-1], p[0], p[9]);
+  }
+
+  {
+    // P is in [a5, a5 + 3].
+    char *p = stpncpy (a5, s, 3);
+    x = p[-4];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-3], p[-2], p[-1], p[0]);
+    sink (p[ 1], p[ 2], p[ 3], p[4]);
+    x = p[ 5];                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    // P is in [ax, ax + 4].
+    char *p = stpncpy (a5, s, 4);
+    x = p[-5];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-4], p[-3], p[-2], p[-1], p[0]);
+    sink (p[ 1], p[ 2], p[ 3], p[4]);
+    x = p[ 5];                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    // P is in [ax, ax + 5].
+    char *p = stpncpy (a5, s, n);
+    x = p[-6];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-5], p[-4], p[-3], p[-2], p[-1], p[0]);
+    sink (p[ 1], p[ 2], p[ 3], p[4]);
+    x = p[ 5];                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    // P is in [ax, ax + 4].
+    char *p = stpncpy (a5, s, 4);
+
+    if (i > -1) i = -1;
+    x = p[i];
+
+    if (i > -2) i = -2;
+    x = p[i];
+
+    if (i > -3) i = -3;
+    x = p[i];
+
+    if (i > -4) i = -4;
+    x = p[i];
+
+    if (i > -5) i = -5;
+    x = p[i];                           // { dg-warning "\\\[-Warray-bounds" }
+  }
+}
+
+/* Verify warnings for indexing into the result of stpncpy with a source
+   an array of size 5.  */
+
+void test_stpncpy_from_a5 (int i, int n, int n3_9)
+{
+  {
+    // The returned pointer is in [ax, ax + 3].
+    char *p = stpncpy (ax, a5, 3);
+    x = p[-4];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-3], p[-2], p[-1], p[0], p[1], p[99]);
+  }
+
+  {
+    // The returned pointer is in [ax, ax + 5].
+    char *p = stpncpy (ax, a5, 5);
+    x = p[-6];                          // { dg-warning "\\\[-Warray-bounds" }
+    x = p[-5];
+    x = p[-1];
+    x = p[ 0];
+    x = p[ 9];
+  }
+
+  {
+    //The returned pointer is in [ax, ax + 5] even though n is not known.
+    char *p = stpncpy (ax, a5, n);
+    x = p[-6];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-5], p[-4], p[-2], p[-1], p[0]);
+    sink (p[1], p[2], p[3], p[4], p[9], p[99]);
+  }
+
+
+  {
+    // The returned pointer is in [a3, a3 + 3].
+    char *p = stpncpy (a3, a5, 3);
+    x = p[-4];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-3], p[-2], p[-1], p[0]);
+    sink (p[ 1], p[ 2]);
+    x = p[ 3];                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    // The returned pointer is in [a3, a3 + 3].
+    char *p = stpncpy (a3, a5, n);
+    x = p[-4];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-3], p[-2], p[-1], p[0]);
+    sink (p[ 1], p[ 2]);
+    x = p[ 3];                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    if (n3_9 < 3 || 9 < n3_9)
+      n3_9 = 3;
+
+    // The returned pointer is in [a3, a3 + 3].
+    char *p = stpncpy (a3, a5, n3_9);
+    x = p[-4];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-3], p[-2], p[-1], p[0]);
+    sink (p[ 1], p[ 2]);
+    x = p[ 3];                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    char *p = stpncpy (a3, a5, 3);
+
+    if (i > -1) i = -1;
+    x = p[i];
+
+    if (i > -2) i = -2;
+    x = p[i];
+
+    if (i > -3) i = -3;
+    x = p[i];
+
+    if (i > -4) i = -4;
+    x = p[i];                           // { dg-warning "\\\[-Warray-bounds" }
+  }
+}
+
+
+/* Verify warnings for indexing into the result of stpncpy with a source
+   an array of size 7.  */
+
+void test_stpncpy_from_a7 (int i, int n, int n3_9)
+{
+  {
+    // The returned pointer is ax + 5.
+    char *p = stpncpy (ax, a7, 5);
+    x = p[-6];                          // { dg-warning "\\\[-Warray-bounds" }
+    x = p[-5];
+    x = p[-1];
+    x = p[ 0];
+    x = p[ 9];
+  }
+
+  {
+    //The returned pointer is in [ax, ax + 7] even though n is not known.
+    char *p = stpncpy (ax, a7, n);
+    x = p[-8];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-7], p[-6], p[-5], p[-4], p[-3], p[-2], p[-1], p[0]);
+    sink (p[1], p[2], p[3], p[4], p[5], p[6], p[7], p[8], p[9]);
+  }
+
+
+  {
+    // The returned pointer is in [a5, a5 + 3].
+    char *p = stpncpy (a5, a7, 3);
+    x = p[-4];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-3], p[-2], p[-1], p[0]);
+    sink (p[1], p[2], p[3], p[4]);
+    x = p[ 5];                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    // The returned pointer is a5 + 4.
+    char *p = stpncpy (a5, a7, 4);
+    x = p[-5];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-4], p[-3], p[-2], p[-1], p[0]);
+    sink (p[1], p[2], p[3], p[4]);
+    x = p[ 5];                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    // The returned pointer is in [a5, a5 + 5].
+    char *p = stpncpy (a5, a7, n);
+    x = p[-6];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-5], p[-4], p[-3], p[-2], p[-1], p[0]);
+    sink (p[1], p[2], p[3], p[4]);
+    x = p[ 5];                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    if (n3_9 < 3 || 9 < n3_9)
+      n3_9 = 3;
+
+    // The returned pointer is in [a5, a5 + 5].
+    char *p = stpncpy (a5, a7, n3_9);
+    x = p[-6];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-5], p[-4], p[-3], p[-2], p[-1], p[0]);
+    sink (p[1], p[2], p[3], p[4]);
+    x = p[ 5];                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    char *p = stpncpy (a5, a7, 4);
+
+    if (i > -1) i = -1;
+    x = p[i];
+
+    if (i > -2) i = -2;
+    x = p[i];
+
+    if (i > -3) i = -3;
+    x = p[i];
+
+    if (i > -4) i = -4;
+    x = p[i];
+
+    if (i > -5) i = -5;
+    x = p[i];                           // { dg-warning "\\\[-Warray-bounds" }
+  }
+}
+
+
+void test_stpncpy_from_a5_to_allocated (int i, int n, int n5_7, int n3_9)
+{
+  if (n5_7 < 5 || 7 < n5_7)
+    n5_7 = 5;
+
+  {
+    char *d = calloc (n5_7, 1);
+    char *p = stpncpy (d, s, n);
+    x = p[-8];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-7], p[-6], p[-5], p[-4], p[-3], p[-2], p[-1], p[0]);
+    sink (p[1], p[2], p[3], p[4], p[5], p[6]);
+    x = p[7];                           // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    char *d = calloc (n5_7, 1);
+    char *p = stpncpy (d, a3, n);
+    x = p[-4];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-3], p[-2], p[-1], p[0]);
+    sink (p[1], p[2], p[3], p[4], p[5], p[6]);
+    x = p[7];                           // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    char *d = calloc (n5_7, 1);
+    char *p = stpncpy (d, a5, n);
+    x = p[-6];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-5], p[-4], p[-3], p[-2], p[-1], p[0]);
+    sink (p[1], p[2], p[3], p[4], p[5], p[6]);
+    x = p[7];                           // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    char *d = calloc (n5_7, 1);
+    char *p = stpncpy (d, a9, n);
+    x = p[-8];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-7], p[-6], p[-5], p[-3], p[-4], p[-2], p[-1], p[0]);
+    sink (p[1], p[2], p[3], p[4], p[5], p[6]);
+    x = p[7];                           // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    char *d = calloc (n5_7, 1);
+    char *p = stpncpy (d, a3, n3_9);
+    x = p[-4];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-3], p[-2], p[-1], p[0]);
+    sink (p[1], p[2], p[3], p[4], p[5], p[6]);
+    x = p[7];                           // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    char *d = calloc (n5_7, 1);
+    char *p = stpncpy (d, a9, n3_9);
+    x = p[-8];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-7], p[-6], p[-5], p[-4], p[-4], p[-2], p[-1], p[0]);
+    sink (p[1], p[2], p[3], p[4], p[5], p[6]);
+    x = p[7];                           // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+}
diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-82.c b/gcc/testsuite/gcc.dg/Warray-bounds-82.c
new file mode 100644
index 00000000000..b5dd919dce1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Warray-bounds-82.c
@@ -0,0 +1,258 @@
+/* PR tree-optimization/101397 - spurious warning writing to the result
+   of stpcpy minus 1
+   Verify warnings for indexing into a pointer returned from mempcpy.
+   The call mempcpy(S1, S2, N) returns &S1[N].
+   { dg-do compile }
+   { dg-options "-O2 -Wall" } */
+
+typedef __SIZE_TYPE__ size_t;
+
+void* mempcpy (void*, const void*, size_t);
+
+extern char ax[], a3[3], a5[5], a7[7], *s;
+
+volatile int x;
+
+/* Verify warnings for indexing into the result of mempcpy with a source
+   pointing to an array of unknown bound.  */
+
+void test_mempcpy_from_ptr (int i)
+{
+  {
+    char *p = mempcpy (ax, s, 5);
+    x = p[-6];                          // { dg-warning "\\\[-Warray-bounds" }
+    x = p[-5];
+    x = p[-1];
+    x = p[ 0];
+    x = p[ 9];
+  }
+
+  {
+    char *p = mempcpy (a5, s, 3);
+    x = p[-4];                          // { dg-warning "\\\[-Warray-bounds" }
+    x = p[-3];
+    x = p[-2];
+    x = p[-1];
+    x = p[ 0];
+    x = p[ 1];
+    x = p[ 2];                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    char *p = mempcpy (a5, s, 4);
+    x = p[-5];                          // { dg-warning "\\\[-Warray-bounds" }
+    x = p[-4];
+    x = p[-3];
+    x = p[-2];
+    x = p[-1];
+    x = p[ 0];
+    x = p[ 1];                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    char *p = mempcpy (a5, s, 4);
+
+    if (i > -1) i = -1;
+    x = p[i];
+
+    if (i > -2) i = -2;
+    x = p[i];
+
+    if (i > -3) i = -3;
+    x = p[i];
+
+    if (i > -4) i = -4;
+    x = p[i];
+
+    if (i > -5) i = -5;
+    x = p[i];                           // { dg-warning "\\\[-Warray-bounds" }
+  }
+}
+
+/* Verify warnings for indexing into the result of mempcpy with a source
+   an array of size 5.  */
+
+void test_mempcpy_from_a5 (int i, int n, int n3_9)
+{
+  {
+    // The returned pointer is ax + 3 as specified by the bound.
+    char *p = mempcpy (ax, a5, 3);
+    x = p[-4];                          // { dg-warning "\\\[-Warray-bounds" }
+    x = p[-3];
+    x = p[-2];
+    x = p[ 0];
+    x = p[ 1];
+    x = p[ 2];
+  }
+
+  {
+    // The returned pointer is ax + 5.
+    char *p = mempcpy (ax, a5, 5);
+    x = p[-6];                          // { dg-warning "\\\[-Warray-bounds" }
+    x = p[-5];
+    x = p[-1];
+    x = p[ 0];
+    x = p[ 9];
+  }
+
+  {
+    //The returned pointer is in [ax, ax + 5] even though n is not known.
+    char *p = mempcpy (ax, a5, n);
+    x = p[-6];                          // { dg-warning "\\\[-Warray-bounds" }
+    x = p[-5];
+    x = p[-1];
+    x = p[ 0];
+    x = p[ 9];
+  }
+
+
+  {
+    // The returned pointer is a3 + 3.
+    char *p = mempcpy (a3, a5, 3);
+    x = p[-4];                          // { dg-warning "\\\[-Warray-bounds" }
+    x = p[-3];
+    x = p[-1];
+    x = p[ 0];                          // { dg-warning "\\\[-Warray-bounds" }
+    x = p[ 1];                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    // The returned pointer is in [a3, a3 + 3].
+    char *p = mempcpy (a3, a5, n);
+    x = p[-4];                          // { dg-warning "\\\[-Warray-bounds" }
+    x = p[-3];
+    x = p[-2];
+    x = p[-1];
+    x = p[ 0];
+    x = p[ 2];
+    x = p[ 3];                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    if (n3_9 < 3 || 9 < n3_9)
+      n3_9 = 3;
+
+    // The returned pointer is a3.
+    char *p = mempcpy (a3, a5, n3_9);
+    x = p[-4];                          // { dg-warning "\\\[-Warray-bounds" }
+    x = p[-3];
+    x = p[-2];
+    x = p[-1];
+    x = p[ 0];                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    char *p = mempcpy (a3, a5, 3);
+
+    if (i > -1) i = -1;
+    x = p[i];
+
+    if (i > -2) i = -2;
+    x = p[i];
+
+    if (i > -3) i = -3;
+    x = p[i];
+
+    if (i > -4) i = -4;
+    x = p[i];                           // { dg-warning "\\\[-Warray-bounds" }
+  }
+}
+
+
+/* Verify warnings for indexing into the result of mempcpy with a source
+   an array of size 7.  */
+
+void test_mempcpy_from_a7 (int i, int n, int n3_9)
+{
+  {
+    // The returned pointer is ax + 5.
+    char *p = mempcpy (ax, a7, 5);
+    x = p[-6];                          // { dg-warning "\\\[-Warray-bounds" }
+    x = p[-5];
+    x = p[-1];
+    x = p[ 0];
+    x = p[ 9];
+  }
+
+  {
+    //The returned pointer is in [ax, ax + 7] even though n is not known.
+    char *p = mempcpy (ax, a7, n);
+    x = p[-8];                          // { dg-warning "\\\[-Warray-bounds" }
+    x = p[-7];
+    x = p[-1];
+    x = p[ 0];
+    x = p[ 9];
+  }
+
+
+  {
+    // The returned pointer is a5 + 3 as specified by the bound.
+    char *p = mempcpy (a5, a7, 3);
+    x = p[-4];                          // { dg-warning "\\\[-Warray-bounds" }
+    x = p[-3];
+    x = p[-2];
+    x = p[ 0];
+    x = p[ 1];
+    x = p[ 2];                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    // The returned pointer is a5 + 4.
+    char *p = mempcpy (a5, a7, 4);
+    x = p[-5];                          // { dg-warning "\\\[-Warray-bounds" }
+    x = p[-4];
+    x = p[-3];
+    x = p[-2];
+    x = p[-1];
+    x = p[ 0];
+    x = p[ 1];                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    // The returned pointer is in [a5, a5 + 5].
+    char *p = mempcpy (a5, a7, n);
+    x = p[-6];                          // { dg-warning "\\\[-Warray-bounds" }
+    x = p[-5];
+    x = p[-3];
+    x = p[-2];
+    x = p[-1];
+    x = p[ 0];
+    x = p[ 4];
+    x = p[ 5];                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    if (n3_9 < 3 || 9 < n3_9)
+      n3_9 = 3;
+
+    // The returned pointer is in [a5 + 3, a5 + 5].
+    char *p = mempcpy (a5, a7, n3_9);
+    x = p[-6];                          // { dg-warning "\\\[-Warray-bounds" }
+    x = p[-5];
+    x = p[-3];
+    x = p[-2];
+    x = p[-1];
+    x = p[ 0];
+    x = p[ 1];
+    x = p[ 2];                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    char *p = mempcpy (a5, a7, 4);
+
+    if (i > -1) i = -1;
+    x = p[i];
+
+    if (i > -2) i = -2;
+    x = p[i];
+
+    if (i > -3) i = -3;
+    x = p[i];
+
+    if (i > -4) i = -4;
+    x = p[i];
+
+    if (i > -5) i = -5;
+    x = p[i];                           // { dg-warning "\\\[-Warray-bounds" }
+  }
+}
diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-83.c b/gcc/testsuite/gcc.dg/Warray-bounds-83.c
new file mode 100644
index 00000000000..b1d02eaffcc
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Warray-bounds-83.c
@@ -0,0 +1,172 @@
+/* PR tree-optimization/101397 - spurious warning writing to the result
+   of stpcpy minus 1
+   Verify warnings for indexing into a pointer returned from stpncpy.
+   The call stpncpy(S1, S2, N) returns the address of the copy of
+   the first NUL is it exists or &S1[N] otherwise.
+   { dg-do compile }
+   { dg-options "-O2 -Wall -Wno-stringop-truncation" } */
+
+typedef __SIZE_TYPE__ size_t;
+
+__attribute__ ((alloc_size (1))) const void* alloc (size_t);
+
+void* memchr (const void*, int, size_t);
+
+void sink (int, ...);
+
+extern char ax[], a3[3], a5[5], a7[7], a9[9];
+
+volatile int x;
+
+/* Verify warnings for indexing into the result of memchr.  */
+
+void test_memchr (int i, int n, int n3_5, int n3_9)
+{
+  {
+    /* Because memchr never returns a past-the-end pointer the result
+       below is in [ax, ax + 4].  */
+    const char *p = memchr (ax, x, 5);
+    x = p[-5];                          // { dg-warning "\\\[-Warray-bounds" }
+    x = p[-4];
+    x = p[-1];
+    x = p[ 0];
+    x = p[ 9];
+  }
+
+  {
+    // The returned pointer is in [ax, ax + n].
+    const char *p = memchr (ax, x, n);
+    sink (p[-99], p[-6], p[-5], p[-4], p[-3], p[-2], p[-1], p[0]);
+    sink (p[1], p[2], p[3], p[4], p[5], p[6], p[7], p[8], p[99]);
+  }
+
+
+  {
+    // The returned pointer is in [a5, a5 + 2].
+    const char *p = memchr (a5, x, 3);
+    x = p[-3];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-2], p[-1], p[0]);
+    sink (p[1], p[2], p[3], p[4]);
+    x = p[ 5];                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    // The returned pointer is a5 + 4.
+    const char *p = memchr (a5, x, 4);
+    x = p[-4];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-3], p[-2], p[-1], p[0]);
+    sink (p[1], p[2], p[3], p[4]);
+    x = p[ 5];                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    // The returned pointer is in [a5, a5 + 4].
+    const char *p = memchr (a5, x, n);
+    x = p[-5];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-4], p[-3], p[-2], p[-1], p[0]);
+    sink (p[1], p[2], p[3], p[4]);
+    x = p[ 5];                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    if (n3_5 < 3 || 5 < n3_5)
+      n3_5 = 3;
+
+    // The returned pointer is in [a7, a7 + 4].
+    const char *p = memchr (a7, x, n3_5);
+    x = p[-5];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-4], p[-3], p[-2], p[-1], p[0]);
+    sink (p[1], p[2], p[3], p[4], p[5], p[6]);
+    x = p[7];                           // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    if (n3_9 < 3 || 9 < n3_9)
+      n3_9 = 3;
+
+    // The returned pointer is in [a5, a5 + 4].
+    const char *p = memchr (a5, x, n3_9);
+    x = p[-5];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-4], p[-3], p[-2], p[-1], p[0]);
+    sink (p[1], p[2], p[3], p[4]);
+    x = p[ 5];                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    const char *p = memchr (a5, x, 4);
+
+    if (i > -1) i = -1;
+    x = p[i];
+
+    if (i > -2) i = -2;
+    x = p[i];
+
+    if (i > -3) i = -3;
+    x = p[i];
+
+    if (i > -4) i = -4;
+    x = p[i];                           // { dg-warning "\\\[-Warray-bounds" }
+  }
+}
+
+
+void test_memchr_in_allocated (int i, int n, int n5_7, int n3_9)
+{
+  if (n5_7 < 5 || 7 < n5_7)
+    n5_7 = 5;
+
+  {
+    const char *s = alloc (n5_7);
+    const char *p = memchr (s, x, n);
+    x = p[-7];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-6], p[-6], p[-5], p[-4], p[-3], p[-2], p[-1], p[0]);
+    sink (p[1], p[2], p[3], p[4], p[5], p[6]);
+    x = p[7];                           // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    const char *s = alloc (n5_7);
+    const char *p = memchr (s, x, n);
+    x = p[-7];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-6], p[-5], p[-4], p[-3], p[-2], p[-1], p[0]);
+    sink (p[1], p[2], p[3], p[4], p[5], p[6]);
+    x = p[7];                           // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    const char *s = alloc (n5_7);
+    const char *p = memchr (s, x, n);
+    x = p[-7];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-6], p[-5], p[-4], p[-3], p[-2], p[-1], p[0]);
+    sink (p[1], p[2], p[3], p[4], p[5], p[6]);
+    x = p[7];                           // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    const char *s = alloc (n5_7);
+    const char *p = memchr (s, x, n);
+    x = p[-7];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-6], p[-5], p[-3], p[-4], p[-2], p[-1], p[0]);
+    sink (p[1], p[2], p[3], p[4], p[5], p[6]);
+    x = p[7];                           // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    const char *s = alloc (n5_7);
+    const char *p = memchr (s, x, n3_9);
+    x = p[-7];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-6], p[-5], p[-4], p[-3], p[-2], p[-1], p[0]);
+    sink (p[1], p[2], p[3], p[4], p[5], p[6]);
+    x = p[7];                           // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    const char *s = alloc (n5_7);
+    const char *p = memchr (s, x, n3_9);
+    x = p[-7];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-6], p[-5], p[-4], p[-4], p[-2], p[-1], p[0]);
+    sink (p[1], p[2], p[3], p[4], p[5], p[6]);
+    x = p[7];                           // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+}
diff --git a/gcc/testsuite/gcc.dg/Warray-bounds-84.c b/gcc/testsuite/gcc.dg/Warray-bounds-84.c
new file mode 100644
index 00000000000..b9350d79ebf
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Warray-bounds-84.c
@@ -0,0 +1,65 @@
+/* PR tree-optimization/101397 - spurious warning writing to the result
+   of stpcpy minus 1
+   { dg-do compile }
+   { dg-options "-O2 -Wall" } */
+
+char* strcpy (char*, const char*);
+
+void sink (int, ...);
+
+extern char ax[], a3[3], a5[5], *s;
+
+volatile int x;
+
+void test_strcpy (int i)
+{
+  {
+    char *p = strcpy (ax, s);
+    x = p[-1];                          // { dg-warning "\\\[-Warray-bounds" }
+    x = p[ 0];
+    x = p[+9];
+  }
+
+  {
+    char *p = strcpy (a3, s);
+    x = p[-1];                          // { dg-warning "\\\[-Warray-bounds" }
+    x = p[0];
+    x = p[1];
+    x = p[2];
+    x = p[3];                           // { dg-warning "\\\[-Warray-bounds" }
+ }
+
+  {
+    char *p = strcpy (a5, s);
+    x = p[-1];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[0], p[1], p[2], p[3], p[4]);
+    x = p[ 5];                          // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    char *p = strcpy (a5 + 1, s);
+    x = p[-2];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-1], p[0], p[1], p[2], p[3]);
+    x = p[4];                           // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    char *p = strcpy (a5 + 2, s);
+    x = p[-3];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-2], p[-1], p[0], p[1], p[2]);
+    x = p[3];                           // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    char *p = strcpy (a5 + 3, s);
+    x = p[-4];                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[-3], p[-2], p[-1], p[0], p[1]);
+    x = p[2];                           // { dg-warning "\\\[-Warray-bounds" }
+  }
+
+  {
+    char *p = strcpy (ax, a3);
+    p[-1] = 1;                          // { dg-warning "\\\[-Warray-bounds" }
+    sink (p[0], p[1], p[2], p[9], p[99]);
+  }
+}
diff --git a/gcc/testsuite/gcc.dg/Wstringop-overflow-46.c b/gcc/testsuite/gcc.dg/Wstringop-overflow-46.c
index b126fcbdcae..042c9676c91 100644
--- a/gcc/testsuite/gcc.dg/Wstringop-overflow-46.c
+++ b/gcc/testsuite/gcc.dg/Wstringop-overflow-46.c
@@ -79,9 +79,8 @@ void warn_memchr_var_memset_range (const void *s, unsigned n)
      as in the first two notes.  The exact value probably isn't too
      important. */
   char *p0 = malloc (UR (5, 7));
-  // { dg-message ": destination object of size \\\[5, 7]" "note 1" { target *-*-* } .-1 }
-  // { dg-message "at offset \\\[1, 7] into destination object of size \\\[5, 7]" "note 2"  { target *-*-* } .-2 }
-  // { dg-message "at offset \\\[2, 7] into destination object of size \\\[5, 7]" "note 3"  { target *-*-* } .-3 }
+  // { dg-message "at offset \\\[\[01\], 6] into destination object of size \\\[5, 7]" "note 2"  { target *-*-* } .-1 }
+  // { dg-message "at offset \\\[2, 7] into destination object of size \\\[5, 7]" "note 3"  { target *-*-* } .-2 }
 
   sink (p0);
   char *p1 = memchr (p0, '1', n);

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

* Re: [PATCH] correct range of stpcpy result (PR 101397)
  2021-07-15  1:49 [PATCH] correct range of stpcpy result (PR 101397) Martin Sebor
@ 2021-07-20 16:08 ` Jeff Law
  2021-07-20 20:06   ` Martin Sebor
  0 siblings, 1 reply; 3+ messages in thread
From: Jeff Law @ 2021-07-20 16:08 UTC (permalink / raw)
  To: Martin Sebor, gcc-patches



On 7/14/2021 7:49 PM, Martin Sebor via Gcc-patches wrote:
> Access warnings look through calls to the subset of built-ins
> that return one of their pointer arguments to find the object
> the pointer it points to and its offset.  The computation is
> wrong for functions like stpcpy, stpncpy and mempcpy that
> return a pointer plus some offset, and leads to a false positive
> -Warray-bounds in Glibc with the recent refactoring of the warning
> to take advantage of this logic.
>
> The attached patch corrects this mistake by accounting for this
> property of these functions while at the same time constraining
> the offset to the size of the source argument for better
> accuracy.
>
> Tested on x86_64-linux and by also building Glibc there.
>
> Martin
>
> gcc-101397.diff
>
> PR middle-end/101397 - spurious warning writing to the result of stpcpy minus 1
>
>
> gcc/ChangeLog:
>
> 	PR middle-end/101397
> 	* builtins.c (gimple_call_return_array): Add argument.  Correct
> 	offsets for memchr, mempcpy, stpcpy, and stpncpy.
> 	(compute_objsize_r): Adjust offset computation for argument returning
> 	built-ins.
>
> gcc/testsuite/ChangeLog:
>
> 	PR middle-end/101397
> 	* gcc.dg/Warray-bounds-80.c: New test.
> 	* gcc.dg/Warray-bounds-81.c: New test.
> 	* gcc.dg/Warray-bounds-82.c: New test.
> 	* gcc.dg/Warray-bounds-83.c: New test.
> 	* gcc.dg/Warray-bounds-84.c: New test.
> 	* gcc.dg/Wstringop-overflow-46.c: Adjust expected output.
>
> diff --git a/gcc/builtins.c b/gcc/builtins.c
> index 39ab139b7e1..170d776c410 100644
> --- a/gcc/builtins.c
> +++ b/gcc/builtins.c
> @@ -5200,12 +5200,19 @@ get_offset_range (tree x, gimple *stmt, offset_int r[2], range_query *rvals)
>   /* Return the argument that the call STMT to a built-in function returns
>      or null if it doesn't.  On success, set OFFRNG[] to the range of offsets
>      from the argument reflected in the value returned by the built-in if it
> -   can be determined, otherwise to 0 and HWI_M1U respectively.  */
> +   can be determined, otherwise to 0 and HWI_M1U respectively.  Set
> +   *PAST_END for functions like mempcpy that might return a past the end
> +   pointer (most functions return a dereferenceable pointer to an existing
> +   element of an array).  */
>   
>   static tree
> -gimple_call_return_array (gimple *stmt, offset_int offrng[2],
> +gimple_call_return_array (gimple *stmt, offset_int offrng[2], bool *past_end,
>   			  range_query *rvals)
>   {
> +  /* Clear and set below for the rare function(s) that might return
> +     a past-the-end pointer.  */
> +  *past_end = false;
> +
>     {
>       /* Check for attribute fn spec to see if the function returns one
>          of its arguments.  */
> @@ -5213,6 +5220,7 @@ gimple_call_return_array (gimple *stmt, offset_int offrng[2],
>       unsigned int argno;
>       if (fnspec.returns_arg (&argno))
>         {
> +	/* Functions return the first argument (not a range).  */
>   	offrng[0] = offrng[1] = 0;
>   	return gimple_call_arg (stmt, argno);
>         }
> @@ -5242,6 +5250,7 @@ gimple_call_return_array (gimple *stmt, offset_int offrng[2],
>         if (gimple_call_num_args (stmt) != 2)
>   	return NULL_TREE;
>   
> +      /* Allocation functions return a pointer to the beginning.  */
>         offrng[0] = offrng[1] = 0;
>         return gimple_call_arg (stmt, 1);
>       }
> @@ -5253,10 +5262,6 @@ gimple_call_return_array (gimple *stmt, offset_int offrng[2],
>       case BUILT_IN_MEMMOVE:
>       case BUILT_IN_MEMMOVE_CHK:
>       case BUILT_IN_MEMSET:
> -    case BUILT_IN_STPCPY:
> -    case BUILT_IN_STPCPY_CHK:
> -    case BUILT_IN_STPNCPY:
> -    case BUILT_IN_STPNCPY_CHK:
>       case BUILT_IN_STRCAT:
>       case BUILT_IN_STRCAT_CHK:
>       case BUILT_IN_STRCPY:
> @@ -5265,18 +5270,34 @@ gimple_call_return_array (gimple *stmt, offset_int offrng[2],
>       case BUILT_IN_STRNCAT_CHK:
>       case BUILT_IN_STRNCPY:
>       case BUILT_IN_STRNCPY_CHK:
> +      /* Functions return the first argument (not a range).  */
>         offrng[0] = offrng[1] = 0;
>         return gimple_call_arg (stmt, 0);
>   
>       case BUILT_IN_MEMPCPY:
>       case BUILT_IN_MEMPCPY_CHK:
>         {
> +	/* The returned pointer is in a range constrained by the smaller
> +	   of the upper bound of the size argument and the source object
> +	   size.  */
ISTM that for the MEMPCPY case the range is constrained by the size 
argument only from an implementation standpoint, but the size of the 
source or dest object can also constrain since if we overflow either 
we've gone into the realm of undefined behavior.  It's a nit for the 
comment, I don't think we need to adjust the implementation further.

OK for the trunk.
jeff


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

* Re: [PATCH] correct range of stpcpy result (PR 101397)
  2021-07-20 16:08 ` Jeff Law
@ 2021-07-20 20:06   ` Martin Sebor
  0 siblings, 0 replies; 3+ messages in thread
From: Martin Sebor @ 2021-07-20 20:06 UTC (permalink / raw)
  To: Jeff Law, gcc-patches

On 7/20/21 10:08 AM, Jeff Law wrote:
> 
> 
> On 7/14/2021 7:49 PM, Martin Sebor via Gcc-patches wrote:
>> Access warnings look through calls to the subset of built-ins
>> that return one of their pointer arguments to find the object
>> the pointer it points to and its offset.  The computation is
>> wrong for functions like stpcpy, stpncpy and mempcpy that
>> return a pointer plus some offset, and leads to a false positive
>> -Warray-bounds in Glibc with the recent refactoring of the warning
>> to take advantage of this logic.
>>
>> The attached patch corrects this mistake by accounting for this
>> property of these functions while at the same time constraining
>> the offset to the size of the source argument for better
>> accuracy.
>>
>> Tested on x86_64-linux and by also building Glibc there.
>>
>> Martin
>>
>> gcc-101397.diff
>>
>> PR middle-end/101397 - spurious warning writing to the result of stpcpy minus 1
>>
>>
>> gcc/ChangeLog:
>>
>> 	PR middle-end/101397
>> 	* builtins.c (gimple_call_return_array): Add argument.  Correct
>> 	offsets for memchr, mempcpy, stpcpy, and stpncpy.
>> 	(compute_objsize_r): Adjust offset computation for argument returning
>> 	built-ins.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	PR middle-end/101397
>> 	* gcc.dg/Warray-bounds-80.c: New test.
>> 	* gcc.dg/Warray-bounds-81.c: New test.
>> 	* gcc.dg/Warray-bounds-82.c: New test.
>> 	* gcc.dg/Warray-bounds-83.c: New test.
>> 	* gcc.dg/Warray-bounds-84.c: New test.
>> 	* gcc.dg/Wstringop-overflow-46.c: Adjust expected output.
>>
>> diff --git a/gcc/builtins.c b/gcc/builtins.c
>> index 39ab139b7e1..170d776c410 100644
>> --- a/gcc/builtins.c
>> +++ b/gcc/builtins.c
>> @@ -5200,12 +5200,19 @@ get_offset_range (tree x, gimple *stmt, offset_int r[2], range_query *rvals)
>>   /* Return the argument that the call STMT to a built-in function returns
>>      or null if it doesn't.  On success, set OFFRNG[] to the range of offsets
>>      from the argument reflected in the value returned by the built-in if it
>> -   can be determined, otherwise to 0 and HWI_M1U respectively.  */
>> +   can be determined, otherwise to 0 and HWI_M1U respectively.  Set
>> +   *PAST_END for functions like mempcpy that might return a past the end
>> +   pointer (most functions return a dereferenceable pointer to an existing
>> +   element of an array).  */
>>   
>>   static tree
>> -gimple_call_return_array (gimple *stmt, offset_int offrng[2],
>> +gimple_call_return_array (gimple *stmt, offset_int offrng[2], bool *past_end,
>>   			  range_query *rvals)
>>   {
>> +  /* Clear and set below for the rare function(s) that might return
>> +     a past-the-end pointer.  */
>> +  *past_end = false;
>> +
>>     {
>>       /* Check for attribute fn spec to see if the function returns one
>>          of its arguments.  */
>> @@ -5213,6 +5220,7 @@ gimple_call_return_array (gimple *stmt, offset_int offrng[2],
>>       unsigned int argno;
>>       if (fnspec.returns_arg (&argno))
>>         {
>> +	/* Functions return the first argument (not a range).  */
>>   	offrng[0] = offrng[1] = 0;
>>   	return gimple_call_arg (stmt, argno);
>>         }
>> @@ -5242,6 +5250,7 @@ gimple_call_return_array (gimple *stmt, offset_int offrng[2],
>>         if (gimple_call_num_args (stmt) != 2)
>>   	return NULL_TREE;
>>   
>> +      /* Allocation functions return a pointer to the beginning.  */
>>         offrng[0] = offrng[1] = 0;
>>         return gimple_call_arg (stmt, 1);
>>       }
>> @@ -5253,10 +5262,6 @@ gimple_call_return_array (gimple *stmt, offset_int offrng[2],
>>       case BUILT_IN_MEMMOVE:
>>       case BUILT_IN_MEMMOVE_CHK:
>>       case BUILT_IN_MEMSET:
>> -    case BUILT_IN_STPCPY:
>> -    case BUILT_IN_STPCPY_CHK:
>> -    case BUILT_IN_STPNCPY:
>> -    case BUILT_IN_STPNCPY_CHK:
>>       case BUILT_IN_STRCAT:
>>       case BUILT_IN_STRCAT_CHK:
>>       case BUILT_IN_STRCPY:
>> @@ -5265,18 +5270,34 @@ gimple_call_return_array (gimple *stmt, offset_int offrng[2],
>>       case BUILT_IN_STRNCAT_CHK:
>>       case BUILT_IN_STRNCPY:
>>       case BUILT_IN_STRNCPY_CHK:
>> +      /* Functions return the first argument (not a range).  */
>>         offrng[0] = offrng[1] = 0;
>>         return gimple_call_arg (stmt, 0);
>>   
>>       case BUILT_IN_MEMPCPY:
>>       case BUILT_IN_MEMPCPY_CHK:
>>         {
>> +	/* The returned pointer is in a range constrained by the smaller
>> +	   of the upper bound of the size argument and the source object
>> +	   size.  */
> ISTM that for the MEMPCPY case the range is constrained by the size 
> argument only from an implementation standpoint, but the size of the 
> source or dest object can also constrain since if we overflow either 
> we've gone into the realm of undefined behavior.  It's a nit for the 
> comment, I don't think we need to adjust the implementation further.

I thought about your observation a bit to see if I may have overlooked
something.  Deriving the constraint from the size of the source does
assume the source is in fact big enough.  If it's not big enough for
the copy another warning detects it so I think both cases are handled
correctly.  I have already pushed the change in r12-2422 but let me
add another test case to cover this.

Thanks
Martin

> 
> OK for the trunk.
> jeff
> 


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

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

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-15  1:49 [PATCH] correct range of stpcpy result (PR 101397) Martin Sebor
2021-07-20 16:08 ` Jeff Law
2021-07-20 20:06   ` Martin Sebor

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).