[ELF] Simplify dynamic table entry creation

[davide]

Summary

It removes some amount of code.

Mips/MipsDynamicTable.h | 35 ++++---------------
OutputELFWriter.cpp | 24 ++++---------
SectionChunks.cpp | 88 ++++++++++++++++++------------------------------
SectionChunks.h | 2 -
4 files changed, 50 insertions(+), 99 deletions(-)

[atanasyan]

LGTM

lib/ReaderWriter/ELF/Mips/MipsDynamicTable.h

37

I think we can initialize d_val field here instead of d_ptr and remove the third argument from the addEntry routine.

[davide]

lib/ReaderWriter/ELF/Mips/MipsDynamicTable.h

37

I was tempted to do so, I honestly dislike more verbose API, but my lack of familiarity with the MIPS backend stopped me. Thanks.

lib/ReaderWriter/ELF/Mips/MipsDynamicTable.h

public:
   MipsDynamicTable(const ELFLinkingContext &ctx, MipsTargetLayout<ELFT> &layout)
       : DynamicTable<ELFT>(ctx, layout, ".dynamic",
                            TargetLayout<ELFT>::ORDER_DYNAMIC),
         _targetLayout(layout) {}
 
   void createDefaultEntries() override {
     DynamicTable<ELFT>::createDefaultEntries();
 
-    typename DynamicTable<ELFT>::Elf_Dyn dyn;
-
     // Version id for the Runtime Linker Interface.
-    dyn.d_un.d_val = 1;
-    dyn.d_tag = DT_MIPS_RLD_VERSION;
-    this->addEntry(dyn);
+    this->addEntry(DT_MIPS_RLD_VERSION, 1);
 
     // MIPS flags.
-    dyn.d_un.d_val = RHF_NOTPOT;
-    dyn.d_tag = DT_MIPS_FLAGS;
-    this->addEntry(dyn);
+    this->addEntry(DT_MIPS_FLAGS, RHF_NOTPOT);
 
     // The base address of the segment.
-    dyn.d_un.d_ptr = 0;
-    dyn.d_tag = DT_MIPS_BASE_ADDRESS;
-    _dt_baseaddr = this->addEntry(dyn);
+    _dt_baseaddr = this->addEntry(DT_MIPS_BASE_ADDRESS, 0, false);
 
     // Number of local global offset table entries.
-    dyn.d_un.d_val = 0;
-    dyn.d_tag = DT_MIPS_LOCAL_GOTNO;
-    _dt_localgot = this->addEntry(dyn);
+    _dt_localgot = this->addEntry(DT_MIPS_LOCAL_GOTNO, 0);
 
     // Number of entries in the .dynsym section.
-    dyn.d_un.d_val = 0;
-    dyn.d_tag = DT_MIPS_SYMTABNO;
-    _dt_symtabno = this->addEntry(dyn);
+    _dt_symtabno = this->addEntry(DT_MIPS_SYMTABNO, 0);
 
     // The index of the first dynamic symbol table entry that corresponds
     // to an entry in the global offset table.
-    dyn.d_un.d_val = 0;
-    dyn.d_tag = DT_MIPS_GOTSYM;
-    _dt_gotsym = this->addEntry(dyn);
+    _dt_gotsym = this->addEntry(DT_MIPS_GOTSYM, 0);
 
     // Address of the .got section.
-    dyn.d_un.d_val = 0;
-    dyn.d_tag = DT_PLTGOT;
-    _dt_pltgot = this->addEntry(dyn);
+    _dt_pltgot = this->addEntry(DT_PLTGOT, 0);
   }
 
   void doPreFlight() override {
     DynamicTable<ELFT>::doPreFlight();
 
     if (_targetLayout.findOutputSection(".MIPS.options")) {
-      typename DynamicTable<ELFT>::Elf_Dyn dyn;
-      dyn.d_un.d_val = 0;
-      dyn.d_tag = DT_MIPS_OPTIONS;
-      _dt_options = this->addEntry(dyn);
+      _dt_options = this->addEntry(DT_MIPS_OPTIONS, 0);
     }
   }
 
   void updateDynamicTable() override {
     DynamicTable<ELFT>::updateDynamicTable();
 
     // Assign the minimum segment address to the DT_MIPS_BASE_ADDRESS tag.
     auto baseAddr = std::numeric_limits<uint64_t>::max();

lib/ReaderWriter/ELF/OutputELFWriter.cpp

void OutputELFWriter<ELFT>::buildDynamicSymbolTable(const File &file) {
   }
   for (const std::unique_ptr<Node> &node : _ctx.getNodes()) {
     StringRef soname = maybeGetSOName(node.get());
     if (!soname.empty())
       _soNeeded.insert(soname);
   }
   // Never mark the dynamic linker as DT_NEEDED
   _soNeeded.erase(sys::path::filename(_ctx.getInterpreter()));
-  for (const auto &loadName : _soNeeded) {
-    Elf_Dyn dyn;
-    dyn.d_tag = DT_NEEDED;
-    dyn.d_un.d_val = _dynamicStringTable->addString(loadName.getKey());
-    _dynamicTable->addEntry(dyn);
-  }
+  for (const auto &loadName : _soNeeded)
+    _dynamicTable->addEntry(DT_NEEDED,
+                            _dynamicStringTable->addString(loadName.getKey()));
   const auto &rpathList = _ctx.getRpathList();
   if (!rpathList.empty()) {
     auto rpath =
         new (_alloc) std::string(join(rpathList.begin(), rpathList.end(), ":"));
-    Elf_Dyn dyn;
-    dyn.d_tag = _ctx.getEnableNewDtags() ? DT_RUNPATH : DT_RPATH;
-    dyn.d_un.d_val = _dynamicStringTable->addString(*rpath);
-    _dynamicTable->addEntry(dyn);
+    _dynamicTable->addEntry(_ctx.getEnableNewDtags() ? DT_RUNPATH : DT_RPATH,
+                            _dynamicStringTable->addString(*rpath));
   }
   StringRef soname = _ctx.sharedObjectName();
-  if (!soname.empty() && _ctx.getOutputELFType() == llvm::ELF::ET_DYN) {
-    Elf_Dyn dyn;
-    dyn.d_tag = DT_SONAME;
-    dyn.d_un.d_val = _dynamicStringTable->addString(soname);
-    _dynamicTable->addEntry(dyn);
-  }
+  if (!soname.empty() && _ctx.getOutputELFType() == llvm::ELF::ET_DYN)
+    _dynamicTable->addEntry(DT_SONAME, _dynamicStringTable->addString(soname));
+
   // The dynamic symbol table need to be sorted earlier because the hash
   // table needs to be built using the dynamic symbol table. It would be
   // late to sort the symbols due to that in finalize. In the dynamic symbol
   // table finalize, we call the symbol table finalize and we don't want to
   // sort again
   _dynamicSymbolTable->sortSymbols();
 
   // Add the dynamic symbols into the hash table

lib/ReaderWriter/ELF/SectionChunks.h

public:
   typedef std::vector<Elf_Dyn> EntriesT;
 
   DynamicTable(const ELFLinkingContext &ctx, TargetLayout<ELFT> &layout,
                StringRef str, int32_t order);
 
   range<typename EntriesT::iterator> entries() { return _entries; }
 
   /// \returns the index of the entry.
-  std::size_t addEntry(Elf_Dyn e);
+  std::size_t addEntry(int64_t tag, uint64_t val, bool isVal = true);
 
   void write(ELFWriter *writer, TargetLayout<ELFT> &layout,
              llvm::FileOutputBuffer &buffer) override;
 
   virtual void createDefaultEntries();
   void doPreFlight() override;
 
   /// \brief Dynamic table tag for .got.plt section referencing.

lib/ReaderWriter/ELF/SectionChunks.cpp

   this->_alignment = ELFT::Is64Bits ? 8 : 4;
   // Reserve space for the DT_NULL entry.
   this->_fsize = sizeof(Elf_Dyn);
   this->_msize = sizeof(Elf_Dyn);
   this->_type = SHT_DYNAMIC;
   this->_flags = SHF_ALLOC;
 }
 
-template <class ELFT> std::size_t DynamicTable<ELFT>::addEntry(Elf_Dyn e) {
-  _entries.push_back(e);
+template <class ELFT>
+std::size_t DynamicTable<ELFT>::addEntry(int64_t tag,
+                                         uint64_t valOrPtr,
+                                         bool isVal) {
+  Elf_Dyn dyn;
+  dyn.d_tag = tag;
+  if (isVal)
+    dyn.d_un.d_val = valOrPtr;
+  else
+    dyn.d_un.d_ptr = valOrPtr;
+  _entries.push_back(dyn);
   this->_fsize = (_entries.size() * sizeof(Elf_Dyn)) + sizeof(Elf_Dyn);
   this->_msize = this->_fsize;
   return _entries.size() - 1;
 }
 
 template <class ELFT>
 void DynamicTable<ELFT>::write(ELFWriter *writer, TargetLayout<ELFT> &layout,
                                llvm::FileOutputBuffer &buffer) {
   uint8_t *chunkBuffer = buffer.getBufferStart();
   uint8_t *dest = chunkBuffer + this->fileOffset();
   // Add the null entry.
   Elf_Dyn d;
   d.d_tag = 0;
   d.d_un.d_val = 0;
   _entries.push_back(d);
   std::memcpy(dest, _entries.data(), this->_fsize);
 }
 
 template <class ELFT> void DynamicTable<ELFT>::createDefaultEntries() {
   bool isRela = this->_ctx.isRelaOutputFormat();
-
-  Elf_Dyn dyn;
-  dyn.d_un.d_val = 0;
-
-  dyn.d_tag = DT_HASH;
-  _dt_hash = addEntry(dyn);
-  dyn.d_tag = DT_STRTAB;
-  _dt_strtab = addEntry(dyn);
-  dyn.d_tag = DT_SYMTAB;
-  _dt_symtab = addEntry(dyn);
-  dyn.d_tag = DT_STRSZ;
-  _dt_strsz = addEntry(dyn);
-  dyn.d_tag = DT_SYMENT;
-  _dt_syment = addEntry(dyn);
+  _dt_hash = addEntry(DT_HASH, 0);
+  _dt_strtab = addEntry(DT_STRTAB, 0);
+  _dt_symtab = addEntry(DT_SYMTAB, 0);
+  _dt_strsz = addEntry(DT_STRSZ, 0);
+  _dt_syment = addEntry(DT_SYMENT, 0);
   if (_layout.hasDynamicRelocationTable()) {
-    dyn.d_tag = isRela ? DT_RELA : DT_REL;
-    _dt_rela = addEntry(dyn);
-    dyn.d_tag = isRela ? DT_RELASZ : DT_RELSZ;
-    _dt_relasz = addEntry(dyn);
-    dyn.d_tag = isRela ? DT_RELAENT : DT_RELENT;
-    _dt_relaent = addEntry(dyn);
-
-    if (_layout.getDynamicRelocationTable()->canModifyReadonlySection()) {
-      dyn.d_tag = DT_TEXTREL;
-      _dt_textrel = addEntry(dyn);
-    }
+    _dt_rela = addEntry(isRela ? DT_RELA : DT_REL, 0);
+    _dt_relasz = addEntry(isRela ? DT_RELASZ : DT_RELSZ, 0);
+    _dt_relaent = addEntry(isRela ? DT_RELAENT : DT_RELENT, 0);
+    if (_layout.getDynamicRelocationTable()->canModifyReadonlySection())
+      _dt_textrel = addEntry(DT_TEXTREL, 0);
   }
   if (_layout.hasPLTRelocationTable()) {
-    dyn.d_tag = DT_PLTRELSZ;
-    _dt_pltrelsz = addEntry(dyn);
-    dyn.d_tag = getGotPltTag();
-    _dt_pltgot = addEntry(dyn);
-    dyn.d_tag = DT_PLTREL;
-    dyn.d_un.d_val = isRela ? DT_RELA : DT_REL;
-    _dt_pltrel = addEntry(dyn);
-    dyn.d_un.d_val = 0;
-    dyn.d_tag = DT_JMPREL;
-    _dt_jmprel = addEntry(dyn);
+    _dt_pltrelsz = addEntry(DT_PLTRELSZ, 0);
+    _dt_pltgot = addEntry(getGotPltTag(), 0);
+    _dt_pltrel = addEntry(DT_PLTREL, isRela ? DT_RELA : DT_REL);
+    _dt_jmprel = addEntry(DT_JMPREL, 0);
   }
 }
 
 template <class ELFT> void DynamicTable<ELFT>::doPreFlight() {
-  Elf_Dyn dyn;
-  dyn.d_un.d_val = 0;
   auto initArray = _layout.findOutputSection(".init_array");
   auto finiArray = _layout.findOutputSection(".fini_array");
   if (initArray) {
-    dyn.d_tag = DT_INIT_ARRAY;
-    _dt_init_array = addEntry(dyn);
-    dyn.d_tag = DT_INIT_ARRAYSZ;
-    _dt_init_arraysz = addEntry(dyn);
+    _dt_init_array = addEntry(DT_INIT_ARRAY, 0);
+    _dt_init_arraysz = addEntry(DT_INIT_ARRAYSZ, 0);
   }
   if (finiArray) {
-    dyn.d_tag = DT_FINI_ARRAY;
-    _dt_fini_array = addEntry(dyn);
-    dyn.d_tag = DT_FINI_ARRAYSZ;
-    _dt_fini_arraysz = addEntry(dyn);
-  }
-  if (getInitAtomLayout()) {
-    dyn.d_tag = DT_INIT;
-    _dt_init = addEntry(dyn);
-  }
-  if (getFiniAtomLayout()) {
-    dyn.d_tag = DT_FINI;
-    _dt_fini = addEntry(dyn);
+    _dt_fini_array = addEntry(DT_FINI_ARRAY, 0);
+    _dt_fini_arraysz = addEntry(DT_FINI_ARRAYSZ, 0);
   }
+  if (getInitAtomLayout())
+    _dt_init = addEntry(DT_INIT, 0);
+  if (getFiniAtomLayout())
+    _dt_fini = addEntry(DT_FINI, 0);
 }
 
 template <class ELFT> void DynamicTable<ELFT>::finalize() {
   StringTable<ELFT> *dynamicStringTable = _dynamicSymbolTable->getStringTable();
   this->_link = dynamicStringTable->ordinal();
   if (this->_outputSection) {
     this->_outputSection->setType(this->_type);
     this->_outputSection->setInfo(this->_info);