[lld][X86] Restore gotEntrySize.

[hvdijk]

Summary

D62727 removed GotEntrySize and GotPltEntrySize with a comment that they are always equal to wordsize(), but that is not entirely true: X32 has a word size of 4, but needs 8-byte GOT entries. This restores gotEntrySize for both, adjusted for current naming conventions, but defaults it to config->wordsize to keep things simple for architectures other than x86_64.

This partially reverts D62727.

[MaskRay]

We shouldn't need both GotEntrySize and GotPltEntrySize. One is sufficient.

lld/test/ELF/x86-x32-plt.s

2

Don'y copy x86-64-plt.s. Newer tests prefer #

Consider riscv-plt.s

8

Most tests llvm-readelf should be used. Its output is succinct.

60

This line is not useful.

63

The comment after the instruction is useful. It computes the target address.

[hvdijk]

In D102509#2760035, @MaskRay wrote:

We shouldn't need both GotEntrySize and GotPltEntrySize. One is sufficient.

Alright, will update to have just GotEntrySize.

lld/test/ELF/x86-x32-plt.s

2

I'd like to avoid unnecessary differences between this and x86-64-plt.s since they both basically test the same thing, just for slightly different targets. Will adjust the other test as well to keep them similar, same for the other comments.

[MaskRay]

lld/test/ELF/x86-x32-plt.s

2

please don't copy x86-64-plt.s. i'll improve x86-64-plt.s shortly.

[MaskRay]

Hope the scheme here should be generic enough to support aarch64-ilp32 and Morello.

[hvdijk]

I'm assuming you meant don't base the test on the old x86-64-plt.s, but the new is fine. Done now, and removed gotPltEntrySize.

[MaskRay]

Is x86-x32-plt.s NFC for now and you'll create another patch to fix x32?

lld/test/ELF/x86-x32-plt.s

7

You can drop %t.so testing.

I added more to x86-64-plt.s because there needs a test to check additional target agnostic properties.

x86-x32-plt.s doesn't need to replicate the testing.

[hvdijk]

In D102509#2760928, @MaskRay wrote:

Is x86-x32-plt.s NFC for now and you'll create another patch to fix x32?

This isn't NFC, this fixes the test in here. Other x32 problems remain but they're separate issues and I'll create separate patches for them.

I'll take the -shared things out of the test.

[hvdijk]

Removed the shared object testing.

[peter.smith]

In D102509#2760109, @MaskRay wrote:

Hope the scheme here should be generic enough to support aarch64-ilp32 and Morello.

For Morello, there is already a similar downstream patch that introduces gotEntrySize as separate from word-size (128-bit and 64-bit respectively) so yes it can work. I'm not familiar with an existing LLD implementation of aarch64-ilp32 but I can't see any reason why it wouldn't work.

lld/ELF/Arch/X86_64.cpp

X86_64::X86_64() {
   pltRel = R_X86_64_JUMP_SLOT;
   relativeRel = R_X86_64_RELATIVE;
   iRelativeRel = R_X86_64_IRELATIVE;
   symbolicRel = R_X86_64_64;
   tlsDescRel = R_X86_64_TLSDESC;
   tlsGotRel = R_X86_64_TPOFF64;
   tlsModuleIndexRel = R_X86_64_DTPMOD64;
   tlsOffsetRel = R_X86_64_DTPOFF64;
+  gotEntrySize = 8;
   pltHeaderSize = 16;
   pltEntrySize = 16;
   ipltEntrySize = 16;
   trapInstr = {0xcc, 0xcc, 0xcc, 0xcc}; // 0xcc = INT3
   nopInstrs = nopInstructions;
 
   // Align to the large page size (known as a superpage or huge page).
   // FreeBSD automatically promotes large, superpage-aligned allocations.

lld/ELF/Symbols.cpp

 }
 
 uint64_t Symbol::getGotVA() const {
   if (gotInIgot)
     return in.igotPlt->getVA() + getGotPltOffset();
   return in.got->getVA() + getGotOffset();
 }
 
-uint64_t Symbol::getGotOffset() const { return gotIndex * config->wordsize; }
+uint64_t Symbol::getGotOffset() const {
+  return gotIndex * target->gotEntrySize;
+}
 
 uint64_t Symbol::getGotPltVA() const {
   if (isInIplt)
     return in.igotPlt->getVA() + getGotPltOffset();
   return in.gotPlt->getVA() + getGotPltOffset();
 }
 
 uint64_t Symbol::getGotPltOffset() const {
   if (isInIplt)
-    return pltIndex * config->wordsize;
-  return (pltIndex + target->gotPltHeaderEntriesNum) * config->wordsize;
+    return pltIndex * target->gotEntrySize;
+  return (pltIndex + target->gotPltHeaderEntriesNum) * target->gotEntrySize;
 }
 
 uint64_t Symbol::getPltVA() const {
   uint64_t outVA = isInIplt
                        ? in.iplt->getVA() + pltIndex * target->ipltEntrySize
                        : in.plt->getVA() + in.plt->headerSize +
                              pltIndex * target->pltEntrySize;
 

lld/ELF/SyntheticSections.cpp

void EhFrameSection::writeTo(uint8_t *buf) {
   for (EhInputSection *s : sections)
     s->relocateAlloc(buf, nullptr);
 
   if (getPartition().ehFrameHdr && getPartition().ehFrameHdr->getParent())
     getPartition().ehFrameHdr->write();
 }
 
 GotSection::GotSection()
-    : SyntheticSection(SHF_ALLOC | SHF_WRITE, SHT_PROGBITS, config->wordsize,
-                       ".got") {
+    : SyntheticSection(SHF_ALLOC | SHF_WRITE, SHT_PROGBITS,
+                       target->gotEntrySize, ".got") {
   numEntries = target->gotHeaderEntriesNum;
 }
 
 void GotSection::addEntry(Symbol &sym) {
   sym.gotIndex = numEntries;
   ++numEntries;
 }
 

 }
 
 void GotPltSection::addEntry(Symbol &sym) {
   assert(sym.pltIndex == entries.size());
   entries.push_back(&sym);
 }
 
 size_t GotPltSection::getSize() const {
-  return (target->gotPltHeaderEntriesNum + entries.size()) * config->wordsize;
+  return (target->gotPltHeaderEntriesNum + entries.size()) *
+         target->gotEntrySize;
 }
 
 void GotPltSection::writeTo(uint8_t *buf) {
   target->writeGotPltHeader(buf);
-  buf += target->gotPltHeaderEntriesNum * config->wordsize;
+  buf += target->gotPltHeaderEntriesNum * target->gotEntrySize;
   for (const Symbol *b : entries) {
     target->writeGotPlt(buf, *b);
-    buf += config->wordsize;
+    buf += target->gotEntrySize;
   }
 }
 
 bool GotPltSection::isNeeded() const {
   // We need to emit GOTPLT even if it's empty if there's a relocation relative
   // to it.
   return !entries.empty() || hasGotPltOffRel;
 }
static StringRef getIgotPltName() {
   return ".got.plt";
 }
 
 // On PowerPC64 the GotPltSection type is SHT_NOBITS so we have to follow suit
 // with the IgotPltSection.
 IgotPltSection::IgotPltSection()
     : SyntheticSection(SHF_ALLOC | SHF_WRITE,
                        config->emachine == EM_PPC64 ? SHT_NOBITS : SHT_PROGBITS,
-                       config->wordsize, getIgotPltName()) {}
+                       target->gotEntrySize, getIgotPltName()) {}
 
 void IgotPltSection::addEntry(Symbol &sym) {
   assert(sym.pltIndex == entries.size());
   entries.push_back(&sym);
 }
 
 size_t IgotPltSection::getSize() const {
-  return entries.size() * config->wordsize;
+  return entries.size() * target->gotEntrySize;
 }
 
 void IgotPltSection::writeTo(uint8_t *buf) {
   for (const Symbol *b : entries) {
     target->writeIgotPlt(buf, *b);
-    buf += config->wordsize;
+    buf += target->gotEntrySize;
   }
 }
 
 StringTableSection::StringTableSection(StringRef name, bool dynamic)
     : SyntheticSection(dynamic ? (uint64_t)SHF_ALLOC : 0, SHT_STRTAB, 1, name),
       dynamic(dynamic) {
   // ELF string tables start with a NUL byte.
   addString("");

lld/ELF/Target.h

public:
   RelType pltRel;
   RelType relativeRel;
   RelType iRelativeRel;
   RelType symbolicRel;
   RelType tlsDescRel;
   RelType tlsGotRel;
   RelType tlsModuleIndexRel;
   RelType tlsOffsetRel;
+  unsigned gotEntrySize = config->wordsize;
   unsigned pltEntrySize;
   unsigned pltHeaderSize;
   unsigned ipltEntrySize;
 
   // At least on x86_64 positions 1 and 2 are used by the first plt entry
   // to support lazy loading.
   unsigned gotPltHeaderEntriesNum = 3;
 

lld/test/ELF/x86-x32-plt.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64--gnux32 %s -o %t.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64--gnux32 %p/Inputs/shared.s -o %t2.o
+# RUN: ld.lld -shared -soname=t2 %t2.o -o %t2.so
+
+# RUN: ld.lld %t.o %t2.so -o %t
+# RUN: ld.lld -shared %t.o %t2.so -o %t.so
+# RUN: llvm-readelf -S -r %t | FileCheck %s --check-prefix=CHECK1
+# RUN: llvm-objdump -d --no-show-raw-insn %t | FileCheck %s --check-prefixes=DISASM,DISASM1
+# RUN: llvm-readelf -S -r %t.so | FileCheck %s --check-prefix=CHECK2
+# RUN: llvm-objdump -d --no-show-raw-insn %t.so | FileCheck %s --check-prefixes=DISASM,DISASM2
+
+# CHECK1:      Name      Type     Address          Off    Size   ES Flg Lk Inf Al
+# CHECK1:      .plt      PROGBITS 002011e0 0001e0 000030 00 AX   0   0 16
+# CHECK1:      .got.plt  PROGBITS 00203278 000278 000028 00 WA   0   0  8
+# CHECK1:      Relocation section '.rela.plt' at offset {{.*}} contains 2 entries:
+# CHECK1:      00203290 {{.*}} R_X86_64_JUMP_SLOT 00000000 bar + 0
+# CHECK1-NEXT: 00203298 {{.*}} R_X86_64_JUMP_SLOT 00000000 weak + 0
+
+# CHECK2:      Name      Type     Address          Off    Size   ES Flg Lk Inf Al
+# CHECK2:      .plt      PROGBITS 00001200 000200 000030 00 AX   0   0 16
+# CHECK2:      .got.plt  PROGBITS 00003290 000290 000028 00 WA   0   0  8
+# CHECK2:      Relocation section '.rela.plt' at offset {{.*}} contains 2 entries:
+# CHECK2:      000032a8 {{.*}} R_X86_64_JUMP_SLOT 00000000 bar + 0
+# CHECK2-NEXT: 000032b0 {{.*}} R_X86_64_JUMP_SLOT 00000000 weak + 0
+
+# DISASM:       <_start>:
+# DISASM-NEXT:    callq {{.*}} <local>
+# DISASM-NEXT:    callq {{.*}} <bar@plt>
+# DISASM-NEXT:    jmp   {{.*}} <bar@plt>
+# DISASM-NEXT:    jmp   {{.*}} <weak@plt>
+
+# DISASM1:      Disassembly of section .plt:
+# DISASM1-EMPTY:
+# DISASM1-NEXT: <.plt>:
+# DISASM1-NEXT: 2011e0:     pushq 8346(%rip)  # 203280
+# DISASM1-NEXT:             jmpq *8348(%rip)  # 203288
+# DISASM1-NEXT:             nopl (%rax)
+# DISASM1-EMPTY:
+# DISASM1-NEXT: <bar@plt>:
+# DISASM1-NEXT: 2011f0:     jmpq *8346(%rip)  # 203290
+# DISASM1-NEXT:             pushq $0
+# DISASM1-NEXT:             jmp 0x2011e0 <.plt>
+# DISASM1-EMPTY:
+# DISASM1-NEXT: <weak@plt>:
+# DISASM1-NEXT: 201200:     jmpq *8338(%rip)  # 203298
+# DISASM1-NEXT:             pushq $1
+# DISASM1-NEXT:             jmp 0x2011e0 <.plt>
+# DISASM1-NOT:  {{.}}
+
+# DISASM2:      Disassembly of section .plt:
+# DISASM2-EMPTY:
+# DISASM2-NEXT: <.plt>:
+# DISASM2-NEXT:   1200:     pushq 8338(%rip)  # 3298
+# DISASM2-NEXT:             jmpq *8340(%rip)  # 32a0
+# DISASM2-NEXT:             nopl (%rax)
+# DISASM2-EMPTY:
+# DISASM2-NEXT: <bar@plt>:
+# DISASM2-NEXT:   1210:     jmpq *8338(%rip)  # 32a8
+# DISASM2-NEXT:             pushq $0
+# DISASM2-NEXT:             jmp 0x1200 <.plt>
+# DISASM2-EMPTY:
+# DISASM2-NEXT: <weak@plt>:
+# DISASM2-NEXT:   1220:     jmpq *8330(%rip)  # 32b0
+# DISASM2-NEXT:             pushq $1
+# DISASM2-NEXT:             jmp 0x1200 <.plt>
+# DISASM2-NOT:  {{.}}
+
+.global _start
+.weak weak
+
+_start:
+  call local
+  call bar
+  jmp bar@plt
+  jmp weak
+
+## foo is local and non-preemptale, no PLT is generated.
+local:
+  ret