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

Rafael Ávila de Espíndola via llvm-commits llvm-commits at lists.llvm.org
Sun Oct 18 07:03:00 PDT 2015


rafael 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();
----------------
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.



================
Comment at: ELF/OutputSections.h:48
@@ -46,3 +47,3 @@
 
 template <class ELFT>
 bool shouldKeepInSymtab(
----------------
Optimizing the sysv hash is a good thing, but lets do it in another patch (before or after this one).

================
Comment at: ELF/OutputSections.h:152
@@ -150,3 +151,3 @@
   void writeLocalSymbols(uint8_t *&Buf);
-  void writeGlobalSymbols(uint8_t *&Buf);
+  void writeGlobalSymbols(uint8_t *Buf);
 
----------------
Independent change, no? If so, please commit it and rebase.

================
Comment at: ELF/OutputSections.h:299
@@ -266,3 +298,3 @@
   static GotSection<ELFT> *Got;
-  static HashTableSection<ELFT> *HashTab;
+  static HashTableSection<ELFT> *SysvHashTab;
   static InterpSection<ELFT> *Interp;
----------------
Most of these members are named after the section name in the output file, so I would keep the old name.


http://reviews.llvm.org/D13815





More information about the llvm-commits mailing list