lkml.org 
[lkml]   [2021]   [Dec]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: Makefile: CC_IMPLICIT_FALLTHROUGH passed quoted as argument to gcc
    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

    \
     
     \ /
      Last update: 2021-12-06 23:55    [W:4.308 / U:0.040 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site