Re: [PATCH] PR c/86407 - Add option to ignore fndecl attributes on function pointers

[Richard Biener <rguenther@suse.de>]

[-- Attachment #1: Type: text/plain, Size: 3490 bytes --]

On Fri, 31 May 2019, Alex Henrie wrote:

> On Fri, May 31, 2019 at 1:38 AM Richard Biener <rguenther@suse.de> wrote:
> >
> > On Thu, 30 May 2019, Alex Henrie wrote:
> >
> > > In Wine we need a way to (without warnings) put ms_hook_prologue into
> > > a macro that is applied to functions, function pointers, and function
> > > pointer typedefs. It sounds like you're saying that you will not
> > > accept a patch that silences or splits off warnings about using
> > > ms_hook_prologue with function pointers and function pointer typedefs.
> > > So how do you think Wine's problem should be solved?
> >
> > I think ms_hook_prologue should be allowed to apply to function types
> > and function decls.  If you say it should apply to function pointers
> > then I suppose you want to have it apply to the pointed to function
> > of the function pointer - but that's not possible without an indirection
> > via a function pointer typedef IIRC.
> 
> No, if ms_hook_prologue is applied to a function pointer, it shouldn't
> do anything except maybe trigger some optimization of the code around
> the indirect function call.
> 
> > I also have the following which _may_ motivate that attributes
> > currently not applying to function types (because they only
> > affect function definitions) should also apply there:
> >
> > typedef int  (myfun)  (int *) __attribute__((nonnull(1)));
> > myfun x;
> > int x(int *p) { return p != (int*)0; }
> >
> > this applies nonnull to the function definition of 'x' but
> > I put the attribute on the typedef.  I didn't manage to
> > do without the myfun x; declaration.
> 
> That is a great example and another compelling reason to allow
> "fndecl" attributes in more places.
> 
> > > It seems to me that any information about the target of a function
> > > pointer, even the flatten attribute or the ms_hook_prologue attribute,
> > > provides information that could be useful for optimizing the code
> > > around the indirect function call. That sounds like a compelling
> > > argument for allowing these attributes in more places without
> > > warnings.
> >
> > Sure.  Can you write down the three cases after macro expansion
> > here to clarify what you need?  Esp. say what the attribute should
> > apply to.  Just silencing the warning without actually achieving
> > what you want would be bad I think ;)
> 
> Essentially, the following needs to compile without warnings:
> 
> #define WINAPI __attribute__((__stdcall__)) \
>                __attribute__((__ms_hook_prologue__))
> 
> typedef unsigned int (WINAPI *APPLICATION_RECOVERY_CALLBACK)(void*);
> 
> void WINAPI foo()
> {
>     APPLICATION_RECOVERY_CALLBACK bar;
>     unsigned int (WINAPI *baz)(void*);
> }

OK, so it's being that attributes with effect only on function
bodies are harmless on function types / indirect calls.  Of
course in case the user expects sth like a thunk to be generated
for calls through such type then a warning that the attribute has
no effect is warranted.

I'd vote for splitting -Wattributes to distinguish

t.c:2:1: warning: ‘ms_ho_prologue’ attribute directive ignored 
[-Wattributes]
 void __attribute__((__ms_ho_prologue__)) bar () {}
 ^~~~
t.c:4:1: warning: ‘ms_hook_prologue’ attribute only applies to functions 
[-Wattributes]
 typedef void __attribute__((__ms_hook_prologue__)) (*fn_t)();

so you could disable the second while retaining the first.  You
could be also more careful in the source where you place the
attributes...

Richard.