On 11/16/21 20:11, Martin Sebor wrote:
On 11/16/21 1:23 PM, Jason Merrill wrote:
On 10/23/21 19:06, Martin Sebor wrote:
On 10/4/21 3:37 PM, Jason Merrill wrote:
On 10/4/21 14:42, Martin Sebor wrote:
While resolving the recent -Waddress enhancement request (PR
PR102103) I came across a 2007 problem report about GCC 4 having
stopped warning for using the address of inline functions in
equality comparisons with null. With inline functions being
commonplace in C++ this seems like an important use case for
the warning.
The change that resulted in suppressing the warning in these
cases was introduced inadvertently in a fix for PR 22252.
To restore the warning, the attached patch enhances
the decl_with_nonnull_addr_p() function to return true also for
weak symbols for which a definition has been provided.
I think you probably want to merge this function with
fold-const.c:maybe_nonzero_address, which already handles more cases.
maybe_nonzero_address() doesn't behave quite like
decl_with_nonnull_addr_p() expects and I'm reluctant to muck
around with the former too much since it's used for codegen,
while the latter just for warnings. (There is even a case
where the functions don't behave the same, and would result
in different warnings between C and C++ without some extra
help.)
So in the attached revision I just have maybe_nonzero_address()
call decl_with_nonnull_addr_p() and then refine the failing
(or uncertain) cases separately, with some overlap between
them.
Since I worked on this someone complained that some instances
of the warning newly enhanced under PR102103 aren't suppresed
in code resulting from macro expansion. Since it's trivial,
I include the fix for that report in this patch as well.
+ allocated stroage might have a null address. */
typo.
OK with that fixed.
After retesting the patch before committing I noticed it triggers
a regression in weak/weak-3.c that I missed the first time around.
Here's the test case:
extern void * ffoo1f (void);
void * foo1f (void)
{
if (ffoo1f) /* { dg-warning "-Waddress" } */
ffoo1f ();
return 0;
}
void * ffoox1f (void) { return (void *)0; }
extern void * ffoo1f (void) __attribute__((weak, alias ("ffoox1f")));
The unexpected error is:
a.c: At top level:
a.c:1:15: error: ‘ffoo1f’ declared weak after being used
1 | extern void * ffoo1f (void);
| ^~~~~~
The error is caused by the new call to maybe_nonzero_address()
made from decl_with_nonnull_addr_p(). The call registers
the symbol as used.
So unless the error is desirable for this case I think it's
best to go back to the originally proposed solution. I attach
it for reference and will plan to commit it tomorrow unless I
hear otherwise.
Hmm, the error seems correct to me: we tested whether the address is
nonzero in the dg-warning line, and presumably evaluating that test
could depend on the absence of weak.
PS I don't know enough about the logic behind issuing this error
in other situations to tell for sure that it's wrong in this one
but I see no difference in the emitted code for a case in the same
test that declares the alias first, before taking its address and
that's accepted and this one. I also checked that both Clang and
ICC accept the code either way, so I'm inclined to think the error
would be a bug.
