[ELF] --pack-dyn-relocs=android+relr: place IRELATIVE in .rela.plt

[MaskRay]

Current Bionic processes relocations in this order:

• DT_ANDROID_REL[A]
• DT_RELR
• DT_REL[A]
• DT_JMPREL

If an IRELATIVE relocation is in DT_ANDROID_REL[A], it would read
unrelocated (incorrect) global variables associated with RELR when
--pack-dyn-relocs=android+relr is enabled. Work around this by placing
IRELATIVE in .rel[a].plt (DT_JMPREL).

Link: https://r.android.com/3014185

[llvmbot]

@llvm/pr-subscribers-lld-elf

@llvm/pr-subscribers-lld

Author: Fangrui Song (MaskRay)

Changes

---

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

2 Files Affected:

• (modified) lld/ELF/Relocations.cpp (+8-2)
• (added) lld/test/ELF/pack-dyn-relocs-ifunc.s (+43)

diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp
index 33c50133bec495..2ffb40c5fa4398 100644
--- a/lld/ELF/Relocations.cpp
+++ b/lld/ELF/Relocations.cpp
@@ -1659,10 +1659,16 @@ static bool handleNonPreemptibleIfunc(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
+  // IRELATIVE relocations in the packed relocation section could not read
+  // globals with RELR relocations. Work around this by placing IRELATIVE in
+  // .rela.plt.
   auto *directSym = makeDefined(cast<Defined>(sym));
   directSym->allocateAux();
-  addPltEntry(*in.iplt, *in.igotPlt, *mainPart->relaDyn, target->iRelativeRel,
-              *directSym);
+  auto &dyn = config->androidPackDynRelocs ? *in.relaPlt : *mainPart->relaDyn;
+  addPltEntry(*in.iplt, *in.igotPlt, dyn, target->iRelativeRel, *directSym);
   sym.allocateAux();
   symAux.back().pltIdx = symAux[directSym->auxIdx].pltIdx;
 
diff --git a/lld/test/ELF/pack-dyn-relocs-ifunc.s b/lld/test/ELF/pack-dyn-relocs-ifunc.s
new file mode 100644
index 00000000000000..ada771aa08ace3
--- /dev/null
+++ b/lld/test/ELF/pack-dyn-relocs-ifunc.s
@@ -0,0 +1,43 @@
+# REQUIRES: aarch64
+## Prior to Android V, there was a bug that caused RELR relocations to be
+## applied after packed relocations. This meant that resolvers referenced
+## IRELATIVE relocations in the packed relocation section could not read
+## globals with RELR relocations. Work around this by placing IRELATIVE in
+## .rela.plt
+
+# RUN: rm -rf %t && split-file %s %t && cd %t
+# RUN: llvm-mc -filetype=obj -triple=aarch64-linux-android a.s -o a.o
+# RUN: llvm-mc -filetype=obj -triple=aarch64-linux-android b.s -o b.o
+# RUN: ld.lld -shared b.o -o b.so
+# RUN: ld.lld --pack-dyn-relocs=android -z separate-loadable-segments a.o b.so -o a
+# RUN: llvm-readobj -r a | FileCheck %s
+# RUN: llvm-objdump -d a | FileCheck %s --check-prefix=ASM
+
+# CHECK:      .rela.plt {
+# CHECK-NEXT:   0x230020 R_AARCH64_JUMP_SLOT bar 0x0
+# CHECK-NEXT:   0x230028 R_AARCH64_IRELATIVE - {{.*}}
+# CHECK-NEXT: }
+
+# ASM:      <.iplt>:
+# ASM-NEXT: adrp    x16, 0x230000
+# ASM-NEXT: ldr     x17, [x16, #0x28]
+
+#--- a.s
+.text
+.type foo, %gnu_indirect_function
+.globl foo
+foo:
+  ret
+
+.globl _start
+_start:
+  bl foo
+  bl bar
+
+.data
+.quad .data
+
+#--- b.s
+.globl bar
+bar:
+  ret

[pcc]

Suggest more accurate wording: "would have been able to read globals with RELR relocations before they were relocated" (likewise below).

[MaskRay]

Oh, I am confused by the sentence "... would have been able to read globals with RELR relocations before they were relocated".

Isn't it "could not ... " because RELR relocations have been applied yet?

[pcc]

They can still read it because the memory is mapped as readable, it's just that the contents will be wrong.

[MaskRay]

Switched to

This meant that resolvers referenced
// IRELATIVE relocations in the packed relocation section would read
// unrelocated globals with RELR relocations when
// --pack-relative-relocs=android+relr is enabled.

hope it is clearer

[pcc]

"referenced" -> "referenced by"

[MaskRay]

Fixed

[pcc]

LGTM. I verified that this patch fixes the test failures that I was seeing.

[MaskRay]

LGTM. I verified that this patch fixes the test failures that I was seeing.

Thanks for testing. I land this tomorrow if I haven't heard of objections.