tighten up validation of built-in redeclarations (PR 93926)

Message ID 062e3164-0f7f-8352-f105-3250c380833a@gmail.com
State New
Headers show
Series
  • tighten up validation of built-in redeclarations (PR 93926)
Related show

Commit Message

Martin Sebor Feb. 27, 2020, 10:40 p.m.
GCC considers valid explicit declarations of built-ins whose return
types match in their modes, even if the types themselves are
incompatible (say integer and pointer of the same size).  This is
more permissive than for argument types where a pointer/integer
mismatch disqualifies the redeclaration.

With -Wextra enabled although -Wbuiltin-declaration-mismatch
diagnoses these "benign" mismatches in return types the C front-end
still considers the mismatched declaration one of the built-in.  That
can lead to problems down the line when the middle attempts to do its
own sanity checking based on some simple and reasonable notion of type
compatibility (like a malloc kind of function returning a pointer).

The attached patch tightens up the requirements a declaration has to
meet in order to match a built-in to apply the same matching to their
return types as to their arguments.

Tested on x86_64-linux.

Martin

Comments

Jeff Law Feb. 28, 2020, 10:12 p.m. | #1
On Thu, 2020-02-27 at 15:40 -0700, Martin Sebor wrote:
> GCC considers valid explicit declarations of built-ins whose return

> types match in their modes, even if the types themselves are

> incompatible (say integer and pointer of the same size).  This is

> more permissive than for argument types where a pointer/integer

> mismatch disqualifies the redeclaration.

> 

> With -Wextra enabled although -Wbuiltin-declaration-mismatch

> diagnoses these "benign" mismatches in return types the C front-end

> still considers the mismatched declaration one of the built-in.  That

> can lead to problems down the line when the middle attempts to do its

> own sanity checking based on some simple and reasonable notion of type

> compatibility (like a malloc kind of function returning a pointer).

> 

> The attached patch tightens up the requirements a declaration has to

> meet in order to match a built-in to apply the same matching to their

> return types as to their arguments.

> 

> Tested on x86_64-linux.

> 

> Martin

> PR middle-end/93926 - ICE on a built-in redeclaration returning an integer

> instead of a pointer

> 

> gcc/c/ChangeLog:

> 

> 	PR middle-end/93926

> 	* c-decl.c (types_close_enough_to_match): New function.

> 	(match_builtin_function_types):

> 	(diagnose_mismatched_decls): Add missing inform call to a warning.

> 

> gcc/testsuite/ChangeLog:

> 

> 	PR middle-end/93926

> 	* gcc.dg/Wbuiltin-declaration-mismatch-13.c: New test.

OK
jeff

Patch

PR middle-end/93926 - ICE on a built-in redeclaration returning an integer instead of a pointer

gcc/c/ChangeLog:

	PR middle-end/93926
	* c-decl.c (types_close_enough_to_match): New function.
	(match_builtin_function_types):
	(diagnose_mismatched_decls): Add missing inform call to a warning.

gcc/testsuite/ChangeLog:

	PR middle-end/93926
	* gcc.dg/Wbuiltin-declaration-mismatch-13.c: New test.


diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 1aa410db6e4..da276e981fa 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -1649,6 +1649,18 @@  c_bind (location_t loc, tree decl, bool is_global)
 
 static GTY(()) tree last_structptr_types[6];
 
+/* Returns true if types T1 and T2 representing return types or types
+   of function arguments are close enough to be considered interchangeable
+   in redeclarations of built-in functions.  */
+
+static bool
+types_close_enough_to_match (tree t1, tree t2)
+{
+  return (TYPE_MODE (t1) == TYPE_MODE (t2)
+	  && POINTER_TYPE_P (t1) == POINTER_TYPE_P (t2)
+	  && FUNCTION_POINTER_TYPE_P (t1) == FUNCTION_POINTER_TYPE_P (t2));
+}
+
 /* Subroutine of compare_decls.  Allow harmless mismatches in return
    and argument types provided that the type modes match.  Set *STRICT
    and *ARGNO to the expected argument type and number in case of
@@ -1659,16 +1671,19 @@  static tree
 match_builtin_function_types (tree newtype, tree oldtype,
 			      tree *strict, unsigned *argno)
 {
-  /* Accept the return type of the new declaration if same modes.  */
-  tree oldrettype = TREE_TYPE (oldtype);
-  tree newrettype = TREE_TYPE (newtype);
-
   *argno = 0;
   *strict = NULL_TREE;
 
-  if (TYPE_MODE (oldrettype) != TYPE_MODE (newrettype))
+  /* Accept the return type of the new declaration if it has the same
+     mode and if they're both pointers or if neither is.  */
+  tree oldrettype = TREE_TYPE (oldtype);
+  tree newrettype = TREE_TYPE (newtype);
+
+  if (!types_close_enough_to_match (oldrettype, newrettype))
     return NULL_TREE;
 
+  /* Check that the return types are compatible but don't fail if they
+     are not (e.g., int vs long in ILP32) and just let the caller know.  */
   if (!comptypes (TYPE_MAIN_VARIANT (oldrettype),
 		  TYPE_MAIN_VARIANT (newrettype)))
     *strict = oldrettype;
@@ -1692,15 +1707,7 @@  match_builtin_function_types (tree newtype, tree oldtype,
       tree oldtype = TYPE_MAIN_VARIANT (TREE_VALUE (oldargs));
       tree newtype = TYPE_MAIN_VARIANT (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))
+      if (!types_close_enough_to_match (oldtype, newtype))
 	return NULL_TREE;
 
       unsigned j = (sizeof (builtin_structptr_types)
@@ -1957,11 +1964,10 @@  diagnose_mismatched_decls (tree newdecl, tree olddecl,
 	  && !C_DECL_DECLARED_BUILTIN (olddecl))
 	{
 	  /* Accept "harmless" mismatches 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.  */
+	     as missing qualifiers or int vs long when they're the same
+	     size.  However, with -Wextra in effect, diagnose return and
+	     argument types that are incompatible according to language
+	     rules.  */
 	  tree mismatch_expect;
 	  unsigned mismatch_argno;
 
@@ -1999,16 +2005,25 @@  diagnose_mismatched_decls (tree newdecl, tree olddecl,
 	      /* If types match only loosely, print a warning but accept
 		 the redeclaration.  */
 	      location_t newloc = DECL_SOURCE_LOCATION (newdecl);
+	      bool warned = false;
 	      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);
+		warned = 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);
+		warned = warning_at (newloc, OPT_Wbuiltin_declaration_mismatch,
+				     "mismatch in return type of built-in "
+				     "function %qD; expected %qT",
+				     newdecl, mismatch_expect);
+	      const char *header = header_for_builtin_fn (olddecl);
+	      if (warned && header)
+		{
+		  rich_location richloc (line_table, newloc);
+		  maybe_add_include_fixit (&richloc, header, true);
+		  inform (&richloc,
+			  "%qD is declared in header %qs", olddecl, header);
+		}
 	    }
 	}
       else if (TREE_CODE (olddecl) == FUNCTION_DECL
diff --git a/gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-13.c b/gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-13.c
new file mode 100644
index 00000000000..f21f407e181
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wbuiltin-declaration-mismatch-13.c
@@ -0,0 +1,78 @@ 
+/* PR middle-end/93926 - ICE on a built-in redeclaration returning an integer
+   instead of a pointer
+   { dg-do compile }
+   { dg-options "-Wall" } */
+
+typedef __SIZE_TYPE__ size_t;
+
+void* ret_calloc (size_t n1, size_t n2)
+{
+  extern size_t calloc (size_t, size_t);    // { dg-warning "\\\[-Wbuiltin-declaration-mismatch" }
+
+  return (void *) calloc (n1, n2);
+}
+
+void* ret_malloc (size_t n)
+{
+  extern size_t malloc (size_t);            // { dg-warning "\\\[-Wbuiltin-declaration-mismatch" }
+
+  return (void *) malloc (n);
+}
+
+void* ret_realloc (void *p, size_t n)
+{
+  extern size_t realloc (void *p, size_t);  // { dg-warning "\\\[-Wbuiltin-declaration-mismatch" }
+
+  return (void *) realloc (p, n);
+}
+
+void* ret_strdup (const char *s)
+{
+  extern size_t strdup (const char*);       // { dg-warning "\\\[-Wbuiltin-declaration-mismatch" }
+
+  return (void *) strdup (s);
+}
+
+void* ret_strndup (const char *s, size_t n)
+{
+  extern size_t
+    strndup (const char*, size_t);          // { dg-warning "\\\[-Wbuiltin-declaration-mismatch" }
+
+  return (void *) strndup (s, n);
+}
+
+// For good measure also exerise strcmp return type (not part of the bug).
+
+char* ret_strcmp (const char *s, const char *t)
+{
+  extern char*
+    strcmp (const char*, const char*);      // { dg-warning "\\\[-Wbuiltin-declaration-mismatch" }
+
+  return strcmp (s, t);
+}
+
+// Exercise warnings for pointer/integer mismatches in argument types
+// (also not part of the bug).
+
+char* ret_strcat (size_t s, const char *t)
+{
+  extern char*
+    strcat (size_t, const char*);           // { dg-warning "\\\[-Wbuiltin-declaration-mismatch" }
+
+  return strcat (s, t);
+}
+
+char* ret_strcpy (char *s, size_t t)
+{
+  extern char* strcpy (char*, size_t);      // { dg-warning "\\\[-Wbuiltin-declaration-mismatch" }
+
+  return strcpy (s, t);
+}
+
+char* ret_strncpy (char *s, const char *t, size_t n)
+{
+  extern char*
+    strncpy (char*, size_t, const char*);   // { dg-warning "\\\[-Wbuiltin-declaration-mismatch" }
+
+  return strncpy (s, n, t);
+}