| Message ID | 20200820220955.3325555-1-ndesaulniers@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Makefile: add -fuse-ld=lld to KBUILD_HOSTLDFLAGS when LLVM=1 | expand |
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 >
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.
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
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
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
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(+)