[C++ PATCH] Don't redefine __* builtins (PR c++/84724, take 2)

[Jakub Jelinek <jakub at redhat dot com>]

On Thu, Mar 08, 2018 at 04:06:24PM -0500, Jason Merrill wrote:
> On Thu, Mar 8, 2018 at 1:01 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> > The C FE just warns and doesn't override builtins, but C++ FE
> > on say int __builtin_trap (); will override the builtin, so later
> > builtin_decl_explicit will return the bogus user function which e.g.
> > doesn't have any merged attributes, can have different arguments or
> > return type etc.
> >
> > This patch prevents that; generally the builtins we want to override
> > are the DECL_ANTICIPATED which we handle properly earlier.
> >
> > The testcase in the PR ICEs not because of the argument mismatch (there is
> > none in this case) or return value mismatch (because nothing cares about the
> > return value), but because the new decl lacks noreturn attribute and GCC
> > relies on builtin_decl_explicit (BUILT_IN_TRAP) to be really noreturn.
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> > Or shall we error on these, or ignore the name checks and just
> > if (DECL_BUILT_IN (olddecl)) return NULL_TREE;, something else?
> 
> Seems like returning NULL_TREE means that we overload the built-in,
> which also seems undesirable; what if we return olddecl unmodified?
> It would also be good to improve the diagnostic to say that we're
> ignoring the declaration.  Perhaps as a permerror.

So like this (if it passes bootstrap/regtest, though I think we don't have
any tests other than these new ones that cover it anyway, so I don't expect
issues)?

2018-03-09  Jakub Jelinek  <jakub@redhat.com>

	PR c++/84724
	* decl.c (duplicate_decls): Don't override __* prefixed builtins
	except for __[^b]*_chk, instead issue permerror and for -fpermissive
	also a note and return olddecl.

	* g++.dg/ext/pr84724.C: New test.

--- gcc/cp/decl.c.jj	2018-03-08 21:53:44.989559552 +0100
+++ gcc/cp/decl.c	2018-03-09 11:53:58.557156637 +0100
@@ -1566,6 +1566,33 @@ next_arg:;
 		   || compparms (TYPE_ARG_TYPES (TREE_TYPE (newdecl)),
 				 TYPE_ARG_TYPES (TREE_TYPE (olddecl))))
 	    {
+	      /* Don't really override olddecl for __* prefixed builtins
+		 except for __[^b]*_chk, the compiler might be using those
+		 explicitly.  */
+	      if (DECL_BUILT_IN (olddecl))
+		{
+		  tree id = DECL_NAME (olddecl);
+		  const char *name = IDENTIFIER_POINTER (id);
+		  size_t len;
+
+		  if (name[0] == '_'
+		      && name[1] == '_'
+		      && (strncmp (name + 2, "builtin_",
+				   strlen ("builtin_")) == 0
+			  || (len = strlen (name)) <= strlen ("___chk")
+			  || memcmp (name + len - strlen ("_chk"),
+				     "_chk", strlen ("_chk") + 1) != 0))
+		    {
+		      if (permerror (DECL_SOURCE_LOCATION (newdecl),
+				     "new declaration %q#D ambiguates built-in"
+				     " declaration %q#D", newdecl, olddecl)
+			  && flag_permissive)
+			inform (DECL_SOURCE_LOCATION (newdecl),
+				"ignoring the %q#D declaration", newdecl);
+		      return olddecl;
+		    }
+		}
+
 	      /* A near match; override the builtin.  */
 
 	      if (TREE_PUBLIC (newdecl))
--- gcc/testsuite/g++.dg/ext/pr84724-1.C.jj	2018-03-09 11:44:49.037993207 +0100
+++ gcc/testsuite/g++.dg/ext/pr84724-1.C	2018-03-09 11:57:11.599204801 +0100
@@ -0,0 +1,14 @@
+// PR c++/84724
+// { dg-do compile }
+// { dg-options "-O3 -fpermissive" }
+
+int __builtin_trap ();		// { dg-warning "ambiguates built-in declaration" }
+				// { dg-message "ignoring the 'int __builtin_trap\\(\\)' declaration" "" { target *-*-* } .-1 }
+
+int
+foo ()
+{
+  int b;
+  int c (&b);			// { dg-warning "invalid conversion from" }
+  return b %= b ? c : 0;
+}
--- gcc/testsuite/g++.dg/ext/pr84724-2.C.jj	2018-03-09 11:57:26.017208398 +0100
+++ gcc/testsuite/g++.dg/ext/pr84724-2.C	2018-03-09 11:57:40.775212078 +0100
@@ -0,0 +1,14 @@
+// PR c++/84724
+// { dg-do compile }
+// { dg-options "-O3 -fpermissive -w" }
+
+int __builtin_trap ();		// { dg-bogus "ambiguates built-in declaration" }
+				// { dg-bogus "ignoring the 'int __builtin_trap\\(\\)' declaration" "" { target *-*-* } .-1 }
+
+int
+foo ()
+{
+  int b;
+  int c (&b);			// { dg-bogus "invalid conversion from" }
+  return b %= b ? c : 0;
+}
--- gcc/testsuite/g++.dg/ext/pr84724-3.C.jj	2018-03-09 11:57:50.000214382 +0100
+++ gcc/testsuite/g++.dg/ext/pr84724-3.C	2018-03-09 11:58:10.797219571 +0100
@@ -0,0 +1,5 @@
+// PR c++/84724
+// { dg-do compile }
+// { dg-options "" }
+
+int __builtin_trap ();		// { dg-error "ambiguates built-in declaration" }


	Jakub