[llvm-readelf/obj] - Change the return type of the `createDRI(...)` to `Expected<>`

[grimar]

Summary

This allows to get rid of "Invalid data was encountered while parsing the file"
error reported in cases when sh_size/sh_offset of sections are broken.

[MaskRay]

LGTM.

[jhenderson]

LGTM.

llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test

579

Not related to this patch, but the inconsistency of where this warning is reported in comparison to the entsize warning looks weird. Looking at code comments (see line 1942 in ElfDumper.cpp in this diff), in one place we ignore the entsize so that we can dump the symbol table, but then later on, we obviously don't. Something for a follow-up patch probably.

llvm/test/tools/llvm-readobj/ELF/dyn-symbols.test

Sections:
   - Name:    .dynamic
     Type:    SHT_DYNAMIC
     Entries:
       - Tag:   DT_SYMENT
         Value: 0x123
       - Tag:   DT_NULL
         Value: 0
 
-## Check we report a warning when the DT_STRSZ value is broken so that the dynamic string
-## table goes past the end of the file. Document we stop dumping symbols and report an error.
+## Case 10: Check we report a warning when the DT_STRSZ value is broken so that the dynamic string
+##          table goes past the end of the file. Document we stop dumping symbols and report an error.
 
 # RUN: yaml2obj %s --docnum=13 -o %t14
 # RUN: llvm-readobj --dyn-symbols %t14 2>&1 | \
 # RUN:   FileCheck %s -DFILE=%t14 --check-prefix=DYNSTR-INVALID-LLVM
 # RUN: llvm-readelf --dyn-symbols %t14 2>&1 | \
 # RUN:   FileCheck %s -DFILE=%t14 --check-prefix=DYNSTR-INVALID-GNU
 
 # DYNSTR-INVALID-LLVM: warning: '[[FILE]]': the dynamic string table at 0x78 goes past the end of the file (0x2a8) with DT_STRSZ = 0xffffffff
## An arbitrary valid symbol to document we report an error before dumping it.
   - StName:  0x1
 ProgramHeaders:
   - Type:  PT_LOAD
     Flags: [ PF_R ]
     Sections:
       - Section: .dynstr
       - Section: .dynamic
 
-## Check we report a warning when the entry size of the dynamic symbol table is zero.
-# RUN: yaml2obj %s --docnum=14 -o %t15
-# RUN: llvm-readobj --dyn-symbols %t15 2>&1 | FileCheck %s -DFILE=%t15 --check-prefix=DYNSYM-ZERO-ENTSIZE-LLVM
-# RUN: llvm-readelf --dyn-symbols %t15 2>&1 | \
-# RUN:   FileCheck %s -DFILE=%t15 --check-prefix=DYNSYM-ZERO-ENTSIZE-GNU --implicit-check-not="Symbol table"
+## Case 11: check various warnings we report when fields of the SHT_DYNSYM section are broken.
+
+## a) check we report a warning when the entry size of the dynamic symbol table is zero.
+# RUN: yaml2obj %s --docnum=14 -DENTSIZE=0x0 -o %t15.entsize
+# RUN: llvm-readobj --dyn-symbols %t15.entsize 2>&1 | FileCheck %s -DFILE=%t15.entsize --check-prefix=DYNSYM-ZERO-ENTSIZE-LLVM
+# RUN: llvm-readelf --dyn-symbols %t15.entsize 2>&1 | \
+# RUN:   FileCheck %s -DFILE=%t15.entsize --check-prefix=DYNSYM-ZERO-ENTSIZE-GNU --implicit-check-not="Symbol table"
 
 # DYNSYM-ZERO-ENTSIZE-LLVM:      DynamicSymbols [
-# DYNSYM-ZERO-ENTSIZE-LLVM-NEXT:  warning: '[[FILE]]': SHT_DYNSYM section with index 1 has invalid size (0x10) or entry size (0x0)
+# DYNSYM-ZERO-ENTSIZE-LLVM-NEXT:  warning: '[[FILE]]': SHT_DYNSYM section with index 1 has invalid size (0x20) or entry size (0x0)
 # DYNSYM-ZERO-ENTSIZE-LLVM-NEXT: ]
 
-# DYNSYM-ZERO-ENTSIZE-GNU:  warning: '[[FILE]]': SHT_DYNSYM section with index 1 has invalid size (0x10) or entry size (0x0)
+# DYNSYM-ZERO-ENTSIZE-GNU:  warning: '[[FILE]]': SHT_DYNSYM section with index 1 has invalid size (0x20) or entry size (0x0)
 
 --- !ELF
 FileHeader:
   Class: ELFCLASS32
   Data:  ELFDATA2LSB
   Type:  ET_DYN
 Sections:
-  - Name:    .dynsym
-    Type:    SHT_DYNSYM
-    EntSize: 0x0
+  - Name:     .dynsym
+    Type:     SHT_DYNSYM
+    EntSize:  [[ENTSIZE=<none>]]
+    ShOffset: [[OFFSET=<none>]]
+    ShSize:   [[SIZE=<none>]]
+DynamicSymbols:
+  - Name: foo
+
+## b) check we report a warning when the sh_offset field of the SHT_DYNSYM section is
+##    invalid (sh_offset + sh_size is greater than the file size). Check we don't dump
+##    dynamic symbols in this case.
+# RUN: yaml2obj --docnum=14 %s -DOFFSET=0xffffffff -o %t15.offset
+# RUN: llvm-readobj %t15.offset --dyn-symbols 2>&1 | FileCheck %s -DFILE=%t15.offset \
+# RUN:   --check-prefixes=OFFSET-BROKEN,OFFSET-BROKEN-LLVM
+# RUN: llvm-readelf %t15.offset --dyn-symbols 2>&1 | FileCheck %s -DFILE=%t15.offset \
+# RUN:   --implicit-check-not="Symbol table" --check-prefix=OFFSET-BROKEN
+
+# OFFSET-BROKEN: warning: '[[FILE]]': unable to read dynamic symbols from SHT_DYNSYM section with index 1: offset (0xffffffff) + size (0x20) is greater than the file size (0x148)
+
+# OFFSET-BROKEN-LLVM:      DynamicSymbols [
+# OFFSET-BROKEN-LLVM-NEXT: ]
+
+## c) check we report a warning when the sh_size field of the SHT_DYNSYM section is
+##    invalid (sh_offset + sh_size is greater than the file size). Check we don't dump
+##    dynamic symbols in this case.
+# RUN: yaml2obj --docnum=14 %s -DSIZE=0xffffffff -o %t15.size
+# RUN: llvm-readobj %t15.size --dyn-symbols 2>&1 | FileCheck %s -DFILE=%t15.size \
+# RUN:   --check-prefixes=SIZE-BROKEN,SIZE-BROKEN-LLVM
+# RUN: llvm-readelf %t15.size --dyn-symbols 2>&1 | FileCheck %s -DFILE=%t15.size \
+# RUN:   --implicit-check-not="Symbol table" --check-prefix=SIZE-BROKEN
+
+# SIZE-BROKEN: warning: '[[FILE]]': unable to read dynamic symbols from SHT_DYNSYM section with index 1: offset (0x34) + size (0xffffffff) is greater than the file size (0x148)
+
+# SIZE-BROKEN-LLVM:      DynamicSymbols [
+# SIZE-BROKEN-LLVM-NEXT: ]

llvm/test/tools/llvm-readobj/ELF/malformed-pt-dynamic.test

 # RUN: llvm-readelf %t1.noheaders --dynamic-table 2>&1 | FileCheck -DFILE=%t1.noheaders %s \
 # RUN:   --check-prefix=WARN1-NOHEADERS --implicit-check-not="Dynamic section"
 
 # WARN1-NOHEADERS: warning: '[[FILE]]': PT_DYNAMIC segment offset (0x1000) + file size (0x119) exceeds the size of the file (0x1118)
 
 ## Case B: Test case where the offset of the PT_DYNAMIC header is too large to be in the file.
 
 ## Case B.1: the section header table is present in the object. Check that we report a warning about the
-##           broken PT_DYNAMIC header, but document that we do not dump the dynamic table, because
-##           return an error earlier.
+##           broken PT_DYNAMIC header. Check we also report a warning about broken fields of the SHT_DYNAMIC section.
 # RUN: yaml2obj %s -DOFFSET=0x1131 -o %t2
-# RUN: not llvm-readobj %t2 --dynamic-table 2>&1 | FileCheck -DFILE=%t2 %s --check-prefix=WARN2
-# RUN: not llvm-readelf %t2 --dynamic-table 2>&1 | FileCheck -DFILE=%t2 %s --check-prefix=WARN2
+# RUN: llvm-readobj %t2 --dynamic-table 2>&1 | FileCheck -DFILE=%t2 %s \
+# RUN:   --check-prefix=WARN2 --implicit-check-not=warning:
+# RUN: llvm-readelf %t2 --dynamic-table 2>&1 | FileCheck -DFILE=%t2 %s \
+# RUN:   --check-prefix=WARN2 --implicit-check-not=warning:
 
 # WARN2: warning: '[[FILE]]': PT_DYNAMIC segment offset (0x1131) + file size (0x10) exceeds the size of the file (0x1130)
-# WARN2: error: '[[FILE]]': Invalid data was encountered while parsing the file
+# WARN2: warning: '[[FILE]]': unable to read the dynamic table from SHT_DYNAMIC section with index 1: offset (0x1131) + size (0x10) is greater than the file size (0x1130)
+# WARN2: warning: '[[FILE]]': no valid dynamic table was found
 
 ## Case B.2: in this case we drop section headers. The dynamic table is not dumped.
 # RUN: yaml2obj %s -DOFFSET=0x1119 -DNOHEADERS=true -o %t2.noheaders
 # RUN: llvm-readobj %t2.noheaders --dynamic-table 2>&1 | FileCheck -DFILE=%t2.noheaders %s \
 # RUN:   --check-prefix=WARN2-NOHEADERS --implicit-check-not="DynamicSection ["
 # RUN: llvm-readelf %t2.noheaders --dynamic-table 2>&1 | FileCheck -DFILE=%t2.noheaders %s \
 # RUN:   --check-prefix=WARN2-NOHEADERS --implicit-check-not="Dynamic section"
 
 # WARN2-NOHEADERS: warning: '[[FILE]]': PT_DYNAMIC segment offset (0x1119) + file size (0x10) exceeds the size of the file (0x1118)
 
 ## Case C: test we report a warning when the offset + the file size of the PT_DYNAMIC is so large a
-##         value that it overflows the platform address size type.
+##         value that it overflows the platform address size type. Check we also report a warning about
+##         broken fields of the SHT_DYNAMIC section.
 
 # RUN: yaml2obj %s -DOFFSET=0xffffffffffffffff -o %t3
-# RUN: not llvm-readobj %t3 --dynamic-table 2>&1 | FileCheck -DFILE=%t3 %s --check-prefix=WARN3
-# RUN: not llvm-readelf %t3 --dynamic-table 2>&1 | FileCheck -DFILE=%t3 %s --check-prefix=WARN3
+# RUN: llvm-readobj %t3 --dynamic-table 2>&1 | FileCheck -DFILE=%t3 %s \
+# RUN:   --check-prefix=WARN3 --implicit-check-not=warning:
+# RUN: llvm-readelf %t3 --dynamic-table 2>&1 | FileCheck -DFILE=%t3 %s \
+# RUN:   --check-prefix=WARN3 --implicit-check-not=warning:
 
 # WARN3: warning: '[[FILE]]': PT_DYNAMIC segment offset (0xffffffffffffffff) + file size (0x10) exceeds the size of the file (0x1130)
-# WARN3: error:   '[[FILE]]': Invalid data was encountered while parsing the file
+# WARN3: warning: '[[FILE]]': unable to read the dynamic table from SHT_DYNAMIC section with index 1: offset (0xffffffffffffffff) + size (0x10) is greater than the file size (0x1130)
+# WARN3: warning: '[[FILE]]': no valid dynamic table was found
 
 # RUN: yaml2obj %s -DNOHEADERS=true -DOFFSET=0xffffffffffffffff -o %t3.noheaders
 # RUN: llvm-readobj %t3.noheaders --dynamic-table 2>&1 | \
 # RUN:   FileCheck -DFILE=%t3.noheaders %s --check-prefix=WARN3-NOHEADERS
 # RUN: llvm-readelf %t3.noheaders --dynamic-table 2>&1 | \
 # RUN:   FileCheck -DFILE=%t3.noheaders %s --check-prefix=WARN3-NOHEADERS
 
 # WARN3-NOHEADERS: warning: '[[FILE]]': PT_DYNAMIC segment offset (0xffffffffffffffff) + file size (0x10) exceeds the size of the file (0x1118)

 # RUN: llvm-readelf %t4.noheaders --dynamic-table 2>&1 | \
 # RUN:   FileCheck -DFILE=%t4.noheaders %s --check-prefix=WARN4-NOHEADERS
 
 # WARN4-NOHEADERS: warning: '[[FILE]]': PT_DYNAMIC segment offset (0x1000) + file size (0xffffffffffffffff) exceeds the size of the file (0x1118)
 
 ## Case D: the same as "Case C", but for a 32-bit object.
 
 # RUN: yaml2obj %s -DBITS=32 -DOFFSET=0xffffffff -o %t5
-# RUN: not llvm-readobj %t5 --dynamic-table 2>&1 | FileCheck -DFILE=%t5 %s --check-prefix=WARN5
-# RUN: not llvm-readelf %t5 --dynamic-table 2>&1 | FileCheck -DFILE=%t5 %s --check-prefix=WARN5
+# RUN: llvm-readobj %t5 --dynamic-table 2>&1 | FileCheck -DFILE=%t5 %s \
+# RUN:   --check-prefix=WARN5 --implicit-check-not=warning:
+# RUN: llvm-readelf %t5 --dynamic-table 2>&1 | FileCheck -DFILE=%t5 %s \
+# RUN:   --check-prefix=WARN5 --implicit-check-not=warning:
 
 # WARN5: warning: '[[FILE]]': PT_DYNAMIC segment offset (0xffffffff) + file size (0x8) exceeds the size of the file (0x10c8)
-# WARN5: error:   '[[FILE]]': Invalid data was encountered while parsing the file
+# WARN5: warning: '[[FILE]]': unable to read the dynamic table from SHT_DYNAMIC section with index 1: offset (0xffffffff) + size (0x8) is greater than the file size (0x10c8)
+# WARN5: warning: '[[FILE]]': no valid dynamic table was found
 
 # RUN: yaml2obj %s -DNOHEADERS=true -DBITS=32 -DOFFSET=0xffffffff -o %t5.noheaders
 # RUN: llvm-readobj %t5.noheaders --dynamic-table 2>&1 | \
 # RUN:   FileCheck -DFILE=%t5.noheaders %s --check-prefix=WARN5-NOHEADERS
 # RUN: llvm-readelf %t5.noheaders --dynamic-table 2>&1 | \
 # RUN:   FileCheck -DFILE=%t5.noheaders %s --check-prefix=WARN5-NOHEADERS
 
 # WARN5-NOHEADERS: warning: '[[FILE]]': PT_DYNAMIC segment offset (0xffffffff) + file size (0x8) exceeds the size of the file (0x10ac)

llvm/tools/llvm-readobj/ELFDumper.cpp

public:
 
   const object::ELFObjectFile<ELFT> *getElfObject() const { return ObjF; };
 
 private:
   std::unique_ptr<DumpStyle<ELFT>> ELFDumperStyle;
 
   TYPEDEF_ELF_TYPES(ELFT)
 
-  DynRegionInfo createDRI(uint64_t Offset, uint64_t Size, uint64_t EntSize) {
+  Expected<DynRegionInfo> createDRI(uint64_t Offset, uint64_t Size,
+                                    uint64_t EntSize) {
     const ELFFile<ELFT> *Obj = ObjF->getELFFile();
     if (Offset + Size < Offset || Offset + Size > Obj->getBufSize())
-      reportError(errorCodeToError(llvm::object::object_error::parse_failed),
+      return createError("offset (0x" + Twine::utohexstr(Offset) +
+                         ") + size (0x" + Twine::utohexstr(Size) +
+                         ") is greater than the file size (0x" +
+                         Twine::utohexstr(Obj->getBufSize()) + ")");
+    return DynRegionInfo(Obj->base() + Offset, Size, EntSize,
                          ObjF->getFileName());
-    return {Obj->base() + Offset, Size, EntSize, ObjF->getFileName()};
   }
 
   void printAttributes();
   void printMipsReginfo();
   void printMipsOptions();
 
   std::pair<const Elf_Phdr *, const Elf_Shdr *>
   findDynamic(const ELFFile<ELFT> *Obj);
void ELFDumper<ELFT>::loadDynamicTable(const ELFFile<ELFT> *Obj) {
   const Elf_Shdr *DynamicSec;
   std::tie(DynamicPhdr, DynamicSec) = findDynamic(Obj);
   if (!DynamicPhdr && !DynamicSec)
     return;
 
   DynRegionInfo FromPhdr(ObjF->getFileName());
   bool IsPhdrTableValid = false;
   if (DynamicPhdr) {
-    FromPhdr = createDRI(DynamicPhdr->p_offset, DynamicPhdr->p_filesz,
-                         sizeof(Elf_Dyn));
+    // Use cantFail(), because p_offset/p_filesz fields of a PT_DYNAMIC are
+    // validated in findDynamic() and so createDRI() is not expected to fail.
+    FromPhdr = cantFail(createDRI(DynamicPhdr->p_offset, DynamicPhdr->p_filesz,
+                                  sizeof(Elf_Dyn)));
     FromPhdr.SizePrintName = "PT_DYNAMIC size";
     FromPhdr.EntSizePrintName = "";
-
     IsPhdrTableValid = !FromPhdr.getAsArrayRef<Elf_Dyn>().empty();
   }
 
   // Locate the dynamic table described in a section header.
   // Ignore sh_entsize and use the expected value for entry size explicitly.
   // This allows us to dump dynamic sections with a broken sh_entsize
   // field.
   DynRegionInfo FromSec(ObjF->getFileName());
   bool IsSecTableValid = false;
   if (DynamicSec) {
-    FromSec =
+    Expected<DynRegionInfo> RegOrErr =
         createDRI(DynamicSec->sh_offset, DynamicSec->sh_size, sizeof(Elf_Dyn));
+    if (RegOrErr) {
+      FromSec = *RegOrErr;
       FromSec.Context = describe(*DynamicSec);
       FromSec.EntSizePrintName = "";
-
       IsSecTableValid = !FromSec.getAsArrayRef<Elf_Dyn>().empty();
+    } else {
+      reportUniqueWarning(createError("unable to read the dynamic table from " +
+                                      describe(*DynamicSec) + ": " +
+                                      toString(RegOrErr.takeError())));
+    }
   }
 
   // When we only have information from one of the SHT_DYNAMIC section header or
   // PT_DYNAMIC program header, just use that.
   if (!DynamicPhdr || !DynamicSec) {
     if ((DynamicPhdr && IsPhdrTableValid) || (DynamicSec && IsSecTableValid)) {
       DynamicTable = DynamicPhdr ? FromPhdr : FromSec;
       parseDynamicTable(Obj);
case ELF::SHT_SYMTAB:
       if (!DotSymtabSec)
         DotSymtabSec = &Sec;
       break;
     case ELF::SHT_DYNSYM:
       if (!DotDynsymSec)
         DotDynsymSec = &Sec;
 
       if (!DynSymRegion) {
-        DynSymRegion = createDRI(Sec.sh_offset, Sec.sh_size, Sec.sh_entsize);
+        Expected<DynRegionInfo> RegOrErr =
+            createDRI(Sec.sh_offset, Sec.sh_size, Sec.sh_entsize);
+        if (RegOrErr) {
+          DynSymRegion = *RegOrErr;
           DynSymRegion->Context = describe(Sec);
 
           if (Expected<StringRef> E = Obj->getStringTableForSymtab(Sec))
             DynamicStringTable = *E;
           else
             reportWarning(E.takeError(), ObjF->getFileName());
+        } else {
+          reportUniqueWarning(createError(
+              "unable to read dynamic symbols from " + describe(Sec) + ": " +
+              toString(RegOrErr.takeError())));
+        }
       }
       break;
     case ELF::SHT_SYMTAB_SHNDX:
       ShndxTable = unwrapOrError(ObjF->getFileName(), Obj->getSHNDXTable(Sec));
       break;
     case ELF::SHT_GNU_versym:
       if (!SymbolVersionSection)
         SymbolVersionSection = &Sec;