Skip to content

Commit ed53e99

Browse files
author
Jens Remus
committed
s390: Prevent GOT access rewrite for misaligned symbols
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 e6213e0 ("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. 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. 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: e6213e0 ("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>
1 parent d463846 commit ed53e99

File tree

7 files changed

+75
-5
lines changed

7 files changed

+75
-5
lines changed

bfd/elf64-s390.c

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2277,7 +2277,6 @@ elf_s390_relocate_section (bfd *output_bfd,
22772277
|| resolved_to_zero)
22782278
{
22792279
Elf_Internal_Sym *isym;
2280-
asection *sym_sec;
22812280

22822281
/* This is actually a static link, or it is a
22832282
-Bsymbolic link and the symbol is defined
@@ -2324,10 +2323,9 @@ elf_s390_relocate_section (bfd *output_bfd,
23242323
&& h != htab->elf.hdynamic
23252324
&& h != htab->elf.hgot
23262325
&& h != htab->elf.hplt
2327-
&& !(isym->st_value & 1)
2328-
&& (sym_sec = bfd_section_from_elf_index (input_bfd,
2329-
isym->st_shndx))
2330-
&& sym_sec->alignment_power)
2326+
&& !((h->root.u.def.value
2327+
+ sec->output_section->vma
2328+
+ sec->output_offset) & 1))
23312329
{
23322330
unsigned short new_insn =
23332331
(0xc000 | (bfd_get_8 (input_bfd,

ld/testsuite/ld-s390/pr32969_64-1.dd

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
tmpdir/pr32969_64-1: file format elf64-s390
2+
3+
Disassembly of section .text:
4+
5+
.* <_start>:
6+
.*: c0 10 00 00 00 0e [ ]*larl %r1,10000cc <foo>
7+
.*: c4 18 00 00 08 1d [ ]*lgrl %r1,10010f0 <_GLOBAL_OFFSET_TABLE_\+0x20>
8+
9+
.* <b>:
10+
.*: c4 18 00 00 08 1a [ ]*lgrl %r1,10010f0 <_GLOBAL_OFFSET_TABLE_\+0x20>
11+
#?.* 07 07 [ ]*nopr %r7
12+
13+
.* <c>:
14+
.* c4 18 00 00 08 16 [ ]*lgrl %r1,10010f0 <_GLOBAL_OFFSET_TABLE_\+0x20>
15+
#?.* 07 07 [ ]*nopr %r7

ld/testsuite/ld-s390/pr32969_64-2.dd

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
tmpdir/pr32969_64-2: file format elf64-s390
2+
3+
Disassembly of section .text:
4+
5+
.* <_start>:
6+
.*: c0 10 00 00 00 0e [ ]*larl %r1,10000cc <foo>
7+
.*: c0 10 00 00 00 0c [ ]*larl %r1,10000ce <bar>
8+
9+
.* <c>:
10+
.*: c0 10 00 00 00 09 [ ]*larl %r1,10000ce <bar>
11+
#?.* 07 07 [ ]*nopr %r7
12+
13+
.* <b>:
14+
.* c0 10 00 00 00 05 [ ]*larl %r1,10000ce <bar>
15+
#?.* 07 07 [ ]*nopr %r7

ld/testsuite/ld-s390/pr32969a.s

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
.text
2+
.globl _start
3+
.type _start,@function
4+
_start:
5+
lgrl %r1,foo@GOTENT
6+
lgrl %r1,bar@GOTENT
7+
8+
.section .rodata,"a",@progbits
9+
.align 1
10+
.globl foo
11+
.type foo,@object
12+
foo:
13+
.byte 0xa
14+
.size foo, .-foo

ld/testsuite/ld-s390/pr32969b.s

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
b:
2+
lgrl %r1,bar@GOTENT
3+
4+
.section .rodata,"aG",@progbits,bar_group,comdat
5+
.align 1
6+
.globl bar
7+
.type bar,@object
8+
bar:
9+
.byte 0xb
10+
.size bar, .-bar

ld/testsuite/ld-s390/pr32969c.s

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
c:
2+
lgrl %r1,bar@GOTENT
3+
4+
.section .rodata,"aG",@progbits,bar_group,comdat
5+
.align 2
6+
.globl bar
7+
.type bar,@object
8+
bar:
9+
.byte 0xc
10+
.size bar, .-bar

ld/testsuite/ld-s390/s390.exp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,14 @@ set s390xtests {
125125
"-m elf64_s390 tmpdir/libpltlib_64.so" "" "-m64" {plt_64-1.s}
126126
{{objdump "-dzrj.plt" plt_64-1.pd} {readelf "-wf" plt_64-1_eh.wf}}
127127
"plt_64-1_eh"}
128+
{"PR32969-1: do not rewrite load of misaligned COMDAT symbol address"
129+
"-m elf64_s390" "" "-m64" {pr32969a.s pr32969b.s pr32969c.s}
130+
{{objdump "-dzrj.text" pr32969_64-1.dd}}
131+
"pr32969_64-1"}
132+
{"PR32969-2: rewrite load of aligned COMDAT symbol address"
133+
"-m elf64_s390" "" "-m64" {pr32969a.s pr32969c.s pr32969b.s}
134+
{{objdump "-dzrj.text" pr32969_64-2.dd}}
135+
"pr32969_64-2"}
128136
}
129137

130138
if [istarget "s390-*-*"] {

0 commit comments

Comments
 (0)