* Re: Makefile: CC_IMPLICIT_FALLTHROUGH passed quoted as argument to gcc
2021-12-06 22:02 ` Salvatore Bonaccorso
@ 2021-12-06 22:54 ` Kees Cook
2021-12-06 23:01 ` Nathan Chancellor
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Kees Cook @ 2021-12-06 22:54 UTC (permalink / raw)
To: Salvatore Bonaccorso
Cc: Masahiro Yamada, Linus Torvalds, Gustavo A. R. Silva,
Nathan Chancellor, Michal Marek, Linux Kbuild mailing list,
Linux Kernel Mailing List
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
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: Makefile: CC_IMPLICIT_FALLTHROUGH passed quoted as argument to gcc
2021-12-06 22:02 ` Salvatore Bonaccorso
2021-12-06 22:54 ` Kees Cook
@ 2021-12-06 23:01 ` Nathan Chancellor
2021-12-06 23:51 ` Gustavo A. R. Silva
2021-12-07 0:46 ` Linus Torvalds
3 siblings, 0 replies; 9+ messages in thread
From: Nathan Chancellor @ 2021-12-06 23:01 UTC (permalink / raw)
To: Salvatore Bonaccorso
Cc: Kees Cook, Masahiro Yamada, Linus Torvalds, Gustavo A. R. Silva,
Michal Marek, Linux Kbuild mailing list,
Linux Kernel Mailing List
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
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Makefile: CC_IMPLICIT_FALLTHROUGH passed quoted as argument to gcc
2021-12-06 22:02 ` Salvatore Bonaccorso
2021-12-06 22:54 ` Kees Cook
2021-12-06 23:01 ` Nathan Chancellor
@ 2021-12-06 23:51 ` Gustavo A. R. Silva
2021-12-07 0:46 ` Linus Torvalds
3 siblings, 0 replies; 9+ messages in thread
From: Gustavo A. R. Silva @ 2021-12-06 23:51 UTC (permalink / raw)
To: Salvatore Bonaccorso
Cc: Kees Cook, Masahiro Yamada, Linus Torvalds, Nathan Chancellor,
Michal Marek, Linux Kbuild mailing list,
Linux Kernel Mailing List
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
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: Makefile: CC_IMPLICIT_FALLTHROUGH passed quoted as argument to gcc
2021-12-06 22:02 ` Salvatore Bonaccorso
` (2 preceding siblings ...)
2021-12-06 23:51 ` Gustavo A. R. Silva
@ 2021-12-07 0:46 ` Linus Torvalds
3 siblings, 0 replies; 9+ messages in thread
From: Linus Torvalds @ 2021-12-07 0:46 UTC (permalink / raw)
To: Salvatore Bonaccorso
Cc: Kees Cook, Masahiro Yamada, Gustavo A. R. Silva,
Nathan Chancellor, Michal Marek, Linux Kbuild mailing list,
Linux Kernel Mailing List
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
^ permalink raw reply [flat|nested] 9+ messages in thread