[ELF] - Make Bss and BssRelRo sections to be synthetic (#3).

[grimar]

Summary

This is splitted from D30541 and polished.
Difference from D30541 that all logic of SharedSymbol
converting to DefinedRegular was removed for now and
will be posted as separate patch on top of this one.

[ruiu]

ELF/Relocations.cpp

483 ↗

(On Diff #91558)

I remember that last time I had to change this type from size_t to uintX_t because your previous change broke buildbots.

ELF/SyntheticSections.h

156–157 ↗

(On Diff #91558)

BssRelSection sections? This needs updating.

"BssSection is used to reserve space for copy relocations. We create two instances of this class for .bss and .bss.rel.ro. .bss is use for writable symbols, and .bss.rel.ro is used for read-only symbols."

165 ↗

(On Diff #91558)

Rafael made a change to rename AddrAlign Alignment. Please take a look at that change and make your change consistent with that.

[grimar]

• Addressed review comments.
• New BssSection is no more <ELFT> templated.

ELF/Relocations.cpp

483 ↗

(On Diff #91558)

Right, thanks.

ELF/SyntheticSections.h

165 ↗

(On Diff #91558)

Done.

[ruiu]

LGTM

ELF/SyntheticSections.h

166 ↗

(On Diff #91725)

Make this private if you are always accessing this via getSize.

[grimar]

• Fixed one more issue that hopefully should be enough now for aarch64 bot.

Issue was relative to symbol alignment. If we had common symbol and ".bss" section for it with alignment 4
and then created copy relocation for symbol with alignment == 8 then final layout was wrong.

It was AAAABBBBBBBB when should be AAAA0000BBBBBBBB, where A - space for common symbol and
B - space for copy relocated.

That happened because previously we would create 2 .bss sections (one for commons, one for relocations),
but now create single.

Testcase is provided.

lld/trunk/ELF/OutputSections.h

public:
   uint8_t *Loc = nullptr;
 };
 
 // All output sections that are handled by the linker specially are
 // globally accessible. Writer initializes them, so don't use them
 // until Writer is initialized.
 struct Out {
   static uint8_t First;
-  static OutputSection *Bss;
-  static OutputSection *BssRelRo;
   static OutputSection *Opd;
   static uint8_t *OpdBuf;
   static PhdrEntry *TlsPhdr;
   static OutputSection *DebugInfo;
   static OutputSection *ElfHeader;
   static OutputSection *ProgramHeaders;
   static OutputSection *PreinitArray;
   static OutputSection *InitArray;

lld/trunk/ELF/OutputSections.cpp

 using namespace llvm::object;
 using namespace llvm::support::endian;
 using namespace llvm::ELF;
 
 using namespace lld;
 using namespace lld::elf;
 
 uint8_t Out::First;
-OutputSection *Out::Bss;
-OutputSection *Out::BssRelRo;
 OutputSection *Out::Opd;
 uint8_t *Out::OpdBuf;
 PhdrEntry *Out::TlsPhdr;
 OutputSection *Out::DebugInfo;
 OutputSection *Out::ElfHeader;
 OutputSection *Out::ProgramHeaders;
 OutputSection *Out::PreinitArray;
 OutputSection *Out::InitArray;

lld/trunk/ELF/Relocations.cpp

template <class ELFT> static void addCopyRelSymbol(SharedSymbol *SS) {
   // Copy relocation against zero-sized symbol doesn't make sense.
   uintX_t SymSize = SS->template getSize<ELFT>();
   if (SymSize == 0)
     fatal("cannot create a copy relocation for symbol " + toString(*SS));
 
   // See if this symbol is in a read-only segment. If so, preserve the symbol's
   // memory protection by reserving space in the .bss.rel.ro section.
   bool IsReadOnly = isReadOnly<ELFT>(SS);
-  OutputSection *OSec = IsReadOnly ? Out::BssRelRo : Out::Bss;
-
-  // Create a SyntheticSection in Out to hold the .bss and the Copy Reloc.
-  auto *ISec =
-      make<CopyRelSection<ELFT>>(IsReadOnly, SS->getAlignment<ELFT>(), SymSize);
-  OSec->addSection(ISec);
+  BssSection *Sec = IsReadOnly ? In<ELFT>::BssRelRo : In<ELFT>::Bss;
+  uintX_t Off = Sec->reserveSpace(SS->getAlignment<ELFT>(), SymSize);
 
   // 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 (SharedSymbol *Sym : getSymbolsAt<ELFT>(SS)) {
     Sym->NeedsCopy = true;
-    Sym->Section = ISec;
+    Sym->CopyRelSec = Sec;
+    Sym->CopyRelSecOff = Off;
     Sym->symbol()->IsUsedInRegularObj = true;
   }
 
-  In<ELFT>::RelaDyn->addReloc({Target->CopyRel, ISec, 0, false, SS, 0});
+  In<ELFT>::RelaDyn->addReloc({Target->CopyRel, Sec, Off, 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 &S,
                           typename ELFT::uint RelOff) {
   bool Preemptible = isPreemptible(Body, Type);

lld/trunk/ELF/Symbols.h

template <class ELFT> uint64_t getSize() const {
     return getSym<ELFT>().st_size;
   }
 
   template <class ELFT> uint32_t getAlignment() const;
 
   // This field is a pointer to the symbol's version definition.
   const void *Verdef;
 
-  // Section is significant only when NeedsCopy is true.
-  InputSection *Section = nullptr;
+  // CopyRelSec and CopyRelSecOff are significant only when NeedsCopy is true.
+  InputSection *CopyRelSec;
+  size_t CopyRelSecOff;
 
 private:
   template <class ELFT> const typename ELFT::Sym &getSym() const {
     return *(const typename ELFT::Sym *)ElfSym;
   }
 
   const void *ElfSym;
 };

lld/trunk/ELF/Symbols.cpp

static typename ELFT::uint getSymVA(const SymbolBody &Body, int64_t &Addend) {
   case SymbolBody::DefinedCommonKind:
     if (!Config->DefineCommon)
       return 0;
     return In<ELFT>::Common->OutSec->Addr + In<ELFT>::Common->OutSecOff +
            cast<DefinedCommon>(Body).Offset;
   case SymbolBody::SharedKind: {
     auto &SS = cast<SharedSymbol>(Body);
     if (SS.NeedsCopy)
-      return SS.Section->OutSec->Addr + SS.Section->OutSecOff;
+      return SS.CopyRelSec->OutSec->Addr + SS.CopyRelSec->OutSecOff +
+             SS.CopyRelSecOff;
     if (SS.NeedsPltAddr)
       return Body.getPltVA<ELFT>();
     return 0;
   }
   case SymbolBody::UndefinedKind:
     return 0;
   case SymbolBody::LazyArchiveKind:
   case SymbolBody::LazyObjectKind:
OutputSection *SymbolBody::getOutputSection() const {
   if (auto *S = dyn_cast<DefinedRegular>(this)) {
     if (S->Section)
       return S->Section->getOutputSection();
     return nullptr;
   }
 
   if (auto *S = dyn_cast<SharedSymbol>(this)) {
     if (S->NeedsCopy)
-      return S->Section->OutSec;
+      return S->CopyRelSec->OutSec;
     return nullptr;
   }
 
   if (isa<DefinedCommon>(this)) {
     if (Config->DefineCommon)
       return InX::Common->OutSec;
     return nullptr;
   }

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;
 };
 
-// For each copy relocation, we create an instance of this class to
-// reserve space in .bss or .bss.rel.ro.
-template <class ELFT> class CopyRelSection final : public SyntheticSection {
+// BssSection is used to reserve space for copy relocations. We create two
+// instances of this class for .bss and .bss.rel.ro. .bss is used for writable
+// symbols, and .bss.rel.ro is used for read-only symbols.
+class BssSection final : public SyntheticSection {
 public:
-  CopyRelSection(bool ReadOnly, uint32_t Alignment, size_t Size);
+  BssSection(StringRef Name);
   void writeTo(uint8_t *) override {}
-
+  bool empty() const override { return getSize() == 0; }
+  size_t reserveSpace(uint32_t Alignment, size_t Size);
   size_t getSize() const override { return Size; }
-  size_t Size;
+
+private:
+  size_t Size = 0;
 };
 
 template <class ELFT> class MipsGotSection final : public SyntheticSection {
   typedef typename ELFT::uint uintX_t;
 
 public:
   MipsGotSection();
   void writeTo(uint8_t *Buf) override;

 template <class ELFT> MergeInputSection *createCommentSection();
 template <class ELFT>
 SymbolBody *addSyntheticLocal(StringRef Name, uint8_t Type, uint64_t Value,
                               uint64_t Size, InputSectionBase *Section);
 
 // Linker generated sections which can be used as inputs.
 struct InX {
   static InputSection *ARMAttributes;
+  static BssSection *Bss;
+  static BssSection *BssRelRo;
   static InputSection *Common;
   static StringTableSection *DynStrTab;
   static InputSection *Interp;
   static GotPltSection *GotPlt;
   static IgotPltSection *IgotPlt;
   static MipsRldMapSection *MipsRldMap;
   static StringTableSection *ShStrTab;
   static StringTableSection *StrTab;

lld/trunk/ELF/SyntheticSections.cpp

void BuildIdSection<ELFT>::computeHash(
   // Compute hash values.
   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, uint32_t Alignment,
-                                     size_t S)
-    : SyntheticSection(SHF_ALLOC, SHT_NOBITS, Alignment,
-                       ReadOnly ? ".bss.rel.ro" : ".bss"),
-      Size(S) {}
+BssSection::BssSection(StringRef Name)
+    : SyntheticSection(SHF_ALLOC | SHF_WRITE, SHT_NOBITS, 0, Name) {}
+
+size_t BssSection::reserveSpace(uint32_t Alignment, size_t Size) {
+  OutSec->updateAlignment(Alignment);
+  this->Size = alignTo(this->Size, Alignment) + Size;
+  this->Alignment = std::max<uint32_t>(this->Alignment, Alignment);
+  return this->Size - Size;
+}
 
 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)));
     });

 }
 
 InputSection *ThunkSection::getTargetInputSection() const {
   const Thunk *T = Thunks.front();
   return T->getTargetInputSection();
 }
 
 InputSection *InX::ARMAttributes;
+BssSection *InX::Bss;
+BssSection *InX::BssRelRo;
 InputSection *InX::Common;
 StringTableSection *InX::DynStrTab;
 InputSection *InX::Interp;
 GotPltSection *InX::GotPlt;
 IgotPltSection *InX::IgotPlt;
 MipsRldMapSection *InX::MipsRldMap;
 StringTableSection *InX::ShStrTab;
 StringTableSection *InX::StrTab;

 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>;

lld/trunk/ELF/Writer.cpp

for (StringRef V : {".rel.", ".rela."}) {
       if (Name.startswith(V)) {
         StringRef Inner = getOutputSectionName(Name.substr(V.size() - 1));
         return Saver.save(Twine(V.drop_back()) + Inner);
       }
     }
   }
 
   for (StringRef V :
-       {".text.", ".rodata.", ".data.rel.ro.", ".data.", ".bss.",
-        ".init_array.", ".fini_array.", ".ctors.", ".dtors.", ".tbss.",
+       {".text.", ".rodata.", ".data.rel.ro.", ".data.", ".bss.rel.ro.",
+        ".bss.", ".init_array.", ".fini_array.", ".ctors.", ".dtors.", ".tbss.",
         ".gcc_except_table.", ".tdata.", ".ARM.exidx."}) {
     StringRef Prefix = V.drop_back();
     if (Name.startswith(V) || Name == Prefix)
       return Prefix;
   }
 
   // CommonSection is identified as "COMMON" in linker scripts.
   // By default, it should go to .bss section.

 // Initialize Out members.
 template <class ELFT> void Writer<ELFT>::createSyntheticSections() {
   // Initialize all pointers with NULL. This is needed because
   // you can call lld::elf::main more than once as a library.
   memset(&Out::First, 0, sizeof(Out));
 
   auto Add = [](InputSectionBase *Sec) { InputSections.push_back(Sec); };
 
-  // Create singleton output sections.
-  Out::Bss = make<OutputSection>(".bss", SHT_NOBITS, SHF_ALLOC | SHF_WRITE);
-  Out::BssRelRo =
-      make<OutputSection>(".bss.rel.ro", SHT_NOBITS, SHF_ALLOC | SHF_WRITE);
-
   In<ELFT>::DynStrTab = make<StringTableSection>(".dynstr", true);
   In<ELFT>::Dynamic = make<DynamicSection<ELFT>>();
   In<ELFT>::RelaDyn = make<RelocationSection<ELFT>>(
       Config->isRela() ? ".rela.dyn" : ".rel.dyn", Config->ZCombreloc);
   In<ELFT>::ShStrTab = make<StringTableSection>(".shstrtab", false);
 
   Out::ElfHeader = make<OutputSection>("", 0, SHF_ALLOC);
   Out::ElfHeader->Size = sizeof(Elf_Ehdr);
template <class ELFT> void Writer<ELFT>::createSyntheticSections() {
   }
 
   InputSection *Common = createCommonSection<ELFT>();
   if (!Common->Data.empty()) {
     In<ELFT>::Common = Common;
     Add(Common);
   }
 
+  In<ELFT>::Bss = make<BssSection>(".bss");
+  Add(In<ELFT>::Bss);
+  In<ELFT>::BssRelRo = make<BssSection>(".bss.rel.ro");
+  Add(In<ELFT>::BssRelRo);
+
   // Add MIPS-specific sections.
   bool HasDynSymTab =
       !Symtab<ELFT>::X->getSharedFiles().empty() || Config->pic() ||
       Config->ExportDynamic;
   if (Config->EMachine == EM_MIPS) {
     if (!Config->Shared && HasDynSymTab) {
       In<ELFT>::MipsRldMap = make<MipsRldMapSection>();
       Add(In<ELFT>::MipsRldMap);
if (Type == SHT_INIT_ARRAY || Type == SHT_FINI_ARRAY ||
     return true;
 
   if (Sec == In<ELFT>::GotPlt->OutSec)
     return Config->ZNow;
   if (Sec == In<ELFT>::Dynamic->OutSec)
     return true;
   if (In<ELFT>::Got && Sec == In<ELFT>::Got->OutSec)
     return true;
-  if (Sec == Out::BssRelRo)
+  if (Sec == In<ELFT>::BssRelRo->OutSec)
     return true;
 
   StringRef S = Sec->Name;
   return S == ".data.rel.ro" || S == ".ctors" || S == ".dtors" || S == ".jcr" ||
          S == ".eh_frame" || S == ".openbsd.randomdata";
 }
 
 template <class ELFT>
if (!Config->Relocatable && !Config->OFormatBinary) {
     Phdrs = Script<ELFT>::X->hasPhdrsCommands() ? Script<ELFT>::X->createPhdrs()
                                                 : createPhdrs();
     addPtArmExid(Phdrs);
     fixHeaders();
   }
 
   // Dynamic section must be the last one in this list and dynamic
   // symbol table section (DynSymTab) must be the first one.
-  applySynthetic({In<ELFT>::DynSymTab, In<ELFT>::GnuHashTab, In<ELFT>::HashTab,
-                  In<ELFT>::SymTab,    In<ELFT>::ShStrTab,   In<ELFT>::StrTab,
-                  In<ELFT>::VerDef,    In<ELFT>::DynStrTab,  In<ELFT>::GdbIndex,
-                  In<ELFT>::Got,       In<ELFT>::MipsGot,    In<ELFT>::IgotPlt,
-                  In<ELFT>::GotPlt,    In<ELFT>::RelaDyn,    In<ELFT>::RelaIplt,
-                  In<ELFT>::RelaPlt,   In<ELFT>::Plt,        In<ELFT>::Iplt,
-                  In<ELFT>::Plt,       In<ELFT>::EhFrameHdr, In<ELFT>::VerSym,
-                  In<ELFT>::VerNeed,   In<ELFT>::Dynamic},
+  applySynthetic({In<ELFT>::DynSymTab,  In<ELFT>::Bss,      In<ELFT>::BssRelRo,
+                  In<ELFT>::GnuHashTab, In<ELFT>::HashTab,  In<ELFT>::SymTab,
+                  In<ELFT>::ShStrTab,   In<ELFT>::StrTab,   In<ELFT>::VerDef,
+                  In<ELFT>::DynStrTab,  In<ELFT>::GdbIndex, In<ELFT>::Got,
+                  In<ELFT>::MipsGot,    In<ELFT>::IgotPlt,  In<ELFT>::GotPlt,
+                  In<ELFT>::RelaDyn,    In<ELFT>::RelaIplt, In<ELFT>::RelaPlt,
+                  In<ELFT>::Plt,        In<ELFT>::Iplt,     In<ELFT>::Plt,
+                  In<ELFT>::EhFrameHdr, In<ELFT>::VerSym,   In<ELFT>::VerNeed,
+                  In<ELFT>::Dynamic},
                  [](SyntheticSection *SS) { SS->finalizeContents(); });
 
   // Some architectures use small displacements for jump instructions.
   // It is linker's responsibility to create thunks containing long
   // jump instructions if jump targets are too far. Create thunks.
   if (Target->NeedsThunks) {
     // FIXME: only ARM Interworking and Mips LA25 Thunks are implemented,
     // these
for (OutputSection *Sec : OutputSections)
     Sec->finalize<ELFT>();
 
   // createThunks may have added local symbols to the static symbol table
   applySynthetic({In<ELFT>::SymTab, In<ELFT>::ShStrTab, In<ELFT>::StrTab},
                  [](SyntheticSection *SS) { SS->postThunkContents(); });
 }
 
 template <class ELFT> void Writer<ELFT>::addPredefinedSections() {
-  // Add BSS sections.
-  auto Add = [=](OutputSection *Sec) {
-    if (!Sec->Sections.empty()) {
-      Sec->assignOffsets();
-      OutputSections.push_back(Sec);
-    }
-  };
-  Add(Out::Bss);
-  Add(Out::BssRelRo);
-
   // ARM ABI requires .ARM.exidx to be terminated by some piece of data.
   // We have the terminater synthetic section class. Add that at the end.
   auto *OS = dyn_cast_or_null<OutputSection>(findSection(".ARM.exidx"));
   if (OS && !OS->Sections.empty() && !Config->Relocatable)
     OS->addSection(make<ARMExidxSentinelSection<ELFT>>());
 }
 
 // The linker is expected to define SECNAME_start and SECNAME_end

lld/trunk/test/ELF/Inputs/relocation-copy-align-common.s

+.data
+.global foo
+.type foo, @object
+.align 8
+.size foo, 8
+foo:
+.quad 0

lld/trunk/test/ELF/relocation-copy-align-common.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux %s -o %t.o
+# RUN: llvm-mc -filetype=obj -triple=x86_64-pc-linux \
+# RUN:   %p/Inputs/relocation-copy-align-common.s -o %t2.o
+# RUN: ld.lld -shared %t2.o -o %t.so
+# RUN: ld.lld %t.o %t.so -o %t3
+# RUN: llvm-readobj -s -r --expand-relocs %t3 | FileCheck %s
+
+# CHECK:      Section {
+# CHECK:        Index:
+# CHECK:        Name: .bss
+# CHECK-NEXT:   Type: SHT_NOBITS
+# CHECK-NEXT:   Flags [
+# CHECK-NEXT:     SHF_ALLOC
+# CHECK-NEXT:     SHF_WRITE
+# CHECK-NEXT:   ]
+# CHECK-NEXT:   Address: 0x203000
+# CHECK-NEXT:   Offset: 0x20B0
+# CHECK-NEXT:   Size: 16
+# CHECK-NEXT:   Link: 0
+# CHECK-NEXT:   Info: 0
+# CHECK-NEXT:   AddressAlignment: 8
+# CHECK-NEXT:   EntrySize: 0
+# CHECK-NEXT: }
+
+# CHECK:      Relocations [
+# CHECK-NEXT:   Section {{.*}} .rela.dyn {
+# CHECK-NEXT:     Relocation {
+# CHECK-NEXT:       Offset: 0x203008
+# CHECK-NEXT:       Type: R_X86_64_COPY
+# CHECK-NEXT:       Symbol: foo
+# CHECK-NEXT:       Addend: 0x0
+# CHECK-NEXT:     }
+# CHECK-NEXT:   }
+# CHECK-NEXT: ]
+
+.global _start
+_start:
+.comm sym1,4,4
+movl $5, foo