From: Nathan Chancellor <nathan@kernel.org>
To: Marco Elver <elver@google.com>,
Andrew Morton <akpm@linux-foundation.org>
Cc: linux-kernel@vger.kernel.org, ndesaulniers@google.com,
ojeda@kernel.org, keescook@chromium.org, peterz@infradead.org,
will@kernel.org, nivedita@alum.mit.edu,
luc.vanoostenryck@gmail.com, masahiroy@kernel.org,
samitolvanen@google.com, arnd@arndb.de,
clang-built-linux@googlegroups.com,
Dmitry Vyukov <dvyukov@google.com>,
Mark Rutland <mark.rutland@arm.com>,
kasan-dev@googlegroups.com
Subject: Re: [PATCH v3] kcov: add __no_sanitize_coverage to fix noinstr for all architectures
Date: Thu, 27 May 2021 12:55:00 -0700 [thread overview]
Message-ID: <be3971b1-cf26-36c7-0f9c-d79c656ec855@kernel.org> (raw)
In-Reply-To: <20210527194448.3470080-1-elver@google.com>
On 5/27/2021 12:44 PM, Marco Elver wrote:
> Until now no compiler supported an attribute to disable coverage
> instrumentation as used by KCOV.
>
> To work around this limitation on x86, noinstr functions have their
> coverage instrumentation turned into nops by objtool. However, this
> solution doesn't scale automatically to other architectures, such as
> arm64, which are migrating to use the generic entry code.
>
> Clang [1] and GCC [2] have added support for the attribute recently.
> [1] https://github.com/llvm/llvm-project/commit/280333021e9550d80f5c1152a34e33e81df1e178
> [2] https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=cec4d4a6782c9bd8d071839c50a239c49caca689
> The changes will appear in Clang 13 and GCC 12.
>
> Add __no_sanitize_coverage for both compilers, and add it to noinstr.
>
> Note: In the Clang case, __has_feature(coverage_sanitizer) is only true
> if the feature is enabled, and therefore we do not require an additional
> defined(CONFIG_KCOV) (like in the GCC case where __has_attribute(..) is
> always true) to avoid adding redundant attributes to functions if KCOV
> is off. That being said, compilers that support the attribute will not
> generate errors/warnings if the attribute is redundantly used; however,
> where possible let's avoid it as it reduces preprocessed code size and
> associated compile-time overheads.
>
> Signed-off-by: Marco Elver <elver@google.com>
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Reviewed-by: Miguel Ojeda <ojeda@kernel.org>
Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> ---
> v3:
> * Add comment explaining __has_feature() in Clang.
> * Add Miguel's Reviewed-by.
>
> v2:
> * Implement __has_feature(coverage_sanitizer) in Clang
> (https://reviews.llvm.org/D103159) and use instead of version check.
> * Add Peter's Ack.
> ---
> include/linux/compiler-clang.h | 17 +++++++++++++++++
> include/linux/compiler-gcc.h | 6 ++++++
> include/linux/compiler_types.h | 2 +-
> 3 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h
> index adbe76b203e2..49b0ac8b6fd3 100644
> --- a/include/linux/compiler-clang.h
> +++ b/include/linux/compiler-clang.h
> @@ -13,6 +13,12 @@
> /* all clang versions usable with the kernel support KASAN ABI version 5 */
> #define KASAN_ABI_VERSION 5
>
> +/*
> + * Note: Checking __has_feature(*_sanitizer) is only true if the feature is
> + * enabled. Therefore it is not required to additionally check defined(CONFIG_*)
> + * to avoid adding redundant attributes in other configurations.
> + */
> +
> #if __has_feature(address_sanitizer) || __has_feature(hwaddress_sanitizer)
> /* Emulate GCC's __SANITIZE_ADDRESS__ flag */
> #define __SANITIZE_ADDRESS__
> @@ -45,6 +51,17 @@
> #define __no_sanitize_undefined
> #endif
>
> +/*
> + * Support for __has_feature(coverage_sanitizer) was added in Clang 13 together
> + * with no_sanitize("coverage"). Prior versions of Clang support coverage
> + * instrumentation, but cannot be queried for support by the preprocessor.
> + */
> +#if __has_feature(coverage_sanitizer)
> +#define __no_sanitize_coverage __attribute__((no_sanitize("coverage")))
> +#else
> +#define __no_sanitize_coverage
> +#endif
> +
> /*
> * Not all versions of clang implement the type-generic versions
> * of the builtin overflow checkers. Fortunately, clang implements
> diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h
> index 5d97ef738a57..cb9217fc60af 100644
> --- a/include/linux/compiler-gcc.h
> +++ b/include/linux/compiler-gcc.h
> @@ -122,6 +122,12 @@
> #define __no_sanitize_undefined
> #endif
>
> +#if defined(CONFIG_KCOV) && __has_attribute(__no_sanitize_coverage__)
> +#define __no_sanitize_coverage __attribute__((no_sanitize_coverage))
> +#else
> +#define __no_sanitize_coverage
> +#endif
> +
> #if GCC_VERSION >= 50100
> #define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1
> #endif
> diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h
> index d29bda7f6ebd..cc2bee7f0977 100644
> --- a/include/linux/compiler_types.h
> +++ b/include/linux/compiler_types.h
> @@ -210,7 +210,7 @@ struct ftrace_likely_data {
> /* Section for code which can't be instrumented at all */
> #define noinstr \
> noinline notrace __attribute((__section__(".noinstr.text"))) \
> - __no_kcsan __no_sanitize_address
> + __no_kcsan __no_sanitize_address __no_sanitize_coverage
>
> #endif /* __KERNEL__ */
>
>
prev parent reply other threads:[~2021-05-27 19:55 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-05-27 19:44 [PATCH v3] kcov: add __no_sanitize_coverage to fix noinstr for all architectures Marco Elver
2021-05-27 19:55 ` Nathan Chancellor [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=be3971b1-cf26-36c7-0f9c-d79c656ec855@kernel.org \
--to=nathan@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=arnd@arndb.de \
--cc=clang-built-linux@googlegroups.com \
--cc=dvyukov@google.com \
--cc=elver@google.com \
--cc=kasan-dev@googlegroups.com \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luc.vanoostenryck@gmail.com \
--cc=mark.rutland@arm.com \
--cc=masahiroy@kernel.org \
--cc=ndesaulniers@google.com \
--cc=nivedita@alum.mit.edu \
--cc=ojeda@kernel.org \
--cc=peterz@infradead.org \
--cc=samitolvanen@google.com \
--cc=will@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.