On Mon, Feb 12, 2018 at 12:44 PM, Ulf Magnusson <ulfalizer@xxxxxxxxx> wrote: > On Mon, Feb 12, 2018 at 11:44 AM, Masahiro Yamada > <yamada.masahiro@xxxxxxxxxxxxx> wrote: >> 2018-02-11 19:34 GMT+09:00 Ulf Magnusson <ulfalizer@xxxxxxxxx>: >>> Looks to me like there's a few unrelated issues here: >>> >>> >>> 1. The stack protector support test scripts >>> >>> Worthwhile IMO if they (*in practice*) prevent hard-to-debug build errors or a >>> subtly broken kernel from being built. >>> >>> A few questions: >>> >>> - How do things fail with a broken stack protector implementation? >>> >>> - How common are those broken compilers? >>> >>> - Do you really need to pass $(KBUILD_CPPFLAGS) when testing for breakage, >>> or would a simpler static test work in practice? >>> >>> I don't know how messy it would be to get $(KBUILD_CPPFLAGS) into >>> Kconfig, but should make sure it's actually needed in any case. >>> >>> The scripts are already split up as >>> >>> scripts/gcc-$(SRCARCH)_$(BITS)-has-stack-protector.sh >>> >>> by the way, though only gcc-x86_32-has-stack-protector.sh and >>> gcc-x86_64-has-stack-protector.sh exist. >>> >>> - How old do you need to go with GCC for -fno-stack-protector to give an >>> error (i.e., for not even the option to be recognized)? Is it still >>> warranted to test for it? >>> >>> Adding some CCs who worked on the stack protector test scripts. >>> >>> And yeah, I was assuming that needing support scripts would be rare, and that >>> you'd usually just check whether gcc accepts the flag. >>> >>> When you Google "gcc broken stack protector", the top hits about are about the >>> scripts/gcc-x86_64-has-stack-protector.sh script in the kernel throwing a false >>> positive by the way (fixed in 82031ea29e45 ("scripts/has-stack-protector: add >>> -fno-PIE")). >>> >>> >>> 2. Whether to hide the Kconfig stack protector alternatives or always show them >>> >>> Or equivalently, whether to automatically fall back on other stack protector >>> alternatives (including no stack protector) if the one specified in the .config >>> isn't available. >>> >>> I'll let you battle this one out. In any case, as a user, I'd want a >>> super-clear message telling me what to change if the build breaks because of >>> missing stack protector support. >>> >>> >>> 3. Whether to implement CC_STACKPROTECTOR_AUTO in Kconfig or the Makefiles >>> >>> I'd just go with whatever is simplest here. I don't find the Kconfig version >>> too bad, but I'm already very familiar with Kconfig, so it's harder for me to >>> tell how it looks to other people. >>> >>> I'd add some comments to explain the idea in the final version. >>> >>> @Kees: >>> I was assuming that the Makefiles would error out with a message if none of the >>> CC_STACKPROTECTOR_* variables are set, in addition to the Kconfig warning. >>> >>> You could offload part of that check to Kconfig with something like >>> >>> config CHOSEN_CC_STACKPROTECTOR_AVAILABLE >>> def_bool CC_STACKPROTECTOR_STRONG || \ >>> CC_STACKPROTECTOR_REGULAR || \ >>> CC_STACKPROTECTOR_NONE >>> >>> CHOSEN_STACKPROTECTOR_AVAILABLE could then be checked in the Makefile. >>> It has the advantage of making the constraint clear in the Kconfig file >>> at least. >>> >>> You could add some kind of assert feature to Kconfig too, but IMO it's not >>> warranted purely for one-offs like this at least. >>> >>> That's details though. I'd want to explain it with a comment in any case if we >>> go with something like this, since it's slightly kludgy and subtle >>> (CC_STACKPROTECTOR_{STRONG,REGULAR,NONE} form a kind of choice, only you can't >>> express it like that directly, since it's derived from other symbols). >>> >>> >>> Here's an overview of the current Kconfig layout by the way, assuming >>> the old no-fallback behavior and CC_STACKPROTECTOR_AUTO being >>> implemented in Kconfig: >>> >>> # Feature tests >>> CC_HAS_STACKPROTECTOR_STRONG >>> CC_HAS_STACKPROTECTOR_REGULAR >>> CC_HAS_STACKPROTECTOR_NONE >>> >>> # User request >>> WANT_CC_STACKPROTECTOR_AUTO >>> WANT_CC_STACKPROTECTOR_STRONG >>> WANT_CC_STACKPROTECTOR_REGULAR >>> WANT_CC_STACKPROTECTOR_NONE >>> >>> # The actual "output" to the Makefiles >>> CC_STACKPROTECTOR_STRONG >>> CC_STACKPROTECTOR_REGULAR >>> CC_STACKPROTECTOR_NONE >>> >>> # Some possible output "nicities" >>> CHOSEN_CC_STACKPROTECTOR_AVAILABLE >>> CC_OPT_STACKPROTECTOR >>> >>> Does anyone have objections to the naming or other things? I saw some >>> references to "Santa's wish list" in messages of commits that dealt with other >>> variables named WANT_*, though I didn't look into those cases. ;) >>> >> >> >> >> I think Linus's comment was dismissed here. >> >> >> Linus said: >> >>> But yes, I also reacted to your earlier " It can't silently rewrite it >>> to _REGULAR because the compiler support for _STRONG regressed." >>> Because it damn well can. If the compiler doesn't support >>> -fstack-protector-strong, we can just fall back on -fstack-protector. >>> Silently. No extra crazy complex logic for that either. >> >> >> If I understood his comment correctly, >> we do not need either WANT_* or _AUTO. >> >> >> >> >> Kees' comment: >> >>> In the stack-protector case, this becomes quite important, since the >>> goal is to record the user's selection regardless of compiler >>> capability. For example, if someone selects _REGULAR, it shouldn't >>> "upgrade" to _STRONG. (Similarly for _NONE.) >> >> No. Kconfig will not do this silently. > > (Playing devil's advocate...) > > If the user selected _STRONG and it becomes unavailable later, it > seems to silently fall back on other options, even for oldnoconfig > (which just checks if there are any new symbols in the choice). > > It would be possible to also check if the old user selection still > applies btw. I do that in Kconfiglib. It's arguable whether that > matches the intent of oldnoconfig. *oldconfig These things are so closely named. :P Cheers, Ulf -- To unsubscribe from this list: send the line "unsubscribe linux-s390" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html