[PATCH BACKPORT 4.14 1/2] Compiler Attributes: add support for __copy (gcc >= 9)

[PATCH BACKPORT 4.14 1/2] Compiler Attributes: add support for __copy (gcc >= 9)

[Stefan Agner]

From: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>

[ Upstream commit c0d9782f5b6d7157635ae2fd782a4b27d55a6013

From the GCC manual:

  copy
  copy(function)

    The copy attribute applies the set of attributes with which function
    has been declared to the declaration of the function to which
    the attribute is applied. The attribute is designed for libraries
    that define aliases or function resolvers that are expected
    to specify the same set of attributes as their targets. The copy
    attribute can be used with functions, variables, or types. However,
    the kind of symbol to which the attribute is applied (either
    function or variable) must match the kind of symbol to which
    the argument refers. The copy attribute copies only syntactic and
    semantic attributes but not attributes that affect a symbol’s
    linkage or visibility such as alias, visibility, or weak.
    The deprecated attribute is also not copied.

  https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html

The upcoming GCC 9 release extends the -Wmissing-attributes warnings
(enabled by -Wall) to C and aliases: it warns when particular function
attributes are missing in the aliases but not in their target, e.g.:

    void __cold f(void) {}
    void __alias("f") g(void);

diagnoses:

    warning: 'g' specifies less restrictive attribute than
    its target 'f': 'cold' [-Wmissing-attributes]

Using __copy(f) we can copy the __cold attribute from f to g:

    void __cold f(void) {}
    void __copy(f) __alias("f") g(void);

This attribute is most useful to deal with situations where an alias
is declared but we don't know the exact attributes the target has.

For instance, in the kernel, the widely used module_init/exit macros
define the init/cleanup_module aliases, but those cannot be marked
always as __init/__exit since some modules do not have their
functions marked as such.

Cc: <stable@vger.kernel.org> # 4.14+
Suggested-by: Martin Sebor <msebor@gcc.gnu.org>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 include/linux/compiler-gcc.h   | 4 ++++
 include/linux/compiler_types.h | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 4816355b9875..6d7ead22c1b4 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -343,6 +343,10 @@
 #define __designated_init __attribute__((designated_init))
 #endif
 
+#if GCC_VERSION >= 90100
+#define __copy(symbol)                 __attribute__((__copy__(symbol)))
+#endif
+
 #endif	/* gcc version >= 40000 specific checks */
 
 #if !defined(__noclone)
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 4be464a07612..20112bb1a8f9 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -230,6 +230,10 @@ struct ftrace_likely_data {
 # define __latent_entropy
 #endif
 
+#ifndef __copy
+# define __copy
+#endif
+
 #ifndef __randomize_layout
 # define __randomize_layout __designated_init
 #endif
-- 
2.21.0

[PATCH BACKPORT 4.14 2/2] include/linux/module.h: copy __init/__exit attrs to init/cleanup_module

[Stefan Agner]

From: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>

[ Upstream commit a6e60d84989fa0e91db7f236eda40453b0e44afa ]

The upcoming GCC 9 release extends the -Wmissing-attributes warnings
(enabled by -Wall) to C and aliases: it warns when particular function
attributes are missing in the aliases but not in their target.

In particular, it triggers for all the init/cleanup_module
aliases in the kernel (defined by the module_init/exit macros),
ending up being very noisy.

These aliases point to the __init/__exit functions of a module,
which are defined as __cold (among other attributes). However,
the aliases themselves do not have the __cold attribute.

Since the compiler behaves differently when compiling a __cold
function as well as when compiling paths leading to calls
to __cold functions, the warning is trying to point out
the possibly-forgotten attribute in the alias.

In order to keep the warning enabled, we decided to silence
this case. Ideally, we would mark the aliases directly
as __init/__exit. However, there are currently around 132 modules
in the kernel which are missing __init/__exit in their init/cleanup
functions (either because they are missing, or for other reasons,
e.g. the functions being called from somewhere else); and
a section mismatch is a hard error.

A conservative alternative was to mark the aliases as __cold only.
However, since we would like to eventually enforce __init/__exit
to be always marked,  we chose to use the new __copy function
attribute (introduced by GCC 9 as well to deal with this).
With it, we copy the attributes used by the target functions
into the aliases. This way, functions that were not marked
as __init/__exit won't have their aliases marked either,
and therefore there won't be a section mismatch.

Note that the warning would go away marking either the extern
declaration, the definition, or both. However, we only mark
the definition of the alias, since we do not want callers
(which only see the declaration) to be compiled as if the function
was __cold (and therefore the paths leading to those calls
would be assumed to be unlikely).

Cc: <stable@vger.kernel.org> # 4.14+
Link: https://lore.kernel.org/lkml/20190123173707.GA16603@gmail.com/
Link: https://lore.kernel.org/lkml/20190206175627.GA20399@gmail.com/
Suggested-by: Martin Sebor <msebor@gcc.gnu.org>
Acked-by: Jessica Yu <jeyu@kernel.org>
Signed-off-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Signed-off-by: Stefan Agner <stefan@agner.ch>
---
 include/linux/module.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/module.h b/include/linux/module.h
index a9d546c5b9aa..c290de08c830 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -128,13 +128,13 @@ extern void cleanup_module(void);
 #define module_init(initfn)					\
 	static inline initcall_t __maybe_unused __inittest(void)		\
 	{ return initfn; }					\
-	int init_module(void) __attribute__((alias(#initfn)));
+	int init_module(void) __copy(initfn) __attribute__((alias(#initfn)));
 
 /* This is only required if you want to be unloadable. */
 #define module_exit(exitfn)					\
 	static inline exitcall_t __maybe_unused __exittest(void)		\
 	{ return exitfn; }					\
-	void cleanup_module(void) __attribute__((alias(#exitfn)));
+	void cleanup_module(void) __copy(exitfn) __attribute__((alias(#exitfn)));
 
 #endif
 
-- 
2.21.0

Re: [PATCH BACKPORT 4.14 1/2] Compiler Attributes: add support for __copy (gcc >= 9)

[Greg KH]

On Tue, Jun 04, 2019 at 11:21:59AM +0200, Stefan Agner wrote:
> From: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
> 
> [ Upstream commit c0d9782f5b6d7157635ae2fd782a4b27d55a6013
> 
> >From the GCC manual:
> 
>   copy
>   copy(function)
> 
>     The copy attribute applies the set of attributes with which function
>     has been declared to the declaration of the function to which
>     the attribute is applied. The attribute is designed for libraries
>     that define aliases or function resolvers that are expected
>     to specify the same set of attributes as their targets. The copy
>     attribute can be used with functions, variables, or types. However,
>     the kind of symbol to which the attribute is applied (either
>     function or variable) must match the kind of symbol to which
>     the argument refers. The copy attribute copies only syntactic and
>     semantic attributes but not attributes that affect a symbol’s
>     linkage or visibility such as alias, visibility, or weak.
>     The deprecated attribute is also not copied.
> 
>   https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html
> 
> The upcoming GCC 9 release extends the -Wmissing-attributes warnings
> (enabled by -Wall) to C and aliases: it warns when particular function
> attributes are missing in the aliases but not in their target, e.g.:
> 
>     void __cold f(void) {}
>     void __alias("f") g(void);
> 
> diagnoses:
> 
>     warning: 'g' specifies less restrictive attribute than
>     its target 'f': 'cold' [-Wmissing-attributes]
> 
> Using __copy(f) we can copy the __cold attribute from f to g:
> 
>     void __cold f(void) {}
>     void __copy(f) __alias("f") g(void);
> 
> This attribute is most useful to deal with situations where an alias
> is declared but we don't know the exact attributes the target has.
> 
> For instance, in the kernel, the widely used module_init/exit macros
> define the init/cleanup_module aliases, but those cannot be marked
> always as __init/__exit since some modules do not have their
> functions marked as such.
> 
> Cc: <stable@vger.kernel.org> # 4.14+
> Suggested-by: Martin Sebor <msebor@gcc.gnu.org>
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
> Signed-off-by: Stefan Agner <stefan@agner.ch>
> ---
>  include/linux/compiler-gcc.h   | 4 ++++
>  include/linux/compiler_types.h | 4 ++++
>  2 files changed, 8 insertions(+)

Can I get a series of these for 4.19.y as well?  I don't want to apply
anything to 4.14 that will regress moving to 4.19.

thanks,

greg k-h

Re: [PATCH BACKPORT 4.14 1/2] Compiler Attributes: add support for __copy (gcc >= 9)

[Stefan Agner]

On 04.06.2019 13:48, Greg KH wrote:
> On Tue, Jun 04, 2019 at 11:21:59AM +0200, Stefan Agner wrote:
>> From: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
>>
>> [ Upstream commit c0d9782f5b6d7157635ae2fd782a4b27d55a6013
>>
>> >From the GCC manual:
>>
>>   copy
>>   copy(function)
>>
>>     The copy attribute applies the set of attributes with which function
>>     has been declared to the declaration of the function to which
>>     the attribute is applied. The attribute is designed for libraries
>>     that define aliases or function resolvers that are expected
>>     to specify the same set of attributes as their targets. The copy
>>     attribute can be used with functions, variables, or types. However,
>>     the kind of symbol to which the attribute is applied (either
>>     function or variable) must match the kind of symbol to which
>>     the argument refers. The copy attribute copies only syntactic and
>>     semantic attributes but not attributes that affect a symbol’s
>>     linkage or visibility such as alias, visibility, or weak.
>>     The deprecated attribute is also not copied.
>>
>>   https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html
>>
>> The upcoming GCC 9 release extends the -Wmissing-attributes warnings
>> (enabled by -Wall) to C and aliases: it warns when particular function
>> attributes are missing in the aliases but not in their target, e.g.:
>>
>>     void __cold f(void) {}
>>     void __alias("f") g(void);
>>
>> diagnoses:
>>
>>     warning: 'g' specifies less restrictive attribute than
>>     its target 'f': 'cold' [-Wmissing-attributes]
>>
>> Using __copy(f) we can copy the __cold attribute from f to g:
>>
>>     void __cold f(void) {}
>>     void __copy(f) __alias("f") g(void);
>>
>> This attribute is most useful to deal with situations where an alias
>> is declared but we don't know the exact attributes the target has.
>>
>> For instance, in the kernel, the widely used module_init/exit macros
>> define the init/cleanup_module aliases, but those cannot be marked
>> always as __init/__exit since some modules do not have their
>> functions marked as such.
>>
>> Cc: <stable@vger.kernel.org> # 4.14+
>> Suggested-by: Martin Sebor <msebor@gcc.gnu.org>
>> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>> Signed-off-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
>> Signed-off-by: Stefan Agner <stefan@agner.ch>
>> ---
>>  include/linux/compiler-gcc.h   | 4 ++++
>>  include/linux/compiler_types.h | 4 ++++
>>  2 files changed, 8 insertions(+)
> 
> Can I get a series of these for 4.19.y as well?  I don't want to apply
> anything to 4.14 that will regress moving to 4.19.

Make sense, will create a backport for 4.19 too.

--
Stefan