On Thu, Mar 08, 2018 at 04:06:24PM -0500, Jason Merrill wrote:
> On Thu, Mar 8, 2018 at 1:01 PM, Jakub Jelinek <ja...@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 <ja...@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
