Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20180831170514.24665-1-miguel.ojeda.sandonis@gmail.com> <20180831170514.24665-6-miguel.ojeda.sandonis@gmail.com>
In-Reply-To: <20180831170514.24665-6-miguel.ojeda.sandonis@gmail.com>
From:   Nick Desaulniers <ndesaulniers@google.com>
Date:   Fri, 31 Aug 2018 14:38:26 -0700
Message-ID: <CAKwvOdkZeFj1QW8QBDSCet2Kv1jRka0DK_eSJwNmfG1Zqn2zcg@mail.gmail.com>
Subject: Re: [PATCH 6/7] Compiler Attributes: remove unneeded sparse
 (__CHECKER__) tests
To:     Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>, sparse@chrisli.org
Cc:     Linus Torvalds <torvalds@linux-foundation.org>,
        LKML <linux-kernel@vger.kernel.org>, efriedma@codeaurora.org,
        Kees Cook <keescook@chromium.org>,
        Ingo Molnar <mingo@kernel.org>,
        Geert Uytterhoeven <geert@linux-m68k.org>,
        Arnd Bergmann <arnd@arndb.de>,
        Greg KH <gregkh@linuxfoundation.org>,
        Masahiro Yamada <yamada.masahiro@socionext.com>,
        joe@perches.com, asmadeus@codewreck.org
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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