OK.
On Fri, Mar 9, 2018 at 6:05 AM, Jakub Jelinek <ja...@redhat.com> wrote:
> 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
