* [PATCH] ubsan, kcsan: don't combine sanitizer with kcov
@ 2020-05-05 14:23 Arnd Bergmann
2020-05-05 14:36 ` Marco Elver
2020-05-22 16:08 ` [tip: locking/kcsan] ubsan, kcsan: Don't " tip-bot2 for Arnd Bergmann
0 siblings, 2 replies; 11+ messages in thread
From: Arnd Bergmann @ 2020-05-05 14:23 UTC (permalink / raw)
To: Andrey Ryabinin, Kees Cook, Andrey Konovalov, Marco Elver
Cc: Arnd Bergmann, Dmitry Vyukov, Paul E. McKenney, Ingo Molnar,
Andrew Morton, Greg Kroah-Hartman, Stephen Rothwell,
Thomas Gleixner, kasan-dev, linux-kernel, clang-built-linux
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 <arnd@arndb.de>
---
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
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)
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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] ubsan, kcsan: don't combine sanitizer with kcov
2020-05-05 14:23 [PATCH] ubsan, kcsan: don't combine sanitizer with kcov Arnd Bergmann
@ 2020-05-05 14:36 ` Marco Elver
2020-05-05 14:50 ` Dmitry Vyukov
2020-05-22 16:08 ` [tip: locking/kcsan] ubsan, kcsan: Don't " tip-bot2 for Arnd Bergmann
1 sibling, 1 reply; 11+ messages in thread
From: Marco Elver @ 2020-05-05 14:36 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Andrey Ryabinin, Kees Cook, Andrey Konovalov, Dmitry Vyukov,
Paul E. McKenney, Ingo Molnar, Andrew Morton, Greg Kroah-Hartman,
Stephen Rothwell, Thomas Gleixner, kasan-dev, LKML,
clang-built-linux
On Tue, 5 May 2020 at 16:23, Arnd Bergmann <arnd@arndb.de> 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 <arnd@arndb.de>
> ---
> 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
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] ubsan, kcsan: don't combine sanitizer with kcov
2020-05-05 14:36 ` Marco Elver
@ 2020-05-05 14:50 ` Dmitry Vyukov
2020-05-05 14:59 ` Arnd Bergmann
0 siblings, 1 reply; 11+ messages in thread
From: Dmitry Vyukov @ 2020-05-05 14:50 UTC (permalink / raw)
To: Marco Elver
Cc: Arnd Bergmann, Andrey Ryabinin, Kees Cook, Andrey Konovalov,
Paul E. McKenney, Ingo Molnar, Andrew Morton, Greg Kroah-Hartman,
Stephen Rothwell, Thomas Gleixner, kasan-dev, LKML,
clang-built-linux
On Tue, May 5, 2020 at 4:36 PM Marco Elver <elver@google.com> 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 <arnd@arndb.de>
> > ---
> > 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
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] ubsan, kcsan: don't combine sanitizer with kcov
2020-05-05 14:50 ` Dmitry Vyukov
@ 2020-05-05 14:59 ` Arnd Bergmann
2020-05-05 15:19 ` Marco Elver
0 siblings, 1 reply; 11+ messages in thread
From: Arnd Bergmann @ 2020-05-05 14:59 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: Marco Elver, Andrey Ryabinin, Kees Cook, Andrey Konovalov,
Paul E. McKenney, Ingo Molnar, Andrew Morton, Greg Kroah-Hartman,
Stephen Rothwell, Thomas Gleixner, kasan-dev, LKML,
clang-built-linux
On Tue, May 5, 2020 at 4:50 PM 'Dmitry Vyukov' via Clang Built Linux
<clang-built-linux@googlegroups.com> wrote:
> On Tue, May 5, 2020 at 4:36 PM Marco Elver <elver@google.com> 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
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] ubsan, kcsan: don't combine sanitizer with kcov
2020-05-05 14:59 ` Arnd Bergmann
@ 2020-05-05 15:19 ` Marco Elver
2020-05-05 15:28 ` Arnd Bergmann
0 siblings, 1 reply; 11+ messages in thread
From: Marco Elver @ 2020-05-05 15:19 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Dmitry Vyukov, Andrey Ryabinin, Kees Cook, Andrey Konovalov,
Paul E. McKenney, Ingo Molnar, Andrew Morton, Greg Kroah-Hartman,
Stephen Rothwell, Thomas Gleixner, kasan-dev, LKML,
clang-built-linux
On Tue, 5 May 2020 at 16:59, Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, May 5, 2020 at 4:50 PM 'Dmitry Vyukov' via Clang Built Linux
> <clang-built-linux@googlegroups.com> wrote:
> > On Tue, May 5, 2020 at 4:36 PM Marco Elver <elver@google.com> 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
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] ubsan, kcsan: don't combine sanitizer with kcov
2020-05-05 15:19 ` Marco Elver
@ 2020-05-05 15:28 ` Arnd Bergmann
2020-05-05 17:07 ` Marco Elver
0 siblings, 1 reply; 11+ messages in thread
From: Arnd Bergmann @ 2020-05-05 15:28 UTC (permalink / raw)
To: Marco Elver
Cc: Dmitry Vyukov, Andrey Ryabinin, Kees Cook, Andrey Konovalov,
Paul E. McKenney, Ingo Molnar, Andrew Morton, Greg Kroah-Hartman,
Stephen Rothwell, Thomas Gleixner, kasan-dev, LKML,
clang-built-linux
On Tue, May 5, 2020 at 5:20 PM 'Marco Elver' via Clang Built Linux
<clang-built-linux@googlegroups.com> 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
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] ubsan, kcsan: don't combine sanitizer with kcov
2020-05-05 15:28 ` Arnd Bergmann
@ 2020-05-05 17:07 ` Marco Elver
2020-05-07 16:25 ` [PATCH] [v2] ubsan, kcsan: don't combine sanitizer with kcov on clang Arnd Bergmann
0 siblings, 1 reply; 11+ messages in thread
From: Marco Elver @ 2020-05-05 17:07 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Dmitry Vyukov, Andrey Ryabinin, Kees Cook, Andrey Konovalov,
Paul E. McKenney, Ingo Molnar, Andrew Morton, Greg Kroah-Hartman,
Stephen Rothwell, Thomas Gleixner, kasan-dev, LKML,
clang-built-linux
On Tue, 5 May 2020 at 17:29, Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Tue, May 5, 2020 at 5:20 PM 'Marco Elver' via Clang Built Linux
> <clang-built-linux@googlegroups.com> 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
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] [v2] ubsan, kcsan: don't combine sanitizer with kcov on clang
2020-05-05 17:07 ` Marco Elver
@ 2020-05-07 16:25 ` Arnd Bergmann
2020-05-07 16:50 ` Marco Elver
2020-05-13 20:02 ` Paul E. McKenney
0 siblings, 2 replies; 11+ messages in thread
From: Arnd Bergmann @ 2020-05-07 16:25 UTC (permalink / raw)
To: Marco Elver
Cc: Arnd Bergmann, Dmitry Vyukov, Paul E. McKenney, Ingo Molnar,
Kees Cook, Andrew Morton, Thomas Gleixner, kasan-dev,
linux-kernel, clang-built-linux
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/20200505142341.1096942-1-arnd@arndb.de
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] [v2] ubsan, kcsan: don't combine sanitizer with kcov on clang
2020-05-07 16:25 ` [PATCH] [v2] ubsan, kcsan: don't combine sanitizer with kcov on clang Arnd Bergmann
@ 2020-05-07 16:50 ` Marco Elver
2020-05-13 20:02 ` Paul E. McKenney
1 sibling, 0 replies; 11+ messages in thread
From: Marco Elver @ 2020-05-07 16:50 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Dmitry Vyukov, Paul E. McKenney, Ingo Molnar, Kees Cook,
Andrew Morton, Thomas Gleixner, kasan-dev, LKML,
clang-built-linux, Alexander Potapenko
On Thu, 7 May 2020 at 18:26, Arnd Bergmann <arnd@arndb.de> 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/20200505142341.1096942-1-arnd@arndb.de
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> 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 <elver@google.com>
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
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] [v2] ubsan, kcsan: don't combine sanitizer with kcov on clang
2020-05-07 16:25 ` [PATCH] [v2] ubsan, kcsan: don't combine sanitizer with kcov on clang Arnd Bergmann
2020-05-07 16:50 ` Marco Elver
@ 2020-05-13 20:02 ` Paul E. McKenney
1 sibling, 0 replies; 11+ messages in thread
From: Paul E. McKenney @ 2020-05-13 20:02 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Marco Elver, Dmitry Vyukov, Ingo Molnar, Kees Cook,
Andrew Morton, Thomas Gleixner, kasan-dev, linux-kernel,
clang-built-linux
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/20200505142341.1096942-1-arnd@arndb.de
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
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
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [tip: locking/kcsan] ubsan, kcsan: Don't combine sanitizer with kcov on clang
2020-05-05 14:23 [PATCH] ubsan, kcsan: don't combine sanitizer with kcov Arnd Bergmann
2020-05-05 14:36 ` Marco Elver
@ 2020-05-22 16:08 ` tip-bot2 for Arnd Bergmann
1 sibling, 0 replies; 11+ messages in thread
From: tip-bot2 for Arnd Bergmann @ 2020-05-22 16:08 UTC (permalink / raw)
To: linux-tip-commits
Cc: Arnd Bergmann, Marco Elver, Borislav Petkov,
Peter Zijlstra (Intel),
x86, LKML
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 <arnd@arndb.de>
AuthorDate: Thu, 21 May 2020 16:20:37 +02:00
Committer: Borislav Petkov <bp@suse.de>
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 <arnd@arndb.de>
Signed-off-by: Marco Elver <elver@google.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Acked-by: Marco Elver <elver@google.com>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://bugs.llvm.org/show_bug.cgi?id=45831
Link: https://lore.kernel.org/lkml/20200505142341.1096942-1-arnd@arndb.de
Link: https://lkml.kernel.org/r/20200521142047.169334-2-elver@google.com
---
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.
^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2020-05-22 16:08 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-05 14:23 [PATCH] ubsan, kcsan: don't combine sanitizer with kcov Arnd Bergmann
2020-05-05 14:36 ` Marco Elver
2020-05-05 14:50 ` Dmitry Vyukov
2020-05-05 14:59 ` Arnd Bergmann
2020-05-05 15:19 ` Marco Elver
2020-05-05 15:28 ` Arnd Bergmann
2020-05-05 17:07 ` Marco Elver
2020-05-07 16:25 ` [PATCH] [v2] ubsan, kcsan: don't combine sanitizer with kcov on clang Arnd Bergmann
2020-05-07 16:50 ` Marco Elver
2020-05-13 20:02 ` Paul E. McKenney
2020-05-22 16:08 ` [tip: locking/kcsan] ubsan, kcsan: Don't " tip-bot2 for Arnd Bergmann
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).