[PATCH] D83037: [llvm-readobj] - Fix a crash scenario in GNUStyle<ELFT>::printHashSymbols().

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/hash-symbols.test:517
+##         from the .hash table. The number of symbols in the dynamic symbol table can be calculated from its size
+##         or derived from the Chain vector of the .hash table. Make both ways to produce a zero to do the check.
+# RUN: yaml2obj --docnum=9 %s -o %t9.1.so
----------------
to produce -> return

(I think it reads cleaner that way. It would be just "produce" anyway).


================
Comment at: llvm/tools/llvm-readobj/ELFDumper.cpp:4047
 
+    const Elf_Sym *FirstSym = DynSyms.empty() ? nullptr : &DynSyms[0];
+    if (!FirstSym) {
----------------
I'm not sure if this does this, but I think we need to distinguish between the dynsym being actually empty (i.e. sh_size == 0 specified by the .dynsym section header) and we just don't know it's empty (i.e. there's no section header at all, but there is a DT_SYMTAB tag). See rGb259ce99 for context. If this code already does that, what in the test ensures we haven't broken this situation? I'm having a bit of a struggle remembering how that change worked, so this might not be relevant at all.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83037/new/

https://reviews.llvm.org/D83037