lib: Kconfig: disable dynamic sanitizers for test builds

  kasan, kcsan and kmsan all have the tendency to blow up the stack
and there isn't a lot of value in having them enabled for test builds,
since they are intended to be useful for runtime debugging. So, disable
them for test builds.

Signed-off-by: Hamza Mahfooz <hamza.mahfooz@amd.com>
---
 lib/Kconfig.kasan | 1 +
 lib/Kconfig.kcsan | 1 +
 lib/Kconfig.kmsan | 1 +
 3 files changed, 3 insertions(+)
  

On Wed, 18 Oct 2023 at 17:32, 'Hamza Mahfooz' via kasan-dev
<kasan-dev@googlegroups.com> wrote:
>
> kasan, kcsan and kmsan all have the tendency to blow up the stack
> and there isn't a lot of value in having them enabled for test builds,
> since they are intended to be useful for runtime debugging. So, disable
> them for test builds.
>
> Signed-off-by: Hamza Mahfooz <hamza.mahfooz@amd.com>
> ---
>  lib/Kconfig.kasan | 1 +
>  lib/Kconfig.kcsan | 1 +
>  lib/Kconfig.kmsan | 1 +
>  3 files changed, 3 insertions(+)

Do you have links to discussions that motivate this change? This has
been discussed in the past. One recommendation is to adjust the
build/test scripts to exclude some combination of configs if they are
causing issues. Or we increase CONFIG_FRAME_WARN if one of them is
enabled (KMSAN sets it to 0, 32-bit KASAN increases it a bit).

That being said, we're aware of KASAN having had more issues and there
are some suboptions that have been disabled because of that (like
KASAN_STACK). I'm not sure if Clang's KASAN instrumentation has had
some recent improvements (we did investigate it, but I can't recall
what the outcome was [1]) - maybe try a more recent compiler? However,
KCSAN and KMSAN shouldn't have any issues (if KMSAN is enabled,
FRAME_WARN is 0). And having build tests with them enabled isn't
useless at all: we're making sure that these tools (even though only
for debugging), still work. We _want_ them to work during random build
testing!

Please share the concrete problem you're having, because this change
will make things worse for everyone in the long run.

[1] https://github.com/llvm/llvm-project/issues/38157

> diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
> index fdca89c05745..fbd85c4872c0 100644
> --- a/lib/Kconfig.kasan
> +++ b/lib/Kconfig.kasan
> @@ -38,6 +38,7 @@ menuconfig KASAN
>                     CC_HAS_WORKING_NOSANITIZE_ADDRESS) || \
>                    HAVE_ARCH_KASAN_HW_TAGS
>         depends on (SLUB && SYSFS && !SLUB_TINY) || (SLAB && !DEBUG_SLAB)
> +       depends on !COMPILE_TEST
>         select STACKDEPOT_ALWAYS_INIT
>         help
>           Enables KASAN (Kernel Address Sanitizer) - a dynamic memory safety

This also disables KASAN_HW_TAGS, which is actually enabled in
production kernels and does not use any compiler instrumentation.

> diff --git a/lib/Kconfig.kcsan b/lib/Kconfig.kcsan
> index 609ddfc73de5..7bcefdbfb46f 100644
> --- a/lib/Kconfig.kcsan
> +++ b/lib/Kconfig.kcsan
> @@ -14,6 +14,7 @@ menuconfig KCSAN
>         bool "KCSAN: dynamic data race detector"
>         depends on HAVE_ARCH_KCSAN && HAVE_KCSAN_COMPILER
>         depends on DEBUG_KERNEL && !KASAN
> +       depends on !COMPILE_TEST
>         select CONSTRUCTORS
>         select STACKTRACE
>         help
> diff --git a/lib/Kconfig.kmsan b/lib/Kconfig.kmsan
> index ef2c8f256c57..eb05c885d3fd 100644
> --- a/lib/Kconfig.kmsan
> +++ b/lib/Kconfig.kmsan
> @@ -13,6 +13,7 @@ config KMSAN
>         depends on HAVE_ARCH_KMSAN && HAVE_KMSAN_COMPILER
>         depends on SLUB && DEBUG_KERNEL && !KASAN && !KCSAN
>         depends on !PREEMPT_RT
> +       depends on !COMPILE_TEST

KMSAN already selects FRAME_WARN of 0 and should not cause you any
issues during build testing.

Nack.
  

On Wed, Oct 18, 2023 at 9:22 AM Marco Elver <elver@google.com> wrote:
>
> That being said, we're aware of KASAN having had more issues and there
> are some suboptions that have been disabled because of that (like
> KASAN_STACK). I'm not sure if Clang's KASAN instrumentation has had
> some recent improvements (we did investigate it, but I can't recall
> what the outcome was [1]) - maybe try a more recent compiler? However,
> KCSAN and KMSAN shouldn't have any issues (if KMSAN is enabled,
> FRAME_WARN is 0). And having build tests with them enabled isn't
> useless at all: we're making sure that these tools (even though only
> for debugging), still work. We _want_ them to work during random build
> testing!
>
> Please share the concrete problem you're having, because this change
> will make things worse for everyone in the long run.
>
> [1] https://github.com/llvm/llvm-project/issues/38157

Some recent issues I discovered in clang that exacerbate stack usage
in general, which is then amplified by KASAN:
1. https://github.com/llvm/llvm-project/issues/68746
2. https://github.com/llvm/llvm-project/issues/68747
Those are next up on my to fix list after what I'm working on now.  I
suspect that those aren't the last issues now that I've found a thread
to pull on.
  

On 10/18/23 12:22, Marco Elver wrote:
> On Wed, 18 Oct 2023 at 17:32, 'Hamza Mahfooz' via kasan-dev
> <kasan-dev@googlegroups.com> wrote:
>>
>> kasan, kcsan and kmsan all have the tendency to blow up the stack
>> and there isn't a lot of value in having them enabled for test builds,
>> since they are intended to be useful for runtime debugging. So, disable
>> them for test builds.
>>
>> Signed-off-by: Hamza Mahfooz <hamza.mahfooz@amd.com>
>> ---
>>   lib/Kconfig.kasan | 1 +
>>   lib/Kconfig.kcsan | 1 +
>>   lib/Kconfig.kmsan | 1 +
>>   3 files changed, 3 insertions(+)
> 
> Do you have links to discussions that motivate this change? This has
> been discussed in the past. One recommendation is to adjust the

Sure, you can checkout:
https://lore.kernel.org/amd-gfx/CADnq5_OyO9CHqahFvdnx7-8s9654udgdfhUntyxfjae+iHey0Q@mail.gmail.com/T/#m5d227dc1ef07b1f4953312287dce4568666c5e09

> build/test scripts to exclude some combination of configs if they are
> causing issues. Or we increase CONFIG_FRAME_WARN if one of them is
> enabled (KMSAN sets it to 0, 32-bit KASAN increases it a bit).
> 
> That being said, we're aware of KASAN having had more issues and there
> are some suboptions that have been disabled because of that (like
> KASAN_STACK). I'm not sure if Clang's KASAN instrumentation has had
> some recent improvements (we did investigate it, but I can't recall
> what the outcome was [1]) - maybe try a more recent compiler? However,
> KCSAN and KMSAN shouldn't have any issues (if KMSAN is enabled,

This patch was initially motivated by KCSAN (i.e. I am able to get it to
blow up the stack with a minimal .config). I don't mind dropping the
other ones since I only included them because Nathan implied that they
could cause similar issues.

> FRAME_WARN is 0). And having build tests with them enabled isn't
> useless at all: we're making sure that these tools (even though only
> for debugging), still work. We _want_ them to work during random build
> testing!
> 
> Please share the concrete problem you're having, because this change
> will make things worse for everyone in the long run.
> 
> [1] https://github.com/llvm/llvm-project/issues/38157
> 
>> diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
>> index fdca89c05745..fbd85c4872c0 100644
>> --- a/lib/Kconfig.kasan
>> +++ b/lib/Kconfig.kasan
>> @@ -38,6 +38,7 @@ menuconfig KASAN
>>                      CC_HAS_WORKING_NOSANITIZE_ADDRESS) || \
>>                     HAVE_ARCH_KASAN_HW_TAGS
>>          depends on (SLUB && SYSFS && !SLUB_TINY) || (SLAB && !DEBUG_SLAB)
>> +       depends on !COMPILE_TEST
>>          select STACKDEPOT_ALWAYS_INIT
>>          help
>>            Enables KASAN (Kernel Address Sanitizer) - a dynamic memory safety
> 
> This also disables KASAN_HW_TAGS, which is actually enabled in
> production kernels and does not use any compiler instrumentation.
> 
>> diff --git a/lib/Kconfig.kcsan b/lib/Kconfig.kcsan
>> index 609ddfc73de5..7bcefdbfb46f 100644
>> --- a/lib/Kconfig.kcsan
>> +++ b/lib/Kconfig.kcsan
>> @@ -14,6 +14,7 @@ menuconfig KCSAN
>>          bool "KCSAN: dynamic data race detector"
>>          depends on HAVE_ARCH_KCSAN && HAVE_KCSAN_COMPILER
>>          depends on DEBUG_KERNEL && !KASAN
>> +       depends on !COMPILE_TEST
>>          select CONSTRUCTORS
>>          select STACKTRACE
>>          help
>> diff --git a/lib/Kconfig.kmsan b/lib/Kconfig.kmsan
>> index ef2c8f256c57..eb05c885d3fd 100644
>> --- a/lib/Kconfig.kmsan
>> +++ b/lib/Kconfig.kmsan
>> @@ -13,6 +13,7 @@ config KMSAN
>>          depends on HAVE_ARCH_KMSAN && HAVE_KMSAN_COMPILER
>>          depends on SLUB && DEBUG_KERNEL && !KASAN && !KCSAN
>>          depends on !PREEMPT_RT
>> +       depends on !COMPILE_TEST
> 
> KMSAN already selects FRAME_WARN of 0 and should not cause you any
> issues during build testing.
> 
> Nack.
  

On Wed, 18 Oct 2023 at 18:43, Hamza Mahfooz <hamza.mahfooz@amd.com> wrote:
>
> On 10/18/23 12:22, Marco Elver wrote:
> > On Wed, 18 Oct 2023 at 17:32, 'Hamza Mahfooz' via kasan-dev
> > <kasan-dev@googlegroups.com> wrote:
> >>
> >> kasan, kcsan and kmsan all have the tendency to blow up the stack
> >> and there isn't a lot of value in having them enabled for test builds,
> >> since they are intended to be useful for runtime debugging. So, disable
> >> them for test builds.
> >>
> >> Signed-off-by: Hamza Mahfooz <hamza.mahfooz@amd.com>
> >> ---
> >>   lib/Kconfig.kasan | 1 +
> >>   lib/Kconfig.kcsan | 1 +
> >>   lib/Kconfig.kmsan | 1 +
> >>   3 files changed, 3 insertions(+)
> >
> > Do you have links to discussions that motivate this change? This has
> > been discussed in the past. One recommendation is to adjust the
>
> Sure, you can checkout:
> https://lore.kernel.org/amd-gfx/CADnq5_OyO9CHqahFvdnx7-8s9654udgdfhUntyxfjae+iHey0Q@mail.gmail.com/T/#m5d227dc1ef07b1f4953312287dce4568666c5e09

I would add this as a Link context to the patch.

> > build/test scripts to exclude some combination of configs if they are
> > causing issues. Or we increase CONFIG_FRAME_WARN if one of them is
> > enabled (KMSAN sets it to 0, 32-bit KASAN increases it a bit).
> >
> > That being said, we're aware of KASAN having had more issues and there
> > are some suboptions that have been disabled because of that (like
> > KASAN_STACK). I'm not sure if Clang's KASAN instrumentation has had
> > some recent improvements (we did investigate it, but I can't recall
> > what the outcome was [1]) - maybe try a more recent compiler? However,
> > KCSAN and KMSAN shouldn't have any issues (if KMSAN is enabled,
>
> This patch was initially motivated by KCSAN (i.e. I am able to get it to
> blow up the stack with a minimal .config). I don't mind dropping the
> other ones since I only included them because Nathan implied that they
> could cause similar issues.

!COMPILE_TEST is not the solution. Clearly from the link you provided
build testing is helpful in catching early issues, so that these tools
remain usable for everyone. But we know they use a little more stack,
and the warnings need to be adjusted accordingly.

My suggestion is to just increase FRAME_WARN for KCSAN, or set it to 0
(like for KMSAN). My guess is that first trying to increase it is the
safer option.

> > FRAME_WARN is 0). And having build tests with them enabled isn't
> > useless at all: we're making sure that these tools (even though only
> > for debugging), still work. We _want_ them to work during random build
> > testing!
> >
> > Please share the concrete problem you're having, because this change
> > will make things worse for everyone in the long run.
> >
> > [1] https://github.com/llvm/llvm-project/issues/38157
> >
> >> diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
> >> index fdca89c05745..fbd85c4872c0 100644
> >> --- a/lib/Kconfig.kasan
> >> +++ b/lib/Kconfig.kasan
> >> @@ -38,6 +38,7 @@ menuconfig KASAN
> >>                      CC_HAS_WORKING_NOSANITIZE_ADDRESS) || \
> >>                     HAVE_ARCH_KASAN_HW_TAGS
> >>          depends on (SLUB && SYSFS && !SLUB_TINY) || (SLAB && !DEBUG_SLAB)
> >> +       depends on !COMPILE_TEST
> >>          select STACKDEPOT_ALWAYS_INIT
> >>          help
> >>            Enables KASAN (Kernel Address Sanitizer) - a dynamic memory safety
> >
> > This also disables KASAN_HW_TAGS, which is actually enabled in
> > production kernels and does not use any compiler instrumentation.
> >
> >> diff --git a/lib/Kconfig.kcsan b/lib/Kconfig.kcsan
> >> index 609ddfc73de5..7bcefdbfb46f 100644
> >> --- a/lib/Kconfig.kcsan
> >> +++ b/lib/Kconfig.kcsan
> >> @@ -14,6 +14,7 @@ menuconfig KCSAN
> >>          bool "KCSAN: dynamic data race detector"
> >>          depends on HAVE_ARCH_KCSAN && HAVE_KCSAN_COMPILER
> >>          depends on DEBUG_KERNEL && !KASAN
> >> +       depends on !COMPILE_TEST
> >>          select CONSTRUCTORS
> >>          select STACKTRACE
> >>          help
> >> diff --git a/lib/Kconfig.kmsan b/lib/Kconfig.kmsan
> >> index ef2c8f256c57..eb05c885d3fd 100644
> >> --- a/lib/Kconfig.kmsan
> >> +++ b/lib/Kconfig.kmsan
> >> @@ -13,6 +13,7 @@ config KMSAN
> >>          depends on HAVE_ARCH_KMSAN && HAVE_KMSAN_COMPILER
> >>          depends on SLUB && DEBUG_KERNEL && !KASAN && !KCSAN
> >>          depends on !PREEMPT_RT
> >> +       depends on !COMPILE_TEST
> >
> > KMSAN already selects FRAME_WARN of 0 and should not cause you any
> > issues during build testing.
> >
> > Nack.
> --
> Hamza
>
  

On Wed, Oct 18, 2023 at 06:22:14PM +0200, Marco Elver wrote:
> On Wed, 18 Oct 2023 at 17:32, 'Hamza Mahfooz' via kasan-dev
> <kasan-dev@googlegroups.com> wrote:

<snip>

> > diff --git a/lib/Kconfig.kmsan b/lib/Kconfig.kmsan
> > index ef2c8f256c57..eb05c885d3fd 100644
> > --- a/lib/Kconfig.kmsan
> > +++ b/lib/Kconfig.kmsan
> > @@ -13,6 +13,7 @@ config KMSAN
> >         depends on HAVE_ARCH_KMSAN && HAVE_KMSAN_COMPILER
> >         depends on SLUB && DEBUG_KERNEL && !KASAN && !KCSAN
> >         depends on !PREEMPT_RT
> > +       depends on !COMPILE_TEST
> 
> KMSAN already selects FRAME_WARN of 0 and should not cause you any
> issues during build testing.

Yeah, this particular case is a bug in the AMDGPU dml2 Makefile, where
CONFIG_FRAME_WARN=0 is not respected.

diff --git a/drivers/gpu/drm/amd/display/dc/dml2/Makefile b/drivers/gpu/drm/amd/display/dc/dml2/Makefile
index f35ed8de260d..66431525f2a0 100644
--- a/drivers/gpu/drm/amd/display/dc/dml2/Makefile
+++ b/drivers/gpu/drm/amd/display/dc/dml2/Makefile
@@ -61,7 +61,7 @@ ifneq ($(CONFIG_FRAME_WARN),0)
 frame_warn_flag := -Wframe-larger-than=2048
 endif
 
-CFLAGS_$(AMDDALPATH)/dc/dml2/display_mode_core.o := $(dml2_ccflags) -Wframe-larger-than=2048
+CFLAGS_$(AMDDALPATH)/dc/dml2/display_mode_core.o := $(dml2_ccflags) $(frame_warn_flag)
 CFLAGS_$(AMDDALPATH)/dc/dml2/display_mode_util.o := $(dml2_ccflags)
 CFLAGS_$(AMDDALPATH)/dc/dml2/dml2_wrapper.o := $(dml2_ccflags)
 CFLAGS_$(AMDDALPATH)/dc/dml2/dml2_utils.o := $(dml2_ccflags)

I will try to send that patch soon, unless one of the AMDGPU folks wants
to beat me to it.

Cheers,
Nathan