This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] tighten up -Wbuiltin-declaration-mismatch (PR 86125)


Attached is an updated patch to tighten up the warning and also
prevent ICEs in the middle-end like in PR 86308 or PR 86202.

I took Richard's suggestion to add the POINTER_TYPE_P() check
to detect pointer/integer conflicts.  That also avoids the ICEs
above.

I also dealt with the fileptr_type_node problem so that file
I/O built-ins can be declared to take any object pointer type
as an argument, and that argument has to be the same for all
them.

I'm not too happy about the interaction with -Wextra but short
of enabling the stricter checks even without it or introducing
multiple levels for -Wbuiltin-declaration-mismatch I don't see
a good alternative.

Martin
PR c/86125 - missing -Wbuiltin-declaration-mismatch on a mismatched return type
PR middle-end/86308 - ICE in verify_gimple calling index() with an invalid declaration
PR middle-end/86202 - ICE in get_range_info calling an invalid memcpy() declaration

gcc/c/ChangeLog:

	PR c/86125
	PR middle-end/86202
	PR middle-end/86308
	* c-decl.c (match_builtin_function_types): Add arguments.
	(diagnose_mismatched_decls): Diagnose mismatched declarations
	of built-ins more strictly.
	* doc/invoke.texi (-Wbuiltin-declaration-mismatch): Update.

gcc/testsuite/ChangeLog:

	PR c/86125
	PR middle-end/86202
	PR middle-end/86308
	* gcc.dg/Wbuiltin-declaration-mismatch.c: New test.
	* gcc.dg/Wbuiltin-declaration-mismatch-2.c: New test.
	* gcc.dg/Wbuiltin-declaration-mismatch-3.c: New test.
	* gcc.dg/Wbuiltin-declaration-mismatch-4.c: New test.
	* gcc.dg/builtins-69.c: New test.

diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index af16cfd..6c9e667 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -1628,43 +1628,82 @@ c_bind (location_t loc, tree decl, bool is_global)
   bind (DECL_NAME (decl), decl, scope, false, nested, loc);
 }
 
+
 /* 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;
+
+  tree oldargs = TYPE_ARG_TYPES (oldtype);
+  tree newargs = TYPE_ARG_TYPES (newtype);
+  tree tryargs = newargs;
 
-  while (oldargs || 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);
+
+      /* Fail for types with incompatible modes/sizes.  */
+      if (TYPE_MODE (TREE_VALUE (oldargs))
+	  != TYPE_MODE (TREE_VALUE (newargs)))
+	return NULL_TREE;
+
+      /* Fail for function and object pointer mismatches.  */
+      if (FUNCTION_POINTER_TYPE_P (oldtype) != FUNCTION_POINTER_TYPE_P (newtype)
+	  || POINTER_TYPE_P (oldtype) != POINTER_TYPE_P (newtype))
+	return NULL_TREE;
+
+      if (oldtype == fileptr_type_node)
+	{
+	  /* Store the first FILE* argument type seen (whatever it is),
+	     and expect any subsequent declarations of file I/O built-ins
+	     to refer to it rather than to fileptr_type_node which is just
+	     void*.  */
+	  static tree last_fileptr_type;
+	  if (last_fileptr_type)
+	    {
+	      if (!comptypes (last_fileptr_type, newtype))
+		{
+		  *argno = i;
+		  *strict = last_fileptr_type;
+		}
+	    }
+	  else
+	    last_fileptr_type = newtype;
+	}
+      else 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,9 +1913,18 @@ diagnose_mismatched_decls (tree newdecl, tree olddecl,
       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;
@@ -1885,11 +1933,30 @@ diagnose_mismatched_decls (tree newdecl, tree olddecl,
 	      /* 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))
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index f231da3..6146e0e 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -6570,8 +6570,10 @@ attributes.
 @item -Wno-builtin-declaration-mismatch
 @opindex Wno-builtin-declaration-mismatch
 @opindex Wbuiltin-declaration-mismatch
-Warn if a built-in function is declared with the wrong signature or 
-as non-function.
+Warn if a built-in function is declared as a non-function or as a function
+with a grossly mismatched signature.  Subtle mismatches such as in pointer
+qualifiers or integer signedness are only diagnosed when @option{-Wextra}
+is enabled as well.
 This warning is enabled by default.
 
 @item -Wno-builtin-macro-redefined
diff --git a/gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-2.c b/gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-2.c
new file mode 100644
index 0000000..32af9fe
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-2.c
@@ -0,0 +1,18 @@
+/* PR c/86125 - missing -Wbuiltin-declaration-mismatch on a mismatched
+   return type
+   Verify that declarations of file I/O built-ins with an arbitrary
+   object pointer do not trigger -Wbuiltin-declaration-mismatch.
+   { dg-do compile }
+   { dg-options "-Wbuiltin-declaration-mismatch -Wextra" } */
+
+typedef __SIZE_TYPE__ size_t;
+
+struct StdioFile;
+
+int fprintf (struct StdioFile*, const char*, ...);
+int vfprintf (struct StdioFile*, const char*, __builtin_va_list);
+int fputc (int, struct StdioFile*);
+int fputs (const char*, struct StdioFile*);
+int fscanf (struct StdioFile*, const char*, ...);
+int vfscanf (struct StdioFile*, const char*, __builtin_va_list);
+size_t fwrite (const void*, size_t, size_t, struct StdioFile*);
diff --git a/gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-3.c b/gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-3.c
new file mode 100644
index 0000000..77a4bff
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-3.c
@@ -0,0 +1,26 @@
+/* PR c/86125 - missing -Wbuiltin-declaration-mismatch on a mismatched
+   return type
+   Verify that a declaration of vfprintf() with withe the wrong last
+   argument triggers -Wbuiltin-declaration-mismatch even without -Wextra.
+   { dg-do compile }
+   { dg-options "-Wbuiltin-declaration-mismatch" } */
+
+struct StdioFile;
+
+typedef __SIZE_TYPE__ size_t;
+
+struct StdioFile;
+
+int fprintf (struct StdioFile*, const char*);   /* { dg-warning "conflicting types for built-in function .fprintf.; expected .int\\\(\[a-z_\]+ \\\*, const char \\\*, \.\.\.\\\)." } */
+
+int vfprintf (struct StdioFile*, const char*, ...);   /* { dg-warning "conflicting types for built-in function .vfprintf.; expected .int\\\(\[a-z_\]+ \\\*, const char \\\*, \[a-z_\]+ \\\*\\\)." } */
+
+int fputc (char, struct StdioFile*);   /* { dg-warning "conflicting types for built-in function .fputc.; expected .int\\\(int,  void \\\*\\\)." } */
+
+size_t fputs (const char*, struct StdioFile*);   /* { dg-warning "conflicting types for built-in function .fputs.; expected .int\\\(const char \\\*, \[a-z_\]+ \\\*\\\)." } */
+
+int fscanf (struct StdioFile*, const char*, size_t, ...);   /* { dg-warning "conflicting types for built-in function .fscanf.; expected .int\\\(\[a-z_\]+ \\\*, const char \\\*, \.\.\.\\\)." } */
+
+int vfscanf (struct StdioFile*, const char*, ...);   /* { dg-warning "conflicting types for built-in function .vfscanf.; expected .int\\\(\[a-z_\]+ \\\*, const char \\\*, \[a-z_\]+ \\\*\\\)." } */
+
+size_t fwrite (const void*, size_t, size_t, struct StdioFile);    /* { dg-warning "conflicting types for built-in function .fwrite.; expected .\(long \)?unsigned int\\\(const void \\\*, \(long \)?unsigned int, *\(long \)?unsigned int, *\[a-z_\]+ \\\*\\\)." } */
diff --git a/gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-4.c b/gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-4.c
new file mode 100644
index 0000000..d06af6528
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-4.c
@@ -0,0 +1,26 @@
+/* PR c/86125 - missing -Wbuiltin-declaration-mismatch on a mismatched
+   return type
+   Verify that declarations of file I/O built-ins with different
+   definitions of struct FILE triggers -Wbuiltin-declaration-mismatch
+   when -Wextra is specified.
+   { dg-do compile }
+   { dg-options "-Wall -Wbuiltin-declaration-mismatch" } */
+
+struct FooFile;
+int fputc (int, struct FooFile*);
+
+typedef struct FooFile AlsoFooFile;
+int fprintf (AlsoFooFile*, const char*, ...);
+
+typedef AlsoFooFile* FooFilePtr;
+int fscanf (FooFilePtr, const char*, ...);
+
+/* No warning here (-Wextra not specified).  */
+struct BarFile;
+int vfprintf (struct BarFile*, const char*, __builtin_va_list);
+
+
+/* Set -Wextra and verify -Wbuiltin-declaration-mismatch is issued.  */
+#pragma GCC diagnostic warning "-Wextra"
+
+int fputs (const char*, struct BarFile*);   /* { dg-warning "mismatch in argument 2 type of built-in function .fputs.; expected .struct FooFile \\\*." } */
diff --git a/gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch.c b/gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch.c
new file mode 100644
index 0000000..498876f
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch.c
@@ -0,0 +1,17 @@
+/* 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\\)." } */
+
+int fprintf (int, const char*, ...);   /* { dg-warning "conflicting types for built-in function .fprintf.; " } */ 
diff --git a/gcc/testsuite/gcc.dg/builtins-69.c b/gcc/testsuite/gcc.dg/builtins-69.c
new file mode 100644
index 0000000..26dfb3b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/builtins-69.c
@@ -0,0 +1,22 @@
+/* PR middle-end/86308 - ICE in verify_gimple calling index() with
+   an invalid declaration
+   { dg-do compile }
+   { dg-options "-O2 -Wall" }  */
+
+int index (int, int);   /* { dg-warning "conflicting types for built-in function .index.; expected .char \\\*\\\(const char \\\*, int\\\)." } */
+
+int test_index (void)
+{
+  return index (0, 0);
+}
+
+
+/* PR middle-end/86202 - ICE in get_range_info calling an invalid memcpy()
+   declaration */
+
+void *memcpy (void *, void *, __SIZE_TYPE__ *);   /* { dg-warning "conflicting types for built-in function .memcpy.; expected .void \\\*\\\(void \\\*, const void \\\*, \(long \)?unsigned int\\\)." } */
+
+void test_memcpy (void *p, void *q, __SIZE_TYPE__ *r)
+{
+  memcpy (p, q, r);
+}

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]