[ELF][ARM][AARCH64][MIPS][PPC] Simplify the logic to create R_*_RELATIVE for absolute relocation types in writable sections

Summary:
Our rule to create R_*_RELATIVE for absolute relocation types were
loose. D63121 made it stricter but it failed to create R_*_RELATIVE for
R_ARM_TARGET1 and R_PPC64_TOC. rLLD363236 worked around that by
reinstating the original behavior for ARM and PPC64.

This patch is an attempt to simplify the logic.

Note, in ld.bfd, R_ARM_TARGET2 --target2=abs also creates
R_ARM_RELATIVE. This seems a very uncommon scenario (moreover,
--target2=got-rel is the default), so I do not implement any logic
related to it.

Also, delete R_AARCH64_ABS32 from AArch64::getDynRel. We don't have
working ILP32 support yet. Allowing it would create an incorrect
R_AARCH64_RELATIVE.

For MIPS, the (if SymbolRel, then RelativeRel) code is to keep its
behavior unchanged.

Note, in ppc64-abs64-dyn.s, R_PPC64_TOC gets an incorrect addend because
computeAddend() doesn't compute the correct address. We seem to have the
wrong behavior for a long time. The important thing seems that a dynamic
relocation R_PPC64_TOC should not be created as the dynamic loader will
error R_PPC64_TOC is not supported.

Reviewers: atanasyan, grimar, peter.smith, ruiu, sfertile, espindola

Reviewed By: ruiu

Differential Revision: https://reviews.llvm.org/D63383

llvm-svn: 363928

13 files changed, 102 insertions, 92 deletions

diff --git a/lld/ELF/Arch/AArch64.cpp b/lld/ELF/Arch/AArch64.cpp
index cb8dc3897d8..0695ffa9db3 100644
--- a/lld/ELF/Arch/AArch64.cpp
+++ b/lld/ELF/Arch/AArch64.cpp
@@ -146,7 +146,7 @@ bool AArch64::usesOnlyLowPageBits(RelType Type) const {
 }
 
 RelType AArch64::getDynRel(RelType Type) const {
-  if (Type == R_AARCH64_ABS32 || Type == R_AARCH64_ABS64)
+  if (Type == R_AARCH64_ABS64)
     return Type;
   return R_AARCH64_NONE;
 }
diff --git a/lld/ELF/Arch/AMDGPU.cpp b/lld/ELF/Arch/AMDGPU.cpp
index f9378fab04c..4a9c9ab6c88 100644
--- a/lld/ELF/Arch/AMDGPU.cpp
+++ b/lld/ELF/Arch/AMDGPU.cpp
@@ -28,6 +28,7 @@ public:
   void relocateOne(uint8_t *Loc, RelType Type, uint64_t Val) const override;
   RelExpr getRelExpr(RelType Type, const Symbol &S,
                      const uint8_t *Loc) const override;
+  RelType getDynRel(RelType Type) const override;
 };
 } // namespace
 
@@ -100,6 +101,12 @@ RelExpr AMDGPU::getRelExpr(RelType Type, const Symbol &S,
   }
 }
 
+RelType AMDGPU::getDynRel(RelType Type) const {
+  if (Type == R_AMDGPU_ABS64)
+    return Type;
+  return R_AMDGPU_NONE;
+}
+
 TargetInfo *elf::getAMDGPUTargetInfo() {
   static AMDGPU Target;
   return &Target;
diff --git a/lld/ELF/Arch/Mips.cpp b/lld/ELF/Arch/Mips.cpp
index f4c81e5a73d..18fc6968f5d 100644
--- a/lld/ELF/Arch/Mips.cpp
+++ b/lld/ELF/Arch/Mips.cpp
@@ -182,8 +182,8 @@ RelExpr MIPS<ELFT>::getRelExpr(RelType Type, const Symbol &S,
 }
 
 template <class ELFT> RelType MIPS<ELFT>::getDynRel(RelType Type) const {
-  if (Type == R_MIPS_32 || Type == R_MIPS_64)
-    return RelativeRel;
+  if (Type == SymbolicRel)
+    return Type;
   return R_MIPS_NONE;
 }
 
diff --git a/lld/ELF/Arch/PPC.cpp b/lld/ELF/Arch/PPC.cpp
index 45f799c3687..30b6966ffbd 100644
--- a/lld/ELF/Arch/PPC.cpp
+++ b/lld/ELF/Arch/PPC.cpp
@@ -23,6 +23,9 @@ namespace {
 class PPC final : public TargetInfo {
 public:
   PPC();
+  RelExpr getRelExpr(RelType Type, const Symbol &S,
+                     const uint8_t *Loc) const override;
+  RelType getDynRel(RelType Type) const override;
   void writeGotHeader(uint8_t *Buf) const override;
   void writePltHeader(uint8_t *Buf) const override {
     llvm_unreachable("should call writePPC32GlinkSection() instead");
@@ -36,8 +39,6 @@ public:
                   uint64_t BranchAddr, const Symbol &S) const override;
   uint32_t getThunkSectionSpacing() const override;
   bool inBranchRange(RelType Type, uint64_t Src, uint64_t Dst) const override;
-  RelExpr getRelExpr(RelType Type, const Symbol &S,
-                     const uint8_t *Loc) const override;
   void relocateOne(uint8_t *Loc, RelType Type, uint64_t Val) const override;
   RelExpr adjustRelaxExpr(RelType Type, const uint8_t *Data,
                           RelExpr Expr) const override;
@@ -230,6 +231,12 @@ RelExpr PPC::getRelExpr(RelType Type, const Symbol &S,
   }
 }
 
+RelType PPC::getDynRel(RelType Type) const {
+  if (Type == R_PPC_ADDR32)
+    return Type;
+  return R_PPC_NONE;
+}
+
 static std::pair<RelType, uint64_t> fromDTPREL(RelType Type, uint64_t Val) {
   uint64_t DTPBiasedVal = Val - 0x8000;
   switch (Type) {
diff --git a/lld/ELF/Arch/PPC64.cpp b/lld/ELF/Arch/PPC64.cpp
index 499e8de4524..e467b12b2bb 100644
--- a/lld/ELF/Arch/PPC64.cpp
+++ b/lld/ELF/Arch/PPC64.cpp
@@ -193,6 +193,7 @@ public:
   uint32_t calcEFlags() const override;
   RelExpr getRelExpr(RelType Type, const Symbol &S,
                      const uint8_t *Loc) const override;
+  RelType getDynRel(RelType Type) const override;
   void writePltHeader(uint8_t *Buf) const override;
   void writePlt(uint8_t *Buf, uint64_t GotPltEntryAddr, uint64_t PltEntryAddr,
                 int32_t Index, unsigned RelOff) const override;
@@ -610,6 +611,12 @@ RelExpr PPC64::getRelExpr(RelType Type, const Symbol &S,
   }
 }
 
+RelType PPC64::getDynRel(RelType Type) const {
+  if (Type == R_PPC64_ADDR64 || Type == R_PPC64_TOC)
+    return R_PPC64_ADDR64;
+  return R_PPC64_NONE;
+}
+
 void PPC64::writeGotHeader(uint8_t *Buf) const {
   write64(Buf, getPPC64TocBase());
 }
diff --git a/lld/ELF/Relocations.cpp b/lld/ELF/Relocations.cpp
index 00a5975048d..0c33e47c29d 100644
--- a/lld/ELF/Relocations.cpp
+++ b/lld/ELF/Relocations.cpp
@@ -910,19 +910,13 @@ static void processRelocAux(InputSectionBase &Sec, RelExpr Expr, RelType Type,
 
   bool CanWrite = (Sec.Flags & SHF_WRITE) || !Config->ZText;
   if (CanWrite) {
-    // FIXME Improve the way we handle absolute relocation types that will
-    // change to relative relocations. ARM has a relocation type R_ARM_TARGET1
-    // that is similar to SymbolicRel. PPC64 may have similar relocation types.
-    if ((!Sym.IsPreemptible &&
-         (Config->EMachine == EM_ARM || Config->EMachine == EM_PPC64 ||
-          Type == Target->SymbolicRel)) ||
-        Expr == R_GOT) {
-      // If this is a symbolic relocation to a non-preemptable symbol, or an
-      // R_GOT, its address is its link-time value plus load address. Represent
-      // it with a relative relocation.
+    RelType Rel = Target->getDynRel(Type);
+    if (Expr == R_GOT || (Rel == Target->SymbolicRel && !Sym.IsPreemptible)) {
       addRelativeReloc(&Sec, Offset, &Sym, Addend, Expr, Type);
       return;
-    } else if (RelType Rel = Target->getDynRel(Type)) {
+    } else if (Rel != 0) {
+      if (Config->EMachine == EM_MIPS && Rel == Target->SymbolicRel)
+        Rel = Target->RelativeRel;
       Sec.getPartition().RelaDyn->addReloc(Rel, &Sec, Offset, &Sym, Addend,
                                            R_ADDEND, Type);
 
diff --git a/lld/ELF/Target.h b/lld/ELF/Target.h
index 7c75be9da79..88aa074074e 100644
--- a/lld/ELF/Target.h
+++ b/lld/ELF/Target.h
@@ -26,7 +26,9 @@ class Symbol;
 class TargetInfo {
 public:
   virtual uint32_t calcEFlags() const { return 0; }
-  virtual RelType getDynRel(RelType Type) const { return Type; }
+  virtual RelExpr getRelExpr(RelType Type, const Symbol &S,
+                             const uint8_t *Loc) const = 0;
+  virtual RelType getDynRel(RelType Type) const { return 0; }
   virtual void writeGotPltHeader(uint8_t *Buf) const {}
   virtual void writeGotHeader(uint8_t *Buf) const {}
   virtual void writeGotPlt(uint8_t *Buf, const Symbol &S) const {};
@@ -74,8 +76,6 @@ public:
   // Return true if we can reach Dst from Src with Relocation RelocType
   virtual bool inBranchRange(RelType Type, uint64_t Src,
                              uint64_t Dst) const;
-  virtual RelExpr getRelExpr(RelType Type, const Symbol &S,
-                             const uint8_t *Loc) const = 0;
 
   virtual void relocateOne(uint8_t *Loc, RelType Type, uint64_t Val) const = 0;
 
diff --git a/lld/test/ELF/aarch64-abs32-dyn.s b/lld/test/ELF/aarch64-abs32-dyn.s
new file mode 100644
index 00000000000..085fe15cba1
--- /dev/null
+++ b/lld/test/ELF/aarch64-abs32-dyn.s
@@ -0,0 +1,14 @@
+# REQUIRES: aarch64
+# RUN: llvm-mc -filetype=obj -triple=aarch64 %s -o %t.o
+# RUN: not ld.lld -shared %t.o -o /dev/null 2>&1 | FileCheck %s
+
+## Test we don't create R_AARCH64_RELATIVE.
+
+# CHECK: error: relocation R_AARCH64_ABS32 cannot be used against symbol hidden; recompile with -fPIC
+
+.globl hidden
+.hidden hidden
+hidden:
+
+.data
+.long hidden
diff --git a/lld/test/ELF/arm-abs32-dyn.s b/lld/test/ELF/arm-abs32-dyn.s
index 63f4e99a01d..36c0353acfb 100644
--- a/lld/test/ELF/arm-abs32-dyn.s
+++ b/lld/test/ELF/arm-abs32-dyn.s
@@ -14,11 +14,18 @@ bar:
  .word foo
  .word bar
 
+// In PIC mode, if R_ARM_TARGET1 represents R_ARM_ABS32 (the default), an
+// R_ARM_TARGET1 to a non-preemptable symbol also creates an R_ARM_RELATIVE in
+// a writable section.
+ .word bar(target1)
+
 // RUN: ld.lld -shared -o %t.so %t.o
 // RUN: llvm-readobj --symbols --dyn-relocations %t.so | FileCheck %s
+// RUN: llvm-readelf -x .data %t.so | FileCheck --check-prefix=HEX %s
 
 // CHECK:      Dynamic Relocations {
 // CHECK-NEXT:   0x2004 R_ARM_RELATIVE
+// CHECK-NEXT:   0x2008 R_ARM_RELATIVE
 // CHECK-NEXT:   0x2000 R_ARM_ABS32 foo 0x0
 // CHECK-NEXT: }
 
@@ -30,3 +37,5 @@ bar:
 // CHECK:        Symbol {
 // CHECK:          Name: foo
 // CHECK-NEXT:     Value: 0x1000
+
+// HEX: 0x00002000 00000000 00100000 00100000
diff --git a/lld/test/ELF/ppc64-abs32-dyn.s b/lld/test/ELF/ppc64-abs32-dyn.s
new file mode 100644
index 00000000000..8bfa0dd686c
--- /dev/null
+++ b/lld/test/ELF/ppc64-abs32-dyn.s
@@ -0,0 +1,14 @@
+# REQUIRES: ppc
+# RUN: llvm-mc -filetype=obj -triple=powerpc64le %s -o %t.o
+# RUN: not ld.lld -shared %t.o -o /dev/null 2>&1 | FileCheck %s
+
+## Test we don't create R_AARCH64_RELATIVE.
+
+# CHECK: error: relocation R_PPC64_ADDR32 cannot be used against symbol hidden; recompile with -fPIC
+
+.globl hidden
+.hidden hidden
+hidden:
+
+.data
+.long hidden
diff --git a/lld/test/ELF/ppc64-abs64-dyn.s b/lld/test/ELF/ppc64-abs64-dyn.s
new file mode 100644
index 00000000000..f8629c3a156
--- /dev/null
+++ b/lld/test/ELF/ppc64-abs64-dyn.s
@@ -0,0 +1,29 @@
+# REQUIRES: ppc
+# RUN: llvm-mc -filetype=obj -triple=powerpc64le-unknown-linux %s -o %t.o
+# RUN: ld.lld -shared %t.o -o %t.so
+# RUN: llvm-readobj -r %t.so | FileCheck %s
+
+# RUN: llvm-mc -filetype=obj -triple=powerpc64-unknown-linux %s -o %t.o
+# RUN: ld.lld -shared %t.o -o %t.so
+# RUN: llvm-readobj -r %t.so | FileCheck %s
+
+## Test that we create R_PPC64_RELATIVE for R_PPC64_ADDR64 to non-preemptable
+## symbols and R_PPC64_TOC in writable sections.
+
+## FIXME the addend for offset 0x20000 should be TOC base+0x8000+1, not 0x80001.
+# CHECK:      .rela.dyn {
+# CHECK-NEXT:   0x20000 R_PPC64_RELATIVE - 0x8001
+# CHECK-NEXT:   0x20008 R_PPC64_RELATIVE - 0x20001
+# CHECK-NEXT:   0x20010 R_PPC64_ADDR64 external 0x1
+# CHECK-NEXT:   0x20018 R_PPC64_ADDR64 global 0x1
+# CHECK-NEXT: }
+
+.data
+.globl global
+global:
+local:
+
+.quad .TOC.@tocbase + 1
+.quad local + 1
+.quad external + 1
+.quad global + 1
diff --git a/lld/test/ELF/relative-dynamic-reloc-ppc64.s b/lld/test/ELF/relative-dynamic-reloc-ppc64.s
deleted file mode 100644
index b38882a9f2c..00000000000
--- a/lld/test/ELF/relative-dynamic-reloc-ppc64.s
+++ /dev/null
@@ -1,71 +0,0 @@
-// REQUIRES: ppc
-// RUN: llvm-mc -filetype=obj -triple=powerpc64le-unknown-linux %s -o %t.o
-// RUN: ld.lld -shared %t.o -o %t.so
-// RUN: llvm-readobj --symbols -r --dyn-syms %t.so | FileCheck %s
-
-// RUN: llvm-mc -filetype=obj -triple=powerpc64-unknown-linux %s -o %t.o
-// RUN: ld.lld -shared %t.o -o %t.so
-// RUN: llvm-readobj --symbols -r --dyn-syms %t.so | FileCheck %s
-
-// Test that we create R_PPC64_RELATIVE relocations but don't put any
-// symbols in the dynamic symbol table.
-
-// CHECK:      Relocations [
-// CHECK-NEXT:   Section ({{.*}}) .rela.dyn {
-// CHECK-NEXT:     0x[[FOO_ADDR:.*]] R_PPC64_RELATIVE - 0x[[FOO_ADDR]]
-// CHECK-NEXT:     0x[[BAR_ADDR:.*]] R_PPC64_RELATIVE - 0x[[BAR_ADDR]]
-// CHECK-NEXT:     0x20010 R_PPC64_RELATIVE - 0x20009
-// CHECK-NEXT:     0x{{.*}} R_PPC64_RELATIVE - 0x[[ZED_ADDR:.*]]
-// CHECK-NEXT:     0x{{.*}} R_PPC64_RELATIVE - 0x[[FOO_ADDR]]
-// CHECK-NEXT:     0x20028 R_PPC64_ADDR64 external 0x0
-// CHECK-NEXT:   }
-// CHECK-NEXT: ]
-
-// CHECK:      Symbols [
-// CHECK:        Name: foo
-// CHECK-NEXT:   Value: 0x[[FOO_ADDR]]
-// CHECK:        Name: bar
-// CHECK-NEXT:   Value: 0x[[BAR_ADDR]]
-// CHECK:        Name: zed
-// CHECK-NEXT:   Value: 0x[[ZED_ADDR]]
-// CHECK:      ]
-
-// CHECK:      DynamicSymbols [
-// CHECK-NEXT:   Symbol {
-// CHECK-NEXT:     Name:
-// CHECK-NEXT:     Value: 0x0
-// CHECK-NEXT:     Size: 0
-// CHECK-NEXT:     Binding: Local
-// CHECK-NEXT:     Type: None
-// CHECK-NEXT:     Other: 0
-// CHECK-NEXT:     Section: Undefined
-// CHECK-NEXT:   }
-// CHECK-NEXT:   Symbol {
-// CHECK-NEXT:     Name: external
-// CHECK-NEXT:     Value: 0x0
-// CHECK-NEXT:     Size: 0
-// CHECK-NEXT:     Binding: Global
-// CHECK-NEXT:     Type: None
-// CHECK-NEXT:     Other: 0
-// CHECK-NEXT:     Section: Undefined
-// CHECK-NEXT:   }
-// CHECK-NEXT: ]
-
-        .data
-foo:
-        .quad foo
-
-        .hidden bar
-        .global bar
-bar:
-        .quad bar
-        .quad bar + 1
-
-        .hidden zed
-        .comm zed,1
-        .quad zed
-
-        .section abc,"aw"
-        .quad foo
-
-        .quad external
diff --git a/lld/test/ELF/shared-ppc64.s b/lld/test/ELF/shared-ppc64.s
index f317cedc843..00a27ad0d00 100644
--- a/lld/test/ELF/shared-ppc64.s
+++ b/lld/test/ELF/shared-ppc64.s
@@ -39,6 +39,6 @@
 .global _start
 _start:
 .data
-.long bar
-.long zed
+.quad bar
+.quad zed