[obj2yaml] - Rework tool's error reporting logic for ELF target.

[grimar]

Summary

ELF.h contains two getSymbol methods
which seems to be used only from obj2yaml.

One of these methods calls another, which in turn
contains untested error message which doesn't
provide enough information.

Problem is that after improving only just that message,
obj2yaml will not show it,
("Error reading file: yaml: Invalid data was
encountered while parsing the file" message will be shown instead),
because internal errors handling of tool is based on ErrorOr<> class which
stores a error code and as a result can only show a predefined error string, what
actually isn't very useful.

In this patch, I rework obj2yaml's error reporting system
for ELF targets to use Error Expected<> classes.
Also, I improve the error message produced
by getSymbol for demonstration of the new functionality.

[MaskRay]

Happy to see more ErrorOr/std::error_code becoming Error/Expected 😊

include/llvm/Object/ELF.h

364 ↗

(On Diff #209472)

Elf_Sym_Range Symbols = *SymsOrErr;

tools/obj2yaml/elf2yaml.cpp

164 ↗

(On Diff #209472)

return E; to avoid clang -Wpessimizing-move (automatic move from lvalues applies here)

176 ↗

(On Diff #209472)

Y->Sections.emplace_back(*SecOrErr);

We can leverage the ctor of unique_ptr here.

197 ↗

(On Diff #209472)

ditto

204 ↗

(On Diff #209472)

ditto

430 ↗

(On Diff #209472)

return E;

444 ↗

(On Diff #209472)

return E;

[grimar]

• Addressed review comments.

tools/obj2yaml/elf2yaml.cpp

164 ↗

(On Diff #209472)

I can't. Otherwise I have the following errors:
(MVSC 2017)

elf2yaml.cpp(164): error C2280: 'llvm::Error::Error(const llvm::Error &)': attempting to reference a deleted function
llvm/Support/Error.h(186): note: see declaration of 'llvm::Error::Error'
llvm/Support/Error.h(186): note: 'llvm::Error::Error(const llvm::Error &)': function was explicitly deleted

When I search for "return std::move(" in LLVMs code, I see many similar places.

[MaskRay]

tools/obj2yaml/elf2yaml.cpp

164 ↗

(On Diff #209472)

Sorry, the -Wpessimizing-move warning is on elf2yaml.cpp:394 (where the return type is Error), not here (where the return type is Expected).

[grimar]

• Addressed review comments.

tools/obj2yaml/elf2yaml.cpp

164 ↗

(On Diff #209472)

Fixed, thanks!

llvm/trunk/include/llvm/Object/ELF.h

if (!IndexOrErr)
     return IndexOrErr.takeError();
   uint32_t Index = *IndexOrErr;
   if (Index == 0)
     return nullptr;
   return getSection(Index);
 }
 
 template <class ELFT>
-inline Expected<const typename ELFT::Sym *>
-getSymbol(typename ELFT::SymRange Symbols, uint32_t Index) {
-  if (Index >= Symbols.size())
-    // TODO: this error is untested.
-    return createError("invalid symbol index");
-  return &Symbols[Index];
-}
-
-template <class ELFT>
 Expected<const typename ELFT::Sym *>
 ELFFile<ELFT>::getSymbol(const Elf_Shdr *Sec, uint32_t Index) const {
-  auto SymtabOrErr = symbols(Sec);
-  if (!SymtabOrErr)
-    return SymtabOrErr.takeError();
-  return object::getSymbol<ELFT>(*SymtabOrErr, Index);
+  auto SymsOrErr = symbols(Sec);
+  if (!SymsOrErr)
+    return SymsOrErr.takeError();
+
+  Elf_Sym_Range Symbols = *SymsOrErr;
+  if (Index >= Symbols.size())
+    return createError("unable to get symbol from section " +
+                       getSecIndexForError(this, Sec) +
+                       ": invalid symbol index (" + Twine(Index) + ")");
+  return &Symbols[Index];
 }
 
 template <class ELFT>
 template <typename T>
 Expected<ArrayRef<T>>
 ELFFile<ELFT>::getSectionContentsAsArray(const Elf_Shdr *Sec) const {
   if (Sec->sh_entsize != sizeof(T) && sizeof(T) != 1)
     return createError("section " + getSecIndexForError(this, Sec) +

llvm/trunk/test/tools/obj2yaml/section-group.test

 ## Checks that the tool is able to read section groups from ELF.
 
-# RUN: yaml2obj %s > %t1.o
+# RUN: yaml2obj --docnum=1 %s > %t1.o
 # RUN: llvm-readobj --elf-section-groups %t1.o | FileCheck %s -check-prefix=OBJ
 # RUN: obj2yaml %t1.o | FileCheck %s --check-prefix YAML
 
 # OBJ:      Groups {
 # OBJ-NEXT:   Group {
 # OBJ-NEXT:     Name: .group
 # OBJ-NEXT:     Index: 1
 # OBJ-NEXT:     Link: 3
Members:
       - SectionOrType: GRP_COMDAT
       - SectionOrType: .rodata
   - Name: .rodata
     Type: SHT_PROGBITS
 Symbols:
   - Name:    signature
     Type:    STT_OBJECT
     Section: .rodata
+
+## Check obj2yaml report an error when sh_info field of
+## group section contains invalid (too large) signature symbol index.
+
+# RUN: yaml2obj --docnum=2 %s > %t2.o
+# RUN: not obj2yaml %t2.o 2>&1 | FileCheck %s --check-prefix ERR
+
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_REL
+  Machine: EM_X86_64
+Sections:
+  - Name: .group
+    Type: SHT_GROUP
+    Link: .symtab
+    Info: 0xFF
+    Members:
+      - SectionOrType: GRP_COMDAT
+
+# ERR: Error reading file: {{.*}}2.o: unable to get symbol from section [index 2]: invalid symbol index (255)

llvm/trunk/test/tools/obj2yaml/special-symbol-indices.yaml

- Name:     processor_specific_index
     Binding:  STB_GLOBAL
   - Name:     unknown_index
     Index:    0xfffe
     Binding:  STB_GLOBAL
 
 ## shn_xindex.o contains a symbol with st_shndx == SHN_XINDEX.
 ## We do not support it at this moment.
 # RUN: not obj2yaml %S/Inputs/shn_xindex.o 2>&1 | FileCheck %s --check-prefix=ERR
-# ERR: Error reading file: {{.*}}shn_xindex.o: Feature not yet implemented.
+# ERR: Error reading file: {{.*}}shn_xindex.o: SHN_XINDEX symbols are not supported

llvm/trunk/tools/obj2yaml/elf2yaml.cpp

class ELFDumper {
   Expected<StringRef> getUniquedSectionName(const Elf_Shdr *Sec);
   Expected<StringRef> getUniquedSymbolName(const Elf_Sym *Sym,
                                            StringRef StrTable,
                                            const Elf_Shdr *SymTab);
 
   const object::ELFFile<ELFT> &Obj;
   ArrayRef<Elf_Word> ShndxTable;
 
-  std::error_code dumpSymbols(const Elf_Shdr *Symtab,
-                              std::vector<ELFYAML::Symbol> &Symbols);
-  std::error_code dumpSymbol(const Elf_Sym *Sym, const Elf_Shdr *SymTab,
-                             StringRef StrTable, ELFYAML::Symbol &S);
-  std::error_code dumpCommonSection(const Elf_Shdr *Shdr, ELFYAML::Section &S);
-  std::error_code dumpCommonRelocationSection(const Elf_Shdr *Shdr,
-                                              ELFYAML::RelocationSection &S);
+  Error dumpSymbols(const Elf_Shdr *Symtab,
+  Error dumpSymbol(const Elf_Sym *Sym, const Elf_Shdr *SymTab,
+  Error dumpCommonSection(const Elf_Shdr *Shdr, ELFYAML::Section &S);
+  Error dumpCommonRelocationSection(const Elf_Shdr *Shdr,
   template <class RelT>
-  std::error_code dumpRelocation(const RelT *Rel, const Elf_Shdr *SymTab,
-                                 ELFYAML::Relocation &R);
-  
-  ErrorOr<ELFYAML::DynamicSection *> dumpDynamicSection(const Elf_Shdr *Shdr);
-  ErrorOr<ELFYAML::RelocationSection *> dumpRelocSection(const Elf_Shdr *Shdr);
-  ErrorOr<ELFYAML::RawContentSection *>
+  Error dumpRelocation(const RelT *Rel, const Elf_Shdr *SymTab,
+  Expected<ELFYAML::DynamicSection *> dumpDynamicSection(const Elf_Shdr *Shdr);
+  Expected<ELFYAML::RelocationSection *> dumpRelocSection(const Elf_Shdr *Shdr);
+  Expected<ELFYAML::RawContentSection *>
   dumpContentSection(const Elf_Shdr *Shdr);
-  ErrorOr<ELFYAML::NoBitsSection *> dumpNoBitsSection(const Elf_Shdr *Shdr);
-  ErrorOr<ELFYAML::VerdefSection *> dumpVerdefSection(const Elf_Shdr *Shdr);
-  ErrorOr<ELFYAML::SymverSection *> dumpSymverSection(const Elf_Shdr *Shdr);
-  ErrorOr<ELFYAML::VerneedSection *> dumpVerneedSection(const Elf_Shdr *Shdr);
-  ErrorOr<ELFYAML::Group *> dumpGroup(const Elf_Shdr *Shdr);
-  ErrorOr<ELFYAML::MipsABIFlags *> dumpMipsABIFlags(const Elf_Shdr *Shdr);
+  Expected<ELFYAML::NoBitsSection *> dumpNoBitsSection(const Elf_Shdr *Shdr);
+  Expected<ELFYAML::VerdefSection *> dumpVerdefSection(const Elf_Shdr *Shdr);
+  Expected<ELFYAML::SymverSection *> dumpSymverSection(const Elf_Shdr *Shdr);
+  Expected<ELFYAML::VerneedSection *> dumpVerneedSection(const Elf_Shdr *Shdr);
+  Expected<ELFYAML::Group *> dumpGroup(const Elf_Shdr *Shdr);
+  Expected<ELFYAML::MipsABIFlags *> dumpMipsABIFlags(const Elf_Shdr *Shdr);
 
 public:
   ELFDumper(const object::ELFFile<ELFT> &O);
-  ErrorOr<ELFYAML::Object *> dump();
+  Expected<ELFYAML::Object *> dump();
 };
 
 }
 
 template <class ELFT>
 ELFDumper<ELFT>::ELFDumper(const object::ELFFile<ELFT> &O)
     : Obj(O) {}
 
if (SymTab->sh_type == ELF::SHT_SYMTAB) {
     else
       SymbolNames[Index] = Name;
     return SymbolNames[Index];
   }
 
   return Name;
 }
 
-template <class ELFT> ErrorOr<ELFYAML::Object *> ELFDumper<ELFT>::dump() {
+template <class ELFT> Expected<ELFYAML::Object *> ELFDumper<ELFT>::dump() {
   auto Y = make_unique<ELFYAML::Object>();
 
   // Dump header. We do not dump SHEntSize, SHOffset, SHNum and SHStrNdx field.
   // When not explicitly set, the values are set by yaml2obj automatically
   // and there is no need to dump them here.
   Y->Header.Class = ELFYAML::ELF_ELFCLASS(Obj.getHeader()->getFileClass());
   Y->Header.Data = ELFYAML::ELF_ELFDATA(Obj.getHeader()->getDataEncoding());
   Y->Header.OSABI = Obj.getHeader()->e_ident[ELF::EI_OSABI];
   Y->Header.ABIVersion = Obj.getHeader()->e_ident[ELF::EI_ABIVERSION];
   Y->Header.Type = Obj.getHeader()->e_type;
   Y->Header.Machine = Obj.getHeader()->e_machine;
   Y->Header.Flags = Obj.getHeader()->e_flags;
   Y->Header.Entry = Obj.getHeader()->e_entry;
 
   // Dump sections
   auto SectionsOrErr = Obj.sections();
   if (!SectionsOrErr)
-    return errorToErrorCode(SectionsOrErr.takeError());
+    return SectionsOrErr.takeError();
   Sections = *SectionsOrErr;
   SectionNames.resize(Sections.size());
 
   // Dump symbols. We need to do this early because other sections might want
   // to access the deduplicated symbol names that we also create here.
   for (const Elf_Shdr &Sec : Sections) {
     if (Sec.sh_type == ELF::SHT_SYMTAB)
-      if (auto EC = dumpSymbols(&Sec, Y->Symbols))
-        return EC;
+      if (Error E = dumpSymbols(&Sec, Y->Symbols))
+        return std::move(E);
     if (Sec.sh_type == ELF::SHT_DYNSYM)
-      if (auto EC = dumpSymbols(&Sec, Y->DynamicSymbols))
-        return EC;
+      if (Error E = dumpSymbols(&Sec, Y->DynamicSymbols))
+        return std::move(E);
   }
 
   for (const Elf_Shdr &Sec : Sections) {
     switch (Sec.sh_type) {
     case ELF::SHT_DYNAMIC: {
-      ErrorOr<ELFYAML::DynamicSection *> S = dumpDynamicSection(&Sec);
-      if (std::error_code EC = S.getError())
-        return EC;
-      Y->Sections.push_back(std::unique_ptr<ELFYAML::Section>(S.get()));
+      Expected<ELFYAML::DynamicSection *> SecOrErr = dumpDynamicSection(&Sec);
+      if (!SecOrErr)
+        return SecOrErr.takeError();
+      Y->Sections.emplace_back(*SecOrErr);
       break;
     }
     case ELF::SHT_NULL:
     case ELF::SHT_STRTAB:
     case ELF::SHT_SYMTAB:
     case ELF::SHT_DYNSYM:
       // Do not dump these sections.
       break;
     case ELF::SHT_SYMTAB_SHNDX: {
       auto TableOrErr = Obj.getSHNDXTable(Sec);
       if (!TableOrErr)
-        return errorToErrorCode(TableOrErr.takeError());
+        return TableOrErr.takeError();
       ShndxTable = *TableOrErr;
       break;
     }
     case ELF::SHT_REL:
     case ELF::SHT_RELA: {
-      ErrorOr<ELFYAML::RelocationSection *> S = dumpRelocSection(&Sec);
-      if (std::error_code EC = S.getError())
-        return EC;
-      Y->Sections.push_back(std::unique_ptr<ELFYAML::Section>(S.get()));
+      Expected<ELFYAML::RelocationSection *> SecOrErr = dumpRelocSection(&Sec);
+      if (!SecOrErr)
+        return SecOrErr.takeError();
+      Y->Sections.emplace_back(*SecOrErr);
       break;
     }
     case ELF::SHT_GROUP: {
-      ErrorOr<ELFYAML::Group *> G = dumpGroup(&Sec);
-      if (std::error_code EC = G.getError())
-        return EC;
-      Y->Sections.push_back(std::unique_ptr<ELFYAML::Section>(G.get()));
+      Expected<ELFYAML::Group *> GroupOrErr = dumpGroup(&Sec);
+      if (!GroupOrErr)
+        return GroupOrErr.takeError();
+      Y->Sections.emplace_back(*GroupOrErr);
       break;
     }
     case ELF::SHT_MIPS_ABIFLAGS: {
-      ErrorOr<ELFYAML::MipsABIFlags *> G = dumpMipsABIFlags(&Sec);
-      if (std::error_code EC = G.getError())
-        return EC;
-      Y->Sections.push_back(std::unique_ptr<ELFYAML::Section>(G.get()));
+      Expected<ELFYAML::MipsABIFlags *> SecOrErr = dumpMipsABIFlags(&Sec);
+      if (!SecOrErr)
+        return SecOrErr.takeError();
+      Y->Sections.emplace_back(*SecOrErr);
       break;
     }
     case ELF::SHT_NOBITS: {
-      ErrorOr<ELFYAML::NoBitsSection *> S = dumpNoBitsSection(&Sec);
-      if (std::error_code EC = S.getError())
-        return EC;
-      Y->Sections.push_back(std::unique_ptr<ELFYAML::Section>(S.get()));
+      Expected<ELFYAML::NoBitsSection *> SecOrErr = dumpNoBitsSection(&Sec);
+      if (!SecOrErr)
+        return SecOrErr.takeError();
+      Y->Sections.emplace_back(*SecOrErr);
       break;
     }
     case ELF::SHT_GNU_verdef: {
-      ErrorOr<ELFYAML::VerdefSection *> S = dumpVerdefSection(&Sec);
-      if (std::error_code EC = S.getError())
-        return EC;
-      Y->Sections.push_back(std::unique_ptr<ELFYAML::Section>(S.get()));
+      Expected<ELFYAML::VerdefSection *> SecOrErr = dumpVerdefSection(&Sec);
+      if (!SecOrErr)
+        return SecOrErr.takeError();
+      Y->Sections.emplace_back(*SecOrErr);
       break;
     }
     case ELF::SHT_GNU_versym: {
-      ErrorOr<ELFYAML::SymverSection *> S = dumpSymverSection(&Sec);
-      if (std::error_code EC = S.getError())
-        return EC;
-      Y->Sections.push_back(std::unique_ptr<ELFYAML::Section>(S.get()));
+      Expected<ELFYAML::SymverSection *> SecOrErr = dumpSymverSection(&Sec);
+      if (!SecOrErr)
+        return SecOrErr.takeError();
+      Y->Sections.emplace_back(*SecOrErr);
       break;
     }
     case ELF::SHT_GNU_verneed: {
-      ErrorOr<ELFYAML::VerneedSection *> S = dumpVerneedSection(&Sec);
-      if (std::error_code EC = S.getError())
-        return EC;
-      Y->Sections.push_back(std::unique_ptr<ELFYAML::Section>(S.get()));
+      Expected<ELFYAML::VerneedSection *> SecOrErr = dumpVerneedSection(&Sec);
+      if (!SecOrErr)
+        return SecOrErr.takeError();
+      Y->Sections.emplace_back(*SecOrErr);
       break;
     }
     default: {
-      ErrorOr<ELFYAML::RawContentSection *> S = dumpContentSection(&Sec);
-      if (std::error_code EC = S.getError())
-        return EC;
-      Y->Sections.push_back(std::unique_ptr<ELFYAML::Section>(S.get()));
+      Expected<ELFYAML::RawContentSection *> SecOrErr =
+          dumpContentSection(&Sec);
+      if (!SecOrErr)
+        return SecOrErr.takeError();
+      Y->Sections.emplace_back(*SecOrErr);
     }
     }
   }
 
   return Y.release();
 }
 
 template <class ELFT>
-std::error_code
-ELFDumper<ELFT>::dumpSymbols(const Elf_Shdr *Symtab,
+Error ELFDumper<ELFT>::dumpSymbols(const Elf_Shdr *Symtab,
                              std::vector<ELFYAML::Symbol> &Symbols) {
   if (!Symtab)
-    return std::error_code();
+    return Error::success();
 
   auto StrTableOrErr = Obj.getStringTableForSymtab(*Symtab);
   if (!StrTableOrErr)
-    return errorToErrorCode(StrTableOrErr.takeError());
+    return StrTableOrErr.takeError();
   StringRef StrTable = *StrTableOrErr;
 
   auto SymtabOrErr = Obj.symbols(Symtab);
   if (!SymtabOrErr)
-    return errorToErrorCode(SymtabOrErr.takeError());
+    return SymtabOrErr.takeError();
 
   if (Symtab->sh_type == ELF::SHT_SYMTAB) {
     SymTable = *SymtabOrErr;
     SymbolNames.resize(SymTable.size());
   }
 
   for (const auto &Sym : (*SymtabOrErr).drop_front()) {
     ELFYAML::Symbol S;
     if (auto EC = dumpSymbol(&Sym, Symtab, StrTable, S))
       return EC;
     Symbols.push_back(S);
   }
 
-  return std::error_code();
+  return Error::success();
 }
 
 template <class ELFT>
-std::error_code
-ELFDumper<ELFT>::dumpSymbol(const Elf_Sym *Sym, const Elf_Shdr *SymTab,
+Error ELFDumper<ELFT>::dumpSymbol(const Elf_Sym *Sym, const Elf_Shdr *SymTab,
                                   StringRef StrTable, ELFYAML::Symbol &S) {
   S.Type = Sym->getType();
   S.Value = Sym->st_value;
   S.Size = Sym->st_size;
   S.Other = Sym->st_other;
   S.Binding = Sym->getBinding();
 
   Expected<StringRef> SymbolNameOrErr =
       getUniquedSymbolName(Sym, StrTable, SymTab);
   if (!SymbolNameOrErr)
-    return errorToErrorCode(SymbolNameOrErr.takeError());
+    return SymbolNameOrErr.takeError();
   S.Name = SymbolNameOrErr.get();
 
   if (Sym->st_shndx >= ELF::SHN_LORESERVE) {
     if (Sym->st_shndx == ELF::SHN_XINDEX)
-      return obj2yaml_error::not_implemented;
+      return createStringError(obj2yaml_error::not_implemented,
+                               "SHN_XINDEX symbols are not supported");
     S.Index = (ELFYAML::ELF_SHN)Sym->st_shndx;
-    return obj2yaml_error::success;
+    return Error::success();
   }
 
   auto ShdrOrErr = Obj.getSection(Sym, SymTab, ShndxTable);
   if (!ShdrOrErr)
-    return errorToErrorCode(ShdrOrErr.takeError());
+    return ShdrOrErr.takeError();
   const Elf_Shdr *Shdr = *ShdrOrErr;
   if (!Shdr)
-    return obj2yaml_error::success;
+    return Error::success();
 
   auto NameOrErr = getUniquedSectionName(Shdr);
   if (!NameOrErr)
-    return errorToErrorCode(NameOrErr.takeError());
+    return NameOrErr.takeError();
   S.Section = NameOrErr.get();
 
-  return obj2yaml_error::success;
+  return Error::success();
 }
 
 template <class ELFT>
 template <class RelT>
-std::error_code ELFDumper<ELFT>::dumpRelocation(const RelT *Rel,
-                                                const Elf_Shdr *SymTab,
-                                                ELFYAML::Relocation &R) {
+Error ELFDumper<ELFT>::dumpRelocation(const RelT *Rel, const Elf_Shdr *SymTab,
   R.Type = Rel->getType(Obj.isMips64EL());
   R.Offset = Rel->r_offset;
   R.Addend = 0;
 
   auto SymOrErr = Obj.getRelocationSymbol(Rel, SymTab);
   if (!SymOrErr)
-    return errorToErrorCode(SymOrErr.takeError());
+    return SymOrErr.takeError();
   const Elf_Sym *Sym = *SymOrErr;
   auto StrTabSec = Obj.getSection(SymTab->sh_link);
   if (!StrTabSec)
-    return errorToErrorCode(StrTabSec.takeError());
+    return StrTabSec.takeError();
   auto StrTabOrErr = Obj.getStringTable(*StrTabSec);
   if (!StrTabOrErr)
-    return errorToErrorCode(StrTabOrErr.takeError());
+    return StrTabOrErr.takeError();
   StringRef StrTab = *StrTabOrErr;
 
   if (Sym) {
     Expected<StringRef> NameOrErr = getUniquedSymbolName(Sym, StrTab, SymTab);
     if (!NameOrErr)
-      return errorToErrorCode(NameOrErr.takeError());
+      return NameOrErr.takeError();
     R.Symbol = NameOrErr.get();
   } else {
     // We have some edge cases of relocations without a symbol associated,
     // e.g. an object containing the invalid (according to the System V
     // ABI) R_X86_64_NONE reloc. Create a symbol with an empty name instead
     // of crashing.
     R.Symbol = "";
   }
 
-  return obj2yaml_error::success;
+  return Error::success();
 }
 
 template <class ELFT>
-std::error_code ELFDumper<ELFT>::dumpCommonSection(const Elf_Shdr *Shdr,
-                                                   ELFYAML::Section &S) {
+Error ELFDumper<ELFT>::dumpCommonSection(const Elf_Shdr *Shdr,
   // Dump fields. We do not dump the ShOffset field. When not explicitly
   // set, the value is set by yaml2obj automatically.
   S.Type = Shdr->sh_type;
   if (Shdr->sh_flags)
     S.Flags = static_cast<ELFYAML::ELF_SHF>(Shdr->sh_flags);
   S.Address = Shdr->sh_addr;
   S.AddressAlign = Shdr->sh_addralign;
   if (Shdr->sh_entsize)
     S.EntSize = static_cast<llvm::yaml::Hex64>(Shdr->sh_entsize);
 
   auto NameOrErr = getUniquedSectionName(Shdr);
   if (!NameOrErr)
-    return errorToErrorCode(NameOrErr.takeError());
+    return NameOrErr.takeError();
   S.Name = NameOrErr.get();
 
   if (Shdr->sh_link != ELF::SHN_UNDEF) {
     auto LinkSection = Obj.getSection(Shdr->sh_link);
     if (LinkSection.takeError())
-      return errorToErrorCode(LinkSection.takeError());
+      return LinkSection.takeError();
     NameOrErr = getUniquedSectionName(*LinkSection);
     if (!NameOrErr)
-      return errorToErrorCode(NameOrErr.takeError());
+      return NameOrErr.takeError();
     S.Link = NameOrErr.get();
   }
 
-  return obj2yaml_error::success;
+  return Error::success();
 }
 
 template <class ELFT>
-std::error_code
-ELFDumper<ELFT>::dumpCommonRelocationSection(const Elf_Shdr *Shdr,
-                                             ELFYAML::RelocationSection &S) {
-  if (std::error_code EC = dumpCommonSection(Shdr, S))
-    return EC;
+Error ELFDumper<ELFT>::dumpCommonRelocationSection(
+    const Elf_Shdr *Shdr, ELFYAML::RelocationSection &S) {
+  if (Error E = dumpCommonSection(Shdr, S))
+    return E;
 
   auto InfoSection = Obj.getSection(Shdr->sh_info);
   if (!InfoSection)
-    return errorToErrorCode(InfoSection.takeError());
+    return InfoSection.takeError();
 
   auto NameOrErr = getUniquedSectionName(*InfoSection);
   if (!NameOrErr)
-    return errorToErrorCode(NameOrErr.takeError());
+    return NameOrErr.takeError();
   S.RelocatableSec = NameOrErr.get();
 
-  return obj2yaml_error::success;
+  return Error::success();
 }
 
 template <class ELFT>
-ErrorOr<ELFYAML::DynamicSection *>
+Expected<ELFYAML::DynamicSection *>
 ELFDumper<ELFT>::dumpDynamicSection(const Elf_Shdr *Shdr) {
   auto S = make_unique<ELFYAML::DynamicSection>();
-  if (std::error_code EC = dumpCommonSection(Shdr, *S))
-    return EC;
+  if (Error E = dumpCommonSection(Shdr, *S))
+    return std::move(E);
 
   auto DynTagsOrErr = Obj.template getSectionContentsAsArray<Elf_Dyn>(Shdr);
   if (!DynTagsOrErr)
-    return errorToErrorCode(DynTagsOrErr.takeError());
+    return DynTagsOrErr.takeError();
 
   for (const Elf_Dyn &Dyn : *DynTagsOrErr)
     S->Entries.push_back({(ELFYAML::ELF_DYNTAG)Dyn.getTag(), Dyn.getVal()});
 
   return S.release();
 }
 
 template <class ELFT>
-ErrorOr<ELFYAML::RelocationSection *>
+Expected<ELFYAML::RelocationSection *>
 ELFDumper<ELFT>::dumpRelocSection(const Elf_Shdr *Shdr) {
   auto S = make_unique<ELFYAML::RelocationSection>();
-  if (std::error_code EC = dumpCommonRelocationSection(Shdr, *S))
-    return EC;
+  if (auto E = dumpCommonRelocationSection(Shdr, *S))
+    return std::move(E);
 
   auto SymTabOrErr = Obj.getSection(Shdr->sh_link);
   if (!SymTabOrErr)
-    return errorToErrorCode(SymTabOrErr.takeError());
+    return SymTabOrErr.takeError();
   const Elf_Shdr *SymTab = *SymTabOrErr;
 
   if (Shdr->sh_type == ELF::SHT_REL) {
     auto Rels = Obj.rels(Shdr);
     if (!Rels)
-      return errorToErrorCode(Rels.takeError());
+      return Rels.takeError();
     for (const Elf_Rel &Rel : *Rels) {
       ELFYAML::Relocation R;
-      if (std::error_code EC = dumpRelocation(&Rel, SymTab, R))
-        return EC;
+      if (Error E = dumpRelocation(&Rel, SymTab, R))
+        return std::move(E);
       S->Relocations.push_back(R);
     }
   } else {
     auto Rels = Obj.relas(Shdr);
     if (!Rels)
-      return errorToErrorCode(Rels.takeError());
+      return Rels.takeError();
     for (const Elf_Rela &Rel : *Rels) {
       ELFYAML::Relocation R;
-      if (std::error_code EC = dumpRelocation(&Rel, SymTab, R))
-        return EC;
+      if (Error E = dumpRelocation(&Rel, SymTab, R))
+        return std::move(E);
       R.Addend = Rel.r_addend;
       S->Relocations.push_back(R);
     }
   }
 
   return S.release();
 }
 
 template <class ELFT>
-ErrorOr<ELFYAML::RawContentSection *>
+Expected<ELFYAML::RawContentSection *>
 ELFDumper<ELFT>::dumpContentSection(const Elf_Shdr *Shdr) {
   auto S = make_unique<ELFYAML::RawContentSection>();
-
-  if (std::error_code EC = dumpCommonSection(Shdr, *S))
-    return EC;
+  if (Error E = dumpCommonSection(Shdr, *S))
+    return std::move(E);
 
   auto ContentOrErr = Obj.getSectionContents(Shdr);
   if (!ContentOrErr)
-    return errorToErrorCode(ContentOrErr.takeError());
+    return ContentOrErr.takeError();
   ArrayRef<uint8_t> Content = *ContentOrErr;
   if (!Content.empty())
     S->Content = yaml::BinaryRef(Content);
   if (Shdr->sh_info)
     S->Info = static_cast<llvm::yaml::Hex64>(Shdr->sh_info);
   return S.release();
 }
 
 template <class ELFT>
-ErrorOr<ELFYAML::NoBitsSection *>
+Expected<ELFYAML::NoBitsSection *>
 ELFDumper<ELFT>::dumpNoBitsSection(const Elf_Shdr *Shdr) {
   auto S = make_unique<ELFYAML::NoBitsSection>();
-
-  if (std::error_code EC = dumpCommonSection(Shdr, *S))
-    return EC;
+  if (Error E = dumpCommonSection(Shdr, *S))
+    return std::move(E);
   S->Size = Shdr->sh_size;
 
   return S.release();
 }
 
 template <class ELFT>
-ErrorOr<ELFYAML::VerdefSection *>
+Expected<ELFYAML::VerdefSection *>
 ELFDumper<ELFT>::dumpVerdefSection(const Elf_Shdr *Shdr) {
   typedef typename ELFT::Verdef Elf_Verdef;
   typedef typename ELFT::Verdaux Elf_Verdaux;
 
   auto S = make_unique<ELFYAML::VerdefSection>();
-  if (std::error_code EC = dumpCommonSection(Shdr, *S))
-    return EC;
+  if (Error E = dumpCommonSection(Shdr, *S))
+    return std::move(E);
 
   S->Info = Shdr->sh_info;
 
   auto StringTableShdrOrErr = Obj.getSection(Shdr->sh_link);
   if (!StringTableShdrOrErr)
-    return errorToErrorCode(StringTableShdrOrErr.takeError());
+    return StringTableShdrOrErr.takeError();
 
   auto StringTableOrErr = Obj.getStringTable(*StringTableShdrOrErr);
   if (!StringTableOrErr)
-    return errorToErrorCode(StringTableOrErr.takeError());
+    return StringTableOrErr.takeError();
 
   auto Contents = Obj.getSectionContents(Shdr);
   if (!Contents)
-    return errorToErrorCode(Contents.takeError());
+    return Contents.takeError();
 
   llvm::ArrayRef<uint8_t> Data = *Contents;
   const uint8_t *Buf = Data.data();
   while (Buf) {
     const Elf_Verdef *Verdef = reinterpret_cast<const Elf_Verdef *>(Buf);
     ELFYAML::VerdefEntry Entry;
     Entry.Version = Verdef->vd_version;
     Entry.Flags = Verdef->vd_flags;
while (Buf) {
     S->Entries.push_back(Entry);
     Buf = Verdef->vd_next ? Buf + Verdef->vd_next : nullptr;
   }
 
   return S.release();
 }
 
 template <class ELFT>
-ErrorOr<ELFYAML::SymverSection *>
+Expected<ELFYAML::SymverSection *>
 ELFDumper<ELFT>::dumpSymverSection(const Elf_Shdr *Shdr) {
   typedef typename ELFT::Half Elf_Half;
 
   auto S = make_unique<ELFYAML::SymverSection>();
-  if (std::error_code EC = dumpCommonSection(Shdr, *S))
-    return EC;
+  if (Error E = dumpCommonSection(Shdr, *S))
+    return std::move(E);
 
   auto VersionsOrErr = Obj.template getSectionContentsAsArray<Elf_Half>(Shdr);
   if (!VersionsOrErr)
-    return errorToErrorCode(VersionsOrErr.takeError());
+    return VersionsOrErr.takeError();
   for (const Elf_Half &E : *VersionsOrErr)
     S->Entries.push_back(E);
 
   return S.release();
 }
 
 template <class ELFT>
-ErrorOr<ELFYAML::VerneedSection *>
+Expected<ELFYAML::VerneedSection *>
 ELFDumper<ELFT>::dumpVerneedSection(const Elf_Shdr *Shdr) {
   typedef typename ELFT::Verneed Elf_Verneed;
   typedef typename ELFT::Vernaux Elf_Vernaux;
 
   auto S = make_unique<ELFYAML::VerneedSection>();
-  if (std::error_code EC = dumpCommonSection(Shdr, *S))
-    return EC;
+  if (Error E = dumpCommonSection(Shdr, *S))
+    return std::move(E);
 
   S->Info = Shdr->sh_info;
 
   auto Contents = Obj.getSectionContents(Shdr);
   if (!Contents)
-    return errorToErrorCode(Contents.takeError());
+    return Contents.takeError();
 
   auto StringTableShdrOrErr = Obj.getSection(Shdr->sh_link);
   if (!StringTableShdrOrErr)
-    return errorToErrorCode(StringTableShdrOrErr.takeError());
+    return StringTableShdrOrErr.takeError();
 
   auto StringTableOrErr = Obj.getStringTable(*StringTableShdrOrErr);
   if (!StringTableOrErr)
-    return errorToErrorCode(StringTableOrErr.takeError());
+    return StringTableOrErr.takeError();
 
   llvm::ArrayRef<uint8_t> Data = *Contents;
   const uint8_t *Buf = Data.data();
   while (Buf) {
     const Elf_Verneed *Verneed = reinterpret_cast<const Elf_Verneed *>(Buf);
 
     ELFYAML::VerneedEntry Entry;
     Entry.Version = Verneed->vn_version;
while (Buf) {
     S->VerneedV.push_back(Entry);
     Buf = Verneed->vn_next ? Buf + Verneed->vn_next : nullptr;
   }
 
   return S.release();
 }
 
 template <class ELFT>
-ErrorOr<ELFYAML::Group *> ELFDumper<ELFT>::dumpGroup(const Elf_Shdr *Shdr) {
+Expected<ELFYAML::Group *> ELFDumper<ELFT>::dumpGroup(const Elf_Shdr *Shdr) {
   auto S = make_unique<ELFYAML::Group>();
-  if (std::error_code EC = dumpCommonSection(Shdr, *S))
-    return EC;
+  if (Error E = dumpCommonSection(Shdr, *S))
+    return std::move(E);
 
   auto SymtabOrErr = Obj.getSection(Shdr->sh_link);
   if (!SymtabOrErr)
-    return errorToErrorCode(SymtabOrErr.takeError());
+    return SymtabOrErr.takeError();
   // Get symbol with index sh_info which name is the signature of the group.
   const Elf_Shdr *Symtab = *SymtabOrErr;
   auto SymOrErr = Obj.getSymbol(Symtab, Shdr->sh_info);
   if (!SymOrErr)
-    return errorToErrorCode(SymOrErr.takeError());
+    return SymOrErr.takeError();
   auto StrTabOrErr = Obj.getStringTableForSymtab(*Symtab);
   if (!StrTabOrErr)
-    return errorToErrorCode(StrTabOrErr.takeError());
+    return StrTabOrErr.takeError();
 
   Expected<StringRef> SymbolName =
       getUniquedSymbolName(*SymOrErr, *StrTabOrErr, Symtab);
   if (!SymbolName)
-    return errorToErrorCode(SymbolName.takeError());
+    return SymbolName.takeError();
   S->Signature = *SymbolName;
 
   auto MembersOrErr = Obj.template getSectionContentsAsArray<Elf_Word>(Shdr);
   if (!MembersOrErr)
-    return errorToErrorCode(MembersOrErr.takeError());
+    return MembersOrErr.takeError();
 
   for (Elf_Word Member : *MembersOrErr) {
     if (Member == llvm::ELF::GRP_COMDAT) {
       S->Members.push_back({"GRP_COMDAT"});
       continue;
     }
 
     auto SHdrOrErr = Obj.getSection(Member);
     if (!SHdrOrErr)
-      return errorToErrorCode(SHdrOrErr.takeError());
+      return SHdrOrErr.takeError();
     auto NameOrErr = getUniquedSectionName(*SHdrOrErr);
     if (!NameOrErr)
-      return errorToErrorCode(NameOrErr.takeError());
+      return NameOrErr.takeError();
     S->Members.push_back({*NameOrErr});
   }
   return S.release();
 }
 
 template <class ELFT>
-ErrorOr<ELFYAML::MipsABIFlags *>
+Expected<ELFYAML::MipsABIFlags *>
 ELFDumper<ELFT>::dumpMipsABIFlags(const Elf_Shdr *Shdr) {
   assert(Shdr->sh_type == ELF::SHT_MIPS_ABIFLAGS &&
          "Section type is not SHT_MIPS_ABIFLAGS");
   auto S = make_unique<ELFYAML::MipsABIFlags>();
-  if (std::error_code EC = dumpCommonSection(Shdr, *S))
-    return EC;
+  if (Error E = dumpCommonSection(Shdr, *S))
+    return std::move(E);
 
   auto ContentOrErr = Obj.getSectionContents(Shdr);
   if (!ContentOrErr)
-    return errorToErrorCode(ContentOrErr.takeError());
+    return ContentOrErr.takeError();
 
   auto *Flags = reinterpret_cast<const object::Elf_Mips_ABIFlags<ELFT> *>(
       ContentOrErr.get().data());
   S->Version = Flags->version;
   S->ISALevel = Flags->isa_level;
   S->ISARevision = Flags->isa_rev;
   S->GPRSize = Flags->gpr_size;
   S->CPR1Size = Flags->cpr1_size;
   S->CPR2Size = Flags->cpr2_size;
   S->FpABI = Flags->fp_abi;
   S->ISAExtension = Flags->isa_ext;
   S->ASEs = Flags->ases;
   S->Flags1 = Flags->flags1;
   S->Flags2 = Flags->flags2;
   return S.release();
 }
 
 template <class ELFT>
-static std::error_code elf2yaml(raw_ostream &Out,
-                                const object::ELFFile<ELFT> &Obj) {
+static Error elf2yaml(raw_ostream &Out, const object::ELFFile<ELFT> &Obj) {
   ELFDumper<ELFT> Dumper(Obj);
-  ErrorOr<ELFYAML::Object *> YAMLOrErr = Dumper.dump();
-  if (std::error_code EC = YAMLOrErr.getError())
-    return EC;
+  Expected<ELFYAML::Object *> YAMLOrErr = Dumper.dump();
+  if (!YAMLOrErr)
+    return YAMLOrErr.takeError();
 
   std::unique_ptr<ELFYAML::Object> YAML(YAMLOrErr.get());
   yaml::Output Yout(Out);
   Yout << *YAML;
 
-  return std::error_code();
+  return Error::success();
 }
 
-std::error_code elf2yaml(raw_ostream &Out, const object::ObjectFile &Obj) {
+Error elf2yaml(raw_ostream &Out, const object::ObjectFile &Obj) {
   if (const auto *ELFObj = dyn_cast<object::ELF32LEObjectFile>(&Obj))
     return elf2yaml(Out, *ELFObj->getELFFile());
 
   if (const auto *ELFObj = dyn_cast<object::ELF32BEObjectFile>(&Obj))
     return elf2yaml(Out, *ELFObj->getELFFile());
 
   if (const auto *ELFObj = dyn_cast<object::ELF64LEObjectFile>(&Obj))
     return elf2yaml(Out, *ELFObj->getELFFile());
 
   if (const auto *ELFObj = dyn_cast<object::ELF64BEObjectFile>(&Obj))
     return elf2yaml(Out, *ELFObj->getELFFile());
 
-  return obj2yaml_error::unsupported_obj_file_format;
+  llvm_unreachable("unknown ELF file format");
 }

llvm/trunk/tools/obj2yaml/obj2yaml.h

 #include "llvm/Object/Minidump.h"
 #include "llvm/Object/Wasm.h"
 #include "llvm/Object/XCOFFObjectFile.h"
 #include "llvm/Support/raw_ostream.h"
 #include <system_error>
 
 std::error_code coff2yaml(llvm::raw_ostream &Out,
                           const llvm::object::COFFObjectFile &Obj);
-std::error_code elf2yaml(llvm::raw_ostream &Out,
+llvm::Error elf2yaml(llvm::raw_ostream &Out,
                          const llvm::object::ObjectFile &Obj);
 std::error_code macho2yaml(llvm::raw_ostream &Out,
                            const llvm::object::Binary &Obj);
 llvm::Error minidump2yaml(llvm::raw_ostream &Out,
                           const llvm::object::MinidumpFile &Obj);
 std::error_code xcoff2yaml(llvm::raw_ostream &Out,
                            const llvm::object::XCOFFObjectFile &Obj);
 std::error_code wasm2yaml(llvm::raw_ostream &Out,

llvm/trunk/tools/obj2yaml/obj2yaml.cpp

 #include "llvm/Object/COFF.h"
 #include "llvm/Object/Minidump.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/InitLLVM.h"
 
 using namespace llvm;
 using namespace llvm::object;
 
-static std::error_code dumpObject(const ObjectFile &Obj) {
+static Error dumpObject(const ObjectFile &Obj) {
   if (Obj.isCOFF())
-    return coff2yaml(outs(), cast<COFFObjectFile>(Obj));
+    return errorCodeToError(coff2yaml(outs(), cast<COFFObjectFile>(Obj)));
 
   if (Obj.isXCOFF())
-    return xcoff2yaml(outs(), cast<XCOFFObjectFile>(Obj));
+    return errorCodeToError(xcoff2yaml(outs(), cast<XCOFFObjectFile>(Obj)));
 
   if (Obj.isELF())
     return elf2yaml(outs(), Obj);
+
   if (Obj.isWasm())
-    return wasm2yaml(outs(), cast<WasmObjectFile>(Obj));
+    return errorCodeToError(wasm2yaml(outs(), cast<WasmObjectFile>(Obj)));
 
-  return obj2yaml_error::unsupported_obj_file_format;
+  return errorCodeToError(obj2yaml_error::unsupported_obj_file_format);
 }
 
 static Error dumpInput(StringRef File) {
   Expected<OwningBinary<Binary>> BinaryOrErr = createBinary(File);
   if (!BinaryOrErr)
     return BinaryOrErr.takeError();
 
   Binary &Binary = *BinaryOrErr.get().getBinary();
   // Universal MachO is not a subclass of ObjectFile, so it needs to be handled
   // here with the other binary types.
   if (Binary.isMachO() || Binary.isMachOUniversalBinary())
     return errorCodeToError(macho2yaml(outs(), Binary));
   // TODO: If this is an archive, then burst it and dump each entry
   if (ObjectFile *Obj = dyn_cast<ObjectFile>(&Binary))
-    return errorCodeToError(dumpObject(*Obj));
+    return dumpObject(*Obj);
   if (MinidumpFile *Minidump = dyn_cast<MinidumpFile>(&Binary))
     return minidump2yaml(outs(), *Minidump);
 
   return Error::success();
 }
 
 static void reportError(StringRef Input, Error Err) {
   if (Input == "-")