diff mbox series

powerpc: vdso: fix building with wrong-endian toolchain

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

Commit Message

Arnd Bergmann June 7, 2024, 6:16 a.m. UTC 
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.
---
 arch/powerpc/kernel/vdso/vdso32.lds.S | 2 +-
 arch/powerpc/kernel/vdso/vdso64.lds.S | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Comments

Michael Ellerman June 7, 2024, 12:42 p.m. UTC | #1 
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
Arnd Bergmann June 7, 2024, 2:11 p.m. UTC | #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
Nathan Chancellor June 7, 2024, 2:43 p.m. UTC | #3 
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
Segher Boessenkool June 7, 2024, 3:41 p.m. UTC | #4 
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 mbox series

Patch

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