All of lore.kernel.org
 help / color / mirror / Atom feed
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
Date: Fri, 18 Feb 2022 09:27:57 +0200	[thread overview]
Message-ID: <87czjk4osi.fsf@ryzen9.i-did-not-set--mail-host-address--so-tickle-me> (raw)
In-Reply-To: <Yg6PousYSZXimFCS@dev-arch.archlinux-ax161>

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
>> 

      reply	other threads:[~2022-02-18  7:28 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-17  3:36 [PATCH] tools: fix unavoidable GCC call in Clang builds Adrian Ratiu
2022-02-17 18:10 ` Nathan Chancellor
2022-02-18  7:27   ` Adrian Ratiu [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87czjk4osi.fsf@ryzen9.i-did-not-set--mail-host-address--so-tickle-me \
    --to=adrian.ratiu@collabora.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=manojgupta@chromium.com \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.