Make CommonInputSection singleton class.

[ruiu]

Summary

All other singleton instances are accessible globally.
CommonInputSection shouldn't be an exception.

[• rafael]

LGTM, nice cleanup!

[evgeny777]

LGTM

[evgeny777]

Rui, I wonder why have you removed zero(&Hdr) from CommonInputSection constructor?
The InputSectionBase<ELFT> constructor reads sh_addralign from Hdr to calculate Alignment field, and this can be done incorrectly in case Hdr is not zeroed

[ruiu]

It is guaranteed by the language spec (and by the fact that they are allocated to the .bss section) that global variables are initialized to zeros, no?

[evgeny777]

Yes, it is. But it looks like CommonInputSection singleton is actually a stack variable. See Writer.cpp:224

[ruiu]

Hdr is a static class member, so it should be initialized to zero, I think.

[evgeny777]

Oh, I didn't noticed that change, sorry

lld/trunk/ELF/InputSection.h

 
 public:
   MipsOptionsInputSection(ObjectFile<ELFT> *F, const Elf_Shdr *Hdr);
   static bool classof(const InputSectionBase<ELFT> *S);
 
   const llvm::object::Elf_Mips_RegInfo<ELFT> *Reginfo = nullptr;
 };
 
-// A special kind of section used to store common symbols
+// Common symbols don't belong to any section. But it is easier for us
+// to handle them as if they belong to some input section. So we defined
+// this class. CommonInputSection is a virtual singleton class that
+// "contains" all common symbols.
 template <class ELFT> class CommonInputSection : public InputSection<ELFT> {
   typedef typename ELFT::uint uintX_t;
 
 public:
   CommonInputSection();
 
+  // The singleton instance of this class.
+  static CommonInputSection<ELFT> *X;
+
 private:
-  typename ELFT::Shdr Hdr;
+  static typename ELFT::Shdr Hdr;
 };
 
+template <class ELFT> CommonInputSection<ELFT> *CommonInputSection<ELFT>::X;
+template <class ELFT> typename ELFT::Shdr CommonInputSection<ELFT>::Hdr;
+
 } // namespace elf
 } // namespace lld
 
 #endif

lld/trunk/ELF/InputSection.cpp

MipsOptionsInputSection<ELFT>::MipsOptionsInputSection(elf::ObjectFile<ELFT> *F,
   }
 }
 
 template <class ELFT>
 bool MipsOptionsInputSection<ELFT>::classof(const InputSectionBase<ELFT> *S) {
   return S->SectionKind == InputSectionBase<ELFT>::MipsOptions;
 }
 
-// Due to initialization order in C++ memberwise initialization or
-// construction is invoked after base class construction. This helper
-// function is needed to zero initialize Elf_Shdr, before passing it
-// to InputSection<ELFT> constructor
-template <class T> static T *zero(T *Val) {
-  return static_cast<T *>(memset(Val, 0, sizeof(*Val)));
-}
-
 template <class ELFT>
 CommonInputSection<ELFT>::CommonInputSection()
-    : InputSection<ELFT>(nullptr, zero(&Hdr)) {
+    : InputSection<ELFT>(nullptr, &Hdr) {
   std::vector<DefinedCommon<ELFT> *> Symbols;
   Hdr.sh_size = 0;
   Hdr.sh_type = SHT_NOBITS;
   Hdr.sh_flags = SHF_ALLOC | SHF_WRITE;
   this->Live = true;
 
   for (Symbol *S : Symtab<ELFT>::X->getSymbols())
     if (auto *C = dyn_cast<DefinedCommon<ELFT>>(S->body()))

lld/trunk/ELF/LinkerScript.h

 #include <functional>
 
 namespace lld {
 namespace elf {
 template <class ELFT> class InputSectionBase;
 template <class ELFT> class OutputSectionBase;
 template <class ELFT> class OutputSectionFactory;
 template <class ELFT> class DefinedCommon;
-template <class ELFT> class CommonInputSection;
 
 typedef std::function<uint64_t(uint64_t)> Expr;
 
 // Parses a linker script. Calling this function updates
 // Config and ScriptConfig.
 void readLinkerScript(MemoryBufferRef MB);
 
 class ScriptParser;

 extern ScriptConfiguration *ScriptConfig;
 
 // This is a runner of the linker script.
 template <class ELFT> class LinkerScript {
   typedef typename ELFT::uint uintX_t;
 
 public:
   std::vector<OutputSectionBase<ELFT> *>
-  createSections(OutputSectionFactory<ELFT> &Factory,
-                 CommonInputSection<ELFT> *Common);
+  createSections(OutputSectionFactory<ELFT> &Factory);
 
   std::vector<PhdrEntry<ELFT>>
   createPhdrs(ArrayRef<OutputSectionBase<ELFT> *> S);
 
   ArrayRef<uint8_t> getFiller(StringRef Name);
   bool shouldKeep(InputSectionBase<ELFT> *S);
   void assignAddresses(ArrayRef<OutputSectionBase<ELFT> *> S);
   int compareSections(StringRef A, StringRef B);
   void addScriptedSymbols();
   bool hasPhdrsCommands();
 
 private:
   std::vector<std::pair<StringRef, const InputSectionDescription *>>
   getSectionMap();
 
   std::vector<InputSectionBase<ELFT> *>
-  getInputSections(const InputSectionDescription *, CommonInputSection<ELFT> *);
+  getInputSections(const InputSectionDescription *);
 
   // "ScriptConfig" is a bit too long, so define a short name for it.
   ScriptConfiguration &Opt = *ScriptConfig;
 
   std::vector<OutputSectionBase<ELFT> *>
   filter(std::vector<OutputSectionBase<ELFT> *> &Sections);
 
   int getSectionIndex(StringRef Name);

lld/trunk/ELF/LinkerScript.cpp

if (auto *Cmd1 = dyn_cast<OutputSectionCommand>(Base1.get()))
           Ret.emplace_back(Cmd1->Name, Cmd2);
 
   return Ret;
 }
 
 // Returns input sections filtered by given glob patterns.
 template <class ELFT>
 std::vector<InputSectionBase<ELFT> *>
-LinkerScript<ELFT>::getInputSections(const InputSectionDescription *I,
-                                     CommonInputSection<ELFT> *Common) {
+LinkerScript<ELFT>::getInputSections(const InputSectionDescription *I) {
   ArrayRef<StringRef> Patterns = I->Patterns;
   ArrayRef<StringRef> ExcludedFiles = I->ExcludedFiles;
   std::vector<InputSectionBase<ELFT> *> Ret;
   for (const std::unique_ptr<ObjectFile<ELFT>> &F :
        Symtab<ELFT>::X->getObjectFiles())
     for (InputSectionBase<ELFT> *S : F->getSections())
       if (!isDiscarded(S) && !S->OutSec && match(Patterns, S->getSectionName()))
         if (ExcludedFiles.empty() ||
             !match(ExcludedFiles, sys::path::filename(F->getName())))
           Ret.push_back(S);
 
   if ((llvm::find(Patterns, "COMMON") != Patterns.end()))
-    Ret.push_back(Common);
+    Ret.push_back(CommonInputSection<ELFT>::X);
 
   return Ret;
 }
 
 template <class ELFT>
 std::vector<OutputSectionBase<ELFT> *>
-LinkerScript<ELFT>::createSections(OutputSectionFactory<ELFT> &Factory,
-                                   CommonInputSection<ELFT> *Common) {
+LinkerScript<ELFT>::createSections(OutputSectionFactory<ELFT> &Factory) {
   std::vector<OutputSectionBase<ELFT> *> Ret;
 
   // Add input section to output section. If there is no output section yet,
   // then create it and add to output section list.
   auto Add = [&](InputSectionBase<ELFT> *C, StringRef Name) {
     OutputSectionBase<ELFT> *Sec;
     bool IsNew;
     std::tie(Sec, IsNew) = Factory.create(C, Name);
     if (IsNew)
       Ret.push_back(Sec);
     Sec->addSection(C);
   };
 
   for (auto &P : getSectionMap()) {
     StringRef OutputName = P.first;
     const InputSectionDescription *I = P.second;
-    for (InputSectionBase<ELFT> *S : getInputSections(I, Common)) {
+    for (InputSectionBase<ELFT> *S : getInputSections(I)) {
       if (OutputName == "/DISCARD/") {
         S->Live = false;
         reportDiscarded(S);
         continue;
       }
       Add(S, OutputName);
     }
   }

lld/trunk/ELF/Writer.cpp

private:
   void writeHeader();
   void writeSections();
   void writeBuildId();
 
   std::unique_ptr<FileOutputBuffer> Buffer;
 
   BumpPtrAllocator Alloc;
   std::vector<OutputSectionBase<ELFT> *> OutputSections;
-  std::unique_ptr<CommonInputSection<ELFT>> CommonSection;
   OutputSectionFactory<ELFT> Factory;
 
   void addRelIpltSymbols();
   void addStartEndSymbols();
   void addStartStopSymbols(OutputSectionBase<ELFT> *Sec);
 
   SymbolTable<ELFT> &Symtab;
   std::vector<Phdr> Phdrs;

 }
 
 // The main function of the writer.
 template <class ELFT> void Writer<ELFT>::run() {
   if (!Config->DiscardAll)
     copyLocalSymbols();
   addReservedSymbols();
 
-  CommonSection = llvm::make_unique<CommonInputSection<ELFT>>();
+  CommonInputSection<ELFT> Common;
+  CommonInputSection<ELFT>::X = &Common;
+
   OutputSections =
       ScriptConfig->DoLayout
-          ? Script<ELFT>::X->createSections(Factory, CommonSection.get())
+          ? Script<ELFT>::X->createSections(Factory)
           : createSections();
   finalizeSections();
   if (HasError)
     return;
 
   if (Config->Relocatable) {
     assignFileOffsets();
   } else {
template <class ELFT> void Writer<ELFT>::finalizeSections() {
   }
 
   // Do not proceed if there was an undefined symbol.
   if (HasError)
     return;
 
   // If linker script processor hasn't added common symbol section yet,
   // then add it to .bss now.
-  if (!CommonSection->OutSec) {
-    Out<ELFT>::Bss->addSection(CommonSection.get());
+  if (!CommonInputSection<ELFT>::X->OutSec) {
+    Out<ELFT>::Bss->addSection(CommonInputSection<ELFT>::X);
     Out<ELFT>::Bss->assignOffsets();
   }
 
   // So far we have added sections from input object files.
   // This function adds linker-created Out<ELFT>::* sections.
   addPredefinedSections();
 
   std::stable_sort(OutputSections.begin(), OutputSections.end(),