[PATCH] D14084: [ELF2] Move sorting from symbol table to the GNU hash section.

[Rui Ueyama via llvm-commits]

ruiu added inline comments.

================
Comment at: ELF/OutputSections.cpp:349
@@ -351,2 +348,3 @@
 template <class ELFT> void GnuHashTableSection<ELFT>::writeTo(uint8_t *Buf) {
+  assert(NumHashed == HashedSymbols.size() && "sortSymbols() was not called!");
   writeHeader(Buf);
----------------
ikudrin wrote:
> The variable is required in the finalize() method. The HashedSymbols is filled in the sortSymbols() method, which is called from finalize() method of the dynamic symbol table. All finalize() methods should not depend on each other and their call order, I think.
Can you count the number of symbols that need to be in .gnu.hash here, instead of calling addSymbol for each symbol? That can be as easy as

  ArrayRef<Symbol> A = Out<ELFT>::DynSymTab->getSymbols();
  NumHashed = std::count(A.begin(), A.end(), includeInGnuHashTable);

================
Comment at: ELF/OutputSections.cpp:405-406
@@ -409,1 +404,4 @@
 template <class ELFT>
+void GnuHashTableSection<ELFT>::sortSymbols(
+    std::vector<SymbolBody *> &Symbols) {
+  if (!NumHashed)
----------------
I think this function should be called "addSymbols" rather than "sortSymbols". This indeed sorts symbols passed to this function, but that's probably secondaly.

================
Comment at: ELF/OutputSections.h:299
@@ +298,3 @@
+  void addSymbol() { ++NumHashed; }
+  // Updates the order of the dynamic symbol section symbols
+  // according to the GNU hash section requirements.
----------------
Add a blank line here.


http://reviews.llvm.org/D14084