Re: [PATCH 2/3] powerpc: Adjust adding stack protector flags to KBUILD_CLAGS for clang

[Nathan Chancellor <nathan@kernel.org>]

Hi Christophe,

First of all, thanks a lot for the quick review.

On Tue, Oct 08, 2024 at 07:10:36AM +0200, Christophe Leroy wrote:
> Le 08/10/2024 à 06:22, Nathan Chancellor a écrit :
> > After fixing the HAVE_STACKPROTECTER checks for clang's in-progress
> > per-task stack protector support [1], the build fails during prepare0
> > because '-mstack-protector-guard-offset' has not been added to
> > KBUILD_CFLAGS yet but the other '-mstack-protector-guard' flags have.
> > 
> >    clang: error: '-mstack-protector-guard=tls' is used without '-mstack-protector-guard-offset', and there is no default
> >    clang: error: '-mstack-protector-guard=tls' is used without '-mstack-protector-guard-offset', and there is no default
> >    make[4]: *** [scripts/Makefile.build:229: scripts/mod/empty.o] Error 1
> >    make[4]: *** [scripts/Makefile.build:102: scripts/mod/devicetable-offsets.s] Error 1
> > 
> > Mirror other architectures and add all '-mstack-protector-guard' flags
> > to KBUILD_CFLAGS atomically during stack_protector_prepare, which
> > resolves the issue and allows clang's implementation to fully work with
> > the kernel.
> > 
> > Link: https://github.com/llvm/llvm-project/pull/110928 [1]
> > Signed-off-by: Nathan Chancellor <nathan@kernel.org>
> 
> Is it worth a Fixes: tag so that it gets applied on stable, or wont CLANG 20
> be used with kernels older than 6.13 ?

Yes, I did consider adding Fixes: + Cc: stable. I think it is probably
worth doing since the fixes are rather benign. Do you have a suggestion
for a good Fixes: commit? It always feels somewhat wrong to blame a
commit for not having foresight into how a future implementation might
do things differently :)

> > -	$(eval KBUILD_CFLAGS += -mstack-protector-guard-offset=$(shell awk '{if ($$2 == "PACA_CANARY") print $$3;}' include/generated/asm-offsets.h))
> > +	$(eval KBUILD_CFLAGS += -mstack-protector-guard=tls \
> > +				-mstack-protector-guard-reg=r13 \
> 
> Can you put both above lines on a single line to avoid having too many lines
> at the end:
> 
> +	$(eval KBUILD_CFLAGS += -mstack-protector-guard=tls -mstack-protector-guard-reg=r13 \

Ack, I have made this change locally for v2.

Cheers,
Nathan