[PATCH] [ELF][Writer] Refactor Section to not have atoms. Move atoms into AtomSection.

[Shankar Kalpathi Easwaran]

================
Comment at: lib/ReaderWriter/ELF/DefaultLayout.h:159-161
@@ -158,5 +158,5 @@
   /// \brief Returns the section to be created
-  virtual Section<ELFT> *getSection(StringRef name, const int32_t contentType,
-                                    const int32_t contentPermissions,
-                                    const int32_t sectionOrder);
+  virtual AtomSection<ELFT> *getSection(
+      StringRef name, int32_t contentType,
+      DefinedAtom::ContentPermissions contentPermissions);
 
----------------
I was thinking that we could have the function take DefinedAtom here, as the function need to access other attributes of the DefinedAtom in the future.

================
Comment at: lib/ReaderWriter/ELF/DefaultLayout.h:159-161
@@ -158,5 +158,5 @@
   /// \brief Returns the section to be created
-  virtual Section<ELFT> *getSection(StringRef name, const int32_t contentType,
-                                    const int32_t contentPermissions,
-                                    const int32_t sectionOrder);
+  virtual AtomSection<ELFT> *getSection(
+      StringRef name, int32_t contentType,
+      DefinedAtom::ContentPermissions contentPermissions);
 
----------------
Shankar Kalpathi Easwaran wrote:
> I was thinking that we could have the function take DefinedAtom here, as the function need to access other attributes of the DefinedAtom in the future.
This also needs corresponding changes in the ELFTargetHandler.

================
Comment at: lib/ReaderWriter/ELF/DefaultLayout.h:415-431
@@ -418,9 +414,19 @@
 
 template <class ELFT>
-Section<ELFT> *DefaultLayout<ELFT>::getSection(
-    StringRef sectionName, int32_t contentType, int32_t permissions,
-    int32_t sectionOrder) {
-  return new (_allocator) Section<ELFT>(_targetInfo, sectionName, contentType,
-                                        permissions, sectionOrder);
+AtomSection<ELFT> *DefaultLayout<ELFT>::getSection(
+    StringRef sectionName, int32_t contentType,
+    DefinedAtom::ContentPermissions permissions) {
+  const SectionKey sectionKey(sectionName, permissions);
+  auto sec = _sectionMap.find(sectionKey);
+  if (sec != _sectionMap.end())
+    return sec->second;
+  SectionOrder sectionOrder =
+      getSectionOrder(sectionName, contentType, permissions);
+  AtomSection<ELFT> *newSec = new (_allocator) AtomSection<ELFT>(
+      _targetInfo, sectionName, contentType, permissions, sectionOrder);
+  newSec->setOrder(sectionOrder);
+  _sections.push_back(newSec);
+  _sectionMap.insert(std::make_pair(sectionKey, newSec));
+  return newSec;
 }
 
----------------
I would prefer this function just create the section, as targets can just override this function and just create whatever section they want. It would be hard for targets to add sections this way as they need to create the section and add ito the _sections, and the _sectionMap, which makes it harder.

================
Comment at: lib/ReaderWriter/ELF/SectionChunks.h:113-147
@@ +112,37 @@
+        _contentType(contentType), _contentPermissions(permissions) {
+    this->setOrder(order);
+    switch (contentType) {
+    case DefinedAtom::typeCode:
+    case DefinedAtom::typeData:
+    case DefinedAtom::typeConstant:
+    case DefinedAtom::typeGOT:
+    case DefinedAtom::typeStub:
+    case DefinedAtom::typeResolver:
+    case DefinedAtom::typeTLVInitialData:
+      this->_type = SHT_PROGBITS;
+      break;
+    case DefinedAtom::typeZeroFill:
+    case DefinedAtom::typeTLVInitialZeroFill:
+      this->_type = SHT_NOBITS;
+      break;
+    }
+
+    switch (permissions) {
+    case DefinedAtom::permR__:
+      this->_flags = SHF_ALLOC;
+      break;
+    case DefinedAtom::permR_X:
+      this->_flags = SHF_ALLOC | SHF_EXECINSTR;
+      break;
+    case DefinedAtom::permRW_:
+    case DefinedAtom::permRW_L:
+      this->_flags = SHF_ALLOC | SHF_WRITE;
+      if (_contentType == DefinedAtom::typeTLVInitialData ||
+          _contentType == DefinedAtom::typeTLVInitialZeroFill)
+        this->_flags |= SHF_TLS;
+      break;
+    case DefinedAtom::permRWX:
+      this->_flags = SHF_ALLOC | SHF_WRITE | SHF_EXECINSTR;
+      break;
+    }
+  }
----------------
Can we have these as two seperate functions, so that targets can override functionality which derive from AtomSection, and they just want to set different flags.

================
Comment at: lib/ReaderWriter/ELF/Chunk.h:40
@@ -39,2 +39,3 @@
     K_ELFSegment, // Segment
     K_ELFSection, // Section
+    K_AtomSection, //< A section containing atoms.
----------------
Is K_ELFSection going to be set for sections where the sections dont have atoms in them ?


http://llvm-reviews.chandlerc.com/D400