[PATCH] D75756: [llvm-objdump] Teach `llvm-objdump` dump dynamic symbols.

[Xing via Phabricator via llvm-commits]

Higuoxing marked an inline comment as done.
Higuoxing added inline comments.


================
Comment at: llvm/include/llvm/Object/ELFObjectFile.h:1024
+      toDRI(DotDynSymSec,
+            DotDynSymSec && DotDynSymSec->sh_size >= sizeof(Elf_Sym) ? 1 : 0);
   return symbol_iterator(SymbolRef(Sym, this));
----------------
Higuoxing wrote:
> grimar wrote:
> > "Index 0 in any symbol table is used to represent undefined symbols.
> > This first entry in a symbol table is always completely zeroed." (Oracle ELF spec)
> > A null symbol is also a symbol, so it is not a correct change.
> > 
> > The change you want to do should not be here, in a higher level library.
> > 
> > Also, did you run llvm tests? I'd expect them should fail with such change.
> I've run tests locally, and no failure is observed. I also checked function call (`getDynamicSymbolIterators()`). [link](https://github.com/llvm/llvm-project/search?q=getDynamicSymbolIterators&unscoped_q=getDynamicSymbolIterators)
> 
> * llvm/tools/llvm-nm/llvm-nm.cpp: There's an attempt to omit NULL symbol in D62148. @MaskRay helps change the behavior of `symbol_begin()` in `D62296`. I think `dynamic_symbol_begin()` should be changed as well.
> * For calls in lib/Object/SymbolSize.cpp, I think this is similar to https://reviews.llvm.org/D62296#1514082
I have opened a new revision for this change in D76081 : )


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75756