Date:   Tue, 28 Aug 2018 09:53:03 -0700
From:   Nick Desaulniers <ndesaulniers@...gle.com>
To:     miguel.ojeda.sandonis@...il.com
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        efriedma@...eaurora.org, sparse@...isli.org,
        Kees Cook <keescook@...omium.org>,
        Ingo Molnar <mingo@...nel.org>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Arnd Bergmann <arnd@...db.de>,
        Greg KH <gregkh@...uxfoundation.org>,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        joe@...ches.com, asmadeus@...ewreck.org,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] include/linux/compiler*.h: Use feature checking instead
 of version checks for attributes

On Tue, Aug 28, 2018 at 8:04 AM Miguel Ojeda
<miguel.ojeda.sandonis@...il.com> wrote:
>
> Hi Nick,
>
> On Mon, Aug 27, 2018 at 7:43 PM, Nick Desaulniers
> <ndesaulniers@...gle.com> wrote:
> > On Sun, Aug 26, 2018 at 10:58 AM Miguel Ojeda
> > <miguel.ojeda.sandonis@...il.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. The attributes have been sorted
> >> and divided between "required" and "optional".
> >
> > Nice! Thanks Miguel.  Regarding sorting, I'm happy with that.  In
> > fact, some of the comments can be removed IMO, as the attributes have
> > common definitions in the docs (maybe an added link to the gcc and
> > clang attribute docs at the top of the file rather than per attribute
> > comments).
>
> Thanks for the review!
>
> I thought about that, although there isn't a single page with them in
> GCC (we could group them by type though: function ones, variable
> ones... and then link to those).
> On the other hand, maybe writing a
> Doc/ file is better and allows us to write as much as one would like
> about each of them (and a link to each page compiler's page about it,
> etc.). I think in the end the Doc/ file might be the best, in order
> not to crowd the header.

A comment is closer to the source, but I guess that's bytes for each
inclusion for every file.  I don't feel passionate about this point
one way or the other.

>
> >
> >>
> >> Further, attributes that are already supported in gcc >= 4.6 and recent clang
> >> were simply made to be required (instead of testing for them):
> >>   * always_inline
> >>   * const (pure was already "required", by the way)
> >>   * gnu_inline
> >
> > There's an important test for gnu_inline that isn't checking that it's
> > supported, but rather what the implicit behavior is depending on which
> > C standard is being used.  It's important not to remove that.
>
> Hm... I actually thought it was not available at some point before 4.6
> and removed the #ifdef. The comment even says it is featuring
> detecting it so that the old GCC inlining is used; but it shouldn't
> matter if you always use it, no?

Good point.  Rather than defining it only if GNU inline is not the
current behavior is a bit more verbose than just always defining it.
This seems to confirm that that should work:
https://godbolt.org/z/igwh32.

>
> I just went looking for more info in d03db2bc2 ("compiler-gcc.h: Add
> __attribute__((gnu_inline)) to all inline declarations") and if I
> understood the commit message, the problem is compiling with implicit
> new standard in newer compilers which trigger the C90 behavior, while
> we need the old one --- but if we use gnu_inline, we are getting it
> regardless.
>
> I am sure I am missing something, but I think a clarification is
> needed (and in the code comment as well) -- a bit off-topic, anyway.
>
> [Also, I wouldn't define an attribute or not depending on some other
> condition. I would, instead, define something some other symbol with
> that logic (i.e. instead of using "__gnu_inline", because that is
> lying -- it is not using the attribute even if the compiler supports
> it).]
>
> >
> >>
> >> Finally, some other bits were cleaned up in the process:
> >>   * __optimize: removed (unused in the whole kernel tree)
> >
> > A+ for removing dead code.  I also don't see it used anywhere.
> >
> >>   * __must_be_array: removed from -gcc and -clang (identical), moved to _types
> >
> > Yep, uses a builtin (we should add guards for that, later, in a
> > similar style change that guards the use of builtins). Looks good.
> >
> >>     (it depends on the unconditionally used  __builtin_types_compatible_p
> >>   * Removes unneeded underscores on the attributes' names
> >
> > That doesn't sound right, but lets see what you mean by that.
>
> Some attributes used the __name__ syntax (i.e. inside the double
> parenthesis), others didn't. I simplified by removing all the extra
> underscores.

A+

>
> >
> >>
> >> There are some things that can be further cleaned up afterwards:
> >>   * __attribute_const__: rename to __const
> >
> > This doesn't look correct to me; the kernel is full of call sites for
> > __attribute_const__. You can't rename the definition without renaming
>
> Of course it is full of use sites! That is why I said it is a possible
> cleanup for *afterwards* this patch :-)
>
> > all of the call sites (and that would be too big a change for this
> > patch, IMO).  Skip the rename, and it also looks like you just removed
> > it outright (Oops).
>
> Not sure what you mean by this (?). The attribute is still there unchanged.

Nevermind, I misinterpretered this part of the patch.

>
> >
> >>   * __noretpoline: avoid checking for defined(__notrepoline)
> >>   * __compiletime_warning/error: they are in two different places,
> >>     -gcc and compiler.h.
> >>   * sparse' attributes could potentially go into the end of attributes.h
> >>     too (as another separate section).
> >>
> >> Compile-tested an x86 allmodconfig for a while with gcc 8.2.0 and 4.6.4.
> >
> > It's important to test changes to compiler-clang.h with clang. ;)
>
> I would agree if the clang build wasn't broken to begin with. ;)
>
> >
> >>
> >> Cc: Eli Friedman <efriedma@...eaurora.org>
> >> Cc: Christopher Li <sparse@...isli.org>
> >> Cc: Kees Cook <keescook@...omium.org>
> >> Cc: Ingo Molnar <mingo@...nel.org>
> >> Cc: Geert Uytterhoeven <geert@...ux-m68k.org>
> >> Cc: Arnd Bergmann <arnd@...db.de>
> >> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> >> Cc: Masahiro Yamada <yamada.masahiro@...ionext.com>
> >> Cc: Joe Perches <joe@...ches.com>
> >> Cc: Dominique Martinet <asmadeus@...ewreck.org>
> >> Cc: Nick Desaulniers <ndesaulniers@...gle.com>
> >> Cc: Linus Torvalds <torvalds@...ux-foundation.org>
> >> Signed-off-by: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
> >> ---
> >> *Seems* to work, but note that I did not finish the entire allmodconfig :)
> >>
> >> A few things could be splitted into their own patch, but I kept it
> >> as one for simplicity for a first review.
> >>
> >> These files are becoming no-headaches-readable again, yay.
> >
> > A+
> >
> >>
> >>  include/linux/compiler-clang.h      |   5 --
> >>  include/linux/compiler-gcc.h        |  60 ----------------
> >>  include/linux/compiler-intel.h      |   6 --
> >>  include/linux/compiler.h            |   4 --
> >>  include/linux/compiler_attributes.h | 105 ++++++++++++++++++++++++++++
> >>  include/linux/compiler_types.h      |  91 ++++--------------------
> >>  6 files changed, 118 insertions(+), 153 deletions(-)
> >>  create mode 100644 include/linux/compiler_attributes.h
> >>
> >> diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
> >> index b1ce500fe8b3..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,6 +39,3 @@
> >>   * 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 763bbad1e258..dde3daae6287 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
> >> @@ -95,8 +88,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__
> >> @@ -133,9 +124,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))
> >> @@ -144,32 +132,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 && !defined(__CHECKER__)
> >> -/*
> >> - * __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:
> >>   *
> >> @@ -201,32 +163,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 4c7f9befa9f6..fb9e77fc65ec 100644
> >> --- a/include/linux/compiler-intel.h
> >> +++ b/include/linux/compiler-intel.h
> >> @@ -37,9 +37,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.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_attributes.h b/include/linux/compiler_attributes.h
> >> new file mode 100644
> >> index 000000000000..af8c8413d136
> >> --- /dev/null
> >> +++ b/include/linux/compiler_attributes.h
> >> @@ -0,0 +1,105 @@
> >> +#ifndef __LINUX_COMPILER_ATTRIBUTES_H
> >> +#define __LINUX_COMPILER_ATTRIBUTES_H
> >> +
> >> +/* This file is meant to be sorted. */
> >> +
> >> +/*
> >> + * Required attributes: your compiler must support these.
> >> + */
> >> +#define __alias(symbol)                __attribute__((alias(#symbol)))
> >> +#define __aligned(x)           __attribute__((aligned(x)))
> >> +#define __aligned_largest      __attribute__((aligned))
> >> +#define __always_inline         inline __attribute__((always_inline))
> >> +#define __always_unused                __attribute__((unused))
> >> +#define __attribute_const__     __attribute__((const))
> >> +#define __cold                 __attribute__((cold))
> >> +#define __gnu_inline            __attribute__((gnu_inline))
> >> +#define __malloc               __attribute__((malloc))
> >> +#define __maybe_unused         __attribute__((unused))
> >> +#define __mode(x)              __attribute__((mode(x)))
> >> +#define   noinline              __attribute__((noinline))
> >> +#define __noreturn             __attribute__((noreturn))
> >> +#define __packed               __attribute__((packed))
> >> +#define __printf(a, b)         __attribute__((format(printf, a, b)))
> >> +#define __pure                 __attribute__((pure))
> >> +#define __scanf(a, b)          __attribute__((format(scanf, a, b)))
> >> +#define __section(S)           __attribute__((section(#S)))
> >> +#define __used                 __attribute__((used))
> >> +#define __weak                 __attribute__((weak))
> >> +
> >> +/*
> >> + * Optional attributes: your compiler may or may not support them.
> >> + *
> >> + * To check for them, 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 it by hand.
> >> + */
> >> +#ifndef __has_attribute
> >> +#define __has_attribute(x) __GCC46_has_attribute_##x
> >> +#define __GCC46_has_attribute_assume_aligned 0
> >> +#define __GCC46_has_attribute_designated_init 0
> >> +#define __GCC46_has_attribute_externally_visible 1
> >> +#define __GCC46_has_attribute_noclone 1
> >> +#define __GCC46_has_attribute_optimize 1
> >> +#define __GCC46_has_attribute_no_sanitize_address 0
> >> +#endif
> >> +
> >> +/*
> >> + * __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;').
> >> + */
> >> +#if __has_attribute(assume_aligned)
> >> +#define __assume_aligned(a, ...) __attribute__((assume_aligned(a, ## __VA_ARGS__)))
> >> +#else
> >> +#define __assume_aligned(a, ...)
> >> +#endif
> >> +
> >> +/*
> >> + * Mark structures as requiring designated initializers.
> >> + * https://gcc.gnu.org/onlinedocs/gcc/Designated-Inits.html
> >> + */
> >> +#if __has_attribute(designated_init)
> >> +#define __designated_init __attribute__((designated_init))
> >> +#else
> >> +#define __designated_init
> >> +#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.
> >> + */
> >> +#if __has_attribute(externally_visible)
> >> +#define __visible __attribute__((externally_visible))
> >> +#else
> >> +#define __visible
> >> +#endif
> >> +
> >> +/* Mark a function definition as prohibited from being cloned. */
> >> +#if __has_attribute(noclone) && __has_attribute(optimize)
> >> +#define __noclone __attribute__((noclone, optimize("no-tracer")))
> >> +#else
> >> +#define __noclone
> >> +#endif
> >> +
> >> +/*
> >> + * 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
> >> + */
> >> +#if __has_attribute(no_sanitize_address)
> >> +#define __no_sanitize_address __attribute__((no_sanitize_address))
> >> +#else
> >> +#define __no_sanitize_address
> >> +#endif
> >> +
> >> +#endif /* __LINUX_COMPILER_ATTRIBUTES_H */
> >> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> >> index 3525c179698c..8cd622bedec4 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;
> >> @@ -119,10 +116,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
> >> @@ -140,17 +133,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))
> >>
> >> @@ -159,14 +141,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
> >> -
> >>  /* Helpers for emitting diagnostics in pragmas. */
> >>  #ifndef __diag
> >>  #define __diag(string)
> >> @@ -186,34 +160,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 __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
> >> @@ -228,18 +174,6 @@ 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
> >> -
> >>  /*
> >>   * Force always-inline if the user requests it so via the .config.
> >>   * GCC does not warn about unused static inline functions for
> >> @@ -254,19 +188,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 inline __attribute__((always_inline, unused)) notrace __gnu_inline
> >>  #else
> >> -#define inline inline  __attribute__((unused)) notrace __gnu_inline
> >> +#define inline inline __attribute__((unused)) notrace __gnu_inline
> >>  #endif
> >>
> >>  #define __inline__ inline
> >> -#define __inline inline
> >> -#define noinline       __attribute__((noinline))
> >> -
> >> -#ifndef __always_inline
> >> -#define __always_inline inline __attribute__((always_inline))
> >> -#endif
> >> +#define __inline   inline
> >
> > All of the changes to inline should not be removed, see above.  It's
> > important to make this work correctly regardless of C standard used.
> >
>
> See above.
>
> >>
> >>  /*
> >>   * Rather then using noinline to prevent stack consumption, use
> >> @@ -274,4 +202,11 @@ struct ftrace_likely_data {
> >>   */
> >>  #define noinline_for_stack noinline
> >>
> >> +#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_TYPES_H */
> >> --
> >> 2.17.1
> >>
> >
> > With the above changes requested, I'm super happy with the spirit of
> > this patch, and look forward to a v2.  Thanks again Miguel!
>
> Thanks again for the very thorough review!

Thanks for the patch! I'm almost ready to sign off, just a few more
comments on the other thread.

-- 
Thanks,
~Nick Desaulniers

Powered by blists - more mailing lists