2019-12-04 22:56:39

by Dmitry Golovin

[permalink] [raw]
Subject: [PATCH] x86/boot: kbuild: allow readelf executable to be specified

Introduce a new READELF variable to top-level Makefile, so the name of
readelf binary can be specified.

Before this change the name of the binary was hardcoded to
"$(CROSS_COMPILE)readelf" which might not be present for every
toolchain.

This allows to build with LLVM Object Reader by using make parameter
READELF=llvm-readelf.

Link: https://github.com/ClangBuiltLinux/linux/issues/771
Signed-off-by: Dmitry Golovin <[email protected]>
Cc: Nick Desaulniers <[email protected]>
---
Makefile | 3 ++-
arch/x86/boot/compressed/Makefile | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 999a197d67d2..612a55d25442 100644
--- a/Makefile
+++ b/Makefile
@@ -414,6 +414,7 @@ STRIP = $(CROSS_COMPILE)strip
OBJCOPY = $(CROSS_COMPILE)objcopy
OBJDUMP = $(CROSS_COMPILE)objdump
OBJSIZE = $(CROSS_COMPILE)size
+READELF = $(CROSS_COMPILE)readelf
PAHOLE = pahole
LEX = flex
YACC = bison
@@ -472,7 +473,7 @@ GCC_PLUGINS_CFLAGS :=
CLANG_FLAGS :=

export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS CROSS_COMPILE AS LD CC
-export CPP AR NM STRIP OBJCOPY OBJDUMP OBJSIZE PAHOLE LEX YACC AWK INSTALLKERNEL
+export CPP AR NM STRIP OBJCOPY OBJDUMP OBJSIZE READELF PAHOLE LEX YACC AWK INSTALLKERNEL
export PERL PYTHON PYTHON2 PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX
export KBUILD_HOSTCXXFLAGS KBUILD_HOSTLDFLAGS KBUILD_HOSTLDLIBS LDFLAGS_MODULE

diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index aa976adb7094..1dac210f7d44 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -103,7 +103,7 @@ vmlinux-objs-$(CONFIG_EFI_MIXED) += $(obj)/efi_thunk_$(BITS).o
quiet_cmd_check_data_rel = DATAREL $@
define cmd_check_data_rel
for obj in $(filter %.o,$^); do \
- ${CROSS_COMPILE}readelf -S $$obj | grep -qF .rel.local && { \
+ $(READELF) -S $$obj | grep -qF .rel.local && { \
echo "error: $$obj has data relocations!" >&2; \
exit 1; \
} || true; \
--
2.23.0


2019-12-04 23:19:03

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH] x86/boot: kbuild: allow readelf executable to be specified

On Wed, Dec 4, 2019 at 2:55 PM Dmitry Golovin <[email protected]> wrote:
>
> Introduce a new READELF variable to top-level Makefile, so the name of
> readelf binary can be specified.

Thanks for the patch!

This is a general cleanup that should improve cross compilation
(readelf should be treated as ever binary in the list like
objcopy/objdump/etc), and allow us to use binutils substitutes for
readelf that aren't called `readelf` (ie. `llvm-readelf`).

>
> Before this change the name of the binary was hardcoded to
> "$(CROSS_COMPILE)readelf" which might not be present for every
> toolchain.
>
> This allows to build with LLVM Object Reader by using make parameter
> READELF=llvm-readelf.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/771
> Signed-off-by: Dmitry Golovin <[email protected]>
> Cc: Nick Desaulniers <[email protected]>

No need to explicitly CC me; I monitor our list like a hawk.

> ---
> Makefile | 3 ++-
> arch/x86/boot/compressed/Makefile | 2 +-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/Makefile b/Makefile
> index 999a197d67d2..612a55d25442 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -414,6 +414,7 @@ STRIP = $(CROSS_COMPILE)strip
> OBJCOPY = $(CROSS_COMPILE)objcopy
> OBJDUMP = $(CROSS_COMPILE)objdump
> OBJSIZE = $(CROSS_COMPILE)size
> +READELF = $(CROSS_COMPILE)readelf
> PAHOLE = pahole
> LEX = flex
> YACC = bison
> @@ -472,7 +473,7 @@ GCC_PLUGINS_CFLAGS :=
> CLANG_FLAGS :=
>
> export ARCH SRCARCH CONFIG_SHELL BASH HOSTCC KBUILD_HOSTCFLAGS CROSS_COMPILE AS LD CC
> -export CPP AR NM STRIP OBJCOPY OBJDUMP OBJSIZE PAHOLE LEX YACC AWK INSTALLKERNEL
> +export CPP AR NM STRIP OBJCOPY OBJDUMP OBJSIZE READELF PAHOLE LEX YACC AWK INSTALLKERNEL
> export PERL PYTHON PYTHON2 PYTHON3 CHECK CHECKFLAGS MAKE UTS_MACHINE HOSTCXX
> export KBUILD_HOSTCXXFLAGS KBUILD_HOSTLDFLAGS KBUILD_HOSTLDLIBS LDFLAGS_MODULE
>
> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> index aa976adb7094..1dac210f7d44 100644
> --- a/arch/x86/boot/compressed/Makefile
> +++ b/arch/x86/boot/compressed/Makefile
> @@ -103,7 +103,7 @@ vmlinux-objs-$(CONFIG_EFI_MIXED) += $(obj)/efi_thunk_$(BITS).o
> quiet_cmd_check_data_rel = DATAREL $@
> define cmd_check_data_rel
> for obj in $(filter %.o,$^); do \
> - ${CROSS_COMPILE}readelf -S $$obj | grep -qF .rel.local && { \
> + $(READELF) -S $$obj | grep -qF .rel.local && { \

Grepping the kernel sources for `READELF`, it looks like
arch/ia64/Makefile makes the same mistake. Would you mind fixing both
cases in the same patch (v2)? I'm also curious about it's use in
arch/ia64/scripts/unwcheck.py, and scripts/faddr2line. +ia64
maintainers and list.

I think if you simply remove the assignment on line 17 of
arch/ia64/Makefile you should be fine.

> echo "error: $$obj has data relocations!" >&2; \
> exit 1; \
> } || true; \
> --
> 2.23.0
>


--
Thanks,
~Nick Desaulniers

2019-12-05 17:07:06

by Nick Desaulniers

[permalink] [raw]
Subject: Re: [PATCH] x86/boot: kbuild: allow readelf executable to be specified

On Thu, Dec 5, 2019 at 8:25 AM Masahiro Yamada <[email protected]> wrote:
>
>
>
> On Thu, Dec 5, 2019 at 9:41 AM Dmitry Golovin <[email protected]> wrote:
>>
>> 05.12.2019, 01:18, "'Nick Desaulniers' via Clang Built Linux" <[email protected]>:
>> >
>> > Grepping the kernel sources for `READELF`, it looks like
>> > arch/ia64/Makefile makes the same mistake. Would you mind fixing both
>> > cases in the same patch (v2)? I'm also curious about it's use in
>> > arch/ia64/scripts/unwcheck.py, and scripts/faddr2line. +ia64
>> > maintainers and list.
>> >
>> > I think if you simply remove the assignment on line 17 of
>> > arch/ia64/Makefile you should be fine.
>>
>> Perhaps something should be done to NM on line 16 of this file as well. Also
>> found similar invocation of `objcopy` in arch/riscv/kernel/vdso/Makefile.
>> I think IA64 and RISC-V changes should be made as separate commits.
>>
>> -- Dmitry
>
>
>
> I am fine with either way.
> I can take this patch, and also a one for IA64.

Whichever, as long as we don't have to wait for the base patch to land
via kbuild tree, then submit arch specific ones to their respective
trees months later.
Reviewed-by: Nick Desaulniers <[email protected]>

>
>
>
> You can send a patch for arch/riscv/kernel/vdso/Makefile
> to the riscv maintainer since it has no patch dependency.
>
>
> --
> Best Regards
> Masahiro Yamada



--
Thanks,
~Nick Desaulniers

2019-12-05 18:00:03

by Masahiro Yamada

[permalink] [raw]
Subject: Re: [PATCH] x86/boot: kbuild: allow readelf executable to be specified

On Thu, Dec 5, 2019 at 9:41 AM Dmitry Golovin <[email protected]> wrote:
>
> 05.12.2019, 01:18, "'Nick Desaulniers' via Clang Built Linux" <[email protected]>:
> >
> > Grepping the kernel sources for `READELF`, it looks like
> > arch/ia64/Makefile makes the same mistake. Would you mind fixing both
> > cases in the same patch (v2)? I'm also curious about it's use in
> > arch/ia64/scripts/unwcheck.py, and scripts/faddr2line. +ia64
> > maintainers and list.
> >
> > I think if you simply remove the assignment on line 17 of
> > arch/ia64/Makefile you should be fine.
>
> Perhaps something should be done to NM on line 16 of this file as well. Also
> found similar invocation of `objcopy` in arch/riscv/kernel/vdso/Makefile.
> I think IA64 and RISC-V changes should be made as separate commits.
>
> -- Dmitry

I am fine with either way.
I can take this patch, and also a one for IA64.

You can send a patch for arch/riscv/kernel/vdso/Makefile
to the riscv maintainer since it has no patch dependency.


--
Best Regards
Masahiro Yamada