[ELF] Factor out relocation checks into separate functions.

[ikudrin]

Summary

It allows to standardize common checks and unify error messages.

[• rafael]

LGTM for me, but please wait to see what Rui thinks.

There is a few extra functions to look at, but there are many uses of them, which makes for a good tradeoff IMHO.

[ruiu]

ELF/Target.cpp

45 ↗

(On Diff #41011)

checkIsInt -> checkInt
Val -> V

46–48 ↗

(On Diff #41011)

It's a minor point, but I don't like to mix strings with getElfRelocationTypeName() function call because the function name is too long. I'd write like this.

if (isInt<N>(V))
  return;
StringRef S = getELFRelocationTypeName(Config->EMachine, Type);
error("Relocation " + S + " out of range");

51 ↗

(On Diff #41011)

Ditto

57 ↗

(On Diff #41011)

Ditto

[ikudrin]

• Rename check functions and rework their bodies.

[ruiu]

LGTM with these fixes.

ELF/Target.cpp

59 ↗

(On Diff #41103)

We usually represent an alignment by its value and not in the form of log2, so for consistency,

uint64_t Mask = N - 1;

or

if ((V & (N - 1)) == 0)

615 ↗

(On Diff #41103)

So this becomes

checkAlignment<4>

818 ↗

(On Diff #41103)

checkAlignment<8>

lld/trunk/ELF/Target.cpp

 
 template <endianness E> static void add32(void *P, int32_t V) {
   write32<E>(P, read32<E>(P) + V);
 }
 
 static void add32le(uint8_t *P, int32_t V) { add32<support::little>(P, V); }
 static void or32le(uint8_t *P, int32_t V) { write32le(P, read32le(P) | V); }
 
+template <unsigned N> static void checkInt(int64_t V, uint32_t Type) {
+  if (isInt<N>(V))
+    return;
+  StringRef S = getELFRelocationTypeName(Config->EMachine, Type);
+  error("Relocation " + S + " out of range");
+}
+
+template <unsigned N> static void checkUInt(uint64_t V, uint32_t Type) {
+  if (isUInt<N>(V))
+    return;
+  StringRef S = getELFRelocationTypeName(Config->EMachine, Type);
+  error("Relocation " + S + " out of range");
+}
+
+template <unsigned N> static void checkAlignment(uint64_t V, uint32_t Type) {
+  if ((V & (N - 1)) == 0)
+    return;
+  StringRef S = getELFRelocationTypeName(Config->EMachine, Type);
+  error("Improper alignment for relocation " + S);
+}
+
 namespace {
 class X86TargetInfo final : public TargetInfo {
 public:
   X86TargetInfo();
   void writeGotPltHeaderEntries(uint8_t *Buf) const override;
   unsigned getDynReloc(unsigned Type) const override;
   bool isTlsDynReloc(unsigned Type) const override;
   void writeGotPltEntry(uint8_t *Buf, uint64_t Plt) const override;
void X86_64TargetInfo::relocateOne(uint8_t *Loc, uint8_t *BufEnd, uint32_t Type,
   case R_X86_64_TPOFF64:
     write32le(Loc, SA - P);
     break;
   case R_X86_64_64:
   case R_X86_64_DTPOFF64:
     write64le(Loc, SA);
     break;
   case R_X86_64_32:
+    checkUInt<32>(SA, Type);
+    write32le(Loc, SA);
+    break;
   case R_X86_64_32S:
-    if (Type == R_X86_64_32 && !isUInt<32>(SA))
-      error("R_X86_64_32 out of range");
-    else if (!isInt<32>(SA))
-      error("R_X86_64_32S out of range");
+    checkInt<32>(SA, Type);
     write32le(Loc, SA);
     break;
   case R_X86_64_DTPOFF32:
     write32le(Loc, SA);
     break;
   case R_X86_64_TPOFF32: {
     uint64_t Val = SA - Out<ELF64LE>::TlsPhdr->p_memsz;
-    if (!isInt<32>(Val))
-      error("R_X86_64_TPOFF32 out of range");
+    checkInt<32>(Val, Type);
     write32le(Loc, Val);
     break;
   }
   default:
     error("unrecognized reloc " + Twine(Type));
   }
 }
 
void PPC64TargetInfo::relocateOne(uint8_t *Loc, uint8_t *BufEnd, uint32_t Type,
   case R_PPC64_TOC16_LO_DS: Type = R_PPC64_ADDR16_LO_DS; SA -= TB; break;
   case R_PPC64_TOC16_HI:    Type = R_PPC64_ADDR16_HI;    SA -= TB; break;
   case R_PPC64_TOC16_HA:    Type = R_PPC64_ADDR16_HA;    SA -= TB; break;
   default: break;
   }
 
   switch (Type) {
   case R_PPC64_ADDR16:
-    if (!isInt<16>(SA))
-      error("Relocation R_PPC64_ADDR16 overflow");
+    checkInt<16>(SA, Type);
     write16be(Loc, SA);
     break;
   case R_PPC64_ADDR16_DS:
-    if (!isInt<16>(SA))
-      error("Relocation R_PPC64_ADDR16_DS overflow");
+    checkInt<16>(SA, Type);
     write16be(Loc, (read16be(Loc) & 3) | (SA & ~3));
     break;
   case R_PPC64_ADDR16_LO:
     write16be(Loc, applyPPCLo(SA));
     break;
   case R_PPC64_ADDR16_LO_DS:
     write16be(Loc, (read16be(Loc) & 3) | (applyPPCLo(SA) & ~3));
     break;
case R_PPC64_ADDR16_HIGHERA:
     break;
   case R_PPC64_ADDR16_HIGHEST:
     write16be(Loc, applyPPCHighest(SA));
     break;
   case R_PPC64_ADDR16_HIGHESTA:
     write16be(Loc, applyPPCHighesta(SA));
     break;
   case R_PPC64_ADDR14: {
-    if ((SA & 3) != 0)
-      error("Improper alignment for relocation R_PPC64_ADDR14");
-
+    checkAlignment<4>(SA, Type);
     // Preserve the AA/LK bits in the branch instruction
     uint8_t AALK = Loc[3];
     write16be(Loc + 2, (AALK & 3) | (SA & 0xfffc));
     break;
   }
   case R_PPC64_REL16_LO:
     write16be(Loc, applyPPCLo(SA - P));
     break;
   case R_PPC64_REL16_HI:
     write16be(Loc, applyPPCHi(SA - P));
     break;
   case R_PPC64_REL16_HA:
     write16be(Loc, applyPPCHa(SA - P));
     break;
   case R_PPC64_ADDR32:
-    if (!isInt<32>(SA))
-      error("Relocation R_PPC64_ADDR32 overflow");
+    checkInt<32>(SA, Type);
     write32be(Loc, SA);
     break;
   case R_PPC64_REL24: {
     // If we have an undefined weak symbol, we might get here with a symbol
     // address of zero. That could overflow, but the code must be unreachable,
     // so don't bother doing anything at all.
     if (!SA)
       break;
if (!InPlt && Out<ELF64BE>::Opd) {
       uint64_t OpdEnd = OpdStart + Out<ELF64BE>::Opd->getSize();
       bool InOpd = OpdStart <= SA && SA < OpdEnd;
 
       if (InOpd)
         SA = read64be(&Out<ELF64BE>::OpdBuf[SA - OpdStart]);
     }
 
     uint32_t Mask = 0x03FFFFFC;
-    if (!isInt<24>(SA - P))
-      error("Relocation R_PPC64_REL24 overflow");
+    checkInt<24>(SA - P, Type);
     write32be(Loc, (read32be(Loc) & ~Mask) | ((SA - P) & Mask));
 
     uint32_t Nop = 0x60000000;
     if (InPlt && Loc + 8 <= BufEnd && read32be(Loc + 4) == Nop)
       write32be(Loc + 4, 0xe8410028); // ld %r2, 40(%r1)
     break;
   }
   case R_PPC64_REL32:
-    if (!isInt<32>(SA - P))
-      error("Relocation R_PPC64_REL32 overflow");
+    checkInt<32>(SA - P, Type);
     write32be(Loc, SA - P);
     break;
   case R_PPC64_REL64:
     write64be(Loc, SA - P);
     break;
   case R_PPC64_ADDR64:
   case R_PPC64_TOC:
     write64be(Loc, SA);

 
 // Page(Expr) is the page address of the expression Expr, defined
 // as (Expr & ~0xFFF). (This applies even if the machine page size
 // supported by the platform has a different value.)
 static uint64_t getAArch64Page(uint64_t Expr) {
   return Expr & (~static_cast<uint64_t>(0xFFF));
 }
 
-template <unsigned N>
-static void checkAArch64OutOfRange(int64_t X, uint32_t Type) {
-  if (!isInt<N>(X))
-    error("Relocation " + getELFRelocationTypeName(EM_AARCH64, Type) +
-          " out of range");
-}
-
 void AArch64TargetInfo::relocateOne(uint8_t *Loc, uint8_t *BufEnd,
                                     uint32_t Type, uint64_t P,
                                     uint64_t SA) const {
   switch (Type) {
   case R_AARCH64_ABS16:
-    checkAArch64OutOfRange<16>(SA, Type);
+    checkInt<16>(SA, Type);
     write16le(Loc, SA);
     break;
   case R_AARCH64_ABS32:
-    checkAArch64OutOfRange<32>(SA, Type);
+    checkInt<32>(SA, Type);
     write32le(Loc, SA);
     break;
   case R_AARCH64_ABS64:
     // No overflow check needed.
     write64le(Loc, SA);
     break;
   case R_AARCH64_ADD_ABS_LO12_NC:
     // No overflow check needed.
     // This relocation stores 12 bits and there's no instruction
     // to do it. Instead, we do a 32 bits store of the value
     // of r_addend bitwise-or'ed Loc. This assumes that the addend
     // bits in Loc are zero.
     or32le(Loc, (SA & 0xFFF) << 10);
     break;
   case R_AARCH64_ADR_PREL_LO21: {
     uint64_t X = SA - P;
-    checkAArch64OutOfRange<21>(X, Type);
+    checkInt<21>(X, Type);
     updateAArch64Adr(Loc, X & 0x1FFFFF);
     break;
   }
   case R_AARCH64_ADR_GOT_PAGE:
   case R_AARCH64_ADR_PREL_PG_HI21: {
     uint64_t X = getAArch64Page(SA) - getAArch64Page(P);
-    checkAArch64OutOfRange<33>(X, Type);
+    checkInt<33>(X, Type);
     updateAArch64Adr(Loc, (X >> 12) & 0x1FFFFF); // X[32:12]
     break;
   }
   case R_AARCH64_JUMP26:
   case R_AARCH64_CALL26: {
     uint64_t X = SA - P;
-    checkAArch64OutOfRange<28>(X, Type);
+    checkInt<28>(X, Type);
     or32le(Loc, (X & 0x0FFFFFFC) >> 2);
     break;
   }
   case R_AARCH64_LDST32_ABS_LO12_NC:
     // No overflow check needed.
     or32le(Loc, (SA & 0xFFC) << 8);
     break;
   case R_AARCH64_LD64_GOT_LO12_NC:
-    if (SA & 0x7)
-      error("Relocation R_AARCH64_LD64_GOT_LO12_NC not aligned");
+    checkAlignment<8>(SA, Type);
     // No overflow check needed.
     or32le(Loc, (SA & 0xFF8) << 7);
     break;
   case R_AARCH64_LDST64_ABS_LO12_NC:
     // No overflow check needed.
     or32le(Loc, (SA & 0xFF8) << 7);
     break;
   case R_AARCH64_LDST8_ABS_LO12_NC:
     // No overflow check needed.
     or32le(Loc, (SA & 0xFFF) << 10);
     break;
   case R_AARCH64_PREL16:
-    checkAArch64OutOfRange<16>(SA - P, Type);
+    checkInt<16>(SA - P, Type);
     write16le(Loc, SA - P);
     break;
   case R_AARCH64_PREL32:
-    checkAArch64OutOfRange<32>(SA - P, Type);
+    checkInt<32>(SA - P, Type);
     write32le(Loc, SA - P);
     break;
   case R_AARCH64_PREL64:
     // No overflow check needed.
     write64le(Loc, SA - P);
     break;
   default:
     error("unrecognized reloc " + Twine(Type));
void MipsTargetInfo<ELFT>::relocateOne(uint8_t *Loc, uint8_t *BufEnd,
   const endianness E = ELFT::TargetEndianness;
   switch (Type) {
   case R_MIPS_32:
     add32<E>(Loc, SA);
     break;
   case R_MIPS_CALL16:
   case R_MIPS_GOT16: {
     int64_t V = SA - getMipsGpAddr<ELFT>();
-    if (Type == R_MIPS_GOT16 && !isInt<16>(V))
-      error("Relocation R_MIPS_GOT16 out of range");
+    if (Type == R_MIPS_GOT16)
+      checkInt<16>(V, Type);
     write32<E>(Loc, (read32<E>(Loc) & 0xffff0000) | (V & 0xffff));
     break;
   }
   default:
     error("unrecognized reloc " + Twine(Type));
   }
 }
 

lld/trunk/test/ELF/ppc64-addr16-error.s

 // RUN: llvm-mc -filetype=obj -triple=powerpc64-unknown-linux %s -o %t
 // RUN: not ld.lld -shared %t -o %t2 2>&1 | FileCheck %s
 // REQUIRES: ppc
 
 .short sym+65539
 
-// CHECK: Relocation R_PPC64_ADDR16 overflow
+// CHECK: Relocation R_PPC64_ADDR16 out of range