On 04/20/2017 02:35 PM, Bernd Edlinger wrote:
Hi!
This implements a new -Wall enabled warning for a rather common, but
completely wrong way to compute an array size by dividing the
sizeof(pointer) / sizeof(pointer[0]) or sizeof(*pointer).
It is often hard to find this kind of error by simple code inspection
in real code, because using sizeof in this way is a quite common idiom
to get the array size of an array variable. And furthermore this
expression may be used in macros, which makes it even more important to
have this warning.
There is a similar warning -Wsizeof-pointer-memaccess which helped in
implementing the infrastructure for the new warning in the C FE.
However I noticed that the -Wsizeof-pointer-memaccess warning was
missing in C, when the sizeof is used inside parentheses, which is
different from C++, so I fixed that too.
Of course, I added some test cases for that as well.
To illustrate the usefulness of this warning, it revealed quite a few
places where bogus sizeof divisions were used in our testsuite.
Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?
That seems like a useful warning. Just a few comments.
First, -Wsizeof-array-argument already diagnoses a subset of
the same problems. For example, with the patch applied, GCC
issues the two warnings below for following test case. One
should be sufficient.
$ cat y.c && gcc -S -Wall y.c
int f (int a[])
{
return sizeof a / sizeof *a;
}
y.c: In function ‘f’:
y.c:3:17: warning: ‘sizeof’ on array function parameter ‘a’ will
return size of ‘int *’ [-Wsizeof-array-argument]
return sizeof a / sizeof *a;
^
y.c:1:12: note: declared here
int f (int a[])
^
y.c:3:19: warning: dividing the pointer size by the element size
[-Wsizeof-pointer-div]
return sizeof a / sizeof *a;
^
Second, I would suggest mentioning the actual types of the operands
rather than referring to "pointer size" and "element size." Maybe
something like:
division 'sizeof (int*) / sizeof (int)' does not compute the number
of array elements
I suggest avoiding "element size" because the pointed-to argument
need not be an array. Mentioning the types should help users better
understand the problem (especially in C++ where types are often
obscured by layers of templates). It might also be a nice touch
to add a note pointing to the declaration of the first sizeof
operand (if it's an object).
Martin
