linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3] kbuild: Check for unknown options with cc-option usage in Kconfig and clang
@ 2019-07-30 16:48 Stephen Boyd
  2019-07-30 16:49 ` Nathan Chancellor
  0 siblings, 1 reply; 4+ messages in thread
From: Stephen Boyd @ 2019-07-30 16:48 UTC (permalink / raw)
  To: Masahiro Yamada, Michal Marek
  Cc: linux-kernel, linux-kbuild, clang-built-linux, Peter Smith,
	Nathan Chancellor, Nick Desaulniers, Douglas Anderson

If the particular version of clang a user has doesn't enable
-Werror=unknown-warning-option by default, even though it is the
default[1], then make sure to pass the option to the Kconfig cc-option
command so that testing options from Kconfig files works properly.
Otherwise, depending on the default values setup in the clang toolchain
we will silently assume options such as -Wmaybe-uninitialized are
supported by clang, when they really aren't.

A compilation issue only started happening for me once commit
589834b3a009 ("kbuild: Add -Werror=unknown-warning-option to
CLANG_FLAGS") was applied on top of commit b303c6df80c9 ("kbuild:
compute false-positive -Wmaybe-uninitialized cases in Kconfig"). This
leads kbuild to try and test for the existence of the
-Wmaybe-uninitialized flag with the cc-option command in
scripts/Kconfig.include, and it doesn't see an error returned from the
option test so it sets the config value to Y. Then the Makefile tries to
pass the unknown option on the command line and
-Werror=unknown-warning-option catches the invalid option and breaks the
build. Before commit 589834b3a009 ("kbuild: Add
-Werror=unknown-warning-option to CLANG_FLAGS") the build works fine,
but any cc-option test of a warning option in Kconfig files silently
evaluates to true, even if the warning option flag isn't supported on
clang.

Note: This doesn't change cc-option usages in Makefiles because those
use a different rule that includes KBUILD_CFLAGS by default (see the
__cc-option command in scripts/Kbuild.incluide). The KBUILD_CFLAGS
variable already has the -Werror=unknown-warning-option flag set. Thanks
to Doug for pointing out the different rule.

[1] https://clang.llvm.org/docs/DiagnosticsReference.html#wunknown-warning-option
Cc: Peter Smith <peter.smith@linaro.org>
Cc: Nathan Chancellor <natechancellor@gmail.com>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Douglas Anderson <dianders@chromium.org>
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
---
 Makefile                | 1 +
 scripts/Kconfig.include | 2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 9be5834073f8..517d0a3f6539 100644
--- a/Makefile
+++ b/Makefile
@@ -536,6 +536,7 @@ KBUILD_AFLAGS	+= $(CLANG_FLAGS)
 export CLANG_FLAGS
 endif
 
+
 # The expansion should be delayed until arch/$(SRCARCH)/Makefile is included.
 # Some architectures define CROSS_COMPILE in arch/$(SRCARCH)/Makefile.
 # CC_VERSION_TEXT is referenced from Kconfig (so it needs export),
diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include
index 8a5c4d645eb1..4bbf4fc163a2 100644
--- a/scripts/Kconfig.include
+++ b/scripts/Kconfig.include
@@ -25,7 +25,7 @@ failure = $(if-success,$(1),n,y)
 
 # $(cc-option,<flag>)
 # Return y if the compiler supports <flag>, n otherwise
-cc-option = $(success,$(CC) -Werror $(1) -E -x c /dev/null -o /dev/null)
+cc-option = $(success,$(CC) -Werror $(CLANG_FLAGS) $(1) -E -x c /dev/null -o /dev/null)
 
 # $(ld-option,<flag>)
 # Return y if the linker supports <flag>, n otherwise
-- 
Sent by a computer through tubes


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] kbuild: Check for unknown options with cc-option usage in Kconfig and clang
  2019-07-30 16:48 [PATCH v3] kbuild: Check for unknown options with cc-option usage in Kconfig and clang Stephen Boyd
@ 2019-07-30 16:49 ` Nathan Chancellor
  2019-07-30 16:51   ` Masahiro Yamada
  0 siblings, 1 reply; 4+ messages in thread
From: Nathan Chancellor @ 2019-07-30 16:49 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Masahiro Yamada, Michal Marek, linux-kernel, linux-kbuild,
	clang-built-linux, Peter Smith, Nick Desaulniers,
	Douglas Anderson

On Tue, Jul 30, 2019 at 09:48:03AM -0700, Stephen Boyd wrote:
> If the particular version of clang a user has doesn't enable
> -Werror=unknown-warning-option by default, even though it is the
> default[1], then make sure to pass the option to the Kconfig cc-option
> command so that testing options from Kconfig files works properly.
> Otherwise, depending on the default values setup in the clang toolchain
> we will silently assume options such as -Wmaybe-uninitialized are
> supported by clang, when they really aren't.
> 
> A compilation issue only started happening for me once commit
> 589834b3a009 ("kbuild: Add -Werror=unknown-warning-option to
> CLANG_FLAGS") was applied on top of commit b303c6df80c9 ("kbuild:
> compute false-positive -Wmaybe-uninitialized cases in Kconfig"). This
> leads kbuild to try and test for the existence of the
> -Wmaybe-uninitialized flag with the cc-option command in
> scripts/Kconfig.include, and it doesn't see an error returned from the
> option test so it sets the config value to Y. Then the Makefile tries to
> pass the unknown option on the command line and
> -Werror=unknown-warning-option catches the invalid option and breaks the
> build. Before commit 589834b3a009 ("kbuild: Add
> -Werror=unknown-warning-option to CLANG_FLAGS") the build works fine,
> but any cc-option test of a warning option in Kconfig files silently
> evaluates to true, even if the warning option flag isn't supported on
> clang.
> 
> Note: This doesn't change cc-option usages in Makefiles because those
> use a different rule that includes KBUILD_CFLAGS by default (see the
> __cc-option command in scripts/Kbuild.incluide). The KBUILD_CFLAGS
> variable already has the -Werror=unknown-warning-option flag set. Thanks
> to Doug for pointing out the different rule.
> 
> [1] https://clang.llvm.org/docs/DiagnosticsReference.html#wunknown-warning-option
> Cc: Peter Smith <peter.smith@linaro.org>
> Cc: Nathan Chancellor <natechancellor@gmail.com>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Cc: Douglas Anderson <dianders@chromium.org>
> Signed-off-by: Stephen Boyd <swboyd@chromium.org>

Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>

> ---
>  Makefile                | 1 +
>  scripts/Kconfig.include | 2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 9be5834073f8..517d0a3f6539 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -536,6 +536,7 @@ KBUILD_AFLAGS	+= $(CLANG_FLAGS)
>  export CLANG_FLAGS
>  endif
>  
> +

Not sure it's worth sending a v4 for but I don't think this should be
there.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] kbuild: Check for unknown options with cc-option usage in Kconfig and clang
  2019-07-30 16:49 ` Nathan Chancellor
@ 2019-07-30 16:51   ` Masahiro Yamada
  2019-08-01  2:56     ` Masahiro Yamada
  0 siblings, 1 reply; 4+ messages in thread
From: Masahiro Yamada @ 2019-07-30 16:51 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Stephen Boyd, Michal Marek, Linux Kernel Mailing List,
	Linux Kbuild mailing list, clang-built-linux, Peter Smith,
	Nick Desaulniers, Douglas Anderson

On Wed, Jul 31, 2019 at 1:50 AM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> On Tue, Jul 30, 2019 at 09:48:03AM -0700, Stephen Boyd wrote:
> > If the particular version of clang a user has doesn't enable
> > -Werror=unknown-warning-option by default, even though it is the
> > default[1], then make sure to pass the option to the Kconfig cc-option
> > command so that testing options from Kconfig files works properly.
> > Otherwise, depending on the default values setup in the clang toolchain
> > we will silently assume options such as -Wmaybe-uninitialized are
> > supported by clang, when they really aren't.
> >
> > A compilation issue only started happening for me once commit
> > 589834b3a009 ("kbuild: Add -Werror=unknown-warning-option to
> > CLANG_FLAGS") was applied on top of commit b303c6df80c9 ("kbuild:
> > compute false-positive -Wmaybe-uninitialized cases in Kconfig"). This
> > leads kbuild to try and test for the existence of the
> > -Wmaybe-uninitialized flag with the cc-option command in
> > scripts/Kconfig.include, and it doesn't see an error returned from the
> > option test so it sets the config value to Y. Then the Makefile tries to
> > pass the unknown option on the command line and
> > -Werror=unknown-warning-option catches the invalid option and breaks the
> > build. Before commit 589834b3a009 ("kbuild: Add
> > -Werror=unknown-warning-option to CLANG_FLAGS") the build works fine,
> > but any cc-option test of a warning option in Kconfig files silently
> > evaluates to true, even if the warning option flag isn't supported on
> > clang.
> >
> > Note: This doesn't change cc-option usages in Makefiles because those
> > use a different rule that includes KBUILD_CFLAGS by default (see the
> > __cc-option command in scripts/Kbuild.incluide). The KBUILD_CFLAGS
> > variable already has the -Werror=unknown-warning-option flag set. Thanks
> > to Doug for pointing out the different rule.
> >
> > [1] https://clang.llvm.org/docs/DiagnosticsReference.html#wunknown-warning-option
> > Cc: Peter Smith <peter.smith@linaro.org>
> > Cc: Nathan Chancellor <natechancellor@gmail.com>
> > Cc: Nick Desaulniers <ndesaulniers@google.com>
> > Cc: Douglas Anderson <dianders@chromium.org>
> > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
>
> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
>
> > ---
> >  Makefile                | 1 +
> >  scripts/Kconfig.include | 2 +-
> >  2 files changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 9be5834073f8..517d0a3f6539 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -536,6 +536,7 @@ KBUILD_AFLAGS     += $(CLANG_FLAGS)
> >  export CLANG_FLAGS
> >  endif
> >
> > +
>
> Not sure it's worth sending a v4 for but I don't think this should be
> there.


I will remove it when I apply this.



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3] kbuild: Check for unknown options with cc-option usage in Kconfig and clang
  2019-07-30 16:51   ` Masahiro Yamada
@ 2019-08-01  2:56     ` Masahiro Yamada
  0 siblings, 0 replies; 4+ messages in thread
From: Masahiro Yamada @ 2019-08-01  2:56 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Stephen Boyd, Michal Marek, Linux Kernel Mailing List,
	Linux Kbuild mailing list, clang-built-linux, Peter Smith,
	Nick Desaulniers, Douglas Anderson

On Wed, Jul 31, 2019 at 1:51 AM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> On Wed, Jul 31, 2019 at 1:50 AM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > On Tue, Jul 30, 2019 at 09:48:03AM -0700, Stephen Boyd wrote:
> > > If the particular version of clang a user has doesn't enable
> > > -Werror=unknown-warning-option by default, even though it is the
> > > default[1], then make sure to pass the option to the Kconfig cc-option
> > > command so that testing options from Kconfig files works properly.
> > > Otherwise, depending on the default values setup in the clang toolchain
> > > we will silently assume options such as -Wmaybe-uninitialized are
> > > supported by clang, when they really aren't.
> > >
> > > A compilation issue only started happening for me once commit
> > > 589834b3a009 ("kbuild: Add -Werror=unknown-warning-option to
> > > CLANG_FLAGS") was applied on top of commit b303c6df80c9 ("kbuild:
> > > compute false-positive -Wmaybe-uninitialized cases in Kconfig"). This
> > > leads kbuild to try and test for the existence of the
> > > -Wmaybe-uninitialized flag with the cc-option command in
> > > scripts/Kconfig.include, and it doesn't see an error returned from the
> > > option test so it sets the config value to Y. Then the Makefile tries to
> > > pass the unknown option on the command line and
> > > -Werror=unknown-warning-option catches the invalid option and breaks the
> > > build. Before commit 589834b3a009 ("kbuild: Add
> > > -Werror=unknown-warning-option to CLANG_FLAGS") the build works fine,
> > > but any cc-option test of a warning option in Kconfig files silently
> > > evaluates to true, even if the warning option flag isn't supported on
> > > clang.
> > >
> > > Note: This doesn't change cc-option usages in Makefiles because those
> > > use a different rule that includes KBUILD_CFLAGS by default (see the
> > > __cc-option command in scripts/Kbuild.incluide). The KBUILD_CFLAGS
> > > variable already has the -Werror=unknown-warning-option flag set. Thanks
> > > to Doug for pointing out the different rule.
> > >
> > > [1] https://clang.llvm.org/docs/DiagnosticsReference.html#wunknown-warning-option
> > > Cc: Peter Smith <peter.smith@linaro.org>
> > > Cc: Nathan Chancellor <natechancellor@gmail.com>
> > > Cc: Nick Desaulniers <ndesaulniers@google.com>
> > > Cc: Douglas Anderson <dianders@chromium.org>
> > > Signed-off-by: Stephen Boyd <swboyd@chromium.org>
> >
> > Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> >
> > > ---
> > >  Makefile                | 1 +
> > >  scripts/Kconfig.include | 2 +-
> > >  2 files changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index 9be5834073f8..517d0a3f6539 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -536,6 +536,7 @@ KBUILD_AFLAGS     += $(CLANG_FLAGS)
> > >  export CLANG_FLAGS
> > >  endif
> > >
> > > +
> >
> > Not sure it's worth sending a v4 for but I don't think this should be
> > there.
>
>
> I will remove it when I apply this.
>

Applied to linux-kbuild/fixes. Thanks.

-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-08-01  2:57 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-07-30 16:48 [PATCH v3] kbuild: Check for unknown options with cc-option usage in Kconfig and clang Stephen Boyd
2019-07-30 16:49 ` Nathan Chancellor
2019-07-30 16:51   ` Masahiro Yamada
2019-08-01  2:56     ` Masahiro Yamada

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).