[PATCH] D30494: [ELF] - Do not list sections explicitly for finalizeSynthetic()

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: ELF/Writer.cpp:1023-1028
+template <class ELFT> static void finalizeSynthetic(SyntheticSection *SS) {
+  if (!SS || !SS->OutSec || SS->empty())
+    return;
+  SS->finalizeContents();
+  SS->OutSec->template assignOffsets<ELFT>();
+}
----------------
ruiu wrote:
> This function probably should go away. You shouldn't call assignOffsets every time you finalize one synthetic section.
I think so. That is for another patch, this one did not change original logic here.


================
Comment at: ELF/Writer.cpp:1030-1033
+// Dynamic section must be the last one in this list and dynamic
+// symbol table section (DynSymTab) must be the first one. Dynamic
+// string table should go after others because some sections may
+// add strings there during finalizing.
----------------
ruiu wrote:
> This comment doesn't seem to make much sense. You want to mention that dynamic symbols depends on other section contents so they need to be finalized later.
Fixed.


================
Comment at: ELF/Writer.cpp:1035-1038
+  auto IsSpecial = [](SyntheticSection *SS) {
+    return SS == In<ELFT>::DynSymTab || SS == In<ELFT>::Dynamic ||
+           SS == In<ELFT>::DynStrTab;
+  };
----------------
ruiu wrote:
> Remove this lambda and inline.
Done.


================
Comment at: ELF/Writer.cpp:1176
 
-  // Dynamic section must be the last one in this list and dynamic
-  // symbol table section (DynSymTab) must be the first one.
-  finalizeSynthetic<ELFT>(
-      {In<ELFT>::DynSymTab, In<ELFT>::GnuHashTab, In<ELFT>::HashTab,
-       In<ELFT>::SymTab,    In<ELFT>::ShStrTab,   In<ELFT>::StrTab,
-       In<ELFT>::VerDef,    In<ELFT>::DynStrTab,  In<ELFT>::GdbIndex,
-       In<ELFT>::Got,       In<ELFT>::MipsGot,    In<ELFT>::IgotPlt,
-       In<ELFT>::GotPlt,    In<ELFT>::RelaDyn,    In<ELFT>::RelaIplt,
-       In<ELFT>::RelaPlt,   In<ELFT>::Plt,        In<ELFT>::Iplt,
-       In<ELFT>::Plt,       In<ELFT>::EhFrameHdr, In<ELFT>::VerSym,
-       In<ELFT>::VerNeed,   In<ELFT>::Dynamic});
+  finalizeAllSynthetics<ELFT>();
 }
----------------
ruiu wrote:
> finalizeSyntheticSections.
Done.


https://reviews.llvm.org/D30494