[ELF] Refactor getDynRel to print error location

[evgeny777]

Summary

Errors in getDynRel are quite common, so it might be worth printing exact place of error.

[ruiu]

ELF/Relocations.cpp

719 ↗

(On Diff #79100)

I think we don't put a period at end of a sentence in error messages.

722 ↗

(On Diff #79100)

Add a blank line between code and comment.

ELF/Target.cpp

697–698 ↗

(On Diff #79100)

Probably unintended change.

739–740 ↗

(On Diff #79100)

Ditto

821–824 ↗

(On Diff #79100)

Ditto

842 ↗

(On Diff #79100)

Ditto (and so on)

ELF/Target.h

29–31 ↗

(On Diff #79100)

The meaning of the second return value is not very clear. How about this? We can add Target::isAbsRel(uint32_t). If it returns true, we should report an error. Then I think we don't need to modify getDynRel.

[evgeny777]

ELF/Target.h

29–31 ↗

(On Diff #79100)

isAbsRel is a bit confusing (in position independent code RelExpr may be R_GOT_PC as well). How about isPICRel ?

[evgeny777]

Addressed review comments

[ruiu]

LGTM with these comments addressed.

ELF/Relocations.cpp

716 ↗

(On Diff #79197)

We have Config->Pic, so it should be named isPicRel.

722 ↗

(On Diff #79100)

ping

ELF/Target.cpp

1250 ↗

(On Diff #79197)

You can just return Type because R_AARCH64_ABS32 was just a dummy value to return something in an error condition.

1980 ↗

(On Diff #79197)

Remove () after return.

1985 ↗

(On Diff #79197)

This is I think just a dummy value, too.

lld/trunk/ELF/Relocations.cpp

if (needsPlt(Expr) || Expr == R_THUNK_ABS || Expr == R_THUNK_PC ||
       // If the produced value is a constant, we just remember to write it
       // when outputting this section. We also have to do it if the format
       // uses Elf_Rel, since in that case the written value is the addend.
       if (Constant || !RelTy::IsRela)
         C.Relocations.push_back({Expr, Type, Offset, Addend, &Body});
     } else {
       // We don't know anything about the finaly symbol. Just ask the dynamic
       // linker to handle the relocation for us.
+      if (!Target->isPicRel(Type))
+        error(getLocation(C, Offset) + ": relocation " + toString(Type) +
+              " cannot be used against shared object; recompile with -fPIC.");
       AddDyn({Target->getDynRel(Type), &C, Offset, false, &Body, Addend});
+
       // MIPS ABI turns using of GOT and dynamic relocations inside out.
       // While regular ABI uses dynamic relocations to fill up GOT entries
       // MIPS ABI requires dynamic linker to fills up GOT entries using
       // specially sorted dynamic symbol table. This affects even dynamic
       // relocations against symbols which do not require GOT entries
       // creation explicitly, i.e. do not have any GOT-relocations. So if
       // a preemptible symbol has a dynamic relocation we anyway have
       // to create a GOT entry for it.

lld/trunk/ELF/Target.h

 class InputFile;
 class SymbolBody;
 
 class TargetInfo {
 public:
   virtual bool isTlsInitialExecRel(uint32_t Type) const;
   virtual bool isTlsLocalDynamicRel(uint32_t Type) const;
   virtual bool isTlsGlobalDynamicRel(uint32_t Type) const;
+  virtual bool isPicRel(uint32_t Type) const { return true; }
   virtual uint32_t getDynRel(uint32_t Type) const { return Type; }
   virtual void writeGotPltHeader(uint8_t *Buf) const {}
   virtual void writeGotPlt(uint8_t *Buf, const SymbolBody &S) const {};
   virtual uint64_t getImplicitAddend(const uint8_t *Buf, uint32_t Type) const;
 
   // If lazy binding is supported, the first entry of the PLT has code
   // to call the dynamic linker to resolve PLT entries the first time
   // they are called. This function writes that code.

lld/trunk/ELF/Target.cpp

if (!isInt<N>(V) && !isUInt<N>(V))
     error("relocation " + toString(Type) + " out of range");
 }
 
 template <unsigned N> static void checkAlignment(uint64_t V, uint32_t Type) {
   if ((V & (N - 1)) != 0)
     error("improper alignment for relocation " + toString(Type));
 }
 
-static void errorDynRel(uint32_t Type) {
-  error("relocation " + toString(Type) +
-        " cannot be used against shared object; recompile with -fPIC.");
-}
-
 namespace {
 class X86TargetInfo final : public TargetInfo {
 public:
   X86TargetInfo();
   RelExpr getRelExpr(uint32_t Type, const SymbolBody &S) const override;
   uint64_t getImplicitAddend(const uint8_t *Buf, uint32_t Type) const override;
   void writeGotPltHeader(uint8_t *Buf) const override;
   uint32_t getDynRel(uint32_t Type) const override;
public:
   void relaxTlsIeToLe(uint8_t *Loc, uint32_t Type, uint64_t Val) const override;
   void relaxTlsLdToLe(uint8_t *Loc, uint32_t Type, uint64_t Val) const override;
 };
 
 template <class ELFT> class X86_64TargetInfo final : public TargetInfo {
 public:
   X86_64TargetInfo();
   RelExpr getRelExpr(uint32_t Type, const SymbolBody &S) const override;
-  uint32_t getDynRel(uint32_t Type) const override;
+  bool isPicRel(uint32_t Type) const override;
   bool isTlsLocalDynamicRel(uint32_t Type) const override;
   bool isTlsGlobalDynamicRel(uint32_t Type) const override;
   bool isTlsInitialExecRel(uint32_t Type) const override;
   void writeGotPltHeader(uint8_t *Buf) const override;
   void writeGotPlt(uint8_t *Buf, const SymbolBody &S) const override;
   void writePltHeader(uint8_t *Buf) const override;
   void writePlt(uint8_t *Buf, uint64_t GotEntryAddr, uint64_t PltEntryAddr,
                 int32_t Index, unsigned RelOff) const override;
void writePlt(uint8_t *Buf, uint64_t GotEntryAddr, uint64_t PltEntryAddr,
                 int32_t Index, unsigned RelOff) const override;
   void relocateOne(uint8_t *Loc, uint32_t Type, uint64_t Val) const override;
 };
 
 class AArch64TargetInfo final : public TargetInfo {
 public:
   AArch64TargetInfo();
   RelExpr getRelExpr(uint32_t Type, const SymbolBody &S) const override;
-  uint32_t getDynRel(uint32_t Type) const override;
+  bool isPicRel(uint32_t Type) const override;
   bool isTlsInitialExecRel(uint32_t Type) const override;
   void writeGotPlt(uint8_t *Buf, const SymbolBody &S) const override;
   void writePltHeader(uint8_t *Buf) const override;
   void writePlt(uint8_t *Buf, uint64_t GotEntryAddr, uint64_t PltEntryAddr,
                 int32_t Index, unsigned RelOff) const override;
   bool usesOnlyLowPageBits(uint32_t Type) const override;
   void relocateOne(uint8_t *Loc, uint32_t Type, uint64_t Val) const override;
   RelExpr adjustRelaxExpr(uint32_t Type, const uint8_t *Data,
public:
   void relocateOne(uint8_t *Loc, uint32_t Type, uint64_t Val) const override;
   RelExpr getRelExpr(uint32_t Type, const SymbolBody &S) const override;
 };
 
 class ARMTargetInfo final : public TargetInfo {
 public:
   ARMTargetInfo();
   RelExpr getRelExpr(uint32_t Type, const SymbolBody &S) const override;
+  bool isPicRel(uint32_t Type) const override;
   uint32_t getDynRel(uint32_t Type) const override;
   uint64_t getImplicitAddend(const uint8_t *Buf, uint32_t Type) const override;
   bool isTlsLocalDynamicRel(uint32_t Type) const override;
   bool isTlsGlobalDynamicRel(uint32_t Type) const override;
   bool isTlsInitialExecRel(uint32_t Type) const override;
   void writeGotPlt(uint8_t *Buf, const SymbolBody &S) const override;
   void writePltHeader(uint8_t *Buf) const override;
   void writePlt(uint8_t *Buf, uint64_t GotEntryAddr, uint64_t PltEntryAddr,
                 int32_t Index, unsigned RelOff) const override;
   RelExpr getThunkExpr(RelExpr Expr, uint32_t RelocType, const InputFile &File,
                        const SymbolBody &S) const override;
   void relocateOne(uint8_t *Loc, uint32_t Type, uint64_t Val) const override;
 };
 
 template <class ELFT> class MipsTargetInfo final : public TargetInfo {
 public:
   MipsTargetInfo();
   RelExpr getRelExpr(uint32_t Type, const SymbolBody &S) const override;
   uint64_t getImplicitAddend(const uint8_t *Buf, uint32_t Type) const override;
+  bool isPicRel(uint32_t Type) const override;
   uint32_t getDynRel(uint32_t Type) const override;
   bool isTlsLocalDynamicRel(uint32_t Type) const override;
   bool isTlsGlobalDynamicRel(uint32_t Type) const override;
   void writeGotPlt(uint8_t *Buf, const SymbolBody &S) const override;
   void writePltHeader(uint8_t *Buf) const override;
   void writePlt(uint8_t *Buf, uint64_t GotEntryAddr, uint64_t PltEntryAddr,
                 int32_t Index, unsigned RelOff) const override;
   RelExpr getThunkExpr(RelExpr Expr, uint32_t RelocType, const InputFile &File,
void X86_64TargetInfo<ELFT>::writePlt(uint8_t *Buf, uint64_t GotEntryAddr,
   memcpy(Buf, Inst, sizeof(Inst));
 
   write32le(Buf + 2, GotEntryAddr - PltEntryAddr - 6);
   write32le(Buf + 7, Index);
   write32le(Buf + 12, -Index * PltEntrySize - PltHeaderSize - 16);
 }
 
 template <class ELFT>
-uint32_t X86_64TargetInfo<ELFT>::getDynRel(uint32_t Type) const {
-  if (Type == R_X86_64_PC32 || Type == R_X86_64_32)
-    errorDynRel(Type);
-  return Type;
+bool X86_64TargetInfo<ELFT>::isPicRel(uint32_t Type) const {
+  return Type != R_X86_64_PC32 && Type != R_X86_64_32;
 }
 
 template <class ELFT>
 bool X86_64TargetInfo<ELFT>::isTlsInitialExecRel(uint32_t Type) const {
   return Type == R_X86_64_GOTTPOFF;
 }
 
 template <class ELFT>
bool AArch64TargetInfo::usesOnlyLowPageBits(uint32_t Type) const {
   }
 }
 
 bool AArch64TargetInfo::isTlsInitialExecRel(uint32_t Type) const {
   return Type == R_AARCH64_TLSIE_ADR_GOTTPREL_PAGE21 ||
          Type == R_AARCH64_TLSIE_LD64_GOTTPREL_LO12_NC;
 }
 
-uint32_t AArch64TargetInfo::getDynRel(uint32_t Type) const {
-  if (Type == R_AARCH64_ABS32 || Type == R_AARCH64_ABS64)
-    return Type;
-  // Keep it going with a dummy value so that we can find more reloc errors.
-  errorDynRel(Type);
-  return R_AARCH64_ABS32;
+bool AArch64TargetInfo::isPicRel(uint32_t Type) const {
+  return Type == R_AARCH64_ABS32 || Type == R_AARCH64_ABS64;
 }
 
 void AArch64TargetInfo::writeGotPlt(uint8_t *Buf, const SymbolBody &) const {
   write64le(Buf, In<ELF64LE>::Plt->getVA());
 }
 
 static uint64_t getAArch64Page(uint64_t Expr) {
   return Expr & (~static_cast<uint64_t>(0xFFF));
case R_ARM_THM_MOVT_PREL:
     return R_PC;
   case R_ARM_NONE:
     return R_HINT;
   case R_ARM_TLS_LE32:
     return R_TLS;
   }
 }
 
+bool ARMTargetInfo::isPicRel(uint32_t Type) const {
+  return (Type == R_ARM_TARGET1 && !Config->Target1Rel) ||
+         (Type == R_ARM_ABS32);
+}
+
 uint32_t ARMTargetInfo::getDynRel(uint32_t Type) const {
   if (Type == R_ARM_TARGET1 && !Config->Target1Rel)
     return R_ARM_ABS32;
   if (Type == R_ARM_ABS32)
     return Type;
   // Keep it going with a dummy value so that we can find more reloc errors.
-  errorDynRel(Type);
   return R_ARM_ABS32;
 }
 
 void ARMTargetInfo::writeGotPlt(uint8_t *Buf, const SymbolBody &) const {
   write32le(Buf, In<ELF32LE>::Plt->getVA());
 }
 
 void ARMTargetInfo::writePltHeader(uint8_t *Buf) const {
case R_MIPS_GOT_PAGE:
     return R_MIPS_GOT_LOCAL_PAGE;
   case R_MIPS_TLS_GD:
     return R_MIPS_TLSGD;
   case R_MIPS_TLS_LDM:
     return R_MIPS_TLSLD;
   }
 }
 
+template <class ELFT> bool MipsTargetInfo<ELFT>::isPicRel(uint32_t Type) const {
+  return Type == R_MIPS_32 || Type == R_MIPS_64;
+}
+
 template <class ELFT>
 uint32_t MipsTargetInfo<ELFT>::getDynRel(uint32_t Type) const {
-  if (Type == R_MIPS_32 || Type == R_MIPS_64)
-    return RelativeRel;
-  // Keep it going with a dummy value so that we can find more reloc errors.
-  errorDynRel(Type);
-  return R_MIPS_32;
 }
 
 template <class ELFT>
 bool MipsTargetInfo<ELFT>::isTlsLocalDynamicRel(uint32_t Type) const {
   return Type == R_MIPS_TLS_LDM;
 }
 
 template <class ELFT>

lld/trunk/test/ELF/aarch64-fpic-abs16.s

 // REQUIRES: aarch64
 // RUN: llvm-mc -filetype=obj -triple=aarch64-none-freebsd %s -o %t.o
 // RUN: not ld.lld -shared %t.o -o %t.so 2>&1 | FileCheck %s
-// CHECK: relocation R_AARCH64_ABS16 cannot be used against shared object; recompile with -fPIC.
+// CHECK: {{.*}}:(.data+0x0): relocation R_AARCH64_ABS16 cannot be used against shared object; recompile with -fPIC.
 
 .data
   .hword foo

lld/trunk/test/ELF/aarch64-fpic-prel16.s

 // REQUIRES: aarch64
 // RUN: llvm-mc -filetype=obj -triple=aarch64-none-freebsd %s -o %t.o
 // RUN: not ld.lld -shared %t.o -o %t.so 2>&1 | FileCheck %s
-// CHECK: relocation R_AARCH64_PREL16 cannot be used against shared object; recompile with -fPIC.
+// CHECK: {{.*}}:(.data+0x0): relocation R_AARCH64_PREL16 cannot be used against shared object; recompile with -fPIC.
 
 .data
   .hword foo - .

lld/trunk/test/ELF/aarch64-fpic-prel32.s

 // REQUIRES: aarch64
 // RUN: llvm-mc -filetype=obj -triple=aarch64-none-freebsd %s -o %t.o
 // RUN: not ld.lld -shared %t.o -o %t.so 2>&1 | FileCheck %s
-// CHECK: relocation R_AARCH64_PREL32 cannot be used against shared object; recompile with -fPIC.
+// CHECK: {{.*}}:(.data+0x0): relocation R_AARCH64_PREL32 cannot be used against shared object; recompile with -fPIC.
 
 .data
   .word foo - .

lld/trunk/test/ELF/aarch64-fpic-prel64.s

 // REQUIRES: aarch64
 // RUN: llvm-mc -filetype=obj -triple=aarch64-none-freebsd %s -o %t.o
 // RUN: not ld.lld -shared %t.o -o %t.so 2>&1 | FileCheck %s
-// CHECK: relocation R_AARCH64_PREL64 cannot be used against shared object; recompile with -fPIC.
+// CHECK: {{.*}}:(.data+0x0): relocation R_AARCH64_PREL64 cannot be used against shared object; recompile with -fPIC.
 
 .data
   .xword foo - .

lld/trunk/test/ELF/x86-64-dyn-rel-error.s

 // REQUIRES: x86
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %p/Inputs/shared.s -o %t2.o
 // RUN: ld.lld %t2.o -shared -o %t2.so
 // RUN: not ld.lld %t.o %t2.so -o %t 2>&1 | FileCheck %s
 
         .global _start
 _start:
         .data
         .long bar
 
-// CHECK: R_X86_64_32 cannot be used against shared object; recompile with -fPIC.
+// CHECK: {{.*}}:(.data+0x0): relocation R_X86_64_32 cannot be used against shared object; recompile with -fPIC.

lld/trunk/test/ELF/x86-64-dyn-rel-error2.s

 // REQUIRES: x86
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
 // RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %p/Inputs/shared.s -o %t2.o
 // RUN: ld.lld %t2.o -shared -o %t2.so
 // RUN: not ld.lld %t.o %t2.so -o %t 2>&1 | FileCheck %s
 
         .global _start
 _start:
         .data
         .long bar - .
 
-// CHECK: R_X86_64_PC32 cannot be used against shared object; recompile with -fPIC.
+// CHECK: {{.*}}:(.data+0x0): relocation R_X86_64_PC32 cannot be used against shared object; recompile with -fPIC.

lld/trunk/test/ELF/x86-64-reloc-32-fpic.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
 # RUN: not ld.lld -shared %t.o -o %t.so 2>&1 | FileCheck %s
-# CHECK: relocation R_X86_64_32 cannot be used against shared object; recompile with -fPIC.
+# CHECK: {{.*}}:(.data+0x0): relocation R_X86_64_32 cannot be used against shared object; recompile with -fPIC.
 
 .data
 .long _shared

lld/trunk/test/ELF/x86-64-reloc-pc32-fpic.s

 # REQUIRES: x86
 # RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t.o
 # RUN: not ld.lld -shared %t.o -o %t.so 2>&1 | FileCheck %s
-# CHECK: relocation R_X86_64_PC32 cannot be used against shared object; recompile with -fPIC.
+# CHECK: {{.*}}:(.data+0x1): relocation R_X86_64_PC32 cannot be used against shared object; recompile with -fPIC.
 
 .data
 call _shared