Skip to content

ELF: Have __rela_iplt_{start,end} surround .rela.iplt with --pack-dyn-relocs=android. #118585

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.

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

Conversation

pcc
Copy link
Contributor

@pcc pcc commented Dec 4, 2024

In #86751 we moved the IRELATIVE relocations to .rela.plt when
--pack-dyn-relocs=android was enabled but we neglected to also move
the _rela_iplt{start,end} symbols. As a result, static binaries
linked with this flag were unable to find their IRELATIVE relocations.
Fix it by having the symbols surround the correct section.

Created using spr 1.3.6-beta.1
@llvmbot
Copy link
Member

llvmbot commented Dec 4, 2024

@llvm/pr-subscribers-lld-elf

@llvm/pr-subscribers-lld

Author: None (pcc)

Changes

In #86751 we moved the IRELATIVE relocations to .rela.plt when
--pack-dyn-relocs=android was enabled but we neglected to also move
the _rela_iplt{start,end} symbols. As a result, static binaries
linked with this flag were unable to find their IRELATIVE relocations.
Fix it by having the symbols surround the correct section.


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

4 Files Affected:

  • (modified) lld/ELF/Relocations.cpp (+11-9)
  • (modified) lld/ELF/Relocations.h (+3)
  • (modified) lld/ELF/Writer.cpp (+9-5)
  • (added) lld/test/ELF/pack-dyn-relocs-ifunc-static.s (+26)
diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp
index 4aa27b0a71bc1d..204599c544bc44 100644
--- a/lld/ELF/Relocations.cpp
+++ b/lld/ELF/Relocations.cpp
@@ -1687,6 +1687,16 @@ template <class ELFT> void elf::scanRelocations(Ctx &ctx) {
     outerFn();
 }
 
+RelocationBaseSection &elf::getIRelativeSection(Ctx &ctx) {
+  // Prior to Android V, there was a bug that caused RELR relocations to be
+  // applied after packed relocations. This meant that resolvers referenced by
+  // IRELATIVE relocations in the packed relocation section would read
+  // unrelocated globals with RELR relocations when
+  // --pack-relative-relocs=android+relr is enabled. Work around this by placing
+  // IRELATIVE in .rela.plt.
+  return ctx.arg.androidPackDynRelocs ? *ctx.in.relaPlt : *ctx.mainPart->relaDyn;
+}
+
 static bool handleNonPreemptibleIfunc(Ctx &ctx, Symbol &sym, uint16_t flags) {
   // Handle a reference to a non-preemptible ifunc. These are special in a
   // few ways:
@@ -1736,17 +1746,9 @@ static bool handleNonPreemptibleIfunc(Ctx &ctx, Symbol &sym, uint16_t flags) {
   // original section/value pairs. For non-GOT non-PLT relocation case below, we
   // may alter section/value, so create a copy of the symbol to make
   // section/value fixed.
-  //
-  // Prior to Android V, there was a bug that caused RELR relocations to be
-  // applied after packed relocations. This meant that resolvers referenced by
-  // IRELATIVE relocations in the packed relocation section would read
-  // unrelocated globals with RELR relocations when
-  // --pack-relative-relocs=android+relr is enabled. Work around this by placing
-  // IRELATIVE in .rela.plt.
   auto *directSym = makeDefined(cast<Defined>(sym));
   directSym->allocateAux(ctx);
-  auto &dyn =
-      ctx.arg.androidPackDynRelocs ? *ctx.in.relaPlt : *ctx.mainPart->relaDyn;
+  auto &dyn = getIRelativeSection(ctx);
   addPltEntry(ctx, *ctx.in.iplt, *ctx.in.igotPlt, dyn, ctx.target->iRelativeRel,
               *directSym);
   sym.allocateAux(ctx);
diff --git a/lld/ELF/Relocations.h b/lld/ELF/Relocations.h
index 71cea0220e04c3..7ca203257ea876 100644
--- a/lld/ELF/Relocations.h
+++ b/lld/ELF/Relocations.h
@@ -22,6 +22,7 @@ class Symbol;
 class InputSection;
 class InputSectionBase;
 class OutputSection;
+class RelocationBaseSection;
 class SectionBase;
 
 // Represents a relocation type, such as R_X86_64_PC32 or R_ARM_THM_CALL.
@@ -356,6 +357,8 @@ sortRels(Relocs<llvm::object::Elf_Crel_Impl<is64>> rels,
   return {};
 }
 
+RelocationBaseSection &getIRelativeSection(Ctx &ctx);
+
 // Returns true if Expr refers a GOT entry. Note that this function returns
 // false for TLS variables even though they need GOT, because TLS variables uses
 // GOT differently than the regular variables.
diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index f10cc54c05a0ca..aa7c36bc9ac13b 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -844,11 +844,15 @@ template <class ELFT> void Writer<ELFT>::setReservedSymbolSections() {
     ctx.sym.globalOffsetTable->section = sec;
   }
 
-  // .rela_iplt_{start,end} mark the start and the end of .rel[a].dyn.
-  if (ctx.sym.relaIpltStart && ctx.mainPart->relaDyn->isNeeded()) {
-    ctx.sym.relaIpltStart->section = ctx.mainPart->relaDyn.get();
-    ctx.sym.relaIpltEnd->section = ctx.mainPart->relaDyn.get();
-    ctx.sym.relaIpltEnd->value = ctx.mainPart->relaDyn->getSize();
+  // .rela_iplt_{start,end} mark the start and the end of the section containing
+  // IRELATIVE relocations.
+  if (ctx.sym.relaIpltStart) {
+    auto &dyn =  getIRelativeSection(ctx);
+    if (dyn.isNeeded()) {
+      ctx.sym.relaIpltStart->section = &dyn;
+      ctx.sym.relaIpltEnd->section = &dyn;
+      ctx.sym.relaIpltEnd->value = dyn.getSize();
+    }
   }
 
   PhdrEntry *last = nullptr;
diff --git a/lld/test/ELF/pack-dyn-relocs-ifunc-static.s b/lld/test/ELF/pack-dyn-relocs-ifunc-static.s
new file mode 100644
index 00000000000000..0f86a6db785b61
--- /dev/null
+++ b/lld/test/ELF/pack-dyn-relocs-ifunc-static.s
@@ -0,0 +1,26 @@
+# REQUIRES: aarch64
+## __rela_iplt_start and __rela_iplt_end must surround the IRELATIVE relocation
+## list even if moved to .rel[a].plt because of packed relocation sections.
+
+# RUN: llvm-mc -filetype=obj -triple=aarch64-linux-android %s -o %t.o
+# RUN: ld.lld --pack-dyn-relocs=android %t.o -o %t
+# RUN: llvm-readelf -sS %t | FileCheck %s
+
+# CHECK: .rela.plt         RELA            0000000000200158 000158 000018 18  AI  0   5  8
+# CHECK: 0000000000200158     0 NOTYPE  LOCAL  HIDDEN      1 __rela_iplt_start
+# CHECK: 0000000000200170     0 NOTYPE  LOCAL  HIDDEN      1 __rela_iplt_end
+
+.text
+.type foo, %gnu_indirect_function
+.globl foo
+foo:
+  ret
+
+.globl _start
+_start:
+  bl foo
+
+.data
+.balign 8
+.quad __rela_iplt_start
+.quad __rela_iplt_end

Copy link

github-actions bot commented Dec 4, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1
@pcc pcc merged commit 970d6d2 into main Dec 5, 2024
6 of 7 checks passed
@pcc pcc deleted the users/pcc/spr/elf-have-__rela_iplt_startend-surround-relaiplt-with-pack-dyn-relocsandroid branch December 5, 2024 01:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants