Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

release/18.x: [lld/ELF] Avoid unnecessary TPOFF relocations in GOT for -pie (#81739) #81990

Merged
merged 1 commit into from
Feb 16, 2024
Merged

release/18.x: [lld/ELF] Avoid unnecessary TPOFF relocations in GOT for -pie (#81739) #81990

merged 1 commit into from
Feb 16, 2024

Conversation

llvmbot
Copy link
Member

@llvmbot llvmbot commented Feb 16, 2024

Backport 6f90773

Requested by: @uweigand

@llvmbot llvmbot added this to the LLVM 18.X Release milestone Feb 16, 2024
@llvmbot
Copy link
Member Author

llvmbot commented Feb 16, 2024

@MaskRay What do you think about merging this PR to the release branch?

@llvmbot llvmbot requested a review from MaskRay February 16, 2024 13:27
@llvmbot llvmbot mentioned this pull request Feb 16, 2024
Merged
@llvmbot
Copy link
Member Author

llvmbot commented Feb 16, 2024

@llvm/pr-subscribers-lld

Author: None (llvmbot)

Changes

Backport 6f90773

Requested by: @uweigand

Full diff: https://github.com/llvm/llvm-project/pull/81990.diff

2 Files Affected:

  • (modified) lld/ELF/Relocations.cpp (+1-1)
  • (modified) lld/test/ELF/systemz-tls-ie.s (+34)
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

@llvmbot
Copy link
Member Author

llvmbot commented Feb 16, 2024

@llvm/pr-subscribers-lld-elf

Author: None (llvmbot)

Changes

Backport 6f90773

Requested by: @uweigand

Full diff: https://github.com/llvm/llvm-project/pull/81990.diff

2 Files Affected:

  • (modified) lld/ELF/Relocations.cpp (+1-1)
  • (modified) lld/test/ELF/systemz-tls-ie.s (+34)
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

@uweigand @tstellar
[lld/ELF] Avoid unnecessary TPOFF relocations in GOT for -pie (llvm#8…
347977c 
…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)
@tstellar tstellar merged commit 347977c into llvm:release/18.x Feb 16, 2024
5 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MaskRay MaskRay MaskRay approved these changes

Assignees
No one assigned
Labels
lld:ELF lld
Projects
LLVM Release Status
Status: Done
Milestone
LLVM 18.X Release
Development

Successfully merging this pull request may close these issues.
4 participants
@llvmbot @MaskRay @tstellar @uweigand