[PATCH v2 2/2] s390: Prevent GOT access rewrite for misaligned symbols

[Ilya Leoshkevich]

On Fri, 2025-05-16 at 17:24 +0200, Jens Remus wrote:
> Dereferences of GOT slots with lgrl or lg for global symbols are
> rewritten to larl to get get rid of the extra memory access.  However
> this is invalid for:
> 
> - symbols marked for absolute addressing
> - symbols at odd addresses (larl can handle only even addresses)
> 
> Commit e6213e09ed0e ("S/390: Prevent GOT access rewrite for certain
> symbols") added checks for the above.  But instead of checking the
> address of a symbol for being halfword aligned, it tries to deduce
> this from whether the symbol value and section the symbol is defined
> in are halfword aligned.  The way it is done has two issues:
> 
> 1. The use of bfd_section_from_elf_index to obtain the section the
>    symbol is defined in may not return the one that remains in the
>    output.  For instance for COMDAT sections getting deduplicated
>    the section retrieved using bfd_section_from_elf_index may not be
>    the same as h->root.u.def.section.  If COMDAT sections of same
>    group signature have different alignment properties the wrong
>    one may be checked. This may then lead to an erroneous rewrite
>    of lgrl %rX, sym@GOTENT to larl %rX, sym, although the symbol in
>    the remaining section is not properly aligned, triggering an
>    "relocation for misaligned symbol" error at link-time.
> 
>    This may for instance occur when mixing C++ modules compiled with
>    GCC and Clang, as GCC emits a 2-byte alignment and Clang a 1-byte
>    alignment for COMDAT sections containing type information:
> 
>      $ cat sample.cpp
>      #include <typeinfo>
>      struct A {};
>      const std::type_info &q() { return typeid(A); }
> 
>      $ g++ -c sample.cpp -o sample_gcc.o
>      $ clang++ -c sample.cpp -o sample_clang.o
>      $ readelf -WS sample_gcc.o sample_clang.o
> 
>      Produces (reformatted and reduced):
>      File           Name           Off    Size   ES Flg Lk Inf Al
>      sample_gcc.o   .rodata._ZTS1A 000080 000004 00  AG  0   0  2
>      sample_clang.o .rodata._ZTS1A 000058 000003 00  AG  0   0  1
> 
> 2. The symbol may end up at an even address, if both the symbol value
>    and the section defining the symbol are 1-byte aligned.  While
> this
>    does not trigger an error, it fails an opportunity to rewrite a
> GOT
>    access.
> 
>    In a Linux Kernel build this causes ~15k GOT accesses using lgrl
> to
>    be skipped to be rewritten to larl.
> 
> Resolve both issues by simply checking whether the symbol address is
> halfword aligned.  Do not check the symbol value nor section defining
> the symbol for halfword alignment.
> 
> While at it simplify the test whether the symbol is absolute by using
> the helper bfd_is_abs_symbol.
> 
> bfd/
> 	PR ld/32969
> 	* elf64-s390.c (elf_s390_relocate_section): Only rewrite
> 	lgrl/lg from GOT to larl if symbol address is halfword
> aligned.
> 	Use bfd_is_abs_symbol to test whether symbol is absolute.
> 
> ld/testsuite/
> 	PR ld/32969
> 	* ld-s390/s390.exp (pr32969_64-1, pr32969_64-2): Add tests
> for
> 	rewrite of GOT access when COMDAT section deduplication is
> 	involved.
> 	* ld-s390/pr32969_64-1.dd: New test for rewrite of GOT
> access
> 	when COMDAT section deduplication is involved.
> 	* ld-s390/pr32969_64-2.dd: Likewise.
> 	* ld-s390/pr32969a.s: Likewise.
> 	* ld-s390/pr32969b.s: Likewise.
> 	* ld-s390/pr32969c.s: Likewise.
> 
> Bug: https://sourceware.org/PR32969
> Fixes: e6213e09ed0e ("S/390: Prevent GOT access rewrite for certain
> symbols")
> Reported-by: Ilya Leoshkevich <iii@linux.ibm.com>
> Signed-off-by: Jens Remus <jremus@linux.ibm.com>

Thanks for the fix!

Would it make sense to move the simplification into a separate patch?