Re: [PATCH] c/67882 - improve -Warray-bounds for invalid offsetof

[Bernd Schmidt <bschmidt@redhat.com>]

On 10/20/2015 06:53 PM, Joseph Myers wrote:
> On Tue, 20 Oct 2015, Martin Sebor wrote:
>
>> I think -Warray-bounds should emit consistent diagnostics for invalid
>> array references regardless of the contexts. I.e., given
>>
>>      struct S {
>>          int A [5][7];
>>          int x;
>>      } s;
>>
>> these should both be diagnosed:
>>
>>      int i = offsetof (struct S, A [0][7]);
>>
>>      int *p = &s.A [0][7];
>>
>> because they are both undefined and both can lead to surprising
>> results when used.
>
> But both are valid.  &s.A [0][7] means s.A[0] + 7 (as explicitly specified
> in C11, neither the & nor the [] is evaluated in this case, but the []
> turns into a +), and s.A[0] is an object of type int[7], which decays to a
> pointer to the first element of that array, so adding 7 produces a
> just-past-end pointer.  It's not valid to dereference that pointer, but
> the pointer itself is valid (and subtracting 1 from it produces a pointer
> you can dereference).

Thanks, Joseph, this is helpful. There are a few other cases that I'm 
uncertain about and which might require additional changes to the patch; 
the one I sent was an experiment more than a finished product.

Consider

struct t { int a; int b; };
struct A { struct t v[2]; } a;

So I think we've established that
   &a.v[2]
is valid, giving a pointer just past the end of the structure. How about
   &a.v[2].a
and
   &a.v[2].b
The first of these gives the same pointer just past the end of the 
array, while the second one gives a higher address. I would expect that 
the second one is invalid, but I'm not so sure about the first one. 
Syntactically we have an access to an out-of-bounds field, but the whole 
expression just computes the valid one-past-the-end address.

I think this has an impact on the tests I quoted in my last mail:

typedef struct FA5_7 {
   int i;
   char a5_7 [5][7];
} FA5_7;

     __builtin_offsetof (FA5_7, a5_7 [0][7]),         // { dg-warning 
"index" }
     __builtin_offsetof (FA5_7, a5_7 [1][7]),         // { dg-warning 
"index" }
     __builtin_offsetof (FA5_7, a5_7 [5][0]),         // { dg-warning 
"index" }
     __builtin_offsetof (FA5_7, a5_7 [5][7]),         // { dg-warning 
"index" }

Here I think the last one of these is most likely invalid (being 8 bytes 
past the end of the object, rather than just one) and the others valid. 
Can you confirm this? (If the &a.v[2].a example is considered invalid, 
then I think the a5_7[5][0] test would be the equivalent and ought to 
also be considered invalid).

It might turn out that we have to do something like pass a pointer to an 
"at_end" boolean to recursive invocations, which would set it to true if 
the expression dealt with by the caller should not increase the address 
any further. Then, when folding the addition we check that the offset we 
have is either zero or we don't have at_end, and warn otherwise.


Bernd