On 2/3/21 3:45 PM, Martin Sebor wrote:
> On 2/3/21 2:57 PM, Jeff Law wrote:
>>
>>
>> On 1/28/21 4:03 PM, Martin Sebor wrote:
>>> The GCC 11 -Warray-bounds enhancement to diagnose accesses whose
>>> leading offset is in bounds but whose trailing offset is not has
>>> been causing some confusion. When the warning is issued for
>>> an access to an in-bounds member via a pointer to a struct that's
>>> larger than the pointed-to object, phrasing this strictly invalid
>>> access in terms of array subscripts can be misleading, especially
>>> when the source code doesn't involve any arrays or indexing.
>>>
>>> Since the problem boils down to an aliasing violation much more
>>> so than an actual out-of-bounds access, the attached patch adjusts
>>> the code to issue a -Wstrict-aliasing warning in these cases instead
>>> of -Warray-bounds. In addition, since the aliasing assumptions in
>>> GCC can be disabled by -fno-strict-aliasing, the patch also makes
>>> these instances of the warning conditional on -fstrict-aliasing
>>> being in effect.
>>>
>>> Martin
>>>
>>> gcc-98503.diff
>>>
>>> PR middle-end/98503 -Warray-bounds when -Wstrict-aliasing would be
>>> more appropriate
>>>
>>> gcc/ChangeLog:
>>>
>>> PR middle-end/98503
>>> * gimple-array-bounds.cc (array_bounds_checker::check_mem_ref):
>>> Issue -Wstrict-aliasing for a subset of violations.
>>> (array_bounds_checker::check_array_bounds): Set new member.
>>> * gimple-array-bounds.h (array_bounds_checker::cref_of_mref): New
>>> data member.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> PR middle-end/98503
>>> * g++.dg/warn/Warray-bounds-10.C: Adjust text of expected warnings.
>>> * g++.dg/warn/Warray-bounds-11.C: Same.
>>> * g++.dg/warn/Warray-bounds-12.C: Same.
>>> * g++.dg/warn/Warray-bounds-13.C: Same.
>>> * gcc.dg/Warray-bounds-63.c: Avoid -Wstrict-aliasing. Adjust text
>>> of expected warnings.
>>> * gcc.dg/Warray-bounds-66.c: Adjust text of expected warnings.
>>> * gcc.dg/Wstrict-aliasing-2.c: New test.
>>> * gcc.dg/Wstrict-aliasing-3.c: New test.
>> What I don't like here is the strict-aliasing warnings inside the file
>> and analysis phase for array bounds checking.
>>
>> ISTM that catching this at cast time would be better. So perhaps in
>> build_c_cast and the C++ equivalent?
>>
>> It would mean we're warning at the cast site rather than the
>> dereference, but that may ultimately be better for the warning anyway.
>> I'm not sure.
>
> I had actually experimented with a this (in the middle end, and only
> for accesses) but even that caused so many warnings that I abandoned
> it, at least for now. -Warray-bounds has the benefit of flow analysis
> (and dead code elimination). In the front end it would have neither
> and be both excessively noisy and ineffective at the same time. For
> example:
I think we agree that this really is an aliasing issue and I would go
further and claim that it has nothing to do with array bounds checking.
Therefore warning for it within gimple-array-bounds just seems wrong.
I realize that you like having DCE run and the ability to look at
offsets and such, but it really feels like the wrong place to do this.
Aliasing issues are also more of a front-end thing since the language
defines what is and what is not valid aliasing -- one might even argue
that any aliasing warning needs to be identified by the front-ends
exclusively (though possibly pruned away later).
Jeff
