[LLD][ELF] Use synthetic section to hold copy relocations

[peter.smith]

Summary

This changes adds a new synthetic section CopyRelSection to hold the ZI reserved for the result of the copy relocation, moreover the dynamic copy relocation is made relative to the CopyRelSection. Previously we altered the size of the OutputSection and made the copy relocation relative to an offset into the OutputSection. This change has a couple of small benefits:

• All dynamic relocations are now relative to an InputSection, this means we can remove the support for OutputSection relative dynamic relocations, which makes us more robust to changes in section offsets after scanRelocs().
• If assignOffsets() is called on .bss or .bss.rel.ro the offsets and final size of the OutputSection will be correct. Previously the work of scanRelocs() would be destroyed by assignOffsets().

[ruiu]

Generally looking good. A few minor comments.

ELF/Relocations.cpp

438–439 ↗

(On Diff #87411)

I wonder how many copy relocations are usually made for a program which needs copy relocations. If it is thousands, it is fine to create thousands of input sections here, but if it's more than that, we want to avoid that. (If it is small, I like this approach to create one section per a copy relocation, as it is simple.)

ELF/Symbols.cpp

87 ↗

(On Diff #87411)

auto -> InputSection<ELFT>

[pcc]

ELF/Symbols.cpp

236 ↗

(On Diff #87411)

Can you remove the CopyIsInBssRelRo field now?

[peter.smith]

Thank you very much for the comments. I've updated the diff to make the requested changes.

I've also had a quick investigate into how many copy relocs we would expect to see. I ran readelf --relocs | grep _COPY over most of the executables on my Linux box and came back with most having 0, quite a few having < 10 and a few outliers with 60 to 70. These were gdb and gpartedbin.

I would prefer to keep one section per copy rel as we can assume the offset in the section is 0, otherwise we'll need to record the offset in SharedSymbol.

number of copy rels	number of executables
0	4159
1	515
2	471
3	343
4	308
5	277
6	239
7	93
8	9
9	25
10	7
11	12
12	10
13	13
14	11
15	4
16	3
17	6
18	1
19	3
20	2
21	1
22	4
26	4
35	4
36	3
66	1
76	1

[ruiu]

LGTM

lld/trunk/ELF/Relocations.cpp

OutputSection<ELFT> *CopySec =
       IsReadOnly ? Out<ELFT>::BssRelRo : Out<ELFT>::Bss;
 
   uintX_t Alignment = getAlignment(SS);
   uintX_t Off = alignTo(CopySec->Size, Alignment);
   CopySec->Size = Off + SymSize;
   CopySec->updateAlignment(Alignment);
   uintX_t Shndx = SS->Sym.st_shndx;
   uintX_t Value = SS->Sym.st_value;
+
+  // Create a SyntheticSection in CopySec to hold the .bss and the Copy Reloc
+  auto *CopyISec = make<CopyRelSection<ELFT>>(IsReadOnly, Alignment, SymSize);
+  CopyISec->OutSecOff = Off;
+  CopyISec->OutSec = CopySec;
+  CopySec->Sections.push_back(CopyISec);
+
   // Look through the DSO's dynamic symbol table for aliases and create a
   // dynamic symbol for each one. This causes the copy relocation to correctly
   // interpose any aliases.
   for (const Elf_Sym &S : SS->file()->getGlobalSymbols()) {
     if (S.st_shndx != Shndx || S.st_value != Value)
       continue;
     auto *Alias = dyn_cast_or_null<SharedSymbol<ELFT>>(
         Symtab<ELFT>::X->find(check(S.getName(SS->file()->getStringTable()))));
     if (!Alias)
       continue;
-    Alias->CopyIsInBssRelRo = IsReadOnly;
-    Alias->CopyOffset = Off;
+    Alias->CopySection = CopyISec;
     Alias->NeedsCopyOrPltAddr = true;
     Alias->symbol()->IsUsedInRegularObj = true;
   }
-  In<ELFT>::RelaDyn->addReloc({Target->CopyRel, CopySec, Off, false, SS, 0});
+  In<ELFT>::RelaDyn->addReloc({Target->CopyRel, CopyISec, 0, false, SS, 0});
 }
 
 template <class ELFT>
 static RelExpr adjustExpr(const elf::ObjectFile<ELFT> &File, SymbolBody &Body,
                           bool IsWrite, RelExpr Expr, uint32_t Type,
                           const uint8_t *Data, InputSectionBase<ELFT> &S,
                           typename ELFT::uint RelOff) {
   bool Preemptible = isPreemptible(Body, Type);

lld/trunk/ELF/Symbols.h

public:
   unsigned Is32BitMipsGot : 1;
 
   // True if this symbol is in the Iplt sub-section of the Plt.
   unsigned IsInIplt : 1;
 
   // True if this symbol is in the Igot sub-section of the .got.plt or .got.
   unsigned IsInIgot : 1;
 
-  // True if this is a shared symbol in a read-only segment which requires a
-  // copy relocation. This causes space for the symbol to be allocated in the
-  // .bss.rel.ro section.
-  unsigned CopyIsInBssRelRo : 1;
-
   // The following fields have the same meaning as the ELF symbol attributes.
   uint8_t Type;    // symbol type
   uint8_t StOther; // st_other field value
 
   // The Type field may also have this value. It means that we have not yet seen
   // a non-Lazy symbol with this name, so we don't know what its type is. The
   // Type field is normally set to this value for Lazy symbols unless we saw a
   // weak undefined symbol first, in which case we need to remember the original
public:
 
   SharedFile<ELFT> *file() { return (SharedFile<ELFT> *)this->File; }
 
   const Elf_Sym &Sym;
 
   // This field is a pointer to the symbol's version definition.
   const Elf_Verdef *Verdef;
 
-  // CopyOffset is significant only when needsCopy() is true.
-  uintX_t CopyOffset = 0;
-
+  // CopySection is significant only when needsCopy() is true.
+  InputSection<ELFT> *CopySection = nullptr;
   bool needsCopy() const { return this->NeedsCopyOrPltAddr && !this->isFunc(); }
 
-  OutputSection<ELFT> *getBssSectionForCopy() const;
+  InputSection<ELFT> *getBssSectionForCopy() const;
 };
 
 // This class represents a symbol defined in an archive file. It is
 // created from an archive file header, and it knows how to load an
 // object file from an archive to replace itself with a defined
 // symbol. If the resolver finds both Undefined and Lazy for
 // the same name, it will ask the Lazy to load a file.
 class Lazy : public SymbolBody {

lld/trunk/ELF/Symbols.cpp

case SymbolBody::DefinedCommonKind:
     return In<ELFT>::Common->OutSec->Addr + In<ELFT>::Common->OutSecOff +
            cast<DefinedCommon>(Body).Offset;
   case SymbolBody::SharedKind: {
     auto &SS = cast<SharedSymbol<ELFT>>(Body);
     if (!SS.NeedsCopyOrPltAddr)
       return 0;
     if (SS.isFunc())
       return Body.getPltVA<ELFT>();
-    return SS.getBssSectionForCopy()->Addr + SS.CopyOffset;
+    InputSection<ELFT> *CopyISec = SS.getBssSectionForCopy();
+    return CopyISec->OutSec->Addr + CopyISec->OutSecOff;
   }
   case SymbolBody::UndefinedKind:
     return 0;
   case SymbolBody::LazyArchiveKind:
   case SymbolBody::LazyObjectKind:
     assert(Body.symbol()->IsUsedInRegularObj && "lazy symbol reached writer");
     return 0;
   }
   llvm_unreachable("invalid symbol kind");
 }
 
 SymbolBody::SymbolBody(Kind K, StringRefZ Name, bool IsLocal, uint8_t StOther,
                        uint8_t Type)
     : SymbolKind(K), NeedsCopyOrPltAddr(false), IsLocal(IsLocal),
       IsInGlobalMipsGot(false), Is32BitMipsGot(false), IsInIplt(false),
-      IsInIgot(false), CopyIsInBssRelRo(false), Type(Type), StOther(StOther),
+      IsInIgot(false), Type(Type), StOther(StOther),
       Name(Name) {}
 
 // Returns true if a symbol can be replaced at load-time by a symbol
 // with the same name defined in other ELF executable or DSO.
 bool SymbolBody::isPreemptible() const {
   if (isLocal())
     return false;
 

 
 Undefined::Undefined(StringRefZ Name, bool IsLocal, uint8_t StOther,
                      uint8_t Type, InputFile *File)
     : SymbolBody(SymbolBody::UndefinedKind, Name, IsLocal, StOther, Type) {
   this->File = File;
 }
 
 template <typename ELFT>
-OutputSection<ELFT> *SharedSymbol<ELFT>::getBssSectionForCopy() const {
+InputSection<ELFT> *SharedSymbol<ELFT>::getBssSectionForCopy() const {
   assert(needsCopy());
-  return CopyIsInBssRelRo ? Out<ELFT>::BssRelRo : Out<ELFT>::Bss;
+  assert(CopySection);
+  return CopySection;
 }
 
 DefinedCommon::DefinedCommon(StringRef Name, uint64_t Size, uint64_t Alignment,
                              uint8_t StOther, uint8_t Type, InputFile *File)
     : Defined(SymbolBody::DefinedCommonKind, Name, /*IsLocal=*/false, StOther,
               Type),
       Alignment(Alignment), Size(Size) {
   this->File = File;

lld/trunk/ELF/SyntheticSections.h

 private:
   void computeHash(llvm::ArrayRef<uint8_t> Buf,
                    std::function<void(uint8_t *, ArrayRef<uint8_t>)> Hash);
 
   size_t HashSize;
   uint8_t *HashBuf;
 };
 
+// SHT_NOBITS section created for a copyReloc
+template <class ELFT>
+class CopyRelSection final : public SyntheticSection<ELFT> {
+  typedef typename ELFT::uint uintX_t;
+
+public:
+  CopyRelSection(bool ReadOnly, uintX_t AddrAlign, size_t Size);
+  void writeTo(uint8_t *) override {}
+  size_t getSize() const override { return Size; }
+  size_t Size;
+};
+
 template <class ELFT>
 class MipsGotSection final : public SyntheticSection<ELFT> {
   typedef typename ELFT::uint uintX_t;
 
 public:
   MipsGotSection();
   void writeTo(uint8_t *Buf) override;
   size_t getSize() const override { return Size; }

 
 public:
   DynamicReloc(uint32_t Type, const InputSectionBase<ELFT> *InputSec,
                uintX_t OffsetInSec, bool UseSymVA, SymbolBody *Sym,
                uintX_t Addend)
       : Type(Type), Sym(Sym), InputSec(InputSec), OffsetInSec(OffsetInSec),
         UseSymVA(UseSymVA), Addend(Addend) {}
 
-  DynamicReloc(uint32_t Type, const OutputSectionBase *OutputSec,
-               uintX_t OffsetInSec, bool UseSymVA, SymbolBody *Sym,
-               uintX_t Addend)
-      : Type(Type), Sym(Sym), OutputSec(OutputSec), OffsetInSec(OffsetInSec),
-        UseSymVA(UseSymVA), Addend(Addend) {}
-
   uintX_t getOffset() const;
   uintX_t getAddend() const;
   uint32_t getSymIndex() const;
-  const OutputSectionBase *getOutputSec() const { return OutputSec; }
   const InputSectionBase<ELFT> *getInputSec() const { return InputSec; }
 
   uint32_t Type;
 
 private:
   SymbolBody *Sym;
   const InputSectionBase<ELFT> *InputSec = nullptr;
-  const OutputSectionBase *OutputSec = nullptr;
   uintX_t OffsetInSec;
   bool UseSymVA;
   uintX_t Addend;
 };
 
 template <class ELFT>
 class DynamicSection final : public SyntheticSection<ELFT> {
   typedef typename ELFT::Dyn Elf_Dyn;

lld/trunk/ELF/SyntheticSections.cpp

void BuildIdSection<ELFT>::computeHash(
   forLoop(0, Chunks.size(),
           [&](size_t I) { HashFn(Hashes.data() + I * HashSize, Chunks[I]); });
 
   // Write to the final output buffer.
   HashFn(HashBuf, Hashes);
 }
 
 template <class ELFT>
+CopyRelSection<ELFT>::CopyRelSection(bool ReadOnly, uintX_t AddrAlign, size_t S)
+    : SyntheticSection<ELFT>(SHF_ALLOC, SHT_NOBITS, AddrAlign,
+                             ReadOnly ? ".bss.rel.ro" : ".bss"),
+      Size(S) {
+  if (!ReadOnly)
+    this->Flags |= SHF_WRITE;
+}
+
+template <class ELFT>
 void BuildIdSection<ELFT>::writeBuildId(ArrayRef<uint8_t> Buf) {
   switch (Config->BuildId) {
   case BuildIdKind::Fast:
     computeHash(Buf, [](uint8_t *Dest, ArrayRef<uint8_t> Arr) {
       write64le(Dest, xxHash64(toStringRef(Arr)));
     });
     break;
   case BuildIdKind::Md5:
case Entry::PlainInt:
       break;
     }
     ++P;
   }
 }
 
 template <class ELFT>
 typename ELFT::uint DynamicReloc<ELFT>::getOffset() const {
-  if (OutputSec)
-    return OutputSec->Addr + OffsetInSec;
   return InputSec->OutSec->Addr + InputSec->getOffset(OffsetInSec);
 }
 
 template <class ELFT>
 typename ELFT::uint DynamicReloc<ELFT>::getAddend() const {
   if (UseSymVA)
     return Sym->getVA<ELFT>(Addend);
   return Addend;
SymbolTableSection<ELFT>::getOutputSection(SymbolBody *Sym) {
   }
   case SymbolBody::DefinedCommonKind:
     if (!Config->DefineCommon)
       return nullptr;
     return In<ELFT>::Common->OutSec;
   case SymbolBody::SharedKind: {
     auto &SS = cast<SharedSymbol<ELFT>>(*Sym);
     if (SS.needsCopy())
-      return SS.getBssSectionForCopy();
+      return SS.getBssSectionForCopy()->OutSec;
     break;
   }
   case SymbolBody::UndefinedKind:
   case SymbolBody::LazyArchiveKind:
   case SymbolBody::LazyObjectKind:
     break;
   }
   return nullptr;

 template class elf::MipsReginfoSection<ELF64LE>;
 template class elf::MipsReginfoSection<ELF64BE>;
 
 template class elf::BuildIdSection<ELF32LE>;
 template class elf::BuildIdSection<ELF32BE>;
 template class elf::BuildIdSection<ELF64LE>;
 template class elf::BuildIdSection<ELF64BE>;
 
+template class elf::CopyRelSection<ELF32LE>;
+template class elf::CopyRelSection<ELF32BE>;
+template class elf::CopyRelSection<ELF64LE>;
+template class elf::CopyRelSection<ELF64BE>;
+
 template class elf::GotSection<ELF32LE>;
 template class elf::GotSection<ELF32BE>;
 template class elf::GotSection<ELF64LE>;
 template class elf::GotSection<ELF64BE>;
 
 template class elf::MipsGotSection<ELF32LE>;
 template class elf::MipsGotSection<ELF32BE>;
 template class elf::MipsGotSection<ELF64LE>;