2022-02-17 08:14:12

by Adrian Ratiu

[permalink] [raw]
Subject: [PATCH] tools: fix unavoidable GCC call in Clang builds

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 <[email protected]>
Signed-off-by: Adrian Ratiu <[email protected]>
---
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


2022-02-17 19:41:55

by Nathan Chancellor

[permalink] [raw]
Subject: Re: [PATCH] tools: fix unavoidable GCC call in Clang builds

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 <[email protected]>
> Signed-off-by: Adrian Ratiu <[email protected]>

Regardless, as long as it works for your use case:

Acked-by: Nathan Chancellor <[email protected]>

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/[email protected]/
[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
>

2022-02-18 08:36:30

by Adrian Ratiu

[permalink] [raw]
Subject: Re: [PATCH] tools: fix unavoidable GCC call in Clang builds

On Thu, 17 Feb 2022, Nathan Chancellor <[email protected]> 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 <[email protected]>
>> Signed-off-by: Adrian Ratiu <[email protected]>
>
> Regardless, as long as it works for your use case:
>
> Acked-by: Nathan Chancellor <[email protected]>
>
> 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/[email protected]/
> [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
>>