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

[Rui Ueyama via llvm-commits]

ruiu 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);
+}
----------------
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.

================
Comment at: ELF/OutputSections.cpp:243-244
@@ +242,4 @@
+}
+
+template <class ELFT>
+unsigned GnuHashTableSection<ELFT>::calcNBuckets(unsigned NumHashed) {
----------------
Can you do this?

================
Comment at: ELF/OutputSections.h:150
@@ -147,1 +149,3 @@
   unsigned getNumSymbols() const { return NumVisible + 1; }
+  unsigned getNumHashed() const { return NumHashed; }
+
----------------
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?

================
Comment at: ELF/OutputSections.h:251-253
@@ +250,5 @@
+class GnuHashTableSection final : public OutputSectionBase<ELFT> {
+  typedef typename llvm::object::ELFFile<ELFT>::Elf_Word Elf_Word;
+  typedef typename llvm::object::ELFFile<ELFT>::Elf_Off Elf_Off;
+  typedef typename llvm::object::ELFFile<ELFT>::uintX_t uintX_t;
+
----------------
Sort


http://reviews.llvm.org/D13815