On 11/20/2016 04:56 AM, Bernd Edlinger wrote:
Hi,
when you add a returns_nonnull to the builtin alloca
then this code in tree-vrp.c (gimple_stmt_nonzero_warnv_p)
should go away:
if (flag_delete_null_pointer_checks &&
lookup_attribute ("returns_nonnull",
TYPE_ATTRIBUTES (gimple_call_fntype (stmt))))
return true;
return gimple_alloca_call_p (stmt);
Yes, that looks like it should be superfluous now. Let me test
it. (It will make the null pointer test removal conditional on
flag_delete_null_pointer_checks but presumably that's okay.)
Regarding __builtin_alloca_with_align, I see no test cases
for that function:
__builtin_alloca_with_align is a bit special, because it allocates
more than the first parameter says, I think it allocates (if size != 0)
size + align/8 in order to be able to return an aligned object.
What if align is very large?
I'm guessing there are only a handful of tests for
__builtin_alloca_with_align because the built-in is used to allocate
VLAs and so it's exercised indirectly by VLA tests. There is one
fairly comprehensive test for the built-in that exercises the case
you mention: builtins-68.c, though perhaps not to the extent it
could or should. The problem is that there is no mechanism for
a compiler to indicate that a VLA has failed to allocate. So while
the following compiles (with my patch with a warning), it crashes
at runtime:
$ cat x.c && gcc -O2 -S x.c
void sink (void*);
void g (void)
{
struct S { int _Alignas (1024) i; };
unsigned long N = __SIZE_MAX__ / _Alignof (struct S);
struct S vla [N];
sink (vla);
}
x.c: In function ‘g’:
x.c:9:12: warning: argument 1 value ‘18446744073709550592ul’ exceeds
maximum object size 9223372036854775807 [-Walloc-size-larger-than=]
struct S vla [N];
^~~
x.c:9:12: note: in a call to built-in allocation function
‘__builtin_alloca_with_align’
Or, with -Wvla-larger-than=1000 (or any other positive number):
x.c:9:12: warning: argument to variable-length array is too large
[-Wvla-larger-than=]
struct S vla [N];
^~~
x.c:9:12: note: limit is 1000 bytes, but argument is 18446744073709550592
I still would prefer separate -walloc-zero and -Walloca-zero
options. Likewise for the -Walloc-larger-than and -Walloca-larger-than
or better -Wmalloc-larger-than and -Walloca-larger-than ?
Sure, I'm open to it. But before I invest time into tweaking
things further I would like to get consensus on the two patches
I've posted (alloca(0) and this one), and ideally get them
at least tentatively approved. Otherwise, I would worry it
might be a wasted effort.
Martin
