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

[George Rimar via Phabricator via llvm-commits]

grimar added a comment.

In https://reviews.llvm.org/D30699#708077, @ruiu wrote:

> Do you mean that https://github.com/hjl-tools/x86-psABI/wiki/intel386-psABI-1.1.pdf and https://github.com/hjl-tools/x86-psABI/wiki/intel386-psABI-draft.pdf are incompatible in such a way that valid code produced by gas 2.27 that worked with the former spec no longer works with the latter spec? I'd be surprised if there's such code because that would mean the new draft spec is going to introduce an incompatible change to the ABI, which shouldn't happen.


No, code produced by GAS is not affected I think. Problem is relative to code produced by llvm-mc.

Released ABI does not say that R_386_GOT32 should be be calculated differently for baseless case. Draft says that.
See what llvm-mc currently produces when -relax-relocations=false:
movl bar at GOT, %eax encoded as a1 00 00 00 00 mov    0x0,%eax (R_386_GOT32)
movl foo at GOT, %ebx encoded as 8b 1d 00 00 00 00 mov    0x0,%ebx (R_386_GOT32)

So uses 0xa1 opcode to encode first operation instead 0x8b opcode + ModR/M byte. 
gold HEAD I think calculates relocation wrong for such objects (see bug I posted https://sourceware.org/bugzilla/show_bug.cgi?id=21285). 
Because gold assumes that "0x8b" is always used to encode "mov", what true for GAS but not true for llvm-mc.

Both draft and released ABI on page 37 says: "mov name at GOT, %eax must be encoded with opcode 0x8b, not 0xa0,** to allow linker optimization**.". 
But I am not doing any optimizations yet. R_386_GOT32 is not supposed to be optimized according to ABI. So code produced by llvm-mc looks correct for me.

And then it is not clear for me how to check the instructions properly to calculate R_386_GOT32.
I can't assume that Buf[-1] is ModRM byte, it can be either instruction opcode or ModRM at the same time. I would like to
see something like "mov name at GOT, %eax must be encoded with opcode 0x8b, not 0xa0 **Period**" in ABI, that way implementation would be clear.


https://reviews.llvm.org/D30699