Re: [PATCH] correct -Wrestrict handling of arrays of arrays (PR 84095)
On Fri, Feb 23, 2018 at 08:52:19AM -0700, Martin Sebor wrote: > There are a large number of failures in these reports in many > tests that were reported previously (before r257910), suggesting > something else is wrong. They all seem to use -fpic. > > If you referring to some other report or your own result please > post a link or say what target/configuration, etc.. The testcase clearly relies on inlining all the wrap_* functions, but as they are void wrap_* with -fpic/-fPIC obviously they can't be inlined at -O2, the executable or some other shared library might define some other implementation of those and interpose it. Some targets default to -fpic even. The following patch just adds static inline to them, so that it works even with -fpic, as tested with: make check-gcc check-g++ RUNTESTFLAGS='--target_board=unix\{,-fpic,-fpie\} dg.exp=Warray-bounds*' Ok for trunk? 2018-02-23 Jakub Jelinek* c-c++-common/Warray-bounds-2.c (wrap_memcpy_src_xsize, wrap_memcpy_src_diff_max, wrap_memcpy_dst_xsize, wrap_memcpy_dst_diff_max, wrap_strcat_src_xsize, wrap_strcat_dst_xsize, wrap_strcpy_src_xsize, wrap_strcpy_dst_xsize, wrap_strncpy_src_xsize, wrap_strncpy_src_diff_max, wrap_strncpy_dst_xsize, wrap_strncpy_dst_diff_max, wrap_strncpy_dstarray_diff_neg): Add static inline. --- gcc/testsuite/c-c++-common/Warray-bounds-2.c.jj 2017-12-18 14:57:15.601127538 +0100 +++ gcc/testsuite/c-c++-common/Warray-bounds-2.c2018-02-23 17:12:47.114113757 +0100 @@ -1,4 +1,4 @@ -/* Test to exercise that -Warray-bounds warnings for memory and sring +/* Test to exercise that -Warray-bounds warnings for memory and string functions are issued even when they are declared in system headers (i.e., not just when they are explicitly declared in the source file.) @@ -24,7 +24,7 @@ struct __attribute__ ((packed)) Array /* Exercise memcpy out-of-bounds offsets with an array of known size. */ -void wrap_memcpy_src_xsize (char *d, const char *s, ptrdiff_t i, size_t n) +static inline void wrap_memcpy_src_xsize (char *d, const char *s, ptrdiff_t i, size_t n) { memcpy (d, s + i, n); /* { dg-warning "offset 46 is out of the bounds \\\[0, 45] of object .ar. with type .(struct )?Array." "memcpy" } */ } @@ -39,7 +39,7 @@ void call_memcpy_src_xsize (char *d, siz /* Exercise memcpy out-of-bounds offsets with an array of unknown size. */ -void wrap_memcpy_src_diff_max (char *d, const char *s, ptrdiff_t i, size_t n) +static inline void wrap_memcpy_src_diff_max (char *d, const char *s, ptrdiff_t i, size_t n) { memcpy (d, s + i, n); /* { dg-warning "pointer overflow between offset \[0-9\]+ and size 3" "memcpy" } */ } @@ -49,7 +49,7 @@ void call_memcpy_src_diff_max (char *d, wrap_memcpy_src_diff_max (d, s, MAX, 3); } -void wrap_memcpy_dst_xsize (char *d, const char *s, ptrdiff_t i, size_t n) +static inline void wrap_memcpy_dst_xsize (char *d, const char *s, ptrdiff_t i, size_t n) { memcpy (d + i, s, n); /* { dg-warning "offset 47 is out of the bounds \\\[0, 45] of object .ar1. with type .(struct )?Array." "memcpy" } */ } @@ -62,7 +62,7 @@ void call_memcpy_dst_xsize (const char * sink (); } -void wrap_memcpy_dst_diff_max (char *d, const char *s, ptrdiff_t i, size_t n) +static inline void wrap_memcpy_dst_diff_max (char *d, const char *s, ptrdiff_t i, size_t n) { memcpy (d + i, s, n); /* { dg-warning "offset -?\[0-9\]+ is out of the bounds \\\[0, 45] of object .ar2. with type .(struct )?Array." "memcpy" } */ } @@ -76,7 +76,7 @@ void call_memcpy_dst_diff_max (const cha } -void wrap_strcat_src_xsize (char *d, const char *s, ptrdiff_t i) +static inline void wrap_strcat_src_xsize (char *d, const char *s, ptrdiff_t i) { strcat (d, s + i); /* { dg-warning "offset 46 is out of the bounds \\\[0, 45] of object .ar3. with type .(struct )?Array." "strcat" } */ } @@ -89,7 +89,7 @@ void call_strcat_src_xsize (char *d) sink (); } -void wrap_strcat_dst_xsize (char *d, const char *s, ptrdiff_t i) +static inline void wrap_strcat_dst_xsize (char *d, const char *s, ptrdiff_t i) { strcat (d + i, s); /* { dg-warning "offset 47 is out of the bounds \\\[0, 45] of object .ar4. with type .(struct )?Array." "strcat" } */ } @@ -103,7 +103,7 @@ void call_strcat_dst_xsize (const char * } -void wrap_strcpy_src_xsize (char *d, const char *s, ptrdiff_t i) +static inline void wrap_strcpy_src_xsize (char *d, const char *s, ptrdiff_t i) { strcpy (d, s + i); /* { dg-warning "offset 48 is out of the bounds \\\[0, 45] of object .ar5. with type .(struct )?Array." "strcpy" } */ } @@ -116,7 +116,7 @@ void call_strcpy_src_xsize (char *d) sink (); } -void wrap_strcpy_dst_xsize (char *d, const char *s, ptrdiff_t i) +static inline void wrap_strcpy_dst_xsize (char *d, const char *s, ptrdiff_t i) { strcpy (d + i, s); /* { dg-warning "offset 49 is out of the bounds \\\[0, 45] of
Re: [PATCH] correct -Wrestrict handling of arrays of arrays (PR 84095)
On Friday 23 February 2018 09:22 PM, Martin Sebor wrote: > I see no failures in this test in any of the recently reported > results on any targets except those below: > > https://gcc.gnu.org/ml/gcc-testresults/2018-02/msg01530.html > https://gcc.gnu.org/ml/gcc-testresults/2018-02/msg01514.html > > There are a large number of failures in these reports in many > tests that were reported previously (before r257910), suggesting > something else is wrong. They all seem to use -fpic. > > If you referring to some other report or your own result please > post a link or say what target/configuration, etc.. This was on my x86_64 box (Xeon(R) CPU E5-2623) and a standard x86_64-pc-linux-gnu native build. It was after an incremental build though, so I'll schedule a fresh one next week to confirm if you think that shouldn't happen. Siddhesh
Re: [PATCH] correct -Wrestrict handling of arrays of arrays (PR 84095)
On 02/22/2018 08:17 PM, Siddhesh Poyarekar wrote: On Friday 02 February 2018 05:15 AM, Martin Sebor wrote: PR middle-end/84095 - false-positive -Wrestrict warnings for memcpy within array gcc/ChangeLog: PR middle-end/84095 * gimple-ssa-warn-restrict.c (builtin_memref::extend_offset_range): New. (builtin_memref::set_base_and_offset): Same. Handle inner references. (builtin_memref::builtin_memref): Factor out parts into set_base_and_offset and call it. gcc/testsuite/ChangeLog: PR middle-end/84095 * c-c++-common/Warray-bounds-3.c: Adjust text of expected warnings. * c-c++-common/Wrestrict.c: Same. * gcc.dg/Wrestrict-6.c: Same. * gcc.dg/Warray-bounds-27.c: New test. * gcc.dg/Wrestrict-8.c: New test. * gcc.dg/Wrestrict-9.c: New test. * gcc.dg/pr84095.c: New test. This is causing failures in Warray-bounds-2.c in the testsuite: FAIL: c-c++-common/Warray-bounds-2.c -Wc++-compat memcpy (test for warnings, line 67) I see no failures in this test in any of the recently reported results on any targets except those below: https://gcc.gnu.org/ml/gcc-testresults/2018-02/msg01530.html https://gcc.gnu.org/ml/gcc-testresults/2018-02/msg01514.html There are a large number of failures in these reports in many tests that were reported previously (before r257910), suggesting something else is wrong. They all seem to use -fpic. If you referring to some other report or your own result please post a link or say what target/configuration, etc.. Martin FAIL: c-c++-common/Warray-bounds-2.c -Wc++-compat (test for warnings, line 72) FAIL: c-c++-common/Warray-bounds-2.c -Wc++-compat strncpy (test for warnings, line 178) FAIL: c-c++-common/Warray-bounds-2.c -Wc++-compat (test for warnings, line 183) FAIL: c-c++-common/Warray-bounds-2.c -std=gnu++98 memcpy (test for warnings, line 67) FAIL: c-c++-common/Warray-bounds-2.c -std=gnu++98 (test for warnings, line 72) FAIL: c-c++-common/Warray-bounds-2.c -std=gnu++98 strncpy (test for warnings, line 178) FAIL: c-c++-common/Warray-bounds-2.c -std=gnu++98 (test for warnings, line 183) FAIL: c-c++-common/Warray-bounds-2.c -std=gnu++11 memcpy (test for warnings, line 67) FAIL: c-c++-common/Warray-bounds-2.c -std=gnu++11 (test for warnings, line 72) FAIL: c-c++-common/Warray-bounds-2.c -std=gnu++11 strncpy (test for warnings, line 178) FAIL: c-c++-common/Warray-bounds-2.c -std=gnu++11 (test for warnings, line 183) FAIL: c-c++-common/Warray-bounds-2.c -std=gnu++14 memcpy (test for warnings, line 67) FAIL: c-c++-common/Warray-bounds-2.c -std=gnu++14 (test for warnings, line 72) FAIL: c-c++-common/Warray-bounds-2.c -std=gnu++14 strncpy (test for warnings, line 178) FAIL: c-c++-common/Warray-bounds-2.c -std=gnu++14 (test for warnings, line 183) Siddhesh
Re: [PATCH] correct -Wrestrict handling of arrays of arrays (PR 84095)
On Friday 02 February 2018 05:15 AM, Martin Sebor wrote: > PR middle-end/84095 - false-positive -Wrestrict warnings for memcpy within > array > > gcc/ChangeLog: > > PR middle-end/84095 > * gimple-ssa-warn-restrict.c (builtin_memref::extend_offset_range): New. > (builtin_memref::set_base_and_offset): Same. Handle inner references. > (builtin_memref::builtin_memref): Factor out parts into > set_base_and_offset and call it. > > gcc/testsuite/ChangeLog: > > PR middle-end/84095 > * c-c++-common/Warray-bounds-3.c: Adjust text of expected warnings. > * c-c++-common/Wrestrict.c: Same. > * gcc.dg/Wrestrict-6.c: Same. > * gcc.dg/Warray-bounds-27.c: New test. > * gcc.dg/Wrestrict-8.c: New test. > * gcc.dg/Wrestrict-9.c: New test. > * gcc.dg/pr84095.c: New test. This is causing failures in Warray-bounds-2.c in the testsuite: FAIL: c-c++-common/Warray-bounds-2.c -Wc++-compat memcpy (test for warnings, line 67) FAIL: c-c++-common/Warray-bounds-2.c -Wc++-compat (test for warnings, line 72) FAIL: c-c++-common/Warray-bounds-2.c -Wc++-compat strncpy (test for warnings, line 178) FAIL: c-c++-common/Warray-bounds-2.c -Wc++-compat (test for warnings, line 183) FAIL: c-c++-common/Warray-bounds-2.c -std=gnu++98 memcpy (test for warnings, line 67) FAIL: c-c++-common/Warray-bounds-2.c -std=gnu++98 (test for warnings, line 72) FAIL: c-c++-common/Warray-bounds-2.c -std=gnu++98 strncpy (test for warnings, line 178) FAIL: c-c++-common/Warray-bounds-2.c -std=gnu++98 (test for warnings, line 183) FAIL: c-c++-common/Warray-bounds-2.c -std=gnu++11 memcpy (test for warnings, line 67) FAIL: c-c++-common/Warray-bounds-2.c -std=gnu++11 (test for warnings, line 72) FAIL: c-c++-common/Warray-bounds-2.c -std=gnu++11 strncpy (test for warnings, line 178) FAIL: c-c++-common/Warray-bounds-2.c -std=gnu++11 (test for warnings, line 183) FAIL: c-c++-common/Warray-bounds-2.c -std=gnu++14 memcpy (test for warnings, line 67) FAIL: c-c++-common/Warray-bounds-2.c -std=gnu++14 (test for warnings, line 72) FAIL: c-c++-common/Warray-bounds-2.c -std=gnu++14 strncpy (test for warnings, line 178) FAIL: c-c++-common/Warray-bounds-2.c -std=gnu++14 (test for warnings, line 183) Siddhesh
Re: [PATCH] correct -Wrestrict handling of arrays of arrays (PR 84095)
On 02/15/2018 10:47 AM, Martin Sebor wrote: > On 02/13/2018 11:14 PM, Jeff Law wrote: >> On 02/01/2018 04:45 PM, Martin Sebor wrote: >>> The previous patch didn't resolve all the false positives >>> in the Linux kernel. The attached is an update that fixes >>> the remaining one having to do with multidimensional array >>> members: >>> >>> struct S { char a[2][4]; }; >>> >>> void f (struct S *p, int i) >>> { >>> strcpy (p->a[0], "012"); >>> strcpy (p->a[i] + 1, p->a[0]); // false positive here >>> } >>> >>> In the process of fixing this I also made a couple of minor >>> restructuring changes to the builtin_memref constructor to >>> in order to make the code easier to follow: I broke it out >>> into a couple of helper functions and called those. >>> >>> As with the first revision of the patch, this one is also >>> meant to be applied on top of >>> >>> https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01488.html >>> >>> Sorry about the late churn. Even though I tested the original >>> implementation with the Linux kernel the bugs were only exposed >>> non-default configurations that I didn't build. >>> >>> Jakub, you had concerns about the code in the constructor >>> and about interpreting the offsets in the diagnostics. >>> I tried to address those in the patch. Please review >>> the changes and let me know if you have any further comments. >>> >>> Thanks >>> Martin >>> >>> On 01/30/2018 04:19 PM, Martin Sebor wrote: Testing GCC 8 with recent Linux kernel sources has uncovered a bug in the handling of arrays of arrays by the -Wrestrict checker where it fails to take references to different array elements into consideration, issuing false positives. The attached patch corrects this mistake. In addition, to make warnings involving excessive offset bounds more meaningful (less confusing), I've made a cosmetic change to constrain them to the bounds of the accessed object. I've done this in response to multiple comments indicating that the warnings are hard to interpret. This change is meant to be applied on top of the patch for bug 83698 (submitted mainly to improve the readability of the offsets): https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01488.html Martin >>> >>> >>> gcc-84095.diff >>> >>> >>> PR middle-end/84095 - false-positive -Wrestrict warnings for memcpy >>> within array >>> >>> gcc/ChangeLog: >>> >>> PR middle-end/84095 >>> * gimple-ssa-warn-restrict.c >>> (builtin_memref::extend_offset_range): New. >>> (builtin_memref::set_base_and_offset): Same. Handle inner >>> references. >>> (builtin_memref::builtin_memref): Factor out parts into >>> set_base_and_offset and call it. >>> >>> gcc/testsuite/ChangeLog: >>> >>> PR middle-end/84095 >>> * c-c++-common/Warray-bounds-3.c: Adjust text of expected warnings. >>> * c-c++-common/Wrestrict.c: Same. >>> * gcc.dg/Wrestrict-6.c: Same. >>> * gcc.dg/Warray-bounds-27.c: New test. >>> * gcc.dg/Wrestrict-8.c: New test. >>> * gcc.dg/Wrestrict-9.c: New test. >>> * gcc.dg/pr84095.c: New test. >>> >>> diff --git a/gcc/gimple-ssa-warn-restrict.c >>> b/gcc/gimple-ssa-warn-restrict.c >>> index 528eb5b..367e05f 100644 >>> --- a/gcc/gimple-ssa-warn-restrict.c >>> +++ b/gcc/gimple-ssa-warn-restrict.c >> >>> + else if (gimple_nop_p (stmt)) >>> + expr = SSA_NAME_VAR (expr); >>> + else >>> + { >>> + base = expr; >>> + return; >>> } >> This looks odd. Can you explain what you're trying to do here? >> >> I'm not offhand why you'd ever want to extract SSA_NAME_VAR. In general >> it's primary use is for dumps and debugging info. I won't quite go so >> far as to say using it for anything else is wrong, but it's certainly >> something you ought to explain. > > It appears to be dead code. Nothing in the GCC test suite hits > this code. It's most likely a vestige of an approach I tried > that didn't work and that I ended up doing differently and forgot > to remove. I'll remove it before committing. > >> The rest looks fairly reasonable. It's a bit hard to follow, but I >> don't think we should do another round of refactoring at this stage. > > Is the patch good to commit then with the unused code above > removed? Yes. Again, sorry for the delays. jeff
Re: [PATCH] correct -Wrestrict handling of arrays of arrays (PR 84095)
On 02/13/2018 11:14 PM, Jeff Law wrote: On 02/01/2018 04:45 PM, Martin Sebor wrote: The previous patch didn't resolve all the false positives in the Linux kernel. The attached is an update that fixes the remaining one having to do with multidimensional array members: struct S { char a[2][4]; }; void f (struct S *p, int i) { strcpy (p->a[0], "012"); strcpy (p->a[i] + 1, p->a[0]); // false positive here } In the process of fixing this I also made a couple of minor restructuring changes to the builtin_memref constructor to in order to make the code easier to follow: I broke it out into a couple of helper functions and called those. As with the first revision of the patch, this one is also meant to be applied on top of https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01488.html Sorry about the late churn. Even though I tested the original implementation with the Linux kernel the bugs were only exposed non-default configurations that I didn't build. Jakub, you had concerns about the code in the constructor and about interpreting the offsets in the diagnostics. I tried to address those in the patch. Please review the changes and let me know if you have any further comments. Thanks Martin On 01/30/2018 04:19 PM, Martin Sebor wrote: Testing GCC 8 with recent Linux kernel sources has uncovered a bug in the handling of arrays of arrays by the -Wrestrict checker where it fails to take references to different array elements into consideration, issuing false positives. The attached patch corrects this mistake. In addition, to make warnings involving excessive offset bounds more meaningful (less confusing), I've made a cosmetic change to constrain them to the bounds of the accessed object. I've done this in response to multiple comments indicating that the warnings are hard to interpret. This change is meant to be applied on top of the patch for bug 83698 (submitted mainly to improve the readability of the offsets): https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01488.html Martin gcc-84095.diff PR middle-end/84095 - false-positive -Wrestrict warnings for memcpy within array gcc/ChangeLog: PR middle-end/84095 * gimple-ssa-warn-restrict.c (builtin_memref::extend_offset_range): New. (builtin_memref::set_base_and_offset): Same. Handle inner references. (builtin_memref::builtin_memref): Factor out parts into set_base_and_offset and call it. gcc/testsuite/ChangeLog: PR middle-end/84095 * c-c++-common/Warray-bounds-3.c: Adjust text of expected warnings. * c-c++-common/Wrestrict.c: Same. * gcc.dg/Wrestrict-6.c: Same. * gcc.dg/Warray-bounds-27.c: New test. * gcc.dg/Wrestrict-8.c: New test. * gcc.dg/Wrestrict-9.c: New test. * gcc.dg/pr84095.c: New test. diff --git a/gcc/gimple-ssa-warn-restrict.c b/gcc/gimple-ssa-warn-restrict.c index 528eb5b..367e05f 100644 --- a/gcc/gimple-ssa-warn-restrict.c +++ b/gcc/gimple-ssa-warn-restrict.c + else if (gimple_nop_p (stmt)) + expr = SSA_NAME_VAR (expr); + else + { + base = expr; + return; } This looks odd. Can you explain what you're trying to do here? I'm not offhand why you'd ever want to extract SSA_NAME_VAR. In general it's primary use is for dumps and debugging info. I won't quite go so far as to say using it for anything else is wrong, but it's certainly something you ought to explain. It appears to be dead code. Nothing in the GCC test suite hits this code. It's most likely a vestige of an approach I tried that didn't work and that I ended up doing differently and forgot to remove. I'll remove it before committing. The rest looks fairly reasonable. It's a bit hard to follow, but I don't think we should do another round of refactoring at this stage. Is the patch good to commit then with the unused code above removed? Martin
Re: [PATCH] correct -Wrestrict handling of arrays of arrays (PR 84095)
On 02/01/2018 04:45 PM, Martin Sebor wrote: > The previous patch didn't resolve all the false positives > in the Linux kernel. The attached is an update that fixes > the remaining one having to do with multidimensional array > members: > > struct S { char a[2][4]; }; > > void f (struct S *p, int i) > { > strcpy (p->a[0], "012"); > strcpy (p->a[i] + 1, p->a[0]); // false positive here > } > > In the process of fixing this I also made a couple of minor > restructuring changes to the builtin_memref constructor to > in order to make the code easier to follow: I broke it out > into a couple of helper functions and called those. > > As with the first revision of the patch, this one is also > meant to be applied on top of > > https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01488.html > > Sorry about the late churn. Even though I tested the original > implementation with the Linux kernel the bugs were only exposed > non-default configurations that I didn't build. > > Jakub, you had concerns about the code in the constructor > and about interpreting the offsets in the diagnostics. > I tried to address those in the patch. Please review > the changes and let me know if you have any further comments. > > Thanks > Martin > > On 01/30/2018 04:19 PM, Martin Sebor wrote: >> Testing GCC 8 with recent Linux kernel sources has uncovered >> a bug in the handling of arrays of arrays by the -Wrestrict >> checker where it fails to take references to different array >> elements into consideration, issuing false positives. >> >> The attached patch corrects this mistake. >> >> In addition, to make warnings involving excessive offset bounds >> more meaningful (less confusing), I've made a cosmetic change >> to constrain them to the bounds of the accessed object. I've >> done this in response to multiple comments indicating that >> the warnings are hard to interpret. This change is meant to >> be applied on top of the patch for bug 83698 (submitted mainly >> to improve the readability of the offsets): >> >> https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01488.html >> >> Martin > > > gcc-84095.diff > > > PR middle-end/84095 - false-positive -Wrestrict warnings for memcpy within > array > > gcc/ChangeLog: > > PR middle-end/84095 > * gimple-ssa-warn-restrict.c (builtin_memref::extend_offset_range): New. > (builtin_memref::set_base_and_offset): Same. Handle inner references. > (builtin_memref::builtin_memref): Factor out parts into > set_base_and_offset and call it. > > gcc/testsuite/ChangeLog: > > PR middle-end/84095 > * c-c++-common/Warray-bounds-3.c: Adjust text of expected warnings. > * c-c++-common/Wrestrict.c: Same. > * gcc.dg/Wrestrict-6.c: Same. > * gcc.dg/Warray-bounds-27.c: New test. > * gcc.dg/Wrestrict-8.c: New test. > * gcc.dg/Wrestrict-9.c: New test. > * gcc.dg/pr84095.c: New test. > > diff --git a/gcc/gimple-ssa-warn-restrict.c b/gcc/gimple-ssa-warn-restrict.c > index 528eb5b..367e05f 100644 > --- a/gcc/gimple-ssa-warn-restrict.c > +++ b/gcc/gimple-ssa-warn-restrict.c > + else if (gimple_nop_p (stmt)) > + expr = SSA_NAME_VAR (expr); > + else > + { > + base = expr; > + return; > } This looks odd. Can you explain what you're trying to do here? I'm not offhand why you'd ever want to extract SSA_NAME_VAR. In general it's primary use is for dumps and debugging info. I won't quite go so far as to say using it for anything else is wrong, but it's certainly something you ought to explain. The rest looks fairly reasonable. It's a bit hard to follow, but I don't think we should do another round of refactoring at this stage. jeff
Re: [PATCH] correct -Wrestrict handling of arrays of arrays (PR 84095)
Ping: https://gcc.gnu.org/ml/gcc-patches/2018-02/msg00076.html On 02/01/2018 04:45 PM, Martin Sebor wrote: The previous patch didn't resolve all the false positives in the Linux kernel. The attached is an update that fixes the remaining one having to do with multidimensional array members: struct S { char a[2][4]; }; void f (struct S *p, int i) { strcpy (p->a[0], "012"); strcpy (p->a[i] + 1, p->a[0]); // false positive here } In the process of fixing this I also made a couple of minor restructuring changes to the builtin_memref constructor to in order to make the code easier to follow: I broke it out into a couple of helper functions and called those. As with the first revision of the patch, this one is also meant to be applied on top of https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01488.html Sorry about the late churn. Even though I tested the original implementation with the Linux kernel the bugs were only exposed non-default configurations that I didn't build. Jakub, you had concerns about the code in the constructor and about interpreting the offsets in the diagnostics. I tried to address those in the patch. Please review the changes and let me know if you have any further comments. Thanks Martin On 01/30/2018 04:19 PM, Martin Sebor wrote: Testing GCC 8 with recent Linux kernel sources has uncovered a bug in the handling of arrays of arrays by the -Wrestrict checker where it fails to take references to different array elements into consideration, issuing false positives. The attached patch corrects this mistake. In addition, to make warnings involving excessive offset bounds more meaningful (less confusing), I've made a cosmetic change to constrain them to the bounds of the accessed object. I've done this in response to multiple comments indicating that the warnings are hard to interpret. This change is meant to be applied on top of the patch for bug 83698 (submitted mainly to improve the readability of the offsets): https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01488.html Martin
Re: [PATCH] correct -Wrestrict handling of arrays of arrays (PR 84095)
The previous patch didn't resolve all the false positives in the Linux kernel. The attached is an update that fixes the remaining one having to do with multidimensional array members: struct S { char a[2][4]; }; void f (struct S *p, int i) { strcpy (p->a[0], "012"); strcpy (p->a[i] + 1, p->a[0]); // false positive here } In the process of fixing this I also made a couple of minor restructuring changes to the builtin_memref constructor to in order to make the code easier to follow: I broke it out into a couple of helper functions and called those. As with the first revision of the patch, this one is also meant to be applied on top of https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01488.html Sorry about the late churn. Even though I tested the original implementation with the Linux kernel the bugs were only exposed non-default configurations that I didn't build. Jakub, you had concerns about the code in the constructor and about interpreting the offsets in the diagnostics. I tried to address those in the patch. Please review the changes and let me know if you have any further comments. Thanks Martin On 01/30/2018 04:19 PM, Martin Sebor wrote: Testing GCC 8 with recent Linux kernel sources has uncovered a bug in the handling of arrays of arrays by the -Wrestrict checker where it fails to take references to different array elements into consideration, issuing false positives. The attached patch corrects this mistake. In addition, to make warnings involving excessive offset bounds more meaningful (less confusing), I've made a cosmetic change to constrain them to the bounds of the accessed object. I've done this in response to multiple comments indicating that the warnings are hard to interpret. This change is meant to be applied on top of the patch for bug 83698 (submitted mainly to improve the readability of the offsets): https://gcc.gnu.org/ml/gcc-patches/2018-01/msg01488.html Martin PR middle-end/84095 - false-positive -Wrestrict warnings for memcpy within array gcc/ChangeLog: PR middle-end/84095 * gimple-ssa-warn-restrict.c (builtin_memref::extend_offset_range): New. (builtin_memref::set_base_and_offset): Same. Handle inner references. (builtin_memref::builtin_memref): Factor out parts into set_base_and_offset and call it. gcc/testsuite/ChangeLog: PR middle-end/84095 * c-c++-common/Warray-bounds-3.c: Adjust text of expected warnings. * c-c++-common/Wrestrict.c: Same. * gcc.dg/Wrestrict-6.c: Same. * gcc.dg/Warray-bounds-27.c: New test. * gcc.dg/Wrestrict-8.c: New test. * gcc.dg/Wrestrict-9.c: New test. * gcc.dg/pr84095.c: New test. diff --git a/gcc/gimple-ssa-warn-restrict.c b/gcc/gimple-ssa-warn-restrict.c index 528eb5b..367e05f 100644 --- a/gcc/gimple-ssa-warn-restrict.c +++ b/gcc/gimple-ssa-warn-restrict.c @@ -158,6 +158,14 @@ struct builtin_memref builtin_memref (tree, tree); tree offset_out_of_bounds (int, offset_int[2]) const; + +private: + + /* Ctor helper to set or extend OFFRANGE based on argument. */ + void extend_offset_range (tree); + + /* Ctor helper to determine BASE and OFFRANGE from argument. */ + void set_base_and_offset (tree); }; /* Description of a memory access by a raw memory or string built-in @@ -214,6 +222,7 @@ class builtin_access bool (builtin_access::*detect_overlap) (); }; + /* Initialize a memory reference representation from a pointer EXPR and a size SIZE in bytes. If SIZE is NULL_TREE then the size is assumed to be unknown. */ @@ -235,11 +244,120 @@ builtin_memref::builtin_memref (tree expr, tree size) const offset_int maxobjsize = tree_to_shwi (max_object_size ()); + /* Find the BASE object or pointer referenced by EXPR and set + the offset range OFFRANGE in the process. */ + set_base_and_offset (expr); + + if (size) +{ + tree range[2]; + /* Determine the size range, allowing for the result to be [0, 0] + for SIZE in the anti-range ~[0, N] where N >= PTRDIFF_MAX. */ + get_size_range (size, range, true); + sizrange[0] = wi::to_offset (range[0]); + sizrange[1] = wi::to_offset (range[1]); + /* get_size_range returns SIZE_MAX for the maximum size. + Constrain it to the real maximum of PTRDIFF_MAX. */ + if (sizrange[1] > maxobjsize) + sizrange[1] = maxobjsize; +} + else +sizrange[1] = maxobjsize; + + tree basetype = TREE_TYPE (base); + if (DECL_P (base) && TREE_CODE (basetype) == ARRAY_TYPE) +{ + /* 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) +