[ELF] Simplify adding default atoms

[atanasyan]

Summary

Now 'writer' creates an instance of RuntimeFile in the constructor, then populates the file in the virtual function addDefaultAtoms, then pass owning of this file to the caller of virtual function createImplicitFiles.

First, we do not need to keep an instance of RuntimeFile so long. It is enough to create the file, right after that populate it and pass the owning.

Second, relationship between createImplicitFiles and addDefaultAtoms is complicated. The createImplicitFiles might call addDefaultAtoms, overridden version of addDefaultAtoms might call base class addDefaultAtoms, and overridden version of createImplicitFiles might call base class createImplicitFiles as well as addDefaultAtoms.

The patch solves both problems above. It creates and populates runtime files right in the createImplicitFiles(), removes addDefaultAtoms at all and does not keep references to runtime files in class fields.

[• shankar.easwaran]

LGTM

[ruiu]

Thank you for doing this. This is a nice cleanup. LGTM.

lib/ReaderWriter/ELF/DynamicLibraryWriter.h

64

addAbsoluteAtom returns a pointer to a new atom, so we discard the pointer here. Looks like we don't use return values from the function at all. This doesn't have to be done in this patch, but we should make the function return void.

[atanasyan]

Thanks for review.

Closed by commit rL234347.

lib/ReaderWriter/ELF/AArch64/AArch64DynamicLibraryWriter.h

 protected:
   // Add any runtime files and their atoms to the output
   void createImplicitFiles(std::vector<std::unique_ptr<File>> &) override;
 
   void finalizeDefaultAtomValues() override {
     return DynamicLibraryWriter<ELFT>::finalizeDefaultAtomValues();
   }
 
-  void addDefaultAtoms() override {
-    return DynamicLibraryWriter<ELFT>::addDefaultAtoms();
-  }
-
 private:
   class GOTFile : public SimpleFile {
   public:
     GOTFile(const ELFLinkingContext &eti) : SimpleFile("GOTFile") {}
     llvm::BumpPtrAllocator _alloc;
   };
 
   std::unique_ptr<GOTFile> _gotFile;

lib/ReaderWriter/ELF/AArch64/AArch64ExecutableWriter.h

 protected:
   // Add any runtime files and their atoms to the output
   void createImplicitFiles(std::vector<std::unique_ptr<File>> &) override;
 
   void finalizeDefaultAtomValues() override {
     return ExecutableWriter<ELFT>::finalizeDefaultAtomValues();
   }
 
-  void addDefaultAtoms() override{
-    return ExecutableWriter<ELFT>::addDefaultAtoms();
-  }
-
 private:
   class GOTFile : public SimpleFile {
   public:
     GOTFile(const ELFLinkingContext &eti) : SimpleFile("GOTFile") {}
     llvm::BumpPtrAllocator _alloc;
   };
 
   std::unique_ptr<GOTFile> _gotFile;

lib/ReaderWriter/ELF/ARM/ARMExecutableWriter.h

public:
   ARMExecutableWriter(ARMLinkingContext &ctx, ARMTargetLayout<ELFT> &layout);
 
 protected:
   // Add any runtime files and their atoms to the output
   void createImplicitFiles(std::vector<std::unique_ptr<File>> &) override;
 
   void finalizeDefaultAtomValues() override;
 
-  void addDefaultAtoms() override {
-    ExecutableWriter<ELFT>::addDefaultAtoms();
-  }
-
   /// \brief Create symbol table.
   unique_bump_ptr<SymbolTable<ELFT>> createSymbolTable() override;
 
   void processUndefinedSymbol(StringRef symName,
                               RuntimeFile<ELFT> &file) const override;
 
   // Setup the ELF header.
   std::error_code setELFHeader() override;

lib/ReaderWriter/ELF/DynamicLibraryWriter.h

 
 //===----------------------------------------------------------------------===//
 //  DynamicLibraryWriter Class
 //===----------------------------------------------------------------------===//
 template<class ELFT>
 class DynamicLibraryWriter : public OutputELFWriter<ELFT> {
 public:
   DynamicLibraryWriter(ELFLinkingContext &ctx, TargetLayout<ELFT> &layout)
-      : OutputELFWriter<ELFT>(ctx, layout),
-        _runtimeFile(new RuntimeFile<ELFT>(ctx, "C runtime")) {}
+      : OutputELFWriter<ELFT>(ctx, layout) {}
 
 protected:
   void buildDynamicSymbolTable(const File &file) override;
-  void addDefaultAtoms() override;
   void createImplicitFiles(std::vector<std::unique_ptr<File>> &) override;
   void finalizeDefaultAtomValues() override;
-
-protected:
-  std::unique_ptr<RuntimeFile<ELFT> > _runtimeFile;
 };
 
 //===----------------------------------------------------------------------===//
 //  DynamicLibraryWriter
 //===----------------------------------------------------------------------===//
 template <class ELFT>
 void DynamicLibraryWriter<ELFT>::buildDynamicSymbolTable(const File &file) {
   // Add all the defined symbols to the dynamic symbol table
if (auto section = dyn_cast<AtomSection<ELFT>>(sec))
       }
 
   for (const UndefinedAtom *a : file.undefined())
     this->_dynamicSymbolTable->addSymbol(a, ELF::SHN_UNDEF);
 
   OutputELFWriter<ELFT>::buildDynamicSymbolTable(file);
 }
 
-template <class ELFT> void DynamicLibraryWriter<ELFT>::addDefaultAtoms() {
-  OutputELFWriter<ELFT>::addDefaultAtoms();
-  _runtimeFile->addAbsoluteAtom("_end");
-}
-
 /// \brief Hook in lld to add CRuntime file
 template <class ELFT>
 void DynamicLibraryWriter<ELFT>::createImplicitFiles(
     std::vector<std::unique_ptr<File> > &result) {
-  // Add the default atoms as defined by executables
-  DynamicLibraryWriter<ELFT>::addDefaultAtoms();
   OutputELFWriter<ELFT>::createImplicitFiles(result);
-  result.push_back(std::move(_runtimeFile));
+  // Add the default atoms as defined by executables
+  auto file = llvm::make_unique<RuntimeFile<ELFT>>(this->_ctx, "C runtime");
+  file->addAbsoluteAtom("_end");
+  result.push_back(std::move(file));
 }
 
 template <class ELFT>
 void DynamicLibraryWriter<ELFT>::finalizeDefaultAtomValues() {
   OutputELFWriter<ELFT>::finalizeDefaultAtomValues();
   lld::AtomLayout *underScoreEndAtom = this->_layout.findAbsoluteAtom("_end");
   assert(underScoreEndAtom);
 

lib/ReaderWriter/ELF/ExecutableWriter.h

 
 //===----------------------------------------------------------------------===//
 //  ExecutableWriter Class
 //===----------------------------------------------------------------------===//
 template<class ELFT>
 class ExecutableWriter : public OutputELFWriter<ELFT> {
 public:
   ExecutableWriter(ELFLinkingContext &ctx, TargetLayout<ELFT> &layout)
-      : OutputELFWriter<ELFT>(ctx, layout),
-        _runtimeFile(new RuntimeFile<ELFT>(ctx, "C runtime")) {}
+      : OutputELFWriter<ELFT>(ctx, layout) {}
 
 protected:
   void buildDynamicSymbolTable(const File &file) override;
-  void addDefaultAtoms() override;
   void createImplicitFiles(std::vector<std::unique_ptr<File>> &) override;
   void finalizeDefaultAtomValues() override;
   void createDefaultSections() override;
 
   bool isNeededTagRequired(const SharedLibraryAtom *sla) const override {
     return this->_layout.isCopied(sla);
   }
 
   unique_bump_ptr<InterpSection<ELFT>> _interpSection;
-  std::unique_ptr<RuntimeFile<ELFT> > _runtimeFile;
+
+private:
+  std::unique_ptr<RuntimeFile<ELFT>> createRuntimeFile();
 };
 
 //===----------------------------------------------------------------------===//
 //  ExecutableWriter
 //===----------------------------------------------------------------------===//
 template<class ELFT>
 void ExecutableWriter<ELFT>::buildDynamicSymbolTable(const File &file) {
   for (auto sec : this->_layout.sections())
for (const UndefinedAtom *a : file.undefined()) {
           a->canBeNull() != UndefinedAtom::canBeNullNever)
         this->_dynamicSymbolTable->addSymbol(a, ELF::SHN_UNDEF);
     }
   }
 
   OutputELFWriter<ELFT>::buildDynamicSymbolTable(file);
 }
 
-/// \brief Add absolute symbols by default. These are linker added
-/// absolute symbols
 template<class ELFT>
-void ExecutableWriter<ELFT>::addDefaultAtoms() {
-  OutputELFWriter<ELFT>::addDefaultAtoms();
-  _runtimeFile->addUndefinedAtom(this->_ctx.entrySymbolName());
-  _runtimeFile->addAbsoluteAtom("__bss_start");
-  _runtimeFile->addAbsoluteAtom("__bss_end");
-  _runtimeFile->addAbsoluteAtom("_end");
-  _runtimeFile->addAbsoluteAtom("end");
-  _runtimeFile->addAbsoluteAtom("__preinit_array_start");
-  _runtimeFile->addAbsoluteAtom("__preinit_array_end");
-  _runtimeFile->addAbsoluteAtom("__init_array_start");
-  _runtimeFile->addAbsoluteAtom("__init_array_end");
+std::unique_ptr<RuntimeFile<ELFT>> ExecutableWriter<ELFT>::createRuntimeFile() {
+  auto file = llvm::make_unique<RuntimeFile<ELFT>>(this->_ctx, "C runtime");
+  file->addUndefinedAtom(this->_ctx.entrySymbolName());
+  file->addAbsoluteAtom("__bss_start");
+  file->addAbsoluteAtom("__bss_end");
+  file->addAbsoluteAtom("_end");
+  file->addAbsoluteAtom("end");
+  file->addAbsoluteAtom("__preinit_array_start");
+  file->addAbsoluteAtom("__preinit_array_end");
+  file->addAbsoluteAtom("__init_array_start");
+  file->addAbsoluteAtom("__init_array_end");
   if (this->_ctx.isRelaOutputFormat()) {
-    _runtimeFile->addAbsoluteAtom("__rela_iplt_start");
-    _runtimeFile->addAbsoluteAtom("__rela_iplt_end");
+    file->addAbsoluteAtom("__rela_iplt_start");
+    file->addAbsoluteAtom("__rela_iplt_end");
   } else {
-    _runtimeFile->addAbsoluteAtom("__rel_iplt_start");
-    _runtimeFile->addAbsoluteAtom("__rel_iplt_end");
+    file->addAbsoluteAtom("__rel_iplt_start");
+    file->addAbsoluteAtom("__rel_iplt_end");
   }
-  _runtimeFile->addAbsoluteAtom("__fini_array_start");
-  _runtimeFile->addAbsoluteAtom("__fini_array_end");
+  file->addAbsoluteAtom("__fini_array_start");
+  file->addAbsoluteAtom("__fini_array_end");
+  return file;
 }
 
 /// \brief Hook in lld to add CRuntime file
 template <class ELFT>
 void ExecutableWriter<ELFT>::createImplicitFiles(
     std::vector<std::unique_ptr<File> > &result) {
-  // Add the default atoms as defined by executables
-  ExecutableWriter<ELFT>::addDefaultAtoms();
   OutputELFWriter<ELFT>::createImplicitFiles(result);
-  result.push_back(std::move(_runtimeFile));
+  result.push_back(createRuntimeFile());
 }
 
 template <class ELFT> void ExecutableWriter<ELFT>::createDefaultSections() {
   OutputELFWriter<ELFT>::createDefaultSections();
   if (this->_ctx.isDynamic()) {
     _interpSection.reset(new (this->_alloc) InterpSection<ELFT>(
         this->_ctx, ".interp", TargetLayout<ELFT>::ORDER_INTERP,
         this->_ctx.getInterpreter()));

lib/ReaderWriter/ELF/Hexagon/HexagonDynamicLibraryWriter.h

protected:
 
   std::error_code setELFHeader() override {
     DynamicLibraryWriter<ELFT>::setELFHeader();
     setHexagonELFHeader(*this->_elfHeader);
     return std::error_code();
   }
 
 private:
-  void addDefaultAtoms() override {
-    _runtimeFile->addAbsoluteAtom("_GLOBAL_OFFSET_TABLE_");
-    _runtimeFile->addAbsoluteAtom("_DYNAMIC");
-  }
-
   HexagonLinkingContext &_ctx;
   HexagonTargetLayout<ELFT> &_targetLayout;
-  std::unique_ptr<HexagonRuntimeFile<ELFT>> _runtimeFile;
 };
 
 template <class ELFT>
 HexagonDynamicLibraryWriter<ELFT>::HexagonDynamicLibraryWriter(
     HexagonLinkingContext &ctx, HexagonTargetLayout<ELFT> &layout)
-    : DynamicLibraryWriter<ELFT>(ctx, layout), _ctx(ctx), _targetLayout(layout),
-      _runtimeFile(new HexagonRuntimeFile<ELFT>(ctx)) {}
+    : DynamicLibraryWriter<ELFT>(ctx, layout), _ctx(ctx),
+      _targetLayout(layout) {}
 
 template <class ELFT>
 void HexagonDynamicLibraryWriter<ELFT>::createImplicitFiles(
     std::vector<std::unique_ptr<File>> &result) {
   DynamicLibraryWriter<ELFT>::createImplicitFiles(result);
   // Add the default atoms as defined for hexagon
-  addDefaultAtoms();
-  result.push_back(std::move(_runtimeFile));
+  auto file = llvm::make_unique<HexagonRuntimeFile<ELFT>>(_ctx);
+  file->addAbsoluteAtom("_GLOBAL_OFFSET_TABLE_");
+  file->addAbsoluteAtom("_DYNAMIC");
+  result.push_back(std::move(file));
 }
 
 template <class ELFT>
 void HexagonDynamicLibraryWriter<ELFT>::finalizeDefaultAtomValues() {
   // Finalize the atom values that are part of the parent.
   DynamicLibraryWriter<ELFT>::finalizeDefaultAtomValues();
   if (_ctx.isDynamic())
     finalizeHexagonRuntimeAtomValues(_targetLayout);
 }
 
 } // namespace elf
 } // namespace lld
 
 #endif // HEXAGON_DYNAMIC_LIBRARY_WRITER_H

lib/ReaderWriter/ELF/Hexagon/HexagonExecutableWriter.h

protected:
 
   std::error_code setELFHeader() override {
     ExecutableWriter<ELFT>::setELFHeader();
     setHexagonELFHeader(*this->_elfHeader);
     return std::error_code();
   }
 
 private:
-  void addDefaultAtoms() override {
-    _runtimeFile->addAbsoluteAtom("_SDA_BASE_");
-    if (this->_ctx.isDynamic()) {
-      _runtimeFile->addAbsoluteAtom("_GLOBAL_OFFSET_TABLE_");
-      _runtimeFile->addAbsoluteAtom("_DYNAMIC");
-    }
-  }
-
   HexagonLinkingContext &_ctx;
   HexagonTargetLayout<ELFT> &_targetLayout;
-  std::unique_ptr<HexagonRuntimeFile<ELFT>> _runtimeFile;
 };
 
 template <class ELFT>
 HexagonExecutableWriter<ELFT>::HexagonExecutableWriter(
     HexagonLinkingContext &ctx, HexagonTargetLayout<ELFT> &layout)
-    : ExecutableWriter<ELFT>(ctx, layout), _ctx(ctx), _targetLayout(layout),
-      _runtimeFile(new HexagonRuntimeFile<ELFT>(ctx)) {}
+    : ExecutableWriter<ELFT>(ctx, layout), _ctx(ctx), _targetLayout(layout) {}
 
 template <class ELFT>
 void HexagonExecutableWriter<ELFT>::createImplicitFiles(
     std::vector<std::unique_ptr<File>> &result) {
   ExecutableWriter<ELFT>::createImplicitFiles(result);
   // Add the default atoms as defined for hexagon
-  addDefaultAtoms();
-  result.push_back(std::move(_runtimeFile));
+  auto file = llvm::make_unique<HexagonRuntimeFile<ELFT>>(_ctx);
+  file->addAbsoluteAtom("_SDA_BASE_");
+  if (this->_ctx.isDynamic()) {
+    file->addAbsoluteAtom("_GLOBAL_OFFSET_TABLE_");
+    file->addAbsoluteAtom("_DYNAMIC");
+  }
+  result.push_back(std::move(file));
 }
 
 template <class ELFT>
 void HexagonExecutableWriter<ELFT>::finalizeDefaultAtomValues() {
   // Finalize the atom values that are part of the parent.
   ExecutableWriter<ELFT>::finalizeDefaultAtomValues();
   AtomLayout *sdabaseAtom = _targetLayout.findAbsoluteAtom("_SDA_BASE_");
   sdabaseAtom->_virtualAddr = _targetLayout.getSDataSection()->virtualAddr();
   if (_ctx.isDynamic())
     finalizeHexagonRuntimeAtomValues(_targetLayout);
 }
 
 } // namespace elf
 } // namespace lld
 
 #endif // HEXAGON_EXECUTABLE_WRITER_H

lib/ReaderWriter/ELF/OutputELFWriter.h

protected:
 
   // Build the section header table
   virtual void buildSectionHeaderTable();
 
   // Assign sections that have no segments such as the symbol table,
   // section header table, string table etc
   virtual void assignSectionsWithNoSegments();
 
-  // Add default atoms that need to be present in the output file
-  virtual void addDefaultAtoms();
-
   // Add any runtime files and their atoms to the output
   void createImplicitFiles(std::vector<std::unique_ptr<File>> &) override;
 
   // Finalize the default atom values
   virtual void finalizeDefaultAtomValues();
 
   // This is called by the write section to apply relocations
   uint64_t addressOfAtom(const Atom *atom) override {
protected:
   /// \name Dynamic sections.
   /// @{
   unique_bump_ptr<DynamicTable<ELFT>> _dynamicTable;
   unique_bump_ptr<DynamicSymbolTable<ELFT>> _dynamicSymbolTable;
   unique_bump_ptr<StringTable<ELFT>> _dynamicStringTable;
   unique_bump_ptr<HashSection<ELFT>> _hashTable;
   llvm::StringSet<> _soNeeded;
   /// @}
-  std::unique_ptr<RuntimeFile<ELFT>> _scriptFile;
 
 private:
   static StringRef maybeGetSOName(Node *node);
 };
 
 //===----------------------------------------------------------------------===//
 //  OutputELFWriter
 //===----------------------------------------------------------------------===//
 template <class ELFT>
 OutputELFWriter<ELFT>::OutputELFWriter(ELFLinkingContext &ctx,
                                        TargetLayout<ELFT> &layout)
-    : _ctx(ctx), _targetHandler(ctx.getTargetHandler()), _layout(layout),
-      _scriptFile(new RuntimeFile<ELFT>(ctx, "Linker script runtime")) {}
+    : _ctx(ctx), _targetHandler(ctx.getTargetHandler()), _layout(layout) {}
 
 template <class ELFT>
 void OutputELFWriter<ELFT>::buildChunks(const File &file) {
   ScopedTask task(getDefaultDomain(), "buildChunks");
   for (const DefinedAtom *definedAtom : file.defined()) {
     DefinedAtom::ContentType contentType = definedAtom->contentType();
     // Dont add COMDAT group atoms and GNU linkonce atoms, as they are used for
     // symbol resolution.
void OutputELFWriter<ELFT>::assignSectionsWithNoSegments() {
   }
   _layout.assignFileOffsetsForMiscSections();
   for (auto sec : _layout.sections())
     if (auto section = dyn_cast<Section<ELFT>>(sec))
       if (!TargetLayout<ELFT>::hasOutputSegment(section))
         _shdrtab->updateSection(section);
 }
 
-template <class ELFT> void OutputELFWriter<ELFT>::addDefaultAtoms() {
-  const llvm::StringSet<> &symbols =
-      _ctx.linkerScriptSema().getScriptDefinedSymbols();
-  for (auto &sym : symbols)
-    _scriptFile->addAbsoluteAtom(sym.getKey());
-}
-
 template <class ELFT>
 void OutputELFWriter<ELFT>::createImplicitFiles(
     std::vector<std::unique_ptr<File>> &result) {
   // Add the virtual archive to resolve undefined symbols.
   // The file will be added later in the linking context.
   auto callback = [this](StringRef sym, RuntimeFile<ELFT> &file) {
     processUndefinedSymbol(sym, file);
   };
   _ctx.setUndefinesResolver(
       llvm::make_unique<DynamicSymbolFile<ELFT>>(_ctx, std::move(callback)));
   // Add script defined symbols
-  result.push_back(std::move(_scriptFile));
+  auto file =
+      llvm::make_unique<RuntimeFile<ELFT>>(_ctx, "Linker script runtime");
+  for (auto &sym : this->_ctx.linkerScriptSema().getScriptDefinedSymbols())
+    file->addAbsoluteAtom(sym.getKey());
+  result.push_back(std::move(file));
 }
 
 template <class ELFT>
 void OutputELFWriter<ELFT>::finalizeDefaultAtomValues() {
   const llvm::StringSet<> &symbols =
       _ctx.linkerScriptSema().getScriptDefinedSymbols();
   for (auto &sym : symbols) {
     uint64_t res =

lib/ReaderWriter/ELF/X86/X86DynamicLibraryWriter.h

 protected:
   // Add any runtime files and their atoms to the output
   void createImplicitFiles(std::vector<std::unique_ptr<File>> &) override;
 
   void finalizeDefaultAtomValues() override {
     return DynamicLibraryWriter<ELFT>::finalizeDefaultAtomValues();
   }
 
-  void addDefaultAtoms() override {
-    return DynamicLibraryWriter<ELFT>::addDefaultAtoms();
-  }
-
 private:
   class GOTFile : public SimpleFile {
   public:
     GOTFile(const ELFLinkingContext &eti) : SimpleFile("GOTFile") {}
     llvm::BumpPtrAllocator _alloc;
   };
 
   std::unique_ptr<GOTFile> _gotFile;

lib/ReaderWriter/ELF/X86/X86ExecutableWriter.h

 protected:
   // Add any runtime files and their atoms to the output
   void createImplicitFiles(std::vector<std::unique_ptr<File>> &) override;
 
   void finalizeDefaultAtomValues() override {
     return ExecutableWriter<ELFT>::finalizeDefaultAtomValues();
   }
 
-  void addDefaultAtoms() override {
-    return ExecutableWriter<ELFT>::addDefaultAtoms();
-  }
-
 private:
   X86LinkingContext &_ctx;
   TargetLayout<ELFT> &_layout;
 };
 
 template <class ELFT>
 X86ExecutableWriter<ELFT>::X86ExecutableWriter(X86LinkingContext &ctx,
                                                TargetLayout<ELFT> &layout)

lib/ReaderWriter/ELF/X86_64/X86_64DynamicLibraryWriter.h

 protected:
   // Add any runtime files and their atoms to the output
   void createImplicitFiles(std::vector<std::unique_ptr<File>> &) override;
 
   void finalizeDefaultAtomValues() override {
     return DynamicLibraryWriter::finalizeDefaultAtomValues();
   }
 
-  void addDefaultAtoms() override {
-    return DynamicLibraryWriter::addDefaultAtoms();
-  }
-
 private:
   class GOTFile : public SimpleFile {
   public:
     GOTFile(const ELFLinkingContext &eti) : SimpleFile("GOTFile") {}
     llvm::BumpPtrAllocator _alloc;
   };
 
   std::unique_ptr<GOTFile> _gotFile;

lib/ReaderWriter/ELF/X86_64/X86_64ExecutableWriter.h

if (_ctx.isDynamic())
       _gotFile->addAtom(*new (_gotFile->_alloc) DynamicAtom(*_gotFile));
     result.push_back(std::move(_gotFile));
   }
 
   void finalizeDefaultAtomValues() override {
     return ExecutableWriter::finalizeDefaultAtomValues();
   }
 
-  void addDefaultAtoms() override {
-    return ExecutableWriter::addDefaultAtoms();
-  }
-
 private:
   class GOTFile : public SimpleFile {
   public:
     GOTFile(const ELFLinkingContext &eti) : SimpleFile("GOTFile") {}
     llvm::BumpPtrAllocator _alloc;
   };
 
   std::unique_ptr<GOTFile> _gotFile;
   X86_64LinkingContext &_ctx;
 };
 
 } // namespace elf
 } // namespace lld
 
 #endif