2015-04-08 15:38:41

by Andrey Ryabinin

[permalink] [raw]
Subject: [PATCH] kasan: Makefile: shut up warnings if CONFIG_COMPILE_TEST=y

It might be annoying to constantly see this:

scripts/Makefile.kasan:16: Cannot use CONFIG_KASAN: -fsanitize=kernel-address is not supported by compiler

while performing allmodconfig/allyesconfig build tests.
Disable this warning if CONFIG_COMPILE_TEST=y.

Signed-off-by: Andrey Ryabinin <[email protected]>
---
scripts/Makefile.kasan | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
index 631619b..3f874d2 100644
--- a/scripts/Makefile.kasan
+++ b/scripts/Makefile.kasan
@@ -13,12 +13,16 @@ CFLAGS_KASAN := $(call cc-option, -fsanitize=kernel-address \
--param asan-instrumentation-with-call-threshold=$(call_threshold))

ifeq ($(call cc-option, $(CFLAGS_KASAN_MINIMAL) -Werror),)
+ ifneq ($(CONFIG_COMPILE_TEST),y)
$(warning Cannot use CONFIG_KASAN: \
-fsanitize=kernel-address is not supported by compiler)
+ endif
else
ifeq ($(CFLAGS_KASAN),)
- $(warning CONFIG_KASAN: compiler does not support all options.\
- Trying minimal configuration)
+ ifneq ($(CONFIG_COMPILE_TEST),y)
+ $(warning CONFIG_KASAN: compiler does not support all options.\
+ Trying minimal configuration)
+ endif
CFLAGS_KASAN := $(CFLAGS_KASAN_MINIMAL)
endif
endif
--
2.3.5


2015-04-09 11:06:47

by Paul Bolle

[permalink] [raw]
Subject: Re: [PATCH] kasan: Makefile: shut up warnings if CONFIG_COMPILE_TEST=y

On Wed, 2015-04-08 at 18:38 +0300, Andrey Ryabinin wrote:
> It might be annoying to constantly see this:
>
> scripts/Makefile.kasan:16: Cannot use CONFIG_KASAN: -fsanitize=kernel-address is not supported by compiler
>
> while performing allmodconfig/allyesconfig build tests.

That warning might be seen - once per build - because allmodconfig and
allyesconfig enable both CONFIG_KASAN and CONFIG_COMPILE_TEST, right?

> Disable this warning if CONFIG_COMPILE_TEST=y.

Do we expect that some people want to enable both KASAN and COMPILE_TEST
manually (ie, not as a result of allmodconfig or allyesconfig)? If so,
those people might now be in for some head-scratching if their compiler
lacks what's apparently needed to run kasan.

> Signed-off-by: Andrey Ryabinin <[email protected]>
> ---
> scripts/Makefile.kasan | 8 ++++++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
> index 631619b..3f874d2 100644
> --- a/scripts/Makefile.kasan
> +++ b/scripts/Makefile.kasan
> @@ -13,12 +13,16 @@ CFLAGS_KASAN := $(call cc-option, -fsanitize=kernel-address \
> --param asan-instrumentation-with-call-threshold=$(call_threshold))
>
> ifeq ($(call cc-option, $(CFLAGS_KASAN_MINIMAL) -Werror),)
> + ifneq ($(CONFIG_COMPILE_TEST),y)
> $(warning Cannot use CONFIG_KASAN: \
> -fsanitize=kernel-address is not supported by compiler)
> + endif
> else
> ifeq ($(CFLAGS_KASAN),)
> - $(warning CONFIG_KASAN: compiler does not support all options.\
> - Trying minimal configuration)
> + ifneq ($(CONFIG_COMPILE_TEST),y)
> + $(warning CONFIG_KASAN: compiler does not support all options.\
> + Trying minimal configuration)

(Side note: in this case the compiler supports that minimal
configuration, doesn't it? So shouldn't the second warning end in
Running with minimal configuration)

or something to that effect?)

> + endif
> CFLAGS_KASAN := $(CFLAGS_KASAN_MINIMAL)
> endif
> endif

Thanks,


Paul Bolle

2015-04-09 12:44:42

by Andrey Ryabinin

[permalink] [raw]
Subject: Re: [PATCH] kasan: Makefile: shut up warnings if CONFIG_COMPILE_TEST=y

On 04/09/2015 02:06 PM, Paul Bolle wrote:
> On Wed, 2015-04-08 at 18:38 +0300, Andrey Ryabinin wrote:
>> It might be annoying to constantly see this:
>>
>> scripts/Makefile.kasan:16: Cannot use CONFIG_KASAN: -fsanitize=kernel-address is not supported by compiler
>>
>> while performing allmodconfig/allyesconfig build tests.
>
> That warning might be seen - once per build - because allmodconfig and
> allyesconfig enable both CONFIG_KASAN and CONFIG_COMPILE_TEST, right?
>

It might be seen once per build because all*config enables CONFIG_KASAN.
There was no dependency on CONFIG_COMPILE_TEST before this patch.

>> Disable this warning if CONFIG_COMPILE_TEST=y.
>
> Do we expect that some people want to enable both KASAN and COMPILE_TEST
> manually (ie, not as a result of allmodconfig or allyesconfig)? If so,
> those people might now be in for some head-scratching if their compiler
> lacks what's apparently needed to run kasan.
>

I think this shouldn't be a problem.
CONFIG_COMPILE_TEST used for building drivers that cannot be loaded on
platform used for compiling these drivers.
So it's unlikely that such kernel will be used for runtime testing.


>> Signed-off-by: Andrey Ryabinin <[email protected]>
>> ---
>> scripts/Makefile.kasan | 8 ++++++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
>> index 631619b..3f874d2 100644
>> --- a/scripts/Makefile.kasan
>> +++ b/scripts/Makefile.kasan
>> @@ -13,12 +13,16 @@ CFLAGS_KASAN := $(call cc-option, -fsanitize=kernel-address \
>> --param asan-instrumentation-with-call-threshold=$(call_threshold))
>>
>> ifeq ($(call cc-option, $(CFLAGS_KASAN_MINIMAL) -Werror),)
>> + ifneq ($(CONFIG_COMPILE_TEST),y)
>> $(warning Cannot use CONFIG_KASAN: \
>> -fsanitize=kernel-address is not supported by compiler)
>> + endif
>> else
>> ifeq ($(CFLAGS_KASAN),)
>> - $(warning CONFIG_KASAN: compiler does not support all options.\
>> - Trying minimal configuration)
>> + ifneq ($(CONFIG_COMPILE_TEST),y)
>> + $(warning CONFIG_KASAN: compiler does not support all options.\
>> + Trying minimal configuration)
>
> (Side note: in this case the compiler supports that minimal
> configuration, doesn't it? So shouldn't the second warning end in
> Running with minimal configuration)
>
> or something to that effect?)
>

It should and it does.
CFLAGS_KASAN reassigned to minimal set of options below.

>> + endif
>> CFLAGS_KASAN := $(CFLAGS_KASAN_MINIMAL)
>> endif
>> endif
>
> Thanks,
>
>
> Paul Bolle
>
>

2015-04-09 16:01:14

by Paul Bolle

[permalink] [raw]
Subject: Re: [PATCH] kasan: Makefile: shut up warnings if CONFIG_COMPILE_TEST=y

On Thu, 2015-04-09 at 15:44 +0300, Andrey Ryabinin wrote:
> On 04/09/2015 02:06 PM, Paul Bolle wrote:
> > On Wed, 2015-04-08 at 18:38 +0300, Andrey Ryabinin wrote:
> >> It might be annoying to constantly see this:
> >>
> >> scripts/Makefile.kasan:16: Cannot use CONFIG_KASAN: -fsanitize=kernel-address is not supported by compiler
> >>
> >> while performing allmodconfig/allyesconfig build tests.
> >
> > That warning might be seen - once per build - because allmodconfig and
> > allyesconfig enable both CONFIG_KASAN and CONFIG_COMPILE_TEST, right?
> >
>
> It might be seen once per build because all*config enables CONFIG_KASAN.
> There was no dependency on CONFIG_COMPILE_TEST before this patch.

Yes, of course.

> >> Disable this warning if CONFIG_COMPILE_TEST=y.
> >
> > Do we expect that some people want to enable both KASAN and COMPILE_TEST
> > manually (ie, not as a result of allmodconfig or allyesconfig)? If so,
> > those people might now be in for some head-scratching if their compiler
> > lacks what's apparently needed to run kasan.
> >
>
> I think this shouldn't be a problem.
> CONFIG_COMPILE_TEST used for building drivers that cannot be loaded on
> platform used for compiling these drivers.
> So it's unlikely that such kernel will be used for runtime testing.

It would be nice if there would at least be a comment explaining what's
being done here (ie, assuming that if CONFIG_COMPILE_TEST equals 'y'
we're doing a allmodconfig or an allyesconfig and that people using
either of those make targets don't care whether their compiler supports
kasan). Because I'd be surprised if people figure that out by looking at
this Makefile.

> >> Signed-off-by: Andrey Ryabinin <[email protected]>
> >> ---
> >> scripts/Makefile.kasan | 8 ++++++--
> >> 1 file changed, 6 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
> >> index 631619b..3f874d2 100644
> >> --- a/scripts/Makefile.kasan
> >> +++ b/scripts/Makefile.kasan
> >> @@ -13,12 +13,16 @@ CFLAGS_KASAN := $(call cc-option, -fsanitize=kernel-address \
> >> --param asan-instrumentation-with-call-threshold=$(call_threshold))
> >>
> >> ifeq ($(call cc-option, $(CFLAGS_KASAN_MINIMAL) -Werror),)
> >> + ifneq ($(CONFIG_COMPILE_TEST),y)
> >> $(warning Cannot use CONFIG_KASAN: \
> >> -fsanitize=kernel-address is not supported by compiler)
> >> + endif
> >> else
> >> ifeq ($(CFLAGS_KASAN),)
> >> - $(warning CONFIG_KASAN: compiler does not support all options.\
> >> - Trying minimal configuration)
> >> + ifneq ($(CONFIG_COMPILE_TEST),y)
> >> + $(warning CONFIG_KASAN: compiler does not support all options.\
> >> + Trying minimal configuration)
> >
> > (Side note: in this case the compiler supports that minimal
> > configuration, doesn't it? So shouldn't the second warning end in
> > Running with minimal configuration)
> >
> > or something to that effect?)
> >
>
> It should and it does.
> CFLAGS_KASAN reassigned to minimal set of options below.

Which makes "Trying" ambiguous, but anyway, it's just a side note.

> >> + endif
> >> CFLAGS_KASAN := $(CFLAGS_KASAN_MINIMAL)
> >> endif
> >> endif

Thanks,


Paul Bolle