On 11/17/21 11:31 AM, Jason Merrill wrote:
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.
Sorry, I don't know enough yet to judge this.
Since the error is unrelated to what I'm fixing I would prefer
not to introduce it in the same patch. I'm happy to open
a separate bug for the missing error for the test case above,
look some more into why it isn't issued, and if it's decided
the error is intended either add the call back to trigger it
or do whatever else may be more appropriate).
Are you okay with me going ahead and committing the most recent
patch as is?
If not, do you want me to commit the previous version and change
the weak-3.c test to expect the error?
Martin
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.
