[PATCH] D37444: [LLD][ELF] Add alignment checks for the LDST<N>_ABS_LO12_NC relocations

[Peter Smith via Phabricator via llvm-commits]

peter.smith created this revision.
Herald added subscribers: kristof.beyls, javed.absar, emaste, aemerson.

The R_AARCH64_LDST<N>_ABS LO12_NC relocations where N is 8, 16, 32, 64 or 128 have a scaled immediate. For example R_AARCH64_LDST32_ABS_LO12_NC shifts the calculated value right by 4. If the target symbol + relocation addend is not aligned properly then bits of the answer will be lost.

      

This change adds an alignment check to the relocations to make sure the target of the relocation is aligned properly. This matches the behavior of GNU ld. The motivation is to catch ODR violations such as a declaration of extern int foo, but a definition of bool foo as the compiler may use R_AARCH64_LDST32_ABS_LO12_NC for the former, but not align the destination.

This was discovered while investigating a BuildBot failure where GNU ld caught the ODR violations but lld did not https://bugs.llvm.org/show_bug.cgi?id=34453


https://reviews.llvm.org/D37444

Files:
  ELF/Arch/AArch64.cpp
  test/ELF/aarch64-lo12-alignment.s


Index: test/ELF/aarch64-lo12-alignment.s
===================================================================
--- /dev/null
+++ test/ELF/aarch64-lo12-alignment.s
@@ -0,0 +1,44 @@
+// RUN: llvm-mc -filetype=obj -triple=aarch64-none-linux %s -o %t
+// RUN: not ld.lld %t -o %t2 2>&1 | FileCheck %s
+
+// Test derived from a typical ODR violation where a global is declared
+// extern int but defined as a half or byte sized type.
+ .section .text
+ .globl _start
+ .type _start, %function
+// Access foo2 as if it were an aligned 32-bit int, expect an error as
+// foo is not aligned
+
+_start:
+ ldrb w2, [x0, #:lo12:foo1]  // Ok as no shift involved
+ ldrh w2, [x0, #:lo12:foo1]  // Error foo1 is not 2-byte aligned
+ ldrh w2, [x0, #:lo12:foo2]  // Ok as foo2 is 2-byte aligned
+ ldr  w2, [x0, #:lo12:foo2]  // Error foo2 is not 4-byte aligned
+ ldr  w2, [x0, #:lo12:foo4]  // Ok as foo4 is 4-byte aligned
+ ldr  x3, [x0, #:lo12:foo4]  // Error foo4 is not 8-byte aligned
+ ldr  x3, [x0, #:lo12:foo8]  // Ok as foo8 is 8-byte aligned
+ ldr  q0, [x0, #:lo12:foo8]  // Error foo8 is not 16-byte aligned
+ ldr  q0, [x0, #:lo12:foo16] // Ok as foo16 is 16-byte aligned
+
+ .section .data.bool, "a", @nobits
+ .balign 16
+ .globl foo16
+ .globl foo1
+ .globl foo2
+ .globl foo4
+ .globl foo8
+foo16:
+ .space 1
+foo1:
+ .space 1
+foo2:
+ .space 2
+foo4:
+ .space 4
+foo8:
+ .space 8
+
+// CHECK: improper alignment for relocation R_AARCH64_LDST16_ABS_LO12_NC
+// CHECK-NEXT: improper alignment for relocation R_AARCH64_LDST32_ABS_LO12_NC
+// CHECK-NEXT: improper alignment for relocation R_AARCH64_LDST64_ABS_LO12_NC
+// CHECK-NEXT: improper alignment for relocation R_AARCH64_LDST128_ABS_LO12_NC
Index: ELF/Arch/AArch64.cpp
===================================================================
--- ELF/Arch/AArch64.cpp
+++ ELF/Arch/AArch64.cpp
@@ -251,15 +251,19 @@
     or32AArch64Imm(Loc, getBits(Val, 0, 11));
     break;
   case R_AARCH64_LDST16_ABS_LO12_NC:
+    checkAlignment<2>(Loc, Val, Type);
     or32AArch64Imm(Loc, getBits(Val, 1, 11));
     break;
   case R_AARCH64_LDST32_ABS_LO12_NC:
+    checkAlignment<4>(Loc, Val, Type);
     or32AArch64Imm(Loc, getBits(Val, 2, 11));
     break;
   case R_AARCH64_LDST64_ABS_LO12_NC:
+    checkAlignment<8>(Loc, Val, Type);
     or32AArch64Imm(Loc, getBits(Val, 3, 11));
     break;
   case R_AARCH64_LDST128_ABS_LO12_NC:
+    checkAlignment<16>(Loc, Val, Type);
     or32AArch64Imm(Loc, getBits(Val, 4, 11));
     break;
   case R_AARCH64_MOVW_UABS_G0_NC:


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D37444.113773.patch
Type: text/x-patch
Size: 2503 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170904/f50ccf82/attachment.bin>