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