[02/10] ELF/AArch64: Add support for checking PREL32 for overflow

[zatrazz]

Summary

Hi ruiu, shankarke,

This is re-post of a previous attempt. This patch adds support for overflow checking when processing R_AARCH64_PREL32 relocations and add tests.

Patch by Will Newton.

[ruiu]

LGTM

lib/ReaderWriter/ELF/AArch64/AArch64RelocationHandler.cpp

77

withinSignedUnsignedRange's definition looks strange to me, so I checked the spec and confirmed that what the function is doing is correct. Can you add a one line note to the function comment for readers like me saying that this is what the spec actually requires?

lib/ReaderWriter/ELF/AArch64/AArch64RelocationHandler.cpp

static void relocR_AARCH64_ABS64(uint8_t *location, uint64_t P, uint64_t S,
   DEBUG(llvm::dbgs() << "\t\tHandle " << LLVM_FUNCTION_NAME << " -";
         llvm::dbgs() << " S: 0x" << Twine::utohexstr(S);
         llvm::dbgs() << " A: 0x" << Twine::utohexstr(A);
         llvm::dbgs() << " P: 0x" << Twine::utohexstr(P);
         llvm::dbgs() << " result: 0x" << Twine::utohexstr(result) << "\n");
   write64le(location, result | read64le(location));
 }
 
-/// \brief R_AARCH64_PREL32 - word32: S + A - P
-static void relocR_AARCH64_PREL32(uint8_t *location, uint64_t P, uint64_t S,
-                                  int64_t A) {
-  int32_t result = (int32_t)((S + A) - P);
-  write32le(location, result + (int32_t)read32le(location));
-}
-
 /// \brief R_AARCH64_ABS32 - word32:  S + A
 static std::error_code relocR_AARCH64_ABS32(uint8_t *location, uint64_t P,
                                             uint64_t S, int64_t A) {
   int64_t result = S + A;
   if (!withinSignedUnsignedRange(result, 32))
     return make_out_of_range_reloc_error();
   DEBUG(llvm::dbgs() << "\t\tHandle " << LLVM_FUNCTION_NAME << " -";
         llvm::dbgs() << " S: 0x" << Twine::utohexstr(S);
DEBUG(llvm::dbgs() << "\t\tHandle " << LLVM_FUNCTION_NAME << " -";
         llvm::dbgs() << " S: 0x" << Twine::utohexstr(S);
         llvm::dbgs() << " A: 0x" << Twine::utohexstr(A);
         llvm::dbgs() << " P: 0x" << Twine::utohexstr(P);
         llvm::dbgs() << " result: 0x" << Twine::utohexstr(result) << "\n");
   write16le(location, result | read16le(location));
   return std::error_code();
 }
 
+/// \brief R_AARCH64_PREL32 - word32: S + A - P
+static std::error_code relocR_AARCH64_PREL32(uint8_t *location, uint64_t P,
+                                             uint64_t S, int64_t A) {
+  int64_t result = S + A - P;
+  if (!withinSignedUnsignedRange(result, 32))
+    return make_out_of_range_reloc_error();
+  write32le(location, result + read32le(location));
+  return std::error_code();
+}
+
 /// \brief R_AARCH64_ADR_PREL_PG_HI21 - Page(S+A) - Page(P)
 static void relocR_AARCH64_ADR_PREL_PG_HI21(uint8_t *location, uint64_t P,
                                             uint64_t S, int64_t A) {
   uint64_t result = (PAGE(S + A) - PAGE(P));
   result = result >> 12;
   uint32_t immlo = result & 0x3;
   uint32_t immhi = result & 0x1FFFFC;
   immlo = immlo << 29;
if (ref.kindNamespace() != Reference::KindNamespace::ELF)
     return std::error_code();
   assert(ref.kindArch() == Reference::KindArch::AArch64);
   switch (ref.kindValue()) {
   case R_AARCH64_NONE:
     break;
   case R_AARCH64_ABS64:
     relocR_AARCH64_ABS64(loc, reloc, target, addend);
     break;
-  case R_AARCH64_PREL32:
-    relocR_AARCH64_PREL32(loc, reloc, target, addend);
-    break;
   case R_AARCH64_ABS32:
     return relocR_AARCH64_ABS32(loc, reloc, target, addend);
   case R_AARCH64_ABS16:
     return relocR_AARCH64_ABS16(loc, reloc, target, addend);
+  case R_AARCH64_PREL32:
+    return relocR_AARCH64_PREL32(loc, reloc, target, addend);
   // Runtime only relocations. Ignore here.
   case R_AARCH64_RELATIVE:
   case R_AARCH64_IRELATIVE:
   case R_AARCH64_JUMP_SLOT:
   case R_AARCH64_GLOB_DAT:
     break;
   case R_AARCH64_ADR_PREL_PG_HI21:
     relocR_AARCH64_ADR_PREL_PG_HI21(loc, reloc, target, addend);

test/elf/AArch64/rel-prel32-overflow.test

+# Check handling of R_AARCH64_PREL32 relocation overflow.
+# RUN: yaml2obj -format=elf %s > %t-obj
+# RUN: not lld -flavor gnu -target arm64 -o %t-exe %t-obj 2>&1 | FileCheck %s
+
+# CHECK: Relocation out of range in file {{.*}}: reference from data1+0 to data2+34359738369 of type 261 (R_AARCH64_PREL32)
+# CHECK: Relocation out of range in file {{.*}}: reference from data2+0 to data1+34359738369 of type 261 (R_AARCH64_PREL32)
+
+!ELF
+FileHeader: !FileHeader
+  Class: ELFCLASS64
+  Data: ELFDATA2LSB
+  Type: ET_REL
+  Machine: EM_AARCH64
+
+Sections:
+- Name: .text
+  Type: SHT_PROGBITS
+  Content: "00000000"
+  AddressAlign: 16
+  Flags: [SHF_ALLOC, SHF_EXECINSTR]
+- Name: .data
+  Type: SHT_PROGBITS
+  Content: "0000000000000000"
+  AddressAlign: 16
+  Flags: [SHF_ALLOC, SHF_WRITE]
+
+- Name: .rela.data
+  Type: SHT_RELA
+  Info: .data
+  AddressAlign: 8
+  Relocations:
+    - Offset: 0x0
+      Symbol: data2
+      Type: R_AARCH64_PREL32
+      Addend: 0x800000001
+    - Offset: 0x4
+      Symbol: data1
+      Type: R_AARCH64_PREL32
+      Addend: 0x800000001
+
+Symbols:
+  Global:
+    - Name: _start
+      Section: .text
+      Value: 0x0
+      Size: 4
+    - Name: data1
+      Section: .data
+      Size: 4
+    - Name: data2
+      Section: .data
+      Value: 0x4
+      Size: 4

test/elf/AArch64/rel-prel32.test

+# Check handling of R_AARCH64_PREL32 relocation.
+# RUN: yaml2obj -format=elf %s > %t-obj
+# RUN: lld -flavor gnu -target arm64 -o %t-exe %t-obj
+# RUN: llvm-objdump -s -t %t-exe | FileCheck %s
+
+# CHECK: Contents of section .data:
+# CHECK-NEXT: 401060 05000080 fdffff7f ........
+#                    ^^ data2 - data1 + 0x80000001 = 0x80000005
+#                             ^^ data1 - data2 + 0x80000001 = 0x7ffffffd
+# CHECK: SYMBOL TABLE:
+# CHECK: 00401060 g  .data  00000004 data1
+# CHECK: 00401064 g  .data  00000004 data2
+
+!ELF
+FileHeader: !FileHeader
+  Class: ELFCLASS64
+  Data: ELFDATA2LSB
+  Type: ET_REL
+  Machine: EM_AARCH64
+
+Sections:
+- Name: .text
+  Type: SHT_PROGBITS
+  Content: "00000000"
+  AddressAlign: 16
+  Flags: [SHF_ALLOC, SHF_EXECINSTR]
+- Name: .data
+  Type: SHT_PROGBITS
+  Content: "0000000000000000"
+  AddressAlign: 16
+  Flags: [SHF_ALLOC, SHF_WRITE]
+
+- Name: .rela.data
+  Type: SHT_RELA
+  Info: .data
+  AddressAlign: 8
+  Relocations:
+    - Offset: 0x0
+      Symbol: data2
+      Type: R_AARCH64_PREL32
+      Addend: 0x80000001
+    - Offset: 0x4
+      Symbol: data1
+      Type: R_AARCH64_PREL32
+      Addend: 0x80000001
+
+Symbols:
+  Global:
+    - Name: _start
+      Section: .text
+      Value: 0x0
+      Size: 4
+    - Name: data1
+      Section: .data
+      Size: 4
+    - Name: data2
+      Section: .data
+      Value: 0x4
+      Size: 4