ELF: Move code to where it is used, and related cleanups. NFC.

[ruiu]

ELF/MarkLive.cpp

42 ↗

(On Diff #54885)

So this code movement is just to move code to the file where it is used (plus a small cleanup), right?

165–168 ↗

(On Diff #54885)

Is this order safe? If getSymbols happens to return non-shared symbols and shared symbols in this order, then you would clear IsUsedInRegularObj bit for all shared symbols, wouldn't you?

[pcc]

ELF/MarkLive.cpp

42 ↗

(On Diff #54885)

Yes.

165–168 ↗

(On Diff #54885)

We only ever process a shared symbol when we remove an section from the work list and call forEachSuccessor. MarkSymbol only calls Enqueue with a section+offset, never a symbol.

[• rafael]

Taking a look.

[• rafael]

test/ELF/gc-sections-shared.s

9 ↗

(On Diff #54885)

This matches the gold and bfd behaviour?

I would actually expect the .so to keep bar2 alive. After all, if a symbol in a .so can force us to export a symbol from an executable, why shouldn't it for us to keep it alive?

That is similar to us not gcing section that contribute to the dynamic table of a .so.

[pcc]

This matches the gold and bfd behaviour?

bfd exports foo and bar. gold exports foo, bar and bar2.

I would actually expect the .so to keep bar2 alive. After all, if a symbol in a .so can force us to export a symbol from an executable, why shouldn't it for us to keep it alive?

That is what I am testing with the definition of bar. In this case, we are testing that we are removing bar2 from dynsym because it ends up being an unused reference.

[pcc]

ELF: Move code to where it is used, and related cleanups. NFC.

[• rafael]

ELF/MarkLive.cpp

28 ↗

(On Diff #55029)

Can you remove the include from InputSection.cpp?

ELF/OutputSections.cpp

1155 ↗

(On Diff #55029)

This code is pretty much exactly what getRelocTargetSym was. Maybe keep just that as an utility function and move the rest to MarkLive?

[pcc]

• Add utility function to replace boilerplate get-target-of-relocation code

ELF/MarkLive.cpp

28 ↗

(On Diff #55029)

We still need it in (e.g.) InputSection<ELFT>::getThunksSize().

ELF/OutputSections.cpp

1155 ↗

(On Diff #55029)

I found that the two lines that retrieved the replacement SymbolBody were duplicated in several places, so I pulled that out into a utility function and updated the users. The rest of the code is only duplicated here and in MarkLive, so it's probably simplest to keep the duplication.

[• rafael]

lgtm

ELF/ICF.cpp

250 ↗

(On Diff #55111)

Do you really need the cast?

ELF/InputFiles.h

132 ↗

(On Diff #55111)

I moved the repl to geSymbolBody. Please remember to drop it here when you merge.

[pcc]

ELF/ICF.cpp

250 ↗

(On Diff #55111)

No, removed.

lld/trunk/ELF/ICF.cpp

 private:
   uint64_t NextId = 1;
 
   static void setLive(SymbolTable<ELFT> *S);
   static uint64_t relSize(InputSection<ELFT> *S);
   static uint64_t getHash(InputSection<ELFT> *S);
   static bool isEligible(InputSectionBase<ELFT> *Sec);
   static std::vector<InputSection<ELFT> *> getSections(SymbolTable<ELFT> *S);
-  static SymbolBody &getSymbol(const InputSection<ELFT> *Sec,
-                               const Elf_Rel *Rel);
 
   void segregate(InputSection<ELFT> **Begin, InputSection<ELFT> **End,
                  Comparator Eq);
 
   void forEachGroup(std::vector<InputSection<ELFT> *> &V, Comparator Eq);
 
   template <class RelTy>
   static bool relocationEq(ArrayRef<RelTy> RA, ArrayRef<RelTy> RB);
ICF<ELFT>::getSections(SymbolTable<ELFT> *Symtab) {
   std::vector<InputSection<ELFT> *> V;
   for (const std::unique_ptr<ObjectFile<ELFT>> &F : Symtab->getObjectFiles())
     for (InputSectionBase<ELFT> *S : F->getSections())
       if (isEligible(S))
         V.push_back(cast<InputSection<ELFT>>(S));
   return V;
 }
 
-template <class ELFT>
-SymbolBody &ICF<ELFT>::getSymbol(const InputSection<ELFT> *Sec,
-                                 const Elf_Rel *Rel) {
-  uint32_t SymIdx = Rel->getSymbol(Config->Mips64EL);
-  return Sec->File->getSymbolBody(SymIdx);
-}
-
 // All sections between Begin and End must have the same group ID before
 // you call this function. This function compare sections between Begin
 // and End using Eq and assign new group IDs for new groups.
 template <class ELFT>
 void ICF<ELFT>::segregate(InputSection<ELFT> **Begin, InputSection<ELFT> **End,
                           Comparator Eq) {
   // This loop rearranges [Begin, End) so that all sections that are
   // equal in terms of Eq are contiguous. The algorithm is quadratic in

 template <class RelTy>
 bool ICF<ELFT>::variableEq(const InputSection<ELFT> *A,
                            const InputSection<ELFT> *B, ArrayRef<RelTy> RelsA,
                            ArrayRef<RelTy> RelsB) {
   const RelTy *IA = RelsA.begin();
   const RelTy *EA = RelsA.end();
   const RelTy *IB = RelsB.begin();
   for (; IA != EA; ++IA, ++IB) {
-    SymbolBody &SA = getSymbol(A, (const Elf_Rel *)IA);
-    SymbolBody &SB = getSymbol(B, (const Elf_Rel *)IB);
+    SymbolBody &SA = A->File->getRelocTargetSym(*IA);
+    SymbolBody &SB = B->File->getRelocTargetSym(*IB);
     if (&SA == &SB)
       continue;
 
     // Or, the symbols should be pointing to the same section
     // in terms of the group ID.
     auto *DA = dyn_cast<DefinedRegular<ELFT>>(&SA);
     auto *DB = dyn_cast<DefinedRegular<ELFT>>(&SB);
     if (!DA || !DB)

lld/trunk/ELF/InputFiles.h

public:
 
   ArrayRef<InputSectionBase<ELFT> *> getSections() const { return Sections; }
   InputSectionBase<ELFT> *getSection(const Elf_Sym &Sym) const;
 
   SymbolBody &getSymbolBody(uint32_t SymbolIndex) const {
     return SymbolBodies[SymbolIndex]->repl();
   }
 
+  template <typename RelT> SymbolBody &getRelocTargetSym(const RelT &Rel) const {
+    uint32_t SymIndex = Rel.getSymbol(Config->Mips64EL);
+    return getSymbolBody(SymIndex);
+  }
+
   const Elf_Shdr *getSymbolTable() const { return this->Symtab; };
 
   // Get MIPS GP0 value defined by this file. This value represents the gp value
   // used to create the relocatable object and required to support
   // R_MIPS_GPREL16 / R_MIPS_GPREL32 relocations.
   uint32_t getMipsGp0() const;
 
   // The number is the offset in the string table. It will be used as the

lld/trunk/ELF/InputSection.h

public:
   uintX_t getOffset(const DefinedRegular<ELFT> &Sym);
 
   // Translate an offset in the input section to an offset in the output
   // section.
   uintX_t getOffset(uintX_t Offset);
 
   ArrayRef<uint8_t> getSectionData() const;
 
-  // Returns a section that Rel is pointing to. Used by the garbage collector.
-  std::pair<InputSectionBase<ELFT> *, uintX_t>
-  getRelocTarget(const Elf_Rel &Rel) const;
-  std::pair<InputSectionBase<ELFT> *, uintX_t>
-  getRelocTarget(const Elf_Rela &Rel) const;
-
   void relocate(uint8_t *Buf, uint8_t *BufEnd);
   std::vector<Relocation> Relocations;
 };
 
 template <class ELFT> InputSectionBase<ELFT> InputSectionBase<ELFT>::Discarded;
 
 // Usually sections are copied to the output as atomic chunks of data,
 // but some special types of sections are split into small pieces of data

lld/trunk/ELF/InputSection.cpp

 
 template <class ELFT>
 typename ELFT::uint
 InputSectionBase<ELFT>::getOffset(const DefinedRegular<ELFT> &Sym) {
   return getOffset(Sym.Value);
 }
 
 template <class ELFT>
-static DefinedRegular<ELFT> *getRelocTargetSym(elf::ObjectFile<ELFT> *File,
-                                               const typename ELFT::Rel &Rel) {
-  uint32_t SymIndex = Rel.getSymbol(Config->Mips64EL);
-  SymbolBody &B = File->getSymbolBody(SymIndex);
-  if (auto *D = dyn_cast<DefinedRegular<ELFT>>(&B))
-    if (D->Section)
-      return D;
-  return nullptr;
-}
-
-// Returns a section that Rel relocation is pointing to.
-template <class ELFT>
-std::pair<InputSectionBase<ELFT> *, typename ELFT::uint>
-InputSectionBase<ELFT>::getRelocTarget(const Elf_Rel &Rel) const {
-  auto *D = getRelocTargetSym(File, Rel);
-  if (!D)
-    return std::make_pair(nullptr, 0);
-  if (!D->isSection())
-    return std::make_pair(D->Section->Repl, D->Value);
-  const uint8_t *BufLoc = getSectionData().begin() + Rel.r_offset;
-  uintX_t Addend =
-      Target->getImplicitAddend(BufLoc, Rel.getType(Config->Mips64EL));
-  return std::make_pair(D->Section->Repl, D->Value + Addend);
-}
-
-template <class ELFT>
-std::pair<InputSectionBase<ELFT> *, typename ELFT::uint>
-InputSectionBase<ELFT>::getRelocTarget(const Elf_Rela &Rel) const {
-  auto *D = getRelocTargetSym(File, Rel);
-  if (!D)
-    return std::make_pair(nullptr, 0);
-  if (!D->isSection())
-    return std::make_pair(D->Section->Repl, D->Value);
-  return std::make_pair(D->Section->Repl, D->Value + Rel.r_addend);
-}
-
-template <class ELFT>
 InputSection<ELFT>::InputSection(elf::ObjectFile<ELFT> *F,
                                  const Elf_Shdr *Header)
     : InputSectionBase<ELFT>(F, Header, Base::Regular) {}
 
 template <class ELFT>
 bool InputSection<ELFT>::classof(const InputSectionBase<ELFT> *S) {
   return S->SectionKind == Base::Regular;
 }

 // to update symbol table offset and section index for each relocation. So we
 // copy relocations one by one.
 template <class ELFT>
 template <class RelTy>
 void InputSection<ELFT>::copyRelocations(uint8_t *Buf, ArrayRef<RelTy> Rels) {
   InputSectionBase<ELFT> *RelocatedSection = getRelocatedSection();
 
   for (const RelTy &Rel : Rels) {
-    uint32_t SymIndex = Rel.getSymbol(Config->Mips64EL);
     uint32_t Type = Rel.getType(Config->Mips64EL);
-    SymbolBody &Body = this->File->getSymbolBody(SymIndex);
+    SymbolBody &Body = this->File->getRelocTargetSym(Rel);
 
     RelTy *P = reinterpret_cast<RelTy *>(Buf);
     Buf += sizeof(RelTy);
 
     P->r_offset = RelocatedSection->getOffset(Rel.r_offset);
     P->setSymbolAndType(Body.DynsymIndex, Type, Config->Mips64EL);
   }
 }

lld/trunk/ELF/MarkLive.cpp

 //
 //===----------------------------------------------------------------------===//
 
 #include "InputSection.h"
 #include "LinkerScript.h"
 #include "OutputSections.h"
 #include "SymbolTable.h"
 #include "Symbols.h"
+#include "Target.h"
 #include "Writer.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Object/ELF.h"
 #include <functional>
 #include <vector>
 
 using namespace llvm;
 using namespace llvm::ELF;
 using namespace llvm::object;
 
 using namespace lld;
 using namespace lld::elf;
 
+// A resolved relocation. The Sec and Offset fields are set if the relocation
+// was resolved to an offset within a section.
+template <class ELFT>
+struct ResolvedReloc {
+  InputSectionBase<ELFT> *Sec;
+  typename ELFT::uint Offset;
+};
+
+template <class ELFT>
+static typename ELFT::uint getAddend(InputSectionBase<ELFT> *Sec,
+                                     const typename ELFT::Rel &Rel) {
+  return Target->getImplicitAddend(Sec->getSectionData().begin(),
+                                   Rel.getType(Config->Mips64EL));
+}
+
+template <class ELFT>
+static typename ELFT::uint getAddend(InputSectionBase<ELFT> *Sec,
+                                     const typename ELFT::Rela &Rel) {
+  return Rel.r_addend;
+}
+
+template <class ELFT, class RelT>
+static ResolvedReloc<ELFT> resolveReloc(InputSection<ELFT> *Sec, RelT &Rel) {
+  SymbolBody &B = Sec->getFile()->getRelocTargetSym(Rel);
+  auto *D = dyn_cast<DefinedRegular<ELFT>>(&B);
+  if (!D || !D->Section)
+    return {nullptr, 0};
+  typename ELFT::uint Offset = D->Value;
+  if (D->isSection())
+    Offset += getAddend(Sec, Rel);
+  return {D->Section->Repl, Offset};
+}
+
 // Calls Fn for each section that Sec refers to via relocations.
 template <class ELFT>
-static void forEachSuccessor(
-    InputSection<ELFT> *Sec,
-    std::function<void(InputSectionBase<ELFT> *, typename ELFT::uint Offset)>
-        Fn) {
+static void forEachSuccessor(InputSection<ELFT> *Sec,
+                             std::function<void(ResolvedReloc<ELFT>)> Fn) {
   typedef typename ELFT::Rel Elf_Rel;
   typedef typename ELFT::Rela Elf_Rela;
   typedef typename ELFT::Shdr Elf_Shdr;
-  typedef typename ELFT::uint uintX_t;
 
   ELFFile<ELFT> &Obj = Sec->getFile()->getObj();
   for (const Elf_Shdr *RelSec : Sec->RelocSections) {
     if (RelSec->sh_type == SHT_RELA) {
-      for (const Elf_Rela &RI : Obj.relas(RelSec)) {
-        std::pair<InputSectionBase<ELFT> *, uintX_t> P =
-            Sec->getRelocTarget(RI);
-        Fn(P.first, P.second);
-      }
+      for (const Elf_Rela &RI : Obj.relas(RelSec))
+        Fn(resolveReloc(Sec, RI));
     } else {
-      for (const Elf_Rel &RI : Obj.rels(RelSec)) {
-        std::pair<InputSectionBase<ELFT> *, uintX_t> P =
-            Sec->getRelocTarget(RI);
-        Fn(P.first, P.second);
-      }
+      for (const Elf_Rel &RI : Obj.rels(RelSec))
+        Fn(resolveReloc(Sec, RI));
     }
   }
 }
 
 // Sections listed below are special because they are used by the loader
 // just by being in an ELF file. They should not be garbage-collected.
 template <class ELFT> static bool isReserved(InputSectionBase<ELFT> *Sec) {
   switch (Sec->getSectionHdr()->sh_type) {

 
 // This is the main function of the garbage collector.
 // Starting from GC-root sections, this function visits all reachable
 // sections to set their "Live" bits.
 template <class ELFT> void elf::markLive(SymbolTable<ELFT> *Symtab) {
   typedef typename ELFT::uint uintX_t;
   SmallVector<InputSection<ELFT> *, 256> Q;
 
-  auto Enqueue = [&](InputSectionBase<ELFT> *Sec, uintX_t Offset) {
-    if (!Sec)
+  auto Enqueue = [&](ResolvedReloc<ELFT> R) {
+    if (!R.Sec)
       return;
-    if (auto *MS = dyn_cast<MergeInputSection<ELFT>>(Sec)) {
+    if (auto *MS = dyn_cast<MergeInputSection<ELFT>>(R.Sec)) {
       std::pair<std::pair<uintX_t, uintX_t> *, uintX_t> T =
-          MS->getRangeAndSize(Offset);
+          MS->getRangeAndSize(R.Offset);
       T.first->second = 0;
     }
-    if (Sec->Live)
+    if (R.Sec->Live)
       return;
-    Sec->Live = true;
-    if (InputSection<ELFT> *S = dyn_cast<InputSection<ELFT>>(Sec))
+    R.Sec->Live = true;
+    if (InputSection<ELFT> *S = dyn_cast<InputSection<ELFT>>(R.Sec))
       Q.push_back(S);
   };
 
   auto MarkSymbol = [&](SymbolBody *Sym) {
     if (Sym)
       if (auto *D = dyn_cast<DefinedRegular<ELFT>>(Sym))
-        Enqueue(D->Section, D->Value);
+        Enqueue({D->Section, D->Value});
   };
 
   // Add GC root symbols.
   if (Config->EntrySym)
     MarkSymbol(Config->EntrySym->Body);
   MarkSymbol(Symtab->find(Config->Init));
   MarkSymbol(Symtab->find(Config->Fini));
   for (StringRef S : Config->Undefined)
     MarkSymbol(Symtab->find(S));
 
   // Preserve externally-visible symbols if the symbols defined by this
   // file can interrupt other ELF file's symbols at runtime.
   for (const Symbol *S : Symtab->getSymbols())
     if (S->includeInDynsym())
       MarkSymbol(S->Body);
 
   // Preserve special sections and those which are specified in linker
   // script KEEP command.
   for (const std::unique_ptr<ObjectFile<ELFT>> &F : Symtab->getObjectFiles())
     for (InputSectionBase<ELFT> *Sec : F->getSections())
       if (Sec && Sec != &InputSection<ELFT>::Discarded)
         if (isReserved(Sec) || Script<ELFT>::X->shouldKeep(Sec))
-          Enqueue(Sec, 0);
+          Enqueue({Sec, 0});
 
   // Mark all reachable sections.
   while (!Q.empty())
     forEachSuccessor<ELFT>(Q.pop_back_val(), Enqueue);
 }
 
 template void elf::markLive<ELF32LE>(SymbolTable<ELF32LE> *);
 template void elf::markLive<ELF32BE>(SymbolTable<ELF32BE> *);
 template void elf::markLive<ELF64LE>(SymbolTable<ELF64LE> *);
 template void elf::markLive<ELF64BE>(SymbolTable<ELF64BE> *);

lld/trunk/ELF/OutputSections.cpp

while (!D.empty()) {
     uint32_t ID = read32<E>(D.data() + 4);
     if (ID == 0) {
       // CIE
       Cie<ELFT> C(S, Index);
       if (Config->EhFrameHdr)
         C.FdeEncoding = getFdeEncoding(D);
 
       SymbolBody *Personality = nullptr;
-      if (HasReloc) {
-        uint32_t SymIndex = RelI->getSymbol(Config->Mips64EL);
-        Personality = &S->getFile()->getSymbolBody(SymIndex);
-      }
+      if (HasReloc)
+        Personality = &S->getFile()->getRelocTargetSym(*RelI);
 
       std::pair<StringRef, SymbolBody *> CieInfo(Entry, Personality);
       auto P = CieMap.insert(std::make_pair(CieInfo, Cies.size()));
       if (P.second) {
         Cies.push_back(C);
         this->Header.sh_size += alignTo(Length, sizeof(uintX_t));
       }
       OffsetToIndex[Offset] = P.first->second;
     } else {
       if (!HasReloc)
         fatal("FDE doesn't reference another section");
-      InputSectionBase<ELFT> *Target = S->getRelocTarget(*RelI).first;
+      SymbolBody &B = S->getFile()->getRelocTargetSym(*RelI);
+      auto *D = dyn_cast<DefinedRegular<ELFT>>(&B);
+      if (D && D->Section) {
+        InputSectionBase<ELFT> *Target = D->Section->Repl;
         if (Target && Target->Live) {
           uint32_t CieOffset = Offset + 4 - ID;
           auto I = OffsetToIndex.find(CieOffset);
           if (I == OffsetToIndex.end())
             fatal("invalid CIE reference");
           Cies[I->second].Fdes.push_back(EHRegion<ELFT>(S, Index));
           Out<ELFT>::EhFrameHdr->reserveFde();
           this->Header.sh_size += alignTo(Length, sizeof(uintX_t));
         }
       }
+    }
 
     Offset = NextOffset;
     D = D.slice(Length);
   }
 }
 
 template <class ELFT>
 void EHOutputSection<ELFT>::addSection(InputSectionBase<ELFT> *C) {

lld/trunk/ELF/Writer.cpp

 // support only MIPS which requires LA25 thunk to call PIC code from non-PIC
 // one. Scan relocations to find each one requires thunk.
 template <class ELFT>
 template <class RelTy>
 void Writer<ELFT>::scanRelocsForThunks(const elf::ObjectFile<ELFT> &File,
                                        ArrayRef<RelTy> Rels) {
   for (const RelTy &RI : Rels) {
     uint32_t Type = RI.getType(Config->Mips64EL);
-    uint32_t SymIndex = RI.getSymbol(Config->Mips64EL);
-    SymbolBody &Body = File.getSymbolBody(SymIndex);
+    SymbolBody &Body = File.getRelocTargetSym(RI);
     if (Body.hasThunk() || !Target->needsThunk(Type, File, Body))
       continue;
     auto *D = cast<DefinedRegular<ELFT>>(&Body);
     auto *S = cast<InputSection<ELFT>>(D->Section);
     S->addThunk(Body);
   }
 }
 
if (IsAlloc)
       Out<ELFT>::RelaDyn->addReloc(Reloc);
   };
 
   const elf::ObjectFile<ELFT> &File = *C.getFile();
   ArrayRef<uint8_t> SectionData = C.getSectionData();
   const uint8_t *Buf = SectionData.begin();
   for (auto I = Rels.begin(), E = Rels.end(); I != E; ++I) {
     const RelTy &RI = *I;
-    uint32_t SymIndex = RI.getSymbol(Config->Mips64EL);
-    SymbolBody &Body = File.getSymbolBody(SymIndex);
+    SymbolBody &Body = File.getRelocTargetSym(RI);
     uint32_t Type = RI.getType(Config->Mips64EL);
 
     // Ignore "hint" relocation because it is for optional code optimization.
     if (Target->isHintRel(Type))
       continue;
 
     uintX_t Offset = C.getOffset(RI.r_offset);
     if (Offset == (uintX_t)-1)