[ELF/AArch64] Factor out overflow check into a separate function.

[ikudrin]

Summary

This patch mainly extracts overflow checks for AArch64 relocations into a separate function.

In addition, it corrects checks for R_AARCH64_PREL16 and R_AARCH64_PREL32 relocations.

[davide]

This, in principle, seems reasonable. I wonder if we can extend the check also to other targets (e.g. x86_64), but maybe that can be done later.

[davide]

ELF/Target.cpp

785

This and PREL16 fix bugs (changing the argument of check from SA to SA - P). I'd commit that separately, because it's a functional change.

[ikudrin]

[ruiu]

I'm not in favor of this change, and I wouldn't do that. The previous code was super clear that it errors if isInt check fails. Now you have to take a look at the definition of checkAArch64OutOfRange function. You saved one line per a function call, but it doesn't seem like a good tradeoff from code readability point of view.

ELF/Target.cpp

 
 // 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 checkOutOfRange(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:
-    if (!isInt<16>(SA))
-      error("Relocation R_AARCH64_ABS16 out of range");
+    checkOutOfRange<16>(SA, Type);
     write16le(Loc, SA);
     break;
   case R_AARCH64_ABS32:
-    if (!isInt<32>(SA))
-      error("Relocation R_AARCH64_ABS32 out of range");
+    checkOutOfRange<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;
-    if (!isInt<21>(X))
-      error("Relocation R_AARCH64_ADR_PREL_LO21 out of range");
+    checkOutOfRange<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);
-    if (!isInt<33>(X)) {
-      if (Type == R_AARCH64_ADR_GOT_PAGE)
-        error("Relocation R_AARCH64_ADR_GOT_PAGE out of range");
-      error("Relocation R_AARCH64_ADR_PREL_PG_HI21 out of range");
-    }
+    checkOutOfRange<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;
-    if (!isInt<28>(X)) {
-      if (Type == R_AARCH64_JUMP26)
-        error("Relocation R_AARCH64_JUMP26 out of range");
-      error("Relocation R_AARCH64_CALL26 out of range");
-    }
+    checkOutOfRange<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");
     // 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:
-    if (!isInt<16>(SA))
-      error("Relocation R_AARCH64_PREL16 out of range");
+    checkOutOfRange<16>(SA - P, Type);
     write16le(Loc, SA - P);
     break;
   case R_AARCH64_PREL32:
-    if (!isInt<32>(SA))
-      error("Relocation R_AARCH64_PREL32 out of range");
+    checkOutOfRange<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));