Date:   Wed, 2 Sep 2020 15:40:03 -0700
From:   Nick Desaulniers <ndesaulniers@...gle.com>
To:     Masahiro Yamada <masahiroy@...nel.org>
Cc:     stable <stable@...r.kernel.org>,
        Matthias Maennich <maennich@...gle.com>,
        Michal Marek <michal.lkml@...kovi.net>,
        Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        clang-built-linux <clang-built-linux@...glegroups.com>,
        Stephen Hines <srhines@...gle.com>,
        Dan Albert <danalbert@...gle.com>,
        Fangrui Song <maskray@...gle.com>,
        Elliott Hughes <enh@...gle.com>
Subject: Re: [PATCH] Makefile: add -fuse-ld=lld to KBUILD_HOSTLDFLAGS when LLVM=1

On Fri, Aug 21, 2020 at 10:14 PM Masahiro Yamada <masahiroy@...nel.org> wrote:
>
> On Fri, Aug 21, 2020 at 7:10 AM 'Nick Desaulniers' via Clang Built
> Linux <clang-built-linux@...glegroups.com> wrote:
> >
> > While moving Android kernels over to use LLVM=1, we observe the failure
> > when building in a hermetic docker image:
> >   HOSTCC  scripts/basic/fixdep
> > clang: error: unable to execute command: Executable "ld" doesn't exist!
> >
> > The is because the build of the host utility fixdep builds the fixdep
> > executable in one step by invoking the compiler as the driver, rather
> > than individual compile then link steps.
> >
> > Clang when configured from source defaults to use the system's linker,
> > and not LLVM's own LLD, unless the CMake config
> > -DCLANG_DEFAULT_LINKER='lld' is set when configuring a build of clang
> > itself.
> >
> > Don't rely on the compiler's implicit default linker; be explicit.
>
>
> I do not understand this patch.
>
> The host compiler should be able to link executables
> without any additional settings.

Correct; there is no issue linking working executables. The issue is
which linker is used by default or implied when -fuse-ld=* is not
explicitly set.

>
> So, can you link a hello world program
> in your docker?
>
> masahiro@zoe:~$ cat test.c
> #include <stdio.h>
> int main(void)
> {
>         printf("helloworld\n");
>         return 0;
> }
> masahiro@zoe:~$ clang test.c

It will fail, because:
1. clang will implicitly default to ld.bfd on linux hosts and ld on
OSX hosts (idk about windows).
2. ld.bfd is not installed, and we *dont'* want to install it.
Instead, we *want* to use ld.lld in a hermetic environment.

> If this fails, your environment is broken.

Disagree.  The environment has unique constraints (cross compiling for
Android from OSX host, caring about builds being hermetic, etc.).

> Just do  -DCLANG_DEFAULT_LINKER='lld'
> if you know GNU ld is missing in your docker environment.

I understand your point. However, I have two reasons I still think
this patch should be upstream rather than downstream:

1. The build of clang that is distributed with Android, "AOSP LLVM"
[0], does not and cannot yet set `-DCLANG_DEFAULT_LINKER='lld'`.  See
the discussion in the comments of [1] where I'm trying to do that.
The reason is that AOSP LLVM is used to build Android userspace,
kernel, and is part of the NDK for developers to target Android from
Windows, OSX, and Linux.  If AOSP is used to build a "host binary" on
OSX, LLD will not work there for that quite yet.  OSX has its own
linker that is not LLD, and LLD support for mach-o binaries is a work
in progress.  NDK has their own timeline that's blocking that change.

You might think "that's Android problem" and that we should just carry
the patch downstream/out of tree since it is somewhat self-inflicted
but a very important second point why I think this should be upstream:

2. clang itself (upstream of AOSP LLVM) doesn't yet default to
-fuse-ld=lld (likely for similar reasons related to OSX).  That means
distributions of clang-10 from your distro package manager such as
Debian's apt won't be hermetic.  That means if you build clang from
source, and don't configure it with -DCLANG_DEFAULT_LINKER='lld', then
your kernel builds with LLVM=1 will not be hermetic.  That means we
have to document this somewhere for other people to know or find this.
That means I have to run around and tell all of the different Kernel
CI folks about this compiler configuration in order to test
hermetically.

...

Or, encouraged by the zen of Python, we can just be explicit about
what linker we want when using LLVM=1, which already signals that that
is what we want to do.

I think there are similar issues with other distros changing default
flags of GCC (like -fstack-protector) [2].  The kernel is already
explicit, so that differences in distro's changes to compiler defaults
don't matter for kernel builds (except where people accidentally wipe
out KBUILD_CFLAGS).  I'd argue my change is in the same bucket.
Please reconsider this patch.

(I should also probably add something like this for `make LD=ld.lld`
and `make LD=ld.bfd`, regardless of compiler, since everyone supports
`-fuse-ld=`)

[0] https://android.googlesource.com/platform/prebuilts/clang/host/linux-x86/
[1] https://android-review.googlesource.com/c/toolchain/llvm_android/+/1007826
[2] https://fedoraproject.org/wiki/Changes/HardenedCompiler#Detailed_Description
-- 
Thanks,
~Nick Desaulniers

Powered by blists - more mailing lists