improve -Wrestrict for struct members (PR 85753)

Message ID ef3b6f36-247d-a41c-f71a-b02fd9166a46@gmail.com
State New
Headers show
Series
  • improve -Wrestrict for struct members (PR 85753)
Related show

Commit Message

Martin Sebor May 11, 2018, 11:09 p.m.
The attached patch extends -Wrestrict to constrain valid offset
ranges into objects of struct types to the bounds of the object
type, the same way the warning already handles arrays.  This
makes it possible to detect overlapping accesses in cases like
the second call to memcpy below:

   char a[16];

   struct { char a[16]; } x;

   void f (int i, int j)
   {
     memcpy (&a[i], &a[j], 9);   // -Wrestrict (good)

     memcpy (&x.a[i], &x.a[j], 9);   // missing -Wrestrict
   }

These is no way to copy 9 bytes within a 16 byte array without
either overlap or without accessing memory outside the bounaries
of the object.

This is for GCC 9.

Thanks
Martin

Comments

Jeff Law May 14, 2018, 9:21 p.m. | #1
On 05/11/2018 05:09 PM, Martin Sebor wrote:
> The attached patch extends -Wrestrict to constrain valid offset
> ranges into objects of struct types to the bounds of the object
> type, the same way the warning already handles arrays.  This
> makes it possible to detect overlapping accesses in cases like
> the second call to memcpy below:
> 
>   char a[16];
> 
>   struct { char a[16]; } x;
> 
>   void f (int i, int j)
>   {
>     memcpy (&a[i], &a[j], 9);   // -Wrestrict (good)
> 
>     memcpy (&x.a[i], &x.a[j], 9);   // missing -Wrestrict
>   }
> 
> These is no way to copy 9 bytes within a 16 byte array without
> either overlap or without accessing memory outside the bounaries
> of the object.
> 
> This is for GCC 9.
> 
> Thanks
> Martin
> 
> gcc-85753.diff
> 
> 
> PR tree-optimization/85753 - missing -Wrestrict on memcpy into a member array
> 
> gcc/ChangeLog:
> 
> 	PR tree-optimization/85753
> 	* gimple-ssa-warn-restrict.c (builtin_memref::builtin_memref): Handle
> 	RECORD_TYPE in addition to ARRAY_TYPE.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR tree-optimization/85753
> 	* gcc.dg/Wrestrict-10.c: Adjust.
> 	* gcc.dg/Wrestrict-16.c: New test.
OK.
jeff
Martin Sebor May 16, 2018, 2:34 a.m. | #2
On 05/14/2018 03:21 PM, Jeff Law wrote:
> On 05/11/2018 05:09 PM, Martin Sebor wrote:
>> The attached patch extends -Wrestrict to constrain valid offset
>> ranges into objects of struct types to the bounds of the object
>> type, the same way the warning already handles arrays.  This
>> makes it possible to detect overlapping accesses in cases like
>> the second call to memcpy below:
>>
>>   char a[16];
>>
>>   struct { char a[16]; } x;
>>
>>   void f (int i, int j)
>>   {
>>     memcpy (&a[i], &a[j], 9);   // -Wrestrict (good)
>>
>>     memcpy (&x.a[i], &x.a[j], 9);   // missing -Wrestrict
>>   }
>>
>> These is no way to copy 9 bytes within a 16 byte array without
>> either overlap or without accessing memory outside the bounaries
>> of the object.
>>
>> This is for GCC 9.
>>
>> Thanks
>> Martin
>>
>> gcc-85753.diff
>>
>>
>> PR tree-optimization/85753 - missing -Wrestrict on memcpy into a member array
>>
>> gcc/ChangeLog:
>>
>> 	PR tree-optimization/85753
>> 	* gimple-ssa-warn-restrict.c (builtin_memref::builtin_memref): Handle
>> 	RECORD_TYPE in addition to ARRAY_TYPE.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	PR tree-optimization/85753
>> 	* gcc.dg/Wrestrict-10.c: Adjust.
>> 	* gcc.dg/Wrestrict-16.c: New test.
> OK.
> jeff

It occurred to me that it doesn't matter if the type is an array,
struct, or a basic type like int.  They all should be treated
the same so after retesting I ended up committing a slightly
simplified version of the original: r260280.

Martin

Patch

PR tree-optimization/85753 - missing -Wrestrict on memcpy into a member array

gcc/ChangeLog:

	PR tree-optimization/85753
	* gimple-ssa-warn-restrict.c (builtin_memref::builtin_memref): Handle
	RECORD_TYPE in addition to ARRAY_TYPE.

gcc/testsuite/ChangeLog:

	PR tree-optimization/85753
	* gcc.dg/Wrestrict-10.c: Adjust.
	* gcc.dg/Wrestrict-16.c: New test.

diff --git a/gcc/gimple-ssa-warn-restrict.c b/gcc/gimple-ssa-warn-restrict.c
index 3d0664d..f461b84 100644
--- a/gcc/gimple-ssa-warn-restrict.c
+++ b/gcc/gimple-ssa-warn-restrict.c
@@ -263,27 +263,33 @@  builtin_memref::builtin_memref (tree expr, tree size)
   else
     sizrange[1] = maxobjsize;
 
+  if (!DECL_P (base))
+    return;
+
   tree basetype = TREE_TYPE (base);
-  if (DECL_P (base) && TREE_CODE (basetype) == ARRAY_TYPE)
+  if (TREE_CODE (basetype) != ARRAY_TYPE
+      && TREE_CODE (basetype) != RECORD_TYPE)
+    return;
+
+  /* If the offset could be in the range of the referenced object
+     constrain its bounds so neither exceeds those of the object.  */
+  if (offrange[0] < 0 && offrange[1] > 0)
+    offrange[0] = 0;
+
+  offset_int maxoff = maxobjsize;
+  if (TREE_CODE (basetype) == ARRAY_TYPE
+      && ref
+      && array_at_struct_end_p (ref))
+    ;   /* Use the maximum possible offset for last member arrays.  */
+  else if (tree basesize = TYPE_SIZE_UNIT (basetype))
+    maxoff = wi::to_offset (basesize);
+
+  if (offrange[0] >= 0)
     {
-      /* If the offset could be in range of the referenced object
-	 constrain its bounds so neither exceeds those of the object.  */
-      if (offrange[0] < 0 && offrange[1] > 0)
-	offrange[0] = 0;
-
-      offset_int maxoff = maxobjsize;
-      if (ref && array_at_struct_end_p (ref))
-	;   /* Use the maximum possible offset for last member arrays.  */
-      else if (tree basesize = TYPE_SIZE_UNIT (basetype))
-	maxoff = wi::to_offset (basesize);
-
-      if (offrange[0] >= 0)
-	{
-	  if (offrange[1] < 0)
-	    offrange[1] = offrange[0] <= maxoff ? maxoff : maxobjsize;
-	  else if (offrange[0] <= maxoff && offrange[1] > maxoff)
-	    offrange[1] = maxoff;
-	}
+      if (offrange[1] < 0)
+	offrange[1] = offrange[0] <= maxoff ? maxoff : maxobjsize;
+      else if (offrange[0] <= maxoff && offrange[1] > maxoff)
+	offrange[1] = maxoff;
     }
 }
 
diff --git a/gcc/testsuite/gcc.dg/Wrestrict-10.c b/gcc/testsuite/gcc.dg/Wrestrict-10.c
index a5a5ff1..c412e42 100644
--- a/gcc/testsuite/gcc.dg/Wrestrict-10.c
+++ b/gcc/testsuite/gcc.dg/Wrestrict-10.c
@@ -58,7 +58,7 @@  test_arr_strncat_2 (void)
 void __attribute__ ((noclone, noinline))
 test_arr_strcpy_1 (void)
 {
-  strcpy (&b.a[i], b.a);
+  strcpy (&b.a[i], b.a);            /* { dg-warning "\\\[-Wrestrict" } */
 }
 
 void __attribute__ ((noclone, noinline))
diff --git a/gcc/testsuite/gcc.dg/Wrestrict-16.c b/gcc/testsuite/gcc.dg/Wrestrict-16.c
new file mode 100644
index 0000000..136e122
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wrestrict-16.c
@@ -0,0 +1,34 @@ 
+/* PR tree-optimization/85753 - missing -Wrestrict on memcpy into a member
+   array
+   { dg-do compile }
+   { dg-options "-O2 -Wall -ftrack-macro-expansion=0" } */
+
+#define memcpy   __builtin_memcpy
+
+char a[16];
+
+struct { char a[16]; } x;
+
+void test_idx_nowarn (int i, int j)
+{
+  memcpy (&a[i], &a[j], 7);
+  memcpy (&x.a[i], &x.a[j], 7);
+}
+
+void test_idx_warn (int i, int j)
+{
+  memcpy (&a[i], &a[j], 9);       /* { dg-warning "\\\[-Wrestrict" } */
+  memcpy (&x.a[i], &x.a[j], 9);   /* { dg-warning "\\\[-Wrestrict" } */
+}
+
+void test_off_nowarn (int i, int j)
+{
+  memcpy (a + i, a + j, 5);
+  memcpy (x.a + i, x.a + j, 5);
+}
+
+void test_off_warn (int i, int j)
+{
+  memcpy (a + i, a + j, 9);       /* { dg-warning "\\\[-Wrestrict" } */
+  memcpy (x.a + i, x.a + j, 9);   /* { dg-warning "\\\[-Wrestrict" } */
+}