linux-kernel - Re: [PATCH] include/linux/compiler*.h: make compiler-*.h mutually exclusive

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 22 Aug 2018 17:20:26 -0700
From:   Joe Perches <joe@...ches.com>
To:     Nick Desaulniers <ndesaulniers@...gle.com>,
        torvalds@...ux-foundation.org
Cc:     keescook@...omium.org, corbet@....net,
        yamada.masahiro@...ionext.com, arnd@...db.de, dwmw@...zon.co.uk,
        linux-kernel@...r.kernel.org, tglx@...utronix.de,
        will.deacon@....com, geert@...ux-m68k.org, mingo@...nel.org,
        akpm@...ux-foundation.org, asmadeus@...ewreck.org,
        daniel@...earbox.net, hpa@...or.com
Subject: Re: [PATCH] include/linux/compiler*.h: make compiler-*.h mutually
 exclusive

On Wed, 2018-08-22 at 16:37 -0700, Nick Desaulniers wrote:
> Commit cafa0010cd51 ("Raise the minimum required gcc version to 4.6")
> recently exposed a brittle part of the build for supporting non-gcc
> compilers.

style trivia:

> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
[]
> @@ -54,32 +54,20 @@ extern void __chk_io_ptr(const volatile void __iomem *);
[]
> +/* Compiler specific macros. */
>  #ifdef __clang__
>  #include <linux/compiler-clang.h>

probably better as

#if defined(__clang)

to match the style of the #elif defined()s below it

> +#elif defined(__INTEL_COMPILER)
> +#include <linux/compiler-intel.h>
> +#elif defined(__GNUC__)
[]
> @@ -272,4 +174,92 @@ struct ftrace_likely_data {
[]
> +#ifdef __GNUC_STDC_INLINE__
> +# define __gnu_inline	__attribute__((gnu_inline))
> +#else
> +# define __gnu_inline
> +#endif

Perhaps __gnu_inline should be in compiler-gcc and this
should use

#ifndef __gnu_inline
#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
> + * -Wunused-function.  This turns out to avoid the need for complex #ifdef
> + * directives.  Suppress the warning in clang as well by using "unused"
> + * function attribute, which is redundant but not harmful for gcc.
> + * Prefer gnu_inline, so that extern inline functions do not emit an
> + * externally visible function. This makes extern inline behave as per gnu89
> + * semantics rather than c99. This prevents multiple symbol definition errors
> + * of extern inline functions at link time.
> + * A lot of inline functions can cause havoc with function tracing.
> + */
> +#if !defined(CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING) || \
> +	!defined(CONFIG_OPTIMIZE_INLINING)
> +#define inline \
> +	inline __attribute__((always_inline, unused)) notrace __gnu_inline
> +#else
> +#define inline inline	__attribute__((unused)) notrace __gnu_inline
> +#endif

This bit might be better adding another __<foo> attribute like:

#if defined(CONFIG_ARCH_SUPPORTS_OPTIMIZED_INLINING) &&
    defined(CONFIG_OPTIMIZE_INLINING)
#define __optimized_inline __unused
#else
#define __optimized_inline __unused __attribute__((always_inline))
#endif

#define inline inline __optimized_inline notrace __gnu_inline

> +
> +#define __inline__ inline
> +#define __inline inline
> +#define noinline	__attribute__((noinline))
> +
> +#ifndef __always_inline
> +#define __always_inline inline __attribute__((always_inline))
> +#endif

[]

> diff --git a/mm/migrate.c b/mm/migrate.c
[]
> @@ -1131,7 +1131,8 @@ static int __unmap_and_move(struct page *page, struct page *newpage,
>   * gcc 4.7 and 4.8 on arm get an ICEs when inlining unmap_and_move().  Work
>   * around it.
>   */
> -#if (GCC_VERSION >= 40700 && GCC_VERSION < 40900) && defined(CONFIG_ARM)
> +#if defined(CONFIG_ARM) && \
> +	defined(GCC_VERSION) && GCC_VERSION < 40900 && GCC_VERSION >= 40700

I find the reversed version tests a bit odd to read

>  #define ICE_noinline noinline
>  #else
>  #define ICE_noinline