Fix PREL31 relocation on ARM

[yuyichao]

Summary

This is a 31bits relative relocation instead of a 32bits absolute relocation.

[peter.smith]

Your solution is the same way that R_ARM_PREL31 is handled in lld, so this looks fine from a correctness point of view.

I'm not familiar with this area so I think you'll need approval from a maintainer, they may also insist on a test being written.

[yuyichao]

Your solution is the same way that R_ARM_PREL31 is handled in lld, so this looks fine from a correctness point of view.

Good to know.... Would have saved me some time....

they may also insist on a test being written.

The failure shows up as invalid unwind info. Any pointer to a similar test / how can I write one?

[peter.smith]

Apologies I'm not familiar with mcjit to know how to write a test for this. Speculating:

• To get a prel31 relocation that will fail there will need to be a negative offset. If the .arm.exidx section is after the executable code or any link to a .ARM.extab section in memory/file the offsets will be negative will show up problems.
• If there is anyway to test the top-bit of the relocated entry you should be able to detect it.

[yuyichao]

lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp

468

I think I made a stupid mistake here... Should be 0x80000000 instead of 0x8000000 (7 zeros instead of 6 ......) Will update later.

[yuyichao]

Fix bit31 mask....

[yuyichao]

Add test.

[peter.smith]

This looks good to me from an ARM perspective. I'm not a contributor in this area so you may want to wait a bit to see if anyone objects on style grounds.

[yuyichao]

Ping. Does this still need more review or is this OK to merge? I'll also need someone else to push for me since I don't have commit access.

[rengolin]

There's nothing controversial about this patch, LGTM.

lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.cpp

DEBUG(dbgs() << "resolveARMRelocation, LocalAddress: "
                << " Addend: " << format("%x", Addend) << "\n");
 
   switch (Type) {
   default:
     llvm_unreachable("Not implemented relocation type!");
 
   case ELF::R_ARM_NONE:
     break;
+    // Write a 31bit signed offset
   case ELF::R_ARM_PREL31:
+    *TargetPtr &= 0x80000000;
+    *TargetPtr |= (Value - FinalAddress) & ~0x80000000;
+    break;
   case ELF::R_ARM_TARGET1:
   case ELF::R_ARM_ABS32:
     *TargetPtr = Value;
     break;
     // Write first 16 bit of 32 bit value to the mov instruction.
     // Last 4 bit should be shifted.
   case ELF::R_ARM_MOVW_ABS_NC:
   case ELF::R_ARM_MOVT_ABS:

test/ExecutionEngine/RuntimeDyld/ARM/ELF_ARM_EXIDX_relocations.s

+# RUN: llvm-mc -triple=arm-linux-gnueabihf -filetype=obj -o %T/reloc.o %s
+# RUN: llvm-rtdyld -triple=arm-linux-gnueabihf -verify -map-section reloc.o,.ARM.exidx=0x6000 -map-section reloc.o,.text=0x4000  -dummy-extern __aeabi_unwind_cpp_pr0=0x1234 -check=%s %T/reloc.o
+
+        .text
+        .syntax unified
+        .eabi_attribute 67, "2.09"      @ Tag_conformance
+        .cpu    cortex-a8
+        .fpu    neon
+        .file   "reloc.c"
+        .globl  g
+        .align  2
+        .type   g,%function
+g:
+        .fnstart
+        movw    r0, #1
+        bx      lr
+        .Lfunc_end0:
+        .size   g, .Lfunc_end0-g
+        .fnend
+
+# rtdyld-check: *{4}(section_addr(reloc.o, .ARM.exidx)) = (g - (section_addr(reloc.o, .ARM.exidx))) & 0x7fffffff
+# Compat unwind info: finish(0xb0), finish(0xb0), finish(0xb0)
+# rtdyld-check: *{4}(section_addr(reloc.o, .ARM.exidx) + 0x4) = 0x80b0b0b0