On 06/14/2018 07:36 AM, Richard Biener wrote:
On Thu, Jun 14, 2018 at 12:35 AM Martin Sebor <mse...@gmail.com> wrote:
The C implementation of the -Wbuiltin-declaration-mismatch
warning relies on TYPE_MODE to detect incompatibilities
between return and argument types in user declarations of
built-in functions. That prevents mistakes like
char* strlen (const char*);
from being detected (where sizeof (char*) == sizeof (size_t)),
while at the same diagnosing similar bugs such as
char* strcmp (const char*, const char*);
where sizeof (char*) != sizeof (int), and always diagnosing
benign declarations like:
void strcpy (char*, const char*)
The attached patch tightens up the detection of incompatible
types so that when -Wextra is set it diagnoses more of these
kinds of problems, including mismatches in qualifiers. (I
added this under -Wextra mainly to avoid the warning with
just -Wall for some of the more benign incompatibilities
like those in const-qualifiers).
This enhancement was prompted by bug 86114. As it is, it
would not have prevented the ICE in that bug because it does
not reject the incompatible redeclaration (I did that to keep
compatibility with prior GCC versions). If there is support
for it, though, I think rejecting all incompatible declarations
would be a better solution. Partly because the middle-end
doesn't seem to fully consider incompatible return types and
so might end up introducing transformations that don't make
sense. And partly because I think at least the C and POSIX
standard built-in functions should be declared with
the types they are specified.
Why not go the simpler way of doing
|| POINTER_TYPE_P (oldrettype) != POINTER_TYPE_P (newrettype)
after the mode check? I don't think a mismatch in pointer vs.
non-pointer was a desired loophole.
Yes, I don't either.
But detecting only pointer/integer mismatches would still allow
declaring built-ins with incompatible pointer types (e.g., strlen
(void*)), or do anything to remove any of the other warts and
inconsistencies.
As I showed above, the warning allows certain questionable
mismatches (even beyond pointers/integers) while diagnosing
entirely benign ones. For instance, declaring snprintf that
returns a struct the size of an int is silently accepted
(regardless of its members) but declaring it void is.
Besides finding the most egregious mistakes I would like
the warning to prompt users to declare standard built-ins
with the exact types they are specified with, including const
qualifiers. It's a gap even in C's weak type system to let
programs that (accidentally or otherwise) declare built-ins
with incompatible types to do conversions that GCC otherwise
takes pains to diagnose (e.g., -Wformat warning about passing
non-char* to %s, or types other than int to %i, etc).
There's a comment in the code about the weak checking being
deliberate (the word harmless here suggests the author may
not have fully appreciated all the conversions it allows):
/* Accept harmless mismatch in function types.
This is for the ffs and fprintf builtins. */
If you concerned about the change causing trouble in this
context, then if there are legacy systems that GCC still
supports that declare standard functions with non-standard
signatures they should be taken into consideration somehow.
Maybe by having the warning for them depend on the language
conformance mode (-std=mode). Since the comment only talks
about ffs and fprintf, perhaps the checking should be relaxed
only for those (and only for the legacy targets). But
I haven't thought too hard about this and there may be better
solutions if this is something we need to worry about.
Martin
