[ELF] Keep empty In.RelaIplt so that __rela_iplt_{start,end} have valid st_shndx

[MaskRay]

Summary

This fixes PR36634 (and https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=233397)

In a statically linked executable, if __rela_iplt_{start,end} are
defined with non-existent .rela.plt (In.RelaIplt), the symbols' st_shndx
are 65535 (SHN_XINDEX) which may confuse readelf, objdump, llvm-objdump,
llvm-readelf and some other binutils because the extended symbol table
may not exist.

ld.bfd and gold do not emit empty .rela.plt but seem to use arbitrary
st_shndx for __rela_iplt_{start,end}. We can just emit the empty
.rela.plt .

[emaste]

This is fine for me (from FreeBSD's perspective), but I suspect some people (on operating systems without ifuncs in libc) may be surprised by having a(n empty) .rela.plt in all of their statically linked binaries.

[MaskRay]

In D54985#1310779, @emaste wrote:

This is fine for me (from FreeBSD's perspective), but I suspect some people (on operating systems without ifuncs in libc) may be surprised by having a(n empty) .rela.plt in all of their statically linked binaries.

Also read my comment https://bugs.llvm.org/show_bug.cgi?id=36634#c3 This makes most sense to me but it requires lots of changes and I haven't investigated how valid it is to have such empty ALLOC sections (and in the segment end but it probably does not matter)

[ruiu]

It feels this patch modifies too many places to fix a cosmetic issue. Doesn't something like this work? https://gist.github.com/rui314/6382995c81689668961caf49a0e695f9

[MaskRay]

In D54985#1311535, @ruiu wrote:

It feels this patch modifies too many places to fix a cosmetic issue. Doesn't something like this work? https://gist.github.com/rui314/6382995c81689668961caf49a0e695f9

This works, but it may better use a SHF_ALLOC section (not sure if that is what ld.bfd uses)

[ruiu]

This works, but it may better use a SHF_ALLOC section (not sure if that is what ld.bfd uses)

Ideally, yes, but if no one really cares, just using 1 is simpler and sufficient.

[MaskRay]

This has been fixed by https://reviews.llvm.org/D56623

ELF/SyntheticSections.h

public:
   void addReloc(RelType DynType, InputSectionBase *IS, uint64_t OffsetInSec,
                 Symbol *Sym);
   // Add a dynamic relocation that might need an addend. This takes care of
   // writing the addend to the output section if needed.
   void addReloc(RelType DynType, InputSectionBase *InputSec,
                 uint64_t OffsetInSec, Symbol *Sym, int64_t Addend, RelExpr Expr,
                 RelType Type);
   void addReloc(const DynamicReloc &Reloc);
-  bool empty() const override { return Relocs.empty(); }
+  bool empty() const override { return Relocs.empty() && !IsIplt; }
   size_t getSize() const override { return Relocs.size() * this->Entsize; }
   size_t getRelativeRelocCount() const { return NumRelativeRelocs; }
   void finalizeContents() override;
+
   int32_t DynamicTag, SizeDynamicTag;
 
+  // Used to keep empty .rela.iplt as otherwise __rela_iplt_start would have an
+  // invalid st_shndx.
+  bool IsIplt = false;
+
 protected:
   std::vector<DynamicReloc> Relocs;
   size_t NumRelativeRelocs = 0;
 };
 
 template <class ELFT>
 class RelocationSection final : public RelocationBaseSection {
   typedef typename ELFT::Rel Elf_Rel;

ELF/SyntheticSections.cpp

 
 void RelocationBaseSection::finalizeContents() {
   // When linking glibc statically, .rel{,a}.plt contains R_*_IRELATIVE
   // relocations due to IFUNC (e.g. strcpy). sh_link will be set to 0 in that
   // case.
   InputSection *SymTab = Config->Relocatable ? In.SymTab : In.DynSymTab;
   getParent()->Link = SymTab ? SymTab->getParent()->SectionIndex : 0;
 
-  if (In.RelaIplt == this || In.RelaPlt == this)
-    getParent()->Info = In.GotPlt->getParent()->SectionIndex;
+  if (In.RelaIplt == this || In.RelaPlt == this) {
+    uint32_t I = In.GotPlt->getParent()->SectionIndex;
+    if (I != UINT32_MAX)
+      getParent()->Info = I;
+  }
 }
 
 RelrBaseSection::RelrBaseSection()
     : SyntheticSection(SHF_ALLOC,
                        Config->UseAndroidRelrTags ? SHT_ANDROID_RELR : SHT_RELR,
                        Config->Wordsize, ".relr.dyn") {}
 
 template <class ELFT>

ELF/Writer.cpp

template <class ELFT> static void createSyntheticSections() {
   // packing is enabled because that would cause a section type mismatch.
   // However, because the Android dynamic loader reads .rel.plt after .rel.dyn,
   // we can get the desired behaviour by placing the iplt section in .rel.plt.
   In.RelaIplt = make<RelocationSection<ELFT>>(
       (Config->EMachine == EM_ARM && !Config->AndroidPackDynRelocs)
           ? ".rel.dyn"
           : In.RelaPlt->Name,
       false /*Sort*/);
+  In.RelaIplt->IsIplt = true;
   Add(In.RelaIplt);
 
   In.Plt = make<PltSection>(false);
   Add(In.Plt);
   In.Iplt = make<PltSection>(true);
   Add(In.Iplt);
 
   // .note.GNU-stack is always added when we are creating a re-linkable