s390 misaligned symbol linker errors

[nathanchance]

After commit 906f69cf65d ("IBM zSystems: Issue error for *DBL relocs on misaligned symbols") in binutils master, I am seeing the following link time error with tinyconfig:

$ make -skj"$(nproc)" ARCH=s390 CC=clang CROSS_COMPILE=s390x-linux-gnu- tinyconfig all
Using .config as base
Merging ./kernel/configs/tiny.config
Value of CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE is redefined by fragment ./kernel/configs/tiny.config:
Previous value: CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE=y
New value: # CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE is not set

Value of CONFIG_CC_OPTIMIZE_FOR_SIZE is redefined by fragment ./kernel/configs/tiny.config:
Previous value: # CONFIG_CC_OPTIMIZE_FOR_SIZE is not set
New value: CONFIG_CC_OPTIMIZE_FOR_SIZE=y

Value of CONFIG_KERNEL_GZIP is redefined by fragment ./kernel/configs/tiny.config:
Previous value: CONFIG_KERNEL_GZIP=y
New value: # CONFIG_KERNEL_GZIP is not set

Value of CONFIG_KERNEL_XZ is redefined by fragment ./kernel/configs/tiny.config:
Previous value: # CONFIG_KERNEL_XZ is not set
New value: CONFIG_KERNEL_XZ=y

Value of CONFIG_SLUB is redefined by fragment ./kernel/configs/tiny.config:
Previous value: CONFIG_SLUB=y
New value: # CONFIG_SLUB is not set

Value of CONFIG_SLOB is redefined by fragment ./kernel/configs/tiny.config:
Previous value: # CONFIG_SLOB is not set
New value: CONFIG_SLOB=y

#
# merged configuration written to .config (needs make)
#
s390x-linux-gnu-ld: arch/s390/kernel/alternative.o(.init.text+0xd8): misaligned symbol `__alt_instructions_end' (0x2bb629) for relocation R_390_PC32DBL
...

I am having a bit of a hard time reducing this because it appears to be related to CONFIG_RELOCATABLE=n, which means I cannot use just one translation unit for a reduction, as there will be undefined symbols. Additionally, if I use defconfig + CONFIG_RELOCATABLE=n, I see a similar but different error:

# Hit '/' in menuconfig, search for RELOCATABLE, then hit '1', 'n', ESC until you are offered to save the config, and Enter for "Yes"
$ make -skj$(nproc) ARCH=s390 CC=clang CROSS_COMPILE=s390x-linux-gnu- defconfig menuconfig vmlinux
s390x-linux-gnu-ld: kernel/panic.o(.text+0x998): misaligned symbol `__end_once' (0x1558e71) for relocation R_390_PC32DBL
...

Another observation is that these symbols are linker defined symbols:

$ rg -Hn --no-heading __end_once
kernel/panic.c:676:     memset(__start_once, 0, __end_once - __start_once);
include/asm-generic/vmlinux.lds.h:351:  __end_once = .;                                                 \
include/asm-generic/sections.h:48:extern char __start_once[], __end_once[];

$ rg -Hn --no-heading __alt_instructions_end arch/s390
arch/s390/kernel/alternative.c:54:extern struct alt_instr __alt_instructions[], __alt_instructions_end[];
arch/s390/kernel/alternative.c:57:      apply_alternatives(__alt_instructions, __alt_instructions_end);
arch/s390/kernel/vmlinux.lds.S:118:             __alt_instructions_end = .;

The output of objdump -Dr in the two functions that contain the problematic symbols:

0000000000000970 <clear_warn_once_set>:
 970:   c0 04 00 00 00 00       jgnop   970 <clear_warn_once_set>
 976:   eb ef f0 88 00 24       stmg    %r14,%r15,136(%r15)
 97c:   b9 04 00 1f             lgr     %r1,%r15
 980:   a7 fb ff e8             aghi    %r15,-24
 984:   e3 10 f0 98 00 24       stg     %r1,152(%r15)
 98a:   c0 e5 00 00 00 00       brasl   %r14,98a <clear_warn_once_set+0x1a>
                        98c: R_390_PLT32DBL     generic_bug_clear_once+0x2
 990:   c0 10 00 00 00 00       larl    %r1,990 <clear_warn_once_set+0x20>
                        992: R_390_PC32DBL      __start_once+0x2
 996:   c0 00 00 00 00 00       larl    %r0,996 <clear_warn_once_set+0x26>
                        998: R_390_PC32DBL      __end_once-0xffe
 99c:   b9 e9 10 20             sgrk    %r2,%r0,%r1
 9a0:   41 20 2f ff             la      %r2,4095(%r2)
 9a4:   ec 28 00 13 ff 7c       cgije   %r2,-1,9ca <clear_warn_once_set+0x5a>
 9aa:   eb 02 00 08 00 0c       srlg    %r0,%r2,8
 9b0:   ec 08 00 0a 00 7c       cgije   %r0,0,9c4 <clear_warn_once_set+0x54>
 9b6:   d7 ff 10 00 10 00       xc      0(256,%r1),0(%r1)
 9bc:   41 10 11 00             la      %r1,256(%r1)
 9c0:   a7 07 ff fb             brctg   %r0,9b6 <clear_warn_once_set+0x46>
 9c4:   c6 20 00 00 00 09       exrl    %r2,9d6 <clear_warn_once_set+0x66>
 9ca:   a7 29 00 00             lghi    %r2,0
 9ce:   eb ef f0 a0 00 04       lmg     %r14,%r15,160(%r15)
 9d4:   07 fe                   br      %r14
 9d6:   d7 00 10 00 10 00       xc      0(1,%r1),0(%r1)

00000000000000b0 <apply_alternative_instructions>:
  b0:   eb bf f0 70 00 24       stmg    %r11,%r15,112(%r15)
  b6:   b9 04 00 1f             lgr     %r1,%r15
  ba:   a7 fb ff d0             aghi    %r15,-48
  be:   e3 10 f0 98 00 24       stg     %r1,152(%r15)
  c4:   c0 10 00 00 00 00       larl    %r1,c4 <apply_alternative_instructions+0x14>
                        c6: R_390_PC32DBL       .init.data+0x2
  ca:   e3 00 10 00 00 94       llc     %r0,0(%r1)
  d0:   ec 06 00 3d 00 7e       cijne   %r0,0,14a <apply_alternative_instructions+0x9a>
  d6:   c0 d0 00 00 00 00       larl    %r13,d6 <apply_alternative_instructions+0x26>
                        d8: R_390_PC32DBL       __alt_instructions_end+0x2
  dc:   c0 c0 00 00 00 00       larl    %r12,dc <apply_alternative_instructions+0x2c>
                        de: R_390_PC32DBL       __alt_instructions+0x2
  e2:   ec cd 00 34 a0 65       clgrjnl %r12,%r13,14a <apply_alternative_instructions+0x9a>
  e8:   c0 b0 00 00 00 00       larl    %r11,e8 <apply_alternative_instructions+0x38>
                        ea: R_390_PC32DBL       alt_stfle_fac_list+0x2
  ee:   e3 10 c0 08 00 91       llgh    %r1,8(%r12)
  f4:   a7 1f 03 ff             cghi    %r1,1023
  f8:   a7 24 00 24             jh      140 <apply_alternative_instructions+0x90>
  fc:   eb 21 00 03 00 0c       srlg    %r2,%r1,3
 102:   e3 0b 20 00 00 76       lb      %r0,0(%r11,%r2)
 108:   a5 17 00 07             nill    %r1,7
 10c:   89 00 10 00             sll     %r0,0(%r1)
 110:   a7 01 00 80             tmll    %r0,128
 114:   a7 84 00 16             je      140 <apply_alternative_instructions+0x90>
 118:   e3 40 c0 0a 00 90       llgc    %r4,10(%r12)
 11e:   a7 41 00 01             tmll    %r4,1
 122:   a7 74 00 0f             jne     140 <apply_alternative_instructions+0x90>
 126:   b9 04 00 2c             lgr     %r2,%r12
 12a:   41 30 c0 04             la      %r3,4(%r12)
 12e:   e3 20 c0 00 00 18       agf     %r2,0(%r12)
 134:   e3 30 c0 04 00 18       agf     %r3,4(%r12)
 13a:   c0 e5 00 00 00 00       brasl   %r14,13a <apply_alternative_instructions+0x8a>
                        13c: R_390_PLT32DBL     s390_kernel_write+0x2
 140:   41 c0 c0 0b             la      %r12,11(%r12)
 144:   ec cd ff d5 40 65       clgrjl  %r12,%r13,ee <apply_alternative_instructions+0x3e>
 14a:   eb bf f0 a0 00 04       lmg     %r11,%r15,160(%r15)
 150:   07 fe                   br      %r14

I do not see any issues with GCC 12.2.1 from Fedora so this seems like something clang or LLVM is doing.

cc @uweigand

[uweigand]

I've been looking into this for a bit, and it unfortunately seems to be a complicated issue. First of all, accessing a symbol that is not 2-byte aligned with LARL is actually a bug - so that new linker error is indeed catching a pre-existing bug. However, why this bug happens is more interesting.

Our ABI specifies that external symbols need to be at least 2-byte aligned, so that the compiler can always simply use LARL to access them. Both GCC and LLVM are supposed to implement this ABI property, both on the symbol definition side (every global symbol is defined with at least 2-byte alignment), and on the symbol use side (every external symbol is assumed to have at least 2-byte alignment, and therefore can be accessed with LARL).

GCC (but currently not LLVM) in addition supports a special bypass allowing to handle symbols that do not comply with the ABI as an extension: if a global definition or declaration is marked with __attribute__((__packed__)), then GCC will neither enforce the 2-byte alignment when defining the symbol, nor will it assume 2-byte alignment when using the symbol (i.e. it will load the symbol address from memory -either a constant pool or the GOT, depending on whether -fpic is used- instead of using LARL).

Unfortunately, on further investigation we just noticed that a patch that went into GCC 6 inadvertently changed this behavior: since then, GCC will no longer make the 2-byte assumption on the use side. This means in particular that since then every external symbol of char or char [] type is no longer accessed via LARL.

Now, in the scenario described in this issue, these symbols are emitted by the linker - which doesn't know about this particular ABI requirement and does not enforce it, so in some cases, those symbols do in fact end up pointing to odd addresses. The reason why this happens to work in GCC is a combination of two effects:

• in the __alt_instructions_end case, the symbol uses __attribute__((__packed__)) and therefore the GCC special bypass applies
• in the __end_once case, the symbol has a char [] type, and therefore the inadvertent GCC 6 change triggers

Now, the question is how to proceed here. One option would be to change the ABI to no longer enforce the 2-byte alignment requirement. I don't really like that, since it really is more efficient to access symbols via LARL, and the cost of enforcing a 2-byte alignment when defining symbols is usually negligable. So I'd prefer to fix GCC to again behave as it did before GCC 6.

However, if we do that, this problem will appear also when compiling the kernel with GCC. So there'd be some change required on the kernel side as well: either by adding appropriate ALIGN statements in the linker script to ensure all symbols defined there do indeed fulfil our ABI requirement; or else use the __attribute__((__packed__)) bypass when accessing all of these symbols from C source, not just some of them.

In addition, for compatibility between the compilers, we'd also need to support the "special bypass" in LLVM just as we do in GCC.

Adding a few more folks
@hcahca - any thoughts from the kernel side?
@JonPsson - can you look into implementing the __attribute__((__packed__)) bypass in LLVM?
@Andreas-Krebbel - and further comments regarding GCC?

[hcahca]

The odd address of the __alt_instructions_end label did already cause a bug in the kernel, since that caused the next symbol to be at an odd address as well, which caused random crashes. See kernel commit c9305b6 ("s390: fix nospec table alignments").
However I didn't realize that there are more problems with this label, unfortunately.
That said, for the s390 specific part of the kernel linker script I don't see a problem enforcing proper alignment of all symbols, however for the common code part this is a different story; this looks like this will be a never-ending story which would be s390 specific.

So right now I think we should simply enforce CONFIG_RELOCATABLE (aka -fPIE) for s390, which should also make sure this problem doesn't exist. At least for the kernel I think this would make sense - I don't see a real use case for non-relocatable kernels anymore.

[nathanchance]

In line with @hcahca 's suggestion, I tested the following diff, which makes CONFIG_RELOCATABLE no longer a choice, and saw no build failures:

diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 318fce77601d..89d328c3bd95 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -568,8 +568,7 @@ config EXPOLINE_FULL
 endchoice

 config RELOCATABLE
-       bool "Build a relocatable kernel"
-       default y
+       def_bool y
        help
          This builds a kernel image that retains relocation information
          so it can be loaded at an arbitrary address.

[nathanchance]

https://lore.kernel.org/20221030182202.2062705-1-hca@linux.ibm.com/

[nathanchance]

https://git.kernel.org/s390/c/80ddf5ce1c9291cb175d52ed1227134ad48c47ee

[nathanchance]

Now in mainline: https://git.kernel.org/linus/80ddf5ce1c9291cb175d52ed1227134ad48c47ee

I won't close this due to Ulrich's comment above but this is basically cleared up for the kernel now.

[nathanchance]

This appears to be back according to CKI:

https://datawarehouse.cki-project.org/kcidb/builds/364003

$ git show -s --format='%h ("%s")'
007034977130 ("Merge tag 'for_linus' of git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost")

$ clang --version | head -1
clang version 16.0.5 (Fedora 16.0.5-1.fc39)

$ s390x-linux-gnu-ld --version | head -1
GNU ld version 2.40-1.fc39

$ curl -LSso .config https://s3.amazonaws.com/arr-cki-prod-trusted-artifacts/trusted-artifacts/906109276/build%20s390x/4508560169/artifacts/kernel-mainline.kernel.org-clang-redhat_906109276-s390x-kernel.config

$ make -skj"$(nproc)" ARCH=s390 CC=clang CROSS_COMPILE=s390x-linux-gnu- olddefconfig all
s390x-linux-gnu-ld: arch/s390/boot/startup.o(.text+0x5b4): misaligned symbol `_decompressor_end' (0x35b0f) for relocation R_390_PC32DBL
make[3]: *** [.../arch/s390/boot/Makefile:78: arch/s390/boot/vmlinux] Error 1
...

It is possible that there is something in the Fedora configuration that tickles this, as it obviously has CONFIG_RELOCATABLE=y.

[nathanchance]

Turns out this is visible with just defconfig. I have not tried to reproduce outside of Fedora yet, it is possible this is a distribution specific issue?

[uweigand]

Looks like the boot decompressor uses a separate set of CFLAGS (KBUILD_CFLAGS_DECOMPRESSOR) that does not include -fPIE even with CONFIG_RELOCATABLE.

It might be possible to just add -fPIE to that set of flags as well, which should fix the problem - however, maybe that isn't feasible as during those early boot stages there is no support for the GOT yet?

In that case, I guess we can just add the appropriate ALIGN statements to the arch/s390/boot/vmlinux.lds.S file - the symbols used by the decompressor seem to be platform-specific, so Heiko's concern about common code may not apply here.

@hcahca any comments or suggestions here?

[huth]

FWIW, we're facing a similar issue with the s390-ccw-bios of QEMU when compiling with binutils 2.40 and Clang : https://bugzilla.redhat.com/show_bug.cgi?id=2216662

[hcahca]

Using -fPIE also for the decompressor CFLAGS works. I just sent a patch for review: https://lore.kernel.org/all/20230622125508.1068457-1-hca@linux.ibm.com/

[Andreas-Krebbel]

Unfortunately, on further investigation we just noticed that a patch that went into GCC 6 inadvertently changed this behavior: since then, GCC will no longer make the 2-byte assumption on the use side. This means in particular that since then every external symbol of char or char [] type is no longer accessed via LARL.

Right, the change we have committed back then relies on the backend requested ABI alignment to be applied to ALL symbols by the middle-end. However, this does not appear to be the case for external symbols. I'm currently testing a patch which assumes all symbols without explicit alignment to be aligned according to the ABI. That's the behavior we had before that change.

[Andreas-Krebbel]

Right, the change we have committed back then relies on the backend requested ABI alignment to be applied to ALL symbols by the middle-end. However, this does not appear to be the case for external symbols. I'm currently testing a patch which assumes all symbols without explicit alignment to be aligned according to the ABI. That's the behavior we had before that change.

Bootstrap and regression test was successful, so I've push the change now:
https://gcc.gnu.org/pipermail/gcc-patches/2023-June/622872.html