[PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Bernd Edlinger]

[-- Attachment #1: Type: text/plain, Size: 3496 bytes --]

Hi!

Currently C++ does not warn at all when built-in functions are
re-defined with a different signature, while C does warn on that
even without -Wall.

Thus I'd like to propose a -Wall enabled warning for that in C++ only.

Initially I tried to warn unconditionally but that made too  many tests
in the C++ testsuite emit that warning :-(

So making the warning dependent on -Wall is a compromise due
to the very many compile only tests, that use this "feature".

There is also a wrong-code side on this redefinition, because
even if the new function has the nothrow attribute the code is
generated as if it could throw.  Fixed as well.

This is an updated version of the patch that was posted
here: https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01463.html
... but got no response so far.

I have seen a few warnings in system headers, when -Wsystem-headers
is used, and fixed most of them, for instance strftime got a
redefinition warning, because the const struct tm* parameter has to be
handled as the FILE* in other builtin functions.

As a little surprise, it turned out, there were warnings missing
from strftime in C due to the type conflict when merging the builtin
with the actual declaration, see the format warnings in the test case
testsuite/g++.dg/pr71973-2.C, which were not working previously in C++.

Then there are cases, where a builtin function and C++ overloads
have the same name like the abs function from cstdlib.  Due to
missing strictness in duplicate_decls the builtin got merged with
one of the C++ overloads.  That was visible due to the new warning.

I believe a builtin function always needs an extern "C" declaration.
Additional C++ overloads are possible, but do not redefine the builtin
function, and create additional overloads.

However these warnings remain, and I have no idea if the header
file is correct or the warning:

/usr/include/unistd.h:551:12: warning: declaration of 'int execve(const 
char*, char* const*, char* const*)' conflicts with built-in declaration 
'int execve(const char*, const char**, const char**)' 
[-Wbuiltin-function-redefined]
  extern int execve (const char *__path, char *const __argv[],
             ^~~~~~
/usr/include/unistd.h:563:12: warning: declaration of 'int execv(const 
char*, char* const*)' conflicts with built-in declaration 'int 
execv(const char*, const char**)' [-Wbuiltin-function-redefined]
  extern int execv (const char *__path, char *const __argv[])
             ^~~~~
/usr/include/unistd.h:578:12: warning: declaration of 'int execvp(const 
char*, char* const*)' conflicts with built-in declaration 'int 
execvp(const char*, const char**)' [-Wbuiltin-function-redefined]
  extern int execvp (const char *__file, char *const __argv[])
             ^~~~~~

Interesting is that, these do not happen with -std=c++03/11/14 but only
with -std=gnu++03/11/14.  I could not figure out how to resolve that.

As said, this is only visible with -Wsystem-headers, and would not
happen, if the declaration in unistd.h would match the builtin function
prototype.


The reg-test also revealed a test case that is IMO invalid, and
depends on the incorrect merging of a builtin definition with a friend
function see testsuite/g++.dg/lookup/extern-c-redecl4.C:
This test case does no longer compile, because the builtin
fails to have an explicit declaration.


Bootstrap and reg-testing on x86_64-pc-linux-gnu.
OK for trunk?


Thanks
Bernd.

[-- Attachment #2: changelog-pr71973.txt --]
[-- Type: text/plain, Size: 1454 bytes --]

gcc:
2016-10-06  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c++/71973
	* doc/invoke.texi: Document -Wbuiltin-function-redefined.
	* builtin-types.def (BT_CONST_TM_PTR): New primitive type.
	(BT_FN_SIZE_STRING_SIZE_CONST_STRING_CONST_PTR): Removed.
	(BT_FN_SIZE_STRING_SIZE_CONST_STRING_CONST_TM_PTR): New function type.
	* builtins.def (strftime): Update builtin function.
	* tree-core.h (TI_CONST_TM_PTR_TYPE): New enum value.
	* tree.h (const_tm_ptr_type_node): New type node.
	* tree.c (free_lang_data, build_common_tree_nodes): Initialize
	const_tm_ptr_type_node.

c-family:
2016-10-06  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c++/71973
	* c.opt (Wbuiltin-function-redefined): New warning.
	* c-common.c (c_common_nodes_and_builtins): Initialize
	const_tm_ptr_type_node.

cp:
2016-10-06  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c++/71973
	* decl.c (duplicate_decls): Warn when a built-in function is redefined.
	Don't overload builtin functions with C++ functions.
	Handle const_tm_ptr_type_node like file_ptr_node.
	Copy the TREE_NOTHROW flag unmodified to the old decl.

lto:
2016-10-06  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c++/71973
	* lto-lang.c (lto_init): Assert const_tm_ptr_type_node is sane.

testsuite:
2016-10-06  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c++/71973
	* g++.dg/pr71973-1.C: New test.
	* g++.dg/pr71973-2.C: New test.
	* g++.dg/lookup/extern-c-redecl4.C: Adjust test expectations.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: patch-pr71973.diff --]
[-- Type: text/x-patch; name="patch-pr71973.diff", Size: 13044 bytes --]

Index: gcc/builtin-types.def
===================================================================
--- gcc/builtin-types.def	(revision 240799)
+++ 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
@@ -511,8 +512,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 240799)
+++ gcc/builtins.def	(working copy)
@@ -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 240799)
+++ gcc/c-family/c-common.c	(working copy)
@@ -5619,9 +5619,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/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt	(revision 240799)
+++ gcc/c-family/c.opt	(working copy)
@@ -323,6 +323,10 @@ Wframe-address
 C ObjC C++ ObjC++ Var(warn_frame_address) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
 Warn when __builtin_frame_address or __builtin_return_address is used unsafely.
 
+Wbuiltin-function-redefined
+C++ ObjC++ Var(warn_builtin_function_redefined) Warning LangEnabledBy(C++ ObjC++,Wall)
+Warn when a built-in function is redefined.
+
 Wbuiltin-macro-redefined
 C ObjC C++ ObjC++ CPP(warn_builtin_macro_redefined) CppReason(CPP_W_BUILTIN_MACRO_REDEFINED) Var(cpp_warn_builtin_macro_redefined) Init(1) Warning
 Warn when a built-in preprocessor macro is undefined or redefined.
Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c	(revision 240799)
+++ gcc/cp/decl.c	(working copy)
@@ -1468,10 +1468,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;
@@ -1478,6 +1483,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);
@@ -1498,8 +1505,36 @@ 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;
+	      if (t1 || t2
+		  || ! same_type_p (TREE_TYPE (TREE_TYPE (olddecl)),
+				    TREE_TYPE (TREE_TYPE (newdecl))))
+		warning_at (DECL_SOURCE_LOCATION (newdecl),
+			    OPT_Wbuiltin_function_redefined,
+			    "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))
@@ -1553,7 +1588,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/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 240799)
+++ gcc/doc/invoke.texi	(working copy)
@@ -256,7 +256,7 @@ Objective-C and Objective-C++ Dialects}.
 -pedantic-errors @gol
 -w  -Wextra  -Wall  -Waddress  -Waggregate-return  @gol
 -Wno-aggressive-loop-optimizations -Warray-bounds -Warray-bounds=@var{n} @gol
--Wno-attributes -Wbool-compare -Wbool-operation @gol
+-Wno-attributes -Wbool-compare -Wbool-operation -Wbuiltin-function-redefined @gol
 -Wno-builtin-macro-redefined -Wc90-c99-compat -Wc99-c11-compat @gol
 -Wc++-compat -Wc++11-compat -Wc++14-compat -Wcast-align  -Wcast-qual  @gol
 -Wchar-subscripts -Wclobbered  -Wcomment -Wconditionally-supported  @gol
@@ -3668,6 +3668,7 @@ Options} and @ref{Objective-C and Objective-C++ Di
 -Warray-bounds=1 @r{(only with} @option{-O2}@r{)}  @gol
 -Wbool-compare  @gol
 -Wbool-operation  @gol
+-Wbuiltin-function-redefined @r{(C++ and Objective-C++ only)} @gol
 -Wc++11-compat  -Wc++14-compat@gol
 -Wchar-subscripts  @gol
 -Wcomment  @gol
@@ -5730,6 +5731,13 @@ unrecognized attributes, function attributes appli
 etc.  This does not stop errors for incorrect use of supported
 attributes.
 
+@item -Wbuiltin-function-redefined @r{(C++ and Objective-C++ only)}
+@opindex Wbuiltin-function-redefined
+@opindex Wno-builtin-function-redefined
+Do warn if built-in functions are redefined.  This option is only
+supported for C++ and Objective-C++.  It is implied by @option{-Wall},
+which can be disabled with @option{-Wno-builtin-function-redefined}.
+
 @item -Wno-builtin-macro-redefined
 @opindex Wno-builtin-macro-redefined
 @opindex Wbuiltin-macro-redefined
Index: gcc/lto/lto-lang.c
===================================================================
--- gcc/lto/lto-lang.c	(revision 240799)
+++ 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 240799)
+++ 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/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/tree-core.h
===================================================================
--- gcc/tree-core.h	(revision 240799)
+++ gcc/tree-core.h	(working copy)
@@ -612,6 +612,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 240799)
+++ gcc/tree.c	(working copy)
@@ -6007,6 +6007,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.  */
@@ -10311,6 +10312,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 240799)
+++ gcc/tree.h	(working copy)
@@ -3657,6 +3657,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]

Re: [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Kyrill Tkachov]

Hi Bernd,
Not familiar with this area but one comment below...

On 06/10/16 15:12, Bernd Edlinger wrote:
> Hi!
>
> Currently C++ does not warn at all when built-in functions are
> re-defined with a different signature, while C does warn on that
> even without -Wall.
>
> Thus I'd like to propose a -Wall enabled warning for that in C++ only.
>
> Initially I tried to warn unconditionally but that made too  many tests
> in the C++ testsuite emit that warning :-(
>
> So making the warning dependent on -Wall is a compromise due
> to the very many compile only tests, that use this "feature".
>
> There is also a wrong-code side on this redefinition, because
> even if the new function has the nothrow attribute the code is
> generated as if it could throw.  Fixed as well.
>
> This is an updated version of the patch that was posted
> here: https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01463.html
> ... but got no response so far.
>
> I have seen a few warnings in system headers, when -Wsystem-headers
> is used, and fixed most of them, for instance strftime got a
> redefinition warning, because the const struct tm* parameter has to be
> handled as the FILE* in other builtin functions.
>
> As a little surprise, it turned out, there were warnings missing
> from strftime in C due to the type conflict when merging the builtin
> with the actual declaration, see the format warnings in the test case
> testsuite/g++.dg/pr71973-2.C, which were not working previously in C++.
>
> Then there are cases, where a builtin function and C++ overloads
> have the same name like the abs function from cstdlib.  Due to
> missing strictness in duplicate_decls the builtin got merged with
> one of the C++ overloads.  That was visible due to the new warning.
>
> I believe a builtin function always needs an extern "C" declaration.
> Additional C++ overloads are possible, but do not redefine the builtin
> function, and create additional overloads.
>
> However these warnings remain, and I have no idea if the header
> file is correct or the warning:
>
> /usr/include/unistd.h:551:12: warning: declaration of 'int execve(const
> char*, char* const*, char* const*)' conflicts with built-in declaration
> 'int execve(const char*, const char**, const char**)'
> [-Wbuiltin-function-redefined]
>    extern int execve (const char *__path, char *const __argv[],
>               ^~~~~~
> /usr/include/unistd.h:563:12: warning: declaration of 'int execv(const
> char*, char* const*)' conflicts with built-in declaration 'int
> execv(const char*, const char**)' [-Wbuiltin-function-redefined]
>    extern int execv (const char *__path, char *const __argv[])
>               ^~~~~
> /usr/include/unistd.h:578:12: warning: declaration of 'int execvp(const
> char*, char* const*)' conflicts with built-in declaration 'int
> execvp(const char*, const char**)' [-Wbuiltin-function-redefined]
>    extern int execvp (const char *__file, char *const __argv[])
>               ^~~~~~
>
> Interesting is that, these do not happen with -std=c++03/11/14 but only
> with -std=gnu++03/11/14.  I could not figure out how to resolve that.
>
> As said, this is only visible with -Wsystem-headers, and would not
> happen, if the declaration in unistd.h would match the builtin function
> prototype.
>
>
> The reg-test also revealed a test case that is IMO invalid, and
> depends on the incorrect merging of a builtin definition with a friend
> function see testsuite/g++.dg/lookup/extern-c-redecl4.C:
> This test case does no longer compile, because the builtin
> fails to have an explicit declaration.
>
>
> Bootstrap and reg-testing on x86_64-pc-linux-gnu.
> OK for trunk?
>
>
> Thanks
> Bernd.

@@ -1553,7 +1588,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);

The comment would need to be updated I think.
Kyrill

Re: [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Bernd Edlinger]

On 10/06/16 16:14, Kyrill Tkachov wrote:
>
> @@ -1553,7 +1588,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);
>
> The comment would need to be updated I think.

Probably, yes.

Actually the code did *not* do what the comment said, and
effectively set the nothrow attribute to zero, thus
the eh handlers were emitted when not needed.

And IMHO the new code does now literally do what the comment
said.

At this point there follow 1000+ lines of code, in the same
function that merge olddecl into newdecl and back again.

The code is dependent on the types_match variable,
and in the end newdecl is free'd an olddecl returned.

At some places the code is impossible to understand:
Look for the memcpy :-/

I *think* the intention is to merge the attribute from the
builtin when the header file is not explicitly giving,
some or all attributes, when the parameters match.

But when the parameters do not match, the header file
changes the builtin's signature, and overrides the
builtin attributes more or less with defaults or
whatever is in the header file.


Bernd.

Re: [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Bernd Edlinger]

On 10/06/16 22:37, Bernd Edlinger wrote:
> On 10/06/16 16:14, Kyrill Tkachov wrote:
>>
>> @@ -1553,7 +1588,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);
>>
>> The comment would need to be updated I think.
>
> Probably, yes.
>
> Actually the code did *not* do what the comment said, and
> effectively set the nothrow attribute to zero, thus
> the eh handlers were emitted when not needed.
>
> And IMHO the new code does now literally do what the comment
> said.
>
> At this point there follow 1000+ lines of code, in the same
> function that merge olddecl into newdecl and back again.
>
> The code is dependent on the types_match variable,
> and in the end newdecl is free'd an olddecl returned.
>
> At some places the code is impossible to understand:
> Look for the memcpy :-/
>
> I *think* the intention is to merge the attribute from the
> builtin when the header file is not explicitly giving,
> some or all attributes, when the parameters match.
>
> But when the parameters do not match, the header file
> changes the builtin's signature, and overrides the
> builtin attributes more or less with defaults or
> whatever is in the header file.
>
>


A few more thoughts, that may help to clarify a few things here.

Regarding this hunk:

  		else if (! same_type_p (TREE_VALUE (t1), TREE_VALUE (t2)))
  		  break;
+	      if (t1 || t2
+		  || ! same_type_p (TREE_TYPE (TREE_TYPE (olddecl)),
+				    TREE_TYPE (TREE_TYPE (newdecl))))
+		warning_at (DECL_SOURCE_LOCATION (newdecl),
+			    OPT_Wbuiltin_function_redefined,
+			    "declaration of %q+#D conflicts with built-in "
+			    "declaration %q#D", newdecl, olddecl);
  	    }
  	  else if ((DECL_EXTERN_C_P (newdecl)

meanwhile I start to think that the "if" here is unnecessary,
because if decls_match returns false, the declarations are certainly
different.  And the warning is thus already justified at this point.
Removing the if changes nothing, the condition is always satisfied.

Regarding this hunk:

        /* Whether or not the builtin can throw exceptions has no
          bearing on this declarator.  */
-      TREE_NOTHROW (olddecl) = 0;
+      TREE_NOTHROW (olddecl) = TREE_NOTHROW (newdecl);

You may ask, why the old code was working most of the time.
I think, usually, when types_match == true, there happens another
assignment to TREE_NOTHROW, later in that function around line 2183:

       /* Merge the type qualifiers.  */
       if (TREE_READONLY (newdecl))
         TREE_READONLY (olddecl) = 1;
       if (TREE_THIS_VOLATILE (newdecl))
         TREE_THIS_VOLATILE (olddecl) = 1;
       if (TREE_NOTHROW (newdecl))
         TREE_NOTHROW (olddecl) = 1;

This is in a big "if (types_match)", so I think that explains,
why the old code did work normally, and why it fails if the
parameter don't match, but I still have no idea what to say
in the comment, except that the code should exactly do what
the comment above says.


Bernd.

Re: [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Bernd Edlinger]

On 10/06/16 16:11, Bernd Edlinger wrote:
> Hi!
>
> Currently C++ does not warn at all when built-in functions are
> re-defined with a different signature, while C does warn on that
> even without -Wall.
>
> Thus I'd like to propose a -Wall enabled warning for that in C++ only.
>
> Initially I tried to warn unconditionally but that made too  many tests
> in the C++ testsuite emit that warning :-(
>
> So making the warning dependent on -Wall is a compromise due
> to the very many compile only tests, that use this "feature".
>
> There is also a wrong-code side on this redefinition, because
> even if the new function has the nothrow attribute the code is
> generated as if it could throw.  Fixed as well.
>
> This is an updated version of the patch that was posted
> here: https://gcc.gnu.org/ml/gcc-patches/2016-08/msg01463.html
> ... but got no response so far.
>
> I have seen a few warnings in system headers, when -Wsystem-headers
> is used, and fixed most of them, for instance strftime got a
> redefinition warning, because the const struct tm* parameter has to be
> handled as the FILE* in other builtin functions.
>
> As a little surprise, it turned out, there were warnings missing
> from strftime in C due to the type conflict when merging the builtin
> with the actual declaration, see the format warnings in the test case
> testsuite/g++.dg/pr71973-2.C, which were not working previously in C++.
>
> Then there are cases, where a builtin function and C++ overloads
> have the same name like the abs function from cstdlib.  Due to
> missing strictness in duplicate_decls the builtin got merged with
> one of the C++ overloads.  That was visible due to the new warning.
>
> I believe a builtin function always needs an extern "C" declaration.
> Additional C++ overloads are possible, but do not redefine the builtin
> function, and create additional overloads.
>
> However these warnings remain, and I have no idea if the header
> file is correct or the warning:
>
> /usr/include/unistd.h:551:12: warning: declaration of 'int execve(const
> char*, char* const*, char* const*)' conflicts with built-in declaration
> 'int execve(const char*, const char**, const char**)'
> [-Wbuiltin-function-redefined]
>  extern int execve (const char *__path, char *const __argv[],
>             ^~~~~~
> /usr/include/unistd.h:563:12: warning: declaration of 'int execv(const
> char*, char* const*)' conflicts with built-in declaration 'int
> execv(const char*, const char**)' [-Wbuiltin-function-redefined]
>  extern int execv (const char *__path, char *const __argv[])
>             ^~~~~
> /usr/include/unistd.h:578:12: warning: declaration of 'int execvp(const
> char*, char* const*)' conflicts with built-in declaration 'int
> execvp(const char*, const char**)' [-Wbuiltin-function-redefined]
>  extern int execvp (const char *__file, char *const __argv[])
>             ^~~~~~
>
> Interesting is that, these do not happen with -std=c++03/11/14 but only
> with -std=gnu++03/11/14.  I could not figure out how to resolve that.
>
> As said, this is only visible with -Wsystem-headers, and would not
> happen, if the declaration in unistd.h would match the builtin function
> prototype.
>

Just in case anybody wants to know, I digged into the warnings
about execv/e/p, and found the following:


First these builtins are declared with DEF_EXT_LIB_BUILTIN, and thus
the builtin function without __builtin_ in the name is only visible with
gnu extensions, therefore the warning does not happen with -std=c++XX
only with -std=gnu++XX.

Second, the declaration in the glibc header files simply look wrong,
because the type of argv, and envp is "char *const *" while the
builtin function wants "const char**", thus only the array of char*
itself is const, not the actual char stings they point to.

Third, in C the  builtins are not diagnosed, because C does only look
at the mode of the parameters see match_builtin_function_types in
c/c-decl.c, which may itself be wrong, because that makes an ABI
decision dependent on the mode of the parameter.

What was broken, because of that mismatch?

Except the missing warning about the redeclared builtin function, there
was also some hidden bug in the execv-builtins:

That is with -fprofile-arcs the execv/e/p is not replaced
with a call to __gcov_execv/e/p, and thus if a C++ program is
left through one of these functions the profile information is
not saved.

This is not fixed with the patch as is:

It would of course be fixed by changing the glibc headers, or if it
turns out that the GCC built-in is actually using the wrong prototype
that would also be possible to fix.

Any ideas which way to go on here?

Maybe a new fix-include rule?


Thanks
Bernd.

Re: [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Joseph Myers]

On Sun, 16 Oct 2016, Bernd Edlinger wrote:

> Second, the declaration in the glibc header files simply look wrong,
> because the type of argv, and envp is "char *const *" while the
> builtin function wants "const char**", thus only the array of char*
> itself is const, not the actual char stings they point to.

char *const * is the POSIX type.  (It can't be const char ** or const char 
*const * because you can't convert char ** implicitly to those types in 
ISO C.)  You'd need to check the list archives for rationale for the 
built-in functions doing something different.

> Third, in C the  builtins are not diagnosed, because C does only look
> at the mode of the parameters see match_builtin_function_types in
> c/c-decl.c, which may itself be wrong, because that makes an ABI
> decision dependent on the mode of the parameter.

The matching needs to be loose because of functions using types such as 
FILE * where the compiler doesn't know the exact contents of the type when 
processing built-in function definitions.  (Historically there were also 
issues with pre-ISO headers, but that may be less relevant now.)

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Bernd Edlinger]

[-- Attachment #1: Type: text/plain, Size: 2415 bytes --]

On 10/17/16 20:05, Joseph Myers wrote:
> On Sun, 16 Oct 2016, Bernd Edlinger wrote:
>
>> Second, the declaration in the glibc header files simply look wrong,
>> because the type of argv, and envp is "char *const *" while the
>> builtin function wants "const char**", thus only the array of char*
>> itself is const, not the actual char stings they point to.
>
> char *const * is the POSIX type.  (It can't be const char ** or const char
> *const * because you can't convert char ** implicitly to those types in
> ISO C.)  You'd need to check the list archives for rationale for the
> built-in functions doing something different.
>

Yes, that was discussed here:
https://gcc.gnu.org/ml/gcc-patches/2004-03/msg01148.html

No mention why the BT_PTR_CONST_TYPE does not match the posix type.
But the right types were used on __gcov_execv/e/p stubs, so the author
did know the right types at least.

So I think that was broken from the beginning, but that was hidden by
the loose checking in the C FE and not warning in the C++ FE when
prototypes don't match.

>> Third, in C the  builtins are not diagnosed, because C does only look
>> at the mode of the parameters see match_builtin_function_types in
>> c/c-decl.c, which may itself be wrong, because that makes an ABI
>> decision dependent on the mode of the parameter.
>
> The matching needs to be loose because of functions using types such as
> FILE * where the compiler doesn't know the exact contents of the type when
> processing built-in function definitions.  (Historically there were also
> issues with pre-ISO headers, but that may be less relevant now.)
>

The C++ FE has exactly the same problem with FILE* and struct tm*
but it solves it differently and "learns" the type but only for FILE*
and with this patch also for const struct tm*.  It is a lot more
restrictive than C, but that is because of the ++ ;)

Well in that case the posix functions have to use the prototypes
from POSIX.1.2008 although their rationale is a bit silly...

This updated patch fixes the prototype of execv/p/e,
and adds a new test case that checks that no type conflict
exists in the execve built-in any more.

Now we have no -Wsystem-headers warnings with glibc-headers any more.
And the gcov builtin also is working with C++.

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


Thanks
Bernd.

[-- Attachment #2: changelog-pr71973.txt --]
[-- Type: text/plain, Size: 1566 bytes --]

gcc:
2016-10-17  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c++/71973
	* doc/invoke.texi: Document -Wbuiltin-function-redefined.
	* builtin-types.def (BT_CONST_TM_PTR): New primitive type.
	(BT_PTR_CONST_STRING): Updated.
	(BT_FN_SIZE_STRING_SIZE_CONST_STRING_CONST_PTR): Removed.
	(BT_FN_SIZE_STRING_SIZE_CONST_STRING_CONST_TM_PTR): New function type.
	* builtins.def (strftime): Update builtin function.
	* tree-core.h (TI_CONST_TM_PTR_TYPE): New enum value.
	* tree.h (const_tm_ptr_type_node): New type node.
	* tree.c (free_lang_data, build_common_tree_nodes): Initialize
	const_tm_ptr_type_node.

c-family:
2016-10-17  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c++/71973
	* c.opt (Wbuiltin-function-redefined): New warning.
	* c-common.c (c_common_nodes_and_builtins): Initialize
	const_tm_ptr_type_node.

cp:
2016-10-17  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c++/71973
	* decl.c (duplicate_decls): Warn when a built-in function is redefined.
	Don't overload builtin functions with C++ functions.
	Handle const_tm_ptr_type_node like file_ptr_node.
	Copy the TREE_NOTHROW flag unmodified to the old decl.

lto:
2016-10-17  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c++/71973
	* lto-lang.c (lto_init): Assert const_tm_ptr_type_node is sane.

testsuite:
2016-10-17  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c++/71973
	* g++.dg/pr71973-1.C: New test.
	* g++.dg/pr71973-2.C: New test.
	* g++.dg/pr71973-3.C: New test.
	* g++.dg/lookup/extern-c-redecl4.C: Adjust test expectations.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: patch-pr71973.diff --]
[-- Type: text/x-patch; name="patch-pr71973.diff", Size: 14029 bytes --]

Index: gcc/builtin-types.def
===================================================================
--- gcc/builtin-types.def	(revision 241271)
+++ 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 241271)
+++ gcc/builtins.def	(working copy)
@@ -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 241271)
+++ gcc/c-family/c-common.c	(working copy)
@@ -4278,9 +4278,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/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt	(revision 241271)
+++ gcc/c-family/c.opt	(working copy)
@@ -323,6 +323,10 @@ Wframe-address
 C ObjC C++ ObjC++ Var(warn_frame_address) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
 Warn when __builtin_frame_address or __builtin_return_address is used unsafely.
 
+Wbuiltin-function-redefined
+C++ ObjC++ Var(warn_builtin_function_redefined) Warning LangEnabledBy(C++ ObjC++,Wall)
+Warn when a built-in function is redefined.
+
 Wbuiltin-macro-redefined
 C ObjC C++ ObjC++ CPP(warn_builtin_macro_redefined) CppReason(CPP_W_BUILTIN_MACRO_REDEFINED) Var(cpp_warn_builtin_macro_redefined) Init(1) Warning
 Warn when a built-in preprocessor macro is undefined or redefined.
Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c	(revision 241271)
+++ 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,34 @@ 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),
+			  OPT_Wbuiltin_function_redefined,
+			  "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 +1607,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/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 241271)
+++ gcc/doc/invoke.texi	(working copy)
@@ -256,7 +256,7 @@ Objective-C and Objective-C++ Dialects}.
 -pedantic-errors @gol
 -w  -Wextra  -Wall  -Waddress  -Waggregate-return  @gol
 -Wno-aggressive-loop-optimizations -Warray-bounds -Warray-bounds=@var{n} @gol
--Wno-attributes -Wbool-compare -Wbool-operation @gol
+-Wno-attributes -Wbool-compare -Wbool-operation -Wbuiltin-function-redefined @gol
 -Wno-builtin-macro-redefined -Wc90-c99-compat -Wc99-c11-compat @gol
 -Wc++-compat -Wc++11-compat -Wc++14-compat -Wcast-align  -Wcast-qual  @gol
 -Wchar-subscripts -Wclobbered  -Wcomment -Wconditionally-supported  @gol
@@ -3673,6 +3673,7 @@ Options} and @ref{Objective-C and Objective-C++ Di
 -Warray-bounds=1 @r{(only with} @option{-O2}@r{)}  @gol
 -Wbool-compare  @gol
 -Wbool-operation  @gol
+-Wbuiltin-function-redefined @r{(C++ and Objective-C++ only)} @gol
 -Wc++11-compat  -Wc++14-compat@gol
 -Wchar-subscripts  @gol
 -Wcomment  @gol
@@ -5793,6 +5794,13 @@ unrecognized attributes, function attributes appli
 etc.  This does not stop errors for incorrect use of supported
 attributes.
 
+@item -Wbuiltin-function-redefined @r{(C++ and Objective-C++ only)}
+@opindex Wbuiltin-function-redefined
+@opindex Wno-builtin-function-redefined
+Do warn if built-in functions are redefined.  This option is only
+supported for C++ and Objective-C++.  It is implied by @option{-Wall},
+which can be disabled with @option{-Wno-builtin-function-redefined}.
+
 @item -Wno-builtin-macro-redefined
 @opindex Wno-builtin-macro-redefined
 @opindex Wbuiltin-macro-redefined
Index: gcc/lto/lto-lang.c
===================================================================
--- gcc/lto/lto-lang.c	(revision 241271)
+++ 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 241271)
+++ 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/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/tree-core.h
===================================================================
--- gcc/tree-core.h	(revision 241271)
+++ gcc/tree-core.h	(working copy)
@@ -615,6 +615,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 241271)
+++ 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.  */
@@ -10310,6 +10311,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 241271)
+++ gcc/tree.h	(working copy)
@@ -3667,6 +3667,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]

[PING] [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Bernd Edlinger]

Hi!

I'd like to ping for my patch here:
https://gcc.gnu.org/ml/gcc-patches/2016-10/msg01348.html


Thanks
Bernd.

On 10/17/16 21:18, Bernd Edlinger wrote:
> On 10/17/16 20:05, Joseph Myers wrote:
>> On Sun, 16 Oct 2016, Bernd Edlinger wrote:
>>
>>> Second, the declaration in the glibc header files simply look wrong,
>>> because the type of argv, and envp is "char *const *" while the
>>> builtin function wants "const char**", thus only the array of char*
>>> itself is const, not the actual char stings they point to.
>>
>> char *const * is the POSIX type.  (It can't be const char ** or const
>> char
>> *const * because you can't convert char ** implicitly to those types in
>> ISO C.)  You'd need to check the list archives for rationale for the
>> built-in functions doing something different.
>>
>
> Yes, that was discussed here:
> https://gcc.gnu.org/ml/gcc-patches/2004-03/msg01148.html
>
> No mention why the BT_PTR_CONST_TYPE does not match the posix type.
> But the right types were used on __gcov_execv/e/p stubs, so the author
> did know the right types at least.
>
> So I think that was broken from the beginning, but that was hidden by
> the loose checking in the C FE and not warning in the C++ FE when
> prototypes don't match.
>
>>> Third, in C the  builtins are not diagnosed, because C does only look
>>> at the mode of the parameters see match_builtin_function_types in
>>> c/c-decl.c, which may itself be wrong, because that makes an ABI
>>> decision dependent on the mode of the parameter.
>>
>> The matching needs to be loose because of functions using types such as
>> FILE * where the compiler doesn't know the exact contents of the type
>> when
>> processing built-in function definitions.  (Historically there were also
>> issues with pre-ISO headers, but that may be less relevant now.)
>>
>
> The C++ FE has exactly the same problem with FILE* and struct tm*
> but it solves it differently and "learns" the type but only for FILE*
> and with this patch also for const struct tm*.  It is a lot more
> restrictive than C, but that is because of the ++ ;)
>
> Well in that case the posix functions have to use the prototypes
> from POSIX.1.2008 although their rationale is a bit silly...
>
> This updated patch fixes the prototype of execv/p/e,
> and adds a new test case that checks that no type conflict
> exists in the execve built-in any more.
>
> Now we have no -Wsystem-headers warnings with glibc-headers any more.
> And the gcov builtin also is working with C++.
>
> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?
>
>
> Thanks
> Bernd.

[PING**2] [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Bernd Edlinger]

Ping...

On 10/24/16 15:36, Bernd Edlinger wrote:
> Hi!
>
> I'd like to ping for my patch here:
> https://gcc.gnu.org/ml/gcc-patches/2016-10/msg01348.html
>
>
> Thanks
> Bernd.
>
> On 10/17/16 21:18, Bernd Edlinger wrote:
>> On 10/17/16 20:05, Joseph Myers wrote:
>>> On Sun, 16 Oct 2016, Bernd Edlinger wrote:
>>>
>>>> Second, the declaration in the glibc header files simply look wrong,
>>>> because the type of argv, and envp is "char *const *" while the
>>>> builtin function wants "const char**", thus only the array of char*
>>>> itself is const, not the actual char stings they point to.
>>>
>>> char *const * is the POSIX type.  (It can't be const char ** or const
>>> char
>>> *const * because you can't convert char ** implicitly to those types in
>>> ISO C.)  You'd need to check the list archives for rationale for the
>>> built-in functions doing something different.
>>>
>>
>> Yes, that was discussed here:
>> https://gcc.gnu.org/ml/gcc-patches/2004-03/msg01148.html
>>
>> No mention why the BT_PTR_CONST_TYPE does not match the posix type.
>> But the right types were used on __gcov_execv/e/p stubs, so the author
>> did know the right types at least.
>>
>> So I think that was broken from the beginning, but that was hidden by
>> the loose checking in the C FE and not warning in the C++ FE when
>> prototypes don't match.
>>
>>>> Third, in C the  builtins are not diagnosed, because C does only look
>>>> at the mode of the parameters see match_builtin_function_types in
>>>> c/c-decl.c, which may itself be wrong, because that makes an ABI
>>>> decision dependent on the mode of the parameter.
>>>
>>> The matching needs to be loose because of functions using types such as
>>> FILE * where the compiler doesn't know the exact contents of the type
>>> when
>>> processing built-in function definitions.  (Historically there were also
>>> issues with pre-ISO headers, but that may be less relevant now.)
>>>
>>
>> The C++ FE has exactly the same problem with FILE* and struct tm*
>> but it solves it differently and "learns" the type but only for FILE*
>> and with this patch also for const struct tm*.  It is a lot more
>> restrictive than C, but that is because of the ++ ;)
>>
>> Well in that case the posix functions have to use the prototypes
>> from POSIX.1.2008 although their rationale is a bit silly...
>>
>> This updated patch fixes the prototype of execv/p/e,
>> and adds a new test case that checks that no type conflict
>> exists in the execve built-in any more.
>>
>> Now we have no -Wsystem-headers warnings with glibc-headers any more.
>> And the gcov builtin also is working with C++.
>>
>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
>> Is it OK for trunk?
>>
>>
>> Thanks
>> Bernd.

Re: [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Jason Merrill]

On 10/17/2016 03:18 PM, Bernd Edlinger wrote:
> Regarding this hunk:
>
>         /* Whether or not the builtin can throw exceptions has no
>           bearing on this declarator.  */
> -      TREE_NOTHROW (olddecl) = 0;
> +      TREE_NOTHROW (olddecl) = TREE_NOTHROW (newdecl);
>
> You may ask, why the old code was working most of the time.
> I think, usually, when types_match == true, there happens another
> assignment to TREE_NOTHROW, later in that function around line 2183:
>
>        /* Merge the type qualifiers.  */
>        if (TREE_READONLY (newdecl))
>          TREE_READONLY (olddecl) = 1;
>        if (TREE_THIS_VOLATILE (newdecl))
>          TREE_THIS_VOLATILE (olddecl) = 1;
>        if (TREE_NOTHROW (newdecl))
>          TREE_NOTHROW (olddecl) = 1;
>
> This is in a big "if (types_match)", so I think that explains,
> why the old code did work normally, and why it fails if the
> parameter don't match, but I still have no idea what to say
> in the comment, except that the code should exactly do what
> the comment above says.

I think a better fix would be to add a copy of TREE_NOTHROW to the else 
block of the if (types_match), to go with the existing copies of 
TREE_READONLY and TREE_THIS_VOLATILE.  But yes, duplicate_decls is a mess.

Jason

Re: [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Jason Merrill]

On 10/17/2016 03:18 PM, Bernd Edlinger wrote:
> +@item -Wbuiltin-function-redefined @r{(C++ and Objective-C++ only)}
> +@opindex Wbuiltin-function-redefined
> +@opindex Wno-builtin-function-redefined
> +Do warn if built-in functions are redefined.  This option is only
> +supported for C++ and Objective-C++.  It is implied by @option{-Wall},
> +which can be disabled with @option{-Wno-builtin-function-redefined}.

There's no redefinition here (only a redeclaration), so perhaps 
-Wbuiltin-redeclaration-mismatch?

I'm not even sure we need a new warning.  Can we combine this warning 
with the block that currently follows?

>           else if ((DECL_EXTERN_C_P (newdecl)
>                     && DECL_EXTERN_C_P (olddecl))
>                    || compparms (TYPE_ARG_TYPES (TREE_TYPE (newdecl)),
>                                  TYPE_ARG_TYPES (TREE_TYPE (olddecl))))
>             {
>               /* A near match; override the builtin.  */
>
>               if (TREE_PUBLIC (newdecl))
>                 warning_at (DECL_SOURCE_LOCATION (newdecl), 0,
>                             "new declaration %q#D ambiguates built-in "
>                             "declaration %q#D", newdecl, olddecl);
>               else
>                 warning (OPT_Wshadow,
>                          DECL_BUILT_IN (olddecl)
>                          ? G_("shadowing built-in function %q#D")
>                          : G_("shadowing library function %q#D"), olddecl);
>             }

It seems to me that if we have an extern "C" declaration that doesn't 
match the built-in, we should just warn.

I looked at some of the testcases you mentioned in the bug report, and 
those declarations aren't extern "C", so we shouldn't be warning about 
them.  Does your current patch still warn about those?

Jason

Re: [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Bernd Edlinger]

On 11/01/16 16:02, Jason Merrill wrote:
> On 10/17/2016 03:18 PM, Bernd Edlinger wrote:
>> Regarding this hunk:
>>
>>         /* Whether or not the builtin can throw exceptions has no
>>           bearing on this declarator.  */
>> -      TREE_NOTHROW (olddecl) = 0;
>> +      TREE_NOTHROW (olddecl) = TREE_NOTHROW (newdecl);
>>
>> You may ask, why the old code was working most of the time.
>> I think, usually, when types_match == true, there happens another
>> assignment to TREE_NOTHROW, later in that function around line 2183:
>>
>>        /* Merge the type qualifiers.  */
>>        if (TREE_READONLY (newdecl))
>>          TREE_READONLY (olddecl) = 1;
>>        if (TREE_THIS_VOLATILE (newdecl))
>>          TREE_THIS_VOLATILE (olddecl) = 1;
>>        if (TREE_NOTHROW (newdecl))
>>          TREE_NOTHROW (olddecl) = 1;
>>
>> This is in a big "if (types_match)", so I think that explains,
>> why the old code did work normally, and why it fails if the
>> parameter don't match, but I still have no idea what to say
>> in the comment, except that the code should exactly do what
>> the comment above says.
>
> I think a better fix would be to add a copy of TREE_NOTHROW to the else
> block of the if (types_match), to go with the existing copies of
> TREE_READONLY and TREE_THIS_VOLATILE.  But yes, duplicate_decls is a mess.
>

Possibly, but I am only aware of problems with redeclarations
of builtins functions, TREE_CODE (olddecl) == FUNCTION_DECL
&& DECL_ARTIFICIAL (olddecl), but the "if (types_match)" can
be called for many other types of decls.

So super scary code here....

Re: [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Bernd Edlinger]

On 11/01/16 16:20, Jason Merrill wrote:
> On 10/17/2016 03:18 PM, Bernd Edlinger wrote:
>> +@item -Wbuiltin-function-redefined @r{(C++ and Objective-C++ only)}
>> +@opindex Wbuiltin-function-redefined
>> +@opindex Wno-builtin-function-redefined
>> +Do warn if built-in functions are redefined.  This option is only
>> +supported for C++ and Objective-C++.  It is implied by @option{-Wall},
>> +which can be disabled with @option{-Wno-builtin-function-redefined}.
>
> There's no redefinition here (only a redeclaration), so perhaps
> -Wbuiltin-redeclaration-mismatch?
>

Works for me.  Thanks.

> I'm not even sure we need a new warning.  Can we combine this warning
> with the block that currently follows?
>

After 20 years of not having a warning on that,
an implicitly enabled warning would at least break lots of bogus
test cases.  Of course in C we have an implicitly enabled warning,
so I would like to at least enable the warning on -Wall, thus
-Wshadow is too weak IMO.

>>           else if ((DECL_EXTERN_C_P (newdecl)
>>                     && DECL_EXTERN_C_P (olddecl))
>>                    || compparms (TYPE_ARG_TYPES (TREE_TYPE (newdecl)),
>>                                  TYPE_ARG_TYPES (TREE_TYPE (olddecl))))
>>             {
>>               /* A near match; override the builtin.  */
>>
>>               if (TREE_PUBLIC (newdecl))
>>                 warning_at (DECL_SOURCE_LOCATION (newdecl), 0,
>>                             "new declaration %q#D ambiguates built-in "
>>                             "declaration %q#D", newdecl, olddecl);
>>               else
>>                 warning (OPT_Wshadow,
>>                          DECL_BUILT_IN (olddecl)
>>                          ? G_("shadowing built-in function %q#D")
>>                          : G_("shadowing library function %q#D"),
>> olddecl);
>>             }
>
> It seems to me that if we have an extern "C" declaration that doesn't
> match the built-in, we should just warn.
>
> I looked at some of the testcases you mentioned in the bug report, and
> those declarations aren't extern "C", so we shouldn't be warning about
> them.  Does your current patch still warn about those?
>

No.  After looking at the false positives with the previous patch,
I changed my mind about that.

This started because I wanted to add builtin functions for some
special_function_p names.  And I wanted to warn the user if he uses a
name that is recognized by special_function_p, but the parameters
don't match.  Now I think we need to teach special_function_p to
distinguish "C" functions from "C++" functions, which it currently
cannot do, because that knowledge is only in the C++ FE.


Bernd.

Re: [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Jason Merrill]

On Tue, Nov 1, 2016 at 11:45 AM, Bernd Edlinger
<bernd.edlinger@hotmail.de> wrote:
> On 11/01/16 16:20, Jason Merrill wrote:
>> On 10/17/2016 03:18 PM, Bernd Edlinger wrote:
>> I'm not even sure we need a new warning.  Can we combine this warning
>> with the block that currently follows?
>
> After 20 years of not having a warning on that,
> an implicitly enabled warning would at least break lots of bogus
> test cases.

Would it, though?  Which test cases still break with the current patch?

> Of course in C we have an implicitly enabled warning,
> so I would like to at least enable the warning on -Wall, thus
> -Wshadow is too weak IMO.

Right.  The -Wshadow warning is for file-local declarations, so that
doesn't apply to your testcase; I was thinking that we should hit the
first (currently unconditional) warning.

>>>           else if ((DECL_EXTERN_C_P (newdecl)
>>>                     && DECL_EXTERN_C_P (olddecl))
>>>                    || compparms (TYPE_ARG_TYPES (TREE_TYPE (newdecl)),
>>>                                  TYPE_ARG_TYPES (TREE_TYPE (olddecl))))

So I was thinking to drop the "else" and the compparms test.

>>>             {
>>>               /* A near match; override the builtin.  */
>>>
>>>               if (TREE_PUBLIC (newdecl))
>>>                 warning_at (DECL_SOURCE_LOCATION (newdecl), 0,
>>>                             "new declaration %q#D ambiguates built-in "
>>>                             "declaration %q#D", newdecl, olddecl);

So we would hit this warning.  And change the message to remove
"ambiguates", since we're removing the compparms.

> This started because I wanted to add builtin functions for some
> special_function_p names.  And I wanted to warn the user if he uses a
> name that is recognized by special_function_p, but the parameters
> don't match.  Now I think we need to teach special_function_p to
> distinguish "C" functions from "C++" functions, which it currently
> cannot do, because that knowledge is only in the C++ FE.

It could look at DECL_ASSEMBLER_NAME rather than DECL_NAME?

Jason

Re: [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Bernd Edlinger]

On 11/01/16 18:11, Jason Merrill wrote:
> On Tue, Nov 1, 2016 at 11:45 AM, Bernd Edlinger
> <bernd.edlinger@hotmail.de> wrote:
>> On 11/01/16 16:20, Jason Merrill wrote:
>>> On 10/17/2016 03:18 PM, Bernd Edlinger wrote:
>>> I'm not even sure we need a new warning.  Can we combine this warning
>>> with the block that currently follows?
>>
>> After 20 years of not having a warning on that,
>> an implicitly enabled warning would at least break lots of bogus
>> test cases.
>
> Would it, though?  Which test cases still break with the current patch?
>

Less than before, but there are still at least a few of them.

I can make a list and send it tomorrow.

>> Of course in C we have an implicitly enabled warning,
>> so I would like to at least enable the warning on -Wall, thus
>> -Wshadow is too weak IMO.
>
> Right.  The -Wshadow warning is for file-local declarations, so that
> doesn't apply to your testcase; I was thinking that we should hit the
> first (currently unconditional) warning.
>
>>>>           else if ((DECL_EXTERN_C_P (newdecl)
>>>>                     && DECL_EXTERN_C_P (olddecl))
>>>>                    || compparms (TYPE_ARG_TYPES (TREE_TYPE (newdecl)),
>>>>                                  TYPE_ARG_TYPES (TREE_TYPE (olddecl))))
>
> So I was thinking to drop the "else" and the compparms test.
>

Yes.  But then we must somehow avoid:

           else
             /* Discard the old built-in function.  */
             return NULL_TREE;

It maybe easier, just to copy the warning to the DECL_ANTICIPATED case?

>>>>             {
>>>>               /* A near match; override the builtin.  */
>>>>
>>>>               if (TREE_PUBLIC (newdecl))
>>>>                 warning_at (DECL_SOURCE_LOCATION (newdecl), 0,
>>>>                             "new declaration %q#D ambiguates built-in "
>>>>                             "declaration %q#D", newdecl, olddecl);
>
> So we would hit this warning.  And change the message to remove
> "ambiguates", since we're removing the compparms.
>
>> This started because I wanted to add builtin functions for some
>> special_function_p names.  And I wanted to warn the user if he uses a
>> name that is recognized by special_function_p, but the parameters
>> don't match.  Now I think we need to teach special_function_p to
>> distinguish "C" functions from "C++" functions, which it currently
>> cannot do, because that knowledge is only in the C++ FE.
>
> It could look at DECL_ASSEMBLER_NAME rather than DECL_NAME?
>

Yes.  I think previously special_function_p must have used the
DECL_ASSEMBLER_NAME, at least the comments still mention that.

Also for a "C" function there are target specific naming rules,
some prepend an underscore, some don't, and more, especially
W..DOS has special conventions IIRC.

Maybe a language callback would be good to have for this task.
So that special_function_p can easily check if something is a
C++ function, then it is immediately clear that it is not a
special function.



Bernd.

Re: [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Jason Merrill]

On Tue, Nov 1, 2016 at 2:15 PM, Bernd Edlinger
<bernd.edlinger@hotmail.de> wrote:
> On 11/01/16 18:11, Jason Merrill wrote:
>> On Tue, Nov 1, 2016 at 11:45 AM, Bernd Edlinger
>> <bernd.edlinger@hotmail.de> wrote:
>>> On 11/01/16 16:20, Jason Merrill wrote:
>>>> On 10/17/2016 03:18 PM, Bernd Edlinger wrote:
>>>> I'm not even sure we need a new warning.  Can we combine this warning
>>>> with the block that currently follows?
>>>
>>> After 20 years of not having a warning on that,
>>> an implicitly enabled warning would at least break lots of bogus
>>> test cases.
>>
>> Would it, though?  Which test cases still break with the current patch?
>
> Less than before, but there are still at least a few of them.
>
> I can make a list and send it tomorrow.
>
>>> Of course in C we have an implicitly enabled warning,
>>> so I would like to at least enable the warning on -Wall, thus
>>> -Wshadow is too weak IMO.
>>
>> Right.  The -Wshadow warning is for file-local declarations, so that
>> doesn't apply to your testcase; I was thinking that we should hit the
>> first (currently unconditional) warning.
>>
>>>>>           else if ((DECL_EXTERN_C_P (newdecl)
>>>>>                     && DECL_EXTERN_C_P (olddecl))
>>>>>                    || compparms (TYPE_ARG_TYPES (TREE_TYPE (newdecl)),
>>>>>                                  TYPE_ARG_TYPES (TREE_TYPE (olddecl))))
>>
>> So I was thinking to drop the "else" and the compparms test.
>
> Yes.  But then we must somehow avoid:
>
>            else
>              /* Discard the old built-in function.  */
>              return NULL_TREE;
>
> It maybe easier, just to copy the warning to the DECL_ANTICIPATED case?

Or even move it there; removing the existing warning doesn't change
anything in the testsuite, and I'm having trouble imagining how to
trigger it.

Jason

Re: [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Bernd Edlinger]

On 11/01/16 20:48, Jason Merrill wrote:
>>>>>>           else if ((DECL_EXTERN_C_P (newdecl)
>>>>>>                     && DECL_EXTERN_C_P (olddecl))
>>>>>>                    || compparms (TYPE_ARG_TYPES (TREE_TYPE (newdecl)),
>>>>>>                                  TYPE_ARG_TYPES (TREE_TYPE (olddecl))))
>>>
>>> So I was thinking to drop the "else" and the compparms test.
>>
>> Yes.  But then we must somehow avoid:
>>
>>            else
>>              /* Discard the old built-in function.  */
>>              return NULL_TREE;
>>
>> It maybe easier, just to copy the warning to the DECL_ANTICIPATED case?
>
> Or even move it there; removing the existing warning doesn't change
> anything in the testsuite, and I'm having trouble imagining how to
> trigger it.
>

Nice.  It must be something, which does not anticipate a declaration.

like:

cat test.cc
int __builtin_abort(void*);

g++ -c test.cc
test.cc:1:5: warning: new declaration 'int __builtin_abort(void*)' 
ambiguates built-in declaration 'void __builtin_abort()'
  int __builtin_abort(void*);
      ^~~~~~~~~~~~~~~

Intersting, the warning comes even though I forgot to add the
extern "C".


Bernd.

Re: [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Jason Merrill]

On Tue, Nov 1, 2016 at 4:00 PM, Bernd Edlinger
<bernd.edlinger@hotmail.de> wrote:
> On 11/01/16 20:48, Jason Merrill wrote:
>>>>>>>           else if ((DECL_EXTERN_C_P (newdecl)
>>>>>>>                     && DECL_EXTERN_C_P (olddecl))
>>>>>>>                    || compparms (TYPE_ARG_TYPES (TREE_TYPE (newdecl)),
>>>>>>>                                  TYPE_ARG_TYPES (TREE_TYPE (olddecl))))
>>>>
>>>> So I was thinking to drop the "else" and the compparms test.
>>>
>>> Yes.  But then we must somehow avoid:
>>>
>>>            else
>>>              /* Discard the old built-in function.  */
>>>              return NULL_TREE;

>>> It maybe easier, just to copy the warning to the DECL_ANTICIPATED case?
>>
>> Or even move it there; removing the existing warning doesn't change
>> anything in the testsuite, and I'm having trouble imagining how to
>> trigger it.
>>
>
> Nice.  It must be something, which does not anticipate a declaration.
>
> like:
>
> cat test.cc
> int __builtin_abort(void*);
>
> g++ -c test.cc
> test.cc:1:5: warning: new declaration 'int __builtin_abort(void*)'
> ambiguates built-in declaration 'void __builtin_abort()'
>   int __builtin_abort(void*);
>       ^~~~~~~~~~~~~~~
>
> Intersting, the warning comes even though I forgot to add the
> extern "C".

Looks like anything starting with __builtin is implicitly extern "C"
(set in grokfndecl).

If we remove the DECL_ANTICIPATED check, I see some failures in
builtin* tests due to missing extern "C".  That seems appropriate at
file scope, but I'm not sure it's right for namespace std.

Jason

Re: [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Bernd Edlinger]

On 11/01/16 19:15, Bernd Edlinger wrote:
> On 11/01/16 18:11, Jason Merrill wrote:
>> On Tue, Nov 1, 2016 at 11:45 AM, Bernd Edlinger
>> <bernd.edlinger@hotmail.de> wrote:
>>> On 11/01/16 16:20, Jason Merrill wrote:
>>>> On 10/17/2016 03:18 PM, Bernd Edlinger wrote:
>>>> I'm not even sure we need a new warning.  Can we combine this warning
>>>> with the block that currently follows?
>>>
>>> After 20 years of not having a warning on that,
>>> an implicitly enabled warning would at least break lots of bogus
>>> test cases.
>>
>> Would it, though?  Which test cases still break with the current patch?
>>
>
> Less than before, but there are still at least a few of them.
>
> I can make a list and send it tomorrow.
>

FAIL: g++.dg/cpp1y/lambda-generic-udt.C  -std=gnu++14 (test for excess 
errors)
FAIL: g++.dg/cpp1y/lambda-generic-xudt.C  -std=gnu++14 (test for excess 
errors)
FAIL: g++.dg/init/new15.C  -std=c++11 (test for excess errors)
FAIL: g++.dg/init/new15.C  -std=c++14 (test for excess errors)
FAIL: g++.dg/init/new15.C  -std=c++98 (test for excess errors)
FAIL: g++.dg/ipa/inline-1.C  -std=gnu++11 (test for excess errors)
FAIL: g++.dg/ipa/inline-1.C  -std=gnu++14 (test for excess errors)
FAIL: g++.dg/ipa/inline-1.C  -std=gnu++98 (test for excess errors)
FAIL: g++.dg/ipa/inline-2.C  -std=gnu++11 (test for excess errors)
FAIL: g++.dg/ipa/inline-2.C  -std=gnu++14 (test for excess errors)
FAIL: g++.dg/ipa/inline-2.C  -std=gnu++98 (test for excess errors)
FAIL: g++.dg/tc1/dr20.C  -std=c++11 (test for excess errors)
FAIL: g++.dg/tc1/dr20.C  -std=c++14 (test for excess errors)
FAIL: g++.dg/tc1/dr20.C  -std=c++98 (test for excess errors)
FAIL: g++.dg/tree-ssa/inline-1.C  -std=gnu++11 (test for excess errors)
FAIL: g++.dg/tree-ssa/inline-1.C  -std=gnu++14 (test for excess errors)
FAIL: g++.dg/tree-ssa/inline-1.C  -std=gnu++98 (test for excess errors)
FAIL: g++.dg/tree-ssa/inline-2.C  -std=gnu++11 (test for excess errors)
FAIL: g++.dg/tree-ssa/inline-2.C  -std=gnu++14 (test for excess errors)
FAIL: g++.dg/tree-ssa/inline-2.C  -std=gnu++98 (test for excess errors)
FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O0 -flto 
-flto-partition=1to1 -fno-use-linker-plugin
FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O0 -flto 
-flto-partition=none -fuse-linker-plugin
FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O0 -flto 
-fuse-linker-plugin -fno-fat-lto-objects
FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O2 -flto 
-flto-partition=1to1 -fno-use-linker-plugin
FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O2 -flto 
-flto-partition=none -fuse-linker-plugin -fno-fat-lto-objects
FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O2 -flto 
-fuse-linker-plugin
FAIL: g++.dg/lto/pr68811 cp_lto_pr68811_0.o assemble, -O2
FAIL: g++.old-deja/g++.law/except1.C  -std=gnu++11 (test for excess errors)
FAIL: g++.old-deja/g++.law/except1.C  -std=gnu++14 (test for excess errors)
FAIL: g++.old-deja/g++.law/except1.C  -std=gnu++98 (test for excess errors)
FAIL: g++.old-deja/g++.mike/p700.C  -std=gnu++11 (test for excess errors)
FAIL: g++.old-deja/g++.mike/p700.C  -std=gnu++14 (test for excess errors)
FAIL: g++.old-deja/g++.mike/p700.C  -std=gnu++98 (test for excess errors)
FAIL: g++.old-deja/g++.other/builtins10.C  -std=c++11 (test for excess 
errors)
FAIL: g++.old-deja/g++.other/builtins10.C  -std=c++14 (test for excess 
errors)
FAIL: g++.old-deja/g++.other/realloc.C  -std=c++11 (test for excess errors)
FAIL: g++.old-deja/g++.other/realloc.C  -std=c++14 (test for excess errors)
FAIL: g++.old-deja/g++.other/realloc.C  -std=c++98 (test for excess errors)
FAIL: g++.old-deja/g++.other/vbase5.C  -std=c++11 (test for excess errors)
FAIL: g++.old-deja/g++.other/vbase5.C  -std=c++14 (test for excess errors)
FAIL: g++.old-deja/g++.other/vbase5.C  -std=c++98 (test for excess errors)


The lto test case does emit the warning when assembling, but
it still produces an executable and even executes it.

Also g++.dg/cpp1y/lambda-generic-udt.C, g++.dg/tc1/dr20.C
and g++.old-deja/g++.other/vbase5.C are execution tests.

So I was wrong to assume these were all compile-only tests.

I think that list should be fixable, if we decide to enable
the warning by default.

>>> Of course in C we have an implicitly enabled warning,
>>> so I would like to at least enable the warning on -Wall, thus
>>> -Wshadow is too weak IMO.
>>
>> Right.  The -Wshadow warning is for file-local declarations, so that
>> doesn't apply to your testcase; I was thinking that we should hit the
>> first (currently unconditional) warning.
>>
>>>>>           else if ((DECL_EXTERN_C_P (newdecl)
>>>>>                     && DECL_EXTERN_C_P (olddecl))
>>>>>                    || compparms (TYPE_ARG_TYPES (TREE_TYPE (newdecl)),
>>>>>                                  TYPE_ARG_TYPES (TREE_TYPE
>>>>> (olddecl))))
>>
>> So I was thinking to drop the "else" and the compparms test.
>>
>
> Yes.  But then we must somehow avoid:
>
>           else
>             /* Discard the old built-in function.  */
>             return NULL_TREE;
>
> It maybe easier, just to copy the warning to the DECL_ANTICIPATED case?
>
>>>>>             {
>>>>>               /* A near match; override the builtin.  */
>>>>>
>>>>>               if (TREE_PUBLIC (newdecl))
>>>>>                 warning_at (DECL_SOURCE_LOCATION (newdecl), 0,
>>>>>                             "new declaration %q#D ambiguates
>>>>> built-in "
>>>>>                             "declaration %q#D", newdecl, olddecl);
>>
>> So we would hit this warning.  And change the message to remove
>> "ambiguates", since we're removing the compparms.
>>
>>> This started because I wanted to add builtin functions for some
>>> special_function_p names.  And I wanted to warn the user if he uses a
>>> name that is recognized by special_function_p, but the parameters
>>> don't match.  Now I think we need to teach special_function_p to
>>> distinguish "C" functions from "C++" functions, which it currently
>>> cannot do, because that knowledge is only in the C++ FE.
>>
>> It could look at DECL_ASSEMBLER_NAME rather than DECL_NAME?
>>
>
> Yes.  I think previously special_function_p must have used the
> DECL_ASSEMBLER_NAME, at least the comments still mention that.
>
> Also for a "C" function there are target specific naming rules,
> some prepend an underscore, some don't, and more, especially
> W..DOS has special conventions IIRC.
>
> Maybe a language callback would be good to have for this task.
> So that special_function_p can easily check if something is a
> C++ function, then it is immediately clear that it is not a
> special function.
>
>
>
> Bernd.

Re: [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Bernd Edlinger]

On 11/02/16 07:11, Bernd Edlinger wrote:
> On 11/01/16 19:15, Bernd Edlinger wrote:
>> On 11/01/16 18:11, Jason Merrill wrote:
>>> On Tue, Nov 1, 2016 at 11:45 AM, Bernd Edlinger
>>> <bernd.edlinger@hotmail.de> wrote:
>>>> On 11/01/16 16:20, Jason Merrill wrote:
>>>>> On 10/17/2016 03:18 PM, Bernd Edlinger wrote:
>>>>> I'm not even sure we need a new warning.  Can we combine this warning
>>>>> with the block that currently follows?
>>>>
>>>> After 20 years of not having a warning on that,
>>>> an implicitly enabled warning would at least break lots of bogus
>>>> test cases.
>>>
>>> Would it, though?  Which test cases still break with the current patch?
>>>
>>
>> Less than before, but there are still at least a few of them.
>>
>> I can make a list and send it tomorrow.
>>
>
> FAIL: g++.dg/cpp1y/lambda-generic-udt.C  -std=gnu++14 (test for excess
> errors)
> FAIL: g++.dg/cpp1y/lambda-generic-xudt.C  -std=gnu++14 (test for excess
> errors)
> FAIL: g++.dg/init/new15.C  -std=c++11 (test for excess errors)
> FAIL: g++.dg/init/new15.C  -std=c++14 (test for excess errors)
> FAIL: g++.dg/init/new15.C  -std=c++98 (test for excess errors)
> FAIL: g++.dg/ipa/inline-1.C  -std=gnu++11 (test for excess errors)
> FAIL: g++.dg/ipa/inline-1.C  -std=gnu++14 (test for excess errors)
> FAIL: g++.dg/ipa/inline-1.C  -std=gnu++98 (test for excess errors)
> FAIL: g++.dg/ipa/inline-2.C  -std=gnu++11 (test for excess errors)
> FAIL: g++.dg/ipa/inline-2.C  -std=gnu++14 (test for excess errors)
> FAIL: g++.dg/ipa/inline-2.C  -std=gnu++98 (test for excess errors)
> FAIL: g++.dg/tc1/dr20.C  -std=c++11 (test for excess errors)
> FAIL: g++.dg/tc1/dr20.C  -std=c++14 (test for excess errors)
> FAIL: g++.dg/tc1/dr20.C  -std=c++98 (test for excess errors)
> FAIL: g++.dg/tree-ssa/inline-1.C  -std=gnu++11 (test for excess errors)
> FAIL: g++.dg/tree-ssa/inline-1.C  -std=gnu++14 (test for excess errors)
> FAIL: g++.dg/tree-ssa/inline-1.C  -std=gnu++98 (test for excess errors)
> FAIL: g++.dg/tree-ssa/inline-2.C  -std=gnu++11 (test for excess errors)
> FAIL: g++.dg/tree-ssa/inline-2.C  -std=gnu++14 (test for excess errors)
> FAIL: g++.dg/tree-ssa/inline-2.C  -std=gnu++98 (test for excess errors)
> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O0 -flto
> -flto-partition=1to1 -fno-use-linker-plugin
> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O0 -flto
> -flto-partition=none -fuse-linker-plugin
> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O0 -flto
> -fuse-linker-plugin -fno-fat-lto-objects
> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O2 -flto
> -flto-partition=1to1 -fno-use-linker-plugin
> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O2 -flto
> -flto-partition=none -fuse-linker-plugin -fno-fat-lto-objects
> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O2 -flto
> -fuse-linker-plugin
> FAIL: g++.dg/lto/pr68811 cp_lto_pr68811_0.o assemble, -O2
> FAIL: g++.old-deja/g++.law/except1.C  -std=gnu++11 (test for excess errors)
> FAIL: g++.old-deja/g++.law/except1.C  -std=gnu++14 (test for excess errors)
> FAIL: g++.old-deja/g++.law/except1.C  -std=gnu++98 (test for excess errors)
> FAIL: g++.old-deja/g++.mike/p700.C  -std=gnu++11 (test for excess errors)
> FAIL: g++.old-deja/g++.mike/p700.C  -std=gnu++14 (test for excess errors)
> FAIL: g++.old-deja/g++.mike/p700.C  -std=gnu++98 (test for excess errors)
> FAIL: g++.old-deja/g++.other/builtins10.C  -std=c++11 (test for excess
> errors)
> FAIL: g++.old-deja/g++.other/builtins10.C  -std=c++14 (test for excess
> errors)
> FAIL: g++.old-deja/g++.other/realloc.C  -std=c++11 (test for excess errors)
> FAIL: g++.old-deja/g++.other/realloc.C  -std=c++14 (test for excess errors)
> FAIL: g++.old-deja/g++.other/realloc.C  -std=c++98 (test for excess errors)
> FAIL: g++.old-deja/g++.other/vbase5.C  -std=c++11 (test for excess errors)
> FAIL: g++.old-deja/g++.other/vbase5.C  -std=c++14 (test for excess errors)
> FAIL: g++.old-deja/g++.other/vbase5.C  -std=c++98 (test for excess errors)
>
>
> The lto test case does emit the warning when assembling, but
> it still produces an executable and even executes it.
>
> Also g++.dg/cpp1y/lambda-generic-udt.C, g++.dg/tc1/dr20.C
> and g++.old-deja/g++.other/vbase5.C are execution tests.
>
> So I was wrong to assume these were all compile-only tests.
>
> I think that list should be fixable, if we decide to enable
> the warning by default.
>

Aehm, sorry.  I forgot to look at other languages...

                 === obj-c++ tests ===


Running target unix
FAIL: obj-c++.dg/lto/trivial-1 obj_cpp_lto_trivial-1_0.o assemble, -O0 
-flto -fg
nu-runtime
FAIL: obj-c++.dg/lto/trivial-1 obj_cpp_lto_trivial-1_0.o assemble, -O2 
-flto -fg
nu-runtime
FAIL: obj-c++.dg/lto/trivial-1 obj_cpp_lto_trivial-1_0.o assemble, -O0 
-flto -fl
to-partition=none -fgnu-runtime
FAIL: obj-c++.dg/lto/trivial-1 obj_cpp_lto_trivial-1_0.o assemble, -O2 
-flto -fl
to-partition=none -fgnu-runtime

                 === libitm tests ===


Running target unix
FAIL: libitm.c++/dropref.C (test for excess errors)
FAIL: libitm.c++/throwdown.C (test for excess errors)


Bernd.

Re: [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Bernd Edlinger]

On 11/01/16 22:31, Jason Merrill wrote:
> On Tue, Nov 1, 2016 at 4:00 PM, Bernd Edlinger
> <bernd.edlinger@hotmail.de> wrote:
>> On 11/01/16 20:48, Jason Merrill wrote:
>>>>>>>>           else if ((DECL_EXTERN_C_P (newdecl)
>>>>>>>>                     && DECL_EXTERN_C_P (olddecl))
>>>>>>>>                    || compparms (TYPE_ARG_TYPES (TREE_TYPE (newdecl)),
>>>>>>>>                                  TYPE_ARG_TYPES (TREE_TYPE (olddecl))))
>>>>>
>>>>> So I was thinking to drop the "else" and the compparms test.
>>>>
>>>> Yes.  But then we must somehow avoid:
>>>>
>>>>            else
>>>>              /* Discard the old built-in function.  */
>>>>              return NULL_TREE;
>
>>>> It maybe easier, just to copy the warning to the DECL_ANTICIPATED case?
>>>
>>> Or even move it there; removing the existing warning doesn't change
>>> anything in the testsuite, and I'm having trouble imagining how to
>>> trigger it.
>>>
>>
>> Nice.  It must be something, which does not anticipate a declaration.
>>
>> like:
>>
>> cat test.cc
>> int __builtin_abort(void*);
>>
>> g++ -c test.cc
>> test.cc:1:5: warning: new declaration 'int __builtin_abort(void*)'
>> ambiguates built-in declaration 'void __builtin_abort()'
>>   int __builtin_abort(void*);
>>       ^~~~~~~~~~~~~~~
>>
>> Intersting, the warning comes even though I forgot to add the
>> extern "C".
>
> Looks like anything starting with __builtin is implicitly extern "C"
> (set in grokfndecl).
>

Oh, Well.

we do also have such builtins that begin with __sync_
like

extern "C"
unsigned char __sync_fetch_and_add_1(volatile void*, unsigned char);

> If we remove the DECL_ANTICIPATED check, I see some failures in
> builtin* tests due to missing extern "C".  That seems appropriate at
> file scope, but I'm not sure it's right for namespace std.
>


Interesting, what have you done?

Sounds a bit like the issue I had with the std::abs overloads.

To get rid of that I added this in the DECL_ANTICIPATED path:

+             /* 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;
+


Bernd.

Re: [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 4988 bytes --]

On 11/02/2016 02:11 AM, Bernd Edlinger wrote:
> On 11/01/16 19:15, Bernd Edlinger wrote:
>> On 11/01/16 18:11, Jason Merrill wrote:
>>> On Tue, Nov 1, 2016 at 11:45 AM, Bernd Edlinger
>>> <bernd.edlinger@hotmail.de> wrote:
>>>> On 11/01/16 16:20, Jason Merrill wrote:
>>>>> On 10/17/2016 03:18 PM, Bernd Edlinger wrote:
>>>>> I'm not even sure we need a new warning.  Can we combine this warning
>>>>> with the block that currently follows?
>>>>
>>>> After 20 years of not having a warning on that,
>>>> an implicitly enabled warning would at least break lots of bogus
>>>> test cases.
>>>
>>> Would it, though?  Which test cases still break with the current patch?
>>
>> Less than before, but there are still at least a few of them.
>>
>> I can make a list and send it tomorrow.
>
> FAIL: g++.dg/cpp1y/lambda-generic-udt.C  -std=gnu++14 (test for excess
> errors)
> FAIL: g++.dg/cpp1y/lambda-generic-xudt.C  -std=gnu++14 (test for excess
> errors)
> FAIL: g++.dg/init/new15.C  -std=c++11 (test for excess errors)
> FAIL: g++.dg/init/new15.C  -std=c++14 (test for excess errors)
> FAIL: g++.dg/init/new15.C  -std=c++98 (test for excess errors)
> FAIL: g++.dg/ipa/inline-1.C  -std=gnu++11 (test for excess errors)
> FAIL: g++.dg/ipa/inline-1.C  -std=gnu++14 (test for excess errors)
> FAIL: g++.dg/ipa/inline-1.C  -std=gnu++98 (test for excess errors)
> FAIL: g++.dg/ipa/inline-2.C  -std=gnu++11 (test for excess errors)
> FAIL: g++.dg/ipa/inline-2.C  -std=gnu++14 (test for excess errors)
> FAIL: g++.dg/ipa/inline-2.C  -std=gnu++98 (test for excess errors)
> FAIL: g++.dg/tc1/dr20.C  -std=c++11 (test for excess errors)
> FAIL: g++.dg/tc1/dr20.C  -std=c++14 (test for excess errors)
> FAIL: g++.dg/tc1/dr20.C  -std=c++98 (test for excess errors)
> FAIL: g++.dg/tree-ssa/inline-1.C  -std=gnu++11 (test for excess errors)
> FAIL: g++.dg/tree-ssa/inline-1.C  -std=gnu++14 (test for excess errors)
> FAIL: g++.dg/tree-ssa/inline-1.C  -std=gnu++98 (test for excess errors)
> FAIL: g++.dg/tree-ssa/inline-2.C  -std=gnu++11 (test for excess errors)
> FAIL: g++.dg/tree-ssa/inline-2.C  -std=gnu++14 (test for excess errors)
> FAIL: g++.dg/tree-ssa/inline-2.C  -std=gnu++98 (test for excess errors)
> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O0 -flto
> -flto-partition=1to1 -fno-use-linker-plugin
> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O0 -flto
> -flto-partition=none -fuse-linker-plugin
> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O0 -flto
> -fuse-linker-plugin -fno-fat-lto-objects
> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O2 -flto
> -flto-partition=1to1 -fno-use-linker-plugin
> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O2 -flto
> -flto-partition=none -fuse-linker-plugin -fno-fat-lto-objects
> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O2 -flto
> -fuse-linker-plugin
> FAIL: g++.dg/lto/pr68811 cp_lto_pr68811_0.o assemble, -O2
> FAIL: g++.old-deja/g++.law/except1.C  -std=gnu++11 (test for excess errors)
> FAIL: g++.old-deja/g++.law/except1.C  -std=gnu++14 (test for excess errors)
> FAIL: g++.old-deja/g++.law/except1.C  -std=gnu++98 (test for excess errors)
> FAIL: g++.old-deja/g++.mike/p700.C  -std=gnu++11 (test for excess errors)
> FAIL: g++.old-deja/g++.mike/p700.C  -std=gnu++14 (test for excess errors)
> FAIL: g++.old-deja/g++.mike/p700.C  -std=gnu++98 (test for excess errors)
> FAIL: g++.old-deja/g++.other/builtins10.C  -std=c++11 (test for excess
> errors)
> FAIL: g++.old-deja/g++.other/builtins10.C  -std=c++14 (test for excess
> errors)
> FAIL: g++.old-deja/g++.other/realloc.C  -std=c++11 (test for excess errors)
> FAIL: g++.old-deja/g++.other/realloc.C  -std=c++14 (test for excess errors)
> FAIL: g++.old-deja/g++.other/realloc.C  -std=c++98 (test for excess errors)
> FAIL: g++.old-deja/g++.other/vbase5.C  -std=c++11 (test for excess errors)
> FAIL: g++.old-deja/g++.other/vbase5.C  -std=c++14 (test for excess errors)
> FAIL: g++.old-deja/g++.other/vbase5.C  -std=c++98 (test for excess errors)
>
>
> The lto test case does emit the warning when assembling, but
> it still produces an executable and even executes it.
>
> Also g++.dg/cpp1y/lambda-generic-udt.C, g++.dg/tc1/dr20.C
> and g++.old-deja/g++.other/vbase5.C are execution tests.
>
> So I was wrong to assume these were all compile-only tests.
>
> I think that list should be fixable, if we decide to enable
> the warning by default.

Yes, either by fixing the prototypes or disabling the warning.

>> If we remove the DECL_ANTICIPATED check, I see some failures in
>> builtin* tests due to missing extern "C".  That seems appropriate at
>> file scope, but I'm not sure it's right for namespace std.
>
> Interesting, what have you done?

The attached.  But now that you've found that the existing warning deals 
with people doing silly things like redeclaring __builtin_* I guess 
let's leave it alone, and add the warning to the DECL_ANTICIPATED block 
the way you proposed.

Jason


[-- Attachment #2: rem-ant.diff --]
[-- Type: text/x-patch, Size: 1413 bytes --]

commit d93d421435f8c38b2019526c7645a59e79a92cc5
Author: Jason Merrill <jason@redhat.com>
Date:   Tue Nov 1 17:16:12 2016 -0400

    rem-ant

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index ecf4d14..963087d 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -1485,10 +1485,6 @@ duplicate_decls (tree newdecl, tree olddecl, bool newdecl_is_friend)
 	}
       else if (!types_match)
 	{
-	  /* Avoid warnings redeclaring built-ins which have not been
-	     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;
@@ -1521,8 +1517,8 @@ duplicate_decls (tree newdecl, tree olddecl, bool newdecl_is_friend)
 	      }
 	    else if (! same_type_p (TREE_VALUE (t1), TREE_VALUE (t2)))
 	      break;
-	    }
-	  else if ((DECL_EXTERN_C_P (newdecl)
+
+	  if ((DECL_EXTERN_C_P (newdecl)
 	       && DECL_EXTERN_C_P (olddecl))
 	      || compparms (TYPE_ARG_TYPES (TREE_TYPE (newdecl)),
 			    TYPE_ARG_TYPES (TREE_TYPE (olddecl))))
@@ -1540,7 +1536,7 @@ duplicate_decls (tree newdecl, tree olddecl, bool newdecl_is_friend)
                          : G_("shadowing library function %q#D"), olddecl);
 	    }
 	  else
-	    /* Discard the old built-in function.  */
+	    /* Not a duplicate.  */
 	    return NULL_TREE;
 
 	  /* Replace the old RTL to avoid problems with inlining.  */

Re: [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Bernd Edlinger]

[-- Attachment #1: Type: text/plain, Size: 5515 bytes --]

On 11/02/16 18:51, Jason Merrill wrote:
> On 11/02/2016 02:11 AM, Bernd Edlinger wrote:
>> On 11/01/16 19:15, Bernd Edlinger wrote:
>>> On 11/01/16 18:11, Jason Merrill wrote:
>>>> On Tue, Nov 1, 2016 at 11:45 AM, Bernd Edlinger
>>>> <bernd.edlinger@hotmail.de> wrote:
>>>>> On 11/01/16 16:20, Jason Merrill wrote:
>>>>>> On 10/17/2016 03:18 PM, Bernd Edlinger wrote:
>>>>>> I'm not even sure we need a new warning.  Can we combine this warning
>>>>>> with the block that currently follows?
>>>>>
>>>>> After 20 years of not having a warning on that,
>>>>> an implicitly enabled warning would at least break lots of bogus
>>>>> test cases.
>>>>
>>>> Would it, though?  Which test cases still break with the current patch?
>>>
>>> Less than before, but there are still at least a few of them.
>>>
>>> I can make a list and send it tomorrow.
>>
>> FAIL: g++.dg/cpp1y/lambda-generic-udt.C  -std=gnu++14 (test for excess
>> errors)
>> FAIL: g++.dg/cpp1y/lambda-generic-xudt.C  -std=gnu++14 (test for excess
>> errors)
>> FAIL: g++.dg/init/new15.C  -std=c++11 (test for excess errors)
>> FAIL: g++.dg/init/new15.C  -std=c++14 (test for excess errors)
>> FAIL: g++.dg/init/new15.C  -std=c++98 (test for excess errors)
>> FAIL: g++.dg/ipa/inline-1.C  -std=gnu++11 (test for excess errors)
>> FAIL: g++.dg/ipa/inline-1.C  -std=gnu++14 (test for excess errors)
>> FAIL: g++.dg/ipa/inline-1.C  -std=gnu++98 (test for excess errors)
>> FAIL: g++.dg/ipa/inline-2.C  -std=gnu++11 (test for excess errors)
>> FAIL: g++.dg/ipa/inline-2.C  -std=gnu++14 (test for excess errors)
>> FAIL: g++.dg/ipa/inline-2.C  -std=gnu++98 (test for excess errors)
>> FAIL: g++.dg/tc1/dr20.C  -std=c++11 (test for excess errors)
>> FAIL: g++.dg/tc1/dr20.C  -std=c++14 (test for excess errors)
>> FAIL: g++.dg/tc1/dr20.C  -std=c++98 (test for excess errors)
>> FAIL: g++.dg/tree-ssa/inline-1.C  -std=gnu++11 (test for excess errors)
>> FAIL: g++.dg/tree-ssa/inline-1.C  -std=gnu++14 (test for excess errors)
>> FAIL: g++.dg/tree-ssa/inline-1.C  -std=gnu++98 (test for excess errors)
>> FAIL: g++.dg/tree-ssa/inline-2.C  -std=gnu++11 (test for excess errors)
>> FAIL: g++.dg/tree-ssa/inline-2.C  -std=gnu++14 (test for excess errors)
>> FAIL: g++.dg/tree-ssa/inline-2.C  -std=gnu++98 (test for excess errors)
>> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O0 -flto
>> -flto-partition=1to1 -fno-use-linker-plugin
>> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O0 -flto
>> -flto-partition=none -fuse-linker-plugin
>> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O0 -flto
>> -fuse-linker-plugin -fno-fat-lto-objects
>> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O2 -flto
>> -flto-partition=1to1 -fno-use-linker-plugin
>> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O2 -flto
>> -flto-partition=none -fuse-linker-plugin -fno-fat-lto-objects
>> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O2 -flto
>> -fuse-linker-plugin
>> FAIL: g++.dg/lto/pr68811 cp_lto_pr68811_0.o assemble, -O2
>> FAIL: g++.old-deja/g++.law/except1.C  -std=gnu++11 (test for excess
>> errors)
>> FAIL: g++.old-deja/g++.law/except1.C  -std=gnu++14 (test for excess
>> errors)
>> FAIL: g++.old-deja/g++.law/except1.C  -std=gnu++98 (test for excess
>> errors)
>> FAIL: g++.old-deja/g++.mike/p700.C  -std=gnu++11 (test for excess errors)
>> FAIL: g++.old-deja/g++.mike/p700.C  -std=gnu++14 (test for excess errors)
>> FAIL: g++.old-deja/g++.mike/p700.C  -std=gnu++98 (test for excess errors)
>> FAIL: g++.old-deja/g++.other/builtins10.C  -std=c++11 (test for excess
>> errors)
>> FAIL: g++.old-deja/g++.other/builtins10.C  -std=c++14 (test for excess
>> errors)
>> FAIL: g++.old-deja/g++.other/realloc.C  -std=c++11 (test for excess
>> errors)
>> FAIL: g++.old-deja/g++.other/realloc.C  -std=c++14 (test for excess
>> errors)
>> FAIL: g++.old-deja/g++.other/realloc.C  -std=c++98 (test for excess
>> errors)
>> FAIL: g++.old-deja/g++.other/vbase5.C  -std=c++11 (test for excess
>> errors)
>> FAIL: g++.old-deja/g++.other/vbase5.C  -std=c++14 (test for excess
>> errors)
>> FAIL: g++.old-deja/g++.other/vbase5.C  -std=c++98 (test for excess
>> errors)
>>
>>
>> The lto test case does emit the warning when assembling, but
>> it still produces an executable and even executes it.
>>
>> Also g++.dg/cpp1y/lambda-generic-udt.C, g++.dg/tc1/dr20.C
>> and g++.old-deja/g++.other/vbase5.C are execution tests.
>>
>> So I was wrong to assume these were all compile-only tests.
>>
>> I think that list should be fixable, if we decide to enable
>> the warning by default.
>
> Yes, either by fixing the prototypes or disabling the warning.
>

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.

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?

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.


Bernd.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch-testsuite.diff --]
[-- Type: text/x-patch; name="patch-testsuite.diff", Size: 6368 bytes --]

Index: gcc/testsuite/g++.dg/cpp1y/lambda-generic-udt.C
===================================================================
--- gcc/testsuite/g++.dg/cpp1y/lambda-generic-udt.C	(revision 241752)
+++ gcc/testsuite/g++.dg/cpp1y/lambda-generic-udt.C	(working copy)
@@ -14,7 +14,7 @@
   bool shadow = false;
 };
 
-extern "C" void printf(...);
+extern "C" int printf(const char*, ...);
 #define assert(e) if (e); else \
 		 printf ("%s:%d: !(%s)\n", __FILE__, __LINE__, #e), __builtin_abort ();
 
Index: gcc/testsuite/g++.dg/init/new15.C
===================================================================
--- gcc/testsuite/g++.dg/init/new15.C	(revision 241752)
+++ gcc/testsuite/g++.dg/init/new15.C	(working copy)
@@ -1,6 +1,6 @@
 // PR c++/9782
 
-extern "C" void printf(const char*, ...);
+extern "C" int printf(const char*, ...);
 
 template <int>
 struct A {
Index: gcc/testsuite/g++.dg/ipa/inline-1.C
===================================================================
--- gcc/testsuite/g++.dg/ipa/inline-1.C	(revision 241752)
+++ gcc/testsuite/g++.dg/ipa/inline-1.C	(working copy)
@@ -3,7 +3,7 @@
 /* { dg-add-options bind_pic_locally } */
 
 namespace std {
-  extern "C" void puts(const char *s);
+  extern "C" int puts(const char *s);
 }
 
 template <class T, class E> void
Index: gcc/testsuite/g++.dg/ipa/inline-2.C
===================================================================
--- gcc/testsuite/g++.dg/ipa/inline-2.C	(revision 241752)
+++ gcc/testsuite/g++.dg/ipa/inline-2.C	(working copy)
@@ -3,7 +3,7 @@
 /* { dg-add-options bind_pic_locally } */
 
 namespace std {
-  extern "C" void puts(const char *s);
+  extern "C" int puts(const char *s);
 }
 
 template <class T, class E> void
Index: gcc/testsuite/g++.dg/lto/20080908-1_0.C
===================================================================
--- gcc/testsuite/g++.dg/lto/20080908-1_0.C	(revision 241752)
+++ gcc/testsuite/g++.dg/lto/20080908-1_0.C	(working copy)
@@ -1,5 +1,5 @@
 /* { dg-lto-do run }  */
-extern "C" { extern void *memcpy (void *, const void *, unsigned); }
+extern "C" { extern void *memcpy (void *, const void *, __SIZE_TYPE__); }
 
 inline int
 bci (const float &source)
Index: gcc/testsuite/g++.dg/lto/pr68811_0.C
===================================================================
--- gcc/testsuite/g++.dg/lto/pr68811_0.C	(revision 241752)
+++ 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" } */
 // { dg-extra-ld-options "-r -nostdlib" }
 extern "C" char *strcpy(char *, const char *);
 char InitXPCOMGlue_lastSlash;
Index: gcc/testsuite/g++.dg/tc1/dr20.C
===================================================================
--- gcc/testsuite/g++.dg/tc1/dr20.C	(revision 241752)
+++ gcc/testsuite/g++.dg/tc1/dr20.C	(working copy)
@@ -2,7 +2,7 @@
 // Origin: Giovanni Bajo <giovannibajo at gcc dot gnu dot org>
 // DR20: Some clarifications needed for 12.8 para 15 
 
-extern "C" void printf(const char*, ...);
+extern "C" int printf(const char*, ...);
 extern "C" void abort(void);
 
 int count = 0;
Index: gcc/testsuite/g++.dg/tree-ssa/inline-1.C
===================================================================
--- gcc/testsuite/g++.dg/tree-ssa/inline-1.C	(revision 241752)
+++ gcc/testsuite/g++.dg/tree-ssa/inline-1.C	(working copy)
@@ -3,7 +3,7 @@
 /* { dg-add-options bind_pic_locally } */
 
 namespace std {
-  extern "C" void puts(const char *s);
+  extern "C" int puts(const char *s);
 }
 
 template <class T, class E> void
Index: gcc/testsuite/g++.dg/tree-ssa/inline-2.C
===================================================================
--- gcc/testsuite/g++.dg/tree-ssa/inline-2.C	(revision 241752)
+++ gcc/testsuite/g++.dg/tree-ssa/inline-2.C	(working copy)
@@ -3,7 +3,7 @@
 /* { dg-add-options bind_pic_locally } */
 
 namespace std {
-  extern "C" void puts(const char *s);
+  extern "C" int puts(const char *s);
 }
 
 template <class T, class E> void
Index: gcc/testsuite/g++.old-deja/g++.law/except1.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.law/except1.C	(revision 241752)
+++ gcc/testsuite/g++.old-deja/g++.law/except1.C	(working copy)
@@ -7,7 +7,7 @@
 // Subject: Bugs
 // Date: Wed, 22 Jul 92 08:29:30 EDT
 
-extern "C" void puts(const char *);
+extern "C" int puts(const char *);
 
 class foo {
 public:
Index: gcc/testsuite/g++.old-deja/g++.mike/p700.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.mike/p700.C	(revision 241752)
+++ 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/realloc.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.other/realloc.C	(revision 241752)
+++ 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/testsuite/g++.old-deja/g++.other/vbase5.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.other/vbase5.C	(revision 241752)
+++ gcc/testsuite/g++.old-deja/g++.other/vbase5.C	(working copy)
@@ -6,7 +6,7 @@
 // vbases. Normally that's just a pessimization, unfortunately during
 // constructoring it leads to uninitialized reads.
 
-extern "C" int printf (...);
+extern "C" int printf (const char*,...);
 
 int fail = 0;
 
Index: gcc/testsuite/obj-c++.dg/lto/trivial-1_0.mm
===================================================================
--- gcc/testsuite/obj-c++.dg/lto/trivial-1_0.mm	(revision 241752)
+++ gcc/testsuite/obj-c++.dg/lto/trivial-1_0.mm	(working copy)
@@ -1,7 +1,7 @@
 /* { dg-lto-do run } */
 /* { dg-skip-if "Needs OBJC2 ABI" { "*-*-darwin*" && lp64 } { "*" } { "" } } */
 extern "C" {
-extern int printf (char *,...) ;
+extern int printf (const char *,...) ;
 extern void abort (void) ;
 } 
 

Re: [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Jason Merrill]

On Wed, Nov 2, 2016 at 5:15 PM, Bernd Edlinger
<bernd.edlinger@hotmail.de> wrote:
> On 11/02/16 18:51, Jason Merrill wrote:
>> On 11/02/2016 02:11 AM, Bernd Edlinger wrote:
>>> On 11/01/16 19:15, Bernd Edlinger wrote:
>>>> On 11/01/16 18:11, Jason Merrill wrote:
>>>>> On Tue, Nov 1, 2016 at 11:45 AM, Bernd Edlinger
>>>>> <bernd.edlinger@hotmail.de> wrote:
>>>>>> On 11/01/16 16:20, Jason Merrill wrote:
>>>>>>> On 10/17/2016 03:18 PM, Bernd Edlinger wrote:
>>>>>>> I'm not even sure we need a new warning.  Can we combine this warning
>>>>>>> with the block that currently follows?
>>>>>>
>>>>>> After 20 years of not having a warning on that,
>>>>>> an implicitly enabled warning would at least break lots of bogus
>>>>>> test cases.
>>>>>
>>>>> Would it, though?  Which test cases still break with the current patch?
>>>>
>>>> Less than before, but there are still at least a few of them.
>>>>
>>>> I can make a list and send it tomorrow.
>>>
>>> FAIL: g++.dg/cpp1y/lambda-generic-udt.C  -std=gnu++14 (test for excess
>>> errors)
>>> FAIL: g++.dg/cpp1y/lambda-generic-xudt.C  -std=gnu++14 (test for excess
>>> errors)
>>> FAIL: g++.dg/init/new15.C  -std=c++11 (test for excess errors)
>>> FAIL: g++.dg/init/new15.C  -std=c++14 (test for excess errors)
>>> FAIL: g++.dg/init/new15.C  -std=c++98 (test for excess errors)
>>> FAIL: g++.dg/ipa/inline-1.C  -std=gnu++11 (test for excess errors)
>>> FAIL: g++.dg/ipa/inline-1.C  -std=gnu++14 (test for excess errors)
>>> FAIL: g++.dg/ipa/inline-1.C  -std=gnu++98 (test for excess errors)
>>> FAIL: g++.dg/ipa/inline-2.C  -std=gnu++11 (test for excess errors)
>>> FAIL: g++.dg/ipa/inline-2.C  -std=gnu++14 (test for excess errors)
>>> FAIL: g++.dg/ipa/inline-2.C  -std=gnu++98 (test for excess errors)
>>> FAIL: g++.dg/tc1/dr20.C  -std=c++11 (test for excess errors)
>>> FAIL: g++.dg/tc1/dr20.C  -std=c++14 (test for excess errors)
>>> FAIL: g++.dg/tc1/dr20.C  -std=c++98 (test for excess errors)
>>> FAIL: g++.dg/tree-ssa/inline-1.C  -std=gnu++11 (test for excess errors)
>>> FAIL: g++.dg/tree-ssa/inline-1.C  -std=gnu++14 (test for excess errors)
>>> FAIL: g++.dg/tree-ssa/inline-1.C  -std=gnu++98 (test for excess errors)
>>> FAIL: g++.dg/tree-ssa/inline-2.C  -std=gnu++11 (test for excess errors)
>>> FAIL: g++.dg/tree-ssa/inline-2.C  -std=gnu++14 (test for excess errors)
>>> FAIL: g++.dg/tree-ssa/inline-2.C  -std=gnu++98 (test for excess errors)
>>> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O0 -flto
>>> -flto-partition=1to1 -fno-use-linker-plugin
>>> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O0 -flto
>>> -flto-partition=none -fuse-linker-plugin
>>> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O0 -flto
>>> -fuse-linker-plugin -fno-fat-lto-objects
>>> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O2 -flto
>>> -flto-partition=1to1 -fno-use-linker-plugin
>>> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O2 -flto
>>> -flto-partition=none -fuse-linker-plugin -fno-fat-lto-objects
>>> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O2 -flto
>>> -fuse-linker-plugin
>>> FAIL: g++.dg/lto/pr68811 cp_lto_pr68811_0.o assemble, -O2
>>> FAIL: g++.old-deja/g++.law/except1.C  -std=gnu++11 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.law/except1.C  -std=gnu++14 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.law/except1.C  -std=gnu++98 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.mike/p700.C  -std=gnu++11 (test for excess errors)
>>> FAIL: g++.old-deja/g++.mike/p700.C  -std=gnu++14 (test for excess errors)
>>> FAIL: g++.old-deja/g++.mike/p700.C  -std=gnu++98 (test for excess errors)
>>> FAIL: g++.old-deja/g++.other/builtins10.C  -std=c++11 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.other/builtins10.C  -std=c++14 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.other/realloc.C  -std=c++11 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.other/realloc.C  -std=c++14 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.other/realloc.C  -std=c++98 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.other/vbase5.C  -std=c++11 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.other/vbase5.C  -std=c++14 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.other/vbase5.C  -std=c++98 (test for excess
>>> errors)
>>>
>>>
>>> The lto test case does emit the warning when assembling, but
>>> it still produces an executable and even executes it.
>>>
>>> Also g++.dg/cpp1y/lambda-generic-udt.C, g++.dg/tc1/dr20.C
>>> and g++.old-deja/g++.other/vbase5.C are execution tests.
>>>
>>> So I was wrong to assume these were all compile-only tests.
>>>
>>> I think that list should be fixable, if we decide to enable
>>> the warning by default.
>>
>> Yes, either by fixing the prototypes or disabling the warning.
>>
>
> 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.
>
> 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.

> 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!

Jason

Re: [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Bernd Edlinger]

[-- Attachment #1: Type: text/plain, Size: 2523 bytes --]

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 #2: changelog-pr71973.txt --]
[-- Type: text/plain, Size: 1694 bytes --]

gcc:
2016-11-05  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c++/71973
	* builtin-types.def (BT_CONST_TM_PTR): New primitive type.
	(BT_PTR_CONST_STRING): Updated.
	(BT_FN_SIZE_STRING_SIZE_CONST_STRING_CONST_PTR): Removed.
	(BT_FN_SIZE_STRING_SIZE_CONST_STRING_CONST_TM_PTR): New function type.
	* builtins.def (DEF_TM_BUILTIN): Disable BOTH_P for TM builtins.
	(strftime): Update builtin function.
	* tree-core.h (TI_CONST_TM_PTR_TYPE): New enum value.
	* tree.h (const_tm_ptr_type_node): New type node.
	* tree.c (free_lang_data, build_common_tree_nodes): Initialize
	const_tm_ptr_type_node.

c-family:
2016-11-05  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c++/71973
	* c-common.c (c_common_nodes_and_builtins): Initialize
	const_tm_ptr_type_node.

cp:
2016-11-05  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c++/71973
	* decl.c (duplicate_decls): Warn when a built-in function is redefined.
	Don't overload builtin functions with C++ functions.
	Handle const_tm_ptr_type_node like file_ptr_node.
	Copy the TREE_NOTHROW flag unmodified to the old decl.

lto:
2016-11-05  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c++/71973
	* lto-lang.c (lto_init): Assert const_tm_ptr_type_node is sane.

testsuite:
2016-11-05  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c++/71973
	* g++.dg/pr71973-1.C: New test.
	* g++.dg/pr71973-2.C: New test.
	* g++.dg/pr71973-3.C: New test.
	* g++.dg/lto/pr68811_0.C: Add -w to first lto-options.
	* g++.dg/lookup/extern-c-redecl4.C: Adjust test expectations.
	* g++.old-deja/g++.mike/p700.C: Add -w to dg-options.
	* g++.old-deja/g++.other/realloc.C: Add -w to dg-options.
	* g++.old-deja/g++.other/builtins10.C: Adjust test expectation.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: patch-pr71973.diff --]
[-- Type: text/x-patch; name="patch-pr71973.diff", Size: 14162 bytes --]

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]

[PING] [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Bernd Edlinger]

Ping...

the latest version of the patch was here:
https://gcc.gnu.org/ml/gcc-patches/2016-11/msg00505.html

Thanks
Bernd.

On 11/02/16 22:15, Bernd Edlinger wrote:
> On 11/02/16 18:51, Jason Merrill wrote:
>> On 11/02/2016 02:11 AM, Bernd Edlinger wrote:
>>> On 11/01/16 19:15, Bernd Edlinger wrote:
>>>> On 11/01/16 18:11, Jason Merrill wrote:
>>>>> On Tue, Nov 1, 2016 at 11:45 AM, Bernd Edlinger
>>>>> <bernd.edlinger@hotmail.de> wrote:
>>>>>> On 11/01/16 16:20, Jason Merrill wrote:
>>>>>>> On 10/17/2016 03:18 PM, Bernd Edlinger wrote:
>>>>>>> I'm not even sure we need a new warning.  Can we combine this
>>>>>>> warning
>>>>>>> with the block that currently follows?
>>>>>>
>>>>>> After 20 years of not having a warning on that,
>>>>>> an implicitly enabled warning would at least break lots of bogus
>>>>>> test cases.
>>>>>
>>>>> Would it, though?  Which test cases still break with the current
>>>>> patch?
>>>>
>>>> Less than before, but there are still at least a few of them.
>>>>
>>>> I can make a list and send it tomorrow.
>>>
>>> FAIL: g++.dg/cpp1y/lambda-generic-udt.C  -std=gnu++14 (test for excess
>>> errors)
>>> FAIL: g++.dg/cpp1y/lambda-generic-xudt.C  -std=gnu++14 (test for excess
>>> errors)
>>> FAIL: g++.dg/init/new15.C  -std=c++11 (test for excess errors)
>>> FAIL: g++.dg/init/new15.C  -std=c++14 (test for excess errors)
>>> FAIL: g++.dg/init/new15.C  -std=c++98 (test for excess errors)
>>> FAIL: g++.dg/ipa/inline-1.C  -std=gnu++11 (test for excess errors)
>>> FAIL: g++.dg/ipa/inline-1.C  -std=gnu++14 (test for excess errors)
>>> FAIL: g++.dg/ipa/inline-1.C  -std=gnu++98 (test for excess errors)
>>> FAIL: g++.dg/ipa/inline-2.C  -std=gnu++11 (test for excess errors)
>>> FAIL: g++.dg/ipa/inline-2.C  -std=gnu++14 (test for excess errors)
>>> FAIL: g++.dg/ipa/inline-2.C  -std=gnu++98 (test for excess errors)
>>> FAIL: g++.dg/tc1/dr20.C  -std=c++11 (test for excess errors)
>>> FAIL: g++.dg/tc1/dr20.C  -std=c++14 (test for excess errors)
>>> FAIL: g++.dg/tc1/dr20.C  -std=c++98 (test for excess errors)
>>> FAIL: g++.dg/tree-ssa/inline-1.C  -std=gnu++11 (test for excess errors)
>>> FAIL: g++.dg/tree-ssa/inline-1.C  -std=gnu++14 (test for excess errors)
>>> FAIL: g++.dg/tree-ssa/inline-1.C  -std=gnu++98 (test for excess errors)
>>> FAIL: g++.dg/tree-ssa/inline-2.C  -std=gnu++11 (test for excess errors)
>>> FAIL: g++.dg/tree-ssa/inline-2.C  -std=gnu++14 (test for excess errors)
>>> FAIL: g++.dg/tree-ssa/inline-2.C  -std=gnu++98 (test for excess errors)
>>> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O0 -flto
>>> -flto-partition=1to1 -fno-use-linker-plugin
>>> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O0 -flto
>>> -flto-partition=none -fuse-linker-plugin
>>> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O0 -flto
>>> -fuse-linker-plugin -fno-fat-lto-objects
>>> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O2 -flto
>>> -flto-partition=1to1 -fno-use-linker-plugin
>>> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O2 -flto
>>> -flto-partition=none -fuse-linker-plugin -fno-fat-lto-objects
>>> FAIL: g++.dg/lto/20080908-1 cp_lto_20080908-1_0.o assemble, -O2 -flto
>>> -fuse-linker-plugin
>>> FAIL: g++.dg/lto/pr68811 cp_lto_pr68811_0.o assemble, -O2
>>> FAIL: g++.old-deja/g++.law/except1.C  -std=gnu++11 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.law/except1.C  -std=gnu++14 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.law/except1.C  -std=gnu++98 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.mike/p700.C  -std=gnu++11 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.mike/p700.C  -std=gnu++14 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.mike/p700.C  -std=gnu++98 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.other/builtins10.C  -std=c++11 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.other/builtins10.C  -std=c++14 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.other/realloc.C  -std=c++11 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.other/realloc.C  -std=c++14 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.other/realloc.C  -std=c++98 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.other/vbase5.C  -std=c++11 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.other/vbase5.C  -std=c++14 (test for excess
>>> errors)
>>> FAIL: g++.old-deja/g++.other/vbase5.C  -std=c++98 (test for excess
>>> errors)
>>>
>>>
>>> The lto test case does emit the warning when assembling, but
>>> it still produces an executable and even executes it.
>>>
>>> Also g++.dg/cpp1y/lambda-generic-udt.C, g++.dg/tc1/dr20.C
>>> and g++.old-deja/g++.other/vbase5.C are execution tests.
>>>
>>> So I was wrong to assume these were all compile-only tests.
>>>
>>> I think that list should be fixable, if we decide to enable
>>> the warning by default.
>>
>> Yes, either by fixing the prototypes or disabling the warning.
>>
>
> 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.
>
> 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?
>
> 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.
>
>
> Bernd.

Re: [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Jason Merrill]

On 11/05/2016 12:44 PM, Bernd Edlinger wrote:
> +	      warning_at (DECL_SOURCE_LOCATION (newdecl), 0,
> +			  "declaration of %q+#D conflicts with built-in "
> +			  "declaration %q#D", newdecl, olddecl);

There needs to be a way to disable this warning, even if it's enabled by 
default.

> -      TREE_NOTHROW (olddecl) = 0;
> +      TREE_NOTHROW (olddecl) = TREE_NOTHROW (newdecl);

I still think a better fix would be to add a copy of TREE_NOTHROW to the 
else block of the if (types_match), to go with the existing copies of 
TREE_READONLY and TREE_THIS_VOLATILE.

Jason

Re: [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Bernd Edlinger]

On 11/18/16 22:19, Jason Merrill wrote:
> On 11/05/2016 12:44 PM, Bernd Edlinger wrote:
>> +          warning_at (DECL_SOURCE_LOCATION (newdecl), 0,
>> +              "declaration of %q+#D conflicts with built-in "
>> +              "declaration %q#D", newdecl, olddecl);
>
> There needs to be a way to disable this warning, even if it's enabled by
> default.
>

It is mostly disabled by -Wno-system-headers.
So it will only warn for declarations in .cc files.

Then I will probably need a better name for it.
I'd like -Wbuiltin-declaration-mismatch for instance?

I wonder if that should also control the C warning, and the case where
the internal __builtin_ is declared differently.  It is somehow
hard to explain why it has to be a C++ only warning, except for
historical reasons.


>> -      TREE_NOTHROW (olddecl) = 0;
>> +      TREE_NOTHROW (olddecl) = TREE_NOTHROW (newdecl);
>
> I still think a better fix would be to add a copy of TREE_NOTHROW to the
> else block of the if (types_match), to go with the existing copies of
> TREE_READONLY and TREE_THIS_VOLATILE.
>

I can give it a try, but I don't understand why that does apparently
not give us problems with other types of declaration mismatches.

It would be good to have a test case where a redeclaration with a
non-builtin causes similar slightly-wrong-code issues.

Possible that some hidden issues exist already, but also possible
that I create new issues, if I don't understand the code completely.

Is the else block ever used for non-builtin functions?
Can variable declarations end up there?
What other types of objects are possible there?

Given that stage1 is already over, should I split the warning
part from the wrong code issue?

What do you think?


Thanks
Bernd.

Re: [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Bernd Edlinger]

On 11/18/16 23:15, Bernd Edlinger wrote:
>>> -      TREE_NOTHROW (olddecl) = 0;
>>> +      TREE_NOTHROW (olddecl) = TREE_NOTHROW (newdecl);
>>
>> I still think a better fix would be to add a copy of TREE_NOTHROW to the
>> else block of the if (types_match), to go with the existing copies of
>> TREE_READONLY and TREE_THIS_VOLATILE.
>>

Hmm, looking at it again, I start to think you are right.

It looks like it is *not* possible that something other than
a builtin function can reach the bottom of that function with
!types_match.

The upper half of duplicate_decls is structured like this:

   /* Check for redeclaration and other discrepancies.  */
   if (TREE_CODE (olddecl) == FUNCTION_DECL
       && DECL_ARTIFICIAL (olddecl))
     {
         if (TREE_CODE (newdecl) != FUNCTION_DECL)
           {
             ....
             return NULL_TREE;
           }
         ....

         TREE_NOTHROW (olddecl) = 0;
         ....
     }
   else if (TREE_CODE (olddecl) != TREE_CODE (newdecl))
     {
         ....
         return error_mark_node;
     }
   else if (!types_match)
     {
         ...
         if (TREE_CODE (newdecl) == TEMPLATE_DECL)
           {
             ....
             return NULL_TREE;
           }
         if (TREE_CODE (newdecl) == FUNCTION_DECL)
           {
              if (DECL_EXTERN_C_P (newdecl) && DECL_EXTERN_C_P (olddecl))
                {
                   ...
                   return NULL_TREE;
                }
              else if (...)
                {
                    ....
                    return error_mark_node;
                }
              else
                return NULL_TREE;
            }
          else
            {
              ...
              return error_mark_node;
            }
      }
    else if (....)
      ....


So in the case types_match=true, it can neither be that olddecl
is something other than a builtin function decl,
nor can newdecl be something other than a function decl.
Everything else makes the function return already here.


Bernd.

[PATCHv2, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Bernd Edlinger]

[-- Attachment #1: Type: text/plain, Size: 1901 bytes --]

Hi,

On 11/18/16 22:19, Jason Merrill wrote:
> On 11/05/2016 12:44 PM, Bernd Edlinger wrote:
>> +          warning_at (DECL_SOURCE_LOCATION (newdecl), 0,
>> +              "declaration of %q+#D conflicts with built-in "
>> +              "declaration %q#D", newdecl, olddecl);
>
> There needs to be a way to disable this warning, even if it's enabled by
> default.
>
>> -      TREE_NOTHROW (olddecl) = 0;
>> +      TREE_NOTHROW (olddecl) = TREE_NOTHROW (newdecl);
>
> I still think a better fix would be to add a copy of TREE_NOTHROW to the
> else block of the if (types_match), to go with the existing copies of
> TREE_READONLY and TREE_THIS_VOLATILE.
>


I changed the patch as requested.  I think meanwhile that this else
block does only handle a built-in function with different signature.

I have now changed also the C front end to emit the warning with the new
default-enabled -Wbuiltin-declaration-mismatch.

When I looked at the c-decl.c code, I saw there are effectively two
warnings, one for a function that does not match the builtin signature,
and another for a global variable that has the the name of a built-in.

like:
test.c
int printf;

gives warning:
test.c:1:5: warning: built-in function 'printf' declared as non-function
  int printf;


the same in C++ gives nothing.

That is because of this in duplicate_decls:

       if (TREE_CODE (newdecl) != FUNCTION_DECL)
         {
           /* Avoid warnings redeclaring built-ins which have not been
              explicitly declared.  */
           if (DECL_ANTICIPATED (olddecl))
             return NULL_TREE;

That is from gcc3.1 in 2002 a rather radical patch I would say.

I left the warning for declaring a variable with the name of built-in
function for next time, maybe...


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


Thanks
Bernd.

[-- Attachment #2: changelog-pr71973.txt --]
[-- Type: text/plain, Size: 2015 bytes --]

gcc:
2016-11-19  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c++/71973
	* doc/invoke.texi (-Wno-builtin-declaration-mismatch): Document the
	new default-enabled warning..
	* builtin-types.def (BT_CONST_TM_PTR): New primitive type.
	(BT_PTR_CONST_STRING): Updated.
	(BT_FN_SIZE_STRING_SIZE_CONST_STRING_CONST_PTR): Removed.
	(BT_FN_SIZE_STRING_SIZE_CONST_STRING_CONST_TM_PTR): New function type.
	* builtins.def (DEF_TM_BUILTIN): Disable BOTH_P for TM builtins.
	(strftime): Update builtin function.
	* tree-core.h (TI_CONST_TM_PTR_TYPE): New enum value.
	* tree.h (const_tm_ptr_type_node): New type node.
	* tree.c (free_lang_data, build_common_tree_nodes): Initialize
	const_tm_ptr_type_node.

c-family:
2016-11-19  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c++/71973
	* c.opt (-Wbuiltin-declaration-mismatch): New warning.
	* c-common.c (c_common_nodes_and_builtins): Initialize
	const_tm_ptr_type_node.

c:
2016-11-19  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c++/71973
	* c-decl.c (diagnose_mismatched_decls): Use
	OPT_Wbuiltin_declaration_mismatch here too.

cp:
2016-11-19  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c++/71973
	* decl.c (duplicate_decls): Warn when a built-in function is redefined.
	Don't overload builtin functions with C++ functions.
	Handle const_tm_ptr_type_node like file_ptr_node.
	Copy the TREE_NOTHROW flag unmodified to the old decl.

lto:
2016-11-05  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c++/71973
	* lto-lang.c (lto_init): Assert const_tm_ptr_type_node is sane.

testsuite:
2016-11-05  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c++/71973
	* g++.dg/pr71973-1.C: New test.
	* g++.dg/pr71973-2.C: New test.
	* g++.dg/pr71973-3.C: New test.
	* g++.dg/lto/pr68811_0.C: Add -w to first lto-options.
	* g++.dg/lookup/extern-c-redecl4.C: Adjust test expectations.
	* g++.old-deja/g++.mike/p700.C: Add -w to dg-options.
	* g++.old-deja/g++.other/realloc.C: Add -w to dg-options.
	* g++.old-deja/g++.other/builtins10.C: Adjust test expectation.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: patch-pr71973.diff --]
[-- Type: text/x-patch; name="patch-pr71973.diff", Size: 17384 bytes --]

Index: gcc/builtin-types.def
===================================================================
--- gcc/builtin-types.def	(revision 242620)
+++ 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 242620)
+++ 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/c-decl.c
===================================================================
--- gcc/c/c-decl.c	(revision 242620)
+++ gcc/c/c-decl.c	(working copy)
@@ -1867,7 +1867,8 @@ 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 (0, "conflicting types for built-in function %q+D",
+	      warning (OPT_Wbuiltin_declaration_mismatch,
+		       "conflicting types for built-in function %q+D",
 		       newdecl);
 	      return false;
 	    }
Index: gcc/c-family/c-common.c
===================================================================
--- gcc/c-family/c-common.c	(revision 242620)
+++ gcc/c-family/c-common.c	(working copy)
@@ -4293,9 +4293,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/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt	(revision 242620)
+++ gcc/c-family/c.opt	(working copy)
@@ -337,6 +337,10 @@ Wframe-address
 C ObjC C++ ObjC++ Var(warn_frame_address) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
 Warn when __builtin_frame_address or __builtin_return_address is used unsafely.
 
+Wbuiltin-declaration-mismatch
+C ObjC C++ ObjC++ Var(warn_builtin_declaraion_mismatch) Init(1) Warning
+Warn when a built-in function is declared with the wrong signature.
+
 Wbuiltin-macro-redefined
 C ObjC C++ ObjC++ CPP(warn_builtin_macro_redefined) CppReason(CPP_W_BUILTIN_MACRO_REDEFINED) Var(cpp_warn_builtin_macro_redefined) Init(1) Warning
 Warn when a built-in preprocessor macro is undefined or redefined.
Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c	(revision 242620)
+++ gcc/cp/decl.c	(working copy)
@@ -1490,10 +1490,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;
@@ -1500,6 +1505,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);
@@ -1520,8 +1527,34 @@ 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),
+			  OPT_Wbuiltin_declaration_mismatch,
+			  "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))
@@ -1531,7 +1564,8 @@ duplicate_decls (tree newdecl, tree olddecl, bool
 	      /* A near match; override the builtin.  */
 
 	      if (TREE_PUBLIC (newdecl))
-		warning_at (DECL_SOURCE_LOCATION (newdecl), 0,
+		warning_at (DECL_SOURCE_LOCATION (newdecl),
+			    OPT_Wbuiltin_declaration_mismatch,
 			    "new declaration %q#D ambiguates built-in "
 			    "declaration %q#D", newdecl, olddecl);
 	      else
@@ -2243,6 +2277,7 @@ duplicate_decls (tree newdecl, tree olddecl, bool
       TREE_TYPE (olddecl) = TREE_TYPE (newdecl);
       TREE_READONLY (olddecl) = TREE_READONLY (newdecl);
       TREE_THIS_VOLATILE (olddecl) = TREE_THIS_VOLATILE (newdecl);
+      TREE_NOTHROW (olddecl) = TREE_NOTHROW (newdecl);
       TREE_SIDE_EFFECTS (olddecl) = TREE_SIDE_EFFECTS (newdecl);
     }
 
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 242620)
+++ gcc/doc/invoke.texi	(working copy)
@@ -259,6 +259,7 @@ Objective-C and Objective-C++ Dialects}.
 -Walloca -Walloca-larger-than=@var{n} @gol
 -Wno-aggressive-loop-optimizations -Warray-bounds -Warray-bounds=@var{n} @gol
 -Wno-attributes -Wbool-compare -Wbool-operation @gol
+-Wno-builtin-declaration-mismatch @gol
 -Wno-builtin-macro-redefined -Wc90-c99-compat -Wc99-c11-compat @gol
 -Wc++-compat -Wc++11-compat -Wc++14-compat -Wcast-align  -Wcast-qual  @gol
 -Wchar-subscripts -Wclobbered  -Wcomment -Wconditionally-supported  @gol
@@ -5925,6 +5926,12 @@ unrecognized attributes, function attributes appli
 etc.  This does not stop errors for incorrect use of supported
 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.
+This warning is enabled by default.
+
 @item -Wno-builtin-macro-redefined
 @opindex Wno-builtin-macro-redefined
 @opindex Wbuiltin-macro-redefined
Index: gcc/lto/lto-lang.c
===================================================================
--- gcc/lto/lto-lang.c	(revision 242620)
+++ 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 242620)
+++ 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 242620)
+++ 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 242620)
+++ 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 "-Wno-deprecated -Wno-register -Wno-builtin-declaration-mismatch" }
 // { 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 242620)
+++ 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 242620)
+++ gcc/testsuite/g++.old-deja/g++.other/realloc.C	(working copy)
@@ -1,4 +1,5 @@
 // { dg-do assemble  }
+// { dg-options "-Wno-builtin-declaration-mismatch" }
 
 extern "C" void realloc();
 
Index: gcc/tree-core.h
===================================================================
--- gcc/tree-core.h	(revision 242620)
+++ 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 242620)
+++ 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 242620)
+++ gcc/tree.h	(working copy)
@@ -3672,6 +3672,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]

Re: [PATCHv2, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Jason Merrill]

OK.

On Sat, Nov 19, 2016 at 6:11 AM, Bernd Edlinger
<bernd.edlinger@hotmail.de> wrote:
> Hi,
>
> On 11/18/16 22:19, Jason Merrill wrote:
>> On 11/05/2016 12:44 PM, Bernd Edlinger wrote:
>>> +          warning_at (DECL_SOURCE_LOCATION (newdecl), 0,
>>> +              "declaration of %q+#D conflicts with built-in "
>>> +              "declaration %q#D", newdecl, olddecl);
>>
>> There needs to be a way to disable this warning, even if it's enabled by
>> default.
>>
>>> -      TREE_NOTHROW (olddecl) = 0;
>>> +      TREE_NOTHROW (olddecl) = TREE_NOTHROW (newdecl);
>>
>> I still think a better fix would be to add a copy of TREE_NOTHROW to the
>> else block of the if (types_match), to go with the existing copies of
>> TREE_READONLY and TREE_THIS_VOLATILE.
>>
>
>
> I changed the patch as requested.  I think meanwhile that this else
> block does only handle a built-in function with different signature.
>
> I have now changed also the C front end to emit the warning with the new
> default-enabled -Wbuiltin-declaration-mismatch.
>
> When I looked at the c-decl.c code, I saw there are effectively two
> warnings, one for a function that does not match the builtin signature,
> and another for a global variable that has the the name of a built-in.
>
> like:
> test.c
> int printf;
>
> gives warning:
> test.c:1:5: warning: built-in function 'printf' declared as non-function
>   int printf;
>
>
> the same in C++ gives nothing.
>
> That is because of this in duplicate_decls:
>
>        if (TREE_CODE (newdecl) != FUNCTION_DECL)
>          {
>            /* Avoid warnings redeclaring built-ins which have not been
>               explicitly declared.  */
>            if (DECL_ANTICIPATED (olddecl))
>              return NULL_TREE;
>
> That is from gcc3.1 in 2002 a rather radical patch I would say.
>
> I left the warning for declaring a variable with the name of built-in
> function for next time, maybe...
>
>
> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?
>
>
> Thanks
> Bernd.

Re: [PATCHv2, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Jakub Jelinek]

On Sat, Nov 19, 2016 at 11:11:18AM +0000, Bernd Edlinger wrote:
> 2016-11-19  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> 
> 	PR c++/71973
> 	* doc/invoke.texi (-Wno-builtin-declaration-mismatch): Document the
> 	new default-enabled warning..
> 	* builtin-types.def (BT_CONST_TM_PTR): New primitive type.
> 	(BT_PTR_CONST_STRING): Updated.
> 	(BT_FN_SIZE_STRING_SIZE_CONST_STRING_CONST_PTR): Removed.
> 	(BT_FN_SIZE_STRING_SIZE_CONST_STRING_CONST_TM_PTR): New function type.
> 	* builtins.def (DEF_TM_BUILTIN): Disable BOTH_P for TM builtins.
> 	(strftime): Update builtin function.
> 	* tree-core.h (TI_CONST_TM_PTR_TYPE): New enum value.
> 	* tree.h (const_tm_ptr_type_node): New type node.
> 	* tree.c (free_lang_data, build_common_tree_nodes): Initialize
> 	const_tm_ptr_type_node.
...

This broke 2 tests on i686-linux, I've committed this as obvious to fix it:

2016-11-21  Jakub Jelinek  <jakub@redhat.com>

	PR c++/71973
	* g++.dg/torture/pr53321.C (size_t): Use __SIZE_TYPE__ instead of
	long unsigned int.
	* g++.dg/torture/pr63512.C (::strlen): Use __SIZE_TYPE__ instead of
	unsigned long.

--- gcc/testsuite/g++.dg/torture/pr53321.C.jj	2012-07-16 14:38:22.514585151 +0200
+++ gcc/testsuite/g++.dg/torture/pr53321.C	2016-11-21 19:52:00.561899801 +0100
@@ -2,7 +2,7 @@
 // { dg-require-profiling "-fprofile-generate" }
 // { dg-options "-fprofile-generate" }
 
-typedef long unsigned int size_t;
+typedef __SIZE_TYPE__ size_t;
 
 extern "C"
 {
--- gcc/testsuite/g++.dg/torture/pr63512.C.jj	2014-10-15 12:28:16.417303928 +0200
+++ gcc/testsuite/g++.dg/torture/pr63512.C	2016-11-21 19:52:45.006330942 +0100
@@ -2,7 +2,7 @@
 
 extern "C" {
 void __assert_fail ();
-unsigned long strlen (const char *);
+__SIZE_TYPE__ strlen (const char *);
 }
 class A
 {


	Jakub

Re: [PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Joseph Myers]

On Sat, 20 Aug 2016, Bernd Edlinger wrote:

> Initially I tried to warn unconditionally but that made too  many tests
> in the C++ testsuite emit that warning :-(

I think that's probably a peculiarity of testsuite code trying to avoid 
standard headers, and does not reflect what normal user C++ code does.

-- 
Joseph S. Myers
joseph@codesourcery.com

[PATCH, C++] Warn on redefinition of builtin functions (PR c++/71973)

[Bernd Edlinger]

[-- Attachment #1: Type: text/plain, Size: 741 bytes --]

Hi!

Currently C++ does not warn at all when built-in functions are re-defined
with a different signature, while C does warn on that even without -Wall.

Thus I'd like to propose a -Wall enabled warning for that in C++.

Initially I tried to warn unconditionally but that made too  many tests
in the C++ testsuite emit that warning :-(

So making the warning dependent on Wall is a compromise due
to the very many compile only tests, that use this "feature".

There is also a wrong-code side on this redefinition, because
even if the new function has the nothrow attribute the code is
generated as if it could throw.  Fixed as well.


Boot-strap and reg-testing on x86_64-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.

[-- Attachment #2: changelog-pr71973.txt --]
[-- Type: text/plain, Size: 700 bytes --]

gcc:
2016-08-20  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c++/71973
	* doc/invoke.texi: Document -Wbuiltin-function-redefined.

c-family:
2016-08-20  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c++/71973
	* c.opt (Wbuiltin-function-redefined): New warning.

cp:
2016-08-20  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c++/71973
	* decl.c (duplicate_decls): Warn when a built-in function is redefined.
	Copy the TREE_NOTHROW flag unmodified to the old decl.

testsuite:
2016-08-20  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	PR c++/71973
	* g++.dg/pr71973.C: New test.
	* g++.dg/warn/noeffect5.C: Add extern "C" to built-in function.
	* g++.old-deja/g++.other/warn01.C: Likewise.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: patch-pr71973.diff --]
[-- Type: text/x-patch; name="patch-pr71973.diff", Size: 4900 bytes --]

Index: gcc/c-family/c.opt
===================================================================
--- gcc/c-family/c.opt	(revision 239624)
+++ gcc/c-family/c.opt	(working copy)
@@ -299,6 +299,10 @@ Wframe-address
 C ObjC C++ ObjC++ Var(warn_frame_address) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall)
 Warn when __builtin_frame_address or __builtin_return_address is used unsafely.
 
+Wbuiltin-function-redefined
+C++ ObjC++ Var(warn_builtin_function_redefined) Warning LangEnabledBy(C++ ObjC++,Wall)
+Warn when a built-in function is redefined.
+
 Wbuiltin-macro-redefined
 C ObjC C++ ObjC++ CPP(warn_builtin_macro_redefined) CppReason(CPP_W_BUILTIN_MACRO_REDEFINED) Var(cpp_warn_builtin_macro_redefined) Init(1) Warning
 Warn when a built-in preprocessor macro is undefined or redefined.
Index: gcc/cp/decl.c
===================================================================
--- gcc/cp/decl.c	(revision 239624)
+++ gcc/cp/decl.c	(working copy)
@@ -1502,6 +1502,14 @@ duplicate_decls (tree newdecl, tree olddecl, bool
 		  }
 		else if (! same_type_p (TREE_VALUE (t1), TREE_VALUE (t2)))
 		  break;
+	      if (t1 || t2
+		  || DECL_LANGUAGE (newdecl) != DECL_LANGUAGE (olddecl)
+		  || ! same_type_p (TREE_TYPE (TREE_TYPE (olddecl)),
+				    TREE_TYPE (TREE_TYPE (newdecl))))
+		warning_at (DECL_SOURCE_LOCATION (newdecl),
+			    OPT_Wbuiltin_function_redefined,
+			    "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))
@@ -1555,7 +1563,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/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi	(revision 239624)
+++ gcc/doc/invoke.texi	(working copy)
@@ -256,8 +256,8 @@ Objective-C and Objective-C++ Dialects}.
 -pedantic-errors @gol
 -w  -Wextra  -Wall  -Waddress  -Waggregate-return  @gol
 -Wno-aggressive-loop-optimizations -Warray-bounds -Warray-bounds=@var{n} @gol
--Wno-attributes -Wbool-compare -Wno-builtin-macro-redefined @gol
--Wc90-c99-compat -Wc99-c11-compat @gol
+-Wno-attributes -Wbool-compare -Wbuiltin-function-redefined @gol
+-Wno-builtin-macro-redefined -Wc90-c99-compat -Wc99-c11-compat @gol
 -Wc++-compat -Wc++11-compat -Wc++14-compat -Wcast-align  -Wcast-qual  @gol
 -Wchar-subscripts -Wclobbered  -Wcomment -Wconditionally-supported  @gol
 -Wconversion -Wcoverage-mismatch -Wno-cpp -Wdangling-else -Wdate-time @gol
@@ -5460,6 +5460,13 @@ unrecognized attributes, function attributes appli
 etc.  This does not stop errors for incorrect use of supported
 attributes.
 
+@item -Wbuiltin-function-redefined @r{(C++ and Objective-C++ only)}
+@opindex Wbuiltin-function-redefined
+@opindex Wno-builtin-function-redefined
+Do warn if built-in functions are redefined.  This option is only
+supported for C++ and Objective-C++.  It is implied by @option{-Wall},
+which can be disabled with @option{-Wno-builtin-function-redefined}.
+
 @item -Wno-builtin-macro-redefined
 @opindex Wno-builtin-macro-redefined
 @opindex Wbuiltin-macro-redefined
Index: gcc/testsuite/g++.dg/pr71973.C
===================================================================
--- gcc/testsuite/g++.dg/pr71973.C	(revision 0)
+++ gcc/testsuite/g++.dg/pr71973.C	(working copy)
@@ -0,0 +1,14 @@
+// { dg-do compile }
+// { dg-options "-Wbuiltin-function-redefined -fdump-tree-eh" }
+
+extern "C"
+void fork () // { dg-warning "conflicts with built-in declaration" }
+__attribute__ ((__nothrow__));
+
+void bar () throw ()
+{
+  fork ();
+}
+
+// { dg-final { scan-tree-dump-not "eh_dispatch" "eh" } }
+// { dg-final { scan-tree-dump-not "resx" "eh" } }
Index: gcc/testsuite/g++.dg/warn/noeffect5.C
===================================================================
--- gcc/testsuite/g++.dg/warn/noeffect5.C	(revision 239624)
+++ gcc/testsuite/g++.dg/warn/noeffect5.C	(working copy)
@@ -2,6 +2,7 @@
 /* { dg-do compile } */
 /* { dg-options "-Wall" } */
 
+extern "C"
 void *memcpy(void *dest, const void *src, __SIZE_TYPE__ n);
 void f (void *dest, const void *src) {
     memcpy (dest, src, 0);
Index: gcc/testsuite/g++.old-deja/g++.other/warn01.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.other/warn01.C	(revision 239624)
+++ gcc/testsuite/g++.old-deja/g++.other/warn01.C	(working copy)
@@ -2,9 +2,11 @@
 // { dg-options "-W -Wall" }
 
 typedef unsigned long size_t;
+extern "C" {
 extern void* malloc (size_t);
 extern void free (void*);
 extern void* realloc (void*, size_t);
+}
 
 struct vtable {
   void* (* _malloc) (size_t);