[PATCH] adjust "partly out of bounds" warning (PR 98503)

[Martin Sebor]

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:

   struct A { int i; };
   struct B { int i; };

   struct A a;
   B *p = (B*)&a;   // FE would warn here (even if there's no access)

That's also why the current -Warray-bounds (and the proposed
-Wstrict-aliasing refinement) only triggers for accesses via larger
types, as in:

   struct C { int i, j; };
   void *p = &a;    // FE can't warn here
   C *q = (C*)p;    // or here
   p->j = 0;        // middle end warns here

Martin

PS The existing -Wsrtrict-aliasing in the front end is superficial
at best.  It only detects problems in simple accesses that don't
involve any data flow.  For instance it warns only if f() below but
not in g():

   int a;

   void f (void)
   {
     *(float*)&a = 0;   // -Wstrict-aliasing
   }

   void g (void)
   {
     float *p = (float*)&a;
     *p = 0;            // silence
   }

I'd like to improve it at some point but to catch the problem in g()
the warning will certainly have to move into the middle end.