[PATCH] Implementation of relocations: R_ARM_REL32, R_ARM_THM_JUMP11, R_ARM_PREL31 for ELF/ARM

[Denis Protivensky]

Couple of functional errors:

- error in use of `SignExtend`
- veneer handling for `THM_JUMP11` reloc is not fixed

Others are code style changes, but please be careful to address (fix or comment) all of them.


REPOSITORY
  rL LLVM

================
Comment at: lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.cpp:83
@@ +82,3 @@
+
+  return llvm::SignExtend32<11>(imm11 << 1);
+}
----------------
You have 12 bits after the shift, so it should be `SignExtend<12>`, no?

================
Comment at: lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.cpp:96
@@ -86,1 +95,3 @@
+  case R_ARM_THM_JUMP11:
+    return readAddend_THM16_CALL(location);
   case R_ARM_CALL:
----------------
I'd ask you to name `readAddend` method to correspond to the reloc name (`readAddend_THM_JUMP11`). Otherwise it's confusing, and moreover, `THM16` suffix is never used (neither in documentation, nor in the code). The way when more generalized name may be used is when you have couple of relocs specifying related instructions.

================
Comment at: lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.cpp:172
@@ +171,3 @@
+
+/// \brief R_ARM_PREL31 - ((S + A) | T) - P => S + A - P
+static void relocR_ARM_PREL31(uint8_t *location, uint64_t P, uint64_t S,
----------------
The comment is wrong. You don't drop the `T` flag in calculations.

================
Comment at: lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.cpp:187
@@ +186,3 @@
+      llvm::dbgs() << " P: 0x" << Twine::utohexstr(P);
+      llvm::dbgs() << " result: 0x" << Twine::utohexstr(result);
+      llvm::dbgs() << " rel31: 0x" << Twine::utohexstr(rel31) << "\n");
----------------
Add `T` symbol output.

================
Comment at: lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.cpp:260
@@ +259,3 @@
+
+  if (needVeneer)
+    return make_out_of_range_reloc_error();
----------------
You didn't fix this and didn't provide any explanation why you left this code unchanged.

================
Comment at: lib/ReaderWriter/ELF/ARM/ARMRelocationHandler.cpp:273
@@ +272,3 @@
+
+  result = (result & 0x0FFE) >> 1; //we cut off first bit because it is always 1 according to p. 4.5.3
+
----------------
This may be out of 80 symbols. Please, run it through `clang-format`.

http://reviews.llvm.org/D7565

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/