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. ;)
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
