[PATCH v2 1/4] init/Kconfig: add config support for detecting linker

[PATCH v2 1/4] init/Kconfig: add config support for detecting linker

[ndesaulniers]

Similar to how we differentiate between CONFIG_CC_IS_GCC and
CONFIG_CC_IS_CLANG, add CONFIG_LD_IS_BFD, CONFIG_LD_IS_GOLD, and
CONFIG_LD_IS_LLD.

This simiplifies patches to Makefiles that need to do different things
for different linkers.

Cc: Sami Tolvanen <samitolvanen@google.com>
Suggested-by: Sedat Dilek <sedat.dilek@gmail.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
Changes V1->V2:
* Add suggested, reviewed, and tested by tags.

 init/Kconfig | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/init/Kconfig b/init/Kconfig
index c9386a365eea..b6046dcf7794 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -26,6 +26,15 @@ config CLANG_VERSION
 config CC_HAS_ASM_GOTO
 	def_bool $(success,$(srctree)/scripts/gcc-goto.sh $(CC))
 
+config LD_IS_BFD
+	def_bool $(success,$(LD) --version | head -n 1 | grep -q 'GNU ld')
+
+config LD_IS_GOLD
+	def_bool $(success,$(LD) --version | head -n 1 | grep -q 'GNU gold')
+
+config LD_IS_LLD
+	def_bool $(success,$(LD) --version | head -n 1 | grep -q 'LLD')
+
 config CONSTRUCTORS
 	bool
 	depends on !UML
-- 
2.20.1.791.gb4d0f1c61a-goog

[PATCH v2 2/4] Makefile: clang: choose GCC_TOOLCHAIN_DIR not on LD

[ndesaulniers]

This causes an issue when trying to build with `make LD=ld.lld` if
ld.lld and the rest of your cross tools aren't in the same directory
(ex. /usr/local/bin) (as is the case for Android's build system), as the
GCC_TOOLCHAIN_DIR then gets set based on `which $(LD)` which will point
where LLVM tools are, not GCC/binutils tools are located.

Instead, select the GCC_TOOLCHAIN_DIR based on another tool provided by
binutils for which LLVM does not provide a substitute for, such as
elfedit.

Fixes commit 785f11aa595b ("kbuild: Add better clang cross build support")

Link: https://github.com/ClangBuiltLinux/linux/issues/341
Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
Changes V1->V2:
* add reviewed and tested tags.

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

diff --git a/Makefile b/Makefile
index 86cf35d1d79d..d3b65e96d183 100644
--- a/Makefile
+++ b/Makefile
@@ -492,7 +492,7 @@ endif
 ifneq ($(shell $(CC) --version 2>&1 | head -n 1 | grep clang),)
 ifneq ($(CROSS_COMPILE),)
 CLANG_FLAGS	:= --target=$(notdir $(CROSS_COMPILE:%-=%))
-GCC_TOOLCHAIN_DIR := $(dir $(shell which $(LD)))
+GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
 CLANG_FLAGS	+= --prefix=$(GCC_TOOLCHAIN_DIR)
 GCC_TOOLCHAIN	:= $(realpath $(GCC_TOOLCHAIN_DIR)/..)
 endif
-- 
2.20.1.791.gb4d0f1c61a-goog

[PATCH v2 3/4] Makefile: lld: tell clang to use lld

[ndesaulniers]

This is needed because clang doesn't select which linker to use based on
$LD but rather -fuse-ld=lld. This is problematic especially for
cc-ldoption, which checks for linker flag support via invoking the
compiler, rather than the linker.

Link: https://github.com/ClangBuiltLinux/linux/issues/342
Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
Changes V1->V2:
* add reviewed and tested by tags.
* move this addition up 2 statments so that it's properly added to
  KBUILD_*FLAGS as per Nathan.

 Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Makefile b/Makefile
index d3b65e96d183..00e8e01d23fc 100644
--- a/Makefile
+++ b/Makefile
@@ -500,6 +500,9 @@ ifneq ($(GCC_TOOLCHAIN),)
 CLANG_FLAGS	+= --gcc-toolchain=$(GCC_TOOLCHAIN)
 endif
 CLANG_FLAGS	+= -no-integrated-as
+ifneq ($(shell $(LD) --version 2>&1 | head -n 1 | grep LLD),)
+CLANG_FLAGS	+= -fuse-ld=lld
+endif
 KBUILD_CFLAGS	+= $(CLANG_FLAGS)
 KBUILD_AFLAGS	+= $(CLANG_FLAGS)
 export CLANG_FLAGS
-- 
2.20.1.791.gb4d0f1c61a-goog

[PATCH v2 4/4] Makefile: lld: set -O2 linker flag when linking with LLD

[ndesaulniers]

For arm64:
0.34% size improvement with lld -O2 over lld for vmlinux.
3.3% size improvement with lld -O2 over lld for Image.lz4-dtb.

Link: https://github.com/ClangBuiltLinux/linux/issues/343
Suggested-by: Rui Ueyama <ruiu@google.com>
Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
Changes V1->V2:
* add tested and reviewed by tags.

 Makefile | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Makefile b/Makefile
index 00e8e01d23fc..8011555745aa 100644
--- a/Makefile
+++ b/Makefile
@@ -718,6 +718,10 @@ else
 KBUILD_CFLAGS += -Wno-unused-but-set-variable
 endif
 
+ifdef CONFIG_LD_IS_LLD
+KBUILD_LDFLAGS += -O2
+endif
+
 KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable)
 ifdef CONFIG_FRAME_POINTER
 KBUILD_CFLAGS	+= -fno-omit-frame-pointer -fno-optimize-sibling-calls
-- 
2.20.1.791.gb4d0f1c61a-goog

[PATCH v2 0/4] Improve kernel LLD support

[ndesaulniers]

This patch cleans up a few places in the Makefile to make way for
landing LLD support in the kernel.  There are still a few arch specific
fixes that need to get upstreamed, but this core set of patches should
be arch independent.

Some very rough numbers: LLD is improving the overall build time of
Google Pixel kernels by about 9%.  The build is mostly dominated by
compilation, but LLD is able to shave off quite a bit of time.

One of the included patches helps get LLD linked kernels to competitive
binary size, but there still some bugs in LLD causing it not to discard
certain sections.  Once that's fixed, I estimate an LLD linked kernel to
be just barely smaller than a BFD linked kernel (less than 1%).

Folks looking to test should use LLD built from source, as I recently
fixed a bug in LLD that prevented LLD linked kernel modules from being
loadable.

$ make LD=ld.lld

Nick Desaulniers (4):
  init/Kconfig: add config support for detecting linker
  Makefile: clang: choose GCC_TOOLCHAIN_DIR not on LD
  Makefile: lld: tell clang to use lld
  Makefile: lld: set -O2 linker flag when linking with LLD

Changes since v1:
* collect reviewed-by, sugguested-by, tested-by tags.
* move where -fuse-ld=lld is added to CFLAGS.

 Makefile     | 9 ++++++++-
 init/Kconfig | 9 +++++++++
 2 files changed, 17 insertions(+), 1 deletion(-)

-- 
2.20.1.791.gb4d0f1c61a-goog

Re: [PATCH v2 4/4] Makefile: lld: set -O2 linker flag when linking with LLD

[Peter Zijlstra]

On Mon, Feb 11, 2019 at 11:30:06AM -0800, ndesaulniers@google.com wrote:
> For arm64:
> 0.34% size improvement with lld -O2 over lld for vmlinux.
> 3.3% size improvement with lld -O2 over lld for Image.lz4-dtb.

The Changelog is utterly failing to explain what it does.

> Link: https://github.com/ClangBuiltLinux/linux/issues/343

That is irrelevant crap.

> Suggested-by: Rui Ueyama <ruiu@google.com>
> Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
> Changes V1->V2:
> * add tested and reviewed by tags.
> 
>  Makefile | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 00e8e01d23fc..8011555745aa 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -718,6 +718,10 @@ else
>  KBUILD_CFLAGS += -Wno-unused-but-set-variable
>  endif
>  
> +ifdef CONFIG_LD_IS_LLD
> +KBUILD_LDFLAGS += -O2
> +endif
> +
>  KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable)
>  ifdef CONFIG_FRAME_POINTER
>  KBUILD_CFLAGS	+= -fno-omit-frame-pointer -fno-optimize-sibling-calls
> -- 
> 2.20.1.791.gb4d0f1c61a-goog
> 

Re: [PATCH v2 3/4] Makefile: lld: tell clang to use lld

[Masahiro Yamada]

On Tue, Feb 12, 2019 at 5:42 AM <ndesaulniers@google.com> wrote:
>
> This is needed because clang doesn't select which linker to use based on
> $LD but rather -fuse-ld=lld. This is problematic especially for
> cc-ldoption, which checks for linker flag support via invoking the
> compiler, rather than the linker.


Sorry, please explain what kind of problem
this patch solves.



[1] $(LD) is used to link vmlinux, modules, etc.

[2] $(CC) is used to link vdso, etc.
    and -fuse-ld= selects which linker is invoked from $(CC)


Is it a problem to use a different
type of linker for [1] and [2] ?


Thanks.





> Link: https://github.com/ClangBuiltLinux/linux/issues/342
> Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
> Changes V1->V2:
> * add reviewed and tested by tags.
> * move this addition up 2 statments so that it's properly added to
>   KBUILD_*FLAGS as per Nathan.
>
>  Makefile | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index d3b65e96d183..00e8e01d23fc 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -500,6 +500,9 @@ ifneq ($(GCC_TOOLCHAIN),)
>  CLANG_FLAGS    += --gcc-toolchain=$(GCC_TOOLCHAIN)
>  endif
>  CLANG_FLAGS    += -no-integrated-as
> +ifneq ($(shell $(LD) --version 2>&1 | head -n 1 | grep LLD),)
> +CLANG_FLAGS    += -fuse-ld=lld
> +endif
>  KBUILD_CFLAGS  += $(CLANG_FLAGS)
>  KBUILD_AFLAGS  += $(CLANG_FLAGS)
>  export CLANG_FLAGS
> --
> 2.20.1.791.gb4d0f1c61a-goog
>


--
Best Regards
Masahiro Yamada

Re: [PATCH v2 3/4] Makefile: lld: tell clang to use lld

[Nick Desaulniers]

On Wed, Feb 13, 2019 at 6:59 AM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> On Tue, Feb 12, 2019 at 5:42 AM <ndesaulniers@google.com> wrote:
> >
> > This is needed because clang doesn't select which linker to use based on
> > $LD but rather -fuse-ld=lld. This is problematic especially for
> > cc-ldoption, which checks for linker flag support via invoking the
> > compiler, rather than the linker.
>
>
> Sorry, please explain what kind of problem
> this patch solves.
>
>
>
> [1] $(LD) is used to link vmlinux, modules, etc.
>
> [2] $(CC) is used to link vdso, etc.
>     and -fuse-ld= selects which linker is invoked from $(CC)

It solves case 2.

>
>
> Is it a problem to use a different
> type of linker for [1] and [2] ?

Ideally, no, but I can think of at least one case where it might be
problematic to mix linkers like that:
You might be mixing linker flags added via ld-option from one linker
that aren't actually supported by the other linker.

-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 4/4] Makefile: lld: set -O2 linker flag when linking with LLD

[Masahiro Yamada]

On Tue, Feb 12, 2019 at 5:42 AM <ndesaulniers@google.com> wrote:
>
> For arm64:
> 0.34% size improvement with lld -O2 over lld for vmlinux.
> 3.3% size improvement with lld -O2 over lld for Image.lz4-dtb.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/343
> Suggested-by: Rui Ueyama <ruiu@google.com>
> Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
> Changes V1->V2:
> * add tested and reviewed by tags.
>
>  Makefile | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index 00e8e01d23fc..8011555745aa 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -718,6 +718,10 @@ else
>  KBUILD_CFLAGS += -Wno-unused-but-set-variable
>  endif
>
> +ifdef CONFIG_LD_IS_LLD
> +KBUILD_LDFLAGS += -O2
> +endif
> +


I wondered why this option should be added only for lld,
and I found the answer in the "Link:"

"for bfd, didn't seem to change the size at all (I can't recall if
this was arm64 or x86 I had tested with though)."


Hmm, I see.
My 'man ld' says it is currently effective only for shared libraries.


       -O level
           If level is a numeric values greater than zero ld optimizes the
           output.  This might take significantly longer and therefore
           probably should only be enabled for the final binary.  At the
           moment this option only affects ELF shared library generation.
           Future releases of the linker may make more use of this option.
           Also currently there is no difference in the linker's behaviour for
           different non-zero values of this option.  Again this may change
           with future releases.



I am fine with adding it unconditionally
as long as it is harmless for bfd, gold.






>  KBUILD_CFLAGS += $(call cc-disable-warning, unused-const-variable)
>  ifdef CONFIG_FRAME_POINTER
>  KBUILD_CFLAGS  += -fno-omit-frame-pointer -fno-optimize-sibling-calls
> --
> 2.20.1.791.gb4d0f1c61a-goog
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 2/4] Makefile: clang: choose GCC_TOOLCHAIN_DIR not on LD

[Masahiro Yamada]

On Tue, Feb 12, 2019 at 5:42 AM <ndesaulniers@google.com> wrote:
>
> This causes an issue when trying to build with `make LD=ld.lld` if
> ld.lld and the rest of your cross tools aren't in the same directory
> (ex. /usr/local/bin) (as is the case for Android's build system), as the
> GCC_TOOLCHAIN_DIR then gets set based on `which $(LD)` which will point
> where LLVM tools are, not GCC/binutils tools are located.
>
> Instead, select the GCC_TOOLCHAIN_DIR based on another tool provided by
> binutils for which LLVM does not provide a substitute for, such as
> elfedit.
>
> Fixes commit 785f11aa595b ("kbuild: Add better clang cross build support")
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/341
> Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---

This one looks correct to me.

Applied to linux-kbuild.



> Changes V1->V2:
> * add reviewed and tested tags.
>
>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index 86cf35d1d79d..d3b65e96d183 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -492,7 +492,7 @@ endif
>  ifneq ($(shell $(CC) --version 2>&1 | head -n 1 | grep clang),)
>  ifneq ($(CROSS_COMPILE),)
>  CLANG_FLAGS    := --target=$(notdir $(CROSS_COMPILE:%-=%))
> -GCC_TOOLCHAIN_DIR := $(dir $(shell which $(LD)))
> +GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
>  CLANG_FLAGS    += --prefix=$(GCC_TOOLCHAIN_DIR)
>  GCC_TOOLCHAIN  := $(realpath $(GCC_TOOLCHAIN_DIR)/..)
>  endif
> --
> 2.20.1.791.gb4d0f1c61a-goog
>


--
Best Regards
Masahiro Yamada

Re: [PATCH v2 3/4] Makefile: lld: tell clang to use lld

[Masahiro Yamada]

On Thu, Feb 14, 2019 at 8:08 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Wed, Feb 13, 2019 at 6:59 AM Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
> >
> > On Tue, Feb 12, 2019 at 5:42 AM <ndesaulniers@google.com> wrote:
> > >
> > > This is needed because clang doesn't select which linker to use based on
> > > $LD but rather -fuse-ld=lld. This is problematic especially for
> > > cc-ldoption, which checks for linker flag support via invoking the
> > > compiler, rather than the linker.
> >
> >
> > Sorry, please explain what kind of problem
> > this patch solves.
> >
> >
> >
> > [1] $(LD) is used to link vmlinux, modules, etc.
> >
> > [2] $(CC) is used to link vdso, etc.
> >     and -fuse-ld= selects which linker is invoked from $(CC)
>
> It solves case 2.
>
> >
> >
> > Is it a problem to use a different
> > type of linker for [1] and [2] ?
>
> Ideally, no, but I can think of at least one case where it might be
> problematic to mix linkers like that:
> You might be mixing linker flags added via ld-option from one linker
> that aren't actually supported by the other linker.

You can do this only when you are sure
that the _exactly_ same linker is used.

In my understanding, -fuse-ld=lld does not guarantee it.



-- 
Best Regards
Masahiro Yamada

Re: [PATCH v2 3/4] Makefile: lld: tell clang to use lld

[Nick Desaulniers]

On Sat, Feb 16, 2019 at 10:09 AM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> On Thu, Feb 14, 2019 at 8:08 AM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> >
> > On Wed, Feb 13, 2019 at 6:59 AM Masahiro Yamada
> > <yamada.masahiro@socionext.com> wrote:
> > >
> > > On Tue, Feb 12, 2019 at 5:42 AM <ndesaulniers@google.com> wrote:
> > > >
> > > > This is needed because clang doesn't select which linker to use based on
> > > > $LD but rather -fuse-ld=lld. This is problematic especially for
> > > > cc-ldoption, which checks for linker flag support via invoking the
> > > > compiler, rather than the linker.
> > >
> > >
> > > Sorry, please explain what kind of problem
> > > this patch solves.
> > >
> > >
> > >
> > > [1] $(LD) is used to link vmlinux, modules, etc.
> > >
> > > [2] $(CC) is used to link vdso, etc.
> > >     and -fuse-ld= selects which linker is invoked from $(CC)
> >
> > It solves case 2.
> >
> > >
> > >
> > > Is it a problem to use a different
> > > type of linker for [1] and [2] ?
> >
> > Ideally, no, but I can think of at least one case where it might be
> > problematic to mix linkers like that:
> > You might be mixing linker flags added via ld-option from one linker
> > that aren't actually supported by the other linker.
>
> You can do this only when you are sure
> that the _exactly_ same linker is used.
>
> In my understanding, -fuse-ld=lld does not guarantee it.

I really don't think we should be mixing and matching linkers during a
kernel build.  When we compile with clang, we don't have escape
hatches that allow for some object files to be compiled with GCC
(mixing clang and GCC compiled object files into one build).
Following the same logic, I think mixing linkers during kernel build
should similarly be dissuaded.  This patch AVOIDS clang using a
different linker than what was specified via $LD, which is CRITICAL
for cc-ldoption kbuild macro.  Masahiro, I hope this patch can be
re-evaluated, or if I'm not understanding your point, that you can
provide additional clarification.

-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v2 3/4] Makefile: lld: tell clang to use lld

[Masahiro Yamada]

Hi Nick,


On Tue, Apr 2, 2019 at 12:54 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Sat, Feb 16, 2019 at 10:09 AM Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
> >
> > On Thu, Feb 14, 2019 at 8:08 AM Nick Desaulniers
> > <ndesaulniers@google.com> wrote:
> > >
> > > On Wed, Feb 13, 2019 at 6:59 AM Masahiro Yamada
> > > <yamada.masahiro@socionext.com> wrote:
> > > >
> > > > On Tue, Feb 12, 2019 at 5:42 AM <ndesaulniers@google.com> wrote:
> > > > >
> > > > > This is needed because clang doesn't select which linker to use based on
> > > > > $LD but rather -fuse-ld=lld. This is problematic especially for
> > > > > cc-ldoption, which checks for linker flag support via invoking the
> > > > > compiler, rather than the linker.
> > > >
> > > >
> > > > Sorry, please explain what kind of problem
> > > > this patch solves.
> > > >
> > > >
> > > >
> > > > [1] $(LD) is used to link vmlinux, modules, etc.
> > > >
> > > > [2] $(CC) is used to link vdso, etc.
> > > >     and -fuse-ld= selects which linker is invoked from $(CC)
> > >
> > > It solves case 2.
> > >
> > > >
> > > >
> > > > Is it a problem to use a different
> > > > type of linker for [1] and [2] ?
> > >
> > > Ideally, no, but I can think of at least one case where it might be
> > > problematic to mix linkers like that:
> > > You might be mixing linker flags added via ld-option from one linker
> > > that aren't actually supported by the other linker.
> >
> > You can do this only when you are sure
> > that the _exactly_ same linker is used.
> >
> > In my understanding, -fuse-ld=lld does not guarantee it.
>
> I really don't think we should be mixing and matching linkers during a
> kernel build.  When we compile with clang, we don't have escape
> hatches that allow for some object files to be compiled with GCC
> (mixing clang and GCC compiled object files into one build).
> Following the same logic, I think mixing linkers during kernel build
> should similarly be dissuaded.  This patch AVOIDS clang using a
> different linker than what was specified via $LD, which is CRITICAL
> for cc-ldoption kbuild macro.  Masahiro, I hope this patch can be
> re-evaluated, or if I'm not understanding your point, that you can
> provide additional clarification.
>



You can pass an absolute pass to LD, like

make LD=/path/to/my/llvm/install/dir/bin/ld.lld

This clarifies which linker is being used
even when multiple versions of llvm are installed
on the machine.


However, -fuse-ld=lld is ambiguous.
Will it use the first ld.lld found in PATH?

So, you cannot avoid mixing linkers by this means.


If we could do -fuse=$(LD), I would agree with it.
Clang accepts -fuse=<absolute-path>, GCC does not.

Is there a way to control the linker search path?


-- 
Best Regards
Masahiro Yamada

[PATCH v3] Makefile: lld: tell clang to use lld

[Nick Desaulniers]

This is needed because clang doesn't select which linker to use based on
$LD but rather -fuse-ld={bfd,gold,lld,<absolute path to linker>}.  This
is problematic especially for cc-ldoption, which checks for linker flag
support via invoking the compiler, rather than the linker.

Select the linker via absolute path from $PATH via `which`. This allows
you to build with:

$ make LD=ld.lld
$ make LD=ld.lld-8
$ make LD=/path/to/ld.lld

Add -Qunused-arguments to KBUILD_CPPFLAGS when using LLD, as otherwise
Clang likes to complain about -fuse-lld= being unused when compiling but
not linking (-c) such as when cc-option is used.

Link: https://github.com/ClangBuiltLinux/linux/issues/342
Link: https://github.com/ClangBuiltLinux/linux/issues/366
Link: https://github.com/ClangBuiltLinux/linux/issues/357
Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
Changes V2->V3:
* Use absolute path based on `which $LD` as per Masahiro.
* Add -Qunused-arguments.
* Drop tested-by/reviewed-by tags, since this patched has changed enough
  to warrant re-testing/re-review, IMO.
* Add more info to the commit message.
Changes V1->V2:
* add reviewed and tested by tags.
* move this addition up 2 statments so that it's properly added to
  KBUILD_*FLAGS as per Nathan.

 Makefile | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Makefile b/Makefile
index 026fbc450906..895c076b6305 100644
--- a/Makefile
+++ b/Makefile
@@ -514,6 +514,10 @@ ifneq ($(GCC_TOOLCHAIN),)
 CLANG_FLAGS	+= --gcc-toolchain=$(GCC_TOOLCHAIN)
 endif
 CLANG_FLAGS	+= -no-integrated-as
+ifneq ($(shell $(LD) --version 2>&1 | head -n 1 | grep LLD),)
+CLANG_FLAGS	+= -fuse-ld=$(shell which $(LD))
+KBUILD_CPPFLAGS += -Qunused-arguments
+endif
 KBUILD_CFLAGS	+= $(CLANG_FLAGS)
 KBUILD_AFLAGS	+= $(CLANG_FLAGS)
 export CLANG_FLAGS
-- 
2.21.0.392.gf8f6787159e-goog

Re: [PATCH v3] Makefile: lld: tell clang to use lld

[Nathan Chancellor]

On Tue, Apr 02, 2019 at 12:08:58AM -0700, Nick Desaulniers wrote:
> This is needed because clang doesn't select which linker to use based on
> $LD but rather -fuse-ld={bfd,gold,lld,<absolute path to linker>}.  This
> is problematic especially for cc-ldoption, which checks for linker flag
> support via invoking the compiler, rather than the linker.
> 
> Select the linker via absolute path from $PATH via `which`. This allows
> you to build with:
> 
> $ make LD=ld.lld
> $ make LD=ld.lld-8
> $ make LD=/path/to/ld.lld
> 
> Add -Qunused-arguments to KBUILD_CPPFLAGS when using LLD, as otherwise
> Clang likes to complain about -fuse-lld= being unused when compiling but
> not linking (-c) such as when cc-option is used.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/342
> Link: https://github.com/ClangBuiltLinux/linux/issues/366
> Link: https://github.com/ClangBuiltLinux/linux/issues/357
> Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
> Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
> Changes V2->V3:
> * Use absolute path based on `which $LD` as per Masahiro.
> * Add -Qunused-arguments.
> * Drop tested-by/reviewed-by tags, since this patched has changed enough
>   to warrant re-testing/re-review, IMO.
> * Add more info to the commit message.
> Changes V1->V2:
> * add reviewed and tested by tags.
> * move this addition up 2 statments so that it's properly added to
>   KBUILD_*FLAGS as per Nathan.
> 
>  Makefile | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 026fbc450906..895c076b6305 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -514,6 +514,10 @@ ifneq ($(GCC_TOOLCHAIN),)
>  CLANG_FLAGS	+= --gcc-toolchain=$(GCC_TOOLCHAIN)
>  endif
>  CLANG_FLAGS	+= -no-integrated-as
> +ifneq ($(shell $(LD) --version 2>&1 | head -n 1 | grep LLD),)
> +CLANG_FLAGS	+= -fuse-ld=$(shell which $(LD))
> +KBUILD_CPPFLAGS += -Qunused-arguments

We should remove the -Qunused-arguments call in the second clang block
at the same time. With that change:

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

> +endif
>  KBUILD_CFLAGS	+= $(CLANG_FLAGS)
>  KBUILD_AFLAGS	+= $(CLANG_FLAGS)
>  export CLANG_FLAGS
> -- 
> 2.21.0.392.gf8f6787159e-goog
> 

[PATCH v4] Makefile: lld: tell clang to use lld

[Nick Desaulniers]

This is needed because clang doesn't select which linker to use based on
$LD but rather -fuse-ld={bfd,gold,lld,<absolute path to linker>}.  This
is problematic especially for cc-ldoption, which checks for linker flag
support via invoking the compiler, rather than the linker.

Select the linker via absolute path from $PATH via `which`. This allows
you to build with:

$ make LD=ld.lld
$ make LD=ld.lld-8
$ make LD=/path/to/ld.lld

Add -Qunused-arguments to KBUILD_CPPFLAGS sooner, as otherwise
Clang likes to complain about -fuse-lld= being unused when compiling but
not linking (-c) such as when cc-option is used. There's no need to
guard with cc-option.

Link: https://github.com/ClangBuiltLinux/linux/issues/342
Link: https://github.com/ClangBuiltLinux/linux/issues/366
Link: https://github.com/ClangBuiltLinux/linux/issues/357
Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
Changes V3->V4:
* Unconditionally add -Qunused-arguments sooners, as per Nathan.
* Slight modification to commit message for that point.
Changes V2->V3:
* Use absolute path based on `which $LD` as per Masahiro.
* Add -Qunused-arguments.
* Drop tested-by/reviewed-by tags, since this patched has changed enough
  to warrant re-testing/re-review, IMO.
* Add more info to the commit message.
Changes V1->V2:
* add reviewed and tested by tags.
* move this addition up 2 statments so that it's properly added to
  KBUILD_*FLAGS as per Nathan.

 Makefile | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 026fbc450906..b290e76e1ca5 100644
--- a/Makefile
+++ b/Makefile
@@ -514,6 +514,10 @@ ifneq ($(GCC_TOOLCHAIN),)
 CLANG_FLAGS	+= --gcc-toolchain=$(GCC_TOOLCHAIN)
 endif
 CLANG_FLAGS	+= -no-integrated-as
+ifneq ($(shell $(LD) --version 2>&1 | head -n 1 | grep LLD),)
+CLANG_FLAGS	+= -fuse-ld=$(shell which $(LD))
+endif
+KBUILD_CPPFLAGS	+= -Qunused-arguments
 KBUILD_CFLAGS	+= $(CLANG_FLAGS)
 KBUILD_AFLAGS	+= $(CLANG_FLAGS)
 export CLANG_FLAGS
@@ -716,7 +720,6 @@ stackp-flags-$(CONFIG_STACKPROTECTOR_STRONG)      := -fstack-protector-strong
 KBUILD_CFLAGS += $(stackp-flags-y)
 
 ifdef CONFIG_CC_IS_CLANG
-KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,)
 KBUILD_CFLAGS += $(call cc-disable-warning, format-invalid-specifier)
 KBUILD_CFLAGS += $(call cc-disable-warning, gnu)
 KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
-- 
2.21.0.392.gf8f6787159e-goog

Re: [PATCH v3] Makefile: lld: tell clang to use lld

[Sedat Dilek]

On Tue, Apr 2, 2019 at 9:09 AM 'Nick Desaulniers' via Clang Built
Linux <clang-built-linux@googlegroups.com> wrote:
>
> This is needed because clang doesn't select which linker to use based on
> $LD but rather -fuse-ld={bfd,gold,lld,<absolute path to linker>}.  This
> is problematic especially for cc-ldoption, which checks for linker flag
> support via invoking the compiler, rather than the linker.
>
> Select the linker via absolute path from $PATH via `which`. This allows
> you to build with:
>
> $ make LD=ld.lld
> $ make LD=ld.lld-8
> $ make LD=/path/to/ld.lld
>
> Add -Qunused-arguments to KBUILD_CPPFLAGS when using LLD, as otherwise
> Clang likes to complain about -fuse-lld= being unused when compiling but
> not linking (-c) such as when cc-option is used.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/342
> Link: https://github.com/ClangBuiltLinux/linux/issues/366
> Link: https://github.com/ClangBuiltLinux/linux/issues/357
> Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
> Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Thanks for bringing this up again, Nick.

Suggested-by: Sedat Dilek <sedat.dilek@gmail.com> (see [1]).

I suggest to do a separate patch on the move of  "KBUILD_CPPFLAGS +=
-Qunused-arguments".
( Patch was attached in [2],[3]. )

As pointed by Nathan this needs the removal in the original code-block.

Other than that:

Reviewed-by: Sedat Dilek <sedat.dilek@gmail.com>

[1] https://github.com/ClangBuiltLinux/linux/issues/366#issuecomment-466836735
[2] https://github.com/ClangBuiltLinux/linux/issues/366#issuecomment-467118373
[3] https://github.com/ClangBuiltLinux/linux/files/2901865/kbuild-clang-lld-Move-Qunused-arguments-CPPFLAGS-patch.txt

> ---
> Changes V2->V3:
> * Use absolute path based on `which $LD` as per Masahiro.
> * Add -Qunused-arguments.
> * Drop tested-by/reviewed-by tags, since this patched has changed enough
>   to warrant re-testing/re-review, IMO.
> * Add more info to the commit message.
> Changes V1->V2:
> * add reviewed and tested by tags.
> * move this addition up 2 statments so that it's properly added to
>   KBUILD_*FLAGS as per Nathan.
>
>  Makefile | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index 026fbc450906..895c076b6305 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -514,6 +514,10 @@ ifneq ($(GCC_TOOLCHAIN),)
>  CLANG_FLAGS    += --gcc-toolchain=$(GCC_TOOLCHAIN)
>  endif
>  CLANG_FLAGS    += -no-integrated-as
> +ifneq ($(shell $(LD) --version 2>&1 | head -n 1 | grep LLD),)
> +CLANG_FLAGS    += -fuse-ld=$(shell which $(LD))
> +KBUILD_CPPFLAGS += -Qunused-arguments
> +endif
>  KBUILD_CFLAGS  += $(CLANG_FLAGS)
>  KBUILD_AFLAGS  += $(CLANG_FLAGS)
>  export CLANG_FLAGS
> --
> 2.21.0.392.gf8f6787159e-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 post to this group, send email to clang-built-linux@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20190402070858.49597-1-ndesaulniers%40google.com.
> For more options, visit https://groups.google.com/d/optout.

Re: [PATCH v3] Makefile: lld: tell clang to use lld

[Nathan Chancellor]

On Tue, Apr 02, 2019 at 09:52:13AM +0200, Sedat Dilek wrote:
> On Tue, Apr 2, 2019 at 9:09 AM 'Nick Desaulniers' via Clang Built
> Linux <clang-built-linux@googlegroups.com> wrote:
> >
> > This is needed because clang doesn't select which linker to use based on
> > $LD but rather -fuse-ld={bfd,gold,lld,<absolute path to linker>}.  This
> > is problematic especially for cc-ldoption, which checks for linker flag
> > support via invoking the compiler, rather than the linker.
> >
> > Select the linker via absolute path from $PATH via `which`. This allows
> > you to build with:
> >
> > $ make LD=ld.lld
> > $ make LD=ld.lld-8
> > $ make LD=/path/to/ld.lld
> >
> > Add -Qunused-arguments to KBUILD_CPPFLAGS when using LLD, as otherwise
> > Clang likes to complain about -fuse-lld= being unused when compiling but
> > not linking (-c) such as when cc-option is used.
> >
> > Link: https://github.com/ClangBuiltLinux/linux/issues/342
> > Link: https://github.com/ClangBuiltLinux/linux/issues/366
> > Link: https://github.com/ClangBuiltLinux/linux/issues/357
> > Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
> > Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> 
> Thanks for bringing this up again, Nick.
> 
> Suggested-by: Sedat Dilek <sedat.dilek@gmail.com> (see [1]).
> 
> I suggest to do a separate patch on the move of  "KBUILD_CPPFLAGS +=
> -Qunused-arguments".

I think it makes sense to do it all in one patch as it doesn't make much
sense to do the move without this change. Nick already sent a v4 doing
this.

Cheers,
Nathan

> ( Patch was attached in [2],[3]. )
> 
> As pointed by Nathan this needs the removal in the original code-block.
> 
> Other than that:
> 
> Reviewed-by: Sedat Dilek <sedat.dilek@gmail.com>
> 
> [1] https://github.com/ClangBuiltLinux/linux/issues/366#issuecomment-466836735
> [2] https://github.com/ClangBuiltLinux/linux/issues/366#issuecomment-467118373
> [3] https://github.com/ClangBuiltLinux/linux/files/2901865/kbuild-clang-lld-Move-Qunused-arguments-CPPFLAGS-patch.txt
> 
> > ---
> > Changes V2->V3:
> > * Use absolute path based on `which $LD` as per Masahiro.
> > * Add -Qunused-arguments.
> > * Drop tested-by/reviewed-by tags, since this patched has changed enough
> >   to warrant re-testing/re-review, IMO.
> > * Add more info to the commit message.
> > Changes V1->V2:
> > * add reviewed and tested by tags.
> > * move this addition up 2 statments so that it's properly added to
> >   KBUILD_*FLAGS as per Nathan.
> >
> >  Makefile | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/Makefile b/Makefile
> > index 026fbc450906..895c076b6305 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -514,6 +514,10 @@ ifneq ($(GCC_TOOLCHAIN),)
> >  CLANG_FLAGS    += --gcc-toolchain=$(GCC_TOOLCHAIN)
> >  endif
> >  CLANG_FLAGS    += -no-integrated-as
> > +ifneq ($(shell $(LD) --version 2>&1 | head -n 1 | grep LLD),)
> > +CLANG_FLAGS    += -fuse-ld=$(shell which $(LD))
> > +KBUILD_CPPFLAGS += -Qunused-arguments
> > +endif
> >  KBUILD_CFLAGS  += $(CLANG_FLAGS)
> >  KBUILD_AFLAGS  += $(CLANG_FLAGS)
> >  export CLANG_FLAGS
> > --
> > 2.21.0.392.gf8f6787159e-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 post to this group, send email to clang-built-linux@googlegroups.com.
> > To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20190402070858.49597-1-ndesaulniers%40google.com.
> > For more options, visit https://groups.google.com/d/optout.

Re: [PATCH v4] Makefile: lld: tell clang to use lld

[Nathan Chancellor]

On Tue, Apr 02, 2019 at 12:33:17AM -0700, Nick Desaulniers wrote:
> This is needed because clang doesn't select which linker to use based on
> $LD but rather -fuse-ld={bfd,gold,lld,<absolute path to linker>}.  This
> is problematic especially for cc-ldoption, which checks for linker flag
> support via invoking the compiler, rather than the linker.
> 
> Select the linker via absolute path from $PATH via `which`. This allows
> you to build with:
> 
> $ make LD=ld.lld
> $ make LD=ld.lld-8
> $ make LD=/path/to/ld.lld
> 
> Add -Qunused-arguments to KBUILD_CPPFLAGS sooner, as otherwise
> Clang likes to complain about -fuse-lld= being unused when compiling but
> not linking (-c) such as when cc-option is used. There's no need to
> guard with cc-option.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/342
> Link: https://github.com/ClangBuiltLinux/linux/issues/366
> Link: https://github.com/ClangBuiltLinux/linux/issues/357
> Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
> Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

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

> ---
> Changes V3->V4:
> * Unconditionally add -Qunused-arguments sooners, as per Nathan.
> * Slight modification to commit message for that point.
> Changes V2->V3:
> * Use absolute path based on `which $LD` as per Masahiro.
> * Add -Qunused-arguments.
> * Drop tested-by/reviewed-by tags, since this patched has changed enough
>   to warrant re-testing/re-review, IMO.
> * Add more info to the commit message.
> Changes V1->V2:
> * add reviewed and tested by tags.
> * move this addition up 2 statments so that it's properly added to
>   KBUILD_*FLAGS as per Nathan.
> 
>  Makefile | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 026fbc450906..b290e76e1ca5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -514,6 +514,10 @@ ifneq ($(GCC_TOOLCHAIN),)
>  CLANG_FLAGS	+= --gcc-toolchain=$(GCC_TOOLCHAIN)
>  endif
>  CLANG_FLAGS	+= -no-integrated-as
> +ifneq ($(shell $(LD) --version 2>&1 | head -n 1 | grep LLD),)
> +CLANG_FLAGS	+= -fuse-ld=$(shell which $(LD))
> +endif
> +KBUILD_CPPFLAGS	+= -Qunused-arguments
>  KBUILD_CFLAGS	+= $(CLANG_FLAGS)
>  KBUILD_AFLAGS	+= $(CLANG_FLAGS)
>  export CLANG_FLAGS
> @@ -716,7 +720,6 @@ stackp-flags-$(CONFIG_STACKPROTECTOR_STRONG)      := -fstack-protector-strong
>  KBUILD_CFLAGS += $(stackp-flags-y)
>  
>  ifdef CONFIG_CC_IS_CLANG
> -KBUILD_CPPFLAGS += $(call cc-option,-Qunused-arguments,)
>  KBUILD_CFLAGS += $(call cc-disable-warning, format-invalid-specifier)
>  KBUILD_CFLAGS += $(call cc-disable-warning, gnu)
>  KBUILD_CFLAGS += $(call cc-disable-warning, address-of-packed-member)
> -- 
> 2.21.0.392.gf8f6787159e-goog
> 

Re: [PATCH v3] Makefile: lld: tell clang to use lld

[Masahiro Yamada]

[-- Attachment #1: Type: text/plain, Size: 1508 bytes --]

On Tue, Apr 2, 2019 at 4:09 PM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> This is needed because clang doesn't select which linker to use based on
> $LD but rather -fuse-ld={bfd,gold,lld,<absolute path to linker>}.  This
> is problematic especially for cc-ldoption, which checks for linker flag
> support via invoking the compiler, rather than the linker.
>
> Select the linker via absolute path from $PATH via `which`. This allows
> you to build with:
>
> $ make LD=ld.lld
> $ make LD=ld.lld-8
> $ make LD=/path/to/ld.lld
>
> Add -Qunused-arguments to KBUILD_CPPFLAGS when using LLD, as otherwise
> Clang likes to complain about -fuse-lld= being unused when compiling but
> not linking (-c) such as when cc-option is used.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/342
> Link: https://github.com/ClangBuiltLinux/linux/issues/366
> Link: https://github.com/ClangBuiltLinux/linux/issues/357
> Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
> Suggested-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
> Changes V2->V3:
> * Use absolute path based on `which $LD` as per Masahiro.

This is not what I suggested.  I wanted to say:
"You cannot do this for GCC, so do not do it at all".

I want to propose alternative solution.
Please check the attached patches.

To apply 0002, you need the following as a prerequisite:

https://lore.kernel.org/patchwork/patch/1057685/



--
Best Regards
Masahiro Yamada

[-- Attachment #2: 0001-ARM-vdso-use-LD-instead-of-CC-to-link-VDSO.patch --]
[-- Type: application/x-patch, Size: 2724 bytes --]

[-- Attachment #3: 0002-arm64-vdso-use-LD-instead-of-CC-to-link-VDSO.patch --]
[-- Type: application/x-patch, Size: 2443 bytes --]

Re: [PATCH v3] Makefile: lld: tell clang to use lld

[Kees Cook]

On Fri, Apr 5, 2019 at 3:17 AM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> I want to propose alternative solution.
> Please check the attached patches.

In both patches:
> I changed the Makefile to use $(LD) rahter than $(CC). I confirmed
> the same vdso.so.raw was still produced.

typo: rahter -> rather

In the 0001 patch of arch/arm/vdso/Makefile:
> ...
> -VDSO_LDFLAGS += $(call cc-ldoption, -fuse-ld=bfd)
> +ldflags-y = -Bsymbolic --no-undefined -soname=linux-vdso.so.1 \
> +           -z max-page-size=4096 -z common-page-size=4096 \
> +           -nostdlib -shared \
> +           $(call ld-option, --hash-style=sysv) \
> +           $(call ld-option, --build-id) \
> +           -T

I see that "-fuse-ld=bfd" is explicitly dropped here, which could
reintroduce the problem fixed in d2b30cd4b722 ("ARM: 8384/1: VDSO:
force use of BFD linker") (and 3473f26592c1c). Does ld.gold still
exhibit this problem? If so, we'll need to detect gold and force bfd
still maybe?

-- 
Kees Cook

Re: [PATCH v3] Makefile: lld: tell clang to use lld

[Nick Desaulniers]

On Fri, Apr 5, 2019 at 9:11 AM Kees Cook <keescook@chromium.org> wrote:
>
> On Fri, Apr 5, 2019 at 3:17 AM Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
> > I want to propose alternative solution.
> > Please check the attached patches.

```
My plan is to rewrite all VDSO Makefiles to use $(LD), then delete cc-ldoption.
```

I agree with that as a possible solution as well; I think it's best to
just invoke the linker directly rather than use the compiler as the
driver.  Just small nits with the 2 attached patches, but I think they
will also work for us.

Looks like there's 15 call sites across 12 files, plus Documentation/
and scripts/Kbuild.include; removing it seems feasible and I'd be
happy to help if you'd welcome the patches?

Let's start with that series of changes, but I suspect we will
ultimately need to revisit -fuse-ld=.  I was just thinking that even
my proposed patch doesn't do anything for KBUILD_HOSTCFLAGS, for
example.  I worry that bfd may still be invoked by Clang at some point
in the build.  I will try to test in an environment with no GNU
binutils.

> In the 0001 patch of arch/arm/vdso/Makefile:
> > ...
> > -VDSO_LDFLAGS += $(call cc-ldoption, -fuse-ld=bfd)
> > +ldflags-y = -Bsymbolic --no-undefined -soname=linux-vdso.so.1 \
> > +           -z max-page-size=4096 -z common-page-size=4096 \
> > +           -nostdlib -shared \
> > +           $(call ld-option, --hash-style=sysv) \

I think the vdso's should all REQUIRE --hash-style=sysv; the kselftest
for vdso's will fail otherwise.  I could always follow up with patches
for that if we didn't want to conflate changes.

> > +           $(call ld-option, --build-id) \
> > +           -T

Was it intentional to add -T standalone?  The man page for `ld` says
it needs an additional filename to follow -T.  Or rather is optional
if a -L flag is specified?

>
> I see that "-fuse-ld=bfd" is explicitly dropped here, which could
> reintroduce the problem fixed in d2b30cd4b722 ("ARM: 8384/1: VDSO:
> force use of BFD linker") (and 3473f26592c1c). Does ld.gold still
> exhibit this problem? If so, we'll need to detect gold and force bfd
> still maybe?

The arm32 vdso Makefile is an oddity in this regard.  I think what
Kees described is best.  Rather than always set the linker to bfd
(which harms linkers like lld), detect if gold is being used and if so
set the linker back to bfd.

For arm64, lld has this issue with -n,
https://github.com/ClangBuiltLinux/linux/issues/340, but I think we
can fix that in a follow up patch.  Same thoughts on --hash-style and
-T.

-- 
Thanks,
~Nick Desaulniers

Re: [PATCH v3] Makefile: lld: tell clang to use lld

[Masahiro Yamada]

Hi Kees, Nick,


On Sat, Apr 6, 2019 at 1:52 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Fri, Apr 5, 2019 at 9:11 AM Kees Cook <keescook@chromium.org> wrote:
> >
> > On Fri, Apr 5, 2019 at 3:17 AM Masahiro Yamada
> > <yamada.masahiro@socionext.com> wrote:
> > > I want to propose alternative solution.
> > > Please check the attached patches.
>
> ```
> My plan is to rewrite all VDSO Makefiles to use $(LD), then delete cc-ldoption.
> ```
>
> I agree with that as a possible solution as well; I think it's best to
> just invoke the linker directly rather than use the compiler as the
> driver.  Just small nits with the 2 attached patches, but I think they
> will also work for us.
>
> Looks like there's 15 call sites across 12 files, plus Documentation/
> and scripts/Kbuild.include; removing it seems feasible and I'd be
> happy to help if you'd welcome the patches?


Yeah, your help is appreciated.

I just worked on two Makefiles
because I wanted to hear opinions
before investing more efforts.


Basically, you are positive to this approach,
so let's go this way.


> Let's start with that series of changes, but I suspect we will
> ultimately need to revisit -fuse-ld=.  I was just thinking that even
> my proposed patch doesn't do anything for KBUILD_HOSTCFLAGS, for
> example.

Host tools are not so important, but I agree.

BTW, why does clang invoke bfd linker instead of
lld by default?

If we want to make llvm self-contained,
I believe clang should use ld.lld by default.




>  I worry that bfd may still be invoked by Clang at some point
> in the build.  I will try to test in an environment with no GNU
> binutils.
>
> > In the 0001 patch of arch/arm/vdso/Makefile:
> > > ...
> > > -VDSO_LDFLAGS += $(call cc-ldoption, -fuse-ld=bfd)
> > > +ldflags-y = -Bsymbolic --no-undefined -soname=linux-vdso.so.1 \
> > > +           -z max-page-size=4096 -z common-page-size=4096 \
> > > +           -nostdlib -shared \
> > > +           $(call ld-option, --hash-style=sysv) \
>
> I think the vdso's should all REQUIRE --hash-style=sysv; the kselftest
> for vdso's will fail otherwise.  I could always follow up with patches
> for that if we didn't want to conflate changes.
>
> > > +           $(call ld-option, --build-id) \
> > > +           -T
>
> Was it intentional to add -T standalone?  The man page for `ld` says
> it needs an additional filename to follow -T.

This is because I wanted to re-use cmd_ld
defined in scripts/Makefile.lib

$(real-prereqs) contains the linker scripts and objects.


>  Or rather is optional
> if a -L flag is specified?
>
> >
> > I see that "-fuse-ld=bfd" is explicitly dropped here, which could
> > reintroduce the problem fixed in d2b30cd4b722 ("ARM: 8384/1: VDSO:
> > force use of BFD linker") (and 3473f26592c1c). Does ld.gold still
> > exhibit this problem? If so, we'll need to detect gold and force bfd
> > still maybe?

I intentionally dropped -fuse-ld=bfd.

With my patch applied, users have full control
of the linker by passing LD=.

Since we cannot link vmlinux with gold,
users should definitely pass a bfd linker.



> The arm32 vdso Makefile is an oddity in this regard.  I think what
> Kees described is best.  Rather than always set the linker to bfd
> (which harms linkers like lld), detect if gold is being used and if so
> set the linker back to bfd.
>
> For arm64, lld has this issue with -n,
> https://github.com/ClangBuiltLinux/linux/issues/340, but I think we
> can fix that in a follow up patch.  Same thoughts on --hash-style and
> -T.


If we need to add something depending on
the linker type,

I do not mind doing like this.

ldflags-$(CONFIG_LD_IS_GOLD) += ...
ldflags-$(CONFIG_LD_IS_LLD)  += ...

But, it is a different issue.






--
Best Regards
Masahiro Yamada