[PATCH kernel] powerpc/makefile: Do not redefine $(CPP) for preprocessor

[Alexey Kardashevskiy]

On 23/04/2021 08:58, Daniel Axtens wrote:
> Hi Alexey,
> 
>> The $(CPP) (do only preprocessing) macro is already defined in Makefile.
>> However POWERPC redefines it and adds $(KBUILD_CFLAGS) which results
>> in flags duplication. Which is not a big deal by itself except for
>> the flags which depend on other flags and the compiler checks them
>> as it parses the command line.
>>
>> Specifically, scripts/Makefile.build:304 generates ksyms for .S files.
>> If clang+llvm+sanitizer are enabled, this results in
>> -fno-lto -flto -fsanitize=cfi-mfcall  .... -fno-lto -flto -fsanitize=cfi-mfcall
> 
> Checkpatch doesn't like this line:
> WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
> #14:
> -fno-lto -flto -fsanitize=cfi-mfcall  .... -fno-lto -flto -fsanitize=cfi-mfcall
> However, it doesn't make sense to wrap the line so we should just ignore
> checkpatch here.
> 
>> in the clang command line and triggers error:
>>
>> clang-13: error: invalid argument '-fsanitize=cfi-mfcall' only allowed with '-flto'
>>
>> This removes unnecessary CPP redifinition.
>>
>> Signed-off-by: Alexey Kardashevskiy <aik at ozlabs.ru>
>> ---
>>   arch/powerpc/Makefile | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile
>> index c9d2c7825cd6..3a2f2001c62b 100644
>> --- a/arch/powerpc/Makefile
>> +++ b/arch/powerpc/Makefile
>> @@ -214,7 +214,6 @@ KBUILD_CPPFLAGS	+= -I $(srctree)/arch/$(ARCH) $(asinstr)
>>   KBUILD_AFLAGS	+= $(AFLAGS-y)
>>   KBUILD_CFLAGS	+= $(call cc-option,-msoft-float)
>>   KBUILD_CFLAGS	+= -pipe $(CFLAGS-y)
>> -CPP		= $(CC) -E $(KBUILD_CFLAGS)
> 
> I was trying to check the history to see why powerpc has its own
> definition. It seems to date back at least as far as merging the two
> powerpc platforms into one, maybe it was helpful then. I agree it
> doesn't seem to be needed now.
> 
> Snowpatch claims that this breaks the build, but I haven't been able to
> reproduce the issue in either pmac32 or ppc64 defconfig. I have sent it
> off to a fork of mpe's linux-ci repo to see if any of those builds hit
> any issues: https://github.com/daxtens/linux-ci/actions

To be precise, you need LLVM + LTO + byte code (-emit-llvm-bc), I am not 
even sure what is the point of having -flto without -emit-llvm-bc.

No flags duplication:

[fstn1-p1 1]$ /mnt/sdb/pbuild/llvm-no-lto/bin/clang-13  -emit-llvm-bc 
-fno-lto -flto -fvisibility=hidden -fsanitize=cfi-mfcall 
/mnt/sdb/pbuild/llvm-bugs/1/a.c
/usr/bin/ld: warning: cannot find entry symbol mit-llvm-bc; defaulting 
to 00000000100003e0
/usr/bin/ld: 
/usr/lib/powerpc64le-linux-gnu/crt1.o:(.data.rel.ro.local+0x8): 
undefined reference to `main'
clang-13: error: linker command failed with exit code 1 (use -v to see 
invocation)

=> command line is fine, the file is not (but it is for debugging this 
problem).


Now I am adding the second -fno-lto:

[fstn1-p1 1]$ /mnt/sdb/pbuild/llvm-no-lto/bin/clang-13  -emit-llvm-bc 
-fno-lto -flto -fvisibility=hidden -fsanitize=cfi-mfcall -fno-lto 
/mnt/sdb/pbuild/llvm-bugs/1/a.c
clang-13: error: invalid argument '-fsanitize=cfi-mfcall' only allowed 
with '-flto'


=> failed.

> Assuming that doesn't break, this patch looks good to me:
> Reviewed-by: Daniel Axtens <dja at axtens.net>
> 
> Kind regards,
> Daniel
> 

-- 
Alexey