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, C++] Warn on redefinition of builtin functions (PR c++/71973)


On 11/03/16 20:58, Jason Merrill wrote:
> On Wed, Nov 2, 2016 at 5:15 PM, Bernd Edlinger
> <bernd.edlinger@hotmail.de> wrote:
>> Yes, I am inclined to enable the warning by default now.
>>
>> Most of the test cases are fixable in a fairly obvious way,
>> see attachment.
>>

Most test cases are fixed now.
I have added -w to the remaining test cases.
Because the wrong declarations seem to be done on purpose.

>> But I am unsure about g++.old-deja/g++.other/builtins10.C:
>>
>> // { dg-do assemble  }
>> // Test that built-in functions don't warn when prototyped without
>> arguments.
>> // Origin: PR c++/9367
>> // Copyright (C) 2003 Free Software Foundation.
>>
>> extern "C" int snprintf();
>>
>>
>> PR c++/9367 was a *bogus* warning.  Either delete the test case, or
>> change the expectation to the new warning?
>
> I think change the expectation to the new warning.  I think we're far
> enough away from the K&R days that we don't need to allow that sort of
> thing anymore.
>

Done.  The snprintf warning did not come with -std=c++98, but that
is probably OK, because -std=c++98 implies not C99.

>> But the libitm warnings are a real bug, the prototypes of the _ITM_
>> builtin functions and in the libitm.h simply do not match, that was
>> just not visible before because the test suite does apparently
>> not use -Wall.
>
> Oops!
>

I think I can solve that.

The libitm.h header file is not part of a public API because it is
not installed, and only used when building libitm itself, and it is
only included on a few test cases.  There is no warning when building
libitm, because the builtins are only enabled when -fgnu-tm is used.
The warning is seen only in C++ test cases when -fgnu-tm is used.

The builtins seem to be used mostly to be able to call these library
functions from the middle-end.

Therefore it is not necessary to define both, the __builtin__ITM_ and
the _ITM_ overloads.  Thus by the change to the DEF_TM_BUILTIN
only the __builtin__ITM_ gets defined, which made the libitm test
suite pass again.

The parameters of __builtin__ITM_ are still bogus, see PR 78202.
But that does not cause warnings any more.

That means we do not have any checks, if the __builtin__ and the
real function match at all, but in the case of _ITM_ the difference
should not be a real problem.


So please find attached the new version of my patch.

Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.

Attachment: changelog-pr71973.txt
Description: changelog-pr71973.txt

Index: gcc/builtin-types.def
===================================================================
--- gcc/builtin-types.def	(revision 241846)
+++ gcc/builtin-types.def	(working copy)
@@ -103,6 +103,7 @@ DEF_PRIMITIVE_TYPE (BT_COMPLEX_LONGDOUBLE, complex
 
 DEF_PRIMITIVE_TYPE (BT_PTR, ptr_type_node)
 DEF_PRIMITIVE_TYPE (BT_FILEPTR, fileptr_type_node)
+DEF_PRIMITIVE_TYPE (BT_CONST_TM_PTR, const_tm_ptr_type_node)
 DEF_PRIMITIVE_TYPE (BT_CONST_PTR, const_ptr_type_node)
 DEF_PRIMITIVE_TYPE (BT_VOLATILE_PTR,
 		    build_pointer_type
@@ -146,7 +147,12 @@ DEF_PRIMITIVE_TYPE (BT_I16, builtin_type_for_size
 
 DEF_PRIMITIVE_TYPE (BT_BND, pointer_bounds_type_node)
 
-DEF_POINTER_TYPE (BT_PTR_CONST_STRING, BT_CONST_STRING)
+/* The C type `char * const *'.  */
+DEF_PRIMITIVE_TYPE (BT_PTR_CONST_STRING,
+		    build_pointer_type
+		     (build_qualified_type (string_type_node,
+					    TYPE_QUAL_CONST)))
+
 DEF_POINTER_TYPE (BT_PTR_UINT, BT_UINT)
 DEF_POINTER_TYPE (BT_PTR_LONG, BT_LONG)
 DEF_POINTER_TYPE (BT_PTR_ULONG, BT_ULONG)
@@ -511,8 +517,8 @@ DEF_FUNCTION_TYPE_4 (BT_FN_SIZE_CONST_PTR_SIZE_SIZ
 		     BT_SIZE, BT_CONST_PTR, BT_SIZE, BT_SIZE, BT_FILEPTR)
 DEF_FUNCTION_TYPE_4 (BT_FN_INT_STRING_SIZE_CONST_STRING_VALIST_ARG,
 		BT_INT, BT_STRING, BT_SIZE, BT_CONST_STRING, BT_VALIST_ARG)
-DEF_FUNCTION_TYPE_4 (BT_FN_SIZE_STRING_SIZE_CONST_STRING_CONST_PTR,
-		BT_SIZE, BT_STRING, BT_SIZE, BT_CONST_STRING, BT_CONST_PTR)
+DEF_FUNCTION_TYPE_4 (BT_FN_SIZE_STRING_SIZE_CONST_STRING_CONST_TM_PTR,
+		BT_SIZE, BT_STRING, BT_SIZE, BT_CONST_STRING, BT_CONST_TM_PTR)
 DEF_FUNCTION_TYPE_4 (BT_FN_PTR_PTR_CONST_PTR_SIZE_SIZE,
 		     BT_PTR, BT_PTR, BT_CONST_PTR, BT_SIZE, BT_SIZE)
 DEF_FUNCTION_TYPE_4 (BT_FN_PTR_PTR_INT_SIZE_SIZE,
Index: gcc/builtins.def
===================================================================
--- gcc/builtins.def	(revision 241846)
+++ gcc/builtins.def	(working copy)
@@ -212,8 +212,8 @@ along with GCC; see the file COPYING3.  If not see
    functions are mapped to the actual implementation of the STM library. */
 #undef DEF_TM_BUILTIN
 #define DEF_TM_BUILTIN(ENUM, NAME, TYPE, ATTRS) \
-  DEF_BUILTIN (ENUM, "__builtin_" NAME, BUILT_IN_NORMAL, TYPE, TYPE,    \
-	       true, true, true, ATTRS, false, flag_tm)
+  DEF_BUILTIN (ENUM, "__builtin_" NAME, BUILT_IN_NORMAL, TYPE, BT_LAST, \
+	       false, true, true, ATTRS, false, flag_tm)
 
 /* Builtin used by the implementation of libsanitizer. These
    functions are mapped to the actual implementation of the 
@@ -866,7 +866,7 @@ DEF_GCC_BUILTIN        (BUILT_IN_RETURN_ADDRESS, "
 DEF_GCC_BUILTIN        (BUILT_IN_SAVEREGS, "saveregs", BT_FN_PTR_VAR, ATTR_NULL)
 DEF_GCC_BUILTIN        (BUILT_IN_SETJMP, "setjmp", BT_FN_INT_PTR, ATTR_RT_NOTHROW_LEAF_LIST)
 DEF_EXT_LIB_BUILTIN    (BUILT_IN_STRFMON, "strfmon", BT_FN_SSIZE_STRING_SIZE_CONST_STRING_VAR, ATTR_FORMAT_STRFMON_NOTHROW_3_4)
-DEF_LIB_BUILTIN        (BUILT_IN_STRFTIME, "strftime", BT_FN_SIZE_STRING_SIZE_CONST_STRING_CONST_PTR, ATTR_FORMAT_STRFTIME_NOTHROW_3_0)
+DEF_LIB_BUILTIN        (BUILT_IN_STRFTIME, "strftime", BT_FN_SIZE_STRING_SIZE_CONST_STRING_CONST_TM_PTR, ATTR_FORMAT_STRFTIME_NOTHROW_3_0)
 DEF_GCC_BUILTIN        (BUILT_IN_TRAP, "trap", BT_FN_VOID, ATTR_NORETURN_NOTHROW_LEAF_LIST)
 DEF_GCC_BUILTIN        (BUILT_IN_UNREACHABLE, "unreachable", BT_FN_VOID, ATTR_CONST_NORETURN_NOTHROW_LEAF_LIST)
 DEF_GCC_BUILTIN        (BUILT_IN_UNWIND_INIT, "unwind_init", BT_FN_VOID, ATTR_NULL)
Index: gcc/c-family/c-common.c
===================================================================
--- gcc/c-family/c-common.c	(revision 241846)
+++ gcc/c-family/c-common.c	(working copy)
@@ -4291,9 +4291,13 @@ c_common_nodes_and_builtins (void)
 	}
 
   if (c_dialect_cxx ())
-    /* For C++, make fileptr_type_node a distinct void * type until
-       FILE type is defined.  */
-    fileptr_type_node = build_variant_type_copy (ptr_type_node);
+    {
+      /* For C++, make fileptr_type_node a distinct void * type until
+	 FILE type is defined.  */
+      fileptr_type_node = build_variant_type_copy (ptr_type_node);
+      /* Likewise for const struct tm*.  */
+      const_tm_ptr_type_node = build_variant_type_copy (const_ptr_type_node);
+    }
 
   record_builtin_type (RID_VOID, NULL, void_type_node);
 
Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c	(revision 241846)
+++ gcc/cp/decl.c	(working copy)
@@ -1489,10 +1489,15 @@ duplicate_decls (tree newdecl, tree olddecl, bool
 	     explicitly declared.  */
 	  if (DECL_ANTICIPATED (olddecl))
 	    {
-	      /* Deal with fileptr_type_node.  FILE type is not known
-		 at the time we create the builtins.  */
 	      tree t1, t2;
 
+	      /* A new declaration doesn't match a built-in one unless it
+		 is also extern "C".  */
+	      gcc_assert (DECL_IS_BUILTIN (olddecl));
+	      gcc_assert (DECL_EXTERN_C_P (olddecl));
+	      if (!DECL_EXTERN_C_P (newdecl))
+		return NULL_TREE;
+
 	      for (t1 = TYPE_ARG_TYPES (TREE_TYPE (newdecl)),
 		   t2 = TYPE_ARG_TYPES (TREE_TYPE (olddecl));
 		   t1 || t2;
@@ -1499,6 +1504,8 @@ duplicate_decls (tree newdecl, tree olddecl, bool
 		   t1 = TREE_CHAIN (t1), t2 = TREE_CHAIN (t2))
 		if (!t1 || !t2)
 		  break;
+	        /* Deal with fileptr_type_node.  FILE type is not known
+		   at the time we create the builtins.  */
 		else if (TREE_VALUE (t2) == fileptr_type_node)
 		  {
 		    tree t = TREE_VALUE (t1);
@@ -1519,8 +1526,33 @@ duplicate_decls (tree newdecl, tree olddecl, bool
 			TYPE_ARG_TYPES (TREE_TYPE (olddecl)) = oldargs;
 		      }
 		  }
+		/* Likewise for const struct tm*.  */
+		else if (TREE_VALUE (t2) == const_tm_ptr_type_node)
+		  {
+		    tree t = TREE_VALUE (t1);
+
+		    if (TYPE_PTR_P (t)
+			&& TYPE_IDENTIFIER (TREE_TYPE (t))
+			   == get_identifier ("tm")
+			&& compparms (TREE_CHAIN (t1), TREE_CHAIN (t2)))
+		      {
+			tree oldargs = TYPE_ARG_TYPES (TREE_TYPE (olddecl));
+
+			TYPE_ARG_TYPES (TREE_TYPE (olddecl))
+			  = TYPE_ARG_TYPES (TREE_TYPE (newdecl));
+			types_match = decls_match (newdecl, olddecl);
+			if (types_match)
+			  return duplicate_decls (newdecl, olddecl,
+						  newdecl_is_friend);
+			TYPE_ARG_TYPES (TREE_TYPE (olddecl)) = oldargs;
+		      }
+		  }
 		else if (! same_type_p (TREE_VALUE (t1), TREE_VALUE (t2)))
 		  break;
+
+	      warning_at (DECL_SOURCE_LOCATION (newdecl), 0,
+			  "declaration of %q+#D conflicts with built-in "
+			  "declaration %q#D", newdecl, olddecl);
 	    }
 	  else if ((DECL_EXTERN_C_P (newdecl)
 		    && DECL_EXTERN_C_P (olddecl))
@@ -1574,7 +1606,7 @@ duplicate_decls (tree newdecl, tree olddecl, bool
 
       /* Whether or not the builtin can throw exceptions has no
 	 bearing on this declarator.  */
-      TREE_NOTHROW (olddecl) = 0;
+      TREE_NOTHROW (olddecl) = TREE_NOTHROW (newdecl);
 
       if (DECL_THIS_STATIC (newdecl) && !DECL_THIS_STATIC (olddecl))
 	{
Index: gcc/lto/lto-lang.c
===================================================================
--- gcc/lto/lto-lang.c	(revision 241846)
+++ gcc/lto/lto-lang.c	(working copy)
@@ -1266,6 +1266,10 @@ lto_init (void)
      always use the C definition here in lto1.  */
   gcc_assert (fileptr_type_node == ptr_type_node);
   gcc_assert (TYPE_MAIN_VARIANT (fileptr_type_node) == ptr_type_node);
+  /* Likewise for const struct tm*.  */
+  gcc_assert (const_tm_ptr_type_node == const_ptr_type_node);
+  gcc_assert (TYPE_MAIN_VARIANT (const_tm_ptr_type_node)
+	      == const_ptr_type_node);
 
   ptrdiff_type_node = integer_type_node;
 
Index: gcc/testsuite/g++.dg/lookup/extern-c-redecl4.C
===================================================================
--- gcc/testsuite/g++.dg/lookup/extern-c-redecl4.C	(revision 241846)
+++ gcc/testsuite/g++.dg/lookup/extern-c-redecl4.C	(working copy)
@@ -3,7 +3,6 @@
 
 // { dg-options "" }
 // { dg-do compile }
-// { dg-final { scan-assembler "call\[\t \]+\[^\$\]*?_Z4forkv" { target i?86-*-* x86_64-*-* } } }
 
 class frok
 {
@@ -14,5 +13,5 @@ class frok
 void
 foo ()
 {
-  fork ();
+  fork (); // { dg-error "was not declared in this scope" }
 }
Index: gcc/testsuite/g++.dg/lto/pr68811_0.C
===================================================================
--- gcc/testsuite/g++.dg/lto/pr68811_0.C	(revision 241846)
+++ gcc/testsuite/g++.dg/lto/pr68811_0.C	(working copy)
@@ -1,5 +1,5 @@
 // { dg-lto-do link }
-/* { dg-lto-options "-O2  -w" } */
+/* { dg-lto-options { { -O2 -w } { -w } } } */
 // { dg-extra-ld-options "-r -nostdlib" }
 extern "C" char *strcpy(char *, const char *);
 char InitXPCOMGlue_lastSlash;
Index: gcc/testsuite/g++.dg/pr71973-1.C
===================================================================
--- gcc/testsuite/g++.dg/pr71973-1.C	(revision 0)
+++ gcc/testsuite/g++.dg/pr71973-1.C	(working copy)
@@ -0,0 +1,14 @@
+// { dg-do compile }
+// { dg-options "-Wall -fdump-tree-eh" }
+
+extern "C"
+void fork () // { dg-warning "conflicts with built-in declaration" }
+__attribute__ ((__nothrow__));
+
+void foo () throw ()
+{
+  fork ();
+}
+
+// { dg-final { scan-tree-dump-not "eh_dispatch" "eh" } }
+// { dg-final { scan-tree-dump-not "resx" "eh" } }
Index: gcc/testsuite/g++.dg/pr71973-2.C
===================================================================
--- gcc/testsuite/g++.dg/pr71973-2.C	(revision 0)
+++ gcc/testsuite/g++.dg/pr71973-2.C	(working copy)
@@ -0,0 +1,18 @@
+// { dg-do compile }
+// { dg-options "-Wall -fdump-tree-eh" }
+
+typedef __SIZE_TYPE__ size_t;
+struct tm;
+
+extern "C"
+size_t strftime (char*, size_t, const char*, const struct tm*)
+__attribute__ ((__nothrow__));
+
+void foo () throw ()
+{
+  strftime (0,0,0,0); // { dg-warning "null argument where non-null required" }
+  // { dg-warning "too many arguments for format" "" { target *-*-* } .-1 }
+}
+
+// { dg-final { scan-tree-dump-not "eh_dispatch" "eh" } }
+// { dg-final { scan-tree-dump-not "resx" "eh" } }
Index: gcc/testsuite/g++.dg/pr71973-3.C
===================================================================
--- gcc/testsuite/g++.dg/pr71973-3.C	(revision 0)
+++ gcc/testsuite/g++.dg/pr71973-3.C	(working copy)
@@ -0,0 +1,14 @@
+// { dg-do compile }
+// { dg-options "-Wall -fdump-tree-eh" }
+
+extern "C"
+int execve (const char *__path, char *const __argv[], char *const __envp[])
+__attribute__ ((__nothrow__));
+
+void foo () throw ()
+{
+  execve (0,0,0);
+}
+
+// { dg-final { scan-tree-dump-not "eh_dispatch" "eh" } }
+// { dg-final { scan-tree-dump-not "resx" "eh" } }
Index: gcc/testsuite/g++.old-deja/g++.mike/p700.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.mike/p700.C	(revision 241846)
+++ gcc/testsuite/g++.old-deja/g++.mike/p700.C	(working copy)
@@ -1,5 +1,5 @@
 // { dg-do assemble  }
-// { dg-options "-Wno-deprecated -Wno-register" }
+// { dg-options "-w" }
 // { dg-error "limited range of data type" "16-bit target" { target xstormy16-*-* } 0 }
 // prms-id: 700
 
Index: gcc/testsuite/g++.old-deja/g++.other/builtins10.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.other/builtins10.C	(revision 241846)
+++ gcc/testsuite/g++.old-deja/g++.other/builtins10.C	(working copy)
@@ -1,7 +1,8 @@
 // { dg-do assemble  }
-// Test that built-in functions don't warn when prototyped without arguments.
+// Test that built-in functions do warn when prototyped without arguments.
 // Origin: PR c++/9367
 // Copyright (C) 2003 Free Software Foundation.
 
-extern "C" int snprintf();
+extern "C" int snprintf(); // { dg-warning "conflicts with built-in declaration" "" { target c++11 } }
+extern "C" int printf(); // { dg-warning "conflicts with built-in declaration" }
 
Index: gcc/testsuite/g++.old-deja/g++.other/realloc.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.other/realloc.C	(revision 241846)
+++ gcc/testsuite/g++.old-deja/g++.other/realloc.C	(working copy)
@@ -1,4 +1,5 @@
 // { dg-do assemble  }
+// { dg-options "-w" }
 
 extern "C" void realloc();
 
Index: gcc/tree-core.h
===================================================================
--- gcc/tree-core.h	(revision 241846)
+++ gcc/tree-core.h	(working copy)
@@ -618,6 +618,7 @@ enum tree_index {
   TI_VA_LIST_FPR_COUNTER_FIELD,
   TI_BOOLEAN_TYPE,
   TI_FILEPTR_TYPE,
+  TI_CONST_TM_PTR_TYPE,
   TI_POINTER_SIZED_TYPE,
 
   TI_POINTER_BOUNDS_TYPE,
Index: gcc/tree.c
===================================================================
--- gcc/tree.c	(revision 241846)
+++ gcc/tree.c	(working copy)
@@ -6006,6 +6006,7 @@ free_lang_data (void)
   /* Create gimple variants for common types.  */
   ptrdiff_type_node = integer_type_node;
   fileptr_type_node = ptr_type_node;
+  const_tm_ptr_type_node = const_ptr_type_node;
 
   /* Reset some langhooks.  Do not reset types_compatible_p, it may
      still be used indirectly via the get_alias_set langhook.  */
@@ -10332,6 +10333,7 @@ build_common_tree_nodes (bool signed_char)
   const_ptr_type_node
     = build_pointer_type (build_type_variant (void_type_node, 1, 0));
   fileptr_type_node = ptr_type_node;
+  const_tm_ptr_type_node = const_ptr_type_node;
 
   pointer_sized_int_node = build_nonstandard_integer_type (POINTER_SIZE, 1);
 
Index: gcc/tree.h
===================================================================
--- gcc/tree.h	(revision 241846)
+++ gcc/tree.h	(working copy)
@@ -3671,6 +3671,8 @@ tree_operand_check_code (const_tree __t, enum tree
 #define va_list_fpr_counter_field	global_trees[TI_VA_LIST_FPR_COUNTER_FIELD]
 /* The C type `FILE *'.  */
 #define fileptr_type_node		global_trees[TI_FILEPTR_TYPE]
+/* The C type `const struct tm *'.  */
+#define const_tm_ptr_type_node		global_trees[TI_CONST_TM_PTR_TYPE]
 #define pointer_sized_int_node		global_trees[TI_POINTER_SIZED_TYPE]
 
 #define boolean_type_node		global_trees[TI_BOOLEAN_TYPE]

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