Re: [PATCH] handle VLA of zero length arrays and vice versa (PR 99121)

[Jakub Jelinek <jakub@redhat.com>]

On Wed, Feb 17, 2021 at 02:11:43PM -0700, Martin Sebor wrote:
> On 2/17/21 1:47 PM, Jakub Jelinek wrote:
> > On Wed, Feb 17, 2021 at 01:27:55PM -0700, Martin Sebor wrote:
> > 
> > Not in this patch, but I've looked at what maxobjsize is and wonder why
> > the roundtrip tree -> HOST_WIDE_INT -> offset_int:
> >    const offset_int maxobjsize = tree_to_shwi (max_object_size ());
> > Can't it be
> >    const offset_int maxobjsize = wi::to_offset (max_object_size ());
> > ?
> 
> Yes, that's what it is elsewhere so this should do the same.  I've
> changed it.

Ok.

> > Doesn't arrbounds[1] == 0 mean there will be warnings for any accesses?
> > For eltsize == 0 I think you shouldn't warn when nelts isn't known,
> > instead of always warning, arr[100000000] will have the same address as
> > arr[0] ...
> 
> This branch is entered for VLAs of zero-length arrays where we want
> to warn, like this:
> 
> void f (void*);
> 
> void g (int n)
> {
>   int a[n][0];
>   ((int*)a)[0] = 0;
>   f (a);
> }

For this you do want to warn, but not the way you warn with the patch:
xxx.c: In function ‘g’:
xxx.c:6:12: warning: array subscript 0 is outside array bounds of ‘int[<Uec60>][0]’ [-Warray-bounds]
    6 |   ((int*)a)[0] = 0;
      |   ~~~~~~~~~^~~
xxx.c:5:7: note: while referencing ‘a’
    5 |   int a[n][0];
      |       ^

The message doesn't make it clear which of the two subscripts is out of
bounds, yes, for [0] it would be outside of bounds, but for the VLA index
no index < n would be outside of bounds.

Consider a different (GNU C, in C++ struct S has non-zero size) testcase:
void f (void*);

void g (int n)
{
  struct S {} a[n];
  ((int*)a)[0] = 0;
  f (a);
}
yyy.c:6:12: warning: array subscript 0 is outside array bounds of ‘struct S[<Ucc60>]’ [-Warray-bounds]
    6 |   ((int*)a)[0] = 0;
      |   ~~~~~~~~~^~~
yyy.c:5:15: note: while referencing ‘a’
    5 |   struct S {} a[n];
      |               ^
I bet that means you are really complaining about the VLA bound rather than
the [0] bound even in the first case, because the wording is otherwise the
same.  And for g (154) the array subscript 0 is certainly not a problem,
so the warning would need to be worded differently in that case.

	Jakub