On 2/18/21 2:30 AM, Jakub Jelinek wrote:
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.
There's only one subscript in '((int*)a)[0] = 0;' and in the vrp1 IL
and that's the one the warning mentions. The size of the VLA is zero
so it doesn't matter what the index is, all accesses to it are out of
bounds. I see nothing wrong here.
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.
I'm not sure I follow what you're saying here either. The vrp1 dump
has this:
void g (int n)
{
struct S a[0:D.1951];
<bb 2> [local count: 1073741824]:
MEM[(int *)&a] = 0;
f (&a);
a ={v} {CLOBBER};
return;
}
The size of the VLA is zero regardless of its bound and accessing
it is invalid so the warning is expected.
VLAs of zero-lengthg arrays are without a doubt rare, pathological
cases. We could special case the warning for them and print
a different message but I see very little value in complicating
the code just for them. Do you consider this special casing
a requirement for approving the fix for the ICE?
Martin
