Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Adrian Ratiu <adrian.ratiu@collabora.com>
To:     Nathan Chancellor <nathan@kernel.org>
Cc:     llvm@lists.linux.dev, linux-kernel@vger.kernel.org,
        Nick Desaulniers <ndesaulniers@google.com>,
        Manoj Gupta <manojgupta@chromium.com>
Subject: Re: [PATCH] tools: fix unavoidable GCC call in Clang builds
In-Reply-To: <Yg6PousYSZXimFCS@dev-arch.archlinux-ax161>
References: <20220217033648.1453303-1-adrian.ratiu@collabora.com>
 <Yg6PousYSZXimFCS@dev-arch.archlinux-ax161>
Date:   Fri, 18 Feb 2022 09:27:57 +0200
Message-ID: <87czjk4osi.fsf@ryzen9.i-did-not-set--mail-host-address--so-tickle-me>
MIME-Version: 1.0
Content-Type: text/plain; format=flowed
Precedence: bulk

On Thu, 17 Feb 2022, Nathan Chancellor <nathan@kernel.org> wrote:
> Hi Adrian 
> 
> On Thu, Feb 17, 2022 at 05:36:48AM +0200, Adrian Ratiu wrote: 
>> In ChromeOS and Gentoo we catch any unwanted mixed Clang/LLVM 
>> and GCC/binutils usage via toolchain wrappers which fail 
>> builds. 
> 
> Neat for hermetic builds. 
> 

Thanks! We're doing this for the entire build and found quite some 
bugs in upstream userspace projects as well, like configure/m4 
hardcoding things like binutils ar instead of our toolchain 
configured llvm-ar.

>> This has revealed that GCC is called unconditionally in Clang 
>> configured builds to populate GCC_TOOLCHAIN_DIR.   Allow 
>> overriding the variable to avoid the GCC call - in our case we 
>> can set GCC_TOOLCHAIN_DIR directly in the ebuild recipe. 
> 
> Would you just set GCC_TOOLCHAIN_DIR to nothing to avoid 
> triggering the 'gcc -print-sysroot' call?

Yes, the intention was to set GCC_TOOLCHAIN_DIR="" in the ebuild, 
but now we have a better approach suggested by you below :)

What we do right now is add an exception for the entire kernel, to 
allow it to make mixed toolchain calls (wrappers don't fail the 
build). AFAICT this is the last bug before we can start enforcing 
"pure" LLVM builds, at least on more recent kernels.

Older kernels have been known to work as well (eg 5.4) after 
fixing some bugs which were not strictly in the kernel codebase, 
like: 

https://archives.gentoo.org/gentoo-dev/message/a4aefebf8969b2ddbbf7298c67659996
 
> 
> An alternative might be allowing CLANG_CROSS_FLAGS to be 
> supplied by the user, so that you can stil benefit from cross 
> compiling tools, but with the flags and sysroot that you expect. 
>

That seems like a better approach, thanks. It makes sense to allow 
distributions to specify locations of their userspace libraries 
since the kernel has no control over how various distros are 
configured.

According to the original commit message adding this change, in 
some cases Clang might be able to autodetect these paths, but 
autodetection does not work for all possible configurations.

If a reliable generic Clang autodetection mechanism is possible, 
then setting these vars/flags becomes a no-op and can be dropped, 
until then CLANG_CROSS_FLAGS solves our problem.
 
>> Suggested-by: Manoj Gupta <manojgupta@chromium.com> 
>> Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com> 
> 
> Regardless, as long as it works for your use case: 
> 
> Acked-by: Nathan Chancellor <nathan@kernel.org> 
> 
> The change that this patch fixes [1] went via bpf-next [2] last 
> cycle, you could either target that tree to have it fixed for 
> 5.18 or make a case for having it merged in the 5.17 cycle via 
> bpf [3]. I would resend this change to the kernel/bpf 
> maintainers, as Nick and I do not currently pick up patches.

If Nick is also ok with the CLANG_CROSS_FLAGS approach, I'll send 
that to bpf-next. Thanks again!

>
> [1]: https://lore.kernel.org/r/20211216163842.829836-2-jean-philippe@linaro.org/
> [2]: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/
> [3]: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git/
>
>> ---
>>  tools/scripts/Makefile.include | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include
>> index 79d102304470..98c098c064dd 100644
>> --- a/tools/scripts/Makefile.include
>> +++ b/tools/scripts/Makefile.include
>> @@ -90,7 +90,7 @@ EXTRA_WARNINGS += -Wstrict-aliasing=3
>>  
>>  else ifneq ($(CROSS_COMPILE),)
>>  CLANG_CROSS_FLAGS := --target=$(notdir $(CROSS_COMPILE:%-=%))
>> -GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)gcc 2>/dev/null))
>> +GCC_TOOLCHAIN_DIR ?= $(dir $(shell which $(CROSS_COMPILE)gcc 2>/dev/null))
>>  ifneq ($(GCC_TOOLCHAIN_DIR),)
>>  CLANG_CROSS_FLAGS += --prefix=$(GCC_TOOLCHAIN_DIR)$(notdir $(CROSS_COMPILE))
>>  CLANG_CROSS_FLAGS += --sysroot=$(shell $(CROSS_COMPILE)gcc -print-sysroot)
>> -- 
>> 2.35.0
>>