Message-ID: <20180302091201.veyw5iv2uwvlvs2h@huvuddator>
Date:   Fri, 2 Mar 2018 10:12:01 +0100
From:   Ulf Magnusson <ulfalizer@...il.com>
To:     Masahiro Yamada <yamada.masahiro@...ionext.com>
Cc:     Arnd Bergmann <arnd@...db.de>, Rich Felker <dalias@...c.org>,
        Kernel Hardening <kernel-hardening@...ts.openwall.com>,
        X86 ML <x86@...nel.org>, Paul Mackerras <paulus@...ba.org>,
        "H. Peter Anvin" <hpa@...or.com>,
        sparclinux <sparclinux@...r.kernel.org>,
        Sam Ravnborg <sam@...nborg.org>,
        Yoshinori Sato <ysato@...rs.sourceforge.jp>,
        Jonathan Corbet <corbet@....net>,
        Richard Weinberger <richard@....at>,
        Linux-sh list <linux-sh@...r.kernel.org>,
        Ingo Molnar <mingo@...hat.com>,
        Emese Revfy <re.emese@...il.com>,
        Kees Cook <keescook@...omium.org>,
        uml-devel <user-mode-linux-devel@...ts.sourceforge.net>,
        Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>,
        Peter Oberparleiter <oberpar@...ux.vnet.ibm.com>,
        Jeff Dike <jdike@...toit.com>,
        linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>,
        user-mode-linux-user@...ts.sourceforge.net,
        Thomas Gleixner <tglx@...utronix.de>,
        Michal Marek <michal.lkml@...kovi.net>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Randy Dunlap <rdunlap@...radead.org>,
        "open list:DOCUMENTATION" <linux-doc@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        "David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH 00/23] kconfig: move compiler capability tests to Kconfig

On Fri, Mar 02, 2018 at 10:03:26AM +0100, Ulf Magnusson wrote:
> On Fri, Mar 02, 2018 at 02:50:39PM +0900, Masahiro Yamada wrote:
> > 2018-02-22 6:39 GMT+09:00 Ulf Magnusson <ulfalizer@...il.com>:
> > > On Wed, Feb 21, 2018 at 09:57:03PM +0900, Masahiro Yamada wrote:
> > >> 2018-02-21 19:52 GMT+09:00 Arnd Bergmann <arnd@...db.de>:
> > >> > On Wed, Feb 21, 2018 at 11:20 AM, Masahiro Yamada
> > >> > <yamada.masahiro@...ionext.com> wrote:
> > >> >> 2018-02-21 18:56 GMT+09:00 Arnd Bergmann <arnd@...db.de>:
> > >> >>> On Wed, Feb 21, 2018 at 8:38 AM, Masahiro Yamada
> > >> >>> <yamada.masahiro@...ionext.com> wrote:
> > >> >>>> 2018-02-20 0:18 GMT+09:00 Ulf Magnusson <ulfalizer@...il.com>:
> > >> >>
> > >> >> Let me clarify my concern.
> > >> >>
> > >> >> When we test the compiler flag, is there a case
> > >> >> where a particular flag depends on -m{32,64} ?
> > >> >>
> > >> >> For example, is there a compiler that supports -fstack-protector
> > >> >> for 64bit mode, but unsupports it for 32bit mode?
> > >> >>
> > >> >>   $(cc-option -m32)                     ->  y
> > >> >>   $(cc-option -m64)                     ->  y
> > >> >>   $(cc-option -fstack-protector)        ->  y
> > >> >>   $(cc-option -m32 -fstack-protector)   ->  n
> > >> >>   $(cc-option -m64 -fstack-protector)   ->  y
> > >> >>
> > >> >> I guess this is unlikely to happen,
> > >> >> but I am not whether it is zero possibility.
> > >> >>
> > >> >> If this could happen,
> > >> >> $(cc-option ) must be evaluated together with
> > >> >> correct bi-arch option (either -m32 or -m64).
> > >> >>
> > >> >>
> > >> >> Currently, -m32/-m64 is specified in Makefile,
> > >> >> but we are moving compiler tests to Kconfig
> > >> >> and, CONFIG_64BIT can be dynamically toggled in Kconfig.
> > >> >
> > >> > I don't think it can happen for this particular combination (stack protector
> > >> > and word size), but I'm sure we'll eventually run into options that
> > >> > need to be tested in combination. For the current CFLAGS_KERNEL
> > >> > setting, we definitely have the case of needing the variables to be
> > >> > evaluated in a specific order.
> > >> >
> > >>
> > >>
> > >>
> > >>
> > >> I was thinking of how we can handle complex cases
> > >> in the current approach.
> > >>
> > >>
> > >>
> > >> (Case 1)
> > >>
> > >> Compiler flag -foo and -bar interacts, so
> > >> we also need to check the combination of the two.
> > >>
> > >>
> > >> config CC_HAS_FOO
> > >>         def_bool $(cc-option -foo)
> > >>
> > >> config CC_HAS_BAR
> > >>         def_bool $(cc-option -bar)
> > >>
> > >> config CC_HAS_FOO_WITH_BAR
> > >>         def_bool $(cc-option -foo -bar)
> > >>
> > >>
> > >>
> > >> (Case 2)
> > >> Compiler flag -foo is sensitive to word-size.
> > >> So, we need to test this option together with -m32/-m64.
> > >> User can toggle CONFIG_64BIT, like i386/x86_64.
> > >>
> > >>
> > >> config CC_NEEDS_M64
> > >>           def_bool $(cc-option -m64) && 64BIT
> > >>
> > >> config CC_NEEDS_M32
> > >>           def_bool $(cc-option -m32) && !64BIT
> > >>
> > >> config CC_HAS_FOO
> > >>          bool
> > >>          default $(cc-option -m64 -foo) if CC_NEEDS_M64
> > >>          default $(cc-option -m32 -foo) if CC_NEEDS_M32
> > >>          default $(cc-option -foo)
> > >>
> > >>
> > >>
> > >> (Case 3)
> > >> Compiler flag -foo is sensitive to endian-ness.
> > >>
> > >>
> > >> config CC_NEEDS_BIG_ENDIAN
> > >>           def_bool $(cc-option -mbig-endian) && CPU_BIG_ENDIAN
> > >>
> > >> config CC_NEEDS_LITTLE_ENDIAN
> > >>           def_bool $(cc-option -mlittle-endian) && CPU_LITTLE_ENDIAN
> > >>
> > >> config CC_HAS_FOO
> > >>          bool
> > >>          default $(cc-option -mbig-endian -foo) if CC_NEEDS_BIG_ENDIAN
> > >>          default $(cc-option -mlittle-endian -foo) if CC_NEEDS_LITTLE_ENDIAN
> > >>          default $(cc-option -foo)
> > >>
> > >>
> > >>
> > >>
> > >> Hmm, I think I can implement those somehow.
> > >> But, I hope we do not have many instances like this...
> > >>
> > >>
> > >> If you know more naive cases, please share your knowledge.
> > >>
> > >> Thanks!
> > >>
> > >>
> > >> --
> > >> Best Regards
> > >> Masahiro Yamada
> > >
> > > Would get pretty bad if a test needs to consider multiple symbols.
> > > Exponential explosion there...
> > >
> > >
> > > I thought some more about the implementation of dynamic (post-parsing)
> > > functions to see how bad it would get with the current implementation.
> > >
> > > Some background on how things work now:
> > >
> > >   1. All expression operands in Kconfig are symbols.
> > >
> > >   2. Returning '$ENV' or '$(fn foo)' as a T_WORD during parsing gets
> > >      you symbols with those strings as names and S_UNKNOWN type (because
> > >      they act like references to undefined symbols).
> > >
> > >   3. For "foo-$(fn foo)", you also get a symbol with that string as its
> > >      name and S_UNKNOWN type (stored among the SYMBOL_CONST symbols)
> > >
> > >   4. Symbols with S_UNKNOWN type get their name as their string value,
> > >      and the tristate value n.
> > >
> > > So, if you do string expansion on the names of symbols with S_UNKNOWN
> > > type in sym_calc_value(), you're almost there with the current
> > > implementation, except for the tristate case.
> > >
> > > Maybe you could set the tristate value of S_UNKNOWN symbols depending on
> > > the string value you end up with. Things are getting pretty confusing at
> > > that point.
> > >
> > > Could have something like S_DYNAMIC as well. More Kconfig complexity...
> > >
> > > Then there's other complications:
> > >
> > >   1. SYMBOL_CONST is no longer constant.
> > >
> > >   2. Dependency loop detection needs to consider symbol references
> > >      within strings.
> > >
> > >   3. Dependency loop detection relies on static knowledge of what
> > >      symbols a symbol depends on. That might get messy for certain
> > >      expansions, though it might be things you wouldn't do in practice.
> > >
> > >   4. Symbols still need to be properly invalidated. It looks like at
> > >      least menuconfig just does a dumb invalidate-everything whenever
> > >      the value of a symbol is changed though, so it might not require
> > >      extra work. (Bit messier in Kconfiglib, which does minimal
> > >      invalidation to keep scripts fast, but just need to extract a few
> > >      extra deps there.)
> > >
> > >
> > > It looks like dynamic functions could get quite messy, but might be
> > > doable if absolutely required. There's probably more devils in the
> > > details though.
> > >
> > > I don't think the static function model precludes switching models later
> > > btw, when people have more experience.
> > 
> > 
> > 
> > I really want to start with the static function model
> > and see if we need the dynamic function implementation.
> 
> Yeah, let's start with static functions, IMO.
> 
> Either we'll learn that they're powerful enough in practice, and save
> ourselves some work, or we'll gain experience for later. Converting from
> static to dynamic functions should be painless, if needed.
> 
> My plan would be something like:
> 
>   1. Implement static functions
> 
>   2. Convert as many simple cases over to them as possible
> 
>   3. See how bad the bad cases get. If they get really bad, then decide
>      what to do next (extend Kconfig, handle them in the Makefiles,
>      etc.)
> 
> > 
> > Here is an idea for the migration path in case
> > we need to do that in the future.
> > 
> > 
> > 
> > Currently, every time user input is given,
> > sym_clear_all_valid() is called.
> > 
> > It is not efficient to blindly re-evaluate expensive $(shell ...)
> 
> I think menuconfig only reevalutes the symbols in the menu that's
> currently shown in the interface (along with their dependencies).
> 
> Maybe that'd be bad enough though.
> 
> > 
> > 
> > So, have a list of symbols the function depends on
> > in its arguments.
> > 
> > For example,
> > 
> > config CC_HAS_SANE_STACKPROTECTOR
> >         def_bool $(shell $srctree/scripts/gcc-has-stack-protector.sh
> > $CC $(1), CFLAGS_BASE)
> > 
> > 
> > Here the first argument
> >   $srctree/scripts/gcc-x86-has-stack-protector.sh $CC $(1)
> > 
> > is the shell command.
> > $(1), $(2), ... will be replaced with the values of symbols (or expressions)
> > that follow when running the shell command.
> > 
> > 
> > The second argument
> > CFLAGS_BASE
> > is the dependency symbol (or expression).
> > 
> > 
> > CFLAGS_BASE can be dynamically changed like
> > 
> > config CFLAGS_BASE
> >         string
> >         default "-m64" if 64BIT
> >         default "-m32"
> > 
> > 
> > When and only when CFLAGS_BASE is updated, the function should be re-calculated.
> > (This will require efforts to minimize the amount of re-evaluation.)
> > 
> > 
> > 
> > 
> > cc-option will be implemented like follows:
> > 
> > macro cc-option $(shell $CC -Werror $$(1) $(1) -c -x c /dev/null -o
> > /dev/null, CFLAGS_BASE)
> > 
> > 
> > 
> > Please notice the difference between $$(1) and $(1).
> > 
> > $(1) is immediately expanded by cc-option macro.
> > 
> > $$(1) is escaped since we want to expand it by $(shell ...), not by
> > $(cc-option ...)
> > 
> > 
> > 
> > For example,
> > 
> >   $(cc-option -fstack-protector)
> > 
> > will be expanded to
> > 
> >   $(shell gcc -Werror $(1) -fstack-protector -c -x c /dev/null -o
> > /dev/null, CFLAGS_BASE)
> > 
> > Since macros are just textual shorthand, so this expansion happens
> > during the parse phase.
> > 
> > 
> > 
> > Then, the evaluation phase does the following every time CFLAGS_BASE is updated.
> > 
> > gcc -Werror [value of CFLAGS_BASE] -fstack-protector -c -x c /dev/null
> > -o /dev/null
> > 
> > 
> > This is a new form of expression, so it will be managed in AST tree
> > with a flag E_SHELL (or E_FUNC) etc.
> > 
> > 
> > Not implemented at all.  Just a rough sketch.
> 
> A simpler syntax like
> 
> 	$(shell $CC -Werror {CFLAGS_BASE} -c -x c /dev/null -o /dev/null)

	*$(shell $CC -Werror {CFLAGS_BASE} -fstack-protector -c -x c /dev/null -o /dev/null)

Powered by blists - more mailing lists