[PATCH] D92021: [llvm-readelf/obj] - Stop using `reportError` when dumping notes.

[George Rimar via Phabricator via llvm-commits]

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


================
Comment at: llvm/test/tools/llvm-readobj/ELF/gnu-notes.test:121
+# ERR1-GNU-NEXT:   Owner                Data size        Description
+# ERR1-GNU-NEXT: warning: '[[FILE]]': unable to read notes from the SHT_NOTE section with index 1: SHT_NOTE section [index 1] has invalid offset (0xffff0000) or size (0x0)
+# ERR1-GNU-NOT: {{.}}
----------------
grimar wrote:
> jhenderson wrote:
> > 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?
> 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.
> 
I've prepared the patch: D92081


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

https://reviews.llvm.org/D92021