Message ID | ef3b6f36-247d-a41c-f71a-b02fd9166a46@gmail.com |
---|---|
State | New |
Headers | show |
Series | improve -Wrestrict for struct members (PR 85753) | expand |
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
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
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" } */ +}