[PATCH 1/7] Compiler Attributes: remove unused attributes

[PATCH 1/7] Compiler Attributes: remove unused attributes

[Miguel Ojeda]

__optimize and __deprecate_for_modules are unused in
the whole kernel tree. Simply drop them.

Cc: Eli Friedman <efriedma@codeaurora.org>
Cc: Christopher Li <sparse@chrisli.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Joe Perches <joe@perches.com>
Cc: Dominique Martinet <asmadeus@codewreck.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
---
 include/linux/compiler-gcc.h   | 2 --
 include/linux/compiler.h       | 4 ----
 include/linux/compiler_types.h | 1 -
 3 files changed, 7 deletions(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 763bbad1e258..0a2d06677d83 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -95,8 +95,6 @@
 
 #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
 
-#define __optimize(level)	__attribute__((__optimize__(level)))
-
 #define __compiletime_object_size(obj) __builtin_object_size(obj, 0)
 
 #ifndef __CHECKER__
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 681d866efb1e..7c0157d50964 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -301,10 +301,6 @@ static inline void *offset_to_ptr(const int *off)
 
 #endif /* __ASSEMBLY__ */
 
-#ifndef __optimize
-# define __optimize(level)
-#endif
-
 /* Compile time object size, -1 for unknown */
 #ifndef __compiletime_object_size
 # define __compiletime_object_size(obj) -1
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 3525c179698c..b6534292ea33 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -108,7 +108,6 @@ struct ftrace_likely_data {
 
 /* Don't. Just don't. */
 #define __deprecated
-#define __deprecated_for_modules
 
 #endif /* __KERNEL__ */
 
-- 
2.17.1

[PATCH 2/7] Compiler Attributes: use the no-underscores syntax

[Miguel Ojeda]

The attribute syntax optionally allows to surround attribute names
with "__" in order to avoid collisions with macros of the same name
(see https://gcc.gnu.org/onlinedocs/gcc/Attribute-Syntax.html).

This homogenizes all attributes to use the syntax without underscores.

Cc: Eli Friedman <efriedma@codeaurora.org>
Cc: Christopher Li <sparse@chrisli.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Joe Perches <joe@perches.com>
Cc: Dominique Martinet <asmadeus@codewreck.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
---
 include/linux/compiler-clang.h | 2 +-
 include/linux/compiler-gcc.h   | 4 ++--
 include/linux/compiler.h       | 4 ++--
 include/linux/compiler_types.h | 8 ++++----
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index b1ce500fe8b3..efda74f4eeba 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -43,4 +43,4 @@
 #define barrier() __asm__ __volatile__("" : : : "memory")
 #define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
 #define __assume_aligned(a, ...)	\
-	__attribute__((__assume_aligned__(a, ## __VA_ARGS__)))
+	__attribute__((assume_aligned(a, ## __VA_ARGS__)))
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 0a2d06677d83..dbfbecf703f8 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -132,7 +132,7 @@
 	} while (0)
 
 /* Mark a function definition as prohibited from being cloned. */
-#define __noclone	__attribute__((__noclone__, __optimize__("no-tracer")))
+#define __noclone	__attribute__((noclone, optimize("no-tracer")))
 
 #if defined(RANDSTRUCT_PLUGIN) && !defined(__CHECKER__)
 #define __randomize_layout __attribute__((randomize_layout))
@@ -165,7 +165,7 @@
  * compiler should see some alignment anyway, when the return value is
  * massaged by 'flags = ptr & 3; ptr &= ~3;').
  */
-#define __assume_aligned(a, ...) __attribute__((__assume_aligned__(a, ## __VA_ARGS__)))
+#define __assume_aligned(a, ...) __attribute__((assume_aligned(a, ## __VA_ARGS__)))
 #endif
 
 /*
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 7c0157d50964..e0e55eb3f242 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -23,7 +23,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
 #define __branch_check__(x, expect, is_constant) ({			\
 			long ______r;					\
 			static struct ftrace_likely_data		\
-				__attribute__((__aligned__(4)))		\
+				__attribute__((aligned(4)))		\
 				__attribute__((section("_ftrace_annotated_branch"))) \
 				______f = {				\
 				.data.func = __func__,			\
@@ -59,7 +59,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
 	({								\
 		int ______r;						\
 		static struct ftrace_branch_data			\
-			__attribute__((__aligned__(4)))			\
+			__attribute__((aligned(4)))			\
 			__attribute__((section("_ftrace_branch")))	\
 			______f = {					\
 				.func = __func__,			\
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index b6534292ea33..7cd958360ead 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -159,7 +159,7 @@ struct ftrace_likely_data {
 	 sizeof(t) == sizeof(int) || sizeof(t) == sizeof(long))
 
 #ifndef __attribute_const__
-#define __attribute_const__	__attribute__((__const__))
+#define __attribute_const__	__attribute__((const))
 #endif
 
 #ifndef __noclone
@@ -203,14 +203,14 @@ struct ftrace_likely_data {
 #define __maybe_unused		__attribute__((unused))
 #define __always_unused		__attribute__((unused))
 #define __mode(x)		__attribute__((mode(x)))
-#define __malloc		__attribute__((__malloc__))
-#define __used			__attribute__((__used__))
+#define __malloc		__attribute__((malloc))
+#define __used			__attribute__((used))
 #define __noreturn		__attribute__((noreturn))
 #define __packed		__attribute__((packed))
 #define __weak			__attribute__((weak))
 #define __alias(symbol)		__attribute__((alias(#symbol)))
 #define __cold			__attribute__((cold))
-#define __section(S)		__attribute__((__section__(#S)))
+#define __section(S)		__attribute__((section(#S)))
 
 
 #ifdef CONFIG_ENABLE_MUST_CHECK
-- 
2.17.1

Re: [PATCH 2/7] Compiler Attributes: use the no-underscores syntax

[Nick Desaulniers]

On Fri, Aug 31, 2018 at 10:05 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> The attribute syntax optionally allows to surround attribute names
> with "__" in order to avoid collisions with macros of the same name
> (see https://gcc.gnu.org/onlinedocs/gcc/Attribute-Syntax.html).
>
> This homogenizes all attributes to use the syntax without underscores.
>
> Cc: Eli Friedman <efriedma@codeaurora.org>
> Cc: Christopher Li <sparse@chrisli.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Joe Perches <joe@perches.com>
> Cc: Dominique Martinet <asmadeus@codewreck.org>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
> ---
>  include/linux/compiler-clang.h | 2 +-
>  include/linux/compiler-gcc.h   | 4 ++--
>  include/linux/compiler.h       | 4 ++--
>  include/linux/compiler_types.h | 8 ++++----
>  4 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
> index b1ce500fe8b3..efda74f4eeba 100644
> --- a/include/linux/compiler-clang.h
> +++ b/include/linux/compiler-clang.h
> @@ -43,4 +43,4 @@
>  #define barrier() __asm__ __volatile__("" : : : "memory")
>  #define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
>  #define __assume_aligned(a, ...)       \
> -       __attribute__((__assume_aligned__(a, ## __VA_ARGS__)))
> +       __attribute__((assume_aligned(a, ## __VA_ARGS__)))
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index 0a2d06677d83..dbfbecf703f8 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -132,7 +132,7 @@
>         } while (0)
>
>  /* Mark a function definition as prohibited from being cloned. */
> -#define __noclone      __attribute__((__noclone__, __optimize__("no-tracer")))
> +#define __noclone      __attribute__((noclone, optimize("no-tracer")))
>
>  #if defined(RANDSTRUCT_PLUGIN) && !defined(__CHECKER__)
>  #define __randomize_layout __attribute__((randomize_layout))
> @@ -165,7 +165,7 @@
>   * compiler should see some alignment anyway, when the return value is
>   * massaged by 'flags = ptr & 3; ptr &= ~3;').
>   */
> -#define __assume_aligned(a, ...) __attribute__((__assume_aligned__(a, ## __VA_ARGS__)))
> +#define __assume_aligned(a, ...) __attribute__((assume_aligned(a, ## __VA_ARGS__)))
>  #endif
>
>  /*
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index 7c0157d50964..e0e55eb3f242 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -23,7 +23,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
>  #define __branch_check__(x, expect, is_constant) ({                    \
>                         long ______r;                                   \
>                         static struct ftrace_likely_data                \
> -                               __attribute__((__aligned__(4)))         \
> +                               __attribute__((aligned(4)))             \

Can this be __aligned(4)? As in make use of the newly feature detected
attributes?

>                                 __attribute__((section("_ftrace_annotated_branch"))) \

Sorry to ask for cleanups on code you didn't touch, but since you're
here, can you make this __section("_ftrace_annotated_branch")?

>                                 ______f = {                             \
>                                 .data.func = __func__,                  \
> @@ -59,7 +59,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
>         ({                                                              \
>                 int ______r;                                            \
>                 static struct ftrace_branch_data                        \
> -                       __attribute__((__aligned__(4)))                 \
> +                       __attribute__((aligned(4)))                     \
>                         __attribute__((section("_ftrace_branch")))      \

Ditto.

>                         ______f = {                                     \
>                                 .func = __func__,                       \
> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> index b6534292ea33..7cd958360ead 100644
> --- a/include/linux/compiler_types.h
> +++ b/include/linux/compiler_types.h
> @@ -159,7 +159,7 @@ struct ftrace_likely_data {
>          sizeof(t) == sizeof(int) || sizeof(t) == sizeof(long))
>
>  #ifndef __attribute_const__
> -#define __attribute_const__    __attribute__((__const__))
> +#define __attribute_const__    __attribute__((const))
>  #endif
>
>  #ifndef __noclone
> @@ -203,14 +203,14 @@ struct ftrace_likely_data {
>  #define __maybe_unused         __attribute__((unused))
>  #define __always_unused                __attribute__((unused))
>  #define __mode(x)              __attribute__((mode(x)))
> -#define __malloc               __attribute__((__malloc__))
> -#define __used                 __attribute__((__used__))
> +#define __malloc               __attribute__((malloc))
> +#define __used                 __attribute__((used))
>  #define __noreturn             __attribute__((noreturn))
>  #define __packed               __attribute__((packed))
>  #define __weak                 __attribute__((weak))
>  #define __alias(symbol)                __attribute__((alias(#symbol)))
>  #define __cold                 __attribute__((cold))
> -#define __section(S)           __attribute__((__section__(#S)))
> +#define __section(S)           __attribute__((section(#S)))
>
>
>  #ifdef CONFIG_ENABLE_MUST_CHECK
> --
> 2.17.1
>

With the above changes requested (or follow up patch added to the series):
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

Also,
Series looks great, trying to provide reviews one by one, just very
busy today (and Monday off).

-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 2/7] Compiler Attributes: use the no-underscores syntax

[Miguel Ojeda]

Hi Nick,

On Fri, Aug 31, 2018 at 8:51 PM, Nick Desaulniers
<ndesaulniers@google.com> wrote:
> On Fri, Aug 31, 2018 at 10:05 AM Miguel Ojeda
>> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
>> index 7c0157d50964..e0e55eb3f242 100644
>> --- a/include/linux/compiler.h
>> +++ b/include/linux/compiler.h
>> @@ -23,7 +23,7 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
>>  #define __branch_check__(x, expect, is_constant) ({                    \
>>                         long ______r;                                   \
>>                         static struct ftrace_likely_data                \
>> -                               __attribute__((__aligned__(4)))         \
>> +                               __attribute__((aligned(4)))             \
>
> Can this be __aligned(4)? As in make use of the newly feature detected
> attributes?

I was thinking of doing this in a full conversion over the full kernel
tree in separate patch(es) after the 7th (maybe for v3?). In
particular, aligned and section have a lot of them:
  * 349 ((aligned
  * 66 ((__aligned
  * 76 ((section
  * 48 ((__section

As well as others like:
  * 218 ((unused
  * 14 ((__unused

But indeed, good catches! These ones I can do it directly here since I
am editing them anyway.

>
> With the above changes requested (or follow up patch added to the series):
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>
> Also,
> Series looks great, trying to provide reviews one by one, just very
> busy today (and Monday off).

Thanks a lot!

Cheers,
Miguel

Re: [PATCH 2/7] Compiler Attributes: use the no-underscores syntax

[Rasmus Villemoes]

On 2018-08-31 19:05, Miguel Ojeda wrote:
> The attribute syntax optionally allows to surround attribute names
> with "__" in order to avoid collisions with macros of the same name
> (see https://gcc.gnu.org/onlinedocs/gcc/Attribute-Syntax.html).
> 
> This homogenizes all attributes to use the syntax without underscores.

At the risk of bikeshedding, why not the other way around, exactly
because of what you write above? We have convenience macros anyway, so
those verbose leading/trailing underscores would only be in compiler*.h,
and some of the attribute names are common words that can appear as
#defines. E.g. error is defined
drivers/gpu/drm/nouveau/nvkm/subdev/bios/init.c ; if that used a
BUILD_BUG_ON somewhere, the

#define __compiletime_error(message) __attribute__((error(message)))

would break.

Rasmus

Re: [PATCH 2/7] Compiler Attributes: use the no-underscores syntax

[Miguel Ojeda]

Hi Ramus,

On Fri, Aug 31, 2018 at 11:49 PM, Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
> On 2018-08-31 19:05, Miguel Ojeda wrote:
>> The attribute syntax optionally allows to surround attribute names
>> with "__" in order to avoid collisions with macros of the same name
>> (see https://gcc.gnu.org/onlinedocs/gcc/Attribute-Syntax.html).
>>
>> This homogenizes all attributes to use the syntax without underscores.
>
> At the risk of bikeshedding, why not the other way around, exactly
> because of what you write above? We have convenience macros anyway, so
> those verbose leading/trailing underscores would only be in compiler*.h,
> and some of the attribute names are common words that can appear as
> #defines. E.g. error is defined
> drivers/gpu/drm/nouveau/nvkm/subdev/bios/init.c ; if that used a
> BUILD_BUG_ON somewhere, the
>
> #define __compiletime_error(message) __attribute__((error(message)))
>
> would break.

Indeed, if we are afraid of such cases, we should go with the __name__
syntax. I was trying to go first for the "cleanest"/simplest way
first, considering no one should be defining many such macros as
"error" (and in lower case, at that). But indeed, good catch!

Grepping for those in compiler_attributes.h (which, as you point out,
are not all of them) yields:

arch/parisc/boot/compressed/misc.c:#define malloc       malloc_gzip
include/linux/decompress/mm.h:#define malloc(a) kmalloc(a, GFP_KERNEL)
lib/inflate.c:#define malloc(a) kmalloc(a, GFP_KERNEL)
include/linux/compiler_types.h:#define noinline_for_stack noinline
include/linux/raid/pq.h:#define noinline __attribute__((noinline))
tools/include/linux/compiler.h:#define noinline
arch/mips/sgi-ip27/ip27-reset.c:#define noreturn while(1);
                 /* Silence gcc.  */

Cheers,
Miguel

Re: [PATCH 2/7] Compiler Attributes: use the no-underscores syntax

[Miguel Ojeda]

On Sat, Sep 1, 2018 at 12:10 AM, Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
> arch/parisc/boot/compressed/misc.c:#define malloc       malloc_gzip
> include/linux/decompress/mm.h:#define malloc(a) kmalloc(a, GFP_KERNEL)
> lib/inflate.c:#define malloc(a) kmalloc(a, GFP_KERNEL)
> include/linux/compiler_types.h:#define noinline_for_stack noinline
> include/linux/raid/pq.h:#define noinline __attribute__((noinline))
> tools/include/linux/compiler.h:#define noinline
> arch/mips/sgi-ip27/ip27-reset.c:#define noreturn while(1);

A better list, searching for all attributes used anywhere in the kernel:

git grep -E '^\s*#define\s+(address_space|alias|aligned|always_inline|assume_aligned|bitwise|bnd_legacy|cold|common|const|constructor|context|deprecated|designated_init|destructor|error|externally_visible|flatten|force|format|format|gnu_inline|hot|hotpatch|indirect_branch|latent_entropy|long_call|malloc|may_alias|mode|model|naked|nocast|noclone|noderef|noinline|no_instrument_function|nonnull|no_randomize_layout|noreturn|no_sanitize_address|optimize|packed|pure|randomize_layout|regparm|require_context|safe|section|syscall_linkage|target|tls_model|unused|used|user|vector_size|visibility|warning|warn_unused_result|weak)(\(|\s|$)'

arch/mips/sgi-ip27/ip27-reset.c:#define noreturn while(1);
                 /* Silence gcc.  */
arch/parisc/boot/compressed/misc.c:#define malloc       malloc_gzip
arch/powerpc/xmon/ansidecl.h:#define    const
drivers/gpu/drm/nouveau/nvkm/subdev/bios/init.c:#define error(fmt,
args...) bioslog(ERROR, fmt, ##args)
include/linux/compiler_attributes.h:#define   noinline
__attribute__((noinline))
include/linux/decompress/mm.h:#define malloc(a) kmalloc(a, GFP_KERNEL)
include/linux/raid/pq.h:#define noinline __attribute__((noinline))
lib/inflate.c:#define malloc(a) kmalloc(a, GFP_KERNEL)
tools/include/linux/compiler-gcc.h:#define  noinline
__attribute__((noinline))
tools/include/linux/compiler.h:#define noinline
tools/testing/selftests/futex/include/logging.h:#define error(message,
err, args...) \

None of these should make a problem. And it would may avoid people
using such common-name-macros in the future ;-)

Cheers,
Miguel

Re: [PATCH 2/7] Compiler Attributes: use the no-underscores syntax

[Geert Uytterhoeven]

Hi Miguel,

On Sat, Sep 1, 2018 at 12:41 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
> On Sat, Sep 1, 2018 at 12:10 AM, Miguel Ojeda
> <miguel.ojeda.sandonis@gmail.com> wrote:
> > arch/parisc/boot/compressed/misc.c:#define malloc       malloc_gzip
> > include/linux/decompress/mm.h:#define malloc(a) kmalloc(a, GFP_KERNEL)
> > lib/inflate.c:#define malloc(a) kmalloc(a, GFP_KERNEL)
> > include/linux/compiler_types.h:#define noinline_for_stack noinline
> > include/linux/raid/pq.h:#define noinline __attribute__((noinline))
> > tools/include/linux/compiler.h:#define noinline
> > arch/mips/sgi-ip27/ip27-reset.c:#define noreturn while(1);
>
> A better list, searching for all attributes used anywhere in the kernel:
>
> git grep -E '^\s*#define\s+(address_space|alias|aligned|always_inline|assume_aligned|bitwise|bnd_legacy|cold|common|const|constructor|context|deprecated|designated_init|destructor|error|externally_visible|flatten|force|format|format|gnu_inline|hot|hotpatch|indirect_branch|latent_entropy|long_call|malloc|may_alias|mode|model|naked|nocast|noclone|noderef|noinline|no_instrument_function|nonnull|no_randomize_layout|noreturn|no_sanitize_address|optimize|packed|pure|randomize_layout|regparm|require_context|safe|section|syscall_linkage|target|tls_model|unused|used|user|vector_size|visibility|warning|warn_unused_result|weak)(\(|\s|$)'
>
> arch/mips/sgi-ip27/ip27-reset.c:#define noreturn while(1);
>                  /* Silence gcc.  */
> arch/parisc/boot/compressed/misc.c:#define malloc       malloc_gzip
> arch/powerpc/xmon/ansidecl.h:#define    const
> drivers/gpu/drm/nouveau/nvkm/subdev/bios/init.c:#define error(fmt,
> args...) bioslog(ERROR, fmt, ##args)
> include/linux/compiler_attributes.h:#define   noinline
> __attribute__((noinline))
> include/linux/decompress/mm.h:#define malloc(a) kmalloc(a, GFP_KERNEL)
> include/linux/raid/pq.h:#define noinline __attribute__((noinline))
> lib/inflate.c:#define malloc(a) kmalloc(a, GFP_KERNEL)
> tools/include/linux/compiler-gcc.h:#define  noinline
> __attribute__((noinline))
> tools/include/linux/compiler.h:#define noinline
> tools/testing/selftests/futex/include/logging.h:#define error(message,
> err, args...) \
>
> None of these should make a problem. And it would may avoid people
> using such common-name-macros in the future ;-)

That's very fragile.
Who knows all (current and future) attribute names by heart, and that defining
them could have such side effects?

At least "alias", "force", "format", "mode", "model", "optimize", "target" are
names I wouldn't hesitate to use as a macro name in a driver...

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH 2/7] Compiler Attributes: use the no-underscores syntax

[Miguel Ojeda]

Hi Geert,

On Mon, Sep 3, 2018 at 8:42 AM, Geert Uytterhoeven <geert@linux-m68k.org> wrote:
> Hi Miguel,
>
> On Sat, Sep 1, 2018 at 12:41 AM Miguel Ojeda
> <miguel.ojeda.sandonis@gmail.com> wrote:
>> On Sat, Sep 1, 2018 at 12:10 AM, Miguel Ojeda
>> <miguel.ojeda.sandonis@gmail.com> wrote:
>> > arch/parisc/boot/compressed/misc.c:#define malloc       malloc_gzip
>> > include/linux/decompress/mm.h:#define malloc(a) kmalloc(a, GFP_KERNEL)
>> > lib/inflate.c:#define malloc(a) kmalloc(a, GFP_KERNEL)
>> > include/linux/compiler_types.h:#define noinline_for_stack noinline
>> > include/linux/raid/pq.h:#define noinline __attribute__((noinline))
>> > tools/include/linux/compiler.h:#define noinline
>> > arch/mips/sgi-ip27/ip27-reset.c:#define noreturn while(1);
>>
>> A better list, searching for all attributes used anywhere in the kernel:
>>
>> git grep -E '^\s*#define\s+(address_space|alias|aligned|always_inline|assume_aligned|bitwise|bnd_legacy|cold|common|const|constructor|context|deprecated|designated_init|destructor|error|externally_visible|flatten|force|format|format|gnu_inline|hot|hotpatch|indirect_branch|latent_entropy|long_call|malloc|may_alias|mode|model|naked|nocast|noclone|noderef|noinline|no_instrument_function|nonnull|no_randomize_layout|noreturn|no_sanitize_address|optimize|packed|pure|randomize_layout|regparm|require_context|safe|section|syscall_linkage|target|tls_model|unused|used|user|vector_size|visibility|warning|warn_unused_result|weak)(\(|\s|$)'
>>
>> arch/mips/sgi-ip27/ip27-reset.c:#define noreturn while(1);
>>                  /* Silence gcc.  */
>> arch/parisc/boot/compressed/misc.c:#define malloc       malloc_gzip
>> arch/powerpc/xmon/ansidecl.h:#define    const
>> drivers/gpu/drm/nouveau/nvkm/subdev/bios/init.c:#define error(fmt,
>> args...) bioslog(ERROR, fmt, ##args)
>> include/linux/compiler_attributes.h:#define   noinline
>> __attribute__((noinline))
>> include/linux/decompress/mm.h:#define malloc(a) kmalloc(a, GFP_KERNEL)
>> include/linux/raid/pq.h:#define noinline __attribute__((noinline))
>> lib/inflate.c:#define malloc(a) kmalloc(a, GFP_KERNEL)
>> tools/include/linux/compiler-gcc.h:#define  noinline
>> __attribute__((noinline))
>> tools/include/linux/compiler.h:#define noinline
>> tools/testing/selftests/futex/include/logging.h:#define error(message,
>> err, args...) \
>>
>> None of these should make a problem. And it would may avoid people
>> using such common-name-macros in the future ;-)
>
> That's very fragile.
> Who knows all (current and future) attribute names by heart, and that defining
> them could have such side effects?

Please see the email I sent after that one -- I clarified that this
was more about the current state and that in v3 it will be changed, if
only to avoid surprises in the future for ourselves. I agree with you
and, as Rasmus pointed out, we will use the macros anyway.

>
> At least "alias", "force", "format", "mode", "model", "optimize", "target" are
> names I wouldn't hesitate to use as a macro name in a driver...

A macro with a lowercase, short & common name is not the best idea in
my opinion; if only because of clarity for future readers. Unless it
really improved the code readability for some reason in a particular
case, I would avoid it. But to each his own! :-)

Thanks for reviewing!

Cheers,
Miguel

[PATCH 3/7] Compiler Attributes: remove unneeded tests

[Miguel Ojeda]

Attributes const and always_inline have tests around them
which are unneeded, since they are supported by gcc >= 4.6,
clang >= 3 and icc >= 13. See https://godbolt.org/z/no0OZH

In the case of gnu_inline, we do not need to test for
__GNUC_STDC_INLINE__ because, regardless of the current
inlining behavior, we can simply always force the old
GCC inlining behavior by using the attribute in all cases.

Cc: Eli Friedman <efriedma@codeaurora.org>
Cc: Christopher Li <sparse@chrisli.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Joe Perches <joe@perches.com>
Cc: Dominique Martinet <asmadeus@codewreck.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
---
 include/linux/compiler_types.h | 19 ++-----------------
 1 file changed, 2 insertions(+), 17 deletions(-)

diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 7cd958360ead..3662b19599fc 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -158,10 +158,6 @@ struct ftrace_likely_data {
 	(sizeof(t) == sizeof(char) || sizeof(t) == sizeof(short) || \
 	 sizeof(t) == sizeof(int) || sizeof(t) == sizeof(long))
 
-#ifndef __attribute_const__
-#define __attribute_const__	__attribute__((const))
-#endif
-
 #ifndef __noclone
 #define __noclone
 #endif
@@ -196,6 +192,7 @@ struct ftrace_likely_data {
  * [...]
  */
 #define __pure			__attribute__((pure))
+#define __const			__attribute__((const))
 #define __aligned(x)		__attribute__((aligned(x)))
 #define __aligned_largest	__attribute__((aligned))
 #define __printf(a, b)		__attribute__((format(printf, a, b)))
@@ -227,17 +224,7 @@ struct ftrace_likely_data {
 
 #define __compiler_offsetof(a, b)	__builtin_offsetof(a, b)
 
-/*
- * Feature detection for gnu_inline (gnu89 extern inline semantics). Either
- * __GNUC_STDC_INLINE__ is defined (not using gnu89 extern inline semantics,
- * and we opt in to the gnu89 semantics), or __GNUC_STDC_INLINE__ is not
- * defined so the gnu89 semantics are the default.
- */
-#ifdef __GNUC_STDC_INLINE__
-# define __gnu_inline	__attribute__((gnu_inline))
-#else
-# define __gnu_inline
-#endif
+#define __gnu_inline	__attribute__((gnu_inline))
 
 /*
  * Force always-inline if the user requests it so via the .config.
@@ -263,9 +250,7 @@ struct ftrace_likely_data {
 #define __inline inline
 #define noinline	__attribute__((noinline))
 
-#ifndef __always_inline
 #define __always_inline inline __attribute__((always_inline))
-#endif
 
 /*
  * Rather then using noinline to prevent stack consumption, use
-- 
2.17.1

Re: [PATCH 3/7] Compiler Attributes: remove unneeded tests

[Nick Desaulniers]

On Fri, Aug 31, 2018 at 10:05 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> Attributes const and always_inline have tests around them
> which are unneeded, since they are supported by gcc >= 4.6,
> clang >= 3 and icc >= 13. See https://godbolt.org/z/no0OZH
>
> In the case of gnu_inline, we do not need to test for
> __GNUC_STDC_INLINE__ because, regardless of the current
> inlining behavior, we can simply always force the old
> GCC inlining behavior by using the attribute in all cases.
>
> Cc: Eli Friedman <efriedma@codeaurora.org>
> Cc: Christopher Li <sparse@chrisli.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Joe Perches <joe@perches.com>
> Cc: Dominique Martinet <asmadeus@codewreck.org>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
> ---
>  include/linux/compiler_types.h | 19 ++-----------------
>  1 file changed, 2 insertions(+), 17 deletions(-)
>
> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> index 7cd958360ead..3662b19599fc 100644
> --- a/include/linux/compiler_types.h
> +++ b/include/linux/compiler_types.h
> @@ -158,10 +158,6 @@ struct ftrace_likely_data {
>         (sizeof(t) == sizeof(char) || sizeof(t) == sizeof(short) || \
>          sizeof(t) == sizeof(int) || sizeof(t) == sizeof(long))
>
> -#ifndef __attribute_const__
> -#define __attribute_const__    __attribute__((const))
> -#endif
> -

If there's a v2, you could order this before the patch that removed
the underscores so that patch is smaller.  Doesn't really matter
though.

>  #ifndef __noclone
>  #define __noclone
>  #endif
> @@ -196,6 +192,7 @@ struct ftrace_likely_data {
>   * [...]
>   */
>  #define __pure                 __attribute__((pure))
> +#define __const                        __attribute__((const))
>  #define __aligned(x)           __attribute__((aligned(x)))
>  #define __aligned_largest      __attribute__((aligned))
>  #define __printf(a, b)         __attribute__((format(printf, a, b)))
> @@ -227,17 +224,7 @@ struct ftrace_likely_data {
>
>  #define __compiler_offsetof(a, b)      __builtin_offsetof(a, b)
>
> -/*
> - * Feature detection for gnu_inline (gnu89 extern inline semantics). Either
> - * __GNUC_STDC_INLINE__ is defined (not using gnu89 extern inline semantics,
> - * and we opt in to the gnu89 semantics), or __GNUC_STDC_INLINE__ is not
> - * defined so the gnu89 semantics are the default.
> - */
> -#ifdef __GNUC_STDC_INLINE__
> -# define __gnu_inline  __attribute__((gnu_inline))
> -#else
> -# define __gnu_inline
> -#endif
> +#define __gnu_inline   __attribute__((gnu_inline))

Can this be moved into the block above with all the rest of required attributes?

>
>  /*
>   * Force always-inline if the user requests it so via the .config.
> @@ -263,9 +250,7 @@ struct ftrace_likely_data {
>  #define __inline inline
>  #define noinline       __attribute__((noinline))
>
> -#ifndef __always_inline
>  #define __always_inline inline __attribute__((always_inline))
> -#endif

Ditto.

>
>  /*
>   * Rather then using noinline to prevent stack consumption, use
> --
> 2.17.1
>

With the requested changes,
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 3/7] Compiler Attributes: remove unneeded tests

[kbuild test robot]

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

Hi Miguel,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.19-rc1 next-20180831]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Miguel-Ojeda/Compiler-Attributes-remove-unused-attributes/20180901-125644
config: i386-tinyconfig (attached as .config)
compiler: gcc-7 (Debian 7.3.0-16) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

Note: the linux-review/Miguel-Ojeda/Compiler-Attributes-remove-unused-attributes/20180901-125644 HEAD 4327b9ceea1b6be41cbce2c51370a365fc6a52cc builds fine.
      It only hurts bisectibility.

All error/warnings (new ones prefixed by >>):

   In file included from include/uapi/linux/swab.h:7:0,
                    from include/linux/swab.h:5,
                    from include/uapi/linux/byteorder/little_endian.h:13,
                    from include/linux/byteorder/little_endian.h:5,
                    from arch/x86/include/uapi/asm/byteorder.h:5,
                    from include/asm-generic/bitops/le.h:6,
                    from arch/x86/include/asm/bitops.h:521,
                    from include/linux/bitops.h:19,
                    from include/linux/kernel.h:11,
                    from include/asm-generic/bug.h:18,
                    from arch/x86/include/asm/bug.h:83,
                    from include/linux/bug.h:5,
                    from include/linux/page-flags.h:10,
                    from kernel/bounds.c:10:
>> arch/x86/include/uapi/asm/swab.h:8:15: error: unknown type name '__attribute_const__'
    static inline __attribute_const__ __u32 __arch_swab32(__u32 val)
                  ^~~~~~~~~~~~~~~~~~~
>> arch/x86/include/uapi/asm/swab.h:8:41: error: expected '=', ',', ';', 'asm' or '__attribute__' before '__arch_swab32'
    static inline __attribute_const__ __u32 __arch_swab32(__u32 val)
                                            ^~~~~~~~~~~~~
   arch/x86/include/uapi/asm/swab.h:15:15: error: unknown type name '__attribute_const__'
    static inline __attribute_const__ __u64 __arch_swab64(__u64 val)
                  ^~~~~~~~~~~~~~~~~~~
>> arch/x86/include/uapi/asm/swab.h:15:41: error: expected '=', ',', ';', 'asm' or '__attribute__' before '__arch_swab64'
    static inline __attribute_const__ __u64 __arch_swab64(__u64 val)
                                            ^~~~~~~~~~~~~
   In file included from include/linux/swab.h:5:0,
                    from include/uapi/linux/byteorder/little_endian.h:13,
                    from include/linux/byteorder/little_endian.h:5,
                    from arch/x86/include/uapi/asm/byteorder.h:5,
                    from include/asm-generic/bitops/le.h:6,
                    from arch/x86/include/asm/bitops.h:521,
                    from include/linux/bitops.h:19,
                    from include/linux/kernel.h:11,
                    from include/asm-generic/bug.h:18,
                    from arch/x86/include/asm/bug.h:83,
                    from include/linux/bug.h:5,
                    from include/linux/page-flags.h:10,
                    from kernel/bounds.c:10:
>> include/uapi/linux/swab.h:47:15: error: unknown type name '__attribute_const__'
    static inline __attribute_const__ __u16 __fswab16(__u16 val)
                  ^~~~~~~~~~~~~~~~~~~
>> include/uapi/linux/swab.h:47:41: error: expected '=', ',', ';', 'asm' or '__attribute__' before '__fswab16'
    static inline __attribute_const__ __u16 __fswab16(__u16 val)
                                            ^~~~~~~~~
   include/uapi/linux/swab.h:56:15: error: unknown type name '__attribute_const__'
    static inline __attribute_const__ __u32 __fswab32(__u32 val)
                  ^~~~~~~~~~~~~~~~~~~
>> include/uapi/linux/swab.h:56:41: error: expected '=', ',', ';', 'asm' or '__attribute__' before '__fswab32'
    static inline __attribute_const__ __u32 __fswab32(__u32 val)
                                            ^~~~~~~~~
   include/uapi/linux/swab.h:65:15: error: unknown type name '__attribute_const__'
    static inline __attribute_const__ __u64 __fswab64(__u64 val)
                  ^~~~~~~~~~~~~~~~~~~
>> include/uapi/linux/swab.h:65:41: error: expected '=', ',', ';', 'asm' or '__attribute__' before '__fswab64'
    static inline __attribute_const__ __u64 __fswab64(__u64 val)
                                            ^~~~~~~~~
   include/uapi/linux/swab.h:78:15: error: unknown type name '__attribute_const__'
    static inline __attribute_const__ __u32 __fswahw32(__u32 val)
                  ^~~~~~~~~~~~~~~~~~~
>> include/uapi/linux/swab.h:78:41: error: expected '=', ',', ';', 'asm' or '__attribute__' before '__fswahw32'
    static inline __attribute_const__ __u32 __fswahw32(__u32 val)
                                            ^~~~~~~~~~
   include/uapi/linux/swab.h:87:15: error: unknown type name '__attribute_const__'
    static inline __attribute_const__ __u32 __fswahb32(__u32 val)
                  ^~~~~~~~~~~~~~~~~~~
>> include/uapi/linux/swab.h:87:41: error: expected '=', ',', ';', 'asm' or '__attribute__' before '__fswahb32'
    static inline __attribute_const__ __u32 __fswahb32(__u32 val)
                                            ^~~~~~~~~~
   include/uapi/linux/swab.h: In function '__swahw32p':
>> include/uapi/linux/swab.h:144:2: error: implicit declaration of function '__fswahw32'; did you mean '__swahw32'? [-Werror=implicit-function-declaration]
     __fswahw32(x))
     ^
>> include/uapi/linux/swab.h:207:9: note: in expansion of macro '__swahw32'
     return __swahw32(*p);
            ^~~~~~~~~
   include/uapi/linux/swab.h: In function '__swahb32p':
>> include/uapi/linux/swab.h:155:2: error: implicit declaration of function '__fswahb32'; did you mean '__swahb32'? [-Werror=implicit-function-declaration]
     __fswahb32(x))
     ^
>> include/uapi/linux/swab.h:222:9: note: in expansion of macro '__swahb32'
     return __swahb32(*p);
            ^~~~~~~~~
   In file included from include/linux/kernel.h:12:0,
                    from include/asm-generic/bug.h:18,
                    from arch/x86/include/asm/bug.h:83,
                    from include/linux/bug.h:5,
                    from include/linux/page-flags.h:10,
                    from kernel/bounds.c:10:
   include/linux/log2.h: At top level:
>> include/linux/log2.h:202:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'int'
    int __order_base_2(unsigned long n)
    ^~~
   cc1: some warnings being treated as errors
   make[2]: *** [kernel/bounds.s] Error 1
   make[2]: Target '__build' not remade because of errors.
   make[1]: *** [prepare0] Error 2
   make[1]: Target 'prepare' not remade because of errors.
   make: *** [sub-make] Error 2

vim +/__attribute_const__ +8 arch/x86/include/uapi/asm/swab.h

5d30a683 arch/x86/include/asm/swab.h Harvey Harrison 2009-01-06   7  
5d30a683 arch/x86/include/asm/swab.h Harvey Harrison 2009-01-06  @8  static inline __attribute_const__ __u32 __arch_swab32(__u32 val)
5d30a683 arch/x86/include/asm/swab.h Harvey Harrison 2009-01-06   9  {
e5bb8ad8 arch/x86/include/asm/swab.h H. Peter Anvin  2012-11-28  10  	asm("bswapl %0" : "=r" (val) : "0" (val));
5d30a683 arch/x86/include/asm/swab.h Harvey Harrison 2009-01-06  11  	return val;
5d30a683 arch/x86/include/asm/swab.h Harvey Harrison 2009-01-06  12  }
5d30a683 arch/x86/include/asm/swab.h Harvey Harrison 2009-01-06  13  #define __arch_swab32 __arch_swab32
5d30a683 arch/x86/include/asm/swab.h Harvey Harrison 2009-01-06  14  
5d30a683 arch/x86/include/asm/swab.h Harvey Harrison 2009-01-06 @15  static inline __attribute_const__ __u64 __arch_swab64(__u64 val)
5d30a683 arch/x86/include/asm/swab.h Harvey Harrison 2009-01-06  16  {
5d30a683 arch/x86/include/asm/swab.h Harvey Harrison 2009-01-06  17  #ifdef __i386__
5d30a683 arch/x86/include/asm/swab.h Harvey Harrison 2009-01-06  18  	union {
5d30a683 arch/x86/include/asm/swab.h Harvey Harrison 2009-01-06  19  		struct {
5d30a683 arch/x86/include/asm/swab.h Harvey Harrison 2009-01-06  20  			__u32 a;
5d30a683 arch/x86/include/asm/swab.h Harvey Harrison 2009-01-06  21  			__u32 b;
5d30a683 arch/x86/include/asm/swab.h Harvey Harrison 2009-01-06  22  		} s;
5d30a683 arch/x86/include/asm/swab.h Harvey Harrison 2009-01-06  23  		__u64 u;
5d30a683 arch/x86/include/asm/swab.h Harvey Harrison 2009-01-06  24  	} v;
5d30a683 arch/x86/include/asm/swab.h Harvey Harrison 2009-01-06  25  	v.u = val;
5d30a683 arch/x86/include/asm/swab.h Harvey Harrison 2009-01-06  26  	asm("bswapl %0 ; bswapl %1 ; xchgl %0,%1"
5d30a683 arch/x86/include/asm/swab.h Harvey Harrison 2009-01-06  27  	    : "=r" (v.s.a), "=r" (v.s.b)
5d30a683 arch/x86/include/asm/swab.h Harvey Harrison 2009-01-06  28  	    : "0" (v.s.a), "1" (v.s.b));
5d30a683 arch/x86/include/asm/swab.h Harvey Harrison 2009-01-06  29  	return v.u;
5d30a683 arch/x86/include/asm/swab.h Harvey Harrison 2009-01-06  30  #else /* __i386__ */
e5bb8ad8 arch/x86/include/asm/swab.h H. Peter Anvin  2012-11-28  31  	asm("bswapq %0" : "=r" (val) : "0" (val));
5d30a683 arch/x86/include/asm/swab.h Harvey Harrison 2009-01-06  32  	return val;
5d30a683 arch/x86/include/asm/swab.h Harvey Harrison 2009-01-06  33  #endif
5d30a683 arch/x86/include/asm/swab.h Harvey Harrison 2009-01-06  34  }
5d30a683 arch/x86/include/asm/swab.h Harvey Harrison 2009-01-06  35  #define __arch_swab64 __arch_swab64
5d30a683 arch/x86/include/asm/swab.h Harvey Harrison 2009-01-06  36  

:::::: The code at line 8 was first introduced by commit
:::::: 5d30a683888c60b8f93bef3ddc139d1a91ca58f4 x86: introduce asm/swab.h

:::::: TO: Harvey Harrison <harvey.harrison@gmail.com>
:::::: CC: Linus Torvalds <torvalds@linux-foundation.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6508 bytes --]

Re: [PATCH 3/7] Compiler Attributes: remove unneeded tests

[kbuild test robot]

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

Hi Miguel,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v4.19-rc1 next-20180831]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Miguel-Ojeda/Compiler-Attributes-remove-unused-attributes/20180901-125644
config: sparc64-allnoconfig (attached as .config)
compiler: sparc64-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=sparc64 

Note: the linux-review/Miguel-Ojeda/Compiler-Attributes-remove-unused-attributes/20180901-125644 HEAD 4327b9ceea1b6be41cbce2c51370a365fc6a52cc builds fine.
      It only hurts bisectibility.

All error/warnings (new ones prefixed by >>):

   WARNING: unmet direct dependencies detected for COMPAT_BINFMT_ELF
   Depends on COMPAT && BINFMT_ELF
   Selected by
   - COMPAT && SPARC64
   In file included from include/linux/swab.h:5:0,
   from include/uapi/linux/byteorder/big_endian.h:13,
   from include/linux/byteorder/big_endian.h:5,
   from arch/sparc/include/uapi/asm/byteorder.h:5,
   from arch/sparc/include/asm/bitops_64.h:16,
   from arch/sparc/include/asm/bitops.h:5,
   from include/linux/bitops.h:19,
   from include/linux/kernel.h:11,
   from include/asm-generic/bug.h:18,
   from arch/sparc/include/asm/bug.h:25,
   from include/linux/bug.h:5,
   from include/linux/page-flags.h:10,
   from kernel/bounds.c:10:
   include/uapi/linux/swab.h:47:15: error: unknown type name '__attribute_const__'
   static inline __attribute_const__ __u16 __fswab16(__u16 val)
   ^~~~~~~~~~~~~~~~~~~
   include/uapi/linux/swab.h:47:41: error: expected '=', ',', ';', 'asm' or '__attribute__' before '__fswab16'
   static inline __attribute_const__ __u16 __fswab16(__u16 val)
   ^~~~~~~~~
   include/uapi/linux/swab.h:56:15: error: unknown type name '__attribute_const__'
   static inline __attribute_const__ __u32 __fswab32(__u32 val)
   ^~~~~~~~~~~~~~~~~~~
   include/uapi/linux/swab.h:56:41: error: expected '=', ',', ';', 'asm' or '__attribute__' before '__fswab32'
   static inline __attribute_const__ __u32 __fswab32(__u32 val)
   ^~~~~~~~~
   include/uapi/linux/swab.h:65:15: error: unknown type name '__attribute_const__'
   static inline __attribute_const__ __u64 __fswab64(__u64 val)
   ^~~~~~~~~~~~~~~~~~~
   include/uapi/linux/swab.h:65:41: error: expected '=', ',', ';', 'asm' or '__attribute__' before '__fswab64'
   static inline __attribute_const__ __u64 __fswab64(__u64 val)
   ^~~~~~~~~
   include/uapi/linux/swab.h:78:15: error: unknown type name '__attribute_const__'
   static inline __attribute_const__ __u32 __fswahw32(__u32 val)
   ^~~~~~~~~~~~~~~~~~~
   include/uapi/linux/swab.h:78:41: error: expected '=', ',', ';', 'asm' or '__attribute__' before '__fswahw32'
   static inline __attribute_const__ __u32 __fswahw32(__u32 val)
   ^~~~~~~~~~
   include/uapi/linux/swab.h:87:15: error: unknown type name '__attribute_const__'
   static inline __attribute_const__ __u32 __fswahb32(__u32 val)
   ^~~~~~~~~~~~~~~~~~~
   include/uapi/linux/swab.h:87:41: error: expected '=', ',', ';', 'asm' or '__attribute__' before '__fswahb32'
   static inline __attribute_const__ __u32 __fswahb32(__u32 val)
   ^~~~~~~~~~
   include/uapi/linux/swab.h: In function '__swahw32p':
>> include/uapi/linux/swab.h:144:2: error: implicit declaration of function '__fswahw32'; did you mean
   __fswahw32(x))
   ^
   include/uapi/linux/swab.h:207:9: note: in expansion of macro '__swahw32'
   return
   ^~~~~~~~~
   include/uapi/linux/swab.h: In function '__swahb32p':
>> include/uapi/linux/swab.h:155:2: error: implicit declaration of function '__fswahb32'; did you mean
   __fswahb32(x))
   ^
   include/uapi/linux/swab.h:222:9: note: in expansion of macro '__swahb32'
   return
   ^~~~~~~~~
   include/linux/byteorder/generic.h: In function 'le16_add_cpu':
>> include/uapi/linux/swab.h:106:2: error: implicit declaration of function '__fswab16'; did you mean
   __fswab16(x))
   ^
   include/uapi/linux/swab.h:104:32: note: in definition of macro '__swab16'
   (__builtin_constant_p((__u16)(x)) ^
   include/linux/byteorder/generic.h:90:21: note: in expansion of macro '__cpu_to_le16'
   #define cpu_to_le16 __cpu_to_le16
   ^~~~~~~~~~~~~
   include/uapi/linux/byteorder/big_endian.h:36:26: note: in expansion of macro '__swab16'
   #define __le16_to_cpu(x) __swab16((__force __u16)(__le16)(x))
   ^~~~~~~~
   include/linux/byteorder/generic.h:91:21: note: in expansion of macro '__le16_to_cpu'
   #define le16_to_cpu __le16_to_cpu
   ^~~~~~~~~~~~~
>> include/linux/byteorder/generic.h:146:21: note: in expansion of macro 'le16_to_cpu'
   = + val);
   ^~~~~~~~~~~
   include/linux/byteorder/generic.h: In function 'le32_add_cpu':
>> include/uapi/linux/swab.h:119:2: error: implicit declaration of function '__fswab32'; did you mean
   __fswab32(x))
   ^
   include/uapi/linux/swab.h:117:32: note: in definition of macro '__swab32'
   (__builtin_constant_p((__u32)(x)) ^
   include/linux/byteorder/generic.h:88:21: note: in expansion of macro '__cpu_to_le32'
   #define cpu_to_le32 __cpu_to_le32
   ^~~~~~~~~~~~~
   include/uapi/linux/byteorder/big_endian.h:34:26: note: in expansion of macro '__swab32'
   #define __le32_to_cpu(x) __swab32((__force __u32)(__le32)(x))
   ^~~~~~~~
   include/linux/byteorder/generic.h:89:21: note: in expansion of macro '__le32_to_cpu'
   #define le32_to_cpu __le32_to_cpu
   ^~~~~~~~~~~~~
>> include/linux/byteorder/generic.h:151:21: note: in expansion of macro 'le32_to_cpu'
   = + val);
   ^~~~~~~~~~~
   include/linux/byteorder/generic.h: In function 'le64_add_cpu':
>> include/uapi/linux/swab.h:132:2: error: implicit declaration of function '__fswab64'; did you mean
   __fswab64(x))
   ^
   include/uapi/linux/swab.h:130:32: note: in definition of macro '__swab64'
   (__builtin_constant_p((__u64)(x)) ^
   include/linux/byteorder/generic.h:86:21: note: in expansion of macro '__cpu_to_le64'
   #define cpu_to_le64 __cpu_to_le64
   ^~~~~~~~~~~~~
   include/uapi/linux/byteorder/big_endian.h:32:26: note: in expansion of macro '__swab64'
   #define __le64_to_cpu(x) __swab64((__force __u64)(__le64)(x))
   ^~~~~~~~
   include/linux/byteorder/generic.h:87:21: note: in expansion of macro '__le64_to_cpu'
   #define le64_to_cpu __le64_to_cpu
   ^~~~~~~~~~~~~
>> include/linux/byteorder/generic.h:156:21: note: in expansion of macro 'le64_to_cpu'
   = + val);
   ^~~~~~~~~~~
   In file included from include/linux/kernel.h:12:0,
   from include/asm-generic/bug.h:18,
   from arch/sparc/include/asm/bug.h:25,
   from include/linux/bug.h:5,
   from include/linux/page-flags.h:10,
   from kernel/bounds.c:10:
   include/linux/log2.h: At top level:
   include/linux/log2.h:202:1: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'int'
   int __order_base_2(unsigned long n)
   ^~~
   cc1: some warnings being treated as errors
   Makefile Module.symvers System.map arch block built-in.a certs crypto drivers firmware fs include init ipc kernel lib mm modules.order net scripts security sound source usr virt vmlinux vmlinux.o Error 1
   Target '__build' not remade because of errors.
   Makefile Module.symvers System.map arch block built-in.a certs crypto drivers firmware fs include init ipc kernel lib mm modules.order net scripts security sound source usr virt vmlinux vmlinux.o Error 2
   Target 'prepare' not remade because of errors.
   make: Makefile Module.symvers System.map arch block built-in.a certs crypto drivers firmware fs include init ipc kernel lib mm modules.order net scripts security sound source usr virt vmlinux vmlinux.o Error 2

vim +/__fswahw32 +144 include/uapi/linux/swab.h

607ca46e David Howells 2012-10-13   86  
607ca46e David Howells 2012-10-13  @87  static inline __attribute_const__ __u32 __fswahb32(__u32 val)
607ca46e David Howells 2012-10-13   88  {
607ca46e David Howells 2012-10-13   89  #ifdef __arch_swahb32
607ca46e David Howells 2012-10-13   90  	return __arch_swahb32(val);
607ca46e David Howells 2012-10-13   91  #else
607ca46e David Howells 2012-10-13   92  	return ___constant_swahb32(val);
607ca46e David Howells 2012-10-13   93  #endif
607ca46e David Howells 2012-10-13   94  }
607ca46e David Howells 2012-10-13   95  
607ca46e David Howells 2012-10-13   96  /**
607ca46e David Howells 2012-10-13   97   * __swab16 - return a byteswapped 16-bit value
607ca46e David Howells 2012-10-13   98   * @x: value to byteswap
607ca46e David Howells 2012-10-13   99   */
7322dd75 Arnd Bergmann 2016-05-05  100  #ifdef __HAVE_BUILTIN_BSWAP16__
7322dd75 Arnd Bergmann 2016-05-05  101  #define __swab16(x) (__u16)__builtin_bswap16((__u16)(x))
7322dd75 Arnd Bergmann 2016-05-05  102  #else
607ca46e David Howells 2012-10-13  103  #define __swab16(x)				\
607ca46e David Howells 2012-10-13  104  	(__builtin_constant_p((__u16)(x)) ?	\
607ca46e David Howells 2012-10-13  105  	___constant_swab16(x) :			\
607ca46e David Howells 2012-10-13 @106  	__fswab16(x))
7322dd75 Arnd Bergmann 2016-05-05  107  #endif
607ca46e David Howells 2012-10-13  108  
607ca46e David Howells 2012-10-13  109  /**
607ca46e David Howells 2012-10-13  110   * __swab32 - return a byteswapped 32-bit value
607ca46e David Howells 2012-10-13  111   * @x: value to byteswap
607ca46e David Howells 2012-10-13  112   */
7322dd75 Arnd Bergmann 2016-05-05  113  #ifdef __HAVE_BUILTIN_BSWAP32__
7322dd75 Arnd Bergmann 2016-05-05  114  #define __swab32(x) (__u32)__builtin_bswap32((__u32)(x))
7322dd75 Arnd Bergmann 2016-05-05  115  #else
607ca46e David Howells 2012-10-13  116  #define __swab32(x)				\
607ca46e David Howells 2012-10-13  117  	(__builtin_constant_p((__u32)(x)) ?	\
607ca46e David Howells 2012-10-13  118  	___constant_swab32(x) :			\
607ca46e David Howells 2012-10-13 @119  	__fswab32(x))
7322dd75 Arnd Bergmann 2016-05-05  120  #endif
607ca46e David Howells 2012-10-13  121  
607ca46e David Howells 2012-10-13  122  /**
607ca46e David Howells 2012-10-13  123   * __swab64 - return a byteswapped 64-bit value
607ca46e David Howells 2012-10-13  124   * @x: value to byteswap
607ca46e David Howells 2012-10-13  125   */
7322dd75 Arnd Bergmann 2016-05-05  126  #ifdef __HAVE_BUILTIN_BSWAP64__
7322dd75 Arnd Bergmann 2016-05-05  127  #define __swab64(x) (__u64)__builtin_bswap64((__u64)(x))
7322dd75 Arnd Bergmann 2016-05-05  128  #else
607ca46e David Howells 2012-10-13  129  #define __swab64(x)				\
607ca46e David Howells 2012-10-13  130  	(__builtin_constant_p((__u64)(x)) ?	\
607ca46e David Howells 2012-10-13  131  	___constant_swab64(x) :			\
607ca46e David Howells 2012-10-13 @132  	__fswab64(x))
7322dd75 Arnd Bergmann 2016-05-05  133  #endif
607ca46e David Howells 2012-10-13  134  
607ca46e David Howells 2012-10-13  135  /**
607ca46e David Howells 2012-10-13  136   * __swahw32 - return a word-swapped 32-bit value
607ca46e David Howells 2012-10-13  137   * @x: value to wordswap
607ca46e David Howells 2012-10-13  138   *
607ca46e David Howells 2012-10-13  139   * __swahw32(0x12340000) is 0x00001234
607ca46e David Howells 2012-10-13  140   */
607ca46e David Howells 2012-10-13  141  #define __swahw32(x)				\
607ca46e David Howells 2012-10-13  142  	(__builtin_constant_p((__u32)(x)) ?	\
607ca46e David Howells 2012-10-13  143  	___constant_swahw32(x) :		\
607ca46e David Howells 2012-10-13 @144  	__fswahw32(x))
607ca46e David Howells 2012-10-13  145  
607ca46e David Howells 2012-10-13  146  /**
607ca46e David Howells 2012-10-13  147   * __swahb32 - return a high and low byte-swapped 32-bit value
607ca46e David Howells 2012-10-13  148   * @x: value to byteswap
607ca46e David Howells 2012-10-13  149   *
607ca46e David Howells 2012-10-13  150   * __swahb32(0x12345678) is 0x34127856
607ca46e David Howells 2012-10-13  151   */
607ca46e David Howells 2012-10-13  152  #define __swahb32(x)				\
607ca46e David Howells 2012-10-13  153  	(__builtin_constant_p((__u32)(x)) ?	\
607ca46e David Howells 2012-10-13  154  	___constant_swahb32(x) :		\
607ca46e David Howells 2012-10-13 @155  	__fswahb32(x))
607ca46e David Howells 2012-10-13  156  

:::::: The code at line 144 was first introduced by commit
:::::: 607ca46e97a1b6594b29647d98a32d545c24bdff UAPI: (Scripted) Disintegrate include/linux

:::::: TO: David Howells <dhowells@redhat.com>
:::::: CC: David Howells <dhowells@redhat.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 5089 bytes --]

[PATCH 4/7] Compiler Attributes: homogenize __must_be_array

[Miguel Ojeda]

Different definitions of __must_be_array:

  * gcc: disabled for __CHECKER__

  * clang: same definition as gcc's, but without __CHECKER__

  * intel: the comment claims __builtin_types_compatible_p()
    is unsupported; but icc seems to support it since 13.0.1
    (released in 2012). See https://godbolt.org/z/S0l6QQ

Therefore, we can remove all of them and have a single definition
in compiler.h

Cc: Eli Friedman <efriedma@codeaurora.org>
Cc: Christopher Li <sparse@chrisli.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Joe Perches <joe@perches.com>
Cc: Dominique Martinet <asmadeus@codewreck.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
---
 include/linux/compiler-clang.h | 1 -
 include/linux/compiler-gcc.h   | 7 -------
 include/linux/compiler-intel.h | 3 ---
 include/linux/compiler.h       | 7 +++++++
 4 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index efda74f4eeba..0aee694d3ab8 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -41,6 +41,5 @@
  * compilers, like ICC.
  */
 #define barrier() __asm__ __volatile__("" : : : "memory")
-#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
 #define __assume_aligned(a, ...)	\
 	__attribute__((assume_aligned(a, ## __VA_ARGS__)))
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index dbfbecf703f8..66e1eb8822d9 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -68,13 +68,6 @@
  */
 #define uninitialized_var(x) x = x
 
-#ifdef __CHECKER__
-#define __must_be_array(a)	0
-#else
-/* &a[0] degrades to a pointer: a different type from an array */
-#define __must_be_array(a)	BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
-#endif
-
 #ifdef RETPOLINE
 #define __noretpoline __attribute__((indirect_branch("keep")))
 #endif
diff --git a/include/linux/compiler-intel.h b/include/linux/compiler-intel.h
index 4c7f9befa9f6..18bd4362deaa 100644
--- a/include/linux/compiler-intel.h
+++ b/include/linux/compiler-intel.h
@@ -29,9 +29,6 @@
  */
 #define OPTIMIZER_HIDE_VAR(var) barrier()
 
-/* Intel ECC compiler doesn't support __builtin_types_compatible_p() */
-#define __must_be_array(a) 0
-
 #endif
 
 /* icc has this, but it's called _bswap16 */
diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index e0e55eb3f242..e4a702f99e50 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -357,4 +357,11 @@ static inline void *offset_to_ptr(const int *off)
 	compiletime_assert(__native_word(t),				\
 		"Need native word sized stores/loads for atomicity.")
 
+#ifdef __CHECKER__
+#define __must_be_array(a)	0
+#else
+/* &a[0] degrades to a pointer: a different type from an array */
+#define __must_be_array(a)	BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
+#endif
+
 #endif /* __LINUX_COMPILER_H */
-- 
2.17.1

Re: [PATCH 4/7] Compiler Attributes: homogenize __must_be_array

[Nick Desaulniers]

On Fri, Aug 31, 2018 at 10:05 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> Different definitions of __must_be_array:
>
>   * gcc: disabled for __CHECKER__
>
>   * clang: same definition as gcc's, but without __CHECKER__
>
>   * intel: the comment claims __builtin_types_compatible_p()
>     is unsupported; but icc seems to support it since 13.0.1
>     (released in 2012). See https://godbolt.org/z/S0l6QQ

Indeed.  It seems that if we can't find a contact for ICC, then 13.0.1
(the oldest version supported by godbolt) is a best effort, IMO.  If
we end up adding patches for minimal compiler versions, maybe v13.0.1
is a good minimal for ICC.  But I would really defer to someone who
has an install of ICC (not me).

>
> Therefore, we can remove all of them and have a single definition
> in compiler.h
>
> Cc: Eli Friedman <efriedma@codeaurora.org>
> Cc: Christopher Li <sparse@chrisli.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Joe Perches <joe@perches.com>
> Cc: Dominique Martinet <asmadeus@codewreck.org>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
> ---
>  include/linux/compiler-clang.h | 1 -
>  include/linux/compiler-gcc.h   | 7 -------
>  include/linux/compiler-intel.h | 3 ---
>  include/linux/compiler.h       | 7 +++++++
>  4 files changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
> index efda74f4eeba..0aee694d3ab8 100644
> --- a/include/linux/compiler-clang.h
> +++ b/include/linux/compiler-clang.h
> @@ -41,6 +41,5 @@
>   * compilers, like ICC.
>   */
>  #define barrier() __asm__ __volatile__("" : : : "memory")
> -#define __must_be_array(a) BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
>  #define __assume_aligned(a, ...)       \
>         __attribute__((assume_aligned(a, ## __VA_ARGS__)))
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index dbfbecf703f8..66e1eb8822d9 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -68,13 +68,6 @@
>   */
>  #define uninitialized_var(x) x = x
>
> -#ifdef __CHECKER__
> -#define __must_be_array(a)     0
> -#else
> -/* &a[0] degrades to a pointer: a different type from an array */
> -#define __must_be_array(a)     BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
> -#endif
> -
>  #ifdef RETPOLINE
>  #define __noretpoline __attribute__((indirect_branch("keep")))
>  #endif
> diff --git a/include/linux/compiler-intel.h b/include/linux/compiler-intel.h
> index 4c7f9befa9f6..18bd4362deaa 100644
> --- a/include/linux/compiler-intel.h
> +++ b/include/linux/compiler-intel.h
> @@ -29,9 +29,6 @@
>   */
>  #define OPTIMIZER_HIDE_VAR(var) barrier()
>
> -/* Intel ECC compiler doesn't support __builtin_types_compatible_p() */
> -#define __must_be_array(a) 0
> -
>  #endif
>
>  /* icc has this, but it's called _bswap16 */
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index e0e55eb3f242..e4a702f99e50 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -357,4 +357,11 @@ static inline void *offset_to_ptr(const int *off)
>         compiletime_assert(__native_word(t),                            \
>                 "Need native word sized stores/loads for atomicity.")
>
> +#ifdef __CHECKER__
> +#define __must_be_array(a)     0
> +#else
> +/* &a[0] degrades to a pointer: a different type from an array */
> +#define __must_be_array(a)     BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
> +#endif
> +
>  #endif /* __LINUX_COMPILER_H */
> --
> 2.17.1
>

Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 4/7] Compiler Attributes: homogenize __must_be_array

[Luc Van Oostenryck]

On Fri, Aug 31, 2018 at 07:05:11PM +0200, Miguel Ojeda wrote:
> Different definitions of __must_be_array:
> 
>   * gcc: disabled for __CHECKER__
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index e0e55eb3f242..e4a702f99e50 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -357,4 +357,11 @@ static inline void *offset_to_ptr(const int *off)
>  	compiletime_assert(__native_word(t),				\
>  		"Need native word sized stores/loads for atomicity.")
>  
> +#ifdef __CHECKER__
> +#define __must_be_array(a)	0
> +#else
> +/* &a[0] degrades to a pointer: a different type from an array */
> +#define __must_be_array(a)	BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
> +#endif
> +
>  #endif /* __LINUX_COMPILER_H */

You can also remove the #ifdef __CHECKER__ because:
1) even ancient version of sparse don't have a problem
2) BUILD_BUG_ON_ZERO() is currently disabled for __CHECKER__

-- Luc Van Oostenryck

Re: [PATCH 4/7] Compiler Attributes: homogenize __must_be_array

[Miguel Ojeda]

Hi Luc,

On Sat, Sep 1, 2018 at 11:17 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
> On Fri, Aug 31, 2018 at 07:05:11PM +0200, Miguel Ojeda wrote:
>> Different definitions of __must_be_array:
>>
>>   * gcc: disabled for __CHECKER__
>> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
>> index e0e55eb3f242..e4a702f99e50 100644
>> --- a/include/linux/compiler.h
>> +++ b/include/linux/compiler.h
>> @@ -357,4 +357,11 @@ static inline void *offset_to_ptr(const int *off)
>>       compiletime_assert(__native_word(t),                            \
>>               "Need native word sized stores/loads for atomicity.")
>>
>> +#ifdef __CHECKER__
>> +#define __must_be_array(a)   0
>> +#else
>> +/* &a[0] degrades to a pointer: a different type from an array */
>> +#define __must_be_array(a)   BUILD_BUG_ON_ZERO(__same_type((a), &(a)[0]))
>> +#endif
>> +
>>  #endif /* __LINUX_COMPILER_H */
>
> You can also remove the #ifdef __CHECKER__ because:
> 1) even ancient version of sparse don't have a problem
> 2) BUILD_BUG_ON_ZERO() is currently disabled for __CHECKER__
>

Nice catch! Will do.

Cheers,
Miguel

[PATCH 5/7] Compiler Attributes: naked was fixed in gcc 4.6

[Miguel Ojeda]

Commit 9c695203a7dd ("compiler-gcc.h: gcc-4.5 needs noclone
and noinline on __naked functions") added noinline and noclone
as a workaround for a gcc 4.5 bug, which was resolved in 4.6.0.

Since now the minimum gcc supported version is 4.6,
we can clean it up.

See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=44290
and https://godbolt.org/z/h6NMIL

Cc: Eli Friedman <efriedma@codeaurora.org>
Cc: Christopher Li <sparse@chrisli.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Joe Perches <joe@perches.com>
Cc: Dominique Martinet <asmadeus@codewreck.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
---
 include/linux/compiler-gcc.h | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 66e1eb8822d9..fdf2fbe6d544 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -77,14 +77,8 @@
  * to trace naked functions because then mcount is called without
  * stack and frame pointer being set up and there is no chance to
  * restore the lr register to the value before mcount was called.
- *
- * The asm() bodies of naked functions often depend on standard calling
- * conventions, therefore they must be noinline and noclone.
- *
- * GCC 4.[56] currently fail to enforce this, so we must do so ourselves.
- * See GCC PR44290.
  */
-#define __naked		__attribute__((naked)) noinline __noclone notrace
+#define __naked		__attribute__((naked)) notrace
 
 #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
 
-- 
2.17.1

Re: [PATCH 5/7] Compiler Attributes: naked was fixed in gcc 4.6

[Arnd Bergmann]

On Fri, Aug 31, 2018 at 7:05 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> Commit 9c695203a7dd ("compiler-gcc.h: gcc-4.5 needs noclone
> and noinline on __naked functions") added noinline and noclone
> as a workaround for a gcc 4.5 bug, which was resolved in 4.6.0.
>
> Since now the minimum gcc supported version is 4.6,
> we can clean it up.
>
> See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=44290
> and https://godbolt.org/z/h6NMIL
>

> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index 66e1eb8822d9..fdf2fbe6d544 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -77,14 +77,8 @@
>   * to trace naked functions because then mcount is called without
>   * stack and frame pointer being set up and there is no chance to
>   * restore the lr register to the value before mcount was called.
> - *
> - * The asm() bodies of naked functions often depend on standard calling
> - * conventions, therefore they must be noinline and noclone.
> - *
> - * GCC 4.[56] currently fail to enforce this, so we must do so ourselves.
> - * See GCC PR44290.
>   */
> -#define __naked                __attribute__((naked)) noinline __noclone notrace
> +#define __naked                __attribute__((naked)) notrace
>

Good catch. Can this be moved into linux/compiler.h now so we
don't need separate definitions for clang and gcc?

        Arnd

Re: [PATCH 5/7] Compiler Attributes: naked was fixed in gcc 4.6

[Miguel Ojeda]

Hi Arnd,

On Fri, Aug 31, 2018 at 9:48 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Fri, Aug 31, 2018 at 7:05 PM Miguel Ojeda
> <miguel.ojeda.sandonis@gmail.com> wrote:
>>
>> Commit 9c695203a7dd ("compiler-gcc.h: gcc-4.5 needs noclone
>> and noinline on __naked functions") added noinline and noclone
>> as a workaround for a gcc 4.5 bug, which was resolved in 4.6.0.
>>
>> Since now the minimum gcc supported version is 4.6,
>> we can clean it up.
>>
>> See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=44290
>> and https://godbolt.org/z/h6NMIL
>>
>
>> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
>> index 66e1eb8822d9..fdf2fbe6d544 100644
>> --- a/include/linux/compiler-gcc.h
>> +++ b/include/linux/compiler-gcc.h
>> @@ -77,14 +77,8 @@
>>   * to trace naked functions because then mcount is called without
>>   * stack and frame pointer being set up and there is no chance to
>>   * restore the lr register to the value before mcount was called.
>> - *
>> - * The asm() bodies of naked functions often depend on standard calling
>> - * conventions, therefore they must be noinline and noclone.
>> - *
>> - * GCC 4.[56] currently fail to enforce this, so we must do so ourselves.
>> - * See GCC PR44290.
>>   */
>> -#define __naked                __attribute__((naked)) noinline __noclone notrace
>> +#define __naked                __attribute__((naked)) notrace
>>
>
> Good catch. Can this be moved into linux/compiler.h now so we
> don't need separate definitions for clang and gcc?

Yes, it can! :-) Actually I was writing that to Nick in the other
thread about clang, but you beat me to it. Good catch too!

Cheers,
Miguel

[PATCH 6/7] Compiler Attributes: remove unneeded sparse (__CHECKER__) tests

[Miguel Ojeda]

Sparse knows about a few more attributes now, so we can remove
the __CHECKER__ conditions from them (which, in turn, allow us
to move some of them later on to compiler_attributes.h).

  * assume_aligned: since sparse's commit ffc860b ("sparse:
    ignore __assume_aligned__ attribute"), included in 0.5.1

  * error: since sparse's commit 0a04210 ("sparse: Add 'error'
    to ignored attributes"), included in 0.5.0

  * hotpatch: since sparse's commit 6043210 ("sparse/parse.c:
    ignore hotpatch attribute"), included in 0.5.1

  * warning: since sparse's commit 977365d ("Avoid "attribute
    'warning': unknown attribute" warning"), included in 0.4.2

Cc: Eli Friedman <efriedma@codeaurora.org>
Cc: Christopher Li <sparse@chrisli.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Joe Perches <joe@perches.com>
Cc: Dominique Martinet <asmadeus@codewreck.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
---
 include/linux/compiler-gcc.h   | 6 ++----
 include/linux/compiler_types.h | 2 +-
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index fdf2fbe6d544..32e6ce06163f 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -84,14 +84,12 @@
 
 #define __compiletime_object_size(obj) __builtin_object_size(obj, 0)
 
-#ifndef __CHECKER__
 #define __compiletime_warning(message) __attribute__((warning(message)))
 #define __compiletime_error(message) __attribute__((error(message)))
 
-#ifdef LATENT_ENTROPY_PLUGIN
+#if defined(LATENT_ENTROPY_PLUGIN) && !defined(__CHECKER__)
 #define __latent_entropy __attribute__((latent_entropy))
 #endif
-#endif /* __CHECKER__ */
 
 /*
  * calling noreturn functions, __builtin_unreachable() and __builtin_trap()
@@ -139,7 +137,7 @@
 
 /* gcc version specific checks */
 
-#if GCC_VERSION >= 40900 && !defined(__CHECKER__)
+#if GCC_VERSION >= 40900
 /*
  * __assume_aligned(n, k): Tell the optimizer that the returned
  * pointer can be assumed to be k modulo n. The second argument is
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 3662b19599fc..5dddc7e0c607 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -216,7 +216,7 @@ struct ftrace_likely_data {
 #define __must_check
 #endif
 
-#if defined(CC_USING_HOTPATCH) && !defined(__CHECKER__)
+#if defined(CC_USING_HOTPATCH)
 #define notrace			__attribute__((hotpatch(0, 0)))
 #else
 #define notrace			__attribute__((no_instrument_function))
-- 
2.17.1

Re: [PATCH 6/7] Compiler Attributes: remove unneeded sparse (__CHECKER__) tests

[Nick Desaulniers]

On Fri, Aug 31, 2018 at 10:05 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> Sparse knows about a few more attributes now, so we can remove
> the __CHECKER__ conditions from them (which, in turn, allow us
> to move some of them later on to compiler_attributes.h).
>
>   * assume_aligned: since sparse's commit ffc860b ("sparse:
>     ignore __assume_aligned__ attribute"), included in 0.5.1
>
>   * error: since sparse's commit 0a04210 ("sparse: Add 'error'
>     to ignored attributes"), included in 0.5.0
>
>   * hotpatch: since sparse's commit 6043210 ("sparse/parse.c:
>     ignore hotpatch attribute"), included in 0.5.1
>
>   * warning: since sparse's commit 977365d ("Avoid "attribute
>     'warning': unknown attribute" warning"), included in 0.4.2
>
> Cc: Eli Friedman <efriedma@codeaurora.org>
> Cc: Christopher Li <sparse@chrisli.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Joe Perches <joe@perches.com>
> Cc: Dominique Martinet <asmadeus@codewreck.org>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
> ---
>  include/linux/compiler-gcc.h   | 6 ++----
>  include/linux/compiler_types.h | 2 +-
>  2 files changed, 3 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index fdf2fbe6d544..32e6ce06163f 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -84,14 +84,12 @@
>
>  #define __compiletime_object_size(obj) __builtin_object_size(obj, 0)
>
> -#ifndef __CHECKER__
>  #define __compiletime_warning(message) __attribute__((warning(message)))
>  #define __compiletime_error(message) __attribute__((error(message)))
>
> -#ifdef LATENT_ENTROPY_PLUGIN
> +#if defined(LATENT_ENTROPY_PLUGIN) && !defined(__CHECKER__)
>  #define __latent_entropy __attribute__((latent_entropy))
>  #endif
> -#endif /* __CHECKER__ */
>
>  /*
>   * calling noreturn functions, __builtin_unreachable() and __builtin_trap()
> @@ -139,7 +137,7 @@
>
>  /* gcc version specific checks */
>
> -#if GCC_VERSION >= 40900 && !defined(__CHECKER__)
> +#if GCC_VERSION >= 40900
>  /*
>   * __assume_aligned(n, k): Tell the optimizer that the returned
>   * pointer can be assumed to be k modulo n. The second argument is
> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> index 3662b19599fc..5dddc7e0c607 100644
> --- a/include/linux/compiler_types.h
> +++ b/include/linux/compiler_types.h
> @@ -216,7 +216,7 @@ struct ftrace_likely_data {
>  #define __must_check
>  #endif
>
> -#if defined(CC_USING_HOTPATCH) && !defined(__CHECKER__)
> +#if defined(CC_USING_HOTPATCH)
>  #define notrace                        __attribute__((hotpatch(0, 0)))
>  #else
>  #define notrace                        __attribute__((no_instrument_function))
> --
> 2.17.1
>

Everything looks correct here.  It would be good for the sparse
maintainer to triple check the commit sha's (as those are for sparse's
code base, not the kernel's) and have their blessing.  If Chris is
happy with it, then you can add my signoff:

Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

Also, do you need to put the cc list in the commit message? Some
people do (hopefully in an automated fashion, because I'd imagine
manually to be difficult) but don't this it's required.  Doesn't
matter, just curious.

-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 6/7] Compiler Attributes: remove unneeded sparse (__CHECKER__) tests

[Miguel Ojeda]

Hi Nick,

On Fri, Aug 31, 2018 at 11:38 PM, Nick Desaulniers
<ndesaulniers@google.com> wrote:
> On Fri, Aug 31, 2018 at 10:05 AM Miguel Ojeda
> <miguel.ojeda.sandonis@gmail.com> wrote:
>>
>> Sparse knows about a few more attributes now, so we can remove
>> the __CHECKER__ conditions from them (which, in turn, allow us
>> to move some of them later on to compiler_attributes.h).
>>
>>   * assume_aligned: since sparse's commit ffc860b ("sparse:
>>     ignore __assume_aligned__ attribute"), included in 0.5.1
>>
>>   * error: since sparse's commit 0a04210 ("sparse: Add 'error'
>>     to ignored attributes"), included in 0.5.0
>>
>>   * hotpatch: since sparse's commit 6043210 ("sparse/parse.c:
>>     ignore hotpatch attribute"), included in 0.5.1
>>
>>   * warning: since sparse's commit 977365d ("Avoid "attribute
>>     'warning': unknown attribute" warning"), included in 0.4.2
>>
>> Cc: Eli Friedman <efriedma@codeaurora.org>
>> Cc: Christopher Li <sparse@chrisli.org>
>> Cc: Kees Cook <keescook@chromium.org>
>> Cc: Ingo Molnar <mingo@kernel.org>
>> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
>> Cc: Joe Perches <joe@perches.com>
>> Cc: Dominique Martinet <asmadeus@codewreck.org>
>> Cc: Nick Desaulniers <ndesaulniers@google.com>
>> Cc: Linus Torvalds <torvalds@linux-foundation.org>
>> Signed-off-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
>> ---
>>  include/linux/compiler-gcc.h   | 6 ++----
>>  include/linux/compiler_types.h | 2 +-
>>  2 files changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
>> index fdf2fbe6d544..32e6ce06163f 100644
>> --- a/include/linux/compiler-gcc.h
>> +++ b/include/linux/compiler-gcc.h
>> @@ -84,14 +84,12 @@
>>
>>  #define __compiletime_object_size(obj) __builtin_object_size(obj, 0)
>>
>> -#ifndef __CHECKER__
>>  #define __compiletime_warning(message) __attribute__((warning(message)))
>>  #define __compiletime_error(message) __attribute__((error(message)))
>>
>> -#ifdef LATENT_ENTROPY_PLUGIN
>> +#if defined(LATENT_ENTROPY_PLUGIN) && !defined(__CHECKER__)
>>  #define __latent_entropy __attribute__((latent_entropy))
>>  #endif
>> -#endif /* __CHECKER__ */
>>
>>  /*
>>   * calling noreturn functions, __builtin_unreachable() and __builtin_trap()
>> @@ -139,7 +137,7 @@
>>
>>  /* gcc version specific checks */
>>
>> -#if GCC_VERSION >= 40900 && !defined(__CHECKER__)
>> +#if GCC_VERSION >= 40900
>>  /*
>>   * __assume_aligned(n, k): Tell the optimizer that the returned
>>   * pointer can be assumed to be k modulo n. The second argument is
>> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
>> index 3662b19599fc..5dddc7e0c607 100644
>> --- a/include/linux/compiler_types.h
>> +++ b/include/linux/compiler_types.h
>> @@ -216,7 +216,7 @@ struct ftrace_likely_data {
>>  #define __must_check
>>  #endif
>>
>> -#if defined(CC_USING_HOTPATCH) && !defined(__CHECKER__)
>> +#if defined(CC_USING_HOTPATCH)
>>  #define notrace                        __attribute__((hotpatch(0, 0)))
>>  #else
>>  #define notrace                        __attribute__((no_instrument_function))
>> --
>> 2.17.1
>>
>
> Everything looks correct here.  It would be good for the sparse
> maintainer to triple check the commit sha's (as those are for sparse's
> code base, not the kernel's) and have their blessing.  If Chris is

Actually, nowadays it is very easy to check in sparse's
gcc-attr-list.h file, but since the file was not always there, I tried
to find out when the support for each attribute was added.

But indeed, Chris, please let us know. (Should have CC'd you).

> happy with it, then you can add my signoff:
>
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>
> Also, do you need to put the cc list in the commit message? Some
> people do (hopefully in an automated fashion, because I'd imagine
> manually to be difficult) but don't this it's required.  Doesn't
> matter, just curious.

Hm... I thought it was supposed to be there for all patches,
considering different patches may have to be targeted to different
people. Also, one has to manage anyway the Acked-by and Reviewed-by
per-patch, so you have to manually go through. On top of that, it is
always handy to have in case you discard a patch into a separate
series or in case you send them through a tool that picks the Cc
individually (instead of something more advanced like git-send-email).
So... I guess? Not sure if it is strictly required, though.

Cheers,
Miguel

Re: [PATCH 6/7] Compiler Attributes: remove unneeded sparse (__CHECKER__) tests

[Luc Van Oostenryck]

On Fri, Aug 31, 2018 at 11:55:51PM +0200, Miguel Ojeda wrote:
> Hi Nick,
> 
> On Fri, Aug 31, 2018 at 11:38 PM, Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> > On Fri, Aug 31, 2018 at 10:05 AM Miguel Ojeda
> > <miguel.ojeda.sandonis@gmail.com> wrote:
> >>
> >> Sparse knows about a few more attributes now, so we can remove
> >> the __CHECKER__ conditions from them (which, in turn, allow us
> >> to move some of them later on to compiler_attributes.h).
> >>
> >>   * assume_aligned: since sparse's commit ffc860b ("sparse:
> >>     ignore __assume_aligned__ attribute"), included in 0.5.1
> >>
> >>   * error: since sparse's commit 0a04210 ("sparse: Add 'error'
> >>     to ignored attributes"), included in 0.5.0
> >>
> >>   * hotpatch: since sparse's commit 6043210 ("sparse/parse.c:
> >>     ignore hotpatch attribute"), included in 0.5.1
> >>
> >>   * warning: since sparse's commit 977365d ("Avoid "attribute
> >>     'warning': unknown attribute" warning"), included in 0.4.2
> >>
> >> Cc: Eli Friedman <efriedma@codeaurora.org>
> >> Cc: Christopher Li <sparse@chrisli.org>
> >> Cc: Kees Cook <keescook@chromium.org>
> >> Cc: Ingo Molnar <mingo@kernel.org>
> >> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> >> Cc: Arnd Bergmann <arnd@arndb.de>
> >> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> >> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> >> Cc: Joe Perches <joe@perches.com>
> >> Cc: Dominique Martinet <asmadeus@codewreck.org>
> >> Cc: Nick Desaulniers <ndesaulniers@google.com>
> >> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> >> Signed-off-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
> >> ---
> >>  include/linux/compiler-gcc.h   | 6 ++----
> >>  include/linux/compiler_types.h | 2 +-
> >>  2 files changed, 3 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> >> index fdf2fbe6d544..32e6ce06163f 100644
> >> --- a/include/linux/compiler-gcc.h
> >> +++ b/include/linux/compiler-gcc.h
> >> @@ -84,14 +84,12 @@
> >>
> >>  #define __compiletime_object_size(obj) __builtin_object_size(obj, 0)
> >>
> >> -#ifndef __CHECKER__
> >>  #define __compiletime_warning(message) __attribute__((warning(message)))
> >>  #define __compiletime_error(message) __attribute__((error(message)))
> >>
> >> -#ifdef LATENT_ENTROPY_PLUGIN
> >> +#if defined(LATENT_ENTROPY_PLUGIN) && !defined(__CHECKER__)
> >>  #define __latent_entropy __attribute__((latent_entropy))
> >>  #endif
> >> -#endif /* __CHECKER__ */
> >>
> >>  /*
> >>   * calling noreturn functions, __builtin_unreachable() and __builtin_trap()
> >> @@ -139,7 +137,7 @@
> >>
> >>  /* gcc version specific checks */
> >>
> >> -#if GCC_VERSION >= 40900 && !defined(__CHECKER__)
> >> +#if GCC_VERSION >= 40900
> >>  /*
> >>   * __assume_aligned(n, k): Tell the optimizer that the returned
> >>   * pointer can be assumed to be k modulo n. The second argument is
> >> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> >> index 3662b19599fc..5dddc7e0c607 100644
> >> --- a/include/linux/compiler_types.h
> >> +++ b/include/linux/compiler_types.h
> >> @@ -216,7 +216,7 @@ struct ftrace_likely_data {
> >>  #define __must_check
> >>  #endif
> >>
> >> -#if defined(CC_USING_HOTPATCH) && !defined(__CHECKER__)
> >> +#if defined(CC_USING_HOTPATCH)
> >>  #define notrace                        __attribute__((hotpatch(0, 0)))
> >>  #else
> >>  #define notrace                        __attribute__((no_instrument_function))
> >> --
> >> 2.17.1
> >>
> >
> > Everything looks correct here.  It would be good for the sparse
> > maintainer to triple check the commit sha's (as those are for sparse's
> > code base, not the kernel's) and have their blessing.  If Chris is
> 
> Actually, nowadays it is very easy to check in sparse's
> gcc-attr-list.h file, but since the file was not always there, I tried
> to find out when the support for each attribute was added.

Acked-by: Luc Van Oostenryck <luc.vanoosternyck@gmail.com>

[PATCH 7/7] Compiler Attributes: use feature checks instead of version checks

[Miguel Ojeda]

Instead of using version checks per-compiler to define (or not)
each attribute, use __has_attribute to test for them, following
the cleanup started with commit 815f0ddb346c
("include/linux/compiler*.h: make compiler-*.h mutually exclusive").

All the attributes that are fairly common/standard (i.e. those that
do not require extra logic to define them) have been moved
to a new file include/linux/compiler_attributes.h.

In an effort to make the file as regular as possible, comments
stating the purpose of attributes have been removed. Instead,
links to the compiler docs have been added (i.e. to gcc and,
if available, to clang as well). In addition, they have been sorted.

Finally, if an attribute is optional (i.e. if it is guarded
by __has_attribute), the reason has been stated for future reference.

Cc: Eli Friedman <efriedma@codeaurora.org>
Cc: Christopher Li <sparse@chrisli.org>
Cc: Kees Cook <keescook@chromium.org>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
Cc: Joe Perches <joe@perches.com>
Cc: Dominique Martinet <asmadeus@codewreck.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
---
 include/linux/compiler-clang.h      |   4 -
 include/linux/compiler-gcc.h        |  51 -------
 include/linux/compiler-intel.h      |   6 -
 include/linux/compiler_attributes.h | 226 ++++++++++++++++++++++++++++
 include/linux/compiler_types.h      |  72 +--------
 5 files changed, 232 insertions(+), 127 deletions(-)
 create mode 100644 include/linux/compiler_attributes.h

diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
index 0aee694d3ab8..3e7dafb3ea80 100644
--- a/include/linux/compiler-clang.h
+++ b/include/linux/compiler-clang.h
@@ -21,8 +21,6 @@
 #define __SANITIZE_ADDRESS__
 #endif
 
-#define __no_sanitize_address __attribute__((no_sanitize("address")))
-
 /*
  * Not all versions of clang implement the the type-generic versions
  * of the builtin overflow checkers. Fortunately, clang implements
@@ -41,5 +39,3 @@
  * compilers, like ICC.
  */
 #define barrier() __asm__ __volatile__("" : : : "memory")
-#define __assume_aligned(a, ...)	\
-	__attribute__((assume_aligned(a, ## __VA_ARGS__)))
diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
index 32e6ce06163f..627fa8941436 100644
--- a/include/linux/compiler-gcc.h
+++ b/include/linux/compiler-gcc.h
@@ -116,9 +116,6 @@
 		__builtin_unreachable();	\
 	} while (0)
 
-/* Mark a function definition as prohibited from being cloned. */
-#define __noclone	__attribute__((noclone, optimize("no-tracer")))
-
 #if defined(RANDSTRUCT_PLUGIN) && !defined(__CHECKER__)
 #define __randomize_layout __attribute__((randomize_layout))
 #define __no_randomize_layout __attribute__((no_randomize_layout))
@@ -127,32 +124,6 @@
 #define randomized_struct_fields_end	} __randomize_layout;
 #endif
 
-/*
- * When used with Link Time Optimization, gcc can optimize away C functions or
- * variables which are referenced only from assembly code.  __visible tells the
- * optimizer that something else uses this function or variable, thus preventing
- * this.
- */
-#define __visible	__attribute__((externally_visible))
-
-/* gcc version specific checks */
-
-#if GCC_VERSION >= 40900
-/*
- * __assume_aligned(n, k): Tell the optimizer that the returned
- * pointer can be assumed to be k modulo n. The second argument is
- * optional (default 0), so we use a variadic macro to make the
- * shorthand.
- *
- * Beware: Do not apply this to functions which may return
- * ERR_PTRs. Also, it is probably unwise to apply it to functions
- * returning extra information in the low bits (but in that case the
- * compiler should see some alignment anyway, when the return value is
- * massaged by 'flags = ptr & 3; ptr &= ~3;').
- */
-#define __assume_aligned(a, ...) __attribute__((assume_aligned(a, ## __VA_ARGS__)))
-#endif
-
 /*
  * GCC 'asm goto' miscompiles certain code sequences:
  *
@@ -184,32 +155,10 @@
 #define KASAN_ABI_VERSION 3
 #endif
 
-#if GCC_VERSION >= 40902
-/*
- * Tell the compiler that address safety instrumentation (KASAN)
- * should not be applied to that function.
- * Conflicts with inlining: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
- */
-#define __no_sanitize_address __attribute__((no_sanitize_address))
-#endif
-
 #if GCC_VERSION >= 50100
-/*
- * Mark structures as requiring designated initializers.
- * https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html
- */
-#define __designated_init __attribute__((designated_init))
 #define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
 #endif
 
-#if !defined(__noclone)
-#define __noclone	/* not needed */
-#endif
-
-#if !defined(__no_sanitize_address)
-#define __no_sanitize_address
-#endif
-
 /*
  * Turn individual warnings and errors on and off locally, depending
  * on version.
diff --git a/include/linux/compiler-intel.h b/include/linux/compiler-intel.h
index 18bd4362deaa..517bd14e1222 100644
--- a/include/linux/compiler-intel.h
+++ b/include/linux/compiler-intel.h
@@ -34,9 +34,3 @@
 /* icc has this, but it's called _bswap16 */
 #define __HAVE_BUILTIN_BSWAP16__
 #define __builtin_bswap16 _bswap16
-
-/* The following are for compatibility with GCC, from compiler-gcc.h,
- * and may be redefined here because they should not be shared with other
- * compilers, like clang.
- */
-#define __visible	__attribute__((externally_visible))
diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
new file mode 100644
index 000000000000..a9dfafc8fd19
--- /dev/null
+++ b/include/linux/compiler_attributes.h
@@ -0,0 +1,226 @@
+#ifndef __LINUX_COMPILER_ATTRIBUTES_H
+#define __LINUX_COMPILER_ATTRIBUTES_H
+
+/*
+ * This file is meant to be sorted (by actual attribute name,
+ * not by #define identifier).
+ *
+ * Do not add here attributes which depend on others or require extra logic.
+ *
+ * If an attribute is optional, state the reason in the comment.
+ */
+
+/*
+ * To check for optional attributes, we use __has_attribute, which is supported
+ * on gcc >= 5, clang >= 2.9 and icc >= 17. In the meantime, to support
+ * 4.6 <= gcc < 5, we implement __has_attribute by hand.
+ */
+#ifndef __has_attribute
+#define __has_attribute(x) __GCC4_has_attribute_##x
+#define __GCC4_has_attribute_externally_visible 1
+#define __GCC4_has_attribute_noclone 1
+#define __GCC4_has_attribute_optimize 1
+#if __GNUC_MINOR__ >= 8
+#define __GCC4_has_attribute_no_sanitize_address 1
+#endif
+#if __GNUC_MINOR__ >= 9
+#define __GCC4_has_attribute_assume_aligned 1
+#endif
+#endif
+
+/*
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-alias-function-attribute
+ */
+#define __alias(symbol)         __attribute__((alias(#symbol)))
+
+/*
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-aligned-function-attribute
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html#index-aligned-type-attribute
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-aligned-variable-attribute
+ */
+#define __aligned(x)            __attribute__((aligned(x)))
+#define __aligned_largest       __attribute__((aligned))
+
+/*
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-always_005finline-function-attribute
+ * clang: mentioned but no proper documentation
+ */
+#define __always_inline         inline __attribute__((always_inline))
+
+/*
+ * The second argument is optional (default 0), so we use a variadic macro
+ * to make the shorthand.
+ *
+ * Beware: Do not apply this to functions which may return
+ * ERR_PTRs. Also, it is probably unwise to apply it to functions
+ * returning extra information in the low bits (but in that case the
+ * compiler should see some alignment anyway, when the return value is
+ * massaged by 'flags = ptr & 3; ptr &= ~3;').
+ *
+ * Optional: only supported since gcc >= 4.9
+ * Optional: not supported by icc
+ *
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-assume_005faligned-function-attribute
+ * clang: https://clang.llvm.org/docs/AttributeReference.html#assume-aligned
+ */
+#if __has_attribute(assume_aligned)
+#define __assume_aligned(a, ...) __attribute__((assume_aligned(a, ## __VA_ARGS__)))
+#else
+#define __assume_aligned(a, ...)
+#endif
+
+/*
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-cold-function-attribute
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Label-Attributes.html#index-cold-label-attribute
+ */
+#define __cold                  __attribute__((cold))
+
+/*
+ * Note the long name.
+ *
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-const-function-attribute
+ */
+#define __attribute_const__     __attribute__((const))
+
+/*
+ * Don't. Just don't. See commit 771c035372a0 ("deprecate the '__deprecated'
+ * attribute warnings entirely and for good") for more information.
+ *
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-deprecated-function-attribute
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html#index-deprecated-type-attribute
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-deprecated-variable-attribute
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Enumerator-Attributes.html#index-deprecated-enumerator-attribute
+ * clang: https://clang.llvm.org/docs/AttributeReference.html#deprecated
+ */
+#define __deprecated
+
+/*
+ * Optional: only supported since gcc >= 5.1
+ * Optional: not supported by clang
+ * Optional: not supported by icc
+ *
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html#index-designated_005finit-type-attribute
+ */
+#if __has_attribute(designated_init)
+#define __designated_init       __attribute__((designated_init))
+#else
+#define __designated_init
+#endif
+
+/*
+ * Optional: not supported by clang
+ *
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-externally_005fvisible-function-attribute
+ */
+#if __has_attribute(externally_visible)
+#define __visible               __attribute__((externally_visible))
+#else
+#define __visible
+#endif
+
+/*
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-format-function-attribute
+ * clang: https://clang.llvm.org/docs/AttributeReference.html#format
+ */
+#define __printf(a, b)          __attribute__((format(printf, a, b)))
+#define __scanf(a, b)           __attribute__((format(scanf, a, b)))
+
+/*
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-gnu_005finline-function-attribute
+ * clang: https://clang.llvm.org/docs/AttributeReference.html#gnu-inline
+ */
+#define __gnu_inline            __attribute__((gnu_inline))
+
+/*
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-malloc-function-attribute
+ */
+#define __malloc                __attribute__((malloc))
+
+/*
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html#index-mode-type-attribute
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-mode-variable-attribute
+ */
+#define __mode(x)               __attribute__((mode(x)))
+
+/*
+ * Optional: not supported by clang
+ * Note: icc does not recognize gcc's no-tracer
+ *
+ *  gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noclone-function-attribute
+ *  gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-optimize-function-attribute
+ */
+#if __has_attribute(noclone) && __has_attribute(optimize)
+#define __noclone               __attribute__((noclone, optimize("no-tracer")))
+#else
+#define __noclone
+#endif
+
+/*
+ * Note the missing underscores.
+ *
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noinline-function-attribute
+ * clang: mentioned but no proper documentation
+ */
+#define   noinline              __attribute__((noinline))
+
+/*
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noreturn-function-attribute
+ * clang: https://clang.llvm.org/docs/AttributeReference.html#noreturn
+ * clang: https://clang.llvm.org/docs/AttributeReference.html#id1
+ */
+#define __noreturn              __attribute__((noreturn))
+
+/*
+ * Optional: only supported since gcc >= 4.8
+ * Optional: not supported by icc
+ *
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-no_005fsanitize_005faddress-function-attribute
+ * clang: https://clang.llvm.org/docs/AttributeReference.html#no-sanitize-address-no-address-safety-analysis
+ */
+#if __has_attribute(no_sanitize_address)
+#define __no_sanitize_address   __attribute__((no_sanitize_address))
+#else
+#define __no_sanitize_address
+#endif
+
+/*
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html#index-packed-type-attribute
+ * clang: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-packed-variable-attribute
+ */
+#define __packed                __attribute__((packed))
+
+/*
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-pure-function-attribute
+ */
+#define __pure                  __attribute__((pure))
+
+/*
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-section-function-attribute
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-section-variable-attribute
+ * clang: https://clang.llvm.org/docs/AttributeReference.html#section-declspec-allocate
+ */
+#define __section(S)            __attribute__((section(#S)))
+
+/*
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-unused-function-attribute
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html#index-unused-type-attribute
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-unused-variable-attribute
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Label-Attributes.html#index-unused-label-attribute
+ * clang: https://clang.llvm.org/docs/AttributeReference.html#maybe-unused-unused
+ */
+#define __always_unused         __attribute__((unused))
+#define __maybe_unused          __attribute__((unused))
+
+/*
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-used-function-attribute
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-used-variable-attribute
+ */
+#define __used                  __attribute__((used))
+
+/*
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-weak-function-attribute
+ *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-weak-variable-attribute
+ */
+#define __weak                  __attribute__((weak))
+
+#endif /* __LINUX_COMPILER_ATTRIBUTES_H */
diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
index 5dddc7e0c607..f1e658433323 100644
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -54,6 +54,9 @@ extern void __chk_io_ptr(const volatile void __iomem *);
 
 #ifdef __KERNEL__
 
+/* Attributes */
+#include <linux/compiler_attributes.h>
+
 /* Compiler specific macros. */
 #ifdef __clang__
 #include <linux/compiler-clang.h>
@@ -78,12 +81,6 @@ extern void __chk_io_ptr(const volatile void __iomem *);
 #include <asm/compiler.h>
 #endif
 
-/*
- * Generic compiler-independent macros required for kernel
- * build go below this comment. Actual compiler/compiler version
- * specific implementations come from the above header files
- */
-
 struct ftrace_branch_data {
 	const char *func;
 	const char *file;
@@ -106,9 +103,6 @@ struct ftrace_likely_data {
 	unsigned long			constant;
 };
 
-/* Don't. Just don't. */
-#define __deprecated
-
 #endif /* __KERNEL__ */
 
 #endif /* __ASSEMBLY__ */
@@ -118,10 +112,6 @@ struct ftrace_likely_data {
  * compilers. We don't consider that to be an error, so set them to nothing.
  * For example, some of them are for compiler specific plugins.
  */
-#ifndef __designated_init
-# define __designated_init
-#endif
-
 #ifndef __latent_entropy
 # define __latent_entropy
 #endif
@@ -139,17 +129,6 @@ struct ftrace_likely_data {
 # define randomized_struct_fields_end
 #endif
 
-#ifndef __visible
-#define __visible
-#endif
-
-/*
- * Assume alignment of return value.
- */
-#ifndef __assume_aligned
-#define __assume_aligned(a, ...)
-#endif
-
 /* Are two types/vars the same type (ignoring qualifiers)? */
 #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
 
@@ -158,10 +137,6 @@ struct ftrace_likely_data {
 	(sizeof(t) == sizeof(char) || sizeof(t) == sizeof(short) || \
 	 sizeof(t) == sizeof(int) || sizeof(t) == sizeof(long))
 
-#ifndef __noclone
-#define __noclone
-#endif
-
 /* Helpers for emitting diagnostics in pragmas. */
 #ifndef __diag
 #define __diag(string)
@@ -181,35 +156,6 @@ struct ftrace_likely_data {
 #define __diag_error(compiler, version, option, comment) \
 	__diag_ ## compiler(version, error, option)
 
-/*
- * From the GCC manual:
- *
- * Many functions have no effects except the return value and their
- * return value depends only on the parameters and/or global
- * variables.  Such a function can be subject to common subexpression
- * elimination and loop optimization just as an arithmetic operator
- * would be.
- * [...]
- */
-#define __pure			__attribute__((pure))
-#define __const			__attribute__((const))
-#define __aligned(x)		__attribute__((aligned(x)))
-#define __aligned_largest	__attribute__((aligned))
-#define __printf(a, b)		__attribute__((format(printf, a, b)))
-#define __scanf(a, b)		__attribute__((format(scanf, a, b)))
-#define __maybe_unused		__attribute__((unused))
-#define __always_unused		__attribute__((unused))
-#define __mode(x)		__attribute__((mode(x)))
-#define __malloc		__attribute__((malloc))
-#define __used			__attribute__((used))
-#define __noreturn		__attribute__((noreturn))
-#define __packed		__attribute__((packed))
-#define __weak			__attribute__((weak))
-#define __alias(symbol)		__attribute__((alias(#symbol)))
-#define __cold			__attribute__((cold))
-#define __section(S)		__attribute__((section(#S)))
-
-
 #ifdef CONFIG_ENABLE_MUST_CHECK
 #define __must_check		__attribute__((warn_unused_result))
 #else
@@ -224,8 +170,6 @@ struct ftrace_likely_data {
 
 #define __compiler_offsetof(a, b)	__builtin_offsetof(a, b)
 
-#define __gnu_inline	__attribute__((gnu_inline))
-
 /*
  * Force always-inline if the user requests it so via the .config.
  * GCC does not warn about unused static inline functions for
@@ -240,17 +184,13 @@ struct ftrace_likely_data {
  */
 #if !defined(CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING) || \
 	!defined(CONFIG_OPTIMIZE_INLINING)
-#define inline \
-	inline __attribute__((always_inline, unused)) notrace __gnu_inline
+#define inline __always_inline __gnu_inline __maybe_unused notrace
 #else
-#define inline inline	__attribute__((unused)) notrace __gnu_inline
+#define inline          inline __gnu_inline __maybe_unused notrace
 #endif
 
 #define __inline__ inline
-#define __inline inline
-#define noinline	__attribute__((noinline))
-
-#define __always_inline inline __attribute__((always_inline))
+#define __inline   inline
 
 /*
  * Rather then using noinline to prevent stack consumption, use
-- 
2.17.1

Re: [PATCH 7/7] Compiler Attributes: use feature checks instead of version checks

[Nick Desaulniers]

Overall, pretty happy with this patch.  Still some thoughts for a v3,
thanks for sending it!

On Fri, Aug 31, 2018 at 10:05 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> Instead of using version checks per-compiler to define (or not)
> each attribute, use __has_attribute to test for them, following
> the cleanup started with commit 815f0ddb346c
> ("include/linux/compiler*.h: make compiler-*.h mutually exclusive").
>
> All the attributes that are fairly common/standard (i.e. those that
> do not require extra logic to define them) have been moved
> to a new file include/linux/compiler_attributes.h.
>
> In an effort to make the file as regular as possible, comments
> stating the purpose of attributes have been removed. Instead,
> links to the compiler docs have been added (i.e. to gcc and,
> if available, to clang as well). In addition, they have been sorted.
>
> Finally, if an attribute is optional (i.e. if it is guarded
> by __has_attribute), the reason has been stated for future reference.
>
> Cc: Eli Friedman <efriedma@codeaurora.org>
> Cc: Christopher Li <sparse@chrisli.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Joe Perches <joe@perches.com>
> Cc: Dominique Martinet <asmadeus@codewreck.org>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
> ---
>  include/linux/compiler-clang.h      |   4 -
>  include/linux/compiler-gcc.h        |  51 -------
>  include/linux/compiler-intel.h      |   6 -
>  include/linux/compiler_attributes.h | 226 ++++++++++++++++++++++++++++
>  include/linux/compiler_types.h      |  72 +--------
>  5 files changed, 232 insertions(+), 127 deletions(-)
>  create mode 100644 include/linux/compiler_attributes.h
>
> diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
> index 0aee694d3ab8..3e7dafb3ea80 100644
> --- a/include/linux/compiler-clang.h
> +++ b/include/linux/compiler-clang.h
> @@ -21,8 +21,6 @@
>  #define __SANITIZE_ADDRESS__
>  #endif
>
> -#define __no_sanitize_address __attribute__((no_sanitize("address")))
> -
>  /*
>   * Not all versions of clang implement the the type-generic versions
>   * of the builtin overflow checkers. Fortunately, clang implements
> @@ -41,5 +39,3 @@
>   * compilers, like ICC.
>   */
>  #define barrier() __asm__ __volatile__("" : : : "memory")
> -#define __assume_aligned(a, ...)       \
> -       __attribute__((assume_aligned(a, ## __VA_ARGS__)))
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index 32e6ce06163f..627fa8941436 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -116,9 +116,6 @@
>                 __builtin_unreachable();        \
>         } while (0)
>
> -/* Mark a function definition as prohibited from being cloned. */
> -#define __noclone      __attribute__((noclone, optimize("no-tracer")))
> -
>  #if defined(RANDSTRUCT_PLUGIN) && !defined(__CHECKER__)
>  #define __randomize_layout __attribute__((randomize_layout))
>  #define __no_randomize_layout __attribute__((no_randomize_layout))
> @@ -127,32 +124,6 @@
>  #define randomized_struct_fields_end   } __randomize_layout;
>  #endif
>
> -/*
> - * When used with Link Time Optimization, gcc can optimize away C functions or
> - * variables which are referenced only from assembly code.  __visible tells the
> - * optimizer that something else uses this function or variable, thus preventing
> - * this.
> - */
> -#define __visible      __attribute__((externally_visible))
> -
> -/* gcc version specific checks */
> -
> -#if GCC_VERSION >= 40900
> -/*
> - * __assume_aligned(n, k): Tell the optimizer that the returned
> - * pointer can be assumed to be k modulo n. The second argument is
> - * optional (default 0), so we use a variadic macro to make the
> - * shorthand.
> - *
> - * Beware: Do not apply this to functions which may return
> - * ERR_PTRs. Also, it is probably unwise to apply it to functions
> - * returning extra information in the low bits (but in that case the
> - * compiler should see some alignment anyway, when the return value is
> - * massaged by 'flags = ptr & 3; ptr &= ~3;').
> - */
> -#define __assume_aligned(a, ...) __attribute__((assume_aligned(a, ## __VA_ARGS__)))
> -#endif
> -
>  /*
>   * GCC 'asm goto' miscompiles certain code sequences:
>   *
> @@ -184,32 +155,10 @@
>  #define KASAN_ABI_VERSION 3
>  #endif
>
> -#if GCC_VERSION >= 40902
> -/*
> - * Tell the compiler that address safety instrumentation (KASAN)
> - * should not be applied to that function.
> - * Conflicts with inlining: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67368
> - */
> -#define __no_sanitize_address __attribute__((no_sanitize_address))
> -#endif
> -
>  #if GCC_VERSION >= 50100
> -/*
> - * Mark structures as requiring designated initializers.
> - * https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html
> - */
> -#define __designated_init __attribute__((designated_init))
>  #define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
>  #endif
>
> -#if !defined(__noclone)
> -#define __noclone      /* not needed */
> -#endif
> -
> -#if !defined(__no_sanitize_address)
> -#define __no_sanitize_address
> -#endif
> -
>  /*
>   * Turn individual warnings and errors on and off locally, depending
>   * on version.
> diff --git a/include/linux/compiler-intel.h b/include/linux/compiler-intel.h
> index 18bd4362deaa..517bd14e1222 100644
> --- a/include/linux/compiler-intel.h
> +++ b/include/linux/compiler-intel.h
> @@ -34,9 +34,3 @@
>  /* icc has this, but it's called _bswap16 */
>  #define __HAVE_BUILTIN_BSWAP16__
>  #define __builtin_bswap16 _bswap16
> -
> -/* The following are for compatibility with GCC, from compiler-gcc.h,
> - * and may be redefined here because they should not be shared with other
> - * compilers, like clang.
> - */
> -#define __visible      __attribute__((externally_visible))
> diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
> new file mode 100644
> index 000000000000..a9dfafc8fd19
> --- /dev/null
> +++ b/include/linux/compiler_attributes.h
> @@ -0,0 +1,226 @@

New file needs an SPDX license identifier right?

> +#ifndef __LINUX_COMPILER_ATTRIBUTES_H
> +#define __LINUX_COMPILER_ATTRIBUTES_H
> +
> +/*
> + * This file is meant to be sorted (by actual attribute name,
> + * not by #define identifier).
> + *
> + * Do not add here attributes which depend on others or require extra logic.
> + *
> + * If an attribute is optional, state the reason in the comment.

A lot of newlines in this comment. Can be condensed?

> + */
> +
> +/*
> + * To check for optional attributes, we use __has_attribute, which is supported
> + * on gcc >= 5, clang >= 2.9 and icc >= 17. In the meantime, to support
> + * 4.6 <= gcc < 5, we implement __has_attribute by hand.
> + */
> +#ifndef __has_attribute
> +#define __has_attribute(x) __GCC4_has_attribute_##x
> +#define __GCC4_has_attribute_externally_visible 1
> +#define __GCC4_has_attribute_noclone 1
> +#define __GCC4_has_attribute_optimize 1
> +#if __GNUC_MINOR__ >= 8
> +#define __GCC4_has_attribute_no_sanitize_address 1
> +#endif
> +#if __GNUC_MINOR__ >= 9
> +#define __GCC4_has_attribute_assume_aligned 1
> +#endif
> +#endif

I'm quite happy with this. Kudos.

> +
> +/*
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-alias-function-attribute
> + */
> +#define __alias(symbol)         __attribute__((alias(#symbol)))
> +
> +/*
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-aligned-function-attribute
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html#index-aligned-type-attribute
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-aligned-variable-attribute
> + */
> +#define __aligned(x)            __attribute__((aligned(x)))
> +#define __aligned_largest       __attribute__((aligned))
> +
> +/*
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-always_005finline-function-attribute
> + * clang: mentioned but no proper documentation

What?! Ok, I'll add to my TODO list, but I think we can leave out that
clang doesn't have docs (yet!).

> + */
> +#define __always_inline         inline __attribute__((always_inline))
> +
> +/*
> + * The second argument is optional (default 0), so we use a variadic macro
> + * to make the shorthand.
> + *
> + * Beware: Do not apply this to functions which may return
> + * ERR_PTRs. Also, it is probably unwise to apply it to functions
> + * returning extra information in the low bits (but in that case the
> + * compiler should see some alignment anyway, when the return value is
> + * massaged by 'flags = ptr & 3; ptr &= ~3;').
> + *
> + * Optional: only supported since gcc >= 4.9
> + * Optional: not supported by icc
> + *
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-assume_005faligned-function-attribute
> + * clang: https://clang.llvm.org/docs/AttributeReference.html#assume-aligned
> + */
> +#if __has_attribute(assume_aligned)
> +#define __assume_aligned(a, ...) __attribute__((assume_aligned(a, ## __VA_ARGS__)))
> +#else
> +#define __assume_aligned(a, ...)
> +#endif
> +
> +/*
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-cold-function-attribute
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Label-Attributes.html#index-cold-label-attribute
> + */
> +#define __cold                  __attribute__((cold))
> +
> +/*
> + * Note the long name.
> + *
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-const-function-attribute
> + */
> +#define __attribute_const__     __attribute__((const))

So I manually wrote down the list of attributes removed from one file,
and added to another, to make sure they balance out and that none were
missed.  I'm quite confident that nothing was missed moving from one
file to another.  Except this.  You've renamed __const to
__attribute_const__.  But you renamed __attribute_const_ to __const in
patch 3/7 in this series.  Surely one of them is a mistake?

> +
> +/*
> + * Don't. Just don't. See commit 771c035372a0 ("deprecate the '__deprecated'
> + * attribute warnings entirely and for good") for more information.
> + *
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-deprecated-function-attribute
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html#index-deprecated-type-attribute
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-deprecated-variable-attribute
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Enumerator-Attributes.html#index-deprecated-enumerator-attribute
> + * clang: https://clang.llvm.org/docs/AttributeReference.html#deprecated
> + */
> +#define __deprecated
> +
> +/*
> + * Optional: only supported since gcc >= 5.1
> + * Optional: not supported by clang
> + * Optional: not supported by icc
> + *
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html#index-designated_005finit-type-attribute
> + */
> +#if __has_attribute(designated_init)
> +#define __designated_init       __attribute__((designated_init))
> +#else
> +#define __designated_init
> +#endif
> +
> +/*
> + * Optional: not supported by clang
> + *
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-externally_005fvisible-function-attribute
> + */
> +#if __has_attribute(externally_visible)
> +#define __visible               __attribute__((externally_visible))
> +#else
> +#define __visible
> +#endif
> +
> +/*
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-format-function-attribute
> + * clang: https://clang.llvm.org/docs/AttributeReference.html#format
> + */
> +#define __printf(a, b)          __attribute__((format(printf, a, b)))
> +#define __scanf(a, b)           __attribute__((format(scanf, a, b)))
> +
> +/*
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-gnu_005finline-function-attribute
> + * clang: https://clang.llvm.org/docs/AttributeReference.html#gnu-inline
> + */
> +#define __gnu_inline            __attribute__((gnu_inline))
> +
> +/*
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-malloc-function-attribute
> + */
> +#define __malloc                __attribute__((malloc))
> +
> +/*
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html#index-mode-type-attribute
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-mode-variable-attribute
> + */
> +#define __mode(x)               __attribute__((mode(x)))
> +
> +/*
> + * Optional: not supported by clang
> + * Note: icc does not recognize gcc's no-tracer

In that case, can you provide 3 definitions for __noclone? Something like:

if has noclone
  if has optimize
    noclone = noclone optimize
  else
    noclone = noclone
else
  noclone =


> + *
> + *  gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noclone-function-attribute
> + *  gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-optimize-function-attribute
> + */
> +#if __has_attribute(noclone) && __has_attribute(optimize)
> +#define __noclone               __attribute__((noclone, optimize("no-tracer")))
> +#else
> +#define __noclone
> +#endif
> +
> +/*
> + * Note the missing underscores.
> + *
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noinline-function-attribute
> + * clang: mentioned but no proper documentation
> + */
> +#define   noinline              __attribute__((noinline))
> +
> +/*
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-noreturn-function-attribute
> + * clang: https://clang.llvm.org/docs/AttributeReference.html#noreturn
> + * clang: https://clang.llvm.org/docs/AttributeReference.html#id1
> + */
> +#define __noreturn              __attribute__((noreturn))
> +
> +/*
> + * Optional: only supported since gcc >= 4.8
> + * Optional: not supported by icc
> + *
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-no_005fsanitize_005faddress-function-attribute
> + * clang: https://clang.llvm.org/docs/AttributeReference.html#no-sanitize-address-no-address-safety-analysis
> + */
> +#if __has_attribute(no_sanitize_address)
> +#define __no_sanitize_address   __attribute__((no_sanitize_address))
> +#else
> +#define __no_sanitize_address
> +#endif
> +
> +/*
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html#index-packed-type-attribute
> + * clang: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-packed-variable-attribute
> + */
> +#define __packed                __attribute__((packed))
> +
> +/*
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-pure-function-attribute
> + */
> +#define __pure                  __attribute__((pure))
> +
> +/*
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-section-function-attribute
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-section-variable-attribute
> + * clang: https://clang.llvm.org/docs/AttributeReference.html#section-declspec-allocate
> + */
> +#define __section(S)            __attribute__((section(#S)))
> +
> +/*
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-unused-function-attribute
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html#index-unused-type-attribute
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-unused-variable-attribute
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Label-Attributes.html#index-unused-label-attribute
> + * clang: https://clang.llvm.org/docs/AttributeReference.html#maybe-unused-unused
> + */
> +#define __always_unused         __attribute__((unused))
> +#define __maybe_unused          __attribute__((unused))
> +
> +/*
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-used-function-attribute
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-used-variable-attribute
> + */
> +#define __used                  __attribute__((used))
> +
> +/*
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-weak-function-attribute
> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-weak-variable-attribute
> + */
> +#define __weak                  __attribute__((weak))
> +
> +#endif /* __LINUX_COMPILER_ATTRIBUTES_H */
> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> index 5dddc7e0c607..f1e658433323 100644
> --- a/include/linux/compiler_types.h
> +++ b/include/linux/compiler_types.h
> @@ -54,6 +54,9 @@ extern void __chk_io_ptr(const volatile void __iomem *);
>
>  #ifdef __KERNEL__
>
> +/* Attributes */
> +#include <linux/compiler_attributes.h>
> +
>  /* Compiler specific macros. */
>  #ifdef __clang__
>  #include <linux/compiler-clang.h>
> @@ -78,12 +81,6 @@ extern void __chk_io_ptr(const volatile void __iomem *);
>  #include <asm/compiler.h>
>  #endif
>
> -/*
> - * Generic compiler-independent macros required for kernel
> - * build go below this comment. Actual compiler/compiler version
> - * specific implementations come from the above header files
> - */
> -
>  struct ftrace_branch_data {
>         const char *func;
>         const char *file;
> @@ -106,9 +103,6 @@ struct ftrace_likely_data {
>         unsigned long                   constant;
>  };
>
> -/* Don't. Just don't. */
> -#define __deprecated
> -
>  #endif /* __KERNEL__ */
>
>  #endif /* __ASSEMBLY__ */
> @@ -118,10 +112,6 @@ struct ftrace_likely_data {
>   * compilers. We don't consider that to be an error, so set them to nothing.
>   * For example, some of them are for compiler specific plugins.
>   */
> -#ifndef __designated_init
> -# define __designated_init
> -#endif
> -
>  #ifndef __latent_entropy
>  # define __latent_entropy
>  #endif
> @@ -139,17 +129,6 @@ struct ftrace_likely_data {
>  # define randomized_struct_fields_end
>  #endif
>
> -#ifndef __visible
> -#define __visible
> -#endif
> -
> -/*
> - * Assume alignment of return value.
> - */
> -#ifndef __assume_aligned
> -#define __assume_aligned(a, ...)
> -#endif
> -
>  /* Are two types/vars the same type (ignoring qualifiers)? */
>  #define __same_type(a, b) __builtin_types_compatible_p(typeof(a), typeof(b))
>
> @@ -158,10 +137,6 @@ struct ftrace_likely_data {
>         (sizeof(t) == sizeof(char) || sizeof(t) == sizeof(short) || \
>          sizeof(t) == sizeof(int) || sizeof(t) == sizeof(long))
>
> -#ifndef __noclone
> -#define __noclone
> -#endif
> -
>  /* Helpers for emitting diagnostics in pragmas. */
>  #ifndef __diag
>  #define __diag(string)
> @@ -181,35 +156,6 @@ struct ftrace_likely_data {
>  #define __diag_error(compiler, version, option, comment) \
>         __diag_ ## compiler(version, error, option)
>
> -/*
> - * From the GCC manual:
> - *
> - * Many functions have no effects except the return value and their
> - * return value depends only on the parameters and/or global
> - * variables.  Such a function can be subject to common subexpression
> - * elimination and loop optimization just as an arithmetic operator
> - * would be.
> - * [...]
> - */
> -#define __pure                 __attribute__((pure))
> -#define __const                        __attribute__((const))

See above comment about renaming __attribute_const__ to __const and back again.

> -#define __aligned(x)           __attribute__((aligned(x)))
> -#define __aligned_largest      __attribute__((aligned))
> -#define __printf(a, b)         __attribute__((format(printf, a, b)))
> -#define __scanf(a, b)          __attribute__((format(scanf, a, b)))
> -#define __maybe_unused         __attribute__((unused))
> -#define __always_unused                __attribute__((unused))
> -#define __mode(x)              __attribute__((mode(x)))
> -#define __malloc               __attribute__((malloc))
> -#define __used                 __attribute__((used))
> -#define __noreturn             __attribute__((noreturn))
> -#define __packed               __attribute__((packed))
> -#define __weak                 __attribute__((weak))
> -#define __alias(symbol)                __attribute__((alias(#symbol)))
> -#define __cold                 __attribute__((cold))
> -#define __section(S)           __attribute__((section(#S)))
> -
> -
>  #ifdef CONFIG_ENABLE_MUST_CHECK
>  #define __must_check           __attribute__((warn_unused_result))

Can this attribute be hoisted to your new header?  I guess there's a
few other __attributes__ still in this file even after this change.
Maybe not something that needs to be in this patch set, but what are
your thoughts on them?

>  #else
> @@ -224,8 +170,6 @@ struct ftrace_likely_data {
>
>  #define __compiler_offsetof(a, b)      __builtin_offsetof(a, b)
>
> -#define __gnu_inline   __attribute__((gnu_inline))
> -
>  /*
>   * Force always-inline if the user requests it so via the .config.
>   * GCC does not warn about unused static inline functions for
> @@ -240,17 +184,13 @@ struct ftrace_likely_data {
>   */
>  #if !defined(CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING) || \
>         !defined(CONFIG_OPTIMIZE_INLINING)
> -#define inline \
> -       inline __attribute__((always_inline, unused)) notrace __gnu_inline
> +#define inline __always_inline __gnu_inline __maybe_unused notrace
>  #else
> -#define inline inline  __attribute__((unused)) notrace __gnu_inline
> +#define inline          inline __gnu_inline __maybe_unused notrace

This seems to have different spacing after the first `inline` than the
line 2 lines above.

>  #endif
>
>  #define __inline__ inline
> -#define __inline inline
> -#define noinline       __attribute__((noinline))
> -
> -#define __always_inline inline __attribute__((always_inline))
> +#define __inline   inline
>
>  /*
>   * Rather then using noinline to prevent stack consumption, use
> --
> 2.17.1
>

Thank you again for this patch.
--
Thanks,
~Nick Desaulniers

Re: [PATCH 7/7] Compiler Attributes: use feature checks instead of version checks

[Miguel Ojeda]

Hi Nick,

On Sat, Sep 1, 2018 at 1:07 AM, Nick Desaulniers
<ndesaulniers@google.com> wrote:
> Overall, pretty happy with this patch.  Still some thoughts for a v3,
>> -#define __visible      __attribute__((externally_visible))
>> diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
>> new file mode 100644
>> index 000000000000..a9dfafc8fd19
>> --- /dev/null
>> +++ b/include/linux/compiler_attributes.h
>> @@ -0,0 +1,226 @@
>
> New file needs an SPDX license identifier right?

Yeah, but I wasn't sure of adding it, since the code I moved (even if
rearranged) from _types.h does not have it either. Any legal expert
here? Is _types.h it implicitly GPL 2? We should add the identifier to
both if so.

>
>> +#ifndef __LINUX_COMPILER_ATTRIBUTES_H
>> +#define __LINUX_COMPILER_ATTRIBUTES_H
>> +
>> +/*
>> + * This file is meant to be sorted (by actual attribute name,
>> + * not by #define identifier).
>> + *
>> + * Do not add here attributes which depend on others or require extra logic.
>> + *
>> + * If an attribute is optional, state the reason in the comment.
>
> A lot of newlines in this comment. Can be condensed?
>

I tried to write it as "bullet points", but as you guys prefer!

>> + */
>> +
>> +/*
>> + * To check for optional attributes, we use __has_attribute, which is supported
>> + * on gcc >= 5, clang >= 2.9 and icc >= 17. In the meantime, to support
>> + * 4.6 <= gcc < 5, we implement __has_attribute by hand.
>> + */
>> +#ifndef __has_attribute
>> +#define __has_attribute(x) __GCC4_has_attribute_##x
>> +#define __GCC4_has_attribute_externally_visible 1
>> +#define __GCC4_has_attribute_noclone 1
>> +#define __GCC4_has_attribute_optimize 1
>> +#if __GNUC_MINOR__ >= 8
>> +#define __GCC4_has_attribute_no_sanitize_address 1
>> +#endif
>> +#if __GNUC_MINOR__ >= 9
>> +#define __GCC4_has_attribute_assume_aligned 1
>> +#endif
>> +#endif
>
> I'm quite happy with this. Kudos.

Thanks! :-)

>
>> +
>> +/*
>> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-alias-function-attribute
>> + */
>> +#define __alias(symbol)         __attribute__((alias(#symbol)))
>> +
>> +/*
>> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-aligned-function-attribute
>> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html#index-aligned-type-attribute
>> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-aligned-variable-attribute
>> + */
>> +#define __aligned(x)            __attribute__((aligned(x)))
>> +#define __aligned_largest       __attribute__((aligned))
>> +
>> +/*
>> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-always_005finline-function-attribute
>> + * clang: mentioned but no proper documentation
>
> What?! Ok, I'll add to my TODO list, but I think we can leave out that
> clang doesn't have docs (yet!).
>

If you are going to do that -- please also check noinline (same thing
happens: you can find it mentioned in an example, but it is not
documented). Anyway, the clang docs state:

"""
Clang supports GCC’s gnu attribute namespace. All GCC attributes which
are accepted with the __attribute__((foo)) syntax are also accepted
as[[gnu::foo]]. This only extends to attributes which are specified by
GCC (see the list of GCC function attributes, GCC variable attributes,
and GCC type attributes). As with the GCC implementation, these
attributes must appertain to the declarator-id in a declaration, which
means they must go either at the start of the declaration or
immediately after the name being declared.
"""

in https://clang.llvm.org/docs/LanguageExtensions.html, so I guess it
is fair ;-) In any case, the GNU/C++/C2x/declspec/... compatibility
tables provided in https://clang.llvm.org/docs/AttributeReference.html
are great, and it would be *very* nice to complete that document to
have a reference for all developers accross all platforms, not just
kernel ones.

>> +
>> +/*
>> + * Note the long name.
>> + *
>> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-const-function-attribute
>> + */
>> +#define __attribute_const__     __attribute__((const))
>
> So I manually wrote down the list of attributes removed from one file,
> and added to another, to make sure they balance out and that none were
> missed.  I'm quite confident that nothing was missed moving from one
> file to another.  Except this.  You've renamed __const to
> __attribute_const__.  But you renamed __attribute_const_ to __const in
> patch 3/7 in this series.  Surely one of them is a mistake?

D'oh! It seems I confused myself when rebasing stuff to split the
patches. Good catch! 3/7 has the typo, which then triggers a second
rename. The end result of the series is the good state (i.e. no change
should happen to the identifiers).

>> +/*
>> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html#index-mode-type-attribute
>> + *   gcc: https://gcc.gnu.org/onlinedocs/gcc/Common-Variable-Attributes.html#index-mode-variable-attribute
>> + */
>> +#define __mode(x)               __attribute__((mode(x)))
>> +
>> +/*
>> + * Optional: not supported by clang
>> + * Note: icc does not recognize gcc's no-tracer
>
> In that case, can you provide 3 definitions for __noclone? Something like:
>
> if has noclone
>   if has optimize
>     noclone = noclone optimize
>   else
>     noclone = noclone
> else
>   noclone =
>

So what happened here is that icc recognizes the attribute itself (not
sure if it honors it or not), so there would be no compiler using the
"noclone && !optimize" definition, which is why I left it at that.

But I see the point in your idea in case there is a 4th compiler around...

>>  #ifdef CONFIG_ENABLE_MUST_CHECK
>>  #define __must_check           __attribute__((warn_unused_result))
>
> Can this attribute be hoisted to your new header?  I guess there's a
> few other __attributes__ still in this file even after this change.
> Maybe not something that needs to be in this patch set, but what are
> your thoughts on them?

This is something I gave quite some thought... I am still not sure
what is the right idea.

The decision I took (for the moment, at least) was to leave
compiler_attributes.h as "regular" and "simple" as possible, defining
whatever attributes that are common enough (i.e. those that can be
defined simply with __has_attribute at most) -- which is why I added
the "Do not add here..." comment in the top of the file, and leaving
the CONFIG- / PLUGIN-dependent definitions out of it.

Because of that logic, I tried to simplify as much as possible all
existing things (e.g. assume_aligned by removing __CHECKER__) so that
they could be moved to compiler_attributes.h. In a way,
compiler_attributes.h defines some extensions to the C language that
all code could use anywhere (actually whether we can move them out of
__KERNEL__ etc. is another question); and I tried as well to only move
those attributes that have a direct mapping to a "standard" attribute
(e.g. "noclone" was borderline :-).

The other alternative is moving everything to compiler_attributes.h
and keeping all logic there for all attributes and pseudo-attributes,
but I thought we could risk ending up with a complex mess there as in
_types.h; and end up with the pseudo-attributes which do not map to
real attributes and that are disabled in many configurations. Again,
the "C programming language extensions" idea could be a nice one (I
actually wrote a Documentation/ file to guide newcomers on this which
I didn't include in the series yet, see below [*]).

I would like to hear opinions on this!

>>  /*
>>   * Force always-inline if the user requests it so via the .config.
>>   * GCC does not warn about unused static inline functions for
>> @@ -240,17 +184,13 @@ struct ftrace_likely_data {
>>   */
>>  #if !defined(CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING) || \
>>         !defined(CONFIG_OPTIMIZE_INLINING)
>> -#define inline \
>> -       inline __attribute__((always_inline, unused)) notrace __gnu_inline
>> +#define inline __always_inline __gnu_inline __maybe_unused notrace
>>  #else
>> -#define inline inline  __attribute__((unused)) notrace __gnu_inline
>> +#define inline          inline __gnu_inline __maybe_unused notrace
>
> This seems to have different spacing after the first `inline` than the
> line 2 lines above.
>

Take a look at it in fixed-sized font (the idea is that
__always_inline contains inline, so it replaces the inline below in
that case to avoid warnings about duplicated specifiers). Does that
explain it?

>>  #endif
>>
>>  #define __inline__ inline
>> -#define __inline inline
>> -#define noinline       __attribute__((noinline))
>> -
>> -#define __always_inline inline __attribute__((always_inline))
>> +#define __inline   inline
>>
>>  /*
>>   * Rather then using noinline to prevent stack consumption, use
>> --
>> 2.17.1
>>
>
> Thank you again for this patch.

You are welcome! I am glad these series is getting a warm welcome.

Cheers,
Miguel

[*]

Documentation/process/programming-language.rst:

.. _programming_language:

Programming Language
====================

The kernel is written in the C programming language [c-language]_.
More precisely, the kernel is typically compiled with ``gcc`` [gcc]_
under ``-std=gnu89`` [gcc-c-dialect-options]_: the GNU dialect of ISO C90
(including some C99 features).

This dialect contains many extensions to the language [gnu-extensions]_,
and many of them are used within the kernel as a matter of course.

There is some support for compiling the kernel with ``clang`` [clang]_
and ``icc`` [icc]_, although at the time of writing it is not completed,
requiring third-party patches.

Attributes
----------

One of the common extensions used throughout the kernel are attributes
[gcc-attribute-syntax]_. Attributes allow to introduce
implementation-defined semantics to language entities (like variables,
functions or types) without having to make significant syntactic changes
to the language (e.g. adding a new keyword) [n2049]_.

In many cases, attributes are optional (i.e. a compiler not supporting them
should still produce proper code, even if it is slower or does not perform
as many compile-time checks/diagnostics).

The kernel defines pseudo-keywords (e.g. ``__pure``) instead of using
directly the GNU attribute syntax (e.g. ``__attribute__((pure))``).

Please refer to ``include/linux/compiler_attributes.h`` to see a reference
of all the common supported attributes.

.. [c-language] http://www.open-std.org/jtc1/sc22/wg14/www/standards
.. [gcc] https://gcc.gnu.org
.. [clang] https://clang.llvm.org
.. [icc] https://software.intel.com/en-us/c-compilers
.. [gcc-c-dialect-options]
https://gcc.gnu.org/onlinedocs/gcc/C-Dialect-Options.html
.. [gnu-extensions]: https://gcc.gnu.org/onlinedocs/gcc/C-Extensions.html
.. [gcc-attribute-syntax]
https://gcc.gnu.org/onlinedocs/gcc/Attribute-Syntax.html
.. [n2049] http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2049.pdf

Re: [PATCH 7/7] Compiler Attributes: use feature checks instead of version checks

[Greg KH]

On Sat, Sep 01, 2018 at 03:38:13PM +0200, Miguel Ojeda wrote:
> Hi Nick,
> 
> On Sat, Sep 1, 2018 at 1:07 AM, Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> > Overall, pretty happy with this patch.  Still some thoughts for a v3,
> >> -#define __visible      __attribute__((externally_visible))
> >> diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
> >> new file mode 100644
> >> index 000000000000..a9dfafc8fd19
> >> --- /dev/null
> >> +++ b/include/linux/compiler_attributes.h
> >> @@ -0,0 +1,226 @@
> >
> > New file needs an SPDX license identifier right?
> 
> Yeah, but I wasn't sure of adding it, since the code I moved (even if
> rearranged) from _types.h does not have it either. Any legal expert
> here? Is _types.h it implicitly GPL 2? We should add the identifier to
> both if so.

It looks like we missed that file in the big "properly add SPDX
identifiers to all files without a license" commit as it came in from a
different tree.

But yes, it is GPLv2 only implicitly, so you can add that.  I should
sweep the tree again to see if anything else has been added accidentally
with that same problem.

thanks,

greg k-h

Re: [PATCH 7/7] Compiler Attributes: use feature checks instead of version checks

[Miguel Ojeda]

Hi Greg,

On Sat, Sep 1, 2018 at 8:39 PM, Greg KH <gregkh@linuxfoundation.org> wrote:
> On Sat, Sep 01, 2018 at 03:38:13PM +0200, Miguel Ojeda wrote:
>> Hi Nick,
>>
>> On Sat, Sep 1, 2018 at 1:07 AM, Nick Desaulniers
>> <ndesaulniers@google.com> wrote:
>> > Overall, pretty happy with this patch.  Still some thoughts for a v3,
>> >> -#define __visible      __attribute__((externally_visible))
>> >> diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
>> >> new file mode 100644
>> >> index 000000000000..a9dfafc8fd19
>> >> --- /dev/null
>> >> +++ b/include/linux/compiler_attributes.h
>> >> @@ -0,0 +1,226 @@
>> >
>> > New file needs an SPDX license identifier right?
>>
>> Yeah, but I wasn't sure of adding it, since the code I moved (even if
>> rearranged) from _types.h does not have it either. Any legal expert
>> here? Is _types.h it implicitly GPL 2? We should add the identifier to
>> both if so.
>
> It looks like we missed that file in the big "properly add SPDX
> identifiers to all files without a license" commit as it came in from a
> different tree.
>
> But yes, it is GPLv2 only implicitly, so you can add that.  I should
> sweep the tree again to see if anything else has been added accidentally
> with that same problem.
>

Thanks Greg! Will do for v3.

Cheers,
Miguel

Re: [PATCH 7/7] Compiler Attributes: use feature checks instead of version checks

[kbuild test robot]

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

Hi Miguel,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v4.19-rc1 next-20180831]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Miguel-Ojeda/Compiler-Attributes-remove-unused-attributes/20180901-125644
config: sh-titan_defconfig (attached as .config)
compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=sh 

All warnings (new ones prefixed by >>):

   In file included from include/linux/byteorder/little_endian.h:5:0,
                    from arch/sh/include/uapi/asm/byteorder.h:6,
                    from arch/sh/include/asm/bitops.h:12,
                    from include/linux/bitops.h:19,
                    from include/linux/kernel.h:11,
                    from include/asm-generic/bug.h:18,
                    from arch/sh/include/asm/bug.h:112,
                    from include/linux/bug.h:5,
                    from include/linux/page-flags.h:10,
                    from kernel/bounds.c:10:
   include/uapi/linux/byteorder/little_endian.h:84:1: warning: 'gnu_inline' attribute ignored [-Wattributes]
    static __always_inline __be16 __cpu_to_be16p(const __u16 *p)
    ^~~~~~
   In file included from <command-line>:0:0:
   include/linux/compiler_types.h:187:16: error: 'no_instrument_function' attribute applies only to functions
    #define inline __always_inline __gnu_inline __maybe_unused notrace
                   ^
   include/linux/compiler_attributes.h:48:33: note: in expansion of macro 'inline'
    #define __always_inline         inline __attribute__((always_inline))
                                    ^~~~~~
   include/uapi/linux/byteorder/little_endian.h:84:8: note: in expansion of macro '__always_inline'
    static __always_inline __be16 __cpu_to_be16p(const __u16 *p)
           ^~~~~~~~~~~~~~~
   In file included from include/linux/byteorder/little_endian.h:5:0,
                    from arch/sh/include/uapi/asm/byteorder.h:6,
                    from arch/sh/include/asm/bitops.h:12,
                    from include/linux/bitops.h:19,
                    from include/linux/kernel.h:11,
                    from include/asm-generic/bug.h:18,
                    from arch/sh/include/asm/bug.h:112,
                    from include/linux/bug.h:5,
                    from include/linux/page-flags.h:10,
                    from kernel/bounds.c:10:
   include/uapi/linux/byteorder/little_endian.h:84:1: warning: 'always_inline' attribute ignored [-Wattributes]
    static __always_inline __be16 __cpu_to_be16p(const __u16 *p)
    ^~~~~~
   include/uapi/linux/byteorder/little_endian.h:84:24: error: expected ',' or ';' before '__be16'
    static __always_inline __be16 __cpu_to_be16p(const __u16 *p)
                           ^~~~~~
   In file included from <command-line>:0:0:
   include/linux/compiler_types.h:187:16: error: type defaults to 'int' in declaration of '__always_inline' [-Werror=implicit-int]
    #define inline __always_inline __gnu_inline __maybe_unused notrace
                   ^
   include/linux/compiler_attributes.h:48:33: note: in expansion of macro 'inline'
    #define __always_inline         inline __attribute__((always_inline))
                                    ^~~~~~
   include/uapi/linux/byteorder/little_endian.h:88:8: note: in expansion of macro '__always_inline'
    static __always_inline __u16 __be16_to_cpup(const __be16 *p)
           ^~~~~~~~~~~~~~~
   In file included from include/linux/byteorder/little_endian.h:5:0,
                    from arch/sh/include/uapi/asm/byteorder.h:6,
                    from arch/sh/include/asm/bitops.h:12,
                    from include/linux/bitops.h:19,
                    from include/linux/kernel.h:11,
                    from include/asm-generic/bug.h:18,
                    from arch/sh/include/asm/bug.h:112,
                    from include/linux/bug.h:5,
                    from include/linux/page-flags.h:10,
                    from kernel/bounds.c:10:
   include/uapi/linux/byteorder/little_endian.h:88:1: warning: 'gnu_inline' attribute ignored [-Wattributes]
    static __always_inline __u16 __be16_to_cpup(const __be16 *p)
    ^~~~~~
   In file included from <command-line>:0:0:
   include/linux/compiler_types.h:187:16: error: 'no_instrument_function' attribute applies only to functions
    #define inline __always_inline __gnu_inline __maybe_unused notrace
                   ^
   include/linux/compiler_attributes.h:48:33: note: in expansion of macro 'inline'
    #define __always_inline         inline __attribute__((always_inline))
                                    ^~~~~~
   include/uapi/linux/byteorder/little_endian.h:88:8: note: in expansion of macro '__always_inline'
    static __always_inline __u16 __be16_to_cpup(const __be16 *p)
           ^~~~~~~~~~~~~~~
   In file included from include/linux/byteorder/little_endian.h:5:0,
                    from arch/sh/include/uapi/asm/byteorder.h:6,
                    from arch/sh/include/asm/bitops.h:12,
                    from include/linux/bitops.h:19,
                    from include/linux/kernel.h:11,
                    from include/asm-generic/bug.h:18,
                    from arch/sh/include/asm/bug.h:112,
                    from include/linux/bug.h:5,
                    from include/linux/page-flags.h:10,
                    from kernel/bounds.c:10:
   include/uapi/linux/byteorder/little_endian.h:88:1: warning: 'always_inline' attribute ignored [-Wattributes]
    static __always_inline __u16 __be16_to_cpup(const __be16 *p)
    ^~~~~~
   include/uapi/linux/byteorder/little_endian.h:88:24: error: expected ',' or ';' before '__u16'
    static __always_inline __u16 __be16_to_cpup(const __be16 *p)
                           ^~~~~
   In file included from include/asm-generic/bug.h:5:0,
                    from arch/sh/include/asm/bug.h:112,
                    from include/linux/bug.h:5,
                    from include/linux/page-flags.h:10,
                    from kernel/bounds.c:10:
   include/linux/atomic.h: In function 'atomic_fetch_add_unless':
   include/linux/compiler.h:252:3: error: implicit declaration of function '__read_once_size' [-Werror=implicit-function-declaration]
      __read_once_size(&(x), __u.__c, sizeof(x));  \
      ^
   include/linux/compiler.h:258:22: note: in expansion of macro '__READ_ONCE'
    #define READ_ONCE(x) __READ_ONCE(x, 1)
                         ^~~~~~~~~~~
>> arch/sh/include/asm/atomic.h:24:25: note: in expansion of macro 'READ_ONCE'
    #define atomic_read(v)  READ_ONCE((v)->counter)
                            ^~~~~~~~~
   include/linux/atomic.h:575:10: note: in expansion of macro 'atomic_read'
     int c = atomic_read(v);
             ^~~~~~~~~~~
   include/linux/compiler.h:254:3: error: implicit declaration of function '__read_once_size_nocheck' [-Werror=implicit-function-declaration]
      __read_once_size_nocheck(&(x), __u.__c, sizeof(x)); \
      ^
   include/linux/compiler.h:258:22: note: in expansion of macro '__READ_ONCE'
    #define READ_ONCE(x) __READ_ONCE(x, 1)
                         ^~~~~~~~~~~
>> arch/sh/include/asm/atomic.h:24:25: note: in expansion of macro 'READ_ONCE'
    #define atomic_read(v)  READ_ONCE((v)->counter)
                            ^~~~~~~~~
   include/linux/atomic.h:575:10: note: in expansion of macro 'atomic_read'
     int c = atomic_read(v);
             ^~~~~~~~~~~
   include/asm-generic/atomic-long.h: In function 'atomic_long_set':
   include/linux/compiler.h:277:2: error: implicit declaration of function '__write_once_size'; did you mean '__builtin_object_size'? [-Werror=implicit-function-declaration]
     __write_once_size(&(x), __u.__c, sizeof(x)); \
     ^
>> arch/sh/include/asm/atomic.h:25:26: note: in expansion of macro 'WRITE_ONCE'
    #define atomic_set(v,i)  WRITE_ONCE((v)->counter, (i))
                             ^~~~~~~~~~
   include/asm-generic/atomic-long.h:35:28: note: in expansion of macro 'atomic_set'
    #define ATOMIC_LONG_PFX(x) atomic ## x
                               ^~~~~~
   include/asm-generic/atomic-long.h:57:2: note: in expansion of macro 'ATOMIC_LONG_PFX'
     ATOMIC_LONG_PFX(_set##mo)(v, i);    \
     ^~~~~~~~~~~~~~~
   include/asm-generic/atomic-long.h:59:1: note: in expansion of macro 'ATOMIC_LONG_SET_OP'
    ATOMIC_LONG_SET_OP()
    ^~~~~~~~~~~~~~~~~~
   In file included from <command-line>:0:0:
   include/asm-generic/atomic-long.h: At top level:
   include/linux/compiler_types.h:187:16: error: type defaults to 'int' in declaration of '__always_inline' [-Werror=implicit-int]
    #define inline __always_inline __gnu_inline __maybe_unused notrace
                   ^
   include/linux/compiler_attributes.h:48:33: note: in expansion of macro 'inline'
    #define __always_inline         inline __attribute__((always_inline))
                                    ^~~~~~
   include/asm-generic/atomic-long.h:119:8: note: in expansion of macro '__always_inline'
    static __always_inline void atomic_long_inc(atomic_long_t *l)
           ^~~~~~~~~~~~~~~
   In file included from include/linux/atomic.h:1315:0,
                    from include/asm-generic/bitops/atomic.h:5,
                    from arch/sh/include/asm/bitops.h:25,
                    from include/linux/bitops.h:19,
                    from include/linux/kernel.h:11,
                    from include/asm-generic/bug.h:18,
                    from arch/sh/include/asm/bug.h:112,
                    from include/linux/bug.h:5,
                    from include/linux/page-flags.h:10,
                    from kernel/bounds.c:10:
   include/asm-generic/atomic-long.h:119:1: warning: 'gnu_inline' attribute ignored [-Wattributes]
    static __always_inline void atomic_long_inc(atomic_long_t *l)
    ^~~~~~
   In file included from <command-line>:0:0:
   include/linux/compiler_types.h:187:16: error: 'no_instrument_function' attribute applies only to functions
    #define inline __always_inline __gnu_inline __maybe_unused notrace
                   ^
   include/linux/compiler_attributes.h:48:33: note: in expansion of macro 'inline'
    #define __always_inline         inline __attribute__((always_inline))
                                    ^~~~~~
   include/asm-generic/atomic-long.h:119:8: note: in expansion of macro '__always_inline'
    static __always_inline void atomic_long_inc(atomic_long_t *l)
           ^~~~~~~~~~~~~~~
   In file included from include/linux/atomic.h:1315:0,
                    from include/asm-generic/bitops/atomic.h:5,
                    from arch/sh/include/asm/bitops.h:25,
                    from include/linux/bitops.h:19,
                    from include/linux/kernel.h:11,
                    from include/asm-generic/bug.h:18,
                    from arch/sh/include/asm/bug.h:112,
                    from include/linux/bug.h:5,
                    from include/linux/page-flags.h:10,
                    from kernel/bounds.c:10:
   include/asm-generic/atomic-long.h:119:1: warning: 'always_inline' attribute ignored [-Wattributes]
    static __always_inline void atomic_long_inc(atomic_long_t *l)
    ^~~~~~
   include/asm-generic/atomic-long.h:119:24: error: expected ',' or ';' before 'void'
    static __always_inline void atomic_long_inc(atomic_long_t *l)
                           ^~~~
   In file included from <command-line>:0:0:
   include/linux/compiler_types.h:187:16: error: type defaults to 'int' in declaration of '__always_inline' [-Werror=implicit-int]
    #define inline __always_inline __gnu_inline __maybe_unused notrace
                   ^
   include/linux/compiler_attributes.h:48:33: note: in expansion of macro 'inline'
    #define __always_inline         inline __attribute__((always_inline))
                                    ^~~~~~
   include/asm-generic/atomic-long.h:126:8: note: in expansion of macro '__always_inline'
    static __always_inline void atomic_long_dec(atomic_long_t *l)
           ^~~~~~~~~~~~~~~
   In file included from include/linux/atomic.h:1315:0,
                    from include/asm-generic/bitops/atomic.h:5,
                    from arch/sh/include/asm/bitops.h:25,
                    from include/linux/bitops.h:19,
                    from include/linux/kernel.h:11,
                    from include/asm-generic/bug.h:18,
                    from arch/sh/include/asm/bug.h:112,
                    from include/linux/bug.h:5,
                    from include/linux/page-flags.h:10,
                    from kernel/bounds.c:10:
   include/asm-generic/atomic-long.h:126:1: warning: 'gnu_inline' attribute ignored [-Wattributes]
    static __always_inline void atomic_long_dec(atomic_long_t *l)
    ^~~~~~
   In file included from <command-line>:0:0:
   include/linux/compiler_types.h:187:16: error: 'no_instrument_function' attribute applies only to functions
    #define inline __always_inline __gnu_inline __maybe_unused notrace
                   ^
   include/linux/compiler_attributes.h:48:33: note: in expansion of macro 'inline'
    #define __always_inline         inline __attribute__((always_inline))
                                    ^~~~~~
   include/asm-generic/atomic-long.h:126:8: note: in expansion of macro '__always_inline'
    static __always_inline void atomic_long_dec(atomic_long_t *l)
           ^~~~~~~~~~~~~~~
   In file included from include/linux/atomic.h:1315:0,
                    from include/asm-generic/bitops/atomic.h:5,
                    from arch/sh/include/asm/bitops.h:25,
                    from include/linux/bitops.h:19,
                    from include/linux/kernel.h:11,
                    from include/asm-generic/bug.h:18,

vim +/READ_ONCE +24 arch/sh/include/asm/atomic.h

^1da177e include/asm-sh/atomic.h      Linus Torvalds 2005-04-16  23  
62e8a325 arch/sh/include/asm/atomic.h Peter Zijlstra 2015-09-18 @24  #define atomic_read(v)		READ_ONCE((v)->counter)
62e8a325 arch/sh/include/asm/atomic.h Peter Zijlstra 2015-09-18 @25  #define atomic_set(v,i)		WRITE_ONCE((v)->counter, (i))
^1da177e include/asm-sh/atomic.h      Linus Torvalds 2005-04-16  26  

:::::: The code at line 24 was first introduced by commit
:::::: 62e8a3258bda118f24ff462fe04cfbe75b8189b5 atomic, arch: Audit atomic_{read,set}()

:::::: TO: Peter Zijlstra <peterz@infradead.org>
:::::: CC: Ingo Molnar <mingo@kernel.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 16391 bytes --]

Re: [PATCH 7/7] Compiler Attributes: use feature checks instead of version checks

[kbuild test robot]

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

Hi Miguel,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v4.19-rc1 next-20180831]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Miguel-Ojeda/Compiler-Attributes-remove-unused-attributes/20180901-125644
config: parisc-default_defconfig (attached as .config)
compiler: hppa-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        GCC_VERSION=7.2.0 make.cross ARCH=parisc 

All warnings (new ones prefixed by >>):

                    from include/linux/bitops.h:19,
                    from include/linux/kernel.h:11,
                    from arch/parisc/include/asm/bug.h:5,
                    from include/linux/bug.h:5,
                    from include/linux/page-flags.h:10,
                    from kernel/bounds.c:10:
   include/uapi/linux/byteorder/big_endian.h:84:1: warning: 'gnu_inline' attribute ignored [-Wattributes]
    static __always_inline __be16 __cpu_to_be16p(const __u16 *p)
    ^~~~~~
   In file included from <command-line>:0:0:
   include/linux/compiler_types.h:187:16: error: 'no_instrument_function' attribute applies only to functions
    #define inline __always_inline __gnu_inline __maybe_unused notrace
                   ^
   include/linux/compiler_attributes.h:48:33: note: in expansion of macro 'inline'
    #define __always_inline         inline __attribute__((always_inline))
                                    ^~~~~~
   include/uapi/linux/byteorder/big_endian.h:84:8: note: in expansion of macro '__always_inline'
    static __always_inline __be16 __cpu_to_be16p(const __u16 *p)
           ^~~~~~~~~~~~~~~
   In file included from include/linux/byteorder/big_endian.h:5:0,
                    from arch/parisc/include/uapi/asm/byteorder.h:5,
                    from arch/parisc/include/asm/bitops.h:11,
                    from include/linux/bitops.h:19,
                    from include/linux/kernel.h:11,
                    from arch/parisc/include/asm/bug.h:5,
                    from include/linux/bug.h:5,
                    from include/linux/page-flags.h:10,
                    from kernel/bounds.c:10:
   include/uapi/linux/byteorder/big_endian.h:84:1: warning: 'always_inline' attribute ignored [-Wattributes]
    static __always_inline __be16 __cpu_to_be16p(const __u16 *p)
    ^~~~~~
   include/uapi/linux/byteorder/big_endian.h:84:24: error: expected ',' or ';' before '__be16'
    static __always_inline __be16 __cpu_to_be16p(const __u16 *p)
                           ^~~~~~
   In file included from <command-line>:0:0:
   include/linux/compiler_types.h:187:16: error: type defaults to 'int' in declaration of '__always_inline' [-Werror=implicit-int]
    #define inline __always_inline __gnu_inline __maybe_unused notrace
                   ^
   include/linux/compiler_attributes.h:48:33: note: in expansion of macro 'inline'
    #define __always_inline         inline __attribute__((always_inline))
                                    ^~~~~~
   include/uapi/linux/byteorder/big_endian.h:88:8: note: in expansion of macro '__always_inline'
    static __always_inline __u16 __be16_to_cpup(const __be16 *p)
           ^~~~~~~~~~~~~~~
   In file included from include/linux/byteorder/big_endian.h:5:0,
                    from arch/parisc/include/uapi/asm/byteorder.h:5,
                    from arch/parisc/include/asm/bitops.h:11,
                    from include/linux/bitops.h:19,
                    from include/linux/kernel.h:11,
                    from arch/parisc/include/asm/bug.h:5,
                    from include/linux/bug.h:5,
                    from include/linux/page-flags.h:10,
                    from kernel/bounds.c:10:
   include/uapi/linux/byteorder/big_endian.h:88:1: warning: 'gnu_inline' attribute ignored [-Wattributes]
    static __always_inline __u16 __be16_to_cpup(const __be16 *p)
    ^~~~~~
   In file included from <command-line>:0:0:
   include/linux/compiler_types.h:187:16: error: 'no_instrument_function' attribute applies only to functions
    #define inline __always_inline __gnu_inline __maybe_unused notrace
                   ^
   include/linux/compiler_attributes.h:48:33: note: in expansion of macro 'inline'
    #define __always_inline         inline __attribute__((always_inline))
                                    ^~~~~~
   include/uapi/linux/byteorder/big_endian.h:88:8: note: in expansion of macro '__always_inline'
    static __always_inline __u16 __be16_to_cpup(const __be16 *p)
           ^~~~~~~~~~~~~~~
   In file included from include/linux/byteorder/big_endian.h:5:0,
                    from arch/parisc/include/uapi/asm/byteorder.h:5,
                    from arch/parisc/include/asm/bitops.h:11,
                    from include/linux/bitops.h:19,
                    from include/linux/kernel.h:11,
                    from arch/parisc/include/asm/bug.h:5,
                    from include/linux/bug.h:5,
                    from include/linux/page-flags.h:10,
                    from kernel/bounds.c:10:
   include/uapi/linux/byteorder/big_endian.h:88:1: warning: 'always_inline' attribute ignored [-Wattributes]
    static __always_inline __u16 __be16_to_cpup(const __be16 *p)
    ^~~~~~
   include/uapi/linux/byteorder/big_endian.h:88:24: error: expected ',' or ';' before '__u16'
    static __always_inline __u16 __be16_to_cpup(const __be16 *p)
                           ^~~~~
   include/linux/byteorder/generic.h: In function 'le32_to_cpu_array':
   include/uapi/linux/byteorder/big_endian.h:95:27: error: implicit declaration of function '__swab32s'; did you mean '__swahb32s'? [-Werror=implicit-function-declaration]
    #define __le32_to_cpus(x) __swab32s((x))
                              ^
   include/linux/byteorder/generic.h:163:3: note: in expansion of macro '__le32_to_cpus'
      __le32_to_cpus(buf);
      ^~~~~~~~~~~~~~
   In file included from include/linux/kernel.h:10:0,
                    from arch/parisc/include/asm/bug.h:5,
                    from include/linux/bug.h:5,
                    from include/linux/page-flags.h:10,
                    from kernel/bounds.c:10:
   arch/parisc/include/asm/atomic.h: In function 'atomic_read':
   include/linux/compiler.h:252:3: error: implicit declaration of function '__read_once_size' [-Werror=implicit-function-declaration]
      __read_once_size(&(x), __u.__c, sizeof(x));  \
      ^
   include/linux/compiler.h:258:22: note: in expansion of macro '__READ_ONCE'
    #define READ_ONCE(x) __READ_ONCE(x, 1)
                         ^~~~~~~~~~~
>> arch/parisc/include/asm/atomic.h:73:9: note: in expansion of macro 'READ_ONCE'
     return READ_ONCE((v)->counter);
            ^~~~~~~~~
   include/linux/compiler.h:254:3: error: implicit declaration of function '__read_once_size_nocheck' [-Werror=implicit-function-declaration]
      __read_once_size_nocheck(&(x), __u.__c, sizeof(x)); \
      ^
   include/linux/compiler.h:258:22: note: in expansion of macro '__READ_ONCE'
    #define READ_ONCE(x) __READ_ONCE(x, 1)
                         ^~~~~~~~~~~
>> arch/parisc/include/asm/atomic.h:73:9: note: in expansion of macro 'READ_ONCE'
     return READ_ONCE((v)->counter);
            ^~~~~~~~~
   In file included from <command-line>:0:0:
   include/asm-generic/atomic-long.h: At top level:
   include/linux/compiler_types.h:187:16: error: type defaults to 'int' in declaration of '__always_inline' [-Werror=implicit-int]
    #define inline __always_inline __gnu_inline __maybe_unused notrace
                   ^
   include/linux/compiler_attributes.h:48:33: note: in expansion of macro 'inline'
    #define __always_inline         inline __attribute__((always_inline))
                                    ^~~~~~
   include/asm-generic/atomic-long.h:119:8: note: in expansion of macro '__always_inline'
    static __always_inline void atomic_long_inc(atomic_long_t *l)
           ^~~~~~~~~~~~~~~
   In file included from include/linux/atomic.h:1315:0,
                    from arch/parisc/include/asm/bitops.h:13,
                    from include/linux/bitops.h:19,
                    from include/linux/kernel.h:11,
                    from arch/parisc/include/asm/bug.h:5,
                    from include/linux/bug.h:5,
                    from include/linux/page-flags.h:10,
                    from kernel/bounds.c:10:
   include/asm-generic/atomic-long.h:119:1: warning: 'gnu_inline' attribute ignored [-Wattributes]
    static __always_inline void atomic_long_inc(atomic_long_t *l)
    ^~~~~~
   In file included from <command-line>:0:0:
   include/linux/compiler_types.h:187:16: error: 'no_instrument_function' attribute applies only to functions
    #define inline __always_inline __gnu_inline __maybe_unused notrace
                   ^
   include/linux/compiler_attributes.h:48:33: note: in expansion of macro 'inline'
    #define __always_inline         inline __attribute__((always_inline))
                                    ^~~~~~
   include/asm-generic/atomic-long.h:119:8: note: in expansion of macro '__always_inline'
    static __always_inline void atomic_long_inc(atomic_long_t *l)
           ^~~~~~~~~~~~~~~
   In file included from include/linux/atomic.h:1315:0,
                    from arch/parisc/include/asm/bitops.h:13,
                    from include/linux/bitops.h:19,
                    from include/linux/kernel.h:11,
                    from arch/parisc/include/asm/bug.h:5,
                    from include/linux/bug.h:5,
                    from include/linux/page-flags.h:10,
                    from kernel/bounds.c:10:
   include/asm-generic/atomic-long.h:119:1: warning: 'always_inline' attribute ignored [-Wattributes]
    static __always_inline void atomic_long_inc(atomic_long_t *l)
    ^~~~~~
   include/asm-generic/atomic-long.h:119:24: error: expected ',' or ';' before 'void'
    static __always_inline void atomic_long_inc(atomic_long_t *l)
                           ^~~~
   In file included from <command-line>:0:0:
   include/linux/compiler_types.h:187:16: error: type defaults to 'int' in declaration of '__always_inline' [-Werror=implicit-int]
    #define inline __always_inline __gnu_inline __maybe_unused notrace
                   ^
   include/linux/compiler_attributes.h:48:33: note: in expansion of macro 'inline'
    #define __always_inline         inline __attribute__((always_inline))
                                    ^~~~~~
   include/asm-generic/atomic-long.h:126:8: note: in expansion of macro '__always_inline'
    static __always_inline void atomic_long_dec(atomic_long_t *l)
           ^~~~~~~~~~~~~~~
   In file included from include/linux/atomic.h:1315:0,
                    from arch/parisc/include/asm/bitops.h:13,
                    from include/linux/bitops.h:19,
                    from include/linux/kernel.h:11,
                    from arch/parisc/include/asm/bug.h:5,
                    from include/linux/bug.h:5,
                    from include/linux/page-flags.h:10,
                    from kernel/bounds.c:10:
   include/asm-generic/atomic-long.h:126:1: warning: 'gnu_inline' attribute ignored [-Wattributes]
    static __always_inline void atomic_long_dec(atomic_long_t *l)
    ^~~~~~
   In file included from <command-line>:0:0:
   include/linux/compiler_types.h:187:16: error: 'no_instrument_function' attribute applies only to functions
    #define inline __always_inline __gnu_inline __maybe_unused notrace
                   ^
   include/linux/compiler_attributes.h:48:33: note: in expansion of macro 'inline'
    #define __always_inline         inline __attribute__((always_inline))
                                    ^~~~~~
   include/asm-generic/atomic-long.h:126:8: note: in expansion of macro '__always_inline'
    static __always_inline void atomic_long_dec(atomic_long_t *l)
           ^~~~~~~~~~~~~~~
   In file included from include/linux/atomic.h:1315:0,
                    from arch/parisc/include/asm/bitops.h:13,
                    from include/linux/bitops.h:19,
                    from include/linux/kernel.h:11,
                    from arch/parisc/include/asm/bug.h:5,
                    from include/linux/bug.h:5,
                    from include/linux/page-flags.h:10,
                    from kernel/bounds.c:10:
   include/asm-generic/atomic-long.h:126:1: warning: 'always_inline' attribute ignored [-Wattributes]
    static __always_inline void atomic_long_dec(atomic_long_t *l)
    ^~~~~~
   include/asm-generic/atomic-long.h:126:24: error: expected ',' or ';' before 'void'
    static __always_inline void atomic_long_dec(atomic_long_t *l)
                           ^~~~
   In file included from <command-line>:0:0:
   include/linux/compiler_types.h:187:16: error: type defaults to 'int' in declaration of '__always_inline' [-Werror=implicit-int]
    #define inline __always_inline __gnu_inline __maybe_unused notrace
                   ^
   include/linux/compiler_attributes.h:48:33: note: in expansion of macro 'inline'
    #define __always_inline         inline __attribute__((always_inline))
                                    ^~~~~~

vim +/READ_ONCE +73 arch/parisc/include/asm/atomic.h

9d664c0ae arch/parisc/include/asm/atomic.h Peter Zijlstra 2017-06-09  70  
^1da177e4 include/asm-parisc/atomic.h      Linus Torvalds 2005-04-16  71  static __inline__ int atomic_read(const atomic_t *v)
^1da177e4 include/asm-parisc/atomic.h      Linus Torvalds 2005-04-16  72  {
62e8a3258 arch/parisc/include/asm/atomic.h Peter Zijlstra 2015-09-18 @73  	return READ_ONCE((v)->counter);
^1da177e4 include/asm-parisc/atomic.h      Linus Torvalds 2005-04-16  74  }
^1da177e4 include/asm-parisc/atomic.h      Linus Torvalds 2005-04-16  75  

:::::: The code at line 73 was first introduced by commit
:::::: 62e8a3258bda118f24ff462fe04cfbe75b8189b5 atomic, arch: Audit atomic_{read,set}()

:::::: TO: Peter Zijlstra <peterz@infradead.org>
:::::: CC: Ingo Molnar <mingo@kernel.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 15263 bytes --]

Re: [PATCH 7/7] Compiler Attributes: use feature checks instead of version checks

[Luc Van Oostenryck]

On Fri, Aug 31, 2018 at 07:05:14PM +0200, Miguel Ojeda wrote:
> Instead of using version checks per-compiler to define (or not)
> each attribute, use __has_attribute to test for them, following
> the cleanup started with commit 815f0ddb346c
> ("include/linux/compiler*.h: make compiler-*.h mutually exclusive").
> 
> All the attributes that are fairly common/standard (i.e. those that
> do not require extra logic to define them) have been moved
> to a new file include/linux/compiler_attributes.h.
> 
> In an effort to make the file as regular as possible, comments
> stating the purpose of attributes have been removed. Instead,
> links to the compiler docs have been added (i.e. to gcc and,
> if available, to clang as well). In addition, they have been sorted.
> 
> Finally, if an attribute is optional (i.e. if it is guarded
> by __has_attribute), the reason has been stated for future reference.
> 
> diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
> new file mode 100644
> index 000000000000..a9dfafc8fd19
> --- /dev/null
> +++ b/include/linux/compiler_attributes.h
> @@ -0,0 +1,226 @@
> +#ifndef __LINUX_COMPILER_ATTRIBUTES_H
> +#define __LINUX_COMPILER_ATTRIBUTES_H
> +
> +/*
> + * This file is meant to be sorted (by actual attribute name,
> + * not by #define identifier).
> + *
> + * Do not add here attributes which depend on others or require extra logic.
> + *
> + * If an attribute is optional, state the reason in the comment.
> + */
> +
> +/*
> + * To check for optional attributes, we use __has_attribute, which is supported
> + * on gcc >= 5, clang >= 2.9 and icc >= 17. In the meantime, to support
> + * 4.6 <= gcc < 5, we implement __has_attribute by hand.
> + */
> +#ifndef __has_attribute
> +#define __has_attribute(x) __GCC4_has_attribute_##x
> +#define __GCC4_has_attribute_externally_visible 1
> +#define __GCC4_has_attribute_noclone 1
> +#define __GCC4_has_attribute_optimize 1
> +#if __GNUC_MINOR__ >= 8
> +#define __GCC4_has_attribute_no_sanitize_address 1
> +#endif
> +#if __GNUC_MINOR__ >= 9
> +#define __GCC4_has_attribute_assume_aligned 1
> +#endif
> +#endif

For sparse (which doesn't support __has_attribute() yet and defines
__GNUC_MINOR__ depending on the compiler used to build it) it won't
be totally correct since the concerned attributes here will be
incorrectly considered as not supported. But, since these attributes
have no semantic effects for sparse, it won't matter.

So, for sparse:
Reviewed-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

-- Luc

Re: [PATCH 7/7] Compiler Attributes: use feature checks instead of version checks

[Miguel Ojeda]

Hi Luc,

On Sat, Sep 1, 2018 at 11:56 AM, Luc Van Oostenryck
<luc.vanoostenryck@gmail.com> wrote:
>
> For sparse (which doesn't support __has_attribute() yet and defines
> __GNUC_MINOR__ depending on the compiler used to build it) it won't
> be totally correct since the concerned attributes here will be
> incorrectly considered as not supported. But, since these attributes
> have no semantic effects for sparse, it won't matter.
>
> So, for sparse:
> Reviewed-by: Luc Van Oostenryck <luc.vanoostenryck@gmail.com>

Thanks for checking (and for the sparse patches too!). I will note
this in the commit message for v3.

Cheers,
Miguel

Re: [PATCH 7/7] Compiler Attributes: use feature checks instead of version checks

[Dominique Martinet]

Miguel Ojeda wrote on Fri, Aug 31, 2018:
> Instead of using version checks per-compiler to define (or not)
> each attribute, use __has_attribute to test for them, following
> the cleanup started with commit 815f0ddb346c
> ("include/linux/compiler*.h: make compiler-*.h mutually exclusive").
> 
> All the attributes that are fairly common/standard (i.e. those that
> do not require extra logic to define them) have been moved
> to a new file include/linux/compiler_attributes.h.
> 
> In an effort to make the file as regular as possible, comments
> stating the purpose of attributes have been removed. Instead,
> links to the compiler docs have been added (i.e. to gcc and,
> if available, to clang as well). In addition, they have been sorted.
> 
> Finally, if an attribute is optional (i.e. if it is guarded
> by __has_attribute), the reason has been stated for future reference.
> 
> Cc: Eli Friedman <efriedma@codeaurora.org>
> Cc: Christopher Li <sparse@chrisli.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Joe Perches <joe@perches.com>
> Cc: Dominique Martinet <asmadeus@codewreck.org>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>

Nice work!
Since I'm being Cc'd I took the time to test this as well, and have no
problem with libbcc-building-with-clang (or native x86 gcc build)

Nick already made many comments so I only have one more.

> [...]
> diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
> new file mode 100644
> index 000000000000..a9dfafc8fd19
> --- /dev/null
> +++ b/include/linux/compiler_attributes.h
> [...]
> +/*
> + * To check for optional attributes, we use __has_attribute, which is supported
> + * on gcc >= 5, clang >= 2.9 and icc >= 17. In the meantime, to support
> + * 4.6 <= gcc < 5, we implement __has_attribute by hand.
> + */
> +#ifndef __has_attribute
> +#define __has_attribute(x) __GCC4_has_attribute_##x
> +#define __GCC4_has_attribute_externally_visible 1
> +#define __GCC4_has_attribute_noclone 1
> +#define __GCC4_has_attribute_optimize 1
> +#if __GNUC_MINOR__ >= 8
> +#define __GCC4_has_attribute_no_sanitize_address 1
> +#endif
> +#if __GNUC_MINOR__ >= 9
> +#define __GCC4_has_attribute_assume_aligned 1
> +#endif
> +#endif

Hmm, if this is in this file and not compiler-gcc, I am not sure about
using GNUC_MINOR without checking the major -- I have no idea what kind
of versions e.g. icc will use (or what attributes ancients version of
clang or old icc support, actually)


It's a bit of research work but I think it'd be cleaner to define
similar macros for all three compilers, if we care about the old
versions... Or actually..

For clang you've implicitely required clang >= 3.0 in patch 3 of this
serie, so presumabely it wouldn't need this compat macro at all.

For icc I think icc 17 is still fairly recent... But I just abused work
to test and linux fails to compile with icc 15/17/18 for other reasons
(unrelated to this patch), so unless anyone helps with this I'm tempted
to suggest leaving it at it, and whoever that is will probably have a
better idea of how far back they want to make icc work / what attributes
are defined there.
It's a bit of a shame there's no linux-compilers list to reach out to :)

(this would need to move the include of this file after the
compiler-specific headers, but from what I can see there is no problem
with that)

-- 
Dominique

Re: [PATCH 7/7] Compiler Attributes: use feature checks instead of version checks

[Miguel Ojeda]

Hi Dominique,

On Sat, Sep 1, 2018 at 12:14 PM, Dominique Martinet
<asmadeus@codewreck.org> wrote:
> Miguel Ojeda wrote on Fri, Aug 31, 2018:
>> Instead of using version checks per-compiler to define (or not)
>> each attribute, use __has_attribute to test for them, following
>> the cleanup started with commit 815f0ddb346c
>> ("include/linux/compiler*.h: make compiler-*.h mutually exclusive").
>>
>> All the attributes that are fairly common/standard (i.e. those that
>> do not require extra logic to define them) have been moved
>> to a new file include/linux/compiler_attributes.h.
>>
>> In an effort to make the file as regular as possible, comments
>> stating the purpose of attributes have been removed. Instead,
>> links to the compiler docs have been added (i.e. to gcc and,
>> if available, to clang as well). In addition, they have been sorted.
>>
>> Finally, if an attribute is optional (i.e. if it is guarded
>> by __has_attribute), the reason has been stated for future reference.
>>
>> Cc: Eli Friedman <efriedma@codeaurora.org>
>> Cc: Christopher Li <sparse@chrisli.org>
>> Cc: Kees Cook <keescook@chromium.org>
>> Cc: Ingo Molnar <mingo@kernel.org>
>> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
>> Cc: Arnd Bergmann <arnd@arndb.de>
>> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
>> Cc: Joe Perches <joe@perches.com>
>> Cc: Dominique Martinet <asmadeus@codewreck.org>
>> Cc: Nick Desaulniers <ndesaulniers@google.com>
>> Cc: Linus Torvalds <torvalds@linux-foundation.org>
>> Signed-off-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
>
> Nice work!
> Since I'm being Cc'd I took the time to test this as well, and have no
> problem with libbcc-building-with-clang (or native x86 gcc build)

Thanks a lot for testing! :-)

>
> Nick already made many comments so I only have one more.
>
>> [...]
>> diff --git a/include/linux/compiler_attributes.h b/include/linux/compiler_attributes.h
>> new file mode 100644
>> index 000000000000..a9dfafc8fd19
>> --- /dev/null
>> +++ b/include/linux/compiler_attributes.h
>> [...]
>> +/*
>> + * To check for optional attributes, we use __has_attribute, which is supported
>> + * on gcc >= 5, clang >= 2.9 and icc >= 17. In the meantime, to support
>> + * 4.6 <= gcc < 5, we implement __has_attribute by hand.
>> + */
>> +#ifndef __has_attribute
>> +#define __has_attribute(x) __GCC4_has_attribute_##x
>> +#define __GCC4_has_attribute_externally_visible 1
>> +#define __GCC4_has_attribute_noclone 1
>> +#define __GCC4_has_attribute_optimize 1
>> +#if __GNUC_MINOR__ >= 8
>> +#define __GCC4_has_attribute_no_sanitize_address 1
>> +#endif
>> +#if __GNUC_MINOR__ >= 9
>> +#define __GCC4_has_attribute_assume_aligned 1
>> +#endif
>> +#endif
>
> Hmm, if this is in this file and not compiler-gcc, I am not sure about
> using GNUC_MINOR without checking the major -- I have no idea what kind
> of versions e.g. icc will use (or what attributes ancients version of
> clang or old icc support, actually)

The idea here is that all non-gcc recent compilers that we may care
about support __has_attribute,so if we are inside the #ifndef, we
*have* to be gcc < 5 (see https://godbolt.org/z/NQFdsK).

>
>
> It's a bit of research work but I think it'd be cleaner to define
> similar macros for all three compilers, if we care about the old
> versions... Or actually..
>
> For clang you've implicitely required clang >= 3.0 in patch 3 of this
> serie, so presumabely it wouldn't need this compat macro at all.
>
> For icc I think icc 17 is still fairly recent... But I just abused work
> to test and linux fails to compile with icc 15/17/18 for other reasons
> (unrelated to this patch), so unless anyone helps with this I'm tempted

Thanks a lot for testing it! I agree that icc 17 is fairly recent, but
is there anybody using it on a regular basis to compile the kernel --
I guess your tests confirm that there isn't, but you never know... :-)
As for clang < 2.9, I doubt anybody is using it for anything
meaningful.

Also, we should note that this is about compiling the latest greatest
kernel release, which someone with older compilers is less likely to
be doing with clang and icc than gcc, I guess.

As for the failures: indeed, if anyone is actually doing it, they
probably need to apply quite some third-party patches anyway, so they
can also patch this file as well. Otherwise, they can always speak up
and ask for support. If nobody says anything though, then we have
proof nobody cares about it...

> to suggest leaving it at it, and whoever that is will probably have a
> better idea of how far back they want to make icc work / what attributes
> are defined there.
> It's a bit of a shame there's no linux-compilers list to reach out to :)
>
> (this would need to move the include of this file after the
> compiler-specific headers, but from what I can see there is no problem
> with that)

I included it as soon as possible so that other code (e.g. the
compiler-specific headers) can depend on the common attributes for
anything they need (in this way, the compiler_attributes.h can be
considered like it defines some extensions to the C language that we
allow all code in the kernel to use -- as if it was not there to begin
with).

By the way, do you know if there are some public docs on the
attributes supported by icc that we can link to? I couldn't find
anything in a quick search, both in google and in the icc's
"Compatibility" pages...

Cheers,
Miguel

Re: [PATCH 7/7] Compiler Attributes: use feature checks instead of version checks

[Dominique Martinet]

Miguel Ojeda wrote on Sat, Sep 01, 2018:
>>> +/*
>>> + * To check for optional attributes, we use __has_attribute, which is supported
>>> + * on gcc >= 5, clang >= 2.9 and icc >= 17. In the meantime, to support
>>> + * 4.6 <= gcc < 5, we implement __has_attribute by hand.
>>> + */
>>> +#ifndef __has_attribute
>>> +#define __has_attribute(x) __GCC4_has_attribute_##x
>>> +#define __GCC4_has_attribute_externally_visible 1
>>> +#define __GCC4_has_attribute_noclone 1
>>> +#define __GCC4_has_attribute_optimize 1
>>> +#if __GNUC_MINOR__ >= 8
>>> +#define __GCC4_has_attribute_no_sanitize_address 1
>>> +#endif
>>> +#if __GNUC_MINOR__ >= 9
>>> +#define __GCC4_has_attribute_assume_aligned 1
>>> +#endif
>>> +#endif
>>
>> Hmm, if this is in this file and not compiler-gcc, I am not sure about
>> using GNUC_MINOR without checking the major -- I have no idea what kind
>> of versions e.g. icc will use (or what attributes ancients version of
>> clang or old icc support, actually)
> 
> The idea here is that all non-gcc recent compilers that we may care
> about support __has_attribute,so if we are inside the #ifndef, we
> *have* to be gcc < 5 (see https://godbolt.org/z/NQFdsK).

That really makes me want to have these in compiler-gcc though :)

But I guess I can also understand wanting to include this file as early
as possible, although I do not think any of the compiler-specific
headers use any of this they could eventually want to override something
from it at some point... Leaving it as an open question, feel free to do
as you prefer here.


> By the way, do you know if there are some public docs on the
> attributes supported by icc that we can link to? I couldn't find
> anything in a quick search, both in google and in the icc's
> "Compatibility" pages...

I'm honestly not using icc much, I just have access to machines where it
is installed...
Actually, now I'm looking there is some html documentation installed
along with icc, but it does not seem complete; instead it points to
software.intel.com ... which has _some_ documentation on attributes, but
is missing a _lot_ of them, see :
https://software.intel.com/en-us/cpp-compiler-18.0-developer-guide-and-reference-attributes

I saw that a lot of these attributes were defined as #pragma first for
icc and had hoped to see something like 'alternative forms' in the
pragma documentations but there does not seem to be anything there
either... Afraid I won't be of much help there.



(from another reply)
> Because of that logic, I tried to simplify as much as possible all
> existing things (e.g. assume_aligned by removing __CHECKER__) so that
> they could be moved to compiler_attributes.h. In a way,
> compiler_attributes.h defines some extensions to the C language that
> all code could use anywhere (actually whether we can move them out of
> __KERNEL__ etc. is another question)

(very good question, though! Figuring out how to share these with the
tools/include/linux/compiler* instead of having an old copy would be a
good start... For another patchset though)

-- 
Dominique

Re: [PATCH 7/7] Compiler Attributes: use feature checks instead of version checks

[Miguel Ojeda]

Hi Dominique,

On Sat, Sep 1, 2018 at 4:17 PM, Dominique Martinet
<asmadeus@codewreck.org> wrote:
>
> I'm honestly not using icc much, I just have access to machines where it
> is installed...
> Actually, now I'm looking there is some html documentation installed
> along with icc, but it does not seem complete; instead it points to
> software.intel.com ... which has _some_ documentation on attributes, but
> is missing a _lot_ of them, see :
> https://software.intel.com/en-us/cpp-compiler-18.0-developer-guide-and-reference-attributes
>

Hm... I am finishing v3 and I was thinking of adding this link (since
it is what we should link to), but it has so little information on
just a few attributes that I don't think it is worth it :-(

Thanks for finding it out, though!

Cheers,
Miguel

Re: [PATCH 1/7] Compiler Attributes: remove unused attributes

[Joe Perches]

On Fri, 2018-08-31 at 19:05 +0200, Miguel Ojeda wrote:
> __optimize and __deprecate_for_modules are unused in
> the whole kernel tree. Simply drop them.

Nice series, thanks Miguel.

It'd be good to have a cover letter for the series.

And I believe there should be the equivalent of:

#if GCC_VERSION < 40600
# error Sorry, your compiler is too old - please upgrade it.
#endif

for compiler-intel.h and compiler-clang.h so that
each supported compiler minimum version is checked.

Is it clang > 13 and icc > 3 ?

Re: [PATCH 1/7] Compiler Attributes: remove unused attributes

[Nick Desaulniers]

On Fri, Aug 31, 2018 at 10:28 AM Joe Perches <joe@perches.com> wrote:
>
> On Fri, 2018-08-31 at 19:05 +0200, Miguel Ojeda wrote:
> > __optimize and __deprecate_for_modules are unused in
> > the whole kernel tree. Simply drop them.
>
> Nice series, thanks Miguel.
>
> It'd be good to have a cover letter for the series.
>
> And I believe there should be the equivalent of:
>
> #if GCC_VERSION < 40600
> # error Sorry, your compiler is too old - please upgrade it.
> #endif
>
> for compiler-intel.h and compiler-clang.h so that
> each supported compiler minimum version is checked.
>
> Is it clang > 13 and icc > 3 ?

Eh, I'm not sure I want to commit yet to a specific minimal version of
Clang.  Right now, we're fixing things so depending on arch's and
configs, the answer might be Top of Tree clang builds.  For Pixel, we
shipped with Clang-4, but pretty quickly we needed Clang-5.
https://lkml.org/lkml/2017/11/22/943

I had sent patches previously for detecting clang version from the C
preprocessor, maybe I should dust those off, then commit to clang 5.

I don't think minimal supported versions are required for these clean
ups, and would not block these patches from landing on that.

Also, haven't found anyone using ICC yet to comment on minimal version
requirements.
-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 1/7] Compiler Attributes: remove unused attributes

[Joe Perches]

On Fri, 2018-08-31 at 11:43 -0700, Nick Desaulniers wrote:
> I don't think minimal supported versions are required for these clean
> ups, and would not block these patches from landing on that.

Nor would I, but it's one of those symmetry things.

Likely, someone would eventually complain if their old
icc/clang compiler failed to produce a functional kernel.

If the minimum versions aren't really known and nobody
notices, then it really doesn't matter.

Re: [PATCH 1/7] Compiler Attributes: remove unused attributes

[Miguel Ojeda]

On Fri, Aug 31, 2018 at 8:43 PM, Nick Desaulniers
<ndesaulniers@google.com> wrote:
> On Fri, Aug 31, 2018 at 10:28 AM Joe Perches <joe@perches.com> wrote:
>>
>> On Fri, 2018-08-31 at 19:05 +0200, Miguel Ojeda wrote:
>> > __optimize and __deprecate_for_modules are unused in
>> > the whole kernel tree. Simply drop them.
>>
>> Nice series, thanks Miguel.
>>
>> It'd be good to have a cover letter for the series.
>>
>> And I believe there should be the equivalent of:
>>
>> #if GCC_VERSION < 40600
>> # error Sorry, your compiler is too old - please upgrade it.
>> #endif
>>
>> for compiler-intel.h and compiler-clang.h so that
>> each supported compiler minimum version is checked.
>>
>> Is it clang > 13 and icc > 3 ?
>
> Eh, I'm not sure I want to commit yet to a specific minimal version of
> Clang.  Right now, we're fixing things so depending on arch's and
> configs, the answer might be Top of Tree clang builds.  For Pixel, we
> shipped with Clang-4, but pretty quickly we needed Clang-5.
> https://lkml.org/lkml/2017/11/22/943
>
> I had sent patches previously for detecting clang version from the C
> preprocessor, maybe I should dust those off, then commit to clang 5.

In my opinion, even if you require clang 7, that is fine, as long as
we get a working build mainlined.

By the way, I am testing the series with clang 8 (2018-08-14) (after
reverting e501ce957a78), and it seems to work. Hopefully that makes
you happy! ;-)

>
> I don't think minimal supported versions are required for these clean
> ups, and would not block these patches from landing on that.
>
> Also, haven't found anyone using ICC yet to comment on minimal version
> requirements.

For clang, by the way, __naked should go out of -gcc.h. I guess that
is breaking ARM clang builds at the moment (didn't check)? I will
include the move for v3.

Cheers,
Miguel

Re: [PATCH 1/7] Compiler Attributes: remove unused attributes

[Nick Desaulniers]

On Fri, Aug 31, 2018 at 1:23 PM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> On Fri, Aug 31, 2018 at 8:43 PM, Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> > On Fri, Aug 31, 2018 at 10:28 AM Joe Perches <joe@perches.com> wrote:
> >>
> >> On Fri, 2018-08-31 at 19:05 +0200, Miguel Ojeda wrote:
> >> > __optimize and __deprecate_for_modules are unused in
> >> > the whole kernel tree. Simply drop them.
> >>
> >> Nice series, thanks Miguel.
> >>
> >> It'd be good to have a cover letter for the series.
> >>
> >> And I believe there should be the equivalent of:
> >>
> >> #if GCC_VERSION < 40600
> >> # error Sorry, your compiler is too old - please upgrade it.
> >> #endif
> >>
> >> for compiler-intel.h and compiler-clang.h so that
> >> each supported compiler minimum version is checked.
> >>
> >> Is it clang > 13 and icc > 3 ?
> >
> > Eh, I'm not sure I want to commit yet to a specific minimal version of
> > Clang.  Right now, we're fixing things so depending on arch's and
> > configs, the answer might be Top of Tree clang builds.  For Pixel, we
> > shipped with Clang-4, but pretty quickly we needed Clang-5.
> > https://lkml.org/lkml/2017/11/22/943
> >
> > I had sent patches previously for detecting clang version from the C
> > preprocessor, maybe I should dust those off, then commit to clang 5.
>
> In my opinion, even if you require clang 7, that is fine, as long as
> we get a working build mainlined.

Clang 7 is aggressive.  I need to think more about how to call out
when a specific set of configs for a given arch requires a compiler
upgrade, without ending up with combinatoral explosion.  I don't want
to cross that bridge with this patch set.

>
> By the way, I am testing the series with clang 8 (2018-08-14) (after
> reverting e501ce957a78), and it seems to work. Hopefully that makes
> you happy! ;-)

That makes me very happy.  It indeed does produce a run-able
executable, for some subset of configs, but can't be relied upon until
we complete our implementation (WIP).  CC me on any bugs you find for
your configs.  I'm also trying to keep a handle on things in
https://github.com/ClangBuiltLinux/linux/issues.

>
> >
> > I don't think minimal supported versions are required for these clean
> > ups, and would not block these patches from landing on that.
> >
> > Also, haven't found anyone using ICC yet to comment on minimal version
> > requirements.
>
> For clang, by the way, __naked should go out of -gcc.h.

Yep, Arnd's note in the other thread was a valuable insight and I agree with it.

> I guess that
> is breaking ARM clang builds at the moment (didn't check)?

Huh?


> I will
> include the move for v3.
>
> Cheers,
> Miguel



-- 
Thanks,
~Nick Desaulniers

Re: [PATCH 1/7] Compiler Attributes: remove unused attributes

[Arnd Bergmann]

On Fri, Aug 31, 2018 at 11:27 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
> On Fri, Aug 31, 2018 at 1:23 PM Miguel Ojeda
> <miguel.ojeda.sandonis@gmail.com> wrote:
> > On Fri, Aug 31, 2018 at 8:43 PM, Nick Desaulniers
> > > I don't think minimal supported versions are required for these clean
> > > ups, and would not block these patches from landing on that.
> > >
> > > Also, haven't found anyone using ICC yet to comment on minimal version
> > > requirements.
> >
> > For clang, by the way, __naked should go out of -gcc.h.
>
> Yep, Arnd's note in the other thread was a valuable insight and I agree with it.
>
> > I guess that
> > is breaking ARM clang builds at the moment (didn't check)?
>
> Huh?

32-bit ARM has a number of problems with clang at the moment,
one of them is the lack of a __naked definition, which causes it
fail building if any of these files are enabled:

arch/arm/mach-exynos/mcpm-exynos.c:static void __naked
exynos_pm_power_up_setup(unsigned int affinity_level)
arch/arm/mach-vexpress/tc2_pm.c:static void __naked
tc2_pm_power_up_setup(unsigned int affinity_level)
arch/arm/mm/copypage-fa.c:static void __naked
arch/arm/mm/copypage-feroceon.c:static void __naked
arch/arm/mm/copypage-v4mc.c:static void __naked
arch/arm/mm/copypage-v4wb.c:static void __naked
arch/arm/mm/copypage-v4wt.c:static void __naked
arch/arm/mm/copypage-xsc3.c:static void __naked
arch/arm/mm/copypage-xscale.c:static void __naked

There is a related problem in all the copypage files: even if we add __naked,
clang refuses to compile them because it is more restrictive than gcc
about enforcing the __naked function rules. I'll send a patch for
that soon.

     Arnd

Re: [PATCH 1/7] Compiler Attributes: remove unused attributes

[Miguel Ojeda]

On Sun, Sep 2, 2018 at 9:54 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Fri, Aug 31, 2018 at 11:27 PM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
>> On Fri, Aug 31, 2018 at 1:23 PM Miguel Ojeda
>>
>> > I guess that
>> > is breaking ARM clang builds at the moment (didn't check)?
>>
>> Huh?
>
> 32-bit ARM has a number of problems with clang at the moment,
> one of them is the lack of a __naked definition, which causes it
> fail building if any of these files are enabled:

+1 This is what I was referring to.

Cheers,
Miguel

Re: [PATCH 1/7] Compiler Attributes: remove unused attributes

[Nick Desaulniers]

On Fri, Aug 31, 2018 at 10:05 AM Miguel Ojeda
<miguel.ojeda.sandonis@gmail.com> wrote:
>
> __optimize and __deprecate_for_modules are unused in
> the whole kernel tree. Simply drop them.

Indeed.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

>
> Cc: Eli Friedman <efriedma@codeaurora.org>
> Cc: Christopher Li <sparse@chrisli.org>
> Cc: Kees Cook <keescook@chromium.org>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Geert Uytterhoeven <geert@linux-m68k.org>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> Cc: Joe Perches <joe@perches.com>
> Cc: Dominique Martinet <asmadeus@codewreck.org>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
> ---
>  include/linux/compiler-gcc.h   | 2 --
>  include/linux/compiler.h       | 4 ----
>  include/linux/compiler_types.h | 1 -
>  3 files changed, 7 deletions(-)
>
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index 763bbad1e258..0a2d06677d83 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -95,8 +95,6 @@
>
>  #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
>
> -#define __optimize(level)      __attribute__((__optimize__(level)))
> -
>  #define __compiletime_object_size(obj) __builtin_object_size(obj, 0)
>
>  #ifndef __CHECKER__
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index 681d866efb1e..7c0157d50964 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -301,10 +301,6 @@ static inline void *offset_to_ptr(const int *off)
>
>  #endif /* __ASSEMBLY__ */
>
> -#ifndef __optimize
> -# define __optimize(level)
> -#endif
> -
>  /* Compile time object size, -1 for unknown */
>  #ifndef __compiletime_object_size
>  # define __compiletime_object_size(obj) -1
> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> index 3525c179698c..b6534292ea33 100644
> --- a/include/linux/compiler_types.h
> +++ b/include/linux/compiler_types.h
> @@ -108,7 +108,6 @@ struct ftrace_likely_data {
>
>  /* Don't. Just don't. */
>  #define __deprecated
> -#define __deprecated_for_modules
>
>  #endif /* __KERNEL__ */
>
> --
> 2.17.1
>


-- 
Thanks,
~Nick Desaulniers