Re: [PATCH] C++: target attribute - local decl

[Jason Merrill <jason@redhat.com>]

On 2/18/21 7:15 AM, Martin Liška wrote:
> We crash when target attribute get_function_versions_dispatcher is called
> for a function that is not registered in call graph. This fixes that
> by emitting a new error.
> 
> Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
> 
> Ready to be installed?
> Thanks,
> Martin
> 
> gcc/cp/ChangeLog:
> 
>      PR c++/99108
>      * call.c (get_function_version_dispatcher): Do not parse
>      target attribute for a function with a missing declaration.
> 
> gcc/testsuite/ChangeLog:
> 
>      PR c++/99108
>      * g++.target/i386/pr99108.C: New test.
> ---
>   gcc/cp/call.c                           |  8 +++++++-
>   gcc/testsuite/g++.target/i386/pr99108.C | 18 ++++++++++++++++++
>   2 files changed, 25 insertions(+), 1 deletion(-)
>   create mode 100644 gcc/testsuite/g++.target/i386/pr99108.C
> 
> diff --git a/gcc/cp/call.c b/gcc/cp/call.c
> index 186feef6fe3..844853e504e 100644
> --- a/gcc/cp/call.c
> +++ b/gcc/cp/call.c
> @@ -8386,8 +8386,14 @@ get_function_version_dispatcher (tree fn)
>             && DECL_FUNCTION_VERSIONED (fn));
> 
>     gcc_assert (targetm.get_function_versions_dispatcher);
> -  dispatcher_decl = targetm.get_function_versions_dispatcher (fn);
> +  if (cgraph_node::get (fn) == NULL)
> +    {
> +      error_at (DECL_SOURCE_LOCATION (fn), "missing declaration "
> +        "for a multiversioned function");

This seems like the wrong message.  The declaration isn't missing, 
you're using its source location for the error.  Do you mean 
"definition"?  But adding definitions earlier in the file doesn't help. 
And having file-scope declarations without definitions is fine.

The problem seems to be with the handling of local decls.  If 
DECL_LOCAL_DECL_P, you need to look at DECL_LOCAL_DECL_ALIAS to find the 
namespace-scope decl.  But then if there is no preceding namespace-scope 
declaration, the new decl created by push_local_extern_decl_alias 
doesn't have a cgraph node, either.  I guess maybe_function_versions 
also needs to look through DECL_LOCAL_DECL_ALIAS.

> +      return NULL;
> +    }
> 
> +  dispatcher_decl = targetm.get_function_versions_dispatcher (fn);
>     if (dispatcher_decl == NULL)
>       {
>         error_at (input_location, "use of multiversioned function "
> diff --git a/gcc/testsuite/g++.target/i386/pr99108.C 
> b/gcc/testsuite/g++.target/i386/pr99108.C
> new file mode 100644
> index 00000000000..b0c4ffa2688
> --- /dev/null
> +++ b/gcc/testsuite/g++.target/i386/pr99108.C
> @@ -0,0 +1,18 @@
> +/* PR c++/99108 */
> +/* { dg-do compile { target c++20 } } */
> +/* { dg-require-ifunc "" }  */
> +
> +struct A {
> +  void foo(auto);
> +};
> +void A::foo(auto)
> +{
> +  int f(void) __attribute__((target("default")));
> +  int f(void) __attribute__((target("arch=atom"))); /* { dg-error 
> "missing declaration for a multiversioned function" } */
> +  int b = f();
> +}
> +void bar(void)
> +{
> +  A c;
> +  c.foo(7);
> +}