[ELF] - Implemented R_386_16 and R_386PC16 relocations

[grimar]

Summary

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. R_386_PC16 and R_386_16 are used by some
applications, for example by FreeBSD loaders.

Previously we did not take addend in account for these relocation,
counting it as 0, what is wrong and was a reason of hangs.

This patch needed for example for FreeBSD pmbr (protective mbr).

[grimar]

• Uploaded correct testcase.

[grimar]

Upd: Second issue I mentioned in description relative to D27200 and I think should be patched after it or different solution be landed.

[emaste]

ELF/Target.cpp

457–461 ↗

(On Diff #79922)

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.

[ruiu]

ELF/Target.cpp

457–461 ↗

(On Diff #79922)

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.

463 ↗

(On Diff #79922)

Don't you need an overflow check?

[emaste]

ELF/Target.cpp

457–461 ↗

(On Diff #79922)

Maybe something like R_386_PC16 and R_386_16 are not part of the current i386 psABI. They are used by 16-bit x86 objects, like boot loaders?

[grimar]

• Addressed review comments.
• Changed testcase to be single file and show how we truncate the value.

ELF/Target.cpp

463 ↗

(On Diff #79922)

If you mean some truncation validation, then no.
intel386-psABI-1.1.pdf says:
"The R_386_16, and R_386_8 relocations
truncate the computed value to 16-bits and 8-bits respectively"
It does not say we should do any checks.

I moved the common checkInt<32>(Loc, Val, Type); check above new code though,
it looks reasonable.

[ruiu]

LGTM

lld/trunk/ELF/Target.cpp

RelExpr X86TargetInfo::getRelExpr(uint32_t Type, const SymbolBody &S) const {
   default:
     return R_ABS;
   case R_386_TLS_GD:
     return R_TLSGD;
   case R_386_TLS_LDM:
     return R_TLSLD;
   case R_386_PLT32:
     return R_PLT_PC;
+  case R_386_PC16:
   case R_386_PC32:
     return R_PC;
   case R_386_GOTPC:
     return R_GOTONLY_PC_FROM_END;
   case R_386_TLS_IE:
     return R_GOT;
   case R_386_GOT32:
   case R_386_GOT32X:
void X86TargetInfo::writePlt(uint8_t *Buf, uint64_t GotEntryAddr,
   write32le(Buf + 12, -Index * PltEntrySize - PltHeaderSize - 16);
 }
 
 uint64_t X86TargetInfo::getImplicitAddend(const uint8_t *Buf,
                                           uint32_t Type) const {
   switch (Type) {
   default:
     return 0;
+  case R_386_16:
   case R_386_32:
   case R_386_GOT32:
   case R_386_GOT32X:
   case R_386_GOTOFF:
   case R_386_GOTPC:
+  case R_386_PC16:
   case R_386_PC32:
   case R_386_PLT32:
   case R_386_TLS_LE:
     return read32le(Buf);
   }
 }
 
 void X86TargetInfo::relocateOne(uint8_t *Loc, uint32_t Type,
                                 uint64_t Val) const {
   checkInt<32>(Loc, Val, Type);
+
+  // R_386_PC16 and R_386_16 are not part of the current i386 psABI. They are
+  // used by 16-bit x86 objects, like boot loaders.
+  if (Type == R_386_16 || Type == R_386_PC16) {
+    write16le(Loc, Val);
+    return;
+  }
   write32le(Loc, Val);
 }
 
 void X86TargetInfo::relaxTlsGdToLe(uint8_t *Loc, uint32_t Type,
                                    uint64_t Val) const {
   // Convert
   //   leal x@tlsgd(, %ebx, 1),
   //   call __tls_get_addr@plt

lld/trunk/test/ELF/i386-pc16.test

+# REQUIRES: x86
+
+# RUN: yaml2obj %s -o %t.o
+# RUN: ld.lld %t.o -o %t.exe
+# RUN: llvm-objdump -s -section=.text %t.exe 2>&1 | FileCheck %s
+
+# CHECK:      Contents of section .text:
+# CHECK-NEXT:  11000 56441111 52341111
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS32
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_386
+Sections:
+  - Type:            SHT_PROGBITS
+    Name:            .text
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    AddressAlign:    0x04
+    Content:         "1111111111111111"
+  - Type:            SHT_REL
+    Name:            .rel.text
+    Link:            .symtab
+    Info:            .text
+    AddressAlign:    0x04
+    Relocations:
+      - Offset:          0
+        Symbol:          _start
+        Type:            R_386_16
+      - Offset:          4
+        Symbol:          _start
+        Type:            R_386_PC16
+Symbols:
+  Global:
+    - Name:     _start
+      Type:     STT_FUNC
+      Section:  .text
+      Value:    0x12345
+      Size:     4