[ELF] - R_386_GOTOFF relocation implemented.

[grimar]

Summary

R_386_GOTOFF is calculated as S + A - GOT, where:
S - Represents the value of the symbol whose index resides in the relocation entry.
A - Represents the addend used to compute the value of the relocatable field.
GOT - Represents the address of the global offset table.

This relocation needs got address.
Problem here was that got table was not created if had no entries. Had to change how GOT gets to the output, test covers that case.

[ruiu]

ELF/OutputSections.h

140

Maybe we should have a separate global variable, HasGotoffRel, instead of this field.

ELF/Target.cpp

331–337

I don't think this is the right place to write this code. This function is doing more than its name implies.

[grimar]

Review comments addressed.

ELF/OutputSections.h

140

I think HasGotOffRel is too specific, at least it can be used for mips as shown in this patch:

Out<ELFT>::Got->Demanded |=
    (isOutputDynamic() && Config->EMachine == EM_MIPS);

I can make it separate static but what the point for it ? It only should be used to keep got section now, but when separate static it looks like it can be used for something else, what is not true.

ELF/Target.cpp

331–337

Yes, I didn`t like that place too, but then only suitable place for me would be adding Target->relocUsesGotAddr() and call it during iterations over relocations at upper level.
That adds one more method to Target thats only useful only for x86 now. Made that change.

[ruiu]

ELF/OutputSections.h

140

Well, I think that is rather too specific. You set Demanded field in Writer.cpp, but that boolean value is consumed immediately in the following line in an obscure way. (You explicitly wrote to the boolean variable but read from it through leaveInOutput() function.)

Can we write like this in Writer.cpp?

bool needsGot = (isOutputDynamic() && Config->EMachine == EM_MIPS);

// If we have a relocation that is relative to GOT (such as GOTOFFREL),
// we need to emit a GOT even if it's empty.
if (HasGotoffRel)
  needsGot = true;

if (needsGotMips || !Out<ELFT>::Got->empty()) {
  ....
}

ELF/Target.h

62

I'd name relocNeedsGot.

[grimar]

Review comments addressed.

ELF/OutputSections.h

140

Done.
(I actually don`t like idea of global variable at all. I would prefer static member of GotSection then. But if you insist..)

ELF/Target.h

62

I`d too, but unfortunately we already have one bool relocNeedsGot(...) above :)
Which is used to determine if relocation needs got entry to be allocated. So I dont think you want to move functionality from relocUsesGotAddr there.
May be relocAddressesGot() ?

[grimar]

• Rebased.

[ruiu]

LGTM with a few nits.

ELF/OutputSections.cpp

24–28 ↗

(On Diff #43111)

bool lld::elf2::HasGotOffRel = false;

ELF/OutputSections.h

39

got -> GOT

ELF/Target.cpp

342

I'd name this isGotRelative.

ELF/Writer.cpp

747–753

I think this flow is slightly better in readability (because special cases are now in ifs.)

bool needsGot = !Out<ELFT>::Got->empty();
// We add the .got section to the result for dynamic MIPS target because
// its address and properties are mentioned in the .dynamic section.
if (Config->EMachine == EM_MIPS)
  needsGot = needsGot || isOutputDynamic();
// If we have a relocation that is relative to GOT (such as GOTOFFREL),
// we need to emit a GOT even if it's empty.
if (HasGotOffRel)
  needsGot = true;

[grimar]

ELF/Writer.cpp

747–753

Ok.
The only distinction - I used |= :

if (Config->EMachine == EM_MIPS)
  needsGot |= isOutputDynamic();

[grimar]

lld/trunk/ELF/Writer.cpp

214 ↗

(On Diff #43349)

btw, what about making this more generic ?

1. Change HasGotOffRel to be not a global variable but a public member of target.
2. remove isGotRelative(Type), create visitRelocation(Type) or something alike instead. Which purpose would be to review relocation and set flags like HasGotOffRel inside.

ELF/OutputSections.h

 template <class ELFT> class InputSection;
 template <class ELFT> class InputSectionBase;
 template <class ELFT> class MergeInputSection;
 template <class ELFT> class OutputSection;
 template <class ELFT> class ObjectFile;
 template <class ELFT> class DefinedRegular;
 template <class ELFT> class ELFSymbolBody;
 
 template <class ELFT>
 static inline typename llvm::object::ELFFile<ELFT>::uintX_t
 getAddend(const typename llvm::object::ELFFile<ELFT>::Elf_Rel &Rel) {
   return 0;
 }
 
 template <class ELFT>
 static inline typename llvm::object::ELFFile<ELFT>::uintX_t
 getAddend(const typename llvm::object::ELFFile<ELFT>::Elf_Rela &Rel) {

 
 public:
   GotSection();
   void finalize() override;
   void writeTo(uint8_t *Buf) override;
   void addEntry(SymbolBody *Sym);
   bool addDynTlsEntry(SymbolBody *Sym);
   bool addCurrentModuleTlsIndex();
-  bool empty() const { return Entries.empty(); }
+  bool leaveInOutput() const { return Demanded || !Entries.empty(); }
   uintX_t getEntryAddr(const SymbolBody &B) const;
   uintX_t getGlobalDynAddr(const SymbolBody &B) const;
   uintX_t getNumEntries() const { return Entries.size(); }
 
   // Returns the symbol which corresponds to the first entry of the global part
   // of GOT on MIPS platform. It is required to fill up MIPS-specific dynamic
   // table properties.
   // Returns nullptr if the global part is empty.
   const SymbolBody *getMipsFirstGlobalEntry() const;
 
   // Returns the number of entries in the local part of GOT including
   // the number of reserved entries. This method is MIPS-specific.
   unsigned getMipsLocalEntriesNum() const;
 
   uint32_t getLocalTlsIndexVA() { return Base::getVA() + LocalTlsIndexOff; }
 
+  bool Demanded = false;
 private:
   std::vector<const SymbolBody *> Entries;
   uint32_t LocalTlsIndexOff = -1;
 };
 
 template <class ELFT>
 class GotPltSection final : public OutputSectionBase<ELFT> {
   typedef typename llvm::object::ELFFile<ELFT>::uintX_t uintX_t;

ELF/Target.h

virtual void writePltEntry(uint8_t *Buf, uint64_t GotAddr,
                              int32_t Index, unsigned RelOff) const = 0;
   virtual bool isRelRelative(uint32_t Type) const;
   virtual bool relocNeedsCopy(uint32_t Type, const SymbolBody &S) const;
   virtual bool relocNeedsGot(uint32_t Type, const SymbolBody &S) const = 0;
   virtual bool relocNeedsPlt(uint32_t Type, const SymbolBody &S) const = 0;
   virtual void relocateOne(uint8_t *Loc, uint8_t *BufEnd, uint32_t Type,
                            uint64_t P, uint64_t SA,
                            uint8_t *PairedLoc = nullptr) const = 0;
+  virtual bool relocUsesGotAddr(uint32_t Type) const;
   virtual bool isTlsOptimized(unsigned Type, const SymbolBody *S) const;
   virtual unsigned relocateTlsOptimize(uint8_t *Loc, uint8_t *BufEnd,
                                        uint32_t Type, uint64_t P, uint64_t SA,
                                        const SymbolBody &S) const;
   virtual ~TargetInfo();
 
 protected:
   unsigned PageSize = 4096;

ELF/Target.cpp

public:
   bool relocNeedsGot(uint32_t Type, const SymbolBody &S) const override;
   bool relocNeedsPlt(uint32_t Type, const SymbolBody &S) const override;
   void relocateOne(uint8_t *Loc, uint8_t *BufEnd, uint32_t Type, uint64_t P,
                    uint64_t SA, uint8_t *PairedLoc = nullptr) const override;
   bool isTlsOptimized(unsigned Type, const SymbolBody *S) const override;
   unsigned relocateTlsOptimize(uint8_t *Loc, uint8_t *BufEnd, uint32_t Type,
                                uint64_t P, uint64_t SA,
                                const SymbolBody &S) const override;
+  bool relocUsesGotAddr(uint32_t Type) const override;
 
 private:
   void relocateTlsLdToLe(uint8_t *Loc, uint8_t *BufEnd, uint64_t P,
                          uint64_t SA) const;
   void relocateTlsGdToIe(uint8_t *Loc, uint8_t *BufEnd, uint64_t P,
                          uint64_t SA) const;
   void relocateTlsGdToLe(uint8_t *Loc, uint8_t *BufEnd, uint64_t P,
                          uint64_t SA) const;

 }
 
 uint64_t TargetInfo::getVAStart() const { return Config->Shared ? 0 : VAStart; }
 
 bool TargetInfo::relocNeedsCopy(uint32_t Type, const SymbolBody &S) const {
   return false;
 }
 
+bool TargetInfo::relocUsesGotAddr(uint32_t Type) const { return false; }
+
 unsigned TargetInfo::getPltRefReloc(unsigned Type) const { return PCRelReloc; }
 
 bool TargetInfo::isRelRelative(uint32_t Type) const { return true; }
 
 unsigned TargetInfo::relocateTlsOptimize(uint8_t *Loc, uint8_t *BufEnd,
                                          uint32_t Type, uint64_t P, uint64_t SA,
                                          const SymbolBody &S) const {
   return 0;

 bool X86TargetInfo::relocNeedsCopy(uint32_t Type, const SymbolBody &S) const {
   if (Type == R_386_32 || Type == R_386_16 || Type == R_386_8)
     if (auto *SS = dyn_cast<SharedSymbol<ELF32LE>>(&S))
       return SS->Sym.getType() == STT_OBJECT;
   return false;
 }
 
 bool X86TargetInfo::relocNeedsGot(uint32_t Type, const SymbolBody &S) const {
   if (S.isTLS() && Type == R_386_TLS_GD)
     return Target->isTlsOptimized(Type, &S) && canBePreempted(&S, true);
   if (Type == R_386_TLS_GOTIE)
     return !isTlsOptimized(Type, &S);
   return Type == R_386_GOT32 || relocNeedsPlt(Type, S);
 }
 
 bool X86TargetInfo::relocNeedsPlt(uint32_t Type, const SymbolBody &S) const {
   return Type == R_386_PLT32 || (Type == R_386_PC32 && S.isShared());
 }
 
+bool X86TargetInfo::relocUsesGotAddr(uint32_t Type) const {
+  // This relocation does not require got entry,
+  // but it is relative to got and needs it to be created.
+  // Here we request for that.
+  return Type == R_386_GOTOFF;
+}
+
 void X86TargetInfo::relocateOne(uint8_t *Loc, uint8_t *BufEnd, uint32_t Type,
                                 uint64_t P, uint64_t SA,
                                 uint8_t *PairedLoc) const {
   switch (Type) {
   case R_386_32:
     add32le(Loc, SA);
     break;
   case R_386_GOT32:
+  case R_386_GOTOFF:
     add32le(Loc, SA - Out<ELF32LE>::Got->getVA());
     break;
   case R_386_GOTPC:
     add32le(Loc, SA + Out<ELF32LE>::Got->getVA() - P);
     break;
   case R_386_PC32:
     add32le(Loc, SA - P);
     break;

ELF/Writer.cpp

void Writer<ELFT>::scanRelocs(
     iterator_range<const Elf_Rel_Impl<ELFT, isRela> *> Rels) {
   typedef Elf_Rel_Impl<ELFT, isRela> RelType;
   const ObjectFile<ELFT> &File = *C.getFile();
   for (const RelType &RI : Rels) {
     uint32_t SymIndex = RI.getSymbol(Config->Mips64EL);
     SymbolBody *Body = File.getSymbolBody(SymIndex);
     uint32_t Type = RI.getType(Config->Mips64EL);
 
+    if (Target->relocUsesGotAddr(Type))
+      Out<ELF32LE>::Got->Demanded = true;
+
     if (Target->isTlsLocalDynamicReloc(Type)) {
       if (Target->isTlsOptimized(Type, nullptr))
         continue;
       if (Out<ELFT>::Got->addCurrentModuleTlsIndex())
         Out<ELFT>::RelaDyn->addReloc({&C, &RI});
       continue;
     }
 
if (Config->EMachine == EM_MIPS && !Config->Shared) {
       Out<ELFT>::MipsRldMap = new (SecAlloc.Allocate())
           OutputSection<ELFT>(".rld_map", SHT_PROGBITS, SHF_ALLOC | SHF_WRITE);
       Out<ELFT>::MipsRldMap->setSize(ELFT::Is64Bits ? 8 : 4);
       Out<ELFT>::MipsRldMap->updateAlign(ELFT::Is64Bits ? 8 : 4);
       OutputSections.push_back(Out<ELFT>::MipsRldMap);
     }
   }
 
   // We add the .got section to the result for dynamic MIPS target because
   // its address and properties are mentioned in the .dynamic section.
-  if (!Out<ELFT>::Got->empty() ||
-      (isOutputDynamic() && Config->EMachine == EM_MIPS))
+  Out<ELFT>::Got->Demanded |=
+      (isOutputDynamic() && Config->EMachine == EM_MIPS);
+
+  if (Out<ELFT>::Got->leaveInOutput())
     OutputSections.push_back(Out<ELFT>::Got);
   if (Out<ELFT>::GotPlt && !Out<ELFT>::GotPlt->empty())
     OutputSections.push_back(Out<ELFT>::GotPlt);
   if (!Out<ELFT>::Plt->empty())
     OutputSections.push_back(Out<ELFT>::Plt);
 
   std::stable_sort(OutputSections.begin(), OutputSections.end(),
                    compareSections<ELFT>);
 

test/ELF/got-i386.s

+// RUN: llvm-mc -filetype=obj -triple=i686-unknown-linux %s -o %t.o
+// RUN: ld.lld %t.o -o %t
+// RUN: llvm-readobj -s -r -t %t | FileCheck %s
+// RUN: llvm-objdump -d %t | FileCheck --check-prefix=DISASM %s
+// REQUIRES: x86
+
+// CHECK:      Name: .got
+// CHECK-NEXT: Type: SHT_PROGBITS
+// CHECK-NEXT: Flags [
+// CHECK-NEXT:   SHF_ALLOC
+// CHECK-NEXT:   SHF_WRITE
+// CHECK-NEXT: ]
+// CHECK-NEXT: Address: 0x12000
+// CHECK-NEXT: Offset:
+// CHECK-NEXT: Size: 0
+// CHECK-NEXT: Link:
+// CHECK-NEXT: Info:
+// CHECK-NEXT: AddressAlignment:
+
+// CHECK:      Symbol {
+// CHECK:       Name: bar
+// CHECK-NEXT:  Value: 0x12000
+// CHECK-NEXT:  Size: 10
+// CHECK-NEXT:  Binding: Global
+// CHECK-NEXT:  Type: Object
+// CHECK-NEXT:  Other: 0
+// CHECK-NEXT:  Section: .bss
+// CHECK-NEXT: }
+// CHECK-NEXT: Symbol {
+// CHECK-NEXT:  Name: obj
+// CHECK-NEXT:  Value: 0x1200A
+// CHECK-NEXT:  Size: 10
+// CHECK-NEXT:  Binding: Global
+// CHECK-NEXT:  Type: Object
+// CHECK-NEXT:  Other: 0
+// CHECK-NEXT:  Section: .bss
+// CHECK-NEXT: }
+
+// 0x12000 - 0 = addr(.got) = 0x12000
+// 0x1200A - 10 = addr(.got) = 0x12000
+// 0x1200A + 5 - 15 = addr(.got) = 0x12000
+// DISASM:      Disassembly of section .text:
+// DISASM-NEXT: _start:
+// DISASM-NEXT: 11000: c7 81 00 00 00 00 01 00 00 00 movl $1, (%ecx)
+// DISASM-NEXT: 1100a: c7 81 0a 00 00 00 02 00 00 00 movl $2, 10(%ecx)
+// DISASM-NEXT: 11014: c7 81 0f 00 00 00 03 00 00 00 movl $3, 15(%ecx)
+
+.global _start
+_start:
+  movl $1, bar@GOTOFF(%ecx)
+  movl $2, obj@GOTOFF(%ecx)
+  movl $3, obj+5@GOTOFF(%ecx)
+  .type bar, @object
+  .comm bar, 10
+  .type obj, @object
+  .comm obj, 10