diff mbox series

[12/43] kcsan: clang: retire CONFIG_KCSAN_KCOV_BROKEN

Message ID 20211214162050.660953-13-glider@google.com (mailing list archive)
State New
Headers show
Series Add KernelMemorySanitizer infrastructure | expand

Commit Message

Alexander Potapenko Dec. 14, 2021, 4:20 p.m. UTC
kcov used to be broken prior to Clang 11, but right now that version is
already the minimum required to build with KCSAN, that is why we don't
need KCSAN_KCOV_BROKEN anymore.

Suggested-by: Marco Elver <elver@google.com>
Signed-off-by: Alexander Potapenko <glider@google.com>
---
Link: https://linux-review.googlesource.com/id/Ida287421577f37de337139b5b5b9e977e4a6fee2
---
 lib/Kconfig.kcsan | 11 -----------
 1 file changed, 11 deletions(-)

Comments

Mark Rutland Dec. 15, 2021, 1:33 p.m. UTC | #1
On Tue, Dec 14, 2021 at 05:20:19PM +0100, Alexander Potapenko wrote:
> kcov used to be broken prior to Clang 11, but right now that version is
> already the minimum required to build with KCSAN, that is why we don't
> need KCSAN_KCOV_BROKEN anymore.

Just to check, how is that requirement enforced?

I see the core Makefiles enforce 10.0.1+, but I couldn't spot an explicit
version dependency in Kconfig.kcsan.

Otherwise, this looks good to me!

Mark.

> Suggested-by: Marco Elver <elver@google.com>
> Signed-off-by: Alexander Potapenko <glider@google.com>
> ---
> Link: https://linux-review.googlesource.com/id/Ida287421577f37de337139b5b5b9e977e4a6fee2
> ---
>  lib/Kconfig.kcsan | 11 -----------
>  1 file changed, 11 deletions(-)
> 
> diff --git a/lib/Kconfig.kcsan b/lib/Kconfig.kcsan
> index e0a93ffdef30e..b81454b2a0d09 100644
> --- a/lib/Kconfig.kcsan
> +++ b/lib/Kconfig.kcsan
> @@ -10,21 +10,10 @@ config HAVE_KCSAN_COMPILER
>  	  For the list of compilers that support KCSAN, please see
>  	  <file:Documentation/dev-tools/kcsan.rst>.
>  
> -config KCSAN_KCOV_BROKEN
> -	def_bool KCOV && CC_HAS_SANCOV_TRACE_PC
> -	depends on CC_IS_CLANG
> -	depends on !$(cc-option,-Werror=unused-command-line-argument -fsanitize=thread -fsanitize-coverage=trace-pc)
> -	help
> -	  Some versions of clang support either KCSAN and KCOV but not the
> -	  combination of the two.
> -	  See https://bugs.llvm.org/show_bug.cgi?id=45831 for the status
> -	  in newer releases.
> -
>  menuconfig KCSAN
>  	bool "KCSAN: dynamic data race detector"
>  	depends on HAVE_ARCH_KCSAN && HAVE_KCSAN_COMPILER
>  	depends on DEBUG_KERNEL && !KASAN
> -	depends on !KCSAN_KCOV_BROKEN
>  	select STACKTRACE
>  	help
>  	  The Kernel Concurrency Sanitizer (KCSAN) is a dynamic
> -- 
> 2.34.1.173.g76aa8bc2d0-goog
>
Marco Elver Dec. 15, 2021, 1:39 p.m. UTC | #2
On Wed, 15 Dec 2021 at 14:33, Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Tue, Dec 14, 2021 at 05:20:19PM +0100, Alexander Potapenko wrote:
> > kcov used to be broken prior to Clang 11, but right now that version is
> > already the minimum required to build with KCSAN, that is why we don't
> > need KCSAN_KCOV_BROKEN anymore.
>
> Just to check, how is that requirement enforced?

HAVE_KCSAN_COMPILER will only be true with Clang 11 or later, due to
no prior compiler having "-tsan-distinguish-volatile=1".

> I see the core Makefiles enforce 10.0.1+, but I couldn't spot an explicit
> version dependency in Kconfig.kcsan.
>
> Otherwise, this looks good to me!

I think 5.17 will be Clang 11 only, so we could actually revert
ea91a1d45d19469001a4955583187b0d75915759:
https://lkml.kernel.org/r/Yao86FeC2ybOobLO@archlinux-ax161

I should resend that to be added to the -kbuild tree.
Mark Rutland Dec. 15, 2021, 2:43 p.m. UTC | #3
On Wed, Dec 15, 2021 at 02:39:43PM +0100, Marco Elver wrote:
> On Wed, 15 Dec 2021 at 14:33, Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > On Tue, Dec 14, 2021 at 05:20:19PM +0100, Alexander Potapenko wrote:
> > > kcov used to be broken prior to Clang 11, but right now that version is
> > > already the minimum required to build with KCSAN, that is why we don't
> > > need KCSAN_KCOV_BROKEN anymore.
> >
> > Just to check, how is that requirement enforced?
> 
> HAVE_KCSAN_COMPILER will only be true with Clang 11 or later, due to
> no prior compiler having "-tsan-distinguish-volatile=1".

I see -- could we add wording to that effect into the commit messge?

> > I see the core Makefiles enforce 10.0.1+, but I couldn't spot an explicit
> > version dependency in Kconfig.kcsan.
> >
> > Otherwise, this looks good to me!
> 
> I think 5.17 will be Clang 11 only, so we could actually revert
> ea91a1d45d19469001a4955583187b0d75915759:
> https://lkml.kernel.org/r/Yao86FeC2ybOobLO@archlinux-ax161
> 
> I should resend that to be added to the -kbuild tree.

FWIW, that also works for me.

Thanks,
Mark.
Alexander Potapenko March 18, 2022, 2:34 p.m. UTC | #4
On Wed, Dec 15, 2021 at 3:43 PM Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Wed, Dec 15, 2021 at 02:39:43PM +0100, Marco Elver wrote:
> > On Wed, 15 Dec 2021 at 14:33, Mark Rutland <mark.rutland@arm.com> wrote:
> > >
> > > On Tue, Dec 14, 2021 at 05:20:19PM +0100, Alexander Potapenko wrote:
> > > > kcov used to be broken prior to Clang 11, but right now that version is
> > > > already the minimum required to build with KCSAN, that is why we don't
> > > > need KCSAN_KCOV_BROKEN anymore.
> > >
> > > Just to check, how is that requirement enforced?
> >
> > HAVE_KCSAN_COMPILER will only be true with Clang 11 or later, due to
> > no prior compiler having "-tsan-distinguish-volatile=1".
>
> I see -- could we add wording to that effect into the commit messge?

Will be done.

> > > I see the core Makefiles enforce 10.0.1+, but I couldn't spot an explicit
> > > version dependency in Kconfig.kcsan.
> > >
> > > Otherwise, this looks good to me!
> >
> > I think 5.17 will be Clang 11 only, so we could actually revert
> > ea91a1d45d19469001a4955583187b0d75915759:
> > https://lkml.kernel.org/r/Yao86FeC2ybOobLO@archlinux-ax161
> >
> > I should resend that to be added to the -kbuild tree.
>
> FWIW, that also works for me.
>
> Thanks,
> Mark.
diff mbox series

Patch

diff --git a/lib/Kconfig.kcsan b/lib/Kconfig.kcsan
index e0a93ffdef30e..b81454b2a0d09 100644
--- a/lib/Kconfig.kcsan
+++ b/lib/Kconfig.kcsan
@@ -10,21 +10,10 @@  config HAVE_KCSAN_COMPILER
 	  For the list of compilers that support KCSAN, please see
 	  <file:Documentation/dev-tools/kcsan.rst>.
 
-config KCSAN_KCOV_BROKEN
-	def_bool KCOV && CC_HAS_SANCOV_TRACE_PC
-	depends on CC_IS_CLANG
-	depends on !$(cc-option,-Werror=unused-command-line-argument -fsanitize=thread -fsanitize-coverage=trace-pc)
-	help
-	  Some versions of clang support either KCSAN and KCOV but not the
-	  combination of the two.
-	  See https://bugs.llvm.org/show_bug.cgi?id=45831 for the status
-	  in newer releases.
-
 menuconfig KCSAN
 	bool "KCSAN: dynamic data race detector"
 	depends on HAVE_ARCH_KCSAN && HAVE_KCSAN_COMPILER
 	depends on DEBUG_KERNEL && !KASAN
-	depends on !KCSAN_KCOV_BROKEN
 	select STACKTRACE
 	help
 	  The Kernel Concurrency Sanitizer (KCSAN) is a dynamic