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.

Martin
PR c/86125 - missing -Wbuiltin-declaration-mismatch on a mismatched return type

gcc/c/ChangeLog:

	PR c/86125
	* c-decl.c (match_builtin_function_types): Add arguments.
	(diagnose_mismatched_decls): Diagnose mismatched declarations
	of built-ins more strictly.

gcc/testsuite/ChangeLog:

	PR c/86125
	* gcc.dg/Wbuiltin-declaration-mismatch.c: New test.

Index: gcc/c/c-decl.c
===================================================================
--- gcc/c/c-decl.c	(revision 261551)
+++ gcc/c/c-decl.c	(working copy)
@@ -1630,41 +1630,54 @@ c_bind (location_t loc, tree decl, bool is_global)
 
 /* Subroutine of compare_decls.  Allow harmless mismatches in return
    and argument types provided that the type modes match.  This function
-   return a unified type given a suitable match, and 0 otherwise.  */
+   returns a unified type given a suitable match, and 0 otherwise.  */
 
 static tree
-match_builtin_function_types (tree newtype, tree oldtype)
+match_builtin_function_types (tree newtype, tree oldtype,
+			      tree *strict, unsigned *argno)
 {
-  tree newrettype, oldrettype;
-  tree newargs, oldargs;
-  tree trytype, tryargs;
-
   /* Accept the return type of the new declaration if same modes.  */
-  oldrettype = TREE_TYPE (oldtype);
-  newrettype = TREE_TYPE (newtype);
+  tree oldrettype = TREE_TYPE (oldtype);
+  tree newrettype = TREE_TYPE (newtype);
 
+  *argno = 0;
+  *strict = NULL_TREE;
+
   if (TYPE_MODE (oldrettype) != TYPE_MODE (newrettype))
     return NULL_TREE;
 
-  oldargs = TYPE_ARG_TYPES (oldtype);
-  newargs = TYPE_ARG_TYPES (newtype);
-  tryargs = newargs;
+  if (!comptypes (oldrettype, newrettype))
+    *strict = oldrettype;
 
-  while (oldargs || newargs)
+  tree oldargs = TYPE_ARG_TYPES (oldtype);
+  tree newargs = TYPE_ARG_TYPES (newtype);
+  tree tryargs = newargs;
+
+  for (unsigned i = 1; oldargs || newargs; ++i)
     {
       if (!oldargs
 	  || !newargs
 	  || !TREE_VALUE (oldargs)
-	  || !TREE_VALUE (newargs)
-	  || TYPE_MODE (TREE_VALUE (oldargs))
-	     != TYPE_MODE (TREE_VALUE (newargs)))
+	  || !TREE_VALUE (newargs))
 	return NULL_TREE;
 
+      tree oldtype = TREE_VALUE (oldargs);
+      tree newtype = TREE_VALUE (newargs);
+      if (TYPE_MODE (TREE_VALUE (oldargs))
+	  != TYPE_MODE (TREE_VALUE (newargs)))
+	return NULL_TREE;
+
+      if (!*strict && !comptypes (oldtype, newtype))
+	{
+	  *argno = i;
+	  *strict = oldtype;
+	}
+
       oldargs = TREE_CHAIN (oldargs);
       newargs = TREE_CHAIN (newargs);
     }
 
-  trytype = build_function_type (newrettype, tryargs);
+  tree trytype = build_function_type (newrettype, tryargs);
 
   /* Allow declaration to change transaction_safe attribute.  */
   tree oldattrs = TYPE_ATTRIBUTES (oldtype);
@@ -1874,10 +1887,19 @@ diagnose_mismatched_decls (tree newdecl, tree oldd
       if (TREE_CODE (olddecl) == FUNCTION_DECL
 	  && DECL_BUILT_IN (olddecl) && !C_DECL_DECLARED_BUILTIN (olddecl))
 	{
-	  /* Accept harmless mismatch in function types.
-	     This is for the ffs and fprintf builtins.  */
-	  tree trytype = match_builtin_function_types (newtype, oldtype);
+	  /* Accept "harmless" mismatch in function types such as
+	     missing qualifiers or pointer vs same size integer
+	     mismatches.  This is for the ffs and fprintf builtins.
+	     However, with -Wextra in effect, diagnose return and
+	     argument types that are incompatible according to
+	     language rules. */
+	  tree mismatch_expect;
+	  unsigned mismatch_argno;
 
+	  tree trytype = match_builtin_function_types (newtype, oldtype,
+						       &mismatch_expect,
+						       &mismatch_argno);
+
 	  if (trytype && comptypes (newtype, trytype))
 	    *oldtypep = oldtype = trytype;
 	  else
@@ -1885,11 +1907,30 @@ diagnose_mismatched_decls (tree newdecl, tree oldd
 	      /* If types don't match for a built-in, throw away the
 		 built-in.  No point in calling locate_old_decl here, it
 		 won't print anything.  */
-	      warning (OPT_Wbuiltin_declaration_mismatch,
-		       "conflicting types for built-in function %q+D",
-		       newdecl);
+	      warning_at (DECL_SOURCE_LOCATION (newdecl),
+			  OPT_Wbuiltin_declaration_mismatch,
+			  "conflicting types for built-in function %qD; "
+			  "expected %qT",
+			  newdecl, oldtype);
 	      return false;
 	    }
+
+	  if (mismatch_expect && extra_warnings)
+	    {
+	      /* If types match only loosely, print a warning but accept
+		 the redeclaration.  */
+	      location_t newloc = DECL_SOURCE_LOCATION (newdecl);
+	      if (mismatch_argno)
+		warning_at (newloc, OPT_Wbuiltin_declaration_mismatch,
+			    "mismatch in argument %u type of built-in "
+			    "function %qD; expected %qT",
+			    mismatch_argno, newdecl, mismatch_expect);
+	      else
+		warning_at (newloc, OPT_Wbuiltin_declaration_mismatch,
+			    "mismatch in return type of built-in "
+			    "function %qD; expected %qT",
+			    newdecl, mismatch_expect);
+	    }
 	}
       else if (TREE_CODE (olddecl) == FUNCTION_DECL
 	       && DECL_IS_BUILTIN (olddecl))
Index: gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch.c
===================================================================
--- gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch.c	(nonexistent)
+++ gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch.c	(working copy)
@@ -0,0 +1,15 @@
+/* PR c/86125 - missing -Wbuiltin-declaration-mismatch on a mismatched
+   return type
+   { dg-do compile }
+   { dg-options "-Wbuiltin-declaration-mismatch -Wextra" } */
+
+char* strlen (const char*);   /* { dg-warning "mismatch in return type of built-in function .strlen.; expected .(long )?unsigned int." } */
+
+enum E { e };
+enum E strcmp (const char*, const char*);   /* { dg-warning "mismatch in return type of built-in function .strcmp.; expected .int." } */
+
+char* strcpy (char*, char*);   /* { dg-warning "mismatch in argument 2 type of built-in function .strcpy.; expected .const char \\*." } */
+
+char* strcat (const char*, char*);   /* { dg-warning "mismatch in argument 1 type of built-in function .strcat.; expected .char \\*." } */
+
+void strchr (const char*, int);      /* { dg-warning "conflicting types for built-in function .strchr.; expected .char \\*\\(const char \\*, int\\)." } */

Reply via email to