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

[PATCH 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: Nathan Chancellor <natechancellor@gmail.com>
Cc: Sami Tolvanen <samitolvanen@google.com>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 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 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>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 3142e67d03f1..0eae4277206e 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 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=$(LD). 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>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 Makefile | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Makefile b/Makefile
index 0eae4277206e..6307c17259ea 100644
--- a/Makefile
+++ b/Makefile
@@ -502,6 +502,9 @@ endif
 CLANG_FLAGS	+= -no-integrated-as
 KBUILD_CFLAGS	+= $(CLANG_FLAGS)
 KBUILD_AFLAGS	+= $(CLANG_FLAGS)
+ifneq ($(shell $(LD) --version 2>&1 | head -n 1 | grep LLD),)
+CLANG_FLAGS	+= -fuse-ld=lld
+endif
 export CLANG_FLAGS
 endif
 
-- 
2.20.1.791.gb4d0f1c61a-goog

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

[ndesaulniers]

-O2 enables tail merging of string table strings.

For arm64:
0.34% size improvement with lld -O2 over lld for vmlinux.
3.30% 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>
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
---
 Makefile | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Makefile b/Makefile
index 6307c17259ea..c07208ec49d4 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 1/4] init/Kconfig: add config support for detecting linker

[Kees Cook]

On Thu, Feb 7, 2019 at 10:02 PM <ndesaulniers@google.com> wrote:
>
> 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: Nathan Chancellor <natechancellor@gmail.com>
> Cc: Sami Tolvanen <samitolvanen@google.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Reviewed-by: Kees Cook <keescook@chromium.org>

-Kees

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


-- 
Kees Cook

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

[Mathieu Desnoyers]

----- ndesaulniers@google.com wrote:
> 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.

What guarantees that the linker used for e.g. make defconfig is the same linker used for make ?

Is it required with this patch ?

How does it work in a cross compile environment ?

Thanks,

Mathieu


> 
> Cc: Nathan Chancellor <natechancellor@gmail.com>
> Cc: Sami Tolvanen <samitolvanen@google.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>  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
> 

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com

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

[Nathan Chancellor]

On Thu, Feb 07, 2019 at 02:01:49PM -0800, ndesaulniers@google.com wrote:
> 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: Nathan Chancellor <natechancellor@gmail.com>
> Cc: Sami Tolvanen <samitolvanen@google.com>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

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

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

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

[Nathan Chancellor]

On Thu, Feb 07, 2019 at 07:57:20PM -0500, Mathieu Desnoyers wrote:
> 
> ----- ndesaulniers@google.com wrote:
> > 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.
> 

Hi Mathieu,

> What guarantees that the linker used for e.g. make defconfig is the same linker used for make ?
> 
> Is it required with this patch ?
> 

The build system ensures that the compiler (and after this patch, the
linker) config values are correct before compiling (and prompting the
user for the new choices that are available to them in that case).

For example:

$ make ARCH=arm64 CC=clang CROSS_COMPILE=aarch64-linux-gnu- defconfig
...

$ rg "CONFIG_CC_IS|CONFIG_LD_IS" .config
10:CONFIG_CC_IS_CLANG=y
12:CONFIG_LD_IS_BFD=y

$ make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- LD=ld.lld
scripts/kconfig/conf  --syncconfig Kconfig
*
* Restart config...
*
*
* General architecture-dependent options
*
Kprobes (KPROBES) [N/y/?] n
Optimize very unlikely/likely branches (JUMP_LABEL) [N/y/?] (NEW)
Stack Protector buffer overflow detection (STACKPROTECTOR) [Y/n/?] y
  Strong Stack Protector (STACKPROTECTOR_STRONG) [Y/n/?] y
Use a virtually-mapped stack (VMAP_STACK) [Y/n/?] y
Perform full reference count validation at the expense of speed (REFCOUNT_FULL) [Y/?] y
*
* GCC plugins
*
GCC plugins (GCC_PLUGINS) [N/y/?] (NEW)
...

$ rg "CONFIG_CC_IS|CONFIG_LD_IS" .config
9:CONFIG_CC_IS_GCC=y
13:CONFIG_LD_IS_LLD=y

> How does it work in a cross compile environment ?
> 

Since $(LD) is defined based on $(CROSS_COMPILE), it should work fine.
I tested it with both arm64 and x86_64 and CONFIG_LD_IS_BFD gets
set by default and I can see CONFIG_LD_IS_GOLD get set with both
LD=aarch64-linux-gnu-ld.gold and LD=ld.gold.

If I misunderstood the questions or my explanation wasn't correct,
please let me know!

Cheers,
Nathan

> Thanks,
> 
> Mathieu
> 
> 
> > 
> > Cc: Nathan Chancellor <natechancellor@gmail.com>
> > Cc: Sami Tolvanen <samitolvanen@google.com>
> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > ---
> >  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
> > 
> 
> -- 
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com

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

[Nathan Chancellor]

On Thu, Feb 07, 2019 at 02:01:50PM -0800, 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>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

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

> ---
>  Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Makefile b/Makefile
> index 3142e67d03f1..0eae4277206e 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
> 

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

[Nathan Chancellor]

On Thu, Feb 07, 2019 at 02:01:51PM -0800, ndesaulniers@google.com wrote:
> This is needed because clang doesn't select which linker to use based on
> $LD but rather -fuse-ld=$(LD). 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>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> ---
>  Makefile | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 0eae4277206e..6307c17259ea 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -502,6 +502,9 @@ endif
>  CLANG_FLAGS	+= -no-integrated-as
>  KBUILD_CFLAGS	+= $(CLANG_FLAGS)
>  KBUILD_AFLAGS	+= $(CLANG_FLAGS)
> +ifneq ($(shell $(LD) --version 2>&1 | head -n 1 | grep LLD),)
> +CLANG_FLAGS	+= -fuse-ld=lld
> +endif

This section needs to be moved up above KBUILD_CFLAGS otherwise it never
actually gets passed along.

With that change, please add:

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

>  export CLANG_FLAGS
>  endif
>  
> -- 
> 2.20.1.791.gb4d0f1c61a-goog
> 

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

[Nathan Chancellor]

On Thu, Feb 07, 2019 at 02:01:52PM -0800, ndesaulniers@google.com wrote:
> -O2 enables tail merging of string table strings.
> 
> For arm64:
> 0.34% size improvement with lld -O2 over lld for vmlinux.
> 3.30% 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>
> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

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

> ---
>  Makefile | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index 6307c17259ea..c07208ec49d4 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 3/4] Makefile: lld: tell clang to use lld

[Nick Desaulniers]

On Thu, Feb 7, 2019 at 9:51 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> On Thu, Feb 07, 2019 at 02:01:51PM -0800, ndesaulniers@google.com wrote:
> > This is needed because clang doesn't select which linker to use based on
> > $LD but rather -fuse-ld=$(LD). 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>
> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > ---
> >  Makefile | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/Makefile b/Makefile
> > index 0eae4277206e..6307c17259ea 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -502,6 +502,9 @@ endif
> >  CLANG_FLAGS  += -no-integrated-as
> >  KBUILD_CFLAGS        += $(CLANG_FLAGS)
> >  KBUILD_AFLAGS        += $(CLANG_FLAGS)
> > +ifneq ($(shell $(LD) --version 2>&1 | head -n 1 | grep LLD),)
> > +CLANG_FLAGS  += -fuse-ld=lld

Sorry, Sedat reports [0] that this is not quite correct; if $(LD)
matches LLD, then we need to set -fuse-ld=$(LD), not -fuse-ld=lld,
since you may have lld installed as lld-7 or lld-8, for example:
https://github.com/ClangBuiltLinux/linux/issues/358

> > +endif
>
> This section needs to be moved up above KBUILD_CFLAGS otherwise it never
> actually gets passed along.
>
> With that change, please add:
>
> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
>
> >  export CLANG_FLAGS
> >  endif
> >
> > --
> > 2.20.1.791.gb4d0f1c61a-goog
> >



-- 
Thanks,
~Nick Desaulniers

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

[Nathan Chancellor]

On Fri, Feb 08, 2019 at 09:27:19AM -0800, Nick Desaulniers wrote:
> On Thu, Feb 7, 2019 at 9:51 PM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > On Thu, Feb 07, 2019 at 02:01:51PM -0800, ndesaulniers@google.com wrote:
> > > This is needed because clang doesn't select which linker to use based on
> > > $LD but rather -fuse-ld=$(LD). 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>
> > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > > ---
> > >  Makefile | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index 0eae4277206e..6307c17259ea 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -502,6 +502,9 @@ endif
> > >  CLANG_FLAGS  += -no-integrated-as
> > >  KBUILD_CFLAGS        += $(CLANG_FLAGS)
> > >  KBUILD_AFLAGS        += $(CLANG_FLAGS)
> > > +ifneq ($(shell $(LD) --version 2>&1 | head -n 1 | grep LLD),)
> > > +CLANG_FLAGS  += -fuse-ld=lld
> 
> Sorry, Sedat reports [0] that this is not quite correct; if $(LD)
> matches LLD, then we need to set -fuse-ld=$(LD), not -fuse-ld=lld,
> since you may have lld installed as lld-7 or lld-8, for example:
> https://github.com/ClangBuiltLinux/linux/issues/358
> 

We'll need to make sure that it is compatible with the way that Clang
expects arguments to '-fuse-ld=' because it looks like 'ld.' is
automatically appended.

https://github.com/llvm/llvm-project/blob/5be3dbdb082ffba4f846e3572ea802d156dd8d34/clang/lib/Driver/ToolChain.cpp#L438-L468

> > > +endif
> >
> > This section needs to be moved up above KBUILD_CFLAGS otherwise it never
> > actually gets passed along.
> >
> > With that change, please add:
> >
> > Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> > Tested-by: Nathan Chancellor <natechancellor@gmail.com>
> >
> > >  export CLANG_FLAGS
> > >  endif
> > >
> > > --
> > > 2.20.1.791.gb4d0f1c61a-goog
> > >
> 
> 
> 
> -- 
> Thanks,
> ~Nick Desaulniers

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

[Masahiro Yamada]

On Fri, Feb 8, 2019 at 2:42 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> On Thu, Feb 07, 2019 at 07:57:20PM -0500, Mathieu Desnoyers wrote:
> >
> > ----- ndesaulniers@google.com wrote:
> > > 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.
> >
>
> Hi Mathieu,
>
> > What guarantees that the linker used for e.g. make defconfig is the same linker used for make ?
> >
> > Is it required with this patch ?
> >
>
> The build system ensures that the compiler (and after this patch, the
> linker) config values are correct before compiling (and prompting the
> user for the new choices that are available to them in that case).
>
> For example:
>
> $ make ARCH=arm64 CC=clang CROSS_COMPILE=aarch64-linux-gnu- defconfig
> ...
>
> $ rg "CONFIG_CC_IS|CONFIG_LD_IS" .config
> 10:CONFIG_CC_IS_CLANG=y
> 12:CONFIG_LD_IS_BFD=y
>
> $ make ARCH=arm64 CROSS_COMPILE=aarch64-linux-gnu- LD=ld.lld
> scripts/kconfig/conf  --syncconfig Kconfig
> *
> * Restart config...
> *
> *
> * General architecture-dependent options
> *
> Kprobes (KPROBES) [N/y/?] n
> Optimize very unlikely/likely branches (JUMP_LABEL) [N/y/?] (NEW)
> Stack Protector buffer overflow detection (STACKPROTECTOR) [Y/n/?] y
>   Strong Stack Protector (STACKPROTECTOR_STRONG) [Y/n/?] y
> Use a virtually-mapped stack (VMAP_STACK) [Y/n/?] y
> Perform full reference count validation at the expense of speed (REFCOUNT_FULL) [Y/?] y
> *
> * GCC plugins
> *
> GCC plugins (GCC_PLUGINS) [N/y/?] (NEW)
> ...
>
> $ rg "CONFIG_CC_IS|CONFIG_LD_IS" .config
> 9:CONFIG_CC_IS_GCC=y
> 13:CONFIG_LD_IS_LLD=y
>
> > How does it work in a cross compile environment ?
> >
>
> Since $(LD) is defined based on $(CROSS_COMPILE), it should work fine.
> I tested it with both arm64 and x86_64 and CONFIG_LD_IS_BFD gets
> set by default and I can see CONFIG_LD_IS_GOLD get set with both
> LD=aarch64-linux-gnu-ld.gold and LD=ld.gold.
>
> If I misunderstood the questions or my explanation wasn't correct,
> please let me know!




With this patch,

include/config/auto.conf.cmd will include the following:

ifneq "$(LD)" "ld"
include/config/auto.conf: FORCE
endif


When $(LD) is changed, Kconfig will be re-run.






--
Best Regards
Masahiro Yamada

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

[Masahiro Yamada]

On Sat, Feb 9, 2019 at 2:27 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
>
> On Thu, Feb 7, 2019 at 9:51 PM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > On Thu, Feb 07, 2019 at 02:01:51PM -0800, ndesaulniers@google.com wrote:
> > > This is needed because clang doesn't select which linker to use based on
> > > $LD but rather -fuse-ld=$(LD). This is problematic especially for
> > > cc-ldoption, which checks for linker flag support via invoking the
> > > compiler, rather than the linker.

What do you mean by 'problematic' ?

I see no build error in
arch/arm64/kernel/vdso/Makefile, at least.

Clang uses BFD linker by default,
which may not match to the $(LD) passed in.

Is this what you mean?





> > > Link: https://github.com/ClangBuiltLinux/linux/issues/342
> > > Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
> > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > > ---
> > >  Makefile | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index 0eae4277206e..6307c17259ea 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -502,6 +502,9 @@ endif
> > >  CLANG_FLAGS  += -no-integrated-as
> > >  KBUILD_CFLAGS        += $(CLANG_FLAGS)
> > >  KBUILD_AFLAGS        += $(CLANG_FLAGS)
> > > +ifneq ($(shell $(LD) --version 2>&1 | head -n 1 | grep LLD),)
> > > +CLANG_FLAGS  += -fuse-ld=lld
>
> Sorry, Sedat reports [0] that this is not quite correct; if $(LD)
> matches LLD, then we need to set -fuse-ld=$(LD), not -fuse-ld=lld,
> since you may have lld installed as lld-7 or lld-8, for example:
> https://github.com/ClangBuiltLinux/linux/issues/358


Hmm, $(LD) might be a full path.

Looks like Clang can take a full-path for -fuse-ld=,
but does it work for GCC?






> > > +endif
> >
> > This section needs to be moved up above KBUILD_CFLAGS otherwise it never
> > actually gets passed along.
> >
> > With that change, please add:
> >
> > Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> > Tested-by: Nathan Chancellor <natechancellor@gmail.com>
> >
> > >  export CLANG_FLAGS
> > >  endif
> > >
> > > --
> > > 2.20.1.791.gb4d0f1c61a-goog
> > >
>
>
>
> --
> Thanks,
> ~Nick Desaulniers



-- 
Best Regards
Masahiro Yamada

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

[Sedat Dilek]

On Mon, Feb 11, 2019 at 4:33 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
[ ... ]
> > > > Link: https://github.com/ClangBuiltLinux/linux/issues/342
> > > > Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
> > > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > > > ---
> > > >  Makefile | 3 +++
> > > >  1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/Makefile b/Makefile
> > > > index 0eae4277206e..6307c17259ea 100644
> > > > --- a/Makefile
> > > > +++ b/Makefile
> > > > @@ -502,6 +502,9 @@ endif
> > > >  CLANG_FLAGS  += -no-integrated-as
> > > >  KBUILD_CFLAGS        += $(CLANG_FLAGS)
> > > >  KBUILD_AFLAGS        += $(CLANG_FLAGS)
> > > > +ifneq ($(shell $(LD) --version 2>&1 | head -n 1 | grep LLD),)
> > > > +CLANG_FLAGS  += -fuse-ld=lld
> >
> > Sorry, Sedat reports [0] that this is not quite correct; if $(LD)
> > matches LLD, then we need to set -fuse-ld=$(LD), not -fuse-ld=lld,
> > since you may have lld installed as lld-7 or lld-8, for example:
> > https://github.com/ClangBuiltLinux/linux/issues/358
>
>
> Hmm, $(LD) might be a full path.
>
> Looks like Clang can take a full-path for -fuse-ld=,
> but does it work for GCC?
>

Nope. GCC expects "-fuse-ld={bfd,gold,lld} ".
Adding suffix "$-VER" was not recognized while building a
llvm-toolchain, so I guess this will be the same for the Linux kernel
side.

- Sedat -

[1] https://gcc.gnu.org/onlinedocs/gcc/Link-Options.html
[2] https://github.com/ClangBuiltLinux/linux/issues/358

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

[Nick Desaulniers]

On Thu, Feb 7, 2019 at 9:51 PM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> On Thu, Feb 07, 2019 at 02:01:51PM -0800, ndesaulniers@google.com wrote:
> > This is needed because clang doesn't select which linker to use based on
> > $LD but rather -fuse-ld=$(LD). 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>
> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > ---
> >  Makefile | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/Makefile b/Makefile
> > index 0eae4277206e..6307c17259ea 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -502,6 +502,9 @@ endif
> >  CLANG_FLAGS  += -no-integrated-as
> >  KBUILD_CFLAGS        += $(CLANG_FLAGS)
> >  KBUILD_AFLAGS        += $(CLANG_FLAGS)
> > +ifneq ($(shell $(LD) --version 2>&1 | head -n 1 | grep LLD),)
> > +CLANG_FLAGS  += -fuse-ld=lld
> > +endif
>
> This section needs to be moved up above KBUILD_CFLAGS otherwise it never
> actually gets passed along.

Great catch! I had that correct in my Pixel kernel patches; messed
that up forward porting them.  Included in v2 (sending imminently).

-- 
Thanks,
~Nick Desaulniers

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

[Nick Desaulniers]

On Mon, Feb 11, 2019 at 7:33 AM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> On Sat, Feb 9, 2019 at 2:27 AM Nick Desaulniers <ndesaulniers@google.com> wrote:
> >
> > On Thu, Feb 7, 2019 at 9:51 PM Nathan Chancellor
> > <natechancellor@gmail.com> wrote:
> > >
> > > On Thu, Feb 07, 2019 at 02:01:51PM -0800, ndesaulniers@google.com wrote:
> > > > This is needed because clang doesn't select which linker to use based on
> > > > $LD but rather -fuse-ld=$(LD). This is problematic especially for
> > > > cc-ldoption, which checks for linker flag support via invoking the
> > > > compiler, rather than the linker.
>
> What do you mean by 'problematic' ?
>
> I see no build error in
> arch/arm64/kernel/vdso/Makefile, at least.
>
> Clang uses BFD linker by default,
> which may not match to the $(LD) passed in.
>
> Is this what you mean?

Right, when using a linker other than BFD, we want cc-ldoption to tell
us whether that linker supports a linker flag or not.

-- 
Thanks,
~Nick Desaulniers

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

[Nick Desaulniers]

On Mon, Feb 11, 2019 at 8:05 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> On Mon, Feb 11, 2019 at 4:33 PM Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
> [ ... ]
> > > > > Link: https://github.com/ClangBuiltLinux/linux/issues/342
> > > > > Suggested-by: Nathan Chancellor <natechancellor@gmail.com>
> > > > > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>
> > > > > ---
> > > > >  Makefile | 3 +++
> > > > >  1 file changed, 3 insertions(+)
> > > > >
> > > > > diff --git a/Makefile b/Makefile
> > > > > index 0eae4277206e..6307c17259ea 100644
> > > > > --- a/Makefile
> > > > > +++ b/Makefile
> > > > > @@ -502,6 +502,9 @@ endif
> > > > >  CLANG_FLAGS  += -no-integrated-as
> > > > >  KBUILD_CFLAGS        += $(CLANG_FLAGS)
> > > > >  KBUILD_AFLAGS        += $(CLANG_FLAGS)
> > > > > +ifneq ($(shell $(LD) --version 2>&1 | head -n 1 | grep LLD),)
> > > > > +CLANG_FLAGS  += -fuse-ld=lld
> > >
> > > Sorry, Sedat reports [0] that this is not quite correct; if $(LD)
> > > matches LLD, then we need to set -fuse-ld=$(LD), not -fuse-ld=lld,
> > > since you may have lld installed as lld-7 or lld-8, for example:
> > > https://github.com/ClangBuiltLinux/linux/issues/358
> >
> >
> > Hmm, $(LD) might be a full path.
> >
> > Looks like Clang can take a full-path for -fuse-ld=,
> > but does it work for GCC?
> >
>
> Nope. GCC expects "-fuse-ld={bfd,gold,lld} ".
> Adding suffix "$-VER" was not recognized while building a
> llvm-toolchain, so I guess this will be the same for the Linux kernel
> side.

Well for now, let's add support for just `ld.lld` since a build of
llvm will create such a symlink.  If folks really need to
differentiate between different versions of lld, let's cross that
bridge when we get there.
-- 
Thanks,
~Nick Desaulniers

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

[Sedat Dilek]

Feel free to add my...

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

[1] https://github.com/ClangBuiltLinux/linux/issues/341#issuecomment-459788558