Page MenuHomePhabricator

[libObject,llvm-readelf] - Stop describing a section/segment in `notes_begin()`.
ClosedPublic

Authored by grimar on Nov 25 2020, 1:07 AM.

Details

Summary

notes_begin() is used for iterating over notes. This API in some cases might print
section type and index. At the same time during iterating, the Elf_Note_Iterator
might omit it as it doesn't have this info.

Because of above we might have the redundant duplication of information in warnings:
(See D92021).

warning: '[[FILE]]': unable to read notes from the SHT_NOTE section with index 1: SHT_NOTE section [index 1] has invalid offset (0x40) or size (0xffff0000)

This change stops reporting section index/type in Object/ELF.h/notes_begin().
(FTR, this was introduced by me for llvm-readobj in D64470).
Instead we can describe sections/program headers on the caller side.

Diff Detail

Event Timeline

grimar created this revision.Nov 25 2020, 1:07 AM
Herald added a project: Restricted Project. · View Herald Transcript

Just to rephrase your initial description to make sure I understand, the reason we can't rely on the notes_begin method to report the context itself is because not all errors that notes_begin could ultimately report have the information needed to provide this context, right? And because of this, we need the context to be added at a higher level, right?

llvm/tools/llvm-readobj/ELFDumper.cpp
5591

There could be more than one PT_NOTE segment, so perhaps this should also say something about the segment's index or other identifying property, so that it can be clearly identified. What do you think?

Just to rephrase your initial description to make sure I understand, the reason we can't rely on the notes_begin method to report the context itself is because not all errors that notes_begin could ultimately report have the information needed to provide this context, right? And because of this, we need the context to be added at a higher level, right?

Yes. notes_begin returns Elf_Note_Iterator which might report errors when used, but it doesn't have a context.

I.e. this construction might report errors from both iterator and notes_begin() call by itself:

Error Err = Error::success();
for (const typename ELFT::Note Note : Obj.notes(P, Err))
llvm/tools/llvm-readobj/ELFDumper.cpp
5591

Yes. This is what D92021 does.

jhenderson accepted this revision.Nov 25 2020, 1:32 AM

LGTM, thanks for the details.

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