| Message ID | 20200423171807.29713-3-natechancellor@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3,1/4] kbuild: add CONFIG_LD_IS_LLD | expand |
On Thu, Apr 23, 2020 at 10:18:06AM -0700, Nathan Chancellor wrote: > Currently, the VDSO is being linked through $(CC). This does not match > how the rest of the kernel links objects, which is through the $(LD) > variable. this causes build errors for me when (cross) compiling a big endian target: target is little endian mips64-linux-gnu-ld: arch/mips/vdso/elf.o: endianness incompatible with that of the selected emulation mips64-linux-gnu-ld: failed to merge target specific data of file arch/mips/vdso/elf.o Thomas.
On Sun, Apr 26, 2020 at 06:27:37PM +0200, Thomas Bogendoerfer wrote: > On Thu, Apr 23, 2020 at 10:18:06AM -0700, Nathan Chancellor wrote: > > Currently, the VDSO is being linked through $(CC). This does not match > > how the rest of the kernel links objects, which is through the $(LD) > > variable. > > this causes build errors for me when (cross) compiling a big endian target: > > target is little endian > mips64-linux-gnu-ld: arch/mips/vdso/elf.o: endianness incompatible with that of the selected emulation > mips64-linux-gnu-ld: failed to merge target specific data of file arch/mips/vdso/elf.o Thanks for the report. I will look into it tomorrow and hopefully have a v4 by then. Cheers, Nathan
On Sun, 26 Apr 2020, Nathan Chancellor wrote: > > this causes build errors for me when (cross) compiling a big endian target: > > > > target is little endian > > mips64-linux-gnu-ld: arch/mips/vdso/elf.o: endianness incompatible with that of the selected emulation > > mips64-linux-gnu-ld: failed to merge target specific data of file arch/mips/vdso/elf.o > > Thanks for the report. I will look into it tomorrow and hopefully have a > v4 by then. Can you actually record in the change description what the difference in the relevant link command is, as shown where `V=1' has been used with `make' invocation? Actually running `diff -bu' on the whole `V=1' build log obtained without and with your proposed change applied and ensuring there are no unwanted changes elsewhere will be a good measure of the correctness of your patch. You may have to prepare to be patient and run with `-j1' to make sure any `make' parallelism does not interfere with the order of commands printed. Maciej
On Mon, Apr 27, 2020 at 05:22:53PM +0100, Maciej W. Rozycki wrote: > On Sun, 26 Apr 2020, Nathan Chancellor wrote: > > > > this causes build errors for me when (cross) compiling a big endian target: > > > > > > target is little endian > > > mips64-linux-gnu-ld: arch/mips/vdso/elf.o: endianness incompatible with that of the selected emulation > > > mips64-linux-gnu-ld: failed to merge target specific data of file arch/mips/vdso/elf.o > > > > Thanks for the report. I will look into it tomorrow and hopefully have a > > v4 by then. > > Can you actually record in the change description what the difference in > the relevant link command is, as shown where `V=1' has been used with > `make' invocation? That will be rather unweildy to put in the commit message since currently, $(CC) + $(KBUILD_CFLAGS) is being used but I can if it is really desired. Otherwise, I can just put it where I put the changelog. > Actually running `diff -bu' on the whole `V=1' build log obtained without > and with your proposed change applied and ensuring there are no unwanted > changes elsewhere will be a good measure of the correctness of your patch. > You may have to prepare to be patient and run with `-j1' to make sure any > `make' parallelism does not interfere with the order of commands printed. > > Maciej > Thanks for the input, I will take a look. Cheers, Nathan
On Sun, Apr 26, 2020 at 06:27:37PM +0200, Thomas Bogendoerfer wrote: > On Thu, Apr 23, 2020 at 10:18:06AM -0700, Nathan Chancellor wrote: > > Currently, the VDSO is being linked through $(CC). This does not match > > how the rest of the kernel links objects, which is through the $(LD) > > variable. > > this causes build errors for me when (cross) compiling a big endian target: > > target is little endian > mips64-linux-gnu-ld: arch/mips/vdso/elf.o: endianness incompatible with that of the selected emulation > mips64-linux-gnu-ld: failed to merge target specific data of file arch/mips/vdso/elf.o > > Thomas. > Thanks for reporting this, I figured it out. This is the solution that I came up with, I'll send out a v4 tomorrow once I do some more testing. Cheers, Nathan From 256e3b6c8fff7a66aa29961ebefc0fe653ec34b6 Mon Sep 17 00:00:00 2001 From: Nathan Chancellor <natechancellor@gmail.com> Date: Mon, 27 Apr 2020 17:02:55 -0700 Subject: [PATCH] MIPS: Unconditionally specify '-EL' or '-EB' This was all done to work around a GCC bug that has been fixed after 4.2. The kernel requires GCC 4.6 or newer so remove all of these hacks and just use the traditional flags. $ mips64-linux-gcc --version | head -n1 mips64-linux-gcc (GCC) 4.6.3 $ mips64-linux-gcc -EB -dM -E -C -x c /dev/null | grep MIPSE #define MIPSEB 1 #define __MIPSEB__ 1 #define _MIPSEB 1 #define __MIPSEB 1 $ mips64-linux-gcc -EL -dM -E -C -x c /dev/null | grep MIPSE #define __MIPSEL__ 1 #define MIPSEL 1 #define _MIPSEL 1 #define __MIPSEL 1 This is necessary when converting the MIPS VDSO to use $(LD) instead of $(CC) to link because the OUTPUT_FORMAT is defaulted to little endian and only flips to big endian when -EB is set on the command line, which is inherited from KBUILD_CFLAGS. Without this, we will see the following error when compiling for big endian (64r2_defconfig): $ make -j$(nproc) ARCH=mips CROSS_COMPILE=mips64-linux- \ 64r2el_defconfig arch/mips/vdso/ ... mips64-linux-ld: arch/mips/vdso/elf.o: compiled for a big endian system and target is little endian mips64-linux-ld: arch/mips/vdso/elf.o: endianness incompatible with that of the selected emulation mips64-linux-ld: failed to merge target specific data of file arch/mips/vdso/elf.o ... Reported-by: Thomas Bogendoerfer <tsbogend@alpha.franken.de> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> --- arch/mips/Makefile | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/arch/mips/Makefile b/arch/mips/Makefile index e1c44aed81565..301efb90b51ed 100644 --- a/arch/mips/Makefile +++ b/arch/mips/Makefile @@ -116,33 +116,8 @@ endif cflags-y += -ffreestanding -# -# We explicitly add the endianness specifier if needed, this allows -# to compile kernels with a toolchain for the other endianness. We -# carefully avoid to add it redundantly because gcc 3.3/3.4 complains -# when fed the toolchain default! -# -# Certain gcc versions up to gcc 4.1.1 (probably 4.2-subversion as of -# 2006-10-10 don't properly change the predefined symbols if -EB / -EL -# are used, so we kludge that here. A bug has been filed at -# http://gcc.gnu.org/bugzilla/show_bug.cgi?id=29413. -# -# clang doesn't suffer from these issues and our checks against -dumpmachine -# don't work so well when cross compiling, since without providing --target -# clang's output will be based upon the build machine. So for clang we simply -# unconditionally specify -EB or -EL as appropriate. -# -ifdef CONFIG_CC_IS_CLANG cflags-$(CONFIG_CPU_BIG_ENDIAN) += -EB cflags-$(CONFIG_CPU_LITTLE_ENDIAN) += -EL -else -undef-all += -UMIPSEB -U_MIPSEB -U__MIPSEB -U__MIPSEB__ -undef-all += -UMIPSEL -U_MIPSEL -U__MIPSEL -U__MIPSEL__ -predef-be += -DMIPSEB -D_MIPSEB -D__MIPSEB -D__MIPSEB__ -predef-le += -DMIPSEL -D_MIPSEL -D__MIPSEL -D__MIPSEL__ -cflags-$(CONFIG_CPU_BIG_ENDIAN) += $(shell $(CC) -dumpmachine |grep -q 'mips.*el-.*' && echo -EB $(undef-all) $(predef-be)) -cflags-$(CONFIG_CPU_LITTLE_ENDIAN) += $(shell $(CC) -dumpmachine |grep -q 'mips.*el-.*' || echo -EL $(undef-all) $(predef-le)) -endif cflags-$(CONFIG_SB1XXX_CORELIS) += $(call cc-option,-mno-sched-prolog) \ -fno-omit-frame-pointer
On Mon, 27 Apr 2020, Nathan Chancellor wrote: > > Can you actually record in the change description what the difference in > > the relevant link command is, as shown where `V=1' has been used with > > `make' invocation? > > That will be rather unweildy to put in the commit message since > currently, $(CC) + $(KBUILD_CFLAGS) is being used but I can if it is > really desired. Otherwise, I can just put it where I put the changelog. Umm, is the difference so huge? I think a note along the lines of: "[...] This change adds/removes[*]: <part of the command affected> from the invocation of [...], which is required for [...]" -- only quoting what's actually changed will be sufficient. Reword as required. Otherwise it's hard to guess now what the change actually does, and it will be even harder for someone who comes across it and tries to understand it the future, when the context might be hard to reproduce. [*] Delete as appropriate. Maciej
On Wed, Apr 29, 2020 at 06:46:33PM +0100, Maciej W. Rozycki wrote: > On Mon, 27 Apr 2020, Nathan Chancellor wrote: > > > > Can you actually record in the change description what the difference in > > > the relevant link command is, as shown where `V=1' has been used with > > > `make' invocation? > > > > That will be rather unweildy to put in the commit message since > > currently, $(CC) + $(KBUILD_CFLAGS) is being used but I can if it is > > really desired. Otherwise, I can just put it where I put the changelog. > > Umm, is the difference so huge? I think a note along the lines of: > > "[...] This change adds/removes[*]: > > <part of the command affected> > > from the invocation of [...], which is required for [...]" > > -- only quoting what's actually changed will be sufficient. Reword as > required. Otherwise it's hard to guess now what the change actually does, > and it will be even harder for someone who comes across it and tries to > understand it the future, when the context might be hard to reproduce. > > [*] Delete as appropriate. > > Maciej I ended up figuring out a way to get the difference proper into the commit message in v4. Please take a look. Cheers, Nathan
diff --git a/arch/mips/vdso/Makefile b/arch/mips/vdso/Makefile index 92b53d1df42c3..da5db947072d5 100644 --- a/arch/mips/vdso/Makefile +++ b/arch/mips/vdso/Makefile @@ -60,10 +60,9 @@ ifdef CONFIG_MIPS_DISABLE_VDSO endif # VDSO linker flags. -VDSO_LDFLAGS := \ - -Wl,-Bsymbolic -Wl,--no-undefined -Wl,-soname=linux-vdso.so.1 \ - $(addprefix -Wl$(comma),$(filter -E%,$(KBUILD_CFLAGS))) \ - -nostdlib -shared -Wl,--hash-style=sysv -Wl,--build-id +ldflags-y := -Bsymbolic --no-undefined -soname=linux-vdso.so.1 \ + $(filter -E%,$(KBUILD_CFLAGS)) -nostdlib -shared \ + --hash-style=sysv --build-id -T CFLAGS_REMOVE_vdso.o = -pg @@ -82,11 +81,7 @@ quiet_cmd_vdso_mips_check = VDSOCHK $@ # quiet_cmd_vdsold_and_vdso_check = LD $@ - cmd_vdsold_and_vdso_check = $(cmd_vdsold); $(cmd_vdso_check); $(cmd_vdso_mips_check) - -quiet_cmd_vdsold = VDSO $@ - cmd_vdsold = $(CC) $(c_flags) $(VDSO_LDFLAGS) \ - -Wl,-T $(filter %.lds,$^) $(filter %.o,$^) -o $@ + cmd_vdsold_and_vdso_check = $(cmd_ld); $(cmd_vdso_check); $(cmd_vdso_mips_check) quiet_cmd_vdsoas_o_S = AS $@ cmd_vdsoas_o_S = $(CC) $(a_flags) -c -o $@ $<
Currently, the VDSO is being linked through $(CC). This does not match how the rest of the kernel links objects, which is through the $(LD) variable. When clang is built in a default configuration, it first attempts to use the target triple's default linker then the system's default linker, unless told otherwise through -fuse-ld=... We do not use -fuse-ld= because it can be brittle and we have support for invoking $(LD) directly. See commit fe00e50b2db8c ("ARM: 8858/1: vdso: use $(LD) instead of $(CC) to link VDSO") and commit 691efbedc60d2 ("arm64: vdso: use $(LD) instead of $(CC) to link VDSO") for examples of doing this in the VDSO. Do the same thing here. Replace the custom linking logic with $(cmd_ld) and ldflags-y so that $(LD) is respected. Before this patch, LD=ld.lld did nothing: $ llvm-readelf -p.comment arch/mips/vdso/vdso.so.dbg | sed 's/(.*//' String dump of section '.comment': [ 0] ClangBuiltLinux clang version 11.0.0 After this patch, it does: $ llvm-readelf -p.comment arch/mips/vdso/vdso.so.dbg | sed 's/(.*//' String dump of section '.comment': [ 0] Linker: LLD 11.0.0 [ 62] ClangBuiltLinux clang version 11.0.0 Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> --- v2 -> v3: * New patch. arch/mips/vdso/Makefile | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-)