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

[Igor Kudrin via llvm-commits]

ikudrin added inline comments.

================
Comment at: ELF/OutputSections.cpp:241
@@ +240,3 @@
+      sizeof(Elf_Word) == sizeof(Elf_Off) ? sizeof(Elf_Word) : 0;
+  this->Header.sh_addralign = sizeof(Elf_Off);
+}
----------------
ruiu wrote:
> Can you then use uintX_t instead of Elf_Off? Elf_Off is, strictly speaking, probably better type, but I think in practice we don't want to distinguish them.
I don't think sizeof(uintX_t) is relevant here as we don't use that type to store values. Maybe it's better to use (Is64Bins ? 8 : 4) to be in sync with the previous line.

================
Comment at: ELF/OutputSections.h:150
@@ -147,1 +149,3 @@
   unsigned getNumSymbols() const { return NumVisible + 1; }
+  unsigned getNumHashed() const { return NumHashed; }
+
----------------
ruiu wrote:
> This name doesn't feel a correct one. Does this return the number of dynamically-linked symbols? Does this return the GNU-style hash table size?
Is "getNumGnuHashed()" a better variant? Any suggestions are welcome.


http://reviews.llvm.org/D13815