| Message ID | 20240607061629.530301-1-arnd@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | powerpc: vdso: fix building with wrong-endian toolchain | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 6 jobs. |
| snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 23 jobs. |
Arnd Bergmann <arnd@kernel.org> writes: > From: Arnd Bergmann <arnd@arndb.de> > > Building powerpc64le kernels with the kernel.org crosstool toolchains > no longer works as the linker attempts to build a big-endian vdso: > > powerpc-linux/lib/gcc/powerpc-linux/12.3.0/../../../../powerpc-linux/bin/ld: arch/powerpc/kernel/vdso/sigtramp32-32.o: compiled for a little endian system and target is big endian > powerpc-linux/lib/gcc/powerpc-linux/12.3.0/../../../../powerpc-linux/bin/ld: failed to merge target specific data of file arch/powerpc/kernel/vdso/sigtramp32-32.o > > Apparently creating the vdso.lds files from the lds.S files fails to > pass the -mlittle-endian argument here, so the output format gets set > wrong. Changing the conditional to check for CONFIG_CPU_LITTLE_ENDIAN > instead still works, as the kernel configuration definitions are visible. > > Signed-off-by: Arnd Bergmann <arnd@arndb.de> > --- > I'm fairly sure this worked in the past, but I did not try to bisect the > issue. It still works for me. I use the korg toolchains every day, and kisskb uses them too. What commit / defconfig are you seeing the errors with? Is it just the 12.3.0 toolchain or all of them? I just tested 12.3.0 here and it built OK. I guess you're building on x86 or arm64? I build on ppc64le, I wonder if that makes a difference. The patch is probably OK regardless, but I'd rather understand what the actual problem is. cheers > diff --git a/arch/powerpc/kernel/vdso/vdso32.lds.S b/arch/powerpc/kernel/vdso/vdso32.lds.S > index 426e1ccc6971..5845ea2d1cba 100644 > --- a/arch/powerpc/kernel/vdso/vdso32.lds.S > +++ b/arch/powerpc/kernel/vdso/vdso32.lds.S > @@ -7,7 +7,7 @@ > #include <asm/page.h> > #include <asm-generic/vmlinux.lds.h> > > -#ifdef __LITTLE_ENDIAN__ > +#ifdef CONFIG_CPU_LITTLE_ENDIAN > OUTPUT_FORMAT("elf32-powerpcle", "elf32-powerpcle", "elf32-powerpcle") > #else > OUTPUT_FORMAT("elf32-powerpc", "elf32-powerpc", "elf32-powerpc") > diff --git a/arch/powerpc/kernel/vdso/vdso64.lds.S b/arch/powerpc/kernel/vdso/vdso64.lds.S > index bda6c8cdd459..82c418b18cce 100644 > --- a/arch/powerpc/kernel/vdso/vdso64.lds.S > +++ b/arch/powerpc/kernel/vdso/vdso64.lds.S > @@ -7,7 +7,7 @@ > #include <asm/page.h> > #include <asm-generic/vmlinux.lds.h> > > -#ifdef __LITTLE_ENDIAN__ > +#ifdef CONFIG_CPU_LITTLE_ENDIAN > OUTPUT_FORMAT("elf64-powerpcle", "elf64-powerpcle", "elf64-powerpcle") > #else > OUTPUT_FORMAT("elf64-powerpc", "elf64-powerpc", "elf64-powerpc") > -- > 2.39.2
On Fri, Jun 7, 2024, at 14:42, Michael Ellerman wrote: > Arnd Bergmann <arnd@kernel.org> writes: >> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de> >> --- >> I'm fairly sure this worked in the past, but I did not try to bisect the >> issue. > > It still works for me. > > I use the korg toolchains every day, and kisskb uses them too. > > What commit / defconfig are you seeing the errors with? > > Is it just the 12.3.0 toolchain or all of them? I just tested 12.3.0 > here and it built OK. > > I guess you're building on x86 or arm64? I build on ppc64le, I wonder if > that makes a difference. > > The patch is probably OK regardless, but I'd rather understand what the > actual problem is. I tested again and found that the problem is actually part of my local build setup, which overrides the 'CPP' variable in the top-level makefile that I use for building multiple kernels concurrently. This ends up clashing with this other line that only powerpc sets: arch/powerpc/Makefile:CPP = $(CC) -E $(KBUILD_CFLAGS) It's rare that someone overrides CPP, so quite possibly I'm the only one that has seen this so far, but it also seems like it should be possible to do that. This patch seems to work as well for me, and is a little more logical, but it's also more invasive and has a higher regression risk: 8<--------- diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile index 65261cbe5bfd..9ad4ca318e34 100644 --- a/arch/powerpc/Makefile +++ b/arch/powerpc/Makefile @@ -62,14 +62,14 @@ KBUILD_LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o endif ifdef CONFIG_CPU_LITTLE_ENDIAN -KBUILD_CFLAGS += -mlittle-endian +KBUILD_CPPFLAGS += -mlittle-endian KBUILD_LDFLAGS += -EL LDEMULATION := lppc GNUTARGET := powerpcle MULTIPLEWORD := -mno-multiple KBUILD_CFLAGS_MODULE += $(call cc-option,-mno-save-toc-indirect) else -KBUILD_CFLAGS += $(call cc-option,-mbig-endian) +KBUILD_CPPFLAGS += $(call cc-option,-mbig-endian) KBUILD_LDFLAGS += -EB LDEMULATION := ppc GNUTARGET := powerpc @@ -95,7 +95,7 @@ aflags-$(CONFIG_CPU_BIG_ENDIAN) += $(call cc-option,-mbig-endian) aflags-$(CONFIG_CPU_LITTLE_ENDIAN) += -mlittle-endian ifeq ($(HAS_BIARCH),y) -KBUILD_CFLAGS += -m$(BITS) +KBUILD_CPPFLAGS += -m$(BITS) KBUILD_AFLAGS += -m$(BITS) KBUILD_LDFLAGS += -m elf$(BITS)$(LDEMULATION) endif @@ -176,7 +176,6 @@ KBUILD_CPPFLAGS += -I $(srctree)/arch/powerpc $(asinstr) KBUILD_AFLAGS += $(AFLAGS-y) KBUILD_CFLAGS += $(call cc-option,-msoft-float) KBUILD_CFLAGS += $(CFLAGS-y) -CPP = $(CC) -E $(KBUILD_CFLAGS) CHECKFLAGS += -m$(BITS) -D__powerpc__ -D__powerpc$(BITS)__ ifdef CONFIG_CPU_BIG_ENDIAN diff --git a/arch/powerpc/kernel/vdso/Makefile b/arch/powerpc/kernel/vdso/Makefile index 1b93655c2857..3516e71926e5 100644 --- a/arch/powerpc/kernel/vdso/Makefile +++ b/arch/powerpc/kernel/vdso/Makefile @@ -59,7 +59,7 @@ ldflags-$(CONFIG_LD_IS_LLD) += $(call cc-option,--ld-path=$(LD),-fuse-ld=lld) ldflags-$(CONFIG_LD_ORPHAN_WARN) += -Wl,--orphan-handling=$(CONFIG_LD_ORPHAN_WARN_LEVEL) # Filter flags that clang will warn are unused for linking -ldflags-y += $(filter-out $(CC_AUTO_VAR_INIT_ZERO_ENABLER) $(CC_FLAGS_FTRACE) -Wa$(comma)%, $(KBUILD_CFLAGS)) +ldflags-y += $(filter-out $(CC_AUTO_VAR_INIT_ZERO_ENABLER) $(CC_FLAGS_FTRACE) -Wa$(comma)%, $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS)) CC32FLAGS := -m32 LD32FLAGS := -Wl,-soname=linux-vdso32.so.1 --------->8 Arnd
On Fri, Jun 07, 2024 at 04:11:25PM +0200, Arnd Bergmann wrote: > This patch seems to work as well for me, and is a little > more logical, but it's also more invasive and has a > higher regression risk: Commit feb843a469fb ("kbuild: add $(CLANG_FLAGS) to KBUILD_CPPFLAGS") did something similar for clang for the same reason, so I would say it is worth pursuing this direction. It also avoids including KBUILD_CFLAGS twice when generating .i files. Cheers, Nathan > 8<--------- > diff --git a/arch/powerpc/Makefile b/arch/powerpc/Makefile > index 65261cbe5bfd..9ad4ca318e34 100644 > --- a/arch/powerpc/Makefile > +++ b/arch/powerpc/Makefile > @@ -62,14 +62,14 @@ KBUILD_LDFLAGS_MODULE += arch/powerpc/lib/crtsavres.o > endif > > ifdef CONFIG_CPU_LITTLE_ENDIAN > -KBUILD_CFLAGS += -mlittle-endian > +KBUILD_CPPFLAGS += -mlittle-endian > KBUILD_LDFLAGS += -EL > LDEMULATION := lppc > GNUTARGET := powerpcle > MULTIPLEWORD := -mno-multiple > KBUILD_CFLAGS_MODULE += $(call cc-option,-mno-save-toc-indirect) > else > -KBUILD_CFLAGS += $(call cc-option,-mbig-endian) > +KBUILD_CPPFLAGS += $(call cc-option,-mbig-endian) > KBUILD_LDFLAGS += -EB > LDEMULATION := ppc > GNUTARGET := powerpc > @@ -95,7 +95,7 @@ aflags-$(CONFIG_CPU_BIG_ENDIAN) += $(call cc-option,-mbig-endian) > aflags-$(CONFIG_CPU_LITTLE_ENDIAN) += -mlittle-endian > > ifeq ($(HAS_BIARCH),y) > -KBUILD_CFLAGS += -m$(BITS) > +KBUILD_CPPFLAGS += -m$(BITS) > KBUILD_AFLAGS += -m$(BITS) > KBUILD_LDFLAGS += -m elf$(BITS)$(LDEMULATION) > endif > @@ -176,7 +176,6 @@ KBUILD_CPPFLAGS += -I $(srctree)/arch/powerpc $(asinstr) > KBUILD_AFLAGS += $(AFLAGS-y) > KBUILD_CFLAGS += $(call cc-option,-msoft-float) > KBUILD_CFLAGS += $(CFLAGS-y) > -CPP = $(CC) -E $(KBUILD_CFLAGS) > > CHECKFLAGS += -m$(BITS) -D__powerpc__ -D__powerpc$(BITS)__ > ifdef CONFIG_CPU_BIG_ENDIAN > diff --git a/arch/powerpc/kernel/vdso/Makefile b/arch/powerpc/kernel/vdso/Makefile > index 1b93655c2857..3516e71926e5 100644 > --- a/arch/powerpc/kernel/vdso/Makefile > +++ b/arch/powerpc/kernel/vdso/Makefile > @@ -59,7 +59,7 @@ ldflags-$(CONFIG_LD_IS_LLD) += $(call cc-option,--ld-path=$(LD),-fuse-ld=lld) > ldflags-$(CONFIG_LD_ORPHAN_WARN) += -Wl,--orphan-handling=$(CONFIG_LD_ORPHAN_WARN_LEVEL) > > # Filter flags that clang will warn are unused for linking > -ldflags-y += $(filter-out $(CC_AUTO_VAR_INIT_ZERO_ENABLER) $(CC_FLAGS_FTRACE) -Wa$(comma)%, $(KBUILD_CFLAGS)) > +ldflags-y += $(filter-out $(CC_AUTO_VAR_INIT_ZERO_ENABLER) $(CC_FLAGS_FTRACE) -Wa$(comma)%, $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS)) > > CC32FLAGS := -m32 > LD32FLAGS := -Wl,-soname=linux-vdso32.so.1 > --------->8 > > Arnd
On Fri, Jun 07, 2024 at 10:42:44PM +1000, Michael Ellerman wrote: > I use the korg toolchains every day, and kisskb uses them too. > > What commit / defconfig are you seeing the errors with? > > Is it just the 12.3.0 toolchain or all of them? I just tested 12.3.0 > here and it built OK. > > I guess you're building on x86 or arm64? I build on ppc64le, I wonder if > that makes a difference. The core problem of course is pre-processing a linker script with the C preprocessor (although linker scripts themselves have much more capable facilities for this), and by doing this as-if it was a piece of assembler code that for some strange reason you want fed through the C preprocessor (as .S file). What is it the C preprocessor is wanted for here? Is there nothing better that can be done? > The patch is probably OK regardless, but I'd rather understand what the > actual problem is. Yeah. The problem was found later in the thread (the CPP env var, or shell var anyway, not sure what it is here) was set. Fun and surprising! If you do nnot like such fun all that much, reduce the surface of eternal surprise? (I don't like saying "attack surface", but that is what it is as well). Segher
diff --git a/arch/powerpc/kernel/vdso/vdso32.lds.S b/arch/powerpc/kernel/vdso/vdso32.lds.S index 426e1ccc6971..5845ea2d1cba 100644 --- a/arch/powerpc/kernel/vdso/vdso32.lds.S +++ b/arch/powerpc/kernel/vdso/vdso32.lds.S @@ -7,7 +7,7 @@ #include <asm/page.h> #include <asm-generic/vmlinux.lds.h> -#ifdef __LITTLE_ENDIAN__ +#ifdef CONFIG_CPU_LITTLE_ENDIAN OUTPUT_FORMAT("elf32-powerpcle", "elf32-powerpcle", "elf32-powerpcle") #else OUTPUT_FORMAT("elf32-powerpc", "elf32-powerpc", "elf32-powerpc") diff --git a/arch/powerpc/kernel/vdso/vdso64.lds.S b/arch/powerpc/kernel/vdso/vdso64.lds.S index bda6c8cdd459..82c418b18cce 100644 --- a/arch/powerpc/kernel/vdso/vdso64.lds.S +++ b/arch/powerpc/kernel/vdso/vdso64.lds.S @@ -7,7 +7,7 @@ #include <asm/page.h> #include <asm-generic/vmlinux.lds.h> -#ifdef __LITTLE_ENDIAN__ +#ifdef CONFIG_CPU_LITTLE_ENDIAN OUTPUT_FORMAT("elf64-powerpcle", "elf64-powerpcle", "elf64-powerpcle") #else OUTPUT_FORMAT("elf64-powerpc", "elf64-powerpc", "elf64-powerpc")