This is an archive of the discontinued LLVM Phabricator instance.

[llvm-readelf/obj] - Stop using `reportError` when dumping notes.
ClosedPublic

Authored by grimar on Nov 24 2020, 4:03 AM.

Details

Summary

This starts using reportUniqueWarnings instead of reportError
in the code that is responsible for dumping notes.

Diff Detail

Event Timeline

grimar created this revision.Nov 24 2020, 4:03 AM
Herald added a project: Restricted Project. · View Herald Transcript
grimar requested review of this revision.Nov 24 2020, 4:03 AM
grimar planned changes to this revision.
grimar updated this revision to Diff 307313.Nov 24 2020, 4:08 AM
  • Updated forgotten test.
jhenderson added inline comments.Nov 24 2020, 5:53 AM
llvm/test/tools/llvm-readobj/ELF/gnu-notes.test
121

The mention of the "SHT_NOTE section [index 1]" happens twice redundantly. Same issue with the program header below. Do you think we could omit some of this redudant context?

grimar marked an inline comment as done.Nov 24 2020, 7:32 AM
grimar added inline comments.
llvm/test/tools/llvm-readobj/ELF/gnu-notes.test
121

The problem is that on one situations notes might report the section index and type:

Elf_Note_Iterator notes_begin(const Elf_Phdr &Phdr, Error &Err) const {
  assert(Phdr.p_type == ELF::PT_NOTE && "Phdr is not of type PT_NOTE");
  ErrorAsOutParameter ErrAsOutParam(&Err);
  if (Phdr.p_offset + Phdr.p_filesz > getBufSize()) {
    Err = createError("PT_NOTE header has invalid offset (0x" +
                      Twine::utohexstr(Phdr.p_offset) + ") or size (0x" +
                      Twine::utohexstr(Phdr.p_filesz) + ")");
    return Elf_Note_Iterator(Err);
  }
  return Elf_Note_Iterator(base() + Phdr.p_offset, Phdr.p_filesz, Err);
}

but in other situations the note iterator might report an error on its own without a context:

class Elf_Note_Iterator_Impl
    : std::iterator<std::forward_iterator_tag, Elf_Note_Impl<ELFT>> {

  // Stop iteration and indicate an overflow.
  void stopWithOverflowError() {
    Nhdr = nullptr;
    *Err = make_error<StringError>("ELF note overflows container",
                                   object_error::parse_failed);
  }

Perhaps, the best solution would be to stop reporting section index/type in Object/ELF.h/notes_begin().
This was introduced by me for llvm-readobj in D64470. Instead we can describe sections/program headers
on the caller side, like we normally do for other warnings/errors.

I'll suggest a patch tomorrow.

grimar marked an inline comment as done.
grimar added inline comments.
llvm/test/tools/llvm-readobj/ELF/gnu-notes.test
121

I've prepared the patch: D92081

grimar updated this revision to Diff 307553.Nov 25 2020, 2:20 AM
grimar edited the summary of this revision. (Show Details)
  • Rebased.
jhenderson accepted this revision.Nov 25 2020, 3:23 AM

LGTM, thanks!

This revision is now accepted and ready to land.Nov 25 2020, 3:23 AM