[PATCH] Fix ICF with error/warning attribute (PR ipa/84628)

Richard Biener rguenther@suse.de
Fri Mar 2 08:15:00 GMT 2018

On Fri, 2 Mar 2018, Jakub Jelinek wrote:

> On Fri, Mar 02, 2018 at 08:58:34AM +0100, Richard Biener wrote:
> > On Fri, 2 Mar 2018, Jakub Jelinek wrote:
> > 
> > > Hi!
> > > 
> > > If we need to use thunks for ICF to functions with warning or error
> > > attribute, their expansion will warn or error.  This patch just punts
> > > in those cases instead.
> > > 
> > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> > 
> > Looks ok but I wonder if marking the call in the thunk with no-warning
> > would work as well?
> No, expr.c doesn't test anything like that (and adding it might regress
> other things, TREE_NO_WARNING is way too overloaded now).
> I was also considering an alternative to add something like:
>   struct cgraph_node *node = cgraph_node::get (current_function_decl);
> and add
>   && (!node || !node->thunk.thunk_p)
> to the two conditions for error/warning attribute:
>         if (fndecl
>             && (attr = lookup_attribute ("error",
>                                          DECL_ATTRIBUTES (fndecl))) != NULL)
>           error ("%Kcall to %qs declared with attribute error: %s",
>                  exp, identifier_to_locale (lang_hooks.decl_printable_name (fndecl, 1)),
>                  TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr))));
>         if (fndecl
>             && (attr = lookup_attribute ("warning",
>                                          DECL_ATTRIBUTES (fndecl))) != NULL)
>           warning_at (tree_nonartificial_location (exp),
>                       0, "%Kcall to %qs declared with attribute warning: %s",
>                       exp, identifier_to_locale (lang_hooks.decl_printable_name (fndecl, 1)),
>                       TREE_STRING_POINTER (TREE_VALUE (TREE_VALUE (attr))));
> I've tried to come up with a testcase where generic thunks are used for C++,
> but failed to do something that would trigger an invalid warning or error.

You probably need a virtual return thunk as otherwise we expand them
directly to asm?

> Would you prefer just being silent in all thunks?

Yes, I think all warnings from thunks are ultimately going to be bogus...

> That said, wonder about thunks (the non-ICF ones) from false-negative
> diagnostic point as well, if I have some method with error/warning attribute
> and call a thunk instead, wonder if we get the diagnostic or not, thunks
> likely don't have the attribute copied over to them.


I guess we should not warn from thunks but instead move those attributes
to the thunks so see if those get called in the end.


More information about the Gcc-patches mailing list