On 8/31/21 10:08 PM, Martin Sebor wrote:
A Coverity run recently uncovered a latent bug in GCC that GCC should
be able to detect itself: comparing the address of a declared object
for equality to null, similar to:
int f (void)
{
int a[2][2];
return &a == 0;
}
GCC issues -Waddress for this code, but the bug Coverity found was
actually closer to the following:
int f (void)
{
int a[2][2];
return a[0] == 0;
}
where the hapless author (yours truly) meant to compare the value
of a[0][0] (as in r12-3268).
This variant is not diagnosed even though the bug in it is the same
and I'd expect more likely to occur in practice. (&a[0] == 0 isn't
diagnosed either, though that's a less likely mistake to make).
The attached patch enhances -Waddress to detect this variant along
with a number of other similar instances of the problem, including
comparing the address of array members to null.
Besides these, the patch also issues -Waddress for null equality
tests of pointer-plus expressions such as in:
int g (int i)
{
return a[0] + i == 0;
}
and in C++ more instances of pointers to members.
Testing on x86_64-linux, besides a few benign issues in GCC sources
a regression test, run shows a failure in gcc.dg/Waddress.c. That's
a test added after GCC for some reason stopped warning for one of
the basic cases that other tools warn about (comparing an array to
null). I suspect the change was unintentional because GCC still
warns for other very similar expressions. The reporter who also
submitted the test in pr36299 argued that the warning wasn't
helpful because tests for arrays sometimes come from macros, and
the test was committed after it was noted that GCC no longer warned
for the reporter's simple case. While it's certainly true that
the warning can be triggered by the null equality tests in macros
(the patch exposed two such instances in GCC) they are easy to
avoid (the patch adds a an additional escape hatch). At the same
time, as is evident from the Coverity bug report and from the two
issues the enhancement exposes in the FORTRAN front end (even if
benign), issuing the warning in these cases does help find bugs
or mistaken assumptions. With that, I've changed the test to
expect the restored -Waddress warning instead.
Testing with Glibc exposed a couple of harmless comparisons of
arrays a large macro in vfprintf-internal.c. I'll submit a fix
to avoid the -Waddress instances if/when this enhancement is
approved.
Testing with Binutils/GDB also turned up a couple of pointless
comparison of arrays to null and a couple of uses in macros that
can be trivially suppressed.
Martin
PS Clang issues a warning for some of the same null pointer tests
the patch diagnoses, including gcc.dg/Waddress.c, except under at
least three different options: some under -Wpointer-bool-conversion,
others under -Wtautological-pointer-compare, and others still under
-Wtautological-compare.
+ while (TREE_CODE (cop) == ARRAY_REF
+ || TREE_CODE (cop) == COMPONENT_REF)
+ {
+ unsigned opno = TREE_CODE (cop) == COMPONENT_REF;
+ cop = TREE_OPERAND (cop, opno);
+ }
1) Maybe 'while (handled_component_p (cop))'?
2) Why handle COMPONENT_REF differently? Operand 1 is the FIELD_DECL,
which doesn't have an address of its own.
Jason
