[PATCH] D27303: [ELF] - Implemented R_386_16 and R_386PC16 relocations

[Rui Ueyama via Phabricator via llvm-commits]

ruiu added inline comments.


================
Comment at: ELF/Target.cpp:457-461
+  // A program or object file using R_386_8, R_386_16, R_386_PC16 or R_386_PC8
+  // relocations is not conformant to latest ABI. The R_386_16, and R_386_8
+  // relocations truncate the computed value to 16 - bits and 8 - bits
+  // respectively. Unfortunately R_386_PC16 and R_386_16 are used by some
+  // applications, for example by FreeBSD loaders.
----------------
emaste wrote:
> I think this comment ought to be reworded slightly -- the boot loaders indeed do not conform to the latest i386 psABI. I think that will be true for any `.code16` assembly, as BIOS bootloaders must be.
> 
> 
Yeah. I think R_386_16 is not a nonstandard weird relocation but it's just that it's out of scope of i386 psABI. By i386, it really means 32-bit protected mode of i386, and not 8086 mode. So I wouldn't say it's unfortunate that we need to support that.


================
Comment at: ELF/Target.cpp:463
+  if (Type == R_386_16 || Type == R_386_PC16) {
+    write16le(Loc, Val);
+    return;
----------------
Don't you need an overflow check?


https://reviews.llvm.org/D27303