LinuxLists.cc - [PATCH] ubsan, kcsan: don't combine sanitizer with kcov

2020-05-05 14:26:43

Subject: [PATCH] ubsan, kcsan: don't combine sanitizer with kcov

2020-05-05 14:39:55

Subject: Re: [PATCH] ubsan, kcsan: don't combine sanitizer with kcov

On Tue, 5 May 2020 at 16:23, Arnd Bergmann <[email protected]> wrote:
>
> Clang does not allow -fsanitize-coverage=trace-{pc,cmp} together
> with -fsanitize=bounds or with ubsan:
>
> clang: error: argument unused during compilation: '-fsanitize-coverage=trace-pc' [-Werror,-Wunused-command-line-argument]
> clang: error: argument unused during compilation: '-fsanitize-coverage=trace-cmp' [-Werror,-Wunused-command-line-argument]
>
> To avoid that case, add a Kconfig dependency. The dependency could
> go either way, disabling CONFIG_KCOV or CONFIG_UBSAN_BOUNDS when the
> other is set. I picked the second option here as this seems to have
> a smaller impact on the resulting kernel.
>
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> lib/Kconfig.kcsan | 2 +-
> lib/Kconfig.ubsan | 1 +
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/Kconfig.kcsan b/lib/Kconfig.kcsan
> index ea28245c6c1d..8f856c8828d5 100644
> --- a/lib/Kconfig.kcsan
> +++ b/lib/Kconfig.kcsan
> @@ -5,7 +5,7 @@ config HAVE_ARCH_KCSAN
>
> menuconfig KCSAN
> bool "KCSAN: dynamic data race detector"
> - depends on HAVE_ARCH_KCSAN && DEBUG_KERNEL && !KASAN
> + depends on HAVE_ARCH_KCSAN && DEBUG_KERNEL && !KASAN && !KCOV

This also disables KCOV with GCC. Why does this not work with KCSAN?

This is a huge problem for us, since syzbot requires KCOV. In fact
I've always been building KCSAN kernels with CONFIG_KCOV=y (with GCC
or Clang) and cannot reproduce the problem.

> select STACKTRACE
> help
> The Kernel Concurrency Sanitizer (KCSAN) is a dynamic
> diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan
> index 929211039bac..f98ef029553e 100644
> --- a/lib/Kconfig.ubsan
> +++ b/lib/Kconfig.ubsan
> @@ -29,6 +29,7 @@ config UBSAN_TRAP
> config UBSAN_BOUNDS
> bool "Perform array index bounds checking"
> default UBSAN
> + depends on !(CC_IS_CLANG && KCOV)

Ditto, we really need KCOV for all sanitizers. I also just tried to
reproduce the problem but can't.

Which version of clang is causing this? I'm currently using Clang 9.
My guess is that we should not fix this by disallowing KCOV, but
rather make Clang work with these configs.

Dmitry, can you comment?

Thanks,
-- Marco

> help
> This option enables detection of directly indexed out of bounds
> array accesses, where the array size is known at compile time.
> --
> 2.26.0
>

2020-05-05 14:52:42

by Dmitry Vyukov

[permalink] [raw]

Subject: Re: [PATCH] ubsan, kcsan: don't combine sanitizer with kcov

On Tue, May 5, 2020 at 4:36 PM Marco Elver <[email protected]> wrote:
> > Clang does not allow -fsanitize-coverage=trace-{pc,cmp} together
> > with -fsanitize=bounds or with ubsan:
> >
> > clang: error: argument unused during compilation: '-fsanitize-coverage=trace-pc' [-Werror,-Wunused-command-line-argument]
> > clang: error: argument unused during compilation: '-fsanitize-coverage=trace-cmp' [-Werror,-Wunused-command-line-argument]
> >
> > To avoid that case, add a Kconfig dependency. The dependency could
> > go either way, disabling CONFIG_KCOV or CONFIG_UBSAN_BOUNDS when the
> > other is set. I picked the second option here as this seems to have
> > a smaller impact on the resulting kernel.
> >
> > Signed-off-by: Arnd Bergmann <[email protected]>
> > ---
> > lib/Kconfig.kcsan | 2 +-
> > lib/Kconfig.ubsan | 1 +
> > 2 files changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/Kconfig.kcsan b/lib/Kconfig.kcsan
> > index ea28245c6c1d..8f856c8828d5 100644
> > --- a/lib/Kconfig.kcsan
> > +++ b/lib/Kconfig.kcsan
> > @@ -5,7 +5,7 @@ config HAVE_ARCH_KCSAN
> >
> > menuconfig KCSAN
> > bool "KCSAN: dynamic data race detector"
> > - depends on HAVE_ARCH_KCSAN && DEBUG_KERNEL && !KASAN
> > + depends on HAVE_ARCH_KCSAN && DEBUG_KERNEL && !KASAN && !KCOV
>
> This also disables KCOV with GCC. Why does this not work with KCSAN?
>
> This is a huge problem for us, since syzbot requires KCOV. In fact
> I've always been building KCSAN kernels with CONFIG_KCOV=y (with GCC
> or Clang) and cannot reproduce the problem.
>
> > select STACKTRACE
> > help
> > The Kernel Concurrency Sanitizer (KCSAN) is a dynamic
> > diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan
> > index 929211039bac..f98ef029553e 100644
> > --- a/lib/Kconfig.ubsan
> > +++ b/lib/Kconfig.ubsan
> > @@ -29,6 +29,7 @@ config UBSAN_TRAP
> > config UBSAN_BOUNDS
> > bool "Perform array index bounds checking"
> > default UBSAN
> > + depends on !(CC_IS_CLANG && KCOV)
>
> Ditto, we really need KCOV for all sanitizers. I also just tried to
> reproduce the problem but can't.
>
> Which version of clang is causing this? I'm currently using Clang 9.
> My guess is that we should not fix this by disallowing KCOV, but
> rather make Clang work with these configs.
>
> Dmitry, can you comment?

FWIW I can reproduce both with clang:

$ clang /tmp/test.c -c -fsanitize-coverage=trace-pc -fsanitize=bounds
clang-11: warning: argument unused during compilation:
'-fsanitize-coverage=trace-pc' [-Wunused-command-line-argument]

$ clang /tmp/test.c -c -fsanitize-coverage=trace-pc -fsanitize=thread
clang-11: warning: argument unused during compilation:
'-fsanitize-coverage=trace-pc' [-Wunused-command-line-argument]

with both my disto's 9.0.1 and fresher 11.0.0
(7b80cb7cf45faf462d6193cc41c2cb7ad556600d.

But both work with gcc

$ gcc /tmp/test.c -c -fsanitize-coverage=trace-pc -fsanitize=thread
$ gcc /tmp/test.c -c -fsanitize-coverage=trace-pc -fsanitize=bounds

Is it a known issue in clang?

Can we somehow disable it only for clang and not gcc?

This will immediately break KCSAN on syzbot as it enables KCSAN and KCOV:
https://syzkaller.appspot.com/upstream?manager=ci2-upstream-kcsan-gce

2020-05-05 15:01:34

by Arnd Bergmann

[permalink] [raw]

Subject: Re: [PATCH] ubsan, kcsan: don't combine sanitizer with kcov

On Tue, May 5, 2020 at 4:50 PM 'Dmitry Vyukov' via Clang Built Linux
<[email protected]> wrote:
> On Tue, May 5, 2020 at 4:36 PM Marco Elver <[email protected]> wrote:
> > > Clang does not allow -fsanitize-coverage=trace-{pc,cmp} together
> > > with -fsanitize=bounds or with ubsan:
> > >
> > > clang: error: argument unused during compilation: '-fsanitize-coverage=trace-pc' [-Werror,-Wunused-command-line-argument]
> > > clang: error: argument unused during compilation: '-fsanitize-coverage=trace-cmp' [-Werror,-Wunused-command-line-argument]
> > >
> > > menuconfig KCSAN
> > > bool "KCSAN: dynamic data race detector"
> > > - depends on HAVE_ARCH_KCSAN && DEBUG_KERNEL && !KASAN
> > > + depends on HAVE_ARCH_KCSAN && DEBUG_KERNEL && !KASAN && !KCOV
> >
> > This also disables KCOV with GCC. Why does this not work with KCSAN?

My mistake, this should be kept enabled for gcc. If we can get the combination
to work in clang, that's something that should also get enabled.

> > This is a huge problem for us, since syzbot requires KCOV. In fact
> > I've always been building KCSAN kernels with CONFIG_KCOV=y (with GCC
> > or Clang) and cannot reproduce the problem.

I have some local patches that change the way we pick the warning options
for each compiler, and enable more of the warnings that are normally disabled.

Maybe -Wunused-command-line-argument is disabled by default?
I only started seeing this problem recently. It's also possible that there
are some other options that interact with it so only Kcov+FOO leads to
KCSAN being ignored.

> > Ditto, we really need KCOV for all sanitizers. I also just tried to
> > reproduce the problem but can't.
> >
> > Which version of clang is causing this? I'm currently using Clang 9.
> > My guess is that we should not fix this by disallowing KCOV, but
> > rather make Clang work with these configs.
> >
> > Dmitry, can you comment?
>
> FWIW I can reproduce both with clang:
>
> $ clang /tmp/test.c -c -fsanitize-coverage=trace-pc -fsanitize=bounds
> clang-11: warning: argument unused during compilation:
> '-fsanitize-coverage=trace-pc' [-Wunused-command-line-argument]
>
> $ clang /tmp/test.c -c -fsanitize-coverage=trace-pc -fsanitize=thread
> clang-11: warning: argument unused during compilation:
> '-fsanitize-coverage=trace-pc' [-Wunused-command-line-argument]
>
> with both my disto's 9.0.1 and fresher 11.0.0
> (7b80cb7cf45faf462d6193cc41c2cb7ad556600d.
>
> But both work with gcc
>
> $ gcc /tmp/test.c -c -fsanitize-coverage=trace-pc -fsanitize=thread
> $ gcc /tmp/test.c -c -fsanitize-coverage=trace-pc -fsanitize=bounds
>
> Is it a known issue in clang?
>
> Can we somehow disable it only for clang and not gcc?
>
> This will immediately break KCSAN on syzbot as it enables KCSAN and KCOV:
> https://syzkaller.appspot.com/upstream?manager=ci2-upstream-kcsan-gce

I can respin the patch with this fixup if you like:

--- a/lib/Kconfig.kcsan
+++ b/lib/Kconfig.kcsan
@@ -5,7 +5,7 @@ config HAVE_ARCH_KCSAN

menuconfig KCSAN
bool "KCSAN: dynamic data race detector"
- depends on HAVE_ARCH_KCSAN && DEBUG_KERNEL && !KASAN && !KCOV
+ depends on HAVE_ARCH_KCSAN && DEBUG_KERNEL && !KASAN && !(KCOV
&& CC_IS_CLANG)
select STACKTRACE
help
The Kernel Concurrency Sanitizer (KCSAN) is a dynamic

As you both say, the combination seems to be quite important, so maybe there
is something else that can be to also enable it with clang.

Arnd

2020-05-05 15:22:37

by Marco Elver

[permalink] [raw]

Subject: Re: [PATCH] ubsan, kcsan: don't combine sanitizer with kcov

On Tue, 5 May 2020 at 16:59, Arnd Bergmann <[email protected]> wrote:
>
> On Tue, May 5, 2020 at 4:50 PM 'Dmitry Vyukov' via Clang Built Linux
> <[email protected]> wrote:
> > On Tue, May 5, 2020 at 4:36 PM Marco Elver <[email protected]> wrote:
> > > > Clang does not allow -fsanitize-coverage=trace-{pc,cmp} together
> > > > with -fsanitize=bounds or with ubsan:
> > > >
> > > > clang: error: argument unused during compilation: '-fsanitize-coverage=trace-pc' [-Werror,-Wunused-command-line-argument]
> > > > clang: error: argument unused during compilation: '-fsanitize-coverage=trace-cmp' [-Werror,-Wunused-command-line-argument]
> > > >
> > > > menuconfig KCSAN
> > > > bool "KCSAN: dynamic data race detector"
> > > > - depends on HAVE_ARCH_KCSAN && DEBUG_KERNEL && !KASAN
> > > > + depends on HAVE_ARCH_KCSAN && DEBUG_KERNEL && !KASAN && !KCOV
> > >
> > > This also disables KCOV with GCC. Why does this not work with KCSAN?
>
> My mistake, this should be kept enabled for gcc. If we can get the combination
> to work in clang, that's something that should also get enabled.

See my suggestion below how we might dynamically determine if the
combination is supported.

> > > This is a huge problem for us, since syzbot requires KCOV. In fact
> > > I've always been building KCSAN kernels with CONFIG_KCOV=y (with GCC
> > > or Clang) and cannot reproduce the problem.
>
> I have some local patches that change the way we pick the warning options
> for each compiler, and enable more of the warnings that are normally disabled.
>
> Maybe -Wunused-command-line-argument is disabled by default?
> I only started seeing this problem recently. It's also possible that there
> are some other options that interact with it so only Kcov+FOO leads to
> KCSAN being ignored.

I see. It certainly seems quite bad if one or the other option is
effectively ignored.

> > > Ditto, we really need KCOV for all sanitizers. I also just tried to
> > > reproduce the problem but can't.
> > >
> > > Which version of clang is causing this? I'm currently using Clang 9.
> > > My guess is that we should not fix this by disallowing KCOV, but
> > > rather make Clang work with these configs.
> > >
> > > Dmitry, can you comment?
> >
> > FWIW I can reproduce both with clang:
> >
> > $ clang /tmp/test.c -c -fsanitize-coverage=trace-pc -fsanitize=bounds
> > clang-11: warning: argument unused during compilation:
> > '-fsanitize-coverage=trace-pc' [-Wunused-command-line-argument]
> >
> > $ clang /tmp/test.c -c -fsanitize-coverage=trace-pc -fsanitize=thread
> > clang-11: warning: argument unused during compilation:
> > '-fsanitize-coverage=trace-pc' [-Wunused-command-line-argument]
> >
> > with both my disto's 9.0.1 and fresher 11.0.0
> > (7b80cb7cf45faf462d6193cc41c2cb7ad556600d.
> >
> > But both work with gcc
> >
> > $ gcc /tmp/test.c -c -fsanitize-coverage=trace-pc -fsanitize=thread
> > $ gcc /tmp/test.c -c -fsanitize-coverage=trace-pc -fsanitize=bounds
> >
> > Is it a known issue in clang?
> >
> > Can we somehow disable it only for clang and not gcc?
> >
> > This will immediately break KCSAN on syzbot as it enables KCSAN and KCOV:
> > https://syzkaller.appspot.com/upstream?manager=ci2-upstream-kcsan-gce
>
> I can respin the patch with this fixup if you like:
>
> --- a/lib/Kconfig.kcsan
> +++ b/lib/Kconfig.kcsan
> @@ -5,7 +5,7 @@ config HAVE_ARCH_KCSAN
>
> menuconfig KCSAN
> bool "KCSAN: dynamic data race detector"
> - depends on HAVE_ARCH_KCSAN && DEBUG_KERNEL && !KASAN && !KCOV
> + depends on HAVE_ARCH_KCSAN && DEBUG_KERNEL && !KASAN && !(KCOV
> && CC_IS_CLANG)

I wonder if we can just add this: depends on !(KCOV &&
!$(cc-option,-Werror -fsanitize=thread -fsanitize-coverage=trace-pc))

Similarly for UBSAN.

That way, once Clang supports this combination, we don't need another
patch to fix it.

Thanks,
-- Marco

> select STACKTRACE
> help
> The Kernel Concurrency Sanitizer (KCSAN) is a dynamic
>
> As you both say, the combination seems to be quite important, so maybe there
> is something else that can be to also enable it with clang.
>
> Arnd

2020-05-05 15:31:24

by Arnd Bergmann

[permalink] [raw]

Subject: Re: [PATCH] ubsan, kcsan: don't combine sanitizer with kcov

On Tue, May 5, 2020 at 5:20 PM 'Marco Elver' via Clang Built Linux
<[email protected]> wrote:

> > --- a/lib/Kconfig.kcsan
> > +++ b/lib/Kconfig.kcsan
> > @@ -5,7 +5,7 @@ config HAVE_ARCH_KCSAN
> >
> > menuconfig KCSAN
> > bool "KCSAN: dynamic data race detector"
> > - depends on HAVE_ARCH_KCSAN && DEBUG_KERNEL && !KASAN && !KCOV
> > + depends on HAVE_ARCH_KCSAN && DEBUG_KERNEL && !KASAN && !(KCOV
> > && CC_IS_CLANG)
>
> I wonder if we can just add this: depends on !(KCOV &&
> !$(cc-option,-Werror -fsanitize=thread -fsanitize-coverage=trace-pc))
>
> Similarly for UBSAN.
>
> That way, once Clang supports this combination, we don't need another
> patch to fix it.

Good idea. It probably get a little more complicated because kcov uses
different flags depending on other options:

kcov-flags-$(CONFIG_CC_HAS_SANCOV_TRACE_PC) += -fsanitize-coverage=trace-pc
kcov-flags-$(CONFIG_KCOV_ENABLE_COMPARISONS) += -fsanitize-coverage=trace-cmp
kcov-flags-$(CONFIG_GCC_PLUGIN_SANCOV) +=
-fplugin=$(objtree)/scripts/gcc-plugins/sancov_plugin.so

Do you have any preference on whether we should make KCSAN or KCOV
conditional in this case? It may be easier to move the compiletime check
into CONFIG_KCOV_ENABLE_COMPARISONS and
CONFIG_CC_HAS_SANCOV_TRACE_PC.

Arnd

2020-05-05 17:09:49

by Marco Elver

[permalink] [raw]

Subject: Re: [PATCH] ubsan, kcsan: don't combine sanitizer with kcov

On Tue, 5 May 2020 at 17:29, Arnd Bergmann <[email protected]> wrote:
>
> On Tue, May 5, 2020 at 5:20 PM 'Marco Elver' via Clang Built Linux
> <[email protected]> wrote:
>
> > > --- a/lib/Kconfig.kcsan
> > > +++ b/lib/Kconfig.kcsan
> > > @@ -5,7 +5,7 @@ config HAVE_ARCH_KCSAN
> > >
> > > menuconfig KCSAN
> > > bool "KCSAN: dynamic data race detector"
> > > - depends on HAVE_ARCH_KCSAN && DEBUG_KERNEL && !KASAN && !KCOV
> > > + depends on HAVE_ARCH_KCSAN && DEBUG_KERNEL && !KASAN && !(KCOV
> > > && CC_IS_CLANG)
> >
> > I wonder if we can just add this: depends on !(KCOV &&
> > !$(cc-option,-Werror -fsanitize=thread -fsanitize-coverage=trace-pc))
> >
> > Similarly for UBSAN.
> >
> > That way, once Clang supports this combination, we don't need another
> > patch to fix it.
>
> Good idea. It probably get a little more complicated because kcov uses
> different flags depending on other options:
>
> kcov-flags-$(CONFIG_CC_HAS_SANCOV_TRACE_PC) += -fsanitize-coverage=trace-pc
> kcov-flags-$(CONFIG_KCOV_ENABLE_COMPARISONS) += -fsanitize-coverage=trace-cmp
> kcov-flags-$(CONFIG_GCC_PLUGIN_SANCOV) +=
> -fplugin=$(objtree)/scripts/gcc-plugins/sancov_plugin.so
>
> Do you have any preference on whether we should make KCSAN or KCOV
> conditional in this case? It may be easier to move the compiletime check
> into CONFIG_KCOV_ENABLE_COMPARISONS and
> CONFIG_CC_HAS_SANCOV_TRACE_PC.

Whichever is easier. I think if we have a config that tries to set
both, but then one gets silently disabled, it likely already breaks
the usecase. It'd be nice if there was a way to warn about only one
being selected so that a developer can then go back and choose the one
they're most interested in (or change compiler).

Thanks,
-- Marco

2020-05-07 16:30:44

by Arnd Bergmann

[permalink] [raw]

Subject: [PATCH] [v2] ubsan, kcsan: don't combine sanitizer with kcov on clang

Clang does not allow -fsanitize-coverage=trace-{pc,cmp} together
with -fsanitize=bounds or with ubsan:

clang: error: argument unused during compilation: '-fsanitize-coverage=trace-pc' [-Werror,-Wunused-command-line-argument]
clang: error: argument unused during compilation: '-fsanitize-coverage=trace-cmp' [-Werror,-Wunused-command-line-argument]

To avoid the warning, check whether clang can handle this correctly
or disallow ubsan and kcsan when kcov is enabled.

Link: https://bugs.llvm.org/show_bug.cgi?id=45831
Link: https://lore.kernel.org/lkml/[email protected]
Signed-off-by: Arnd Bergmann <[email protected]>
---
v2: this implements Marco's suggestion to check what the compiler
actually supports, and references the bug report I now opened.

Let's wait for replies on that bug report before this gets applied,
in case the feedback there changes the conclusion.
---
lib/Kconfig.kcsan | 11 +++++++++++
lib/Kconfig.ubsan | 11 +++++++++++
2 files changed, 22 insertions(+)

diff --git a/lib/Kconfig.kcsan b/lib/Kconfig.kcsan
index ea28245c6c1d..a7276035ca0d 100644
--- a/lib/Kconfig.kcsan
+++ b/lib/Kconfig.kcsan
@@ -3,9 +3,20 @@
config HAVE_ARCH_KCSAN
bool

+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 && DEBUG_KERNEL && !KASAN
+ depends on !KCSAN_KCOV_BROKEN
select STACKTRACE
help
The Kernel Concurrency Sanitizer (KCSAN) is a dynamic
diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan
index 929211039bac..a5ba2fd51823 100644
--- a/lib/Kconfig.ubsan
+++ b/lib/Kconfig.ubsan
@@ -26,9 +26,20 @@ config UBSAN_TRAP
the system. For some system builders this is an acceptable
trade-off.

+config UBSAN_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=bounds -fsanitize-coverage=trace-pc)
+ help
+ Some versions of clang support either UBSAN or KCOV but not the
+ combination of the two.
+ See https://bugs.llvm.org/show_bug.cgi?id=45831 for the status
+ in newer releases.
+
config UBSAN_BOUNDS
bool "Perform array index bounds checking"
default UBSAN
+ depends on !UBSAN_KCOV_BROKEN
help
This option enables detection of directly indexed out of bounds
array accesses, where the array size is known at compile time.
--
2.26.0

2020-05-07 16:54:58

by Marco Elver

[permalink] [raw]

Subject: Re: [PATCH] [v2] ubsan, kcsan: don't combine sanitizer with kcov on clang

On Thu, 7 May 2020 at 18:26, Arnd Bergmann <[email protected]> wrote:
>
> Clang does not allow -fsanitize-coverage=trace-{pc,cmp} together
> with -fsanitize=bounds or with ubsan:
>
> clang: error: argument unused during compilation: '-fsanitize-coverage=trace-pc' [-Werror,-Wunused-command-line-argument]
> clang: error: argument unused during compilation: '-fsanitize-coverage=trace-cmp' [-Werror,-Wunused-command-line-argument]
>
> To avoid the warning, check whether clang can handle this correctly
> or disallow ubsan and kcsan when kcov is enabled.
>
> Link: https://bugs.llvm.org/show_bug.cgi?id=45831
> Link: https://lore.kernel.org/lkml/[email protected]
> Signed-off-by: Arnd Bergmann <[email protected]>
> ---
> v2: this implements Marco's suggestion to check what the compiler
> actually supports, and references the bug report I now opened.
>
> Let's wait for replies on that bug report before this gets applied,
> in case the feedback there changes the conclusion.

Waiting makes sense, if this is not very urgent.

Acked-by: Marco Elver <[email protected]>

Thank you!

> ---
> lib/Kconfig.kcsan | 11 +++++++++++
> lib/Kconfig.ubsan | 11 +++++++++++
> 2 files changed, 22 insertions(+)
>
> diff --git a/lib/Kconfig.kcsan b/lib/Kconfig.kcsan
> index ea28245c6c1d..a7276035ca0d 100644
> --- a/lib/Kconfig.kcsan
> +++ b/lib/Kconfig.kcsan
> @@ -3,9 +3,20 @@
> config HAVE_ARCH_KCSAN
> bool
>
> +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 && DEBUG_KERNEL && !KASAN
> + depends on !KCSAN_KCOV_BROKEN
> select STACKTRACE
> help
> The Kernel Concurrency Sanitizer (KCSAN) is a dynamic
> diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan
> index 929211039bac..a5ba2fd51823 100644
> --- a/lib/Kconfig.ubsan
> +++ b/lib/Kconfig.ubsan
> @@ -26,9 +26,20 @@ config UBSAN_TRAP
> the system. For some system builders this is an acceptable
> trade-off.
>
> +config UBSAN_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=bounds -fsanitize-coverage=trace-pc)
> + help
> + Some versions of clang support either UBSAN or KCOV but not the
> + combination of the two.
> + See https://bugs.llvm.org/show_bug.cgi?id=45831 for the status
> + in newer releases.
> +
> config UBSAN_BOUNDS
> bool "Perform array index bounds checking"
> default UBSAN
> + depends on !UBSAN_KCOV_BROKEN
> help
> This option enables detection of directly indexed out of bounds
> array accesses, where the array size is known at compile time.
> --
> 2.26.0
>

2020-05-13 20:07:12

by Paul E. McKenney

[permalink] [raw]

Subject: Re: [PATCH] [v2] ubsan, kcsan: don't combine sanitizer with kcov on clang

On Thu, May 07, 2020 at 06:25:31PM +0200, Arnd Bergmann wrote:
> Clang does not allow -fsanitize-coverage=trace-{pc,cmp} together
> with -fsanitize=bounds or with ubsan:
>
> clang: error: argument unused during compilation: '-fsanitize-coverage=trace-pc' [-Werror,-Wunused-command-line-argument]
> clang: error: argument unused during compilation: '-fsanitize-coverage=trace-cmp' [-Werror,-Wunused-command-line-argument]
>
> To avoid the warning, check whether clang can handle this correctly
> or disallow ubsan and kcsan when kcov is enabled.
>
> Link: https://bugs.llvm.org/show_bug.cgi?id=45831
> Link: https://lore.kernel.org/lkml/[email protected]
> Signed-off-by: Arnd Bergmann <[email protected]>

Applied for v5.9 and pushed, thank you!

Thanx, Paul

> ---
> v2: this implements Marco's suggestion to check what the compiler
> actually supports, and references the bug report I now opened.
>
> Let's wait for replies on that bug report before this gets applied,
> in case the feedback there changes the conclusion.
> ---
> lib/Kconfig.kcsan | 11 +++++++++++
> lib/Kconfig.ubsan | 11 +++++++++++
> 2 files changed, 22 insertions(+)
>
> diff --git a/lib/Kconfig.kcsan b/lib/Kconfig.kcsan
> index ea28245c6c1d..a7276035ca0d 100644
> --- a/lib/Kconfig.kcsan
> +++ b/lib/Kconfig.kcsan
> @@ -3,9 +3,20 @@
> config HAVE_ARCH_KCSAN
> bool
>
> +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 && DEBUG_KERNEL && !KASAN
> + depends on !KCSAN_KCOV_BROKEN
> select STACKTRACE
> help
> The Kernel Concurrency Sanitizer (KCSAN) is a dynamic
> diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan
> index 929211039bac..a5ba2fd51823 100644
> --- a/lib/Kconfig.ubsan
> +++ b/lib/Kconfig.ubsan
> @@ -26,9 +26,20 @@ config UBSAN_TRAP
> the system. For some system builders this is an acceptable
> trade-off.
>
> +config UBSAN_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=bounds -fsanitize-coverage=trace-pc)
> + help
> + Some versions of clang support either UBSAN or KCOV but not the
> + combination of the two.
> + See https://bugs.llvm.org/show_bug.cgi?id=45831 for the status
> + in newer releases.
> +
> config UBSAN_BOUNDS
> bool "Perform array index bounds checking"
> default UBSAN
> + depends on !UBSAN_KCOV_BROKEN
> help
> This option enables detection of directly indexed out of bounds
> array accesses, where the array size is known at compile time.
> --
> 2.26.0
>

2020-05-22 16:10:53

by tip-bot2 for Alexey Makhalov

[permalink] [raw]

Subject: [tip: locking/kcsan] ubsan, kcsan: Don't combine sanitizer with kcov on clang

The following commit has been merged into the locking/kcsan branch of tip:

Commit-ID: e87c5783e9e43ec8bd5c0a1cf7cfa4add33603b0
Gitweb: https://git.kernel.org/tip/e87c5783e9e43ec8bd5c0a1cf7cfa4add33603b0
Author: Arnd Bergmann <[email protected]>
AuthorDate: Thu, 21 May 2020 16:20:37 +02:00
Committer: Borislav Petkov <[email protected]>
CommitterDate: Fri, 22 May 2020 14:34:26 +02:00

ubsan, kcsan: Don't combine sanitizer with kcov on clang

Clang does not allow -fsanitize-coverage=trace-{pc,cmp} together
with -fsanitize=bounds or with ubsan:

clang: error: argument unused during compilation: '-fsanitize-coverage=trace-pc' [-Werror,-Wunused-command-line-argument]
clang: error: argument unused during compilation: '-fsanitize-coverage=trace-cmp' [-Werror,-Wunused-command-line-argument]

To avoid the warning, check whether clang can handle this correctly or
disallow ubsan and kcsan when kcov is enabled.

Signed-off-by: Arnd Bergmann <[email protected]>
Signed-off-by: Marco Elver <[email protected]>
Signed-off-by: Borislav Petkov <[email protected]>
Acked-by: Marco Elver <[email protected]>
Acked-by: Peter Zijlstra (Intel) <[email protected]>
Link: https://bugs.llvm.org/show_bug.cgi?id=45831
Link: https://lore.kernel.org/lkml/[email protected]
Link: https://lkml.kernel.org/r/[email protected]
---
lib/Kconfig.kcsan | 11 +++++++++++
lib/Kconfig.ubsan | 11 +++++++++++
2 files changed, 22 insertions(+)

diff --git a/lib/Kconfig.kcsan b/lib/Kconfig.kcsan
index 689b6b8..b5d88ac 100644
--- a/lib/Kconfig.kcsan
+++ b/lib/Kconfig.kcsan
@@ -3,9 +3,20 @@
config HAVE_ARCH_KCSAN
bool

+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 && DEBUG_KERNEL && !KASAN
+ depends on !KCSAN_KCOV_BROKEN
select STACKTRACE
help
The Kernel Concurrency Sanitizer (KCSAN) is a dynamic
diff --git a/lib/Kconfig.ubsan b/lib/Kconfig.ubsan
index 48469c9..3baea77 100644
--- a/lib/Kconfig.ubsan
+++ b/lib/Kconfig.ubsan
@@ -26,9 +26,20 @@ config UBSAN_TRAP
the system. For some system builders this is an acceptable
trade-off.

+config UBSAN_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=bounds -fsanitize-coverage=trace-pc)
+ help
+ Some versions of clang support either UBSAN or KCOV but not the
+ combination of the two.
+ See https://bugs.llvm.org/show_bug.cgi?id=45831 for the status
+ in newer releases.
+
config UBSAN_BOUNDS
bool "Perform array index bounds checking"
default UBSAN
+ depends on !UBSAN_KCOV_BROKEN
help
This option enables detection of directly indexed out of bounds
array accesses, where the array size is known at compile time.