[PATCH] D13815: [ELF2] Add support for Gnu Hash section

[Igor Kudrin via llvm-commits]

ikudrin added inline comments.

================
Comment at: ELF/OutputSections.cpp:194
@@ -193,3 +195,1 @@
 
-template <class ELFT> void HashTableSection<ELFT>::addSymbol(SymbolBody *S) {
-  StringRef Name = S->getName();
----------------
rafael wrote:
> This is being removed because the symbols have to be sorted, right? 
> 
> It is a bit untidy that we now call setDynamicSymbolTableIndex from two places.
> 
> Maybe organize this as: 
> 
> * In the current loop:
>    Out<ELFT>::DynSymTab->addSymbol(...)
> * Sort the DynSymTab if needed.
> * Walk the DynSymTab, calling setDynamicSymbolTableIndex and adding the symbol to one or both hash tables.
> 
> 
I consider the main reason to remove this function as it is not a hash table's responsibility to fill up a dynamic symbol table.

I like the idea to sort entries of a symbol table within the symbol table class, but in case of the Gnu hash section, it imposes not only some requirements to the order of entries, but also the oreder itself, as it is calculated using hash values and NBuckets parameter. I can't find a better solution than sorting the symbols which belongs to Gnu Hash table within the GnuHashTableSection class.


http://reviews.llvm.org/D13815