On Tue, Oct 08, 2024 at 07:14:26AM +0200, Christophe Leroy wrote:
Le 08/10/2024 à 06:22, Nathan Chancellor a écrit :
quoted
Kbuild uses the powerpc64le-linux-gnu target for clang, which causes the
Kconfig check for 32-bit powerpc stack protector support to fail because
nothing flips the target to 32-bit:
$ clang --target=powerpc64le-linux-gnu \
-mstack-protector-guard=tls
-mstack-protector-guard-reg=r2 \
-mstack-protector-guard-offset=0 \
-x c -c -o /dev/null /dev/null
clang: error: invalid value 'r2' in 'mstack-protector-guard-reg=', expected one of: r13
Why is there any restriction at all on which register can be used ? I can't
see such restriction in GCC documentation :
https://gcc.gnu.org/onlinedocs/gcc/RS_002f6000-and-PowerPC-Options.html
Keith may be able to answer this better than I can but I believe this is
an existing restriction in the LLVM code that dates back to the original
stack protector support:
https://github.com/llvm/llvm-project/commit/a1d8bc559761c94dded3d1e7200cb264a982d1c5
It always uses r2 for 32-bit and r13 for 64-bit for loading the stack
canary. Keith's patch basically just allows customizing the offset,
rather than the register (ultimately for simplicity in the compiler
patch I believe, since we really only care about supporting what the
kernel uses).
quoted
Use the Kconfig macro '$(m32-flag)', which expands to '-m32' when
supported, in the stack protector support cc-option call to properly
switch the target to a 32-bit one, which matches what happens in Kbuild.
While the 64-bit macro does not strictly need it, add the equivalent
64-bit option for symmetry.
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
---
arch/powerpc/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index eb98050b8c016bb23887a9d669d29e69d933c9c8..6aaca48955a34b2a38af1415bfa36f74f35c3f3e 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -275,8 +275,8 @@ config PPC
select HAVE_RSEQ
select HAVE_SETUP_PER_CPU_AREA if PPC64
select HAVE_SOFTIRQ_ON_OWN_STACK
- select HAVE_STACKPROTECTOR if PPC32 && $(cc-option,-mstack-protector-guard=tls -mstack-protector-guard-reg=r2 -mstack-protector-guard-offset=0)
- select HAVE_STACKPROTECTOR if PPC64 && $(cc-option,-mstack-protector-guard=tls -mstack-protector-guard-reg=r13 -mstack-protector-guard-offset=0)
+ select HAVE_STACKPROTECTOR if PPC32 && $(cc-option,$(m32-flag) -mstack-protector-guard=tls -mstack-protector-guard-reg=r2 -mstack-protector-guard-offset=0)
+ select HAVE_STACKPROTECTOR if PPC64 && $(cc-option,$(m64-flag) -mstack-protector-guard=tls -mstack-protector-guard-reg=r13 -mstack-protector-guard-offset=0)
You modify the exact same line than Patch 1, if this patch is really
required it should be squashed into patch 1 I think.
I believe it will still be required based on my comment above. I can
certainly squash it into patch 1, although the commit message might get
a bit wordy with both explanations in there. I suppose it still makes
sense to do so since "fix the Kconfig test" can encompass both issues
easily.
Cheers,
Nathan