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

Igor Kudrin via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 19 11:24:19 PDT 2015

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.


More information about the llvm-commits mailing list