[PATCH v2 2/2] s390: Prevent GOT access rewrite for misaligned symbols
Jens Remus
jremus@linux.ibm.com
Fri May 16 15:24:12 GMT 2025
More information about the Binutils mailing list
Fri May 16 15:24:12 GMT 2025
- Previous message (by thread): [PATCH v2 1/2] s390: Improve diagnostic for reloc against misaligned sym
- Next message (by thread): [PATCH v2 2/2] s390: Prevent GOT access rewrite for misaligned symbols
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
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>
---
Notes (jremus):
Changes in v2:
- Simplify test whether symbol is absolute, by using bfd_is_abs_symbol.
Instructions from Ilya to reproduce BCC build to trigger relocation
for misaligned symbol error:
$ docker run -it --rm debian:trixie
# apt-get update && apt-get install -y cmake g++ git libclang-19-dev libelf-dev libpolly-19-dev llvm-19-dev python3-setuptools zip
# git clone --branch=v0.34.0 --depth=1 https://github.com/iovisor/bcc.git
# cd bcc
# cmake .
# make -j"$(nproc)"
/usr/bin/ld: /usr/lib/gcc/s390x-linux-gnu/14/libstdc++.a(shared_ptr.o)(.text._ZNSt19_Sp_make_shared_tag5_S_eqERKSt9type_info+0x2): misaligned symbol `_ZTSSt19_Sp_make_shared_tag' (0x8574f77) for relocation R_390_PC32DBL
bfd/elf64-s390.c | 14 ++++----------
ld/testsuite/ld-s390/pr32969_64-1.dd | 15 +++++++++++++++
ld/testsuite/ld-s390/pr32969_64-2.dd | 15 +++++++++++++++
ld/testsuite/ld-s390/pr32969a.s | 14 ++++++++++++++
ld/testsuite/ld-s390/pr32969b.s | 10 ++++++++++
ld/testsuite/ld-s390/pr32969c.s | 10 ++++++++++
ld/testsuite/ld-s390/s390.exp | 8 ++++++++
7 files changed, 76 insertions(+), 10 deletions(-)
create mode 100644 ld/testsuite/ld-s390/pr32969_64-1.dd
create mode 100644 ld/testsuite/ld-s390/pr32969_64-2.dd
create mode 100644 ld/testsuite/ld-s390/pr32969a.s
create mode 100644 ld/testsuite/ld-s390/pr32969b.s
create mode 100644 ld/testsuite/ld-s390/pr32969c.s
diff --git a/bfd/elf64-s390.c b/bfd/elf64-s390.c
index 7ae11dc8e8b7..e758d087a61b 100644
--- a/bfd/elf64-s390.c
+++ b/bfd/elf64-s390.c
@@ -2276,9 +2276,6 @@ elf_s390_relocate_section (bfd *output_bfd,
|| SYMBOL_REFERENCES_LOCAL (info, h)
|| resolved_to_zero)
{
- Elf_Internal_Sym *isym;
- asection *sym_sec;
-
/* This is actually a static link, or it is a
-Bsymbolic link and the symbol is defined
locally, or the symbol was forced to be local
@@ -2318,16 +2315,13 @@ elf_s390_relocate_section (bfd *output_bfd,
& 0xff00f000) == 0xe300c000
&& bfd_get_8 (input_bfd,
contents + rel->r_offset + 3) == 0x04))
- && (isym = bfd_sym_from_r_symndx (&htab->elf.sym_cache,
- input_bfd, r_symndx))
- && isym->st_shndx != SHN_ABS
+ && !bfd_is_abs_symbol (&h->root)
&& h != htab->elf.hdynamic
&& h != htab->elf.hgot
&& h != htab->elf.hplt
- && !(isym->st_value & 1)
- && (sym_sec = bfd_section_from_elf_index (input_bfd,
- isym->st_shndx))
- && sym_sec->alignment_power)
+ && !((h->root.u.def.value
+ + sec->output_section->vma
+ + sec->output_offset) & 1))
{
unsigned short new_insn =
(0xc000 | (bfd_get_8 (input_bfd,
diff --git a/ld/testsuite/ld-s390/pr32969_64-1.dd b/ld/testsuite/ld-s390/pr32969_64-1.dd
new file mode 100644
index 000000000000..fce262f5f39d
--- /dev/null
+++ b/ld/testsuite/ld-s390/pr32969_64-1.dd
@@ -0,0 +1,15 @@
+tmpdir/pr32969_64-1: file format elf64-s390
+
+Disassembly of section .text:
+
+.* <_start>:
+.*: c0 10 00 00 00 0e [ ]*larl %r1,10000cc <foo>
+.*: c4 18 00 00 08 1d [ ]*lgrl %r1,10010f0 <_GLOBAL_OFFSET_TABLE_\+0x20>
+
+.* <b>:
+.*: c4 18 00 00 08 1a [ ]*lgrl %r1,10010f0 <_GLOBAL_OFFSET_TABLE_\+0x20>
+#?.* 07 07 [ ]*nopr %r7
+
+.* <c>:
+.* c4 18 00 00 08 16 [ ]*lgrl %r1,10010f0 <_GLOBAL_OFFSET_TABLE_\+0x20>
+#?.* 07 07 [ ]*nopr %r7
diff --git a/ld/testsuite/ld-s390/pr32969_64-2.dd b/ld/testsuite/ld-s390/pr32969_64-2.dd
new file mode 100644
index 000000000000..88c770ec882a
--- /dev/null
+++ b/ld/testsuite/ld-s390/pr32969_64-2.dd
@@ -0,0 +1,15 @@
+tmpdir/pr32969_64-2: file format elf64-s390
+
+Disassembly of section .text:
+
+.* <_start>:
+.*: c0 10 00 00 00 0e [ ]*larl %r1,10000cc <foo>
+.*: c0 10 00 00 00 0c [ ]*larl %r1,10000ce <bar>
+
+.* <c>:
+.*: c0 10 00 00 00 09 [ ]*larl %r1,10000ce <bar>
+#?.* 07 07 [ ]*nopr %r7
+
+.* <b>:
+.* c0 10 00 00 00 05 [ ]*larl %r1,10000ce <bar>
+#?.* 07 07 [ ]*nopr %r7
diff --git a/ld/testsuite/ld-s390/pr32969a.s b/ld/testsuite/ld-s390/pr32969a.s
new file mode 100644
index 000000000000..baee6ca6d247
--- /dev/null
+++ b/ld/testsuite/ld-s390/pr32969a.s
@@ -0,0 +1,14 @@
+ .text
+ .globl _start
+ .type _start,@function
+_start:
+ lgrl %r1,foo@GOTENT
+ lgrl %r1,bar@GOTENT
+
+ .section .rodata,"a",@progbits
+ .align 1
+ .globl foo
+ .type foo,@object
+foo:
+ .byte 0xa
+ .size foo, .-foo
diff --git a/ld/testsuite/ld-s390/pr32969b.s b/ld/testsuite/ld-s390/pr32969b.s
new file mode 100644
index 000000000000..54ee36993734
--- /dev/null
+++ b/ld/testsuite/ld-s390/pr32969b.s
@@ -0,0 +1,10 @@
+b:
+ lgrl %r1,bar@GOTENT
+
+ .section .rodata,"aG",@progbits,bar_group,comdat
+ .align 1
+ .globl bar
+ .type bar,@object
+bar:
+ .byte 0xb
+ .size bar, .-bar
diff --git a/ld/testsuite/ld-s390/pr32969c.s b/ld/testsuite/ld-s390/pr32969c.s
new file mode 100644
index 000000000000..02cf58417d59
--- /dev/null
+++ b/ld/testsuite/ld-s390/pr32969c.s
@@ -0,0 +1,10 @@
+c:
+ lgrl %r1,bar@GOTENT
+
+ .section .rodata,"aG",@progbits,bar_group,comdat
+ .align 2
+ .globl bar
+ .type bar,@object
+bar:
+ .byte 0xc
+ .size bar, .-bar
diff --git a/ld/testsuite/ld-s390/s390.exp b/ld/testsuite/ld-s390/s390.exp
index d91eeacc730a..aaba88d512ab 100644
--- a/ld/testsuite/ld-s390/s390.exp
+++ b/ld/testsuite/ld-s390/s390.exp
@@ -125,6 +125,14 @@ set s390xtests {
"-m elf64_s390 tmpdir/libpltlib_64.so" "" "-m64" {plt_64-1.s}
{{objdump "-dzrj.plt" plt_64-1.pd} {readelf "-wf" plt_64-1_eh.wf}}
"plt_64-1_eh"}
+ {"PR32969-1: do not rewrite load of misaligned COMDAT symbol address"
+ "-m elf64_s390" "" "-m64" {pr32969a.s pr32969b.s pr32969c.s}
+ {{objdump "-dzrj.text" pr32969_64-1.dd}}
+ "pr32969_64-1"}
+ {"PR32969-2: rewrite load of aligned COMDAT symbol address"
+ "-m elf64_s390" "" "-m64" {pr32969a.s pr32969c.s pr32969b.s}
+ {{objdump "-dzrj.text" pr32969_64-2.dd}}
+ "pr32969_64-2"}
}
if [istarget "s390-*-*"] {
--
2.45.2
- Previous message (by thread): [PATCH v2 1/2] s390: Improve diagnostic for reloc against misaligned sym
- Next message (by thread): [PATCH v2 2/2] s390: Prevent GOT access rewrite for misaligned symbols
- Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]
More information about the Binutils mailing list