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
On Sat, Dec 4, 2021 at 5:13 AM Salvatore Bonaccorso <[email protected]> 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
On Sun, Dec 5, 2021 at 1:53 AM Linus Torvalds
<[email protected]> wrote:
>
> On Sat, Dec 4, 2021 at 5:13 AM Salvatore Bonaccorso <[email protected]> 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
On Sun, Dec 05, 2021 at 02:54:05AM +0900, Masahiro Yamada wrote:
> On Sun, Dec 5, 2021 at 1:53 AM Linus Torvalds
> <[email protected]> wrote:
> >
> > On Sat, Dec 4, 2021 at 5:13 AM Salvatore Bonaccorso <[email protected]> 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
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
> > <[email protected]> wrote:
> > >
> > > On Sat, Dec 4, 2021 at 5:13 AM Salvatore Bonaccorso <[email protected]> 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 <[email protected]>
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 <[email protected]>
Link: https://bugs.debian.org/1001083
Signed-off-by: Salvatore Bonaccorso <[email protected]>
---
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
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
> > > <[email protected]> wrote:
> > > >
> > > > On Sat, Dec 4, 2021 at 5:13 AM Salvatore Bonaccorso <[email protected]> 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 <[email protected]>
> 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 <[email protected]>
> Link: https://bugs.debian.org/1001083
> Signed-off-by: Salvatore Bonaccorso <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
Does anyone have a preference as to who should take this? Gustavo,
Marahiro, me, or Linus directly?
--
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
> > > <[email protected]> wrote:
> > > >
> > > > On Sat, Dec 4, 2021 at 5:13 AM Salvatore Bonaccorso <[email protected]> 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 <[email protected]>
> 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 <[email protected]>
> Link: https://bugs.debian.org/1001083
> Signed-off-by: Salvatore Bonaccorso <[email protected]>
Reviewed-by: Nathan Chancellor <[email protected]>
> ---
> 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
>
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
> > > <[email protected]> wrote:
> > > >
> > > > On Sat, Dec 4, 2021 at 5:13 AM Salvatore Bonaccorso <[email protected]> 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 <[email protected]>
> 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 <[email protected]>
> Link: https://bugs.debian.org/1001083
> Signed-off-by: Salvatore Bonaccorso <[email protected]>
Reviewed-by: Gustavo A. R. Silva <[email protected]>
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
>
On Mon, Dec 6, 2021 at 2:02 PM Salvatore Bonaccorso <[email protected]> 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