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


Reply via email to