[PATCH] D16991: Split the creation of program headers in a few steps

[George Rimar via llvm-commits]

grimar added a comment.

That looks better then my approach I think, thanks ! Have a consern about dummies output sections thought.


================
Comment at: ELF/Writer.cpp:96
@@ +95,3 @@
+  // We createa a section for the elf header and one for the program headers.
+  const unsigned int NumDummySections = 2;
+  ArrayRef<OutputSectionBase<ELFT> *> getSections() const {
----------------
"create"

================
Comment at: ELF/Writer.cpp:103
@@ -82,2 +102,3 @@
+  }
 
   void addRelIpltSymbols();
----------------
Honestly, I dont like the approach to have dummies like that. Thats looks like a hack (more like a hack in compare with 'special' first load). 
Linkerscript allows to put elf header and phdr anywhere afaik. But the question is do we really need to support that ?
In my patch about PHDRS (http://reviews.llvm.org/D16771) I introduced the restriction about that:
first phdr is always PT_PHDR and first load must contain elf and phdr headers. It seems your implementation is really close or the same to mine here (at least it assumes them are at start always), but do we really need to have dummies logic then ?
IMO special first load and less entities looks better here.

================
Comment at: ELF/Writer.cpp:186
@@ -162,2 +185,3 @@
+  Out<ELFT>::ProgramHeaders = &ProgramHeaders;
 
   Writer<ELFT>(*Symtab).run();
----------------
Let's place them before all other sections as they are usually always go before all other ones ?


Repository:
  rL LLVM

http://reviews.llvm.org/D16991