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

[grimar]

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.

[jhenderson]

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?

[grimar]

In D92081#2415682, @jhenderson wrote:

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]

LGTM, thanks for the details.

llvm/include/llvm/Object/ELF.h

public:
   ///
   /// \param Phdr the program header to iterate over.
   /// \param Err [out] an error to support fallible iteration, which should
   ///  be checked after iteration ends.
   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) + ")");
+      Err =
+          createError("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);
   }
 
   /// Get an iterator over notes in a section.
   ///
   /// The section must be of type \c SHT_NOTE.
   ///
   /// \param Shdr the section to iterate over.
   /// \param Err [out] an error to support fallible iteration, which should
   ///  be checked after iteration ends.
   Elf_Note_Iterator notes_begin(const Elf_Shdr &Shdr, Error &Err) const {
     assert(Shdr.sh_type == ELF::SHT_NOTE && "Shdr is not of type SHT_NOTE");
     ErrorAsOutParameter ErrAsOutParam(&Err);
     if (Shdr.sh_offset + Shdr.sh_size > getBufSize()) {
-      Err = createError("SHT_NOTE section " + getSecIndexForError(*this, Shdr) +
-                        " has invalid offset (0x" +
-                        Twine::utohexstr(Shdr.sh_offset) + ") or size (0x" +
-                        Twine::utohexstr(Shdr.sh_size) + ")");
+      Err =
+          createError("invalid offset (0x" + Twine::utohexstr(Shdr.sh_offset) +
+                      ") or size (0x" + Twine::utohexstr(Shdr.sh_size) + ")");
       return Elf_Note_Iterator(Err);
     }
     return Elf_Note_Iterator(base() + Shdr.sh_offset, Shdr.sh_size, Err);
   }
 
   /// Get the end iterator for notes.
   Elf_Note_Iterator notes_end() const {
     return Elf_Note_Iterator();

llvm/test/tools/llvm-readobj/ELF/gnu-notes.test

 
 ## Test tools report an error if a note section has an invalid offset
 ## that goes past the end of file.
 
 # RUN: yaml2obj --docnum=2 -DSHOFFSET=0xffff0000 %s -o %t2.so
 # RUN: not llvm-readelf --notes %t2.so 2>&1 | FileCheck -DFILE=%t2.so %s --check-prefix=ERR1
 # RUN: not llvm-readobj --notes %t2.so 2>&1 | FileCheck -DFILE=%t2.so %s --check-prefix=ERR1
 
-# ERR1: error: '[[FILE]]': SHT_NOTE section [index 1] has invalid offset (0xffff0000) or size (0x0)
+# ERR1: error: '[[FILE]]': unable to read notes from the SHT_NOTE section with index 1: invalid offset (0xffff0000) or size (0x0)
 
 --- !ELF
 FileHeader:
   Class: ELFCLASS64
   Data:  ELFDATA2LSB
   Type:  ET_EXEC
 Sections:
   - Name:     .note
     Type:     SHT_NOTE
     Notes:    []
     ShOffset: [[SHOFFSET=<none>]]
     ShSize:   [[SHSIZE=<none>]]
 
 ## Test tools report an error if a note section has invalid size
 ## that goes past the end of file.
 
 # RUN: yaml2obj --docnum=2 -DSHSIZE=0xffff0000 %s -o %t3.so
 # RUN: not llvm-readelf --notes %t3.so 2>&1 | FileCheck -DFILE=%t3.so %s --check-prefix=ERR2
 # RUN: not llvm-readobj --notes %t3.so 2>&1 | FileCheck -DFILE=%t3.so %s --check-prefix=ERR2
 
-# ERR2: error: '[[FILE]]': SHT_NOTE section [index 1] has invalid offset (0x40) or size (0xffff0000)
+# ERR2: error: '[[FILE]]': unable to read notes from the SHT_NOTE section with index 1: invalid offset (0x40) or size (0xffff0000)
 
 ## Test tools report an error if a note program header has an invalid offset that
 ## goes past the end of file.
 
 # RUN: yaml2obj --docnum=3 -DPHOFFSET=0xffff0000 %s -o %t4.so
 # RUN: not llvm-readelf --notes %t4.so 2>&1 | FileCheck -DFILE=%t4.so %s --check-prefix=ERR3
 # RUN: not llvm-readobj --notes %t4.so 2>&1 | FileCheck -DFILE=%t4.so %s --check-prefix=ERR3
 
-# ERR3: error: '[[FILE]]': PT_NOTE header has invalid offset (0xffff0000) or size (0x0)
+# ERR3: error: '[[FILE]]': unable to read notes from the PT_NOTE segment: invalid offset (0xffff0000) or size (0x0)
 
 --- !ELF
 FileHeader:
   Class: ELFCLASS64
   Data:  ELFDATA2LSB
   Type:  ET_CORE
 ProgramHeaders:
   - Type:     PT_NOTE
     Offset:   [[PHOFFSET=<none>]]
     FileSize: [[PHFILESIZE=<none>]]
 
 ## Test tools report an error if a note program header has an invalid size that
 ## goes past the end of file.
 
 # RUN: yaml2obj --docnum=3 -DPHFILESIZE=0xffff0000 %s -o %t5.so
 # RUN: not llvm-readelf --notes %t5.so 2>&1 | FileCheck -DFILE=%t5.so  %s --check-prefix=ERR4
 # RUN: not llvm-readobj --notes %t5.so 2>&1 | FileCheck -DFILE=%t5.so %s --check-prefix=ERR4
 
-# ERR4: error: '[[FILE]]': PT_NOTE header has invalid offset (0x0) or size (0xffff0000)
+# ERR4: error: '[[FILE]]': unable to read notes from the PT_NOTE segment: invalid offset (0x0) or size (0xffff0000)
 
 ## Check we report a warning when we are unable to locate the PT_NOTE
 ## segment because of broken program headers.
 # RUN: yaml2obj --docnum=1 -DPHENTSIZE=1 -DSHNUM=0 %s -o %t6.so
 # RUN: llvm-readelf --notes %t6.so 2>&1 | FileCheck %s -DFILE=%t6.so --check-prefix=PHENTSIZE-WARN-GNU
 # RUN: llvm-readobj --notes %t6.so 2>&1 | FileCheck %s -DFILE=%t6.so --check-prefix=PHENTSIZE-WARN-LLVM
 
 # PHENTSIZE-WARN-GNU: warning: '[[FILE]]': unable to read program headers to locate the PT_DYNAMIC segment: invalid e_phentsize: 1
 # PHENTSIZE-WARN-GNU: warning: '[[FILE]]': unable to read program headers to locate the PT_NOTE segment: invalid e_phentsize: 1
 
 # PHENTSIZE-WARN-LLVM:      Notes [
 # PHENTSIZE-WARN-LLVM-NEXT: warning: '[[FILE]]': unable to read program headers to locate the PT_NOTE segment: invalid e_phentsize: 1
 # PHENTSIZE-WARN-LLVM-NEXT: ]

llvm/tools/llvm-readobj/ELFDumper.cpp

for (const typename ELFT::Shdr &S : Sections) {
       if (S.sh_type != SHT_NOTE)
         continue;
       StartNotesFn(expectedToOptional(Obj.getSectionName(S)), S.sh_offset,
                    S.sh_size);
       Error Err = Error::success();
       for (const typename ELFT::Note &Note : Obj.notes(S, Err))
         ProcessNoteFn(Note);
       if (Err)
-        reportError(std::move(Err), Dumper.getElfObject().getFileName());
+        reportError(createError("unable to read notes from the " +
+                                describe(Obj, S) + ": " +
+                                toString(std::move(Err))),
+                    Dumper.getElfObject().getFileName());
       FinishNotesFn();
     }
     return;
   }
 
   Expected<ArrayRef<typename ELFT::Phdr>> PhdrsOrErr = Obj.program_headers();
   if (!PhdrsOrErr) {
     Dumper.reportUniqueWarning(createError(
         "unable to read program headers to locate the PT_NOTE segment: " +
         toString(PhdrsOrErr.takeError())));
     return;
   }
 
   for (const typename ELFT::Phdr &P : *PhdrsOrErr) {
     if (P.p_type != PT_NOTE)
       continue;
     StartNotesFn(/*SecName=*/None, P.p_offset, P.p_filesz);
     Error Err = Error::success();
     for (const typename ELFT::Note Note : Obj.notes(P, Err))
       ProcessNoteFn(Note);
     if (Err)
-      reportError(std::move(Err), Dumper.getElfObject().getFileName());
+      reportError(
+          createError("unable to read notes from the PT_NOTE segment: " +
+                      toString(std::move(Err))),
+          Dumper.getElfObject().getFileName());
     FinishNotesFn();
   }
 }
 
 template <class ELFT> void GNUStyle<ELFT>::printNotes() {
   auto PrintHeader = [&](Optional<StringRef> SecName,
                          const typename ELFT::Off Offset,
                          const typename ELFT::Addr Size) {