diff mbox series

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

Message ID 20180308180125.GU5867@tucnak
State New
Headers show
Series [C++] Don't redefine __* builtins (PR c++/84724) | expand

Commit Message

Jakub Jelinek March 8, 2018, 6:01 p.m. UTC 
Hi!

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?

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

	PR c++/84724
	* decl.c (duplicate_decls): Don't override __* prefixed builtins
	except for __[^b]*_chk.

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


	Jakub

Comments

Jason Merrill March 8, 2018, 9:06 p.m. UTC | #1 
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.

Jason
diff mbox series

Patch

--- gcc/cp/decl.c.jj	2018-03-05 17:23:45.901130856 +0100
+++ gcc/cp/decl.c	2018-03-08 10:28:55.574179119 +0100
@@ -1578,6 +1578,28 @@  next_arg:;
                          DECL_BUILT_IN (olddecl)
                          ? G_("shadowing built-in function %q#D")
                          : G_("shadowing library function %q#D"), 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);
+
+		  if (name[0] == '_' && name[1] == '_')
+		    {
+		      if (strncmp (name + 2, "builtin_",
+				   strlen ("builtin_")) == 0)
+			return NULL_TREE;
+
+		      size_t len = strlen (name);
+
+		      if (len <= strlen ("___chk")
+			  || memcmp (name + len - strlen ("_chk"),
+				     "_chk", strlen ("_chk") + 1) != 0)
+			return NULL_TREE;
+		    }
+		}
 	    }
 	  else
 	    /* Discard the old built-in function.  */
--- gcc/testsuite/g++.dg/ext/pr84724.C.jj	2018-03-08 10:31:03.683183914 +0100
+++ gcc/testsuite/g++.dg/ext/pr84724.C	2018-03-08 10:30:45.489183233 +0100
@@ -0,0 +1,13 @@ 
+// PR c++/84724
+// { dg-do compile }
+// { dg-options "-O3 -fpermissive" }
+
+int __builtin_trap ();		// { dg-warning "ambiguates built-in declaration" }
+
+int
+foo ()
+{
+  int b;
+  int c (&b);			// { dg-warning "invalid conversion from" }
+  return b %= b ? c : 0;
+}