[PATCH] D30699: [ELF] - Stop producing broken output for R_386_GOT32X relocation.

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: ELF/Target.cpp:369
+RelExpr X86TargetInfo::getRelExpr(uint32_t Type, const SymbolBody &S,
+                                  const uint8_t *Loc) const {
+  // R_386_GOT32X usually calculated as G + A - GOT.
----------------
Is there any way to not add a new parameter to this function? This part of code has grown organically to the point that that's hard to understand. Needs simplifying.


================
Comment at: ELF/Target.cpp:377-388
+    // ModR/M byte has form XX YYY ZZZ, where
+    // YYY is MODRM.reg(register 2), ZZZ is MODRM.rm(register 1).
+    // XX has different meanings:
+    // 00: The operand's memory address is in reg1.
+    // 01: The operand's memory address is reg1 + a byte-sized displacement.
+    // 10: The operand's memory address is reg1 + a word-sized displacement.
+    // 11: The operand is reg1 itself.
----------------
I don't think you need to repeat the x86 instruction format here. Instead you want to describe it at high level. If I understand correctly, you are checking if the instruction the relocation is pointing to has %ebp as its operand, right?


================
Comment at: ELF/Target.cpp:394-395
+              " against '" + S.getName() +
+              "' without base register can not be "
+              "used when PIC enabled");
+      return R_GOT;
----------------
Concatenate the strings.


https://reviews.llvm.org/D30699