[PATCH] D56396: [LLD][ELF] Fix ARM and Thumb V7PILongThunk overflow behavior.

Wed Jan 9 00:42:55 PST 2019

grimar accepted this revision.
grimar added a comment.
This revision is now accepted and ready to land.

This LGTM. Minor nits.

> Remove the overflow check for R_ARM_MOVT_PREL or R_ARM_THM_MOVT_PREL. The ELF for the ARM Architecture document defines these relocations as having no overflow checking so the overflow check was spurious anyway.

FTR spec says: "In §4.6.1.4 Table 4-11, removed incorrect overflow check on R_ARM_MOVT_ABS,
R_ARM_MOVT_PREL and R_ARM_MOVT_BREL."

I guess that could be a reason why overflow was implemented in LLD initially. Anyways this patch seems to do what the latest spec says.

================
Comment at: test/ELF/arm-extreme-range-pi-thunk.s:42
+// CHECK-NEXT:      13c:        ff cf 4e e3     movt    r12, #61439
+// 0x140 + 0xEFFF0000 + 0x0000FEB8 +8 = 0xf0000000 = high
+// CHECK-NEXT:      140:        0f c0 8c e0     add     r12, r12, pc
----------------
Please add a few spaces. Before the `8` here and in the expressions below. For consistency.

================
Comment at: test/ELF/arm-extreme-range-pi-thunk.s:54
+// CHECK-NEXT: f000000c:        00 c0 41 e3     movt    r12, #4096
+// 0xf0000010 + 0x10000000 + 0x0000118 +8 = bits32(0x100000130) = 0x130 = _start
+// CHECK-NEXT: f0000010:        0f c0 8c e0     add     r12, r12, pc
----------------
`+ 8`

================
Comment at: test/ELF/arm-extreme-range-pi-thunk.s:68
+// CHECK-THUMB-NEXT:      13c:  ce f6 ff 7c     movt    r12, #61439
+// 0x140 + 0xEFFF0000 + 0x0000FEBD +4 = 0xf0000001 = high
+// CHECK-THUMB-NEXT:      140:  fc 44   add     r12, pc
----------------
`+ 4`

================
Comment at: test/ELF/arm-extreme-range-pi-thunk.s:80
+// CHECK-THUMB-NEXT: f000000c:  c1 f2 00 0c     movt    r12, #4096
+// 0xf0000010 + 0x10000000 + 0x000011d +4 =0xbits32(0x100000131) = 0x131= _start
+// CHECK-THUMB-NEXT: f0000010:  fc 44   add     r12, pc
----------------
`= 0xbits32(0x100000131)`, `0x131 =`

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56396/new/

https://reviews.llvm.org/D56396