All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] tools: fix unavoidable GCC call in Clang builds
@ 2022-02-17  3:36 Adrian Ratiu
  2022-02-17 18:10 ` Nathan Chancellor
  0 siblings, 1 reply; 3+ messages in thread
From: Adrian Ratiu @ 2022-02-17  3:36 UTC (permalink / raw)
  To: llvm; +Cc: linux-kernel, Nick Desaulniers, Nathan Chancellor, Manoj Gupta

In ChromeOS and Gentoo we catch any unwanted mixed Clang/LLVM
and GCC/binutils usage via toolchain wrappers which fail builds.
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.

Suggested-by: Manoj Gupta <manojgupta@chromium.com>
Signed-off-by: Adrian Ratiu <adrian.ratiu@collabora.com>
---
 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


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] tools: fix unavoidable GCC call in Clang builds
  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
  0 siblings, 1 reply; 3+ messages in thread
From: Nathan Chancellor @ 2022-02-17 18:10 UTC (permalink / raw)
  To: Adrian Ratiu; +Cc: llvm, linux-kernel, Nick Desaulniers, Manoj Gupta

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.

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

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.

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

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] tools: fix unavoidable GCC call in Clang builds
  2022-02-17 18:10 ` Nathan Chancellor
@ 2022-02-18  7:27   ` Adrian Ratiu
  0 siblings, 0 replies; 3+ messages in thread
From: Adrian Ratiu @ 2022-02-18  7:27 UTC (permalink / raw)
  To: Nathan Chancellor; +Cc: llvm, linux-kernel, Nick Desaulniers, Manoj Gupta

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-02-18  7:28 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 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.