Makefile: CC_IMPLICIT_FALLTHROUGH passed quoted as argument to gcc

Makefile: CC_IMPLICIT_FALLTHROUGH passed quoted as argument to gcc

[Salvatore Bonaccorso]

Hi Gustavo,

Since dee2b702bcf0 ("kconfig: Add support for -Wimplicit-fallthrough")
CONFIG_CC_IMPLICIT_FALLTHROUGH value is passed quoted to the gcc
invocation.

This appears to cause issues for (external) module builds. It was
reported in Debian for the nvidia module, cf.
https://bugs.debian.org/1001083 but might happen as well in other
cases.

Andreas suggested to replace the

KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH)

with

KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(patsubst "%",%,$(CONFIG_CC_IMPLICIT_FALLTHROUGH))

Is this something you would consider doing or should the issue be
handled exclusively in the particular OOT module build case?

Regards,
Salvatore

Re: Makefile: CC_IMPLICIT_FALLTHROUGH passed quoted as argument to gcc

[Linus Torvalds]

On Sat, Dec 4, 2021 at 5:13 AM Salvatore Bonaccorso <carnil@debian.org> wrote:
>
> Andreas suggested to replace the
>
> KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
>
> with
>
> KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(patsubst "%",%,$(CONFIG_CC_IMPLICIT_FALLTHROUGH))

Ugh. I think the external build environment is a bit broken, but
whatever. The above is ugly but I guess it works.

Another alternative would be to make the Kconfig strings simply not
have '"' as part of them.

When you do

    a = "hello"
    print $a

in any normal language, you generally wouldn't expect it to print the
quotes, it should just print the bare word.

But that's what the Kconfig string language basically does in this
case. And I guess several users expect and take advantage of that ;(

Masahiro? Comments?

             Linus

Re: Makefile: CC_IMPLICIT_FALLTHROUGH passed quoted as argument to gcc

[Masahiro Yamada]

On Sun, Dec 5, 2021 at 1:53 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Sat, Dec 4, 2021 at 5:13 AM Salvatore Bonaccorso <carnil@debian.org> wrote:
> >
> > Andreas suggested to replace the
> >
> > KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
> >
> > with
> >
> > KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(patsubst "%",%,$(CONFIG_CC_IMPLICIT_FALLTHROUGH))
>
> Ugh. I think the external build environment is a bit broken, but
> whatever. The above is ugly but I guess it works.
>
> Another alternative would be to make the Kconfig strings simply not
> have '"' as part of them.
>
> When you do
>
>     a = "hello"
>     print $a
>
> in any normal language, you generally wouldn't expect it to print the
> quotes, it should just print the bare word.
>
> But that's what the Kconfig string language basically does in this
> case. And I guess several users expect and take advantage of that ;(
>
> Masahiro? Comments?

Yes, you get to the point.

In fact, this is in my TODO list for a while
(and this is the reason I was doing prerequisite Kconfig refactoring
in the previous development cycle).
I will try to find some spare time to complete this work.



Kconfig generates two similar files,

 -   .config
 -   include/config/auto.conf

Changing the format of the .config is presumably problematic
since it is the saved user configuration as well.

It is possible (and more reasonable) to change include/config/auto.conf
so strings are not quoted.

In Makefiles, quotations are just normal characters; they have no
special functionality.

So, in Makefile context, it is more handy to do

     CONFIG_X=foo bar

instead of

    CONFIG_X="foo bar"



One problem is include/config/auto.conf is included not only by Makefiles
but also by shell scripts.


In shell context, the right hand side must be quoted
in case the value contains spaces.

   CONFIG_X="foo bar"



My plan is to fix
  scripts/link-vmlinux.sh
  scripts/gen_autoksyms.sh
etc. to not directly include the auto.conf.
Later, change Kconfig to generate the auto.conf without "".



In the meantime,

KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(patsubst
"%",%,$(CONFIG_CC_IMPLICIT_FALLTHROUGH))

 or if you prefer slightly shorter form,

KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH:"%"=%)

will be a workaround.





>
>              Linus



-- 
Best Regards
Masahiro Yamada

Re: Makefile: CC_IMPLICIT_FALLTHROUGH passed quoted as argument to gcc

[Kees Cook]

On Sun, Dec 05, 2021 at 02:54:05AM +0900, Masahiro Yamada wrote:
> On Sun, Dec 5, 2021 at 1:53 AM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > On Sat, Dec 4, 2021 at 5:13 AM Salvatore Bonaccorso <carnil@debian.org> wrote:
> > >
> > > Andreas suggested to replace the
> > >
> > > KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
> > >
> > > with
> > >
> > > KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(patsubst "%",%,$(CONFIG_CC_IMPLICIT_FALLTHROUGH))
> >
> > Ugh. I think the external build environment is a bit broken, but
> > whatever. The above is ugly but I guess it works.
> >
> > Another alternative would be to make the Kconfig strings simply not
> > have '"' as part of them.
> >
> > When you do
> >
> >     a = "hello"
> >     print $a
> >
> > in any normal language, you generally wouldn't expect it to print the
> > quotes, it should just print the bare word.
> >
> > But that's what the Kconfig string language basically does in this
> > case. And I guess several users expect and take advantage of that ;(
> >
> > Masahiro? Comments?
> 
> Yes, you get to the point.
> 
> In fact, this is in my TODO list for a while
> (and this is the reason I was doing prerequisite Kconfig refactoring
> in the previous development cycle).
> I will try to find some spare time to complete this work.
> 
> 
> 
> Kconfig generates two similar files,
> 
>  -   .config
>  -   include/config/auto.conf
> 
> Changing the format of the .config is presumably problematic
> since it is the saved user configuration as well.
> 
> It is possible (and more reasonable) to change include/config/auto.conf
> so strings are not quoted.
> 
> In Makefiles, quotations are just normal characters; they have no
> special functionality.
> 
> So, in Makefile context, it is more handy to do
> 
>      CONFIG_X=foo bar
> 
> instead of
> 
>     CONFIG_X="foo bar"
> 
> 
> 
> One problem is include/config/auto.conf is included not only by Makefiles
> but also by shell scripts.
> 
> 
> In shell context, the right hand side must be quoted
> in case the value contains spaces.
> 
>    CONFIG_X="foo bar"
> 
> 
> 
> My plan is to fix
>   scripts/link-vmlinux.sh
>   scripts/gen_autoksyms.sh
> etc. to not directly include the auto.conf.
> Later, change Kconfig to generate the auto.conf without "".
> 
> 
> 
> In the meantime,
> 
> KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(patsubst
> "%",%,$(CONFIG_CC_IMPLICIT_FALLTHROUGH))
> 
>  or if you prefer slightly shorter form,
> 
> KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH:"%"=%)
> 
> will be a workaround.

It'll be nice to get this fixed. There are a few places where there is
a test for a compiler flag in Kconfig, and then the option is repeated
in the Makefile, due to the above quoting issues. For example:

arch/arm64/Kconfig:
	config CC_HAS_BRANCH_PROT_PAC_RET
	     # GCC 9 or later, clang 8 or later
	     def_bool $(cc-option,-mbranch-protection=pac-ret+leaf)

arch/arm64/Makefile:
	branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=pac-ret+leaf


I like the $(CONFIG_CC_IMPLICIT_FALLTHROUGH:"%"=%) solution: it's short.

-- 
Kees Cook

Re: Makefile: CC_IMPLICIT_FALLTHROUGH passed quoted as argument to gcc

[Salvatore Bonaccorso]

Hi,

On Mon, Dec 06, 2021 at 11:53:41AM -0800, Kees Cook wrote:
> On Sun, Dec 05, 2021 at 02:54:05AM +0900, Masahiro Yamada wrote:
> > On Sun, Dec 5, 2021 at 1:53 AM Linus Torvalds
> > <torvalds@linux-foundation.org> wrote:
> > >
> > > On Sat, Dec 4, 2021 at 5:13 AM Salvatore Bonaccorso <carnil@debian.org> wrote:
> > > >
> > > > Andreas suggested to replace the
> > > >
> > > > KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
> > > >
> > > > with
> > > >
> > > > KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(patsubst "%",%,$(CONFIG_CC_IMPLICIT_FALLTHROUGH))
> > >
> > > Ugh. I think the external build environment is a bit broken, but
> > > whatever. The above is ugly but I guess it works.
> > >
> > > Another alternative would be to make the Kconfig strings simply not
> > > have '"' as part of them.
> > >
> > > When you do
> > >
> > >     a = "hello"
> > >     print $a
> > >
> > > in any normal language, you generally wouldn't expect it to print the
> > > quotes, it should just print the bare word.
> > >
> > > But that's what the Kconfig string language basically does in this
> > > case. And I guess several users expect and take advantage of that ;(
> > >
> > > Masahiro? Comments?
> > 
> > Yes, you get to the point.
> > 
> > In fact, this is in my TODO list for a while
> > (and this is the reason I was doing prerequisite Kconfig refactoring
> > in the previous development cycle).
> > I will try to find some spare time to complete this work.
> > 
> > 
> > 
> > Kconfig generates two similar files,
> > 
> >  -   .config
> >  -   include/config/auto.conf
> > 
> > Changing the format of the .config is presumably problematic
> > since it is the saved user configuration as well.
> > 
> > It is possible (and more reasonable) to change include/config/auto.conf
> > so strings are not quoted.
> > 
> > In Makefiles, quotations are just normal characters; they have no
> > special functionality.
> > 
> > So, in Makefile context, it is more handy to do
> > 
> >      CONFIG_X=foo bar
> > 
> > instead of
> > 
> >     CONFIG_X="foo bar"
> > 
> > 
> > 
> > One problem is include/config/auto.conf is included not only by Makefiles
> > but also by shell scripts.
> > 
> > 
> > In shell context, the right hand side must be quoted
> > in case the value contains spaces.
> > 
> >    CONFIG_X="foo bar"
> > 
> > 
> > 
> > My plan is to fix
> >   scripts/link-vmlinux.sh
> >   scripts/gen_autoksyms.sh
> > etc. to not directly include the auto.conf.
> > Later, change Kconfig to generate the auto.conf without "".
> > 
> > 
> > 
> > In the meantime,
> > 
> > KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(patsubst
> > "%",%,$(CONFIG_CC_IMPLICIT_FALLTHROUGH))
> > 
> >  or if you prefer slightly shorter form,
> > 
> > KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH:"%"=%)
> > 
> > will be a workaround.
> 
> It'll be nice to get this fixed. There are a few places where there is
> a test for a compiler flag in Kconfig, and then the option is repeated
> in the Makefile, due to the above quoting issues. For example:
> 
> arch/arm64/Kconfig:
> 	config CC_HAS_BRANCH_PROT_PAC_RET
> 	     # GCC 9 or later, clang 8 or later
> 	     def_bool $(cc-option,-mbranch-protection=pac-ret+leaf)
> 
> arch/arm64/Makefile:
> 	branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=pac-ret+leaf
> 
> 
> I like the $(CONFIG_CC_IMPLICIT_FALLTHROUGH:"%"=%) solution: it's short.

Does the following look correct, as well from formal style/commit
description? I have not yet done many contributions directly.

Regards,
Salvatore

From c2d01ea3ee1c7cc539468bba5b25522245d513de Mon Sep 17 00:00:00 2001
From: Salvatore Bonaccorso <carnil@debian.org>
Date: Mon, 6 Dec 2021 21:42:01 +0100
Subject: [PATCH] Makefile: Do not quote value for
 CONFIG_CC_IMPLICIT_FALLTHROUGH

Andreas reported that a specific build environment for an external
module, being a bit broken, does pass CC_IMPLICIT_FALLTHROUGH quoted as
argument to gcc, causing an error

	gcc-11: error: "-Wimplicit-fallthrough=5": linker input file not found: No such file or directory

Until this is more generally fixed as outlined in [1], by fixing
scripts/link-vmlinux.sh, scripts/gen_autoksyms.sh, etc to not directly
include the include/config/auto.conf, and in a second step, change
Kconfig to generate the auto.conf without "", workaround the issue by
explicitly unquoting CC_IMPLICIT_FALLTHROUGH.

 [1] https://lore.kernel.org/linux-kbuild/CAK7LNAR-VXwHFEJqCcrFDZj+_4+Xd6oynbj_0eS8N504_ydmyw@mail.gmail.com/

Reported-by: Andreas Beckmann <anbe@debian.org>
Link: https://bugs.debian.org/1001083
Signed-off-by: Salvatore Bonaccorso <carnil@debian.org>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 8e35d7804fef..ef967a26bcd3 100644
--- a/Makefile
+++ b/Makefile
@@ -789,7 +789,7 @@ stackp-flags-$(CONFIG_STACKPROTECTOR_STRONG)      := -fstack-protector-strong
 KBUILD_CFLAGS += $(stackp-flags-y)
 
 KBUILD_CFLAGS-$(CONFIG_WERROR) += -Werror
-KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
+KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH:"%"=%)
 
 ifdef CONFIG_CC_IS_CLANG
 KBUILD_CPPFLAGS += -Qunused-arguments
-- 
2.34.1

Re: Makefile: CC_IMPLICIT_FALLTHROUGH passed quoted as argument to gcc

[Kees Cook]

On Mon, Dec 06, 2021 at 11:02:05PM +0100, Salvatore Bonaccorso wrote:
> Hi,
> 
> On Mon, Dec 06, 2021 at 11:53:41AM -0800, Kees Cook wrote:
> > On Sun, Dec 05, 2021 at 02:54:05AM +0900, Masahiro Yamada wrote:
> > > On Sun, Dec 5, 2021 at 1:53 AM Linus Torvalds
> > > <torvalds@linux-foundation.org> wrote:
> > > >
> > > > On Sat, Dec 4, 2021 at 5:13 AM Salvatore Bonaccorso <carnil@debian.org> wrote:
> > > > >
> > > > > Andreas suggested to replace the
> > > > >
> > > > > KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
> > > > >
> > > > > with
> > > > >
> > > > > KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(patsubst "%",%,$(CONFIG_CC_IMPLICIT_FALLTHROUGH))
> > > >
> > > > Ugh. I think the external build environment is a bit broken, but
> > > > whatever. The above is ugly but I guess it works.
> > > >
> > > > Another alternative would be to make the Kconfig strings simply not
> > > > have '"' as part of them.
> > > >
> > > > When you do
> > > >
> > > >     a = "hello"
> > > >     print $a
> > > >
> > > > in any normal language, you generally wouldn't expect it to print the
> > > > quotes, it should just print the bare word.
> > > >
> > > > But that's what the Kconfig string language basically does in this
> > > > case. And I guess several users expect and take advantage of that ;(
> > > >
> > > > Masahiro? Comments?
> > > 
> > > Yes, you get to the point.
> > > 
> > > In fact, this is in my TODO list for a while
> > > (and this is the reason I was doing prerequisite Kconfig refactoring
> > > in the previous development cycle).
> > > I will try to find some spare time to complete this work.
> > > 
> > > 
> > > 
> > > Kconfig generates two similar files,
> > > 
> > >  -   .config
> > >  -   include/config/auto.conf
> > > 
> > > Changing the format of the .config is presumably problematic
> > > since it is the saved user configuration as well.
> > > 
> > > It is possible (and more reasonable) to change include/config/auto.conf
> > > so strings are not quoted.
> > > 
> > > In Makefiles, quotations are just normal characters; they have no
> > > special functionality.
> > > 
> > > So, in Makefile context, it is more handy to do
> > > 
> > >      CONFIG_X=foo bar
> > > 
> > > instead of
> > > 
> > >     CONFIG_X="foo bar"
> > > 
> > > 
> > > 
> > > One problem is include/config/auto.conf is included not only by Makefiles
> > > but also by shell scripts.
> > > 
> > > 
> > > In shell context, the right hand side must be quoted
> > > in case the value contains spaces.
> > > 
> > >    CONFIG_X="foo bar"
> > > 
> > > 
> > > 
> > > My plan is to fix
> > >   scripts/link-vmlinux.sh
> > >   scripts/gen_autoksyms.sh
> > > etc. to not directly include the auto.conf.
> > > Later, change Kconfig to generate the auto.conf without "".
> > > 
> > > 
> > > 
> > > In the meantime,
> > > 
> > > KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(patsubst
> > > "%",%,$(CONFIG_CC_IMPLICIT_FALLTHROUGH))
> > > 
> > >  or if you prefer slightly shorter form,
> > > 
> > > KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH:"%"=%)
> > > 
> > > will be a workaround.
> > 
> > It'll be nice to get this fixed. There are a few places where there is
> > a test for a compiler flag in Kconfig, and then the option is repeated
> > in the Makefile, due to the above quoting issues. For example:
> > 
> > arch/arm64/Kconfig:
> > 	config CC_HAS_BRANCH_PROT_PAC_RET
> > 	     # GCC 9 or later, clang 8 or later
> > 	     def_bool $(cc-option,-mbranch-protection=pac-ret+leaf)
> > 
> > arch/arm64/Makefile:
> > 	branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=pac-ret+leaf
> > 
> > 
> > I like the $(CONFIG_CC_IMPLICIT_FALLTHROUGH:"%"=%) solution: it's short.
> 
> Does the following look correct, as well from formal style/commit
> description? I have not yet done many contributions directly.

Looks good to me; thanks!

> 
> Regards,
> Salvatore
> 
> From c2d01ea3ee1c7cc539468bba5b25522245d513de Mon Sep 17 00:00:00 2001
> From: Salvatore Bonaccorso <carnil@debian.org>
> Date: Mon, 6 Dec 2021 21:42:01 +0100
> Subject: [PATCH] Makefile: Do not quote value for
>  CONFIG_CC_IMPLICIT_FALLTHROUGH
> 
> Andreas reported that a specific build environment for an external
> module, being a bit broken, does pass CC_IMPLICIT_FALLTHROUGH quoted as
> argument to gcc, causing an error
> 
> 	gcc-11: error: "-Wimplicit-fallthrough=5": linker input file not found: No such file or directory
> 
> Until this is more generally fixed as outlined in [1], by fixing
> scripts/link-vmlinux.sh, scripts/gen_autoksyms.sh, etc to not directly
> include the include/config/auto.conf, and in a second step, change
> Kconfig to generate the auto.conf without "", workaround the issue by
> explicitly unquoting CC_IMPLICIT_FALLTHROUGH.
> 
>  [1] https://lore.kernel.org/linux-kbuild/CAK7LNAR-VXwHFEJqCcrFDZj+_4+Xd6oynbj_0eS8N504_ydmyw@mail.gmail.com/
> 
> Reported-by: Andreas Beckmann <anbe@debian.org>
> Link: https://bugs.debian.org/1001083
> Signed-off-by: Salvatore Bonaccorso <carnil@debian.org>

Reviewed-by: Kees Cook <keescook@chromium.org>

Does anyone have a preference as to who should take this? Gustavo,
Marahiro, me, or Linus directly?

-- 
Kees Cook

Re: Makefile: CC_IMPLICIT_FALLTHROUGH passed quoted as argument to gcc

[Nathan Chancellor]

On Mon, Dec 06, 2021 at 11:02:05PM +0100, Salvatore Bonaccorso wrote:
> Hi,
> 
> On Mon, Dec 06, 2021 at 11:53:41AM -0800, Kees Cook wrote:
> > On Sun, Dec 05, 2021 at 02:54:05AM +0900, Masahiro Yamada wrote:
> > > On Sun, Dec 5, 2021 at 1:53 AM Linus Torvalds
> > > <torvalds@linux-foundation.org> wrote:
> > > >
> > > > On Sat, Dec 4, 2021 at 5:13 AM Salvatore Bonaccorso <carnil@debian.org> wrote:
> > > > >
> > > > > Andreas suggested to replace the
> > > > >
> > > > > KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
> > > > >
> > > > > with
> > > > >
> > > > > KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(patsubst "%",%,$(CONFIG_CC_IMPLICIT_FALLTHROUGH))
> > > >
> > > > Ugh. I think the external build environment is a bit broken, but
> > > > whatever. The above is ugly but I guess it works.
> > > >
> > > > Another alternative would be to make the Kconfig strings simply not
> > > > have '"' as part of them.
> > > >
> > > > When you do
> > > >
> > > >     a = "hello"
> > > >     print $a
> > > >
> > > > in any normal language, you generally wouldn't expect it to print the
> > > > quotes, it should just print the bare word.
> > > >
> > > > But that's what the Kconfig string language basically does in this
> > > > case. And I guess several users expect and take advantage of that ;(
> > > >
> > > > Masahiro? Comments?
> > > 
> > > Yes, you get to the point.
> > > 
> > > In fact, this is in my TODO list for a while
> > > (and this is the reason I was doing prerequisite Kconfig refactoring
> > > in the previous development cycle).
> > > I will try to find some spare time to complete this work.
> > > 
> > > 
> > > 
> > > Kconfig generates two similar files,
> > > 
> > >  -   .config
> > >  -   include/config/auto.conf
> > > 
> > > Changing the format of the .config is presumably problematic
> > > since it is the saved user configuration as well.
> > > 
> > > It is possible (and more reasonable) to change include/config/auto.conf
> > > so strings are not quoted.
> > > 
> > > In Makefiles, quotations are just normal characters; they have no
> > > special functionality.
> > > 
> > > So, in Makefile context, it is more handy to do
> > > 
> > >      CONFIG_X=foo bar
> > > 
> > > instead of
> > > 
> > >     CONFIG_X="foo bar"
> > > 
> > > 
> > > 
> > > One problem is include/config/auto.conf is included not only by Makefiles
> > > but also by shell scripts.
> > > 
> > > 
> > > In shell context, the right hand side must be quoted
> > > in case the value contains spaces.
> > > 
> > >    CONFIG_X="foo bar"
> > > 
> > > 
> > > 
> > > My plan is to fix
> > >   scripts/link-vmlinux.sh
> > >   scripts/gen_autoksyms.sh
> > > etc. to not directly include the auto.conf.
> > > Later, change Kconfig to generate the auto.conf without "".
> > > 
> > > 
> > > 
> > > In the meantime,
> > > 
> > > KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(patsubst
> > > "%",%,$(CONFIG_CC_IMPLICIT_FALLTHROUGH))
> > > 
> > >  or if you prefer slightly shorter form,
> > > 
> > > KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH:"%"=%)
> > > 
> > > will be a workaround.
> > 
> > It'll be nice to get this fixed. There are a few places where there is
> > a test for a compiler flag in Kconfig, and then the option is repeated
> > in the Makefile, due to the above quoting issues. For example:
> > 
> > arch/arm64/Kconfig:
> > 	config CC_HAS_BRANCH_PROT_PAC_RET
> > 	     # GCC 9 or later, clang 8 or later
> > 	     def_bool $(cc-option,-mbranch-protection=pac-ret+leaf)
> > 
> > arch/arm64/Makefile:
> > 	branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=pac-ret+leaf
> > 
> > 
> > I like the $(CONFIG_CC_IMPLICIT_FALLTHROUGH:"%"=%) solution: it's short.
> 
> Does the following look correct, as well from formal style/commit
> description? I have not yet done many contributions directly.
> 
> Regards,
> Salvatore
> 
> From c2d01ea3ee1c7cc539468bba5b25522245d513de Mon Sep 17 00:00:00 2001
> From: Salvatore Bonaccorso <carnil@debian.org>
> Date: Mon, 6 Dec 2021 21:42:01 +0100
> Subject: [PATCH] Makefile: Do not quote value for
>  CONFIG_CC_IMPLICIT_FALLTHROUGH
> 
> Andreas reported that a specific build environment for an external
> module, being a bit broken, does pass CC_IMPLICIT_FALLTHROUGH quoted as
> argument to gcc, causing an error
> 
> 	gcc-11: error: "-Wimplicit-fallthrough=5": linker input file not found: No such file or directory
> 
> Until this is more generally fixed as outlined in [1], by fixing
> scripts/link-vmlinux.sh, scripts/gen_autoksyms.sh, etc to not directly
> include the include/config/auto.conf, and in a second step, change
> Kconfig to generate the auto.conf without "", workaround the issue by
> explicitly unquoting CC_IMPLICIT_FALLTHROUGH.
> 
>  [1] https://lore.kernel.org/linux-kbuild/CAK7LNAR-VXwHFEJqCcrFDZj+_4+Xd6oynbj_0eS8N504_ydmyw@mail.gmail.com/
> 
> Reported-by: Andreas Beckmann <anbe@debian.org>
> Link: https://bugs.debian.org/1001083
> Signed-off-by: Salvatore Bonaccorso <carnil@debian.org>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 8e35d7804fef..ef967a26bcd3 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -789,7 +789,7 @@ stackp-flags-$(CONFIG_STACKPROTECTOR_STRONG)      := -fstack-protector-strong
>  KBUILD_CFLAGS += $(stackp-flags-y)
>  
>  KBUILD_CFLAGS-$(CONFIG_WERROR) += -Werror
> -KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
> +KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH:"%"=%)
>  
>  ifdef CONFIG_CC_IS_CLANG
>  KBUILD_CPPFLAGS += -Qunused-arguments
> -- 
> 2.34.1
> 

Re: Makefile: CC_IMPLICIT_FALLTHROUGH passed quoted as argument to gcc

[Gustavo A. R. Silva]

On Mon, Dec 06, 2021 at 11:02:05PM +0100, Salvatore Bonaccorso wrote:
> Hi,
> 
> On Mon, Dec 06, 2021 at 11:53:41AM -0800, Kees Cook wrote:
> > On Sun, Dec 05, 2021 at 02:54:05AM +0900, Masahiro Yamada wrote:
> > > On Sun, Dec 5, 2021 at 1:53 AM Linus Torvalds
> > > <torvalds@linux-foundation.org> wrote:
> > > >
> > > > On Sat, Dec 4, 2021 at 5:13 AM Salvatore Bonaccorso <carnil@debian.org> wrote:
> > > > >
> > > > > Andreas suggested to replace the
> > > > >
> > > > > KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
> > > > >
> > > > > with
> > > > >
> > > > > KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(patsubst "%",%,$(CONFIG_CC_IMPLICIT_FALLTHROUGH))
> > > >
> > > > Ugh. I think the external build environment is a bit broken, but
> > > > whatever. The above is ugly but I guess it works.
> > > >
> > > > Another alternative would be to make the Kconfig strings simply not
> > > > have '"' as part of them.
> > > >
> > > > When you do
> > > >
> > > >     a = "hello"
> > > >     print $a
> > > >
> > > > in any normal language, you generally wouldn't expect it to print the
> > > > quotes, it should just print the bare word.
> > > >
> > > > But that's what the Kconfig string language basically does in this
> > > > case. And I guess several users expect and take advantage of that ;(
> > > >
> > > > Masahiro? Comments?
> > > 
> > > Yes, you get to the point.
> > > 
> > > In fact, this is in my TODO list for a while
> > > (and this is the reason I was doing prerequisite Kconfig refactoring
> > > in the previous development cycle).
> > > I will try to find some spare time to complete this work.
> > > 
> > > 
> > > 
> > > Kconfig generates two similar files,
> > > 
> > >  -   .config
> > >  -   include/config/auto.conf
> > > 
> > > Changing the format of the .config is presumably problematic
> > > since it is the saved user configuration as well.
> > > 
> > > It is possible (and more reasonable) to change include/config/auto.conf
> > > so strings are not quoted.
> > > 
> > > In Makefiles, quotations are just normal characters; they have no
> > > special functionality.
> > > 
> > > So, in Makefile context, it is more handy to do
> > > 
> > >      CONFIG_X=foo bar
> > > 
> > > instead of
> > > 
> > >     CONFIG_X="foo bar"
> > > 
> > > 
> > > 
> > > One problem is include/config/auto.conf is included not only by Makefiles
> > > but also by shell scripts.
> > > 
> > > 
> > > In shell context, the right hand side must be quoted
> > > in case the value contains spaces.
> > > 
> > >    CONFIG_X="foo bar"
> > > 
> > > 
> > > 
> > > My plan is to fix
> > >   scripts/link-vmlinux.sh
> > >   scripts/gen_autoksyms.sh
> > > etc. to not directly include the auto.conf.
> > > Later, change Kconfig to generate the auto.conf without "".
> > > 
> > > 
> > > 
> > > In the meantime,
> > > 
> > > KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(patsubst
> > > "%",%,$(CONFIG_CC_IMPLICIT_FALLTHROUGH))
> > > 
> > >  or if you prefer slightly shorter form,
> > > 
> > > KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH:"%"=%)
> > > 
> > > will be a workaround.
> > 
> > It'll be nice to get this fixed. There are a few places where there is
> > a test for a compiler flag in Kconfig, and then the option is repeated
> > in the Makefile, due to the above quoting issues. For example:
> > 
> > arch/arm64/Kconfig:
> > 	config CC_HAS_BRANCH_PROT_PAC_RET
> > 	     # GCC 9 or later, clang 8 or later
> > 	     def_bool $(cc-option,-mbranch-protection=pac-ret+leaf)
> > 
> > arch/arm64/Makefile:
> > 	branch-prot-flags-$(CONFIG_CC_HAS_BRANCH_PROT_PAC_RET) := -mbranch-protection=pac-ret+leaf
> > 
> > 
> > I like the $(CONFIG_CC_IMPLICIT_FALLTHROUGH:"%"=%) solution: it's short.
> 
> Does the following look correct, as well from formal style/commit
> description? I have not yet done many contributions directly.
> 
> Regards,
> Salvatore
> 
> From c2d01ea3ee1c7cc539468bba5b25522245d513de Mon Sep 17 00:00:00 2001
> From: Salvatore Bonaccorso <carnil@debian.org>
> Date: Mon, 6 Dec 2021 21:42:01 +0100
> Subject: [PATCH] Makefile: Do not quote value for
>  CONFIG_CC_IMPLICIT_FALLTHROUGH
> 
> Andreas reported that a specific build environment for an external
> module, being a bit broken, does pass CC_IMPLICIT_FALLTHROUGH quoted as
> argument to gcc, causing an error
> 
> 	gcc-11: error: "-Wimplicit-fallthrough=5": linker input file not found: No such file or directory
> 
> Until this is more generally fixed as outlined in [1], by fixing
> scripts/link-vmlinux.sh, scripts/gen_autoksyms.sh, etc to not directly
> include the include/config/auto.conf, and in a second step, change
> Kconfig to generate the auto.conf without "", workaround the issue by
> explicitly unquoting CC_IMPLICIT_FALLTHROUGH.
> 
>  [1] https://lore.kernel.org/linux-kbuild/CAK7LNAR-VXwHFEJqCcrFDZj+_4+Xd6oynbj_0eS8N504_ydmyw@mail.gmail.com/
> 
> Reported-by: Andreas Beckmann <anbe@debian.org>
> Link: https://bugs.debian.org/1001083
> Signed-off-by: Salvatore Bonaccorso <carnil@debian.org>

Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org>

Thanks
--
Gustavo

> ---
>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 8e35d7804fef..ef967a26bcd3 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -789,7 +789,7 @@ stackp-flags-$(CONFIG_STACKPROTECTOR_STRONG)      := -fstack-protector-strong
>  KBUILD_CFLAGS += $(stackp-flags-y)
>  
>  KBUILD_CFLAGS-$(CONFIG_WERROR) += -Werror
> -KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH)
> +KBUILD_CFLAGS += $(KBUILD_CFLAGS-y) $(CONFIG_CC_IMPLICIT_FALLTHROUGH:"%"=%)
>  
>  ifdef CONFIG_CC_IS_CLANG
>  KBUILD_CPPFLAGS += -Qunused-arguments
> -- 
> 2.34.1
> 

Re: Makefile: CC_IMPLICIT_FALLTHROUGH passed quoted as argument to gcc

[Linus Torvalds]

On Mon, Dec 6, 2021 at 2:02 PM Salvatore Bonaccorso <carnil@debian.org> wrote:
>
> Does the following look correct, as well from formal style/commit
> description? I have not yet done many contributions directly.

Thanks, looks good, applied.

Hopefully we'll get this cleaned up at a Kconfig level at some point,
but at least this avoids the external build environment issue for now.

             Linus