[PATCH] D27055: [ELF] Refactor getDynRel to print error location

[Rui Ueyama via llvm-commits]

ruiu added inline comments.


================
Comment at: ELF/Relocations.cpp:719
+        error(getLocation(C, Offset) + ": relocation " + getRelName(Type) +
+              " cannot be used against shared object; recompile with -fPIC.");
+
----------------
I think we don't put a period at end of a sentence in error messages.


================
Comment at: ELF/Relocations.cpp:722
+      AddDyn({P.first, &C, Offset, false, &Body, Addend});
       // MIPS ABI turns using of GOT and dynamic relocations inside out.
       // While regular ABI uses dynamic relocations to fill up GOT entries
----------------
Add a blank line between code and comment.


================
Comment at: ELF/Target.cpp:697-698
   memcpy(Loc - 4, Inst, sizeof(Inst));
-  // Both code sequences are PC relatives, but since we are moving the constant
+  // Both code sequences are PC relatives, but since we are moving the
+  // constant
   // forward by 8 bytes we have to subtract the value by 8.
----------------
Probably unintended change.


================
Comment at: ELF/Target.cpp:739-740
   } else {
-    fatal("R_X86_64_GOTTPOFF must be used in MOVQ or ADDQ instructions only");
+    fatal("R_X86_64_GOTTPOFF must be used in MOVQ or ADDQ instructions "
+          "only");
   }
----------------
Ditto


================
Comment at: ELF/Target.cpp:821-824
+  // lower 32 bit address space, memory operand in mov can be converted
+  // into
+  // immediate operand. Otherwise, mov must be changed to lea. We support
+  // only
----------------
Ditto


================
Comment at: ELF/Target.cpp:842
+// handles such relaxations. Instructions encoding information was taken
+// from:
 // "Intel 64 and IA-32 Architectures Software Developer's Manual V2"
----------------
Ditto (and so on)


================
Comment at: ELF/Target.h:29-31
+  virtual std::pair<uint32_t, bool> getDynRel(uint32_t Type) const {
+    return {Type, true};
+  }
----------------
The meaning of the second return value is not very clear. How about this? We can add `Target::isAbsRel(uint32_t)`. If it returns true, we should report an error. Then I think we don't need to modify getDynRel.


Repository:
  rL LLVM

https://reviews.llvm.org/D27055