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

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

[Alex Henrie]

---
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86407#c6
---
 gcc/c-family/c.opt                     |  4 ++++
 gcc/c/c-decl.c                         |  4 +++-
 gcc/config/i386/i386-options.c         | 12 ++++++++++--
 gcc/testsuite/c-c++-common/pr86407-1.c | 23 +++++++++++++++++++++++
 gcc/testsuite/c-c++-common/pr86407-2.c | 25 +++++++++++++++++++++++++
 5 files changed, 65 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/c-c++-common/pr86407-1.c
 create mode 100644 gcc/testsuite/c-c++-common/pr86407-2.c

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 046d489f7eb..3b99c96e53d 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -776,6 +776,10 @@ Wsizeof-array-argument
 C ObjC C++ ObjC++ Var(warn_sizeof_array_argument) Warning Init(1)
 Warn when sizeof is applied on a parameter declared as an array.
 
+Wstrict-function-attributes
+C C++ Var(warn_strict_function_attributes) Init(1) Warning
+Warn when function definition attributes are applied to function pointers.
+
 Wstringop-overflow
 C ObjC C++ LTO ObjC++ Warning Alias(Wstringop-overflow=, 2, 0)
 Warn about buffer overflow in string manipulation functions like memcpy
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 181a8c2e9aa..427d573c8d3 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -6192,7 +6192,9 @@ grokdeclarator (const struct c_declarator *declarator,
 	      inner_decl = inner_decl->declarator;
 	    if (inner_decl->kind == cdk_id)
 	      attr_flags |= (int) ATTR_FLAG_DECL_NEXT;
-	    else if (inner_decl->kind == cdk_function)
+	    else if (inner_decl->kind == cdk_function
+		     || (inner_decl->kind == cdk_pointer
+			 && TREE_CODE (type) == FUNCTION_TYPE))
 	      attr_flags |= (int) ATTR_FLAG_FUNCTION_NEXT;
 	    else if (inner_decl->kind == cdk_array)
 	      attr_flags |= (int) ATTR_FLAG_ARRAY_NEXT;
diff --git a/gcc/config/i386/i386-options.c b/gcc/config/i386/i386-options.c
index 0f236626005..6b50f143b05 100644
--- a/gcc/config/i386/i386-options.c
+++ b/gcc/config/i386/i386-options.c
@@ -3489,8 +3489,16 @@ ix86_handle_fndecl_attribute (tree *node, tree name, tree args, int,
 {
   if (TREE_CODE (*node) != FUNCTION_DECL)
     {
-      warning (OPT_Wattributes, "%qE attribute only applies to functions",
-               name);
+      if (FUNCTION_POINTER_TYPE_P (TREE_TYPE (*node)))
+	{
+	  warning (OPT_Wstrict_function_attributes,
+		   "%qE attribute only applies to function definitions", name);
+	}
+      else
+	{
+	  warning (OPT_Wattributes,
+		   "%qE attribute only applies to functions", name);
+	}
       *no_add_attrs = true;
     }
 
diff --git a/gcc/testsuite/c-c++-common/pr86407-1.c b/gcc/testsuite/c-c++-common/pr86407-1.c
new file mode 100644
index 00000000000..37993d8c051
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr86407-1.c
@@ -0,0 +1,23 @@
+/* PR c/86407 */
+/* Test a function attribute that is not a calling convention and does not
+   affect the function type. There should be no warnings when using the
+   attribute on a function declaration, function pointer, or function pointer
+   typedef, but there should be a warning when using the attribute on
+   non-function-pointer variables and typedefs. */
+
+/* { dg-options "-Wstrict-function-attributes -Wno-ignored-attributes" } */
+
+int __attribute__((__ms_hook_prologue__)) func(); /* no warnings */
+
+int (__attribute__((__ms_hook_prologue__)) *func_ptr)(); /* { dg-warning "'ms_hook_prologue' attribute only applies to function definitions" } */
+
+typedef int (__attribute__((__ms_hook_prologue__)) *FUNC_PTR)(); /* { dg-warning "'ms_hook_prologue' attribute only applies to function definitions" } */
+
+int __attribute__((__ms_hook_prologue__)) *var_ptr; /* { dg-warning "'ms_hook_prologue' attribute only applies to functions" } */
+
+typedef int __attribute__((__ms_hook_prologue__)) *VAR_PTR; /* { dg-warning "'ms_hook_prologue' attribute only applies to functions" } */
+
+int main(int argc, char **argv)
+{
+    return 0;
+}
diff --git a/gcc/testsuite/c-c++-common/pr86407-2.c b/gcc/testsuite/c-c++-common/pr86407-2.c
new file mode 100644
index 00000000000..6840b0180dd
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr86407-2.c
@@ -0,0 +1,25 @@
+/* PR c/86407 */
+/* Test a function attribute that is not a calling convention and does not
+   affect the function type. A function pointer without the attribute may point
+   to a function with the attribute and vice-versa. */
+
+/* { dg-options "-Wno-strict-function-attributes -Wno-ignored-attributes" } */
+
+int ordinary_func()
+{
+    return 0;
+}
+
+int __attribute__((__ms_hook_prologue__)) special_func()
+{
+    return 0;
+}
+
+int main(int argc, char **argv)
+{
+    int (*ordinary_func_ptr)() = special_func;
+
+    int (__attribute__((__ms_hook_prologue__)) *special_func_ptr)() = ordinary_func;
+
+    return 0;
+}
-- 
2.21.0

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

[Richard Biener]

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

On Thu, 23 May 2019, Alex Henrie wrote:

> ---
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86407#c6
> ---
>  gcc/c-family/c.opt                     |  4 ++++
>  gcc/c/c-decl.c                         |  4 +++-
>  gcc/config/i386/i386-options.c         | 12 ++++++++++--
>  gcc/testsuite/c-c++-common/pr86407-1.c | 23 +++++++++++++++++++++++
>  gcc/testsuite/c-c++-common/pr86407-2.c | 25 +++++++++++++++++++++++++
>  5 files changed, 65 insertions(+), 3 deletions(-)
>  create mode 100644 gcc/testsuite/c-c++-common/pr86407-1.c
>  create mode 100644 gcc/testsuite/c-c++-common/pr86407-2.c
> 
> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> index 046d489f7eb..3b99c96e53d 100644
> --- a/gcc/c-family/c.opt
> +++ b/gcc/c-family/c.opt
> @@ -776,6 +776,10 @@ Wsizeof-array-argument
>  C ObjC C++ ObjC++ Var(warn_sizeof_array_argument) Warning Init(1)
>  Warn when sizeof is applied on a parameter declared as an array.
>  
> +Wstrict-function-attributes
> +C C++ Var(warn_strict_function_attributes) Init(1) Warning
> +Warn when function definition attributes are applied to function pointers.
> +
>  Wstringop-overflow
>  C ObjC C++ LTO ObjC++ Warning Alias(Wstringop-overflow=, 2, 0)
>  Warn about buffer overflow in string manipulation functions like memcpy
> diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
> index 181a8c2e9aa..427d573c8d3 100644
> --- a/gcc/c/c-decl.c
> +++ b/gcc/c/c-decl.c
> @@ -6192,7 +6192,9 @@ grokdeclarator (const struct c_declarator *declarator,
>  	      inner_decl = inner_decl->declarator;
>  	    if (inner_decl->kind == cdk_id)
>  	      attr_flags |= (int) ATTR_FLAG_DECL_NEXT;
> -	    else if (inner_decl->kind == cdk_function)
> +	    else if (inner_decl->kind == cdk_function
> +		     || (inner_decl->kind == cdk_pointer
> +			 && TREE_CODE (type) == FUNCTION_TYPE))
>  	      attr_flags |= (int) ATTR_FLAG_FUNCTION_NEXT;
>  	    else if (inner_decl->kind == cdk_array)
>  	      attr_flags |= (int) ATTR_FLAG_ARRAY_NEXT;
> diff --git a/gcc/config/i386/i386-options.c b/gcc/config/i386/i386-options.c
> index 0f236626005..6b50f143b05 100644
> --- a/gcc/config/i386/i386-options.c
> +++ b/gcc/config/i386/i386-options.c
> @@ -3489,8 +3489,16 @@ ix86_handle_fndecl_attribute (tree *node, tree name, tree args, int,
>  {
>    if (TREE_CODE (*node) != FUNCTION_DECL)
>      {
> -      warning (OPT_Wattributes, "%qE attribute only applies to functions",
> -               name);
> +      if (FUNCTION_POINTER_TYPE_P (TREE_TYPE (*node)))
> +	{
> +	  warning (OPT_Wstrict_function_attributes,
> +		   "%qE attribute only applies to function definitions", name);
> +	}
> +      else
> +	{
> +	  warning (OPT_Wattributes,
> +		   "%qE attribute only applies to functions", name);
> +	}

I'm not sure we really need a new warning for this.

>        *no_add_attrs = true;
>      }
>  
> diff --git a/gcc/testsuite/c-c++-common/pr86407-1.c b/gcc/testsuite/c-c++-common/pr86407-1.c
> new file mode 100644
> index 00000000000..37993d8c051
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/pr86407-1.c
> @@ -0,0 +1,23 @@
> +/* PR c/86407 */
> +/* Test a function attribute that is not a calling convention and does not
> +   affect the function type. There should be no warnings when using the
> +   attribute on a function declaration, function pointer, or function pointer
> +   typedef, but there should be a warning when using the attribute on
> +   non-function-pointer variables and typedefs. */
> +
> +/* { dg-options "-Wstrict-function-attributes -Wno-ignored-attributes" } */
> +
> +int __attribute__((__ms_hook_prologue__)) func(); /* no warnings */
> +

But this is a declaration?

> +int (__attribute__((__ms_hook_prologue__)) *func_ptr)(); /* { dg-warning "'ms_hook_prologue' attribute only applies to function definitions" } */
> +
> +typedef int (__attribute__((__ms_hook_prologue__)) *FUNC_PTR)(); /* { dg-warning "'ms_hook_prologue' attribute only applies to function definitions" } */
> +
> +int __attribute__((__ms_hook_prologue__)) *var_ptr; /* { dg-warning "'ms_hook_prologue' attribute only applies to functions" } */
> +
> +typedef int __attribute__((__ms_hook_prologue__)) *VAR_PTR; /* { dg-warning "'ms_hook_prologue' attribute only applies to functions" } */
> +
> +int main(int argc, char **argv)
> +{
> +    return 0;
> +}
> diff --git a/gcc/testsuite/c-c++-common/pr86407-2.c b/gcc/testsuite/c-c++-common/pr86407-2.c
> new file mode 100644
> index 00000000000..6840b0180dd
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/pr86407-2.c
> @@ -0,0 +1,25 @@
> +/* PR c/86407 */
> +/* Test a function attribute that is not a calling convention and does not
> +   affect the function type. A function pointer without the attribute may point
> +   to a function with the attribute and vice-versa. */
> +
> +/* { dg-options "-Wno-strict-function-attributes -Wno-ignored-attributes" } */
> +
> +int ordinary_func()
> +{
> +    return 0;
> +}
> +
> +int __attribute__((__ms_hook_prologue__)) special_func()
> +{
> +    return 0;
> +}
> +
> +int main(int argc, char **argv)
> +{
> +    int (*ordinary_func_ptr)() = special_func;
> +
> +    int (__attribute__((__ms_hook_prologue__)) *special_func_ptr)() = ordinary_func;
> +
> +    return 0;
> +}
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG NÌrnberg)

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

[Alex Henrie]

On Fri, May 24, 2019 at 2:01 AM Richard Biener <rguenther@suse.de> wrote:
>
> I'm not sure we really need a new warning for this.

On Fri, May 24, 2019 at 9:23 AM Martin Sebor <msebor@gmail.com> wrote:
>
> I don't think GCC makes a formal distinction between function
> attributes that affect only function definitions vs those that
> affect its users, or both.  It might be a useful distinction
> to introduce, perhaps even for functions as well as variables,
> but as it is, users (as well as GCC developers) are on our own
> to figure it out.

Then is it preferable to simply silence Wattributes in this case?

On Fri, May 24, 2019 at 2:01 AM Richard Biener <rguenther@suse.de> wrote:
>
> > +int __attribute__((__ms_hook_prologue__)) func(); /* no warnings */
> > +
>
> But this is a declaration?

On Fri, May 24, 2019 at 9:23 AM Martin Sebor <msebor@gmail.com> wrote:
>
> My first question is: what is the meaning of "function definition
> attributes?"  Presumably those that affect only the definition of
> a function and not its callers or other users?

As far as I can tell, "fndecl" is a misnomer: these attributes are
more accurately called "function definition attributes", i.e.
attributes that affect the assembly code of the function but do not
affect its calling convention.

On Fri, May 24, 2019 at 9:23 AM Martin Sebor <msebor@gmail.com> wrote:
>
> Finally, with this as a prerequisite, if we decided that a warning
> like this would be useful, tests to verify that it works for all
> the definition attributes and not for the rest would need to be
> added (i.e., in addition to ms_hook_prologue).

Okay, I will add tests for the other function attributes that should
behave in the same way, commenting out the tests that will require
more work to pass.

The end goal is to include __ms_hook_prologue__ in the WINAPI macro on
Wine without causing spurious warnings. This will go a long way
towards making Wine compatible with current and future Windows
programs. Thank you for help.

-Alex

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

[Richard Biener]

On May 24, 2019 5:49:38 PM GMT+02:00, Alex Henrie <alexhenrie24@gmail.com> wrote:
>On Fri, May 24, 2019 at 2:01 AM Richard Biener <rguenther@suse.de>
>wrote:
>>
>> I'm not sure we really need a new warning for this.
>
>On Fri, May 24, 2019 at 9:23 AM Martin Sebor <msebor@gmail.com> wrote:
>>
>> I don't think GCC makes a formal distinction between function
>> attributes that affect only function definitions vs those that
>> affect its users, or both.  It might be a useful distinction
>> to introduce, perhaps even for functions as well as variables,
>> but as it is, users (as well as GCC developers) are on our own
>> to figure it out.
>
>Then is it preferable to simply silence Wattributes in this case?
>
>On Fri, May 24, 2019 at 2:01 AM Richard Biener <rguenther@suse.de>
>wrote:
>>
>> > +int __attribute__((__ms_hook_prologue__)) func(); /* no warnings
>*/
>> > +
>>
>> But this is a declaration?
>
>On Fri, May 24, 2019 at 9:23 AM Martin Sebor <msebor@gmail.com> wrote:
>>
>> My first question is: what is the meaning of "function definition
>> attributes?"  Presumably those that affect only the definition of
>> a function and not its callers or other users?
>
>As far as I can tell, "fndecl" is a misnomer: these attributes are
>more accurately called "function definition attributes", i.e.
>attributes that affect the assembly code of the function but do not
>affect its calling convention.

Yes. The others are attributes also applicable to function types to properly support attributed indirect calls. 

>On Fri, May 24, 2019 at 9:23 AM Martin Sebor <msebor@gmail.com> wrote:
>>
>> Finally, with this as a prerequisite, if we decided that a warning
>> like this would be useful, tests to verify that it works for all
>> the definition attributes and not for the rest would need to be
>> added (i.e., in addition to ms_hook_prologue).
>
>Okay, I will add tests for the other function attributes that should
>behave in the same way, commenting out the tests that will require
>more work to pass.
>
>The end goal is to include __ms_hook_prologue__ in the WINAPI macro on
>Wine without causing spurious warnings. This will go a long way
>towards making Wine compatible with current and future Windows
>programs. Thank you for help.
>
>-Alex

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

[Alex Henrie]

On Sat, May 25, 2019 at 12:34 AM Richard Biener <rguenther@suse.de> wrote:
>
> On May 24, 2019 5:49:38 PM GMT+02:00, Alex Henrie <alexhenrie24@gmail.com> wrote:
> >As far as I can tell, "fndecl" is a misnomer: these attributes are
> >more accurately called "function definition attributes", i.e.
> >attributes that affect the assembly code of the function but do not
> >affect its calling convention.
>
> Yes. The others are attributes also applicable to function types to properly support attributed indirect calls.

I'm happy to rename ix86_handle_fndecl_attribute to
ix86_handle_fndef_attribute then. Should I do that in this patch or a
separate patch?

-Alex

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

[Richard Biener]

On Sat, 25 May 2019, Alex Henrie wrote:

> On Sat, May 25, 2019 at 12:34 AM Richard Biener <rguenther@suse.de> wrote:
> >
> > On May 24, 2019 5:49:38 PM GMT+02:00, Alex Henrie <alexhenrie24@gmail.com> wrote:
> > >As far as I can tell, "fndecl" is a misnomer: these attributes are
> > >more accurately called "function definition attributes", i.e.
> > >attributes that affect the assembly code of the function but do not
> > >affect its calling convention.
> >
> > Yes. The others are attributes also applicable to function types to properly support attributed indirect calls.
> 
> I'm happy to rename ix86_handle_fndecl_attribute to
> ix86_handle_fndef_attribute then. Should I do that in this patch or a
> separate patch?

I don't see this fixes anything.  It's pretty well-know that attributes
affecting call semantics have to be accepted both at type and decl
position and that they should be added to the type attributes.
Attributes that only affect code generation for the function itself
should be added to decl attributes and do not necessarily need to
be accepted in type position.

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

[Martin Sebor]

On 5/24/19 9:49 AM, Alex Henrie wrote:
> On Fri, May 24, 2019 at 2:01 AM Richard Biener <rguenther@suse.de> wrote:
>>
>> I'm not sure we really need a new warning for this.
> 
> On Fri, May 24, 2019 at 9:23 AM Martin Sebor <msebor@gmail.com> wrote:
>>
>> I don't think GCC makes a formal distinction between function
>> attributes that affect only function definitions vs those that
>> affect its users, or both.  It might be a useful distinction
>> to introduce, perhaps even for functions as well as variables,
>> but as it is, users (as well as GCC developers) are on our own
>> to figure it out.
> 
> Then is it preferable to simply silence Wattributes in this case?

This case being PR86407?  I'd say yes.  I think one general problem
to solve is the missing suppression for typedefs.  This could be
useful for other warnings as well.  Another is providing a knob to
control the warning when one kind of an attribute is used with
an entity of a different kind (function vs type, or variable vs
type).  Yet another might be to control warnings for individual
attributes.

> 
> On Fri, May 24, 2019 at 2:01 AM Richard Biener <rguenther@suse.de> wrote:
>>
>>> +int __attribute__((__ms_hook_prologue__)) func(); /* no warnings */
>>> +
>>
>> But this is a declaration?
> 
> On Fri, May 24, 2019 at 9:23 AM Martin Sebor <msebor@gmail.com> wrote:
>>
>> My first question is: what is the meaning of "function definition
>> attributes?"  Presumably those that affect only the definition of
>> a function and not its callers or other users?
> 
> As far as I can tell, "fndecl" is a misnomer: these attributes are
> more accurately called "function definition attributes", i.e.
> attributes that affect the assembly code of the function but do not
> affect its calling convention.

That's one possible definition but there are examples that don't
fit it (at least not very neatly).

Attribute malloc attaches only to fndecl and not its type but
doesn't affect the code for a function definition.  FWIW, I
think this is just a bug -- attribute malloc should apply
to a function type for the same reason the closely related
attribute alloc_size does.

Attribute constructor also attaches to a fndecl even though it
doesn't affect the function's codegen but that of its caller
(the runtime).  In this case, though, I'd say that's fine.
Should it be classified as a function definition attribute?

Attributes cold and hot also apply to a fndecl and not its type
but they affect both the function's definition and its callers.
I think this also makes sense.  Should they be classified as
function definition attributes?

> On Fri, May 24, 2019 at 9:23 AM Martin Sebor <msebor@gmail.com> wrote:
>>
>> Finally, with this as a prerequisite, if we decided that a warning
>> like this would be useful, tests to verify that it works for all
>> the definition attributes and not for the rest would need to be
>> added (i.e., in addition to ms_hook_prologue).
> 
> Okay, I will add tests for the other function attributes that should
> behave in the same way, commenting out the tests that will require
> more work to pass.
> 
> The end goal is to include __ms_hook_prologue__ in the WINAPI macro on
> Wine without causing spurious warnings. This will go a long way
> towards making Wine compatible with current and future Windows
> programs. Thank you for help.

Yes, I understand the goal.  I'm not sure that the proposed change
is the right way to do it.  It seems to me that a more targeted bug
to fix with the broadest benefit is that _Pragma GCC diagnostic (or
some such mechanism) cannot be used to suppress the warning for
typedefs.

Martin

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

[Richard Biener]

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

On Tue, 28 May 2019, Martin Sebor wrote:

> On 5/24/19 9:49 AM, Alex Henrie wrote:
> > On Fri, May 24, 2019 at 2:01 AM Richard Biener <rguenther@suse.de> wrote:
> > > 
> > > I'm not sure we really need a new warning for this.
> > 
> > On Fri, May 24, 2019 at 9:23 AM Martin Sebor <msebor@gmail.com> wrote:
> > > 
> > > I don't think GCC makes a formal distinction between function
> > > attributes that affect only function definitions vs those that
> > > affect its users, or both.  It might be a useful distinction
> > > to introduce, perhaps even for functions as well as variables,
> > > but as it is, users (as well as GCC developers) are on our own
> > > to figure it out.
> > 
> > Then is it preferable to simply silence Wattributes in this case?
> 
> This case being PR86407?  I'd say yes.  I think one general problem
> to solve is the missing suppression for typedefs.  This could be
> useful for other warnings as well.  Another is providing a knob to
> control the warning when one kind of an attribute is used with
> an entity of a different kind (function vs type, or variable vs
> type).  Yet another might be to control warnings for individual
> attributes.
> 
> > 
> > On Fri, May 24, 2019 at 2:01 AM Richard Biener <rguenther@suse.de> wrote:
> > > 
> > > > +int __attribute__((__ms_hook_prologue__)) func(); /* no warnings */
> > > > +
> > > 
> > > But this is a declaration?
> > 
> > On Fri, May 24, 2019 at 9:23 AM Martin Sebor <msebor@gmail.com> wrote:
> > > 
> > > My first question is: what is the meaning of "function definition
> > > attributes?"  Presumably those that affect only the definition of
> > > a function and not its callers or other users?
> > 
> > As far as I can tell, "fndecl" is a misnomer: these attributes are
> > more accurately called "function definition attributes", i.e.
> > attributes that affect the assembly code of the function but do not
> > affect its calling convention.
> 
> That's one possible definition but there are examples that don't
> fit it (at least not very neatly).
> 
> Attribute malloc attaches only to fndecl and not its type but
> doesn't affect the code for a function definition.  FWIW, I
> think this is just a bug -- attribute malloc should apply
> to a function type for the same reason the closely related
> attribute alloc_size does.

Yeah, we have existing attributes that do not apply well to
indirect calls due to mistakes made in the past.  Of course
for things like 'malloc' this isn't a correctness issue.

> Attribute constructor also attaches to a fndecl even though it
> doesn't affect the function's codegen but that of its caller
> (the runtime).  In this case, though, I'd say that's fine.
> Should it be classified as a function definition attribute?

I think the issue is that internally we know what a fndecl is
but that doesn't match what the C standard says is a function
declaration.  So it's important to not mix terms for GCC internals
and terms used in diagnostics which generally should use terms
from the language standard.

> Attributes cold and hot also apply to a fndecl and not its type
> but they affect both the function's definition and its callers.
> I think this also makes sense.  Should they be classified as
> function definition attributes?
> 
> > On Fri, May 24, 2019 at 9:23 AM Martin Sebor <msebor@gmail.com> wrote:
> > > 
> > > Finally, with this as a prerequisite, if we decided that a warning
> > > like this would be useful, tests to verify that it works for all
> > > the definition attributes and not for the rest would need to be
> > > added (i.e., in addition to ms_hook_prologue).
> > 
> > Okay, I will add tests for the other function attributes that should
> > behave in the same way, commenting out the tests that will require
> > more work to pass.
> > 
> > The end goal is to include __ms_hook_prologue__ in the WINAPI macro on
> > Wine without causing spurious warnings. This will go a long way
> > towards making Wine compatible with current and future Windows
> > programs. Thank you for help.
> 
> Yes, I understand the goal.  I'm not sure that the proposed change
> is the right way to do it.  It seems to me that a more targeted bug
> to fix with the broadest benefit is that _Pragma GCC diagnostic (or
> some such mechanism) cannot be used to suppress the warning for
> typedefs.
> 
> Martin
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Linux GmbH, Maxfeldstrasse 5, 90409 Nuernberg, Germany;
GF: Felix Imendörffer, Mary Higgins, Sri Rasiah; HRB 21284 (AG Nürnberg)

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

[Alex Henrie]

On Tue, May 28, 2019 at 1:05 PM Martin Sebor <msebor@gmail.com> wrote:
>
> On 5/24/19 9:49 AM, Alex Henrie wrote:
> > Then is it preferable to simply silence Wattributes in this case?
>
> This case being PR86407?  I'd say yes.

> Attribute malloc attaches only to fndecl and not its type but
> doesn't affect the code for a function definition.  FWIW, I
> think this is just a bug -- attribute malloc should apply
> to a function type for the same reason the closely related
> attribute alloc_size does.
>
> Attribute constructor also attaches to a fndecl even though it
> doesn't affect the function's codegen but that of its caller
> (the runtime).  In this case, though, I'd say that's fine.
> Should it be classified as a function definition attribute?
>
> Attributes cold and hot also apply to a fndecl and not its type
> but they affect both the function's definition and its callers.
> I think this also makes sense.  Should they be classified as
> function definition attributes?

Those are very good points. The more information the optimizer has
about how the function works, the better it can optimize the code near
the function call. This would even apply to ms_hook_prologue because
the optimizer should definitely not inline indirect calls to hookable
functions even if it would inline a similar non-hookable function.

At this point I think I'm convinced that any attribute that applies to
a function should also be allowed on a function pointer without any
warnings. We can start by turning off the warnings for the "fndecl"
attributes and then clean up the other attributes as time goes on.
Sound good?

-Alex

On Tue, May 28, 2019 at 1:05 PM Martin Sebor <msebor@gmail.com> wrote:
>
> On 5/24/19 9:49 AM, Alex Henrie wrote:
> > On Fri, May 24, 2019 at 2:01 AM Richard Biener <rguenther@suse.de> wrote:
> >>
> >> I'm not sure we really need a new warning for this.
> >
> > On Fri, May 24, 2019 at 9:23 AM Martin Sebor <msebor@gmail.com> wrote:
> >>
> >> I don't think GCC makes a formal distinction between function
> >> attributes that affect only function definitions vs those that
> >> affect its users, or both.  It might be a useful distinction
> >> to introduce, perhaps even for functions as well as variables,
> >> but as it is, users (as well as GCC developers) are on our own
> >> to figure it out.
> >
> > Then is it preferable to simply silence Wattributes in this case?
>
> This case being PR86407?  I'd say yes.  I think one general problem
> to solve is the missing suppression for typedefs.  This could be
> useful for other warnings as well.  Another is providing a knob to
> control the warning when one kind of an attribute is used with
> an entity of a different kind (function vs type, or variable vs
> type).  Yet another might be to control warnings for individual
> attributes.
>
> >
> > On Fri, May 24, 2019 at 2:01 AM Richard Biener <rguenther@suse.de> wrote:
> >>
> >>> +int __attribute__((__ms_hook_prologue__)) func(); /* no warnings */
> >>> +
> >>
> >> But this is a declaration?
> >
> > On Fri, May 24, 2019 at 9:23 AM Martin Sebor <msebor@gmail.com> wrote:
> >>
> >> My first question is: what is the meaning of "function definition
> >> attributes?"  Presumably those that affect only the definition of
> >> a function and not its callers or other users?
> >
> > As far as I can tell, "fndecl" is a misnomer: these attributes are
> > more accurately called "function definition attributes", i.e.
> > attributes that affect the assembly code of the function but do not
> > affect its calling convention.
>
> That's one possible definition but there are examples that don't
> fit it (at least not very neatly).
>
> Attribute malloc attaches only to fndecl and not its type but
> doesn't affect the code for a function definition.  FWIW, I
> think this is just a bug -- attribute malloc should apply
> to a function type for the same reason the closely related
> attribute alloc_size does.
>
> Attribute constructor also attaches to a fndecl even though it
> doesn't affect the function's codegen but that of its caller
> (the runtime).  In this case, though, I'd say that's fine.
> Should it be classified as a function definition attribute?
>
> Attributes cold and hot also apply to a fndecl and not its type
> but they affect both the function's definition and its callers.
> I think this also makes sense.  Should they be classified as
> function definition attributes?
>
> > On Fri, May 24, 2019 at 9:23 AM Martin Sebor <msebor@gmail.com> wrote:
> >>
> >> Finally, with this as a prerequisite, if we decided that a warning
> >> like this would be useful, tests to verify that it works for all
> >> the definition attributes and not for the rest would need to be
> >> added (i.e., in addition to ms_hook_prologue).
> >
> > Okay, I will add tests for the other function attributes that should
> > behave in the same way, commenting out the tests that will require
> > more work to pass.
> >
> > The end goal is to include __ms_hook_prologue__ in the WINAPI macro on
> > Wine without causing spurious warnings. This will go a long way
> > towards making Wine compatible with current and future Windows
> > programs. Thank you for help.
>
> Yes, I understand the goal.  I'm not sure that the proposed change
> is the right way to do it.  It seems to me that a more targeted bug
> to fix with the broadest benefit is that _Pragma GCC diagnostic (or
> some such mechanism) cannot be used to suppress the warning for
> typedefs.
>
> Martin

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

[Joseph Myers]

On Thu, 30 May 2019, Alex Henrie wrote:

> At this point I think I'm convinced that any attribute that applies to
> a function should also be allowed on a function pointer without any
> warnings. We can start by turning off the warnings for the "fndecl"
> attributes and then clean up the other attributes as time goes on.

This is inherently a property of the attribute in question.  The issue is 
not whether it applies to function pointers; it's whether it applies to 
function types.

For example, the "section" or "alias" attributes are attributes that apply 
to a declaration, but not a type.  Because they apply to variables as well 
as functions, they are meaningful on function pointers - but the meaning 
is *not* the same as applying them to the pointed-to function.

The "flatten" attribute, however, seems only meaningful for functions, not 
variables, not function types and not function pointers.

We should try to work out for each attribute exactly what construct it 
appertains to - which for many but not all function attributes is indeed 
the type of the function rather than the function itself.  Then move to 
making such attributes work on types.  But for attributes such as 
"flatten" that logically appertain to the declaration not its type, we 
should continue to diagnose them on function pointers or types.

-- 
Joseph S. Myers
joseph@codesourcery.com

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

[Alex Henrie]

On Thu, May 30, 2019 at 6:59 PM Joseph Myers <joseph@codesourcery.com> wrote:
>
> On Thu, 30 May 2019, Alex Henrie wrote:
>
> > At this point I think I'm convinced that any attribute that applies to
> > a function should also be allowed on a function pointer without any
> > warnings. We can start by turning off the warnings for the "fndecl"
> > attributes and then clean up the other attributes as time goes on.
>
> This is inherently a property of the attribute in question.  The issue is
> not whether it applies to function pointers; it's whether it applies to
> function types.
>
> For example, the "section" or "alias" attributes are attributes that apply
> to a declaration, but not a type.  Because they apply to variables as well
> as functions, they are meaningful on function pointers - but the meaning
> is *not* the same as applying them to the pointed-to function.
>
> The "flatten" attribute, however, seems only meaningful for functions, not
> variables, not function types and not function pointers.
>
> We should try to work out for each attribute exactly what construct it
> appertains to - which for many but not all function attributes is indeed
> the type of the function rather than the function itself.  Then move to
> making such attributes work on types.  But for attributes such as
> "flatten" that logically appertain to the declaration not its type, we
> should continue to diagnose them on function pointers or types.

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?

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.

-Alex

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

[Richard Biener]

On Thu, 30 May 2019, Alex Henrie wrote:

> On Thu, May 30, 2019 at 6:59 PM Joseph Myers <joseph@codesourcery.com> wrote:
> >
> > On Thu, 30 May 2019, Alex Henrie wrote:
> >
> > > At this point I think I'm convinced that any attribute that applies to
> > > a function should also be allowed on a function pointer without any
> > > warnings. We can start by turning off the warnings for the "fndecl"
> > > attributes and then clean up the other attributes as time goes on.
> >
> > This is inherently a property of the attribute in question.  The issue is
> > not whether it applies to function pointers; it's whether it applies to
> > function types.
> >
> > For example, the "section" or "alias" attributes are attributes that apply
> > to a declaration, but not a type.  Because they apply to variables as well
> > as functions, they are meaningful on function pointers - but the meaning
> > is *not* the same as applying them to the pointed-to function.
> >
> > The "flatten" attribute, however, seems only meaningful for functions, not
> > variables, not function types and not function pointers.
> >
> > We should try to work out for each attribute exactly what construct it
> > appertains to - which for many but not all function attributes is indeed
> > the type of the function rather than the function itself.  Then move to
> > making such attributes work on types.  But for attributes such as
> > "flatten" that logically appertain to the declaration not its type, we
> > should continue to diagnose them on function pointers or types.
> 
> 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.

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.

> 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 ;)

Richard.

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

[Alex Henrie]

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*);
}

-Alex

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

[Richard Biener]

[-- 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.

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

[Joseph Myers]

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?

By actually implementing the required semantics, not by silencing warnings 
that say you're using an attribute in a location for which no semantics 
for that attribute are known.

1. An attribute appertaining to a function pointer (an object whose type 
is pointer to function) is something different from an attribute 
appertaining to the type of the pointed-to function.  For example, the 
"section" attribute appertains to declarations.  If you apply it to the 
declaration of a function, it puts that function in that section.  If you 
apply it to the declaration of an object whose type is pointer to 
function, it puts that object in that section (it does not say what 
section the pointed-to function is in).

2. The "Attribute Syntax" section in the manual describes how to determine 
to what construct an attribute in the GNU syntax appertains.  Please note 
that this is *not* always the same construct to which a C++ / C2x 
attribute in the same place would appertain (I might look at C2x attribute 
implementation and other C2x features implementation around 
August/September, not before then).  You should first be familiar with how 
the C standard defines the syntax for declarators and their corresponding 
types, in order to follow the extensions for attributes included in 
declarators.

3. There are certain cases where GCC is deliberately lax about attributes 
being in the syntactically correct places, which in particular makes it 
possible to change an attribute from being a declaration attribute to one 
on function types without breaking existing source code expecting it to be 
on function declarations.  See the type_required and 
function_type_required members of struct attribute_spec.  Specifying an 
attribute as being a function type attribute like that means it can be 
specified in a position where syntactically it should appertain to the 
declaration of the function pointer object, and be automatically made to 
appertain to the pointed-to type instead - but that only works if all the 
code in GCC that looks to see whether some declaration has the attribute 
present, and acts accordingly, is changed to check for the attribute on 
the type instead.

4. So suppose you have a particular attribute, currently only accepted on 
function declarations, that you wish to be accepted on function types.  
You need to find all the places checking for that attribute on 
declarations within GCC, to implement its semantics, and change them (a) 
to check the type of the declaration instead and (b) to work with the 
appropriate type when there isn't a declaration at all, as for indirect 
calls.  (What's involved in this may depend on whether the code just 
checks the list of attributes on the declaration, or whether there is some 
other bit set on the tree that may or may not be present for types at 
all.)  You also need to change the relevant table of attributes to set 
type_required and function_type_required instead of decl_required.  And 
you need to change the attribute handler accordingly so that it works when 
given a type rather than a declaration.

-- 
Joseph S. Myers
joseph@codesourcery.com

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

[Martin Sebor]

On 5/23/19 9:57 PM, Alex Henrie wrote:
> ---
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86407#c6
> ---
>   gcc/c-family/c.opt                     |  4 ++++
>   gcc/c/c-decl.c                         |  4 +++-
>   gcc/config/i386/i386-options.c         | 12 ++++++++++--
>   gcc/testsuite/c-c++-common/pr86407-1.c | 23 +++++++++++++++++++++++
>   gcc/testsuite/c-c++-common/pr86407-2.c | 25 +++++++++++++++++++++++++
>   5 files changed, 65 insertions(+), 3 deletions(-)
>   create mode 100644 gcc/testsuite/c-c++-common/pr86407-1.c
>   create mode 100644 gcc/testsuite/c-c++-common/pr86407-2.c
> 
> diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
> index 046d489f7eb..3b99c96e53d 100644
> --- a/gcc/c-family/c.opt
> +++ b/gcc/c-family/c.opt
> @@ -776,6 +776,10 @@ Wsizeof-array-argument
>   C ObjC C++ ObjC++ Var(warn_sizeof_array_argument) Warning Init(1)
>   Warn when sizeof is applied on a parameter declared as an array.
>   
> +Wstrict-function-attributes
> +C C++ Var(warn_strict_function_attributes) Init(1) Warning
> +Warn when function definition attributes are applied to function pointers.

I thought I'd added my comments about the patch to the bug but
I guess I never saved it.

My first question is: what is the meaning of "function definition
attributes?"  Presumably those that affect only the definition of
a function and not its callers or other users?

I don't think GCC makes a formal distinction between function
attributes that affect only function definitions vs those that
affect its users, or both.  It might be a useful distinction
to introduce, perhaps even for functions as well as variables,
but as it is, users (as well as GCC developers) are on our own
to figure it out.

If such a distinction were to be introduced I think we then would
need to go through all attributes and decide which is which, make
sure it makes sense, and document it for each.  This might be
a useful exercise for other reasons as well: we should also
document whether an attribute attaches to the declaration or to
its type (this determines whether a pointer to a function can be
declared to have the same attribute as the function itself).

Finally, with this as a prerequisite, if we decided that a warning
like this would be useful, tests to verify that it works for all
the definition attributes and not for the rest would need to be
added (i.e., in addition to ms_hook_prologue).

Martin

> +
>   Wstringop-overflow
>   C ObjC C++ LTO ObjC++ Warning Alias(Wstringop-overflow=, 2, 0)
>   Warn about buffer overflow in string manipulation functions like memcpy
> diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
> index 181a8c2e9aa..427d573c8d3 100644
> --- a/gcc/c/c-decl.c
> +++ b/gcc/c/c-decl.c
> @@ -6192,7 +6192,9 @@ grokdeclarator (const struct c_declarator *declarator,
>   	      inner_decl = inner_decl->declarator;
>   	    if (inner_decl->kind == cdk_id)
>   	      attr_flags |= (int) ATTR_FLAG_DECL_NEXT;
> -	    else if (inner_decl->kind == cdk_function)
> +	    else if (inner_decl->kind == cdk_function
> +		     || (inner_decl->kind == cdk_pointer
> +			 && TREE_CODE (type) == FUNCTION_TYPE))
>   	      attr_flags |= (int) ATTR_FLAG_FUNCTION_NEXT;
>   	    else if (inner_decl->kind == cdk_array)
>   	      attr_flags |= (int) ATTR_FLAG_ARRAY_NEXT;
> diff --git a/gcc/config/i386/i386-options.c b/gcc/config/i386/i386-options.c
> index 0f236626005..6b50f143b05 100644
> --- a/gcc/config/i386/i386-options.c
> +++ b/gcc/config/i386/i386-options.c
> @@ -3489,8 +3489,16 @@ ix86_handle_fndecl_attribute (tree *node, tree name, tree args, int,
>   {
>     if (TREE_CODE (*node) != FUNCTION_DECL)
>       {
> -      warning (OPT_Wattributes, "%qE attribute only applies to functions",
> -               name);
> +      if (FUNCTION_POINTER_TYPE_P (TREE_TYPE (*node)))
> +	{
> +	  warning (OPT_Wstrict_function_attributes,
> +		   "%qE attribute only applies to function definitions", name);
> +	}
> +      else
> +	{
> +	  warning (OPT_Wattributes,
> +		   "%qE attribute only applies to functions", name);
> +	}
>         *no_add_attrs = true;
>       }
>   
> diff --git a/gcc/testsuite/c-c++-common/pr86407-1.c b/gcc/testsuite/c-c++-common/pr86407-1.c
> new file mode 100644
> index 00000000000..37993d8c051
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/pr86407-1.c
> @@ -0,0 +1,23 @@
> +/* PR c/86407 */
> +/* Test a function attribute that is not a calling convention and does not
> +   affect the function type. There should be no warnings when using the
> +   attribute on a function declaration, function pointer, or function pointer
> +   typedef, but there should be a warning when using the attribute on
> +   non-function-pointer variables and typedefs. */
> +
> +/* { dg-options "-Wstrict-function-attributes -Wno-ignored-attributes" } */
> +
> +int __attribute__((__ms_hook_prologue__)) func(); /* no warnings */
> +
> +int (__attribute__((__ms_hook_prologue__)) *func_ptr)(); /* { dg-warning "'ms_hook_prologue' attribute only applies to function definitions" } */
> +
> +typedef int (__attribute__((__ms_hook_prologue__)) *FUNC_PTR)(); /* { dg-warning "'ms_hook_prologue' attribute only applies to function definitions" } */
> +
> +int __attribute__((__ms_hook_prologue__)) *var_ptr; /* { dg-warning "'ms_hook_prologue' attribute only applies to functions" } */
> +
> +typedef int __attribute__((__ms_hook_prologue__)) *VAR_PTR; /* { dg-warning "'ms_hook_prologue' attribute only applies to functions" } */
> +
> +int main(int argc, char **argv)
> +{
> +    return 0;
> +}
> diff --git a/gcc/testsuite/c-c++-common/pr86407-2.c b/gcc/testsuite/c-c++-common/pr86407-2.c
> new file mode 100644
> index 00000000000..6840b0180dd
> --- /dev/null
> +++ b/gcc/testsuite/c-c++-common/pr86407-2.c
> @@ -0,0 +1,25 @@
> +/* PR c/86407 */
> +/* Test a function attribute that is not a calling convention and does not
> +   affect the function type. A function pointer without the attribute may point
> +   to a function with the attribute and vice-versa. */
> +
> +/* { dg-options "-Wno-strict-function-attributes -Wno-ignored-attributes" } */
> +
> +int ordinary_func()
> +{
> +    return 0;
> +}
> +
> +int __attribute__((__ms_hook_prologue__)) special_func()
> +{
> +    return 0;
> +}
> +
> +int main(int argc, char **argv)
> +{
> +    int (*ordinary_func_ptr)() = special_func;
> +
> +    int (__attribute__((__ms_hook_prologue__)) *special_func_ptr)() = ordinary_func;
> +
> +    return 0;
> +}
>