diff mbox series

Makefile: add -fuse-ld=lld to KBUILD_HOSTLDFLAGS when LLVM=1

Message ID 20200820220955.3325555-1-ndesaulniers@google.com
State New
Headers show
Series Makefile: add -fuse-ld=lld to KBUILD_HOSTLDFLAGS when LLVM=1 | expand

Commit Message

Nick Desaulniers Aug. 20, 2020, 10:09 p.m. UTC
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.

Cc: stable@vger.kernel.org
Fixes: commit a0d1c951ef08 ("kbuild: support LLVM=1 to switch the default tools to Clang/LLVM")
Reported-by: Matthias Maennich <maennich@google.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

---
 Makefile | 1 +
 1 file changed, 1 insertion(+)

-- 
2.28.0.297.g1956fa8f8d-goog

Comments

Nathan Chancellor Aug. 20, 2020, 10:20 p.m. UTC | #1
On Thu, Aug 20, 2020 at 03:09:55PM -0700, 'Nick Desaulniers' via Clang Built Linux 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.

> 

> Cc: stable@vger.kernel.org

> Fixes: commit a0d1c951ef08 ("kbuild: support LLVM=1 to switch the default tools to Clang/LLVM")


Minor nit, "commit" is unnecessary here and might be flagged by some tag
checking scripts.

> Reported-by: Matthias Maennich <maennich@google.com>

> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>


Regardless of the above, this should work fine so:

Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>


> ---

>  Makefile | 1 +

>  1 file changed, 1 insertion(+)

> 

> diff --git a/Makefile b/Makefile

> index def590b743a9..b4e93b228a26 100644

> --- a/Makefile

> +++ b/Makefile

> @@ -436,6 +436,7 @@ OBJDUMP		= llvm-objdump

>  READELF		= llvm-readelf

>  OBJSIZE		= llvm-size

>  STRIP		= llvm-strip

> +KBUILD_HOSTLDFLAGS	+= -fuse-ld=lld

>  else

>  CC		= $(CROSS_COMPILE)gcc

>  LD		= $(CROSS_COMPILE)ld

> -- 

> 2.28.0.297.g1956fa8f8d-goog

>
Masahiro Yamada Aug. 22, 2020, 5:13 a.m. UTC | #2
On Fri, Aug 21, 2020 at 7:10 AM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.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.

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


If this fails, your environment is broken.

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








> Cc: stable@vger.kernel.org

> Fixes: commit a0d1c951ef08 ("kbuild: support LLVM=1 to switch the default tools to Clang/LLVM")

> Reported-by: Matthias Maennich <maennich@google.com>

> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

> ---

>  Makefile | 1 +

>  1 file changed, 1 insertion(+)

>

> diff --git a/Makefile b/Makefile

> index def590b743a9..b4e93b228a26 100644

> --- a/Makefile

> +++ b/Makefile

> @@ -436,6 +436,7 @@ OBJDUMP             = llvm-objdump

>  READELF                = llvm-readelf

>  OBJSIZE                = llvm-size

>  STRIP          = llvm-strip

> +KBUILD_HOSTLDFLAGS     += -fuse-ld=lld

>  else

>  CC             = $(CROSS_COMPILE)gcc

>  LD             = $(CROSS_COMPILE)ld

> --

> 2.28.0.297.g1956fa8f8d-goog

>

> --

> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.

> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20200820220955.3325555-1-ndesaulniers%40google.com.




-- 
Best Regards
Masahiro Yamada
Nick Desaulniers Sept. 2, 2020, 10:40 p.m. UTC | #3
On Fri, Aug 21, 2020 at 10:14 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>

> On Fri, Aug 21, 2020 at 7:10 AM 'Nick Desaulniers' via Clang Built

> Linux <clang-built-linux@googlegroups.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
Masahiro Yamada Sept. 3, 2020, 1:45 a.m. UTC | #4
On Thu, Sep 3, 2020 at 7:40 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
>

> On Fri, Aug 21, 2020 at 10:14 PM Masahiro Yamada <masahiroy@kernel.org> wrote:

> >

> > On Fri, Aug 21, 2020 at 7:10 AM 'Nick Desaulniers' via Clang Built

> > Linux <clang-built-linux@googlegroups.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.



I am still not convinced with this.

If you care which linker is internally used,
you can/should set -DCLANG_DEFAULT_LINKER='lld',
and that is what 'configure' exists for.



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



Is it so important?
This is just host programs we are talking about.

If you really want to ensure lld is used everywhere,
you need to ask any other projects to add -fuse-ld=lld
in their build systems, but it is not realistic.

So, I tend to stick to the default
for host programs.


Your environment is _unique_, at least.

Kbuild provides a way to add extra flags to HOSTCC.

What do you think about doing this?

$ make LLVM=1 HOSTCFLAGS=-fuse-ld=lld


This is documented in Documentation/kbuild/kbuild.rst

HOSTCFLAGS
----------
Additional flags to be passed to $(HOSTCC) when building host programs.






>

> ...

>

> 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




-- 
Best Regards
Masahiro Yamada
diff mbox series

Patch

diff --git a/Makefile b/Makefile
index def590b743a9..b4e93b228a26 100644
--- a/Makefile
+++ b/Makefile
@@ -436,6 +436,7 @@  OBJDUMP		= llvm-objdump
 READELF		= llvm-readelf
 OBJSIZE		= llvm-size
 STRIP		= llvm-strip
+KBUILD_HOSTLDFLAGS	+= -fuse-ld=lld
 else
 CC		= $(CROSS_COMPILE)gcc
 LD		= $(CROSS_COMPILE)ld