This is an archive of the discontinued LLVM Phabricator instance.

[llvm-readobj/elf] - Simplify and refine the implementation which dumps .stack_sizes
ClosedPublic

Authored by grimar on Sep 9 2020, 4:31 AM.

Details

Summary

Our implementation of stack sizes section dumping heavily uses ELFObjectFile<ELFT>,
while the rest of the code uses ELFFile<ELFT>.

That APIs are very different. ELFObjectFile<ELFT> is very generic
and has SectionRef, RelocationRef, SymbolRef and other generic concepts.
The ELFFile<ELFT> class works directly with Elf_Shdr, Elf_Rel[a], Elf_Sym etc,
what is probably much cleaner for ELF dumper.

Also, ELFObjectFile<ELFT> API does not always provide a way to check
for possible errors. E.g. the implementation of symbol_end() does not verify the sh_size:

template <class ELFT>
basic_symbol_iterator ELFObjectFile<ELFT>::symbol_end() const {
  const Elf_Shdr *SymTab = DotSymtabSec;
  if (!SymTab)
    return symbol_begin();
  DataRefImpl Sym = toDRI(SymTab, SymTab->sh_size / sizeof(Elf_Sym));
  return basic_symbol_iterator(SymbolRef(Sym, this));
}

There are many other examples which makes me thing we might win from
switching to ELFFile<ELFT> API, where we heavily validate an input data already.

This patch is the first step in this direction. I've converted the large portion of the code
to use ELFFile<ELFT>.

Diff Detail

Event Timeline

grimar created this revision.Sep 9 2020, 4:31 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 9 2020, 4:31 AM
Herald added a subscriber: rupprecht. · View Herald Transcript
grimar requested review of this revision.Sep 9 2020, 4:31 AM
jhenderson accepted this revision.Sep 15 2020, 12:45 AM

LGTM, with nit.

llvm/tools/llvm-readobj/ELFDumper.cpp
5761–5764

Nit: I prefer the old name, since it's more descriptive.

This revision is now accepted and ready to land.Sep 15 2020, 12:45 AM
This revision was automatically updated to reflect the committed changes.
grimar marked an inline comment as done.