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.
True...
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.
Richard.
