[lld/ELF] Avoid unnecessary TPOFF relocations in GOT for -pie #81739
Conversation
With the new SystemZ port we noticed that -pie executables generated from files containing R_390_TLS_IEENT relocations will have unnecessary relocations in their GOT: 9e8d8: R_390_TLS_TPOFF *ABS*+0x18 This is caused by the config->isPic conditon in addTpOffsetGotEntry: static void addTpOffsetGotEntry(Symbol &sym) { in.got->addEntry(sym); uint64_t off = sym.getGotOffset(); if (!sym.isPreemptible && !config->isPic) { in.got->addConstant({R_TPREL, target->symbolicRel, off, 0, &sym}); return; } It is correct that we need to retain a TPOFF relocation if the target symbol is preemptible or if we're building a shared library. But when building a -pie executable, those values are fixed at link time and there's no need for any remaining dynamic relocation. Note that the equivalent MIPS-specific code in MipsGotSection::build checks for config->shared instead of config->isPic; we should use the same check here. (Note also that on many other platforms we're not even using addTpOffsetGotEntry in this case as an IE->LE relaxation is applied before; we don't have this type of relaxation on SystemZ.) 
| @llvm/pr-subscribers-lld-elf Author: Ulrich Weigand (uweigand) ChangesWith the new SystemZ port we noticed that -pie executables generated from files containing R_390_TLS_IEENT relocations will have unnecessary relocations in their GOT: This is caused by the config->isPic conditon in addTpOffsetGotEntry: static void addTpOffsetGotEntry(Symbol &sym) { It is correct that we need to retain a TPOFF relocation if the target symbol is preemptible or if we're building a shared library. But when building a -pie executable, those values are fixed at link time and there's no need for any remaining dynamic relocation. Note that the equivalent MIPS-specific code in MipsGotSection::build checks for config->shared instead of config->isPic; we should use the same check here. (Note also that on many other platforms we're not even using addTpOffsetGotEntry in this case as an IE->LE relaxation is applied before; we don't have this type of relaxation on SystemZ.) Full diff: https://github.com/llvm/llvm-project/pull/81739.diff 2 Files Affected:
diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp index f64b4219e0acc1..619fbaf5dc5452 100644 --- a/lld/ELF/Relocations.cpp +++ b/lld/ELF/Relocations.cpp @@ -940,7 +940,7 @@ void elf::addGotEntry(Symbol &sym) { static void addTpOffsetGotEntry(Symbol &sym) { in.got->addEntry(sym); uint64_t off = sym.getGotOffset(); - if (!sym.isPreemptible && !config->isPic) { + if (!sym.isPreemptible && !config->shared) { in.got->addConstant({R_TPREL, target->symbolicRel, off, 0, &sym}); return; } diff --git a/lld/test/ELF/systemz-tls-ie.s b/lld/test/ELF/systemz-tls-ie.s index 27b642ed2dfc5f..85e2f24cb61f62 100644 --- a/lld/test/ELF/systemz-tls-ie.s +++ b/lld/test/ELF/systemz-tls-ie.s @@ -12,6 +12,14 @@ # RUN: llvm-objdump --section .data --full-contents %t | FileCheck --check-prefix=LE-DATA %s # RUN: llvm-objdump --section .got --full-contents %t | FileCheck --check-prefix=LE-GOT %s +## With -pie we still have the R_390_RELATIVE for the data element, but all GOT +## entries should be fully resolved without any remaining R_390_TLS_TPOFF. +# RUN: ld.lld -pie %t.o -o %t.pie +# RUN: llvm-readelf -r %t.pie | FileCheck --check-prefix=PIE-REL %s +# RUN: llvm-objdump -d --no-show-raw-insn %t.pie | FileCheck --check-prefix=PIE %s +# RUN: llvm-objdump --section .data --full-contents %t.pie | FileCheck --check-prefix=PIE-DATA %s +# RUN: llvm-objdump --section .got --full-contents %t.pie | FileCheck --check-prefix=PIE-GOT %s + # IE-REL: Relocation section '.rela.dyn' at offset {{.*}} contains 4 entries: # IE-REL: 0000000000003478 000000000000000c R_390_RELATIVE 2460 # IE-REL: 0000000000002460 0000000100000038 R_390_TLS_TPOFF 0000000000000008 a + 0 @@ -58,6 +66,32 @@ # LE-GOT: 1002248 00000000 00000000 ffffffff fffffff8 # LE-GOT: 1002258 ffffffff fffffffc 00000000 00000000 +# PIE-REL: Relocation section '.rela.dyn' at offset {{.*}} contains 1 entries: +# PIE-REL: 00000000000033d0 000000000000000c R_390_RELATIVE 23b8 + +## TP offset for a is at 0x23b8 +# PIE: lgrl %r1, 0x23b8 +# PIE-NEXT: lgf %r1, 0(%r1,%r7) + +## TP offset for b is at 0x23c0 +# PIE-NEXT: lgrl %r1, 0x23c0 +# PIE-NEXT: lgf %r1, 0(%r1,%r7) + +## TP offset for c is at 0x23c8 +# PIE-NEXT: lgrl %r1, 0x23c8 +# PIE-NEXT: lgf %r1, 0(%r1,%r7) + +## Data element: TP offset for a is at 0x23b8 (relocated via R_390_RELATIVE above) +# PIE-DATA: 33d0 00000000 00000000 + +## TP offsets in GOT: +# a: -8 +# b: -4 +# c: 0 +# PIE-GOT: 23a0 00000000 000022d0 00000000 00000000 +# PIE-GOT: 23b0 00000000 00000000 ffffffff fffffff8 +# PIE-GOT: 23c0 ffffffff fffffffc 00000000 00000000 + ear %r7,%a0 sllg %r7,%r1,32 ear %r7,%a1 |
| @llvm/pr-subscribers-lld Author: Ulrich Weigand (uweigand) ChangesWith the new SystemZ port we noticed that -pie executables generated from files containing R_390_TLS_IEENT relocations will have unnecessary relocations in their GOT: This is caused by the config->isPic conditon in addTpOffsetGotEntry: static void addTpOffsetGotEntry(Symbol &sym) { It is correct that we need to retain a TPOFF relocation if the target symbol is preemptible or if we're building a shared library. But when building a -pie executable, those values are fixed at link time and there's no need for any remaining dynamic relocation. Note that the equivalent MIPS-specific code in MipsGotSection::build checks for config->shared instead of config->isPic; we should use the same check here. (Note also that on many other platforms we're not even using addTpOffsetGotEntry in this case as an IE->LE relaxation is applied before; we don't have this type of relaxation on SystemZ.) Full diff: https://github.com/llvm/llvm-project/pull/81739.diff 2 Files Affected:
diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp index f64b4219e0acc1..619fbaf5dc5452 100644 --- a/lld/ELF/Relocations.cpp +++ b/lld/ELF/Relocations.cpp @@ -940,7 +940,7 @@ void elf::addGotEntry(Symbol &sym) { static void addTpOffsetGotEntry(Symbol &sym) { in.got->addEntry(sym); uint64_t off = sym.getGotOffset(); - if (!sym.isPreemptible && !config->isPic) { + if (!sym.isPreemptible && !config->shared) { in.got->addConstant({R_TPREL, target->symbolicRel, off, 0, &sym}); return; } diff --git a/lld/test/ELF/systemz-tls-ie.s b/lld/test/ELF/systemz-tls-ie.s index 27b642ed2dfc5f..85e2f24cb61f62 100644 --- a/lld/test/ELF/systemz-tls-ie.s +++ b/lld/test/ELF/systemz-tls-ie.s @@ -12,6 +12,14 @@ # RUN: llvm-objdump --section .data --full-contents %t | FileCheck --check-prefix=LE-DATA %s # RUN: llvm-objdump --section .got --full-contents %t | FileCheck --check-prefix=LE-GOT %s +## With -pie we still have the R_390_RELATIVE for the data element, but all GOT +## entries should be fully resolved without any remaining R_390_TLS_TPOFF. +# RUN: ld.lld -pie %t.o -o %t.pie +# RUN: llvm-readelf -r %t.pie | FileCheck --check-prefix=PIE-REL %s +# RUN: llvm-objdump -d --no-show-raw-insn %t.pie | FileCheck --check-prefix=PIE %s +# RUN: llvm-objdump --section .data --full-contents %t.pie | FileCheck --check-prefix=PIE-DATA %s +# RUN: llvm-objdump --section .got --full-contents %t.pie | FileCheck --check-prefix=PIE-GOT %s + # IE-REL: Relocation section '.rela.dyn' at offset {{.*}} contains 4 entries: # IE-REL: 0000000000003478 000000000000000c R_390_RELATIVE 2460 # IE-REL: 0000000000002460 0000000100000038 R_390_TLS_TPOFF 0000000000000008 a + 0 @@ -58,6 +66,32 @@ # LE-GOT: 1002248 00000000 00000000 ffffffff fffffff8 # LE-GOT: 1002258 ffffffff fffffffc 00000000 00000000 +# PIE-REL: Relocation section '.rela.dyn' at offset {{.*}} contains 1 entries: +# PIE-REL: 00000000000033d0 000000000000000c R_390_RELATIVE 23b8 + +## TP offset for a is at 0x23b8 +# PIE: lgrl %r1, 0x23b8 +# PIE-NEXT: lgf %r1, 0(%r1,%r7) + +## TP offset for b is at 0x23c0 +# PIE-NEXT: lgrl %r1, 0x23c0 +# PIE-NEXT: lgf %r1, 0(%r1,%r7) + +## TP offset for c is at 0x23c8 +# PIE-NEXT: lgrl %r1, 0x23c8 +# PIE-NEXT: lgf %r1, 0(%r1,%r7) + +## Data element: TP offset for a is at 0x23b8 (relocated via R_390_RELATIVE above) +# PIE-DATA: 33d0 00000000 00000000 + +## TP offsets in GOT: +# a: -8 +# b: -4 +# c: 0 +# PIE-GOT: 23a0 00000000 000022d0 00000000 00000000 +# PIE-GOT: 23b0 00000000 00000000 ffffffff fffffff8 +# PIE-GOT: 23c0 ffffffff fffffffc 00000000 00000000 + ear %r7,%a0 sllg %r7,%r1,32 ear %r7,%a1 |
| # RUN: ld.lld -pie %t.o -o %t.pie | ||
| # RUN: llvm-readelf -r %t.pie | FileCheck --check-prefix=PIE-REL %s | ||
| # RUN: llvm-objdump -d --no-show-raw-insn %t.pie | FileCheck --check-prefix=PIE %s | ||
| # RUN: llvm-objdump --section .data --full-contents %t.pie | FileCheck --check-prefix=PIE-DATA %s |
There was a problem hiding this comment.
To inspect section content, llvm-readelf -x .got -x .data is shorter:)
| /cherry-pick 6f90773 |
| Failed to cherry-pick: 6f90773 https://github.com/llvm/llvm-project/actions/runs/7907226425 Please manually backport the fix and push it to your github fork. Once this is done, please create a pull request |
| This backport requires the the lld/SystemZ patch to be cherry-picked first. |
| /cherry-pick 6f90773 |
…1739) With the new SystemZ port we noticed that -pie executables generated from files containing R_390_TLS_IEENT relocations will have unnecessary relocations in their GOT: 9e8d8: R_390_TLS_TPOFF *ABS*+0x18 This is caused by the config->isPic conditon in addTpOffsetGotEntry: static void addTpOffsetGotEntry(Symbol &sym) { in.got->addEntry(sym); uint64_t off = sym.getGotOffset(); if (!sym.isPreemptible && !config->isPic) { in.got->addConstant({R_TPREL, target->symbolicRel, off, 0, &sym}); return; } It is correct that we need to retain a TPOFF relocation if the target symbol is preemptible or if we're building a shared library. But when building a -pie executable, those values are fixed at link time and there's no need for any remaining dynamic relocation. Note that the equivalent MIPS-specific code in MipsGotSection::build checks for config->shared instead of config->isPic; we should use the same check here. (Note also that on many other platforms we're not even using addTpOffsetGotEntry in this case as an IE->LE relaxation is applied before; we don't have this type of relaxation on SystemZ.) (cherry picked from commit 6f90773)
| /pull-request #81990 |
…1739) With the new SystemZ port we noticed that -pie executables generated from files containing R_390_TLS_IEENT relocations will have unnecessary relocations in their GOT: 9e8d8: R_390_TLS_TPOFF *ABS*+0x18 This is caused by the config->isPic conditon in addTpOffsetGotEntry: static void addTpOffsetGotEntry(Symbol &sym) { in.got->addEntry(sym); uint64_t off = sym.getGotOffset(); if (!sym.isPreemptible && !config->isPic) { in.got->addConstant({R_TPREL, target->symbolicRel, off, 0, &sym}); return; } It is correct that we need to retain a TPOFF relocation if the target symbol is preemptible or if we're building a shared library. But when building a -pie executable, those values are fixed at link time and there's no need for any remaining dynamic relocation. Note that the equivalent MIPS-specific code in MipsGotSection::build checks for config->shared instead of config->isPic; we should use the same check here. (Note also that on many other platforms we're not even using addTpOffsetGotEntry in this case as an IE->LE relaxation is applied before; we don't have this type of relaxation on SystemZ.) (cherry picked from commit 6f90773)
3 participants
With the new SystemZ port we noticed that -pie executables generated from files containing R_390_TLS_IEENT relocations will have unnecessary relocations in their GOT:
This is caused by the config->isPic conditon in addTpOffsetGotEntry:
static void addTpOffsetGotEntry(Symbol &sym) {
in.got->addEntry(sym);
uint64_t off = sym.getGotOffset();
if (!sym.isPreemptible && !config->isPic) {
in.got->addConstant({R_TPREL, target->symbolicRel, off, 0, &sym});
return;
}
It is correct that we need to retain a TPOFF relocation if the target symbol is preemptible or if we're building a shared library. But when building a -pie executable, those values are fixed at link time and there's no need for any remaining dynamic relocation.
Note that the equivalent MIPS-specific code in MipsGotSection::build checks for config->shared instead of config->isPic; we should use the same check here. (Note also that on many other platforms we're not even using addTpOffsetGotEntry in this case as an IE->LE relaxation is applied before; we don't have this type of relaxation on SystemZ.)