[COFF] Migrate COFFObjectFile to Expected<T>

[rnk]

Summary

I noticed that std::error_code() does one-time initialization. Avoid
that overhead with Expected<T> and llvm::Error. Also, it is consistent
with the virtual interface and ELF, and generally cleaner.

[rnk]

• Also convert getSection, which is hot

[MaskRay]

llvm/tools/obj2yaml/coff2yaml.cpp

142

In the binary utilities (e.g. obj2yaml), we tend to not capitalize error messages. I think the error message may be temporary, because we should propagate the error to the caller, instead of calling exit directly when something goes wrong.

[rnk]

llvm/tools/obj2yaml/coff2yaml.cpp

142

I'll lower case it. But, seeing as this is tool code, should we really propagate the error to the caller? Shouldn't the tool just exit here?

lld/COFF/Chunks.cpp

static void maybeReportRelocationToDiscarded(const SectionChunk *fromChunk,
   // have to go back to the object file.
   ObjFile *file = fromChunk->file;
   StringRef name;
   if (sym) {
     name = sym->getName();
   } else {
     COFFSymbolRef coffSym =
         check(file->getCOFFObj()->getSymbol(rel.SymbolTableIndex));
-    file->getCOFFObj()->getSymbolName(coffSym, name);
+    name = check(file->getCOFFObj()->getSymbolName(coffSym));
   }
 
   std::vector<std::string> symbolLocations =
       getSymbolLocations(file, rel.SymbolTableIndex);
 
   std::string out;
   llvm::raw_string_ostream os(out);
   os << "relocation against symbol in discarded section: " + name;

lld/COFF/InputFiles.cpp

void LazyObjFile::parse() {
   std::unique_ptr<Binary> coffObjPtr = CHECK(createBinary(mb), this);
   COFFObjectFile *coffObj = cast<COFFObjectFile>(coffObjPtr.get());
   uint32_t numSymbols = coffObj->getNumberOfSymbols();
   for (uint32_t i = 0; i < numSymbols; ++i) {
     COFFSymbolRef coffSym = check(coffObj->getSymbol(i));
     if (coffSym.isUndefined() || !coffSym.isExternal() ||
         coffSym.isWeakExternal())
       continue;
-    StringRef name;
-    coffObj->getSymbolName(coffSym, name);
+    StringRef name = check(coffObj->getSymbolName(coffSym));
     if (coffSym.isAbsolute() && ignoredSymbolName(name))
       continue;
     symtab->addLazyObject(this, name);
     i += coffSym.getNumberOfAuxSymbols();
   }
 }
 
 void ObjFile::parse() {
void ObjFile::parse() {
 
   // Read section and symbol tables.
   initializeChunks();
   initializeSymbols();
   initializeFlags();
   initializeDependencies();
 }
 
-const coff_section* ObjFile::getSection(uint32_t i) {
-  const coff_section *sec;
-  if (auto ec = coffObj->getSection(i, sec))
-    fatal("getSection failed: #" + Twine(i) + ": " + ec.message());
-  return sec;
+const coff_section *ObjFile::getSection(uint32_t i) {
+  auto sec = coffObj->getSection(i);
+  if (!sec)
+    fatal("getSection failed: #" + Twine(i) + ": " + toString(sec.takeError()));
+  return *sec;
 }
 
 // We set SectionChunk pointers in the SparseChunks vector to this value
 // temporarily to mark comdat sections as having an unknown resolution. As we
 // walk the object file's symbol table, once we visit either a leader symbol or
 // an associative section definition together with the parent comdat's leader,
 // we set the pointer to either nullptr (to mark the section as discarded) or a
 // valid SectionChunk for that section.

 
 void ObjFile::readAssociativeDefinition(COFFSymbolRef sym,
                                         const coff_aux_section_definition *def,
                                         uint32_t parentIndex) {
   SectionChunk *parent = sparseChunks[parentIndex];
   int32_t sectionNumber = sym.getSectionNumber();
 
   auto diag = [&]() {
-    StringRef name, parentName;
-    coffObj->getSymbolName(sym, name);
+    StringRef name = check(coffObj->getSymbolName(sym));
 
+    StringRef parentName;
     const coff_section *parentSec = getSection(parentIndex);
     if (Expected<StringRef> e = coffObj->getSectionName(parentSec))
       parentName = *e;
     error(toString(this) + ": associative comdat " + name + " (sec " +
           Twine(sectionNumber) + ") has invalid reference to section " +
           parentName + " (sec " + Twine(parentIndex) + ")");
   };
 
void ObjFile::recordPrevailingSymbolForMingw(
   // For comdat symbols in executable sections, where this is the copy
   // of the section chunk we actually include instead of discarding it,
   // add the symbol to a map to allow using it for implicitly
   // associating .[px]data$<func> sections to it.
   int32_t sectionNumber = sym.getSectionNumber();
   SectionChunk *sc = sparseChunks[sectionNumber];
   if (sc && sc->getOutputCharacteristics() & IMAGE_SCN_MEM_EXECUTE) {
     StringRef name;
-    coffObj->getSymbolName(sym, name);
+    name = check(coffObj->getSymbolName(sym));
     if (getMachineType() == I386)
       name.consume_front("_");
     prevailingSectionMap[name] = sectionNumber;
   }
 }
 
 void ObjFile::maybeAssociateSEHForMingw(
     COFFSymbolRef sym, const coff_aux_section_definition *def,
     const DenseMap<StringRef, uint32_t> &prevailingSectionMap) {
-  StringRef name;
-  coffObj->getSymbolName(sym, name);
+  StringRef name = check(coffObj->getSymbolName(sym));
   if (name.consume_front(".pdata$") || name.consume_front(".xdata$") ||
       name.consume_front(".eh_frame$")) {
     // For MinGW, treat .[px]data$<func> and .eh_frame$<func> as implicitly
     // associative to the symbol <func>.
     auto parentSym = prevailingSectionMap.find(name);
     if (parentSym != prevailingSectionMap.end())
       readAssociativeDefinition(sym, def, parentSym->second);
   }
 }
 
 Symbol *ObjFile::createRegular(COFFSymbolRef sym) {
   SectionChunk *sc = sparseChunks[sym.getSectionNumber()];
   if (sym.isExternal()) {
-    StringRef name;
-    coffObj->getSymbolName(sym, name);
+    StringRef name = check(coffObj->getSymbolName(sym));
     if (sc)
       return symtab->addRegular(this, name, sym.getGeneric(), sc,
                                 sym.getValue());
     // For MinGW symbols named .weak.* that point to a discarded section,
     // don't create an Undefined symbol. If nothing ever refers to the symbol,
     // everything should be fine. If something actually refers to the symbol
     // (e.g. the undefined weak alias), linking will fail due to undefined
     // references at the end.
for (uint32_t i : pendingIndexes) {
     COFFSymbolRef sym = check(coffObj->getSymbol(i));
     if (const coff_aux_section_definition *def = sym.getSectionDefinition()) {
       if (def->Selection == IMAGE_COMDAT_SELECT_ASSOCIATIVE)
         readAssociativeDefinition(sym, def);
       else if (config->mingw)
         maybeAssociateSEHForMingw(sym, def, prevailingSectionMap);
     }
     if (sparseChunks[sym.getSectionNumber()] == pendingComdat) {
-      StringRef name;
-      coffObj->getSymbolName(sym, name);
+      StringRef name = check(coffObj->getSymbolName(sym));
       log("comdat section " + name +
           " without leader and unassociated, discarding");
       continue;
     }
     symbols[i] = createRegular(sym);
   }
 
   for (auto &kv : weakAliases) {
     Symbol *sym = kv.first;
     uint32_t idx = kv.second;
     checkAndSetWeakAlias(symtab, this, sym, symbols[idx]);
   }
 }
 
 Symbol *ObjFile::createUndefined(COFFSymbolRef sym) {
-  StringRef name;
-  coffObj->getSymbolName(sym, name);
+  StringRef name = check(coffObj->getSymbolName(sym));
   return symtab->addUndefined(name, this, sym.isWeakExternal());
 }
 
 void ObjFile::handleComdatSelection(COFFSymbolRef sym, COMDATType &selection,
                                     bool &prevailing, DefinedRegular *leader) {
   if (prevailing)
     return;
   // There's already an existing comdat for this symbol: `Leader`.
case IMAGE_COMDAT_SELECT_ASSOCIATIVE:
     // (This means lld-link doesn't produce duplicate symbol errors for
     // associative comdats while link.exe does, but associate comdats
     // are never extern in practice.)
     llvm_unreachable("createDefined not called for associative comdats");
 
   case IMAGE_COMDAT_SELECT_LARGEST:
     if (leaderChunk->getSize() < getSection(sym)->SizeOfRawData) {
       // Replace the existing comdat symbol with the new one.
-      StringRef name;
-      coffObj->getSymbolName(sym, name);
+      StringRef name = check(coffObj->getSymbolName(sym));
       // FIXME: This is incorrect: With /opt:noref, the previous sections
       // make it into the final executable as well. Correct handling would
       // be to undo reading of the whole old section that's being replaced,
       // or doing one pass that determines what the final largest comdat
       // is for all IMAGE_COMDAT_SELECT_LARGEST comdats and then reading
       // only the largest one.
       replaceSymbol<DefinedRegular>(leader, this, name, /*IsCOMDAT*/ true,
                                     /*IsExternal*/ true, sym.getGeneric(),
                                     nullptr);
       prevailing = true;
     }
     break;
 
   case IMAGE_COMDAT_SELECT_NEWEST:
     llvm_unreachable("should have been rejected earlier");
   }
 }
 
 Optional<Symbol *> ObjFile::createDefined(
     COFFSymbolRef sym,
     std::vector<const coff_aux_section_definition *> &comdatDefs,
     bool &prevailing) {
   prevailing = false;
-  auto getName = [&]() {
-    StringRef s;
-    coffObj->getSymbolName(sym, s);
-    return s;
-  };
+  auto getName = [&]() { return check(coffObj->getSymbolName(sym)); };
 
   if (sym.isCommon()) {
     auto *c = make<CommonChunk>(sym);
     chunks.push_back(c);
     return symtab->addCommon(this, getName(), sym.getValue(), sym.getGeneric(),
                              c);
   }
 

lld/COFF/Symbols.cpp

 }
 
 namespace coff {
 
 void Symbol::computeName() {
   assert(nameData == nullptr &&
          "should only compute the name once for DefinedCOFF symbols");
   auto *d = cast<DefinedCOFF>(this);
-  StringRef nameStr;
-  cast<ObjFile>(d->file)->getCOFFObj()->getSymbolName(d->sym, nameStr);
+  StringRef nameStr =
+      check(cast<ObjFile>(d->file)->getCOFFObj()->getSymbolName(d->sym));
   nameData = nameStr.data();
   nameSize = nameStr.size();
   assert(nameSize == nameStr.size() && "name length truncated");
 }
 
 InputFile *Symbol::getFile() {
   if (auto *sym = dyn_cast<DefinedCOFF>(this))
     return sym->file;

llvm/include/llvm/Object/COFF.h

private:
   const export_directory_table_entry *ExportDirectory;
   const coff_base_reloc_block_header *BaseRelocHeader;
   const coff_base_reloc_block_header *BaseRelocEnd;
   const debug_directory *DebugDirectoryBegin;
   const debug_directory *DebugDirectoryEnd;
   // Either coff_load_configuration32 or coff_load_configuration64.
   const void *LoadConfig = nullptr;
 
-  std::error_code getString(uint32_t offset, StringRef &Res) const;
+  Expected<StringRef> getString(uint32_t offset) const;
 
   template <typename coff_symbol_type>
   const coff_symbol_type *toSymb(DataRefImpl Symb) const;
   const coff_section *toSec(DataRefImpl Sec) const;
   const coff_relocation *toRel(DataRefImpl Rel) const;
 
   std::error_code initSymbolTablePtr();
   std::error_code initImportTablePtr();
public:
   const coff_bigobj_file_header *getCOFFBigObjHeader() const {
     return COFFBigObjHeader;
   }
   const pe32_header *getPE32Header() const { return PE32Header; }
   const pe32plus_header *getPE32PlusHeader() const { return PE32PlusHeader; }
 
   std::error_code getDataDirectory(uint32_t index,
                                    const data_directory *&Res) const;
-  std::error_code getSection(int32_t index, const coff_section *&Res) const;
-  std::error_code getSection(StringRef SectionName,
-                             const coff_section *&Res) const;
+  Expected<const coff_section *> getSection(int32_t index) const;
 
-  template <typename coff_symbol_type>
-  std::error_code getSymbol(uint32_t Index,
-                            const coff_symbol_type *&Res) const {
-    if (Index >= getNumberOfSymbols())
-      return object_error::parse_failed;
-
-    Res = reinterpret_cast<coff_symbol_type *>(getSymbolTable()) + Index;
-    return std::error_code();
-  }
   Expected<COFFSymbolRef> getSymbol(uint32_t index) const {
-    if (SymbolTable16) {
-      const coff_symbol16 *Symb = nullptr;
-      if (std::error_code EC = getSymbol(index, Symb))
-        return errorCodeToError(EC);
-      return COFFSymbolRef(Symb);
-    }
-    if (SymbolTable32) {
-      const coff_symbol32 *Symb = nullptr;
-      if (std::error_code EC = getSymbol(index, Symb))
-        return errorCodeToError(EC);
-      return COFFSymbolRef(Symb);
-    }
+    if (index >= getNumberOfSymbols())
+      return errorCodeToError(object_error::parse_failed);
+    if (SymbolTable16)
+      return COFFSymbolRef(SymbolTable16 + index);
+    if (SymbolTable32)
+      return COFFSymbolRef(SymbolTable32 + index);
     return errorCodeToError(object_error::parse_failed);
   }
 
   template <typename T>
   std::error_code getAuxSymbol(uint32_t index, const T *&Res) const {
     Expected<COFFSymbolRef> S = getSymbol(index);
     if (Error E = S.takeError())
       return errorToErrorCode(std::move(E));
     Res = reinterpret_cast<const T *>(S->getRawPtr());
     return std::error_code();
   }
 
-  std::error_code getSymbolName(COFFSymbolRef Symbol, StringRef &Res) const;
-  std::error_code getSymbolName(const coff_symbol_generic *Symbol,
-                                StringRef &Res) const;
+  Expected<StringRef> getSymbolName(COFFSymbolRef Symbol) const;
+  Expected<StringRef> getSymbolName(const coff_symbol_generic *Symbol) const;
 
   ArrayRef<uint8_t> getSymbolAuxData(COFFSymbolRef Symbol) const;
 
   uint32_t getSymbolIndex(COFFSymbolRef Symbol) const;
 
   size_t getSymbolTableEntrySize() const {
     if (COFFHeader)
       return sizeof(coff_symbol16);

llvm/lib/Object/COFFObjectFile.cpp

if (SymbolTable16) {
     Symb += 1 + Symb->NumberOfAuxSymbols;
     Ref.p = std::min(reinterpret_cast<uintptr_t>(Symb), End);
   } else {
     llvm_unreachable("no symbol table pointer!");
   }
 }
 
 Expected<StringRef> COFFObjectFile::getSymbolName(DataRefImpl Ref) const {
-  COFFSymbolRef Symb = getCOFFSymbol(Ref);
-  StringRef Result;
-  if (std::error_code EC = getSymbolName(Symb, Result))
-    return errorCodeToError(EC);
-  return Result;
+  return getSymbolName(getCOFFSymbol(Ref));
 }
 
 uint64_t COFFObjectFile::getSymbolValueImpl(DataRefImpl Ref) const {
   return getCOFFSymbol(Ref).getValue();
 }
 
 uint32_t COFFObjectFile::getSymbolAlignment(DataRefImpl Ref) const {
   // MSVC/link.exe seems to align symbols to the next-power-of-2
   // up to 32 bytes.
   COFFSymbolRef Symb = getCOFFSymbol(Ref);
   return std::min(uint64_t(32), PowerOf2Ceil(Symb.getValue()));
 }
 
 Expected<uint64_t> COFFObjectFile::getSymbolAddress(DataRefImpl Ref) const {
   uint64_t Result = cantFail(getSymbolValue(Ref));
   COFFSymbolRef Symb = getCOFFSymbol(Ref);
   int32_t SectionNumber = Symb.getSectionNumber();
 
   if (Symb.isAnyUndefined() || Symb.isCommon() ||
       COFF::isReservedSectionNumber(SectionNumber))
     return Result;
 
-  const coff_section *Section = nullptr;
-  if (std::error_code EC = getSection(SectionNumber, Section))
-    return errorCodeToError(EC);
-  Result += Section->VirtualAddress;
+  Expected<const coff_section *> Section = getSection(SectionNumber);
+  if (!Section)
+    return Section.takeError();
+  Result += (*Section)->VirtualAddress;
 
   // The section VirtualAddress does not include ImageBase, and we want to
   // return virtual addresses.
   Result += getImageBase();
 
   return Result;
 }
 
uint64_t COFFObjectFile::getCommonSymbolSizeImpl(DataRefImpl Ref) const {
   return Symb.getValue();
 }
 
 Expected<section_iterator>
 COFFObjectFile::getSymbolSection(DataRefImpl Ref) const {
   COFFSymbolRef Symb = getCOFFSymbol(Ref);
   if (COFF::isReservedSectionNumber(Symb.getSectionNumber()))
     return section_end();
-  const coff_section *Sec = nullptr;
-  if (std::error_code EC = getSection(Symb.getSectionNumber(), Sec))
-    return errorCodeToError(EC);
+  Expected<const coff_section *> Sec = getSection(Symb.getSectionNumber());
+  if (!Sec)
+    return Sec.takeError();
   DataRefImpl Ret;
-  Ret.p = reinterpret_cast<uintptr_t>(Sec);
+  Ret.p = reinterpret_cast<uintptr_t>(*Sec);
   return section_iterator(SectionRef(Ret, this));
 }
 
 unsigned COFFObjectFile::getSymbolSectionID(SymbolRef Sym) const {
   COFFSymbolRef Symb = getCOFFSymbol(Sym.getRawDataRefImpl());
   return Symb.getSectionNumber();
 }
 
COFFObjectFile::getDataDirectory(uint32_t Index,
   if (Index >= NumEnt) {
     Res = nullptr;
     return object_error::parse_failed;
   }
   Res = &DataDirectory[Index];
   return std::error_code();
 }
 
-std::error_code COFFObjectFile::getSection(int32_t Index,
-                                           const coff_section *&Result) const {
-  Result = nullptr;
+Expected<const coff_section *> COFFObjectFile::getSection(int32_t Index) const {
+  // Perhaps getting the section of a reserved section index should be an error,
+  // but callers rely on this to return null.
   if (COFF::isReservedSectionNumber(Index))
-    return std::error_code();
+    return (const coff_section *)nullptr;
   if (static_cast<uint32_t>(Index) <= getNumberOfSections()) {
     // We already verified the section table data, so no need to check again.
-    Result = SectionTable + (Index - 1);
-    return std::error_code();
-  }
-  return object_error::parse_failed;
-}
-
-std::error_code COFFObjectFile::getSection(StringRef SectionName,
-                                           const coff_section *&Result) const {
-  Result = nullptr;
-  for (const SectionRef &Section : sections()) {
-    auto NameOrErr = Section.getName();
-    if (!NameOrErr)
-      return errorToErrorCode(NameOrErr.takeError());
-
-    if (*NameOrErr == SectionName) {
-      Result = getCOFFSection(Section);
-      return std::error_code();
-    }
+    return SectionTable + (Index - 1);
   }
-  return object_error::parse_failed;
+  return errorCodeToError(object_error::parse_failed);
 }
 
-std::error_code COFFObjectFile::getString(uint32_t Offset,
-                                          StringRef &Result) const {
+Expected<StringRef> COFFObjectFile::getString(uint32_t Offset) const {
   if (StringTableSize <= 4)
     // Tried to get a string from an empty string table.
-    return object_error::parse_failed;
+    return errorCodeToError(object_error::parse_failed);
   if (Offset >= StringTableSize)
-    return object_error::unexpected_eof;
-  Result = StringRef(StringTable + Offset);
-  return std::error_code();
+    return errorCodeToError(object_error::unexpected_eof);
+  return StringRef(StringTable + Offset);
 }
 
-std::error_code COFFObjectFile::getSymbolName(COFFSymbolRef Symbol,
-                                              StringRef &Res) const {
-  return getSymbolName(Symbol.getGeneric(), Res);
+Expected<StringRef> COFFObjectFile::getSymbolName(COFFSymbolRef Symbol) const {
+  return getSymbolName(Symbol.getGeneric());
 }
 
-std::error_code COFFObjectFile::getSymbolName(const coff_symbol_generic *Symbol,
-                                              StringRef &Res) const {
+Expected<StringRef>
+COFFObjectFile::getSymbolName(const coff_symbol_generic *Symbol) const {
   // Check for string table entry. First 4 bytes are 0.
-  if (Symbol->Name.Offset.Zeroes == 0) {
-    if (std::error_code EC = getString(Symbol->Name.Offset.Offset, Res))
-      return EC;
-    return std::error_code();
-  }
+  if (Symbol->Name.Offset.Zeroes == 0)
+    return getString(Symbol->Name.Offset.Offset);
 
-  if (Symbol->Name.ShortName[COFF::NameSize - 1] == 0)
-    // Null terminated, let ::strlen figure out the length.
-    Res = StringRef(Symbol->Name.ShortName);
-  else
-    // Not null terminated, use all 8 bytes.
-    Res = StringRef(Symbol->Name.ShortName, COFF::NameSize);
-  return std::error_code();
+  if (Symbol->Name.ShortName[COFF::NameSize - 1] == 0)
+    return StringRef(Symbol->Name.ShortName);
+
+  return StringRef(Symbol->Name.ShortName, COFF::NameSize);
 }
 
 ArrayRef<uint8_t>
 COFFObjectFile::getSymbolAuxData(COFFSymbolRef Symbol) const {
   const uint8_t *Aux = nullptr;
 
   size_t SymbolSize = getSymbolTableEntrySize();
   if (Symbol.getNumberOfAuxSymbols() > 0) {
if (Name.startswith("//")) {
       if (decodeBase64StringEntry(Name.substr(2), Offset))
         return createStringError(object_error::parse_failed,
                                  "invalid section name");
     } else {
       if (Name.substr(1).getAsInteger(10, Offset))
         return createStringError(object_error::parse_failed,
                                  "invalid section name");
     }
-    if (std::error_code EC = getString(Offset, Name))
-      return errorCodeToError(EC);
+    return getString(Offset);
   }
 
   return Name;
 }
 
 uint64_t COFFObjectFile::getSectionSize(const coff_section *Sec) const {
   // SizeOfRawData and VirtualSize change what they represent depending on
   // whether or not we have an executable image.
if (RelocsForOffset.first != RelocsForOffset.second) {
     }
     if (R.Type != RVAReloc)
       return createStringError(object_error::parse_failed,
                                "unexpected relocation type");
     // Get the relocation's symbol
     Expected<COFFSymbolRef> Sym = Obj->getSymbol(R.SymbolTableIndex);
     if (!Sym)
       return Sym.takeError();
-    const coff_section *Section = nullptr;
     // And the symbol's section
-    if (std::error_code EC = Obj->getSection(Sym->getSectionNumber(), Section))
-      return errorCodeToError(EC);
+    Expected<const coff_section *> Section =
+        Obj->getSection(Sym->getSectionNumber());
+    if (!Section)
+      return Section.takeError();
     // Add the initial value of DataRVA to the symbol's offset to find the
     // data it points at.
     uint64_t Offset = Entry.DataRVA + Sym->getValue();
     ArrayRef<uint8_t> Contents;
-    if (Error E = Obj->getSectionContents(Section, Contents))
+    if (Error E = Obj->getSectionContents(*Section, Contents))
       return std::move(E);
     if (Offset + Entry.DataSize > Contents.size())
       return createStringError(object_error::parse_failed,
                                "data outside of section");
     // Return a reference to the data inside the section.
     return StringRef(reinterpret_cast<const char *>(Contents.data()) + Offset,
                      Entry.DataSize);
   } else {

llvm/tools/llvm-objcopy/COFF/Reader.cpp

Error COFFReader::readExecutableHeaders(Object &Obj) const {
   }
   return Error::success();
 }
 
 Error COFFReader::readSections(Object &Obj) const {
   std::vector<Section> Sections;
   // Section indexing starts from 1.
   for (size_t I = 1, E = COFFObj.getNumberOfSections(); I <= E; I++) {
-    const coff_section *Sec;
-    if (auto EC = COFFObj.getSection(I, Sec))
-      return errorCodeToError(EC);
+    Expected<const coff_section *> SecOrErr = COFFObj.getSection(I);
+    if (!SecOrErr)
+      return SecOrErr.takeError();
+    const coff_section *Sec = *SecOrErr;
     Sections.push_back(Section());
     Section &S = Sections.back();
     S.Header = *Sec;
     S.Header.Characteristics &= ~COFF::IMAGE_SCN_LNK_NRELOC_OVFL;
     ArrayRef<uint8_t> Contents;
     if (Error E = COFFObj.getSectionContents(Sec, Contents))
       return E;
     S.setContentsRef(Contents);
for (uint32_t I = 0, E = COFFObj.getRawNumberOfSymbols(); I < E;) {
     Symbol &Sym = Symbols.back();
     // Copy symbols from the original form into an intermediate coff_symbol32.
     if (IsBigObj)
       copySymbol(Sym.Sym,
                  *reinterpret_cast<const coff_symbol32 *>(SymRef.getRawPtr()));
     else
       copySymbol(Sym.Sym,
                  *reinterpret_cast<const coff_symbol16 *>(SymRef.getRawPtr()));
-    if (auto EC = COFFObj.getSymbolName(SymRef, Sym.Name))
-      return errorCodeToError(EC);
+    auto NameOrErr = COFFObj.getSymbolName(SymRef);
+    if (!NameOrErr)
+      return NameOrErr.takeError();
+    Sym.Name = *NameOrErr;
 
     ArrayRef<uint8_t> AuxData = COFFObj.getSymbolAuxData(SymRef);
     size_t SymSize = IsBigObj ? sizeof(coff_symbol32) : sizeof(coff_symbol16);
     assert(AuxData.size() == SymSize * SymRef.getNumberOfAuxSymbols());
     // The auxillary symbols are structs of sizeof(coff_symbol16) each.
     // In the big object format (where symbols are coff_symbol32), each
     // auxillary symbol is padded with 2 bytes at the end. Copy each
     // auxillary symbol to the Sym.AuxData vector. For file symbols,

llvm/tools/llvm-objdump/COFFDump.cpp

 }
 
 void objdump::printCOFFSymbolTable(const COFFObjectFile *coff) {
   for (unsigned SI = 0, SE = coff->getNumberOfSymbols(); SI != SE; ++SI) {
     Expected<COFFSymbolRef> Symbol = coff->getSymbol(SI);
     if (!Symbol)
       reportError(Symbol.takeError(), coff->getFileName());
 
-    StringRef Name;
-    if (std::error_code EC = coff->getSymbolName(*Symbol, Name))
-      reportError(errorCodeToError(EC), coff->getFileName());
+    Expected<StringRef> NameOrErr = coff->getSymbolName(*Symbol);
+    if (!NameOrErr)
+      reportError(NameOrErr.takeError(), coff->getFileName());
+    StringRef Name = *NameOrErr;
 
     outs() << "[" << format("%2d", SI) << "]"
            << "(sec " << format("%2d", int(Symbol->getSectionNumber())) << ")"
            << "(fl 0x00)" // Flag bits, which COFF doesn't have.
            << "(ty " << format("%3x", unsigned(Symbol->getType())) << ")"
            << "(scl " << format("%3x", unsigned(Symbol->getStorageClass()))
            << ") "
            << "(nx " << unsigned(Symbol->getNumberOfAuxSymbols()) << ") "

llvm/tools/llvm-readobj/COFFDumper.cpp

if (SectionNumber == llvm::COFF::IMAGE_SYM_UNDEFINED)
     return StringRef("IMAGE_SYM_UNDEFINED");
   return StringRef("");
 }
 
 void COFFDumper::printSymbol(const SymbolRef &Sym) {
   DictScope D(W, "Symbol");
 
   COFFSymbolRef Symbol = Obj->getCOFFSymbol(Sym);
-  const coff_section *Section;
-  if (std::error_code EC = Obj->getSection(Symbol.getSectionNumber(), Section)) {
-    W.startLine() << "Invalid section number: " << EC.message() << "\n";
+  Expected<const coff_section *> SecOrErr =
+      Obj->getSection(Symbol.getSectionNumber());
+  if (!SecOrErr) {
+    W.startLine() << "Invalid section number: " << Symbol.getSectionNumber()
+                  << "\n";
     W.flush();
+    consumeError(SecOrErr.takeError());
     return;
   }
+  const coff_section *Section = *SecOrErr;
 
   StringRef SymbolName;
-  if (Obj->getSymbolName(Symbol, SymbolName))
-    SymbolName = "";
+  if (Expected<StringRef> SymNameOrErr = Obj->getSymbolName(Symbol))
+    SymbolName = *SymNameOrErr;
 
   StringRef SectionName;
-  if (Expected<StringRef> NameOrErr =
+  if (Expected<StringRef> SecNameOrErr =
           getSectionName(Obj, Symbol.getSectionNumber(), Section))
-    SectionName = *NameOrErr;
+    SectionName = *SecNameOrErr;
 
   W.printString("Name", SymbolName);
   W.printNumber("Value", Symbol.getValue());
   W.printNumber("Section", SectionName, Symbol.getSectionNumber());
   W.printEnum  ("BaseType", Symbol.getBaseType(), makeArrayRef(ImageSymType));
   W.printEnum  ("ComplexType", Symbol.getComplexType(),
                                                    makeArrayRef(ImageSymDType));
   W.printEnum  ("StorageClass", Symbol.getStorageClass(),
if (Symbol.isFunctionDefinition()) {
       const coff_aux_weak_external *Aux;
       if (std::error_code EC = getSymbolAuxData(Obj, Symbol, I, Aux))
         reportError(errorCodeToError(EC), Obj->getFileName());
 
       Expected<COFFSymbolRef> Linked = Obj->getSymbol(Aux->TagIndex);
       if (!Linked)
         reportError(Linked.takeError(), Obj->getFileName());
 
-      StringRef LinkedName;
-      if (std::error_code EC = Obj->getSymbolName(*Linked, LinkedName))
-        reportError(errorCodeToError(EC), Obj->getFileName());
+      Expected<StringRef> LinkedName = Obj->getSymbolName(*Linked);
+      if (!LinkedName)
+        reportError(LinkedName.takeError(), Obj->getFileName());
 
       DictScope AS(W, "AuxWeakExternal");
-      W.printNumber("Linked", LinkedName, Aux->TagIndex);
+      W.printNumber("Linked", *LinkedName, Aux->TagIndex);
       W.printEnum  ("Search", Aux->Characteristics,
                     makeArrayRef(WeakExternalCharacteristics));
 
     } else if (Symbol.isFileRecord()) {
       const char *FileName;
       if (std::error_code EC = getSymbolAuxData(Obj, Symbol, I, FileName))
         reportError(errorCodeToError(EC), Obj->getFileName());
       DictScope AS(W, "AuxFileRecord");
if (Symbol.isFunctionDefinition()) {
       W.printNumber("RelocationCount", Aux->NumberOfRelocations);
       W.printNumber("LineNumberCount", Aux->NumberOfLinenumbers);
       W.printHex("Checksum", Aux->CheckSum);
       W.printNumber("Number", AuxNumber);
       W.printEnum("Selection", Aux->Selection, makeArrayRef(ImageCOMDATSelect));
 
       if (Section && Section->Characteristics & COFF::IMAGE_SCN_LNK_COMDAT
           && Aux->Selection == COFF::IMAGE_COMDAT_SELECT_ASSOCIATIVE) {
-        const coff_section *Assoc;
-        StringRef AssocName = "";
-        if (std::error_code EC = Obj->getSection(AuxNumber, Assoc))
-          reportError(errorCodeToError(EC), Obj->getFileName());
-        Expected<StringRef> Res = getSectionName(Obj, AuxNumber, Assoc);
-        if (!Res)
-          reportError(Res.takeError(), Obj->getFileName());
-        AssocName = *Res;
+        Expected<const coff_section *> Assoc = Obj->getSection(AuxNumber);
+        if (!Assoc)
+          reportError(Assoc.takeError(), Obj->getFileName());
+        Expected<StringRef> AssocName = getSectionName(Obj, AuxNumber, *Assoc);
+        if (!AssocName)
+          reportError(AssocName.takeError(), Obj->getFileName());
 
-        W.printNumber("AssocSection", AssocName, AuxNumber);
+        W.printNumber("AssocSection", *AssocName, AuxNumber);
       }
     } else if (Symbol.isCLRToken()) {
       const coff_aux_clr_token *Aux;
       if (std::error_code EC = getSymbolAuxData(Obj, Symbol, I, Aux))
         reportError(errorCodeToError(EC), Obj->getFileName());
 
       Expected<COFFSymbolRef> ReferredSym =
           Obj->getSymbol(Aux->SymbolTableIndex);
       if (!ReferredSym)
         reportError(ReferredSym.takeError(), Obj->getFileName());
 
-      StringRef ReferredName;
-      if (std::error_code EC = Obj->getSymbolName(*ReferredSym, ReferredName))
-        reportError(errorCodeToError(EC), Obj->getFileName());
+      Expected<StringRef> ReferredName = Obj->getSymbolName(*ReferredSym);
+      if (!ReferredName)
+        reportError(ReferredName.takeError(), Obj->getFileName());
 
       DictScope AS(W, "AuxCLRToken");
       W.printNumber("AuxType", Aux->AuxType);
       W.printNumber("Reserved", Aux->Reserved);
-      W.printNumber("SymbolTableIndex", ReferredName, Aux->SymbolTableIndex);
+      W.printNumber("SymbolTableIndex", *ReferredName, Aux->SymbolTableIndex);
 
     } else {
       W.startLine() << "<unhandled auxiliary record>\n";
     }
   }
 }
 
 void COFFDumper::printUnwindInfo() {
while (Cur != End) {
     uint64_t SymIndex = decodeULEB128(Cur, &Size, End, &Err);
     if (Err)
       reportError(createError(Err), Obj->getFileName());
 
     Expected<COFFSymbolRef> Sym = Obj->getSymbol(SymIndex);
     if (!Sym)
       reportError(Sym.takeError(), Obj->getFileName());
 
-    StringRef SymName;
-    if (std::error_code EC = Obj->getSymbolName(*Sym, SymName))
-      reportError(errorCodeToError(EC), Obj->getFileName());
+    Expected<StringRef> SymName = Obj->getSymbolName(*Sym);
+    if (!SymName)
+      reportError(SymName.takeError(), Obj->getFileName());
 
-    W.printNumber("Sym", SymName, SymIndex);
+    W.printNumber("Sym", *SymName, SymIndex);
     Cur += Size;
   }
 }
 
 void llvm::dumpCodeViewMergedTypes(ScopedPrinter &Writer,
                                    ArrayRef<ArrayRef<uint8_t>> IpiRecords,
                                    ArrayRef<ArrayRef<uint8_t>> TpiRecords) {
   TypeTableCollection TpiTypes(TpiRecords);

llvm/tools/obj2yaml/coff2yaml.cpp

void COFFDumper::dumpHeader() {
   YAMLObj.Header.Machine = Obj.getMachine();
   YAMLObj.Header.Characteristics = Obj.getCharacteristics();
 }
 
 static void
 initializeFileAndStringTable(const llvm::object::COFFObjectFile &Obj,
                              codeview::StringsAndChecksumsRef &SC) {
 
-  ExitOnError Err("Invalid .debug$S section!");
+  ExitOnError Err("invalid .debug$S section");
   // Iterate all .debug$S sections looking for the checksums and string table.
   // Exit as soon as both sections are found.
   for (const auto &S : Obj.sections()) {
     if (SC.hasStrings() && SC.hasChecksums())
       break;
 
     Expected<StringRef> SectionNameOrErr = S.getName();
     if (!SectionNameOrErr) {
initializeFileAndStringTable(const llvm::object::COFFObjectFile &Obj,
   }
 }
 
 void COFFDumper::dumpSections(unsigned NumSections) {
   std::vector<COFFYAML::Section> &YAMLSections = YAMLObj.Sections;
   codeview::StringsAndChecksumsRef SC;
   initializeFileAndStringTable(Obj, SC);
 
+  ExitOnError Err("invalid section table");
   StringMap<bool> SymbolUnique;
   for (const auto &S : Obj.symbols()) {
-    object::COFFSymbolRef Symbol = Obj.getCOFFSymbol(S);
-    StringRef Name;
-    Obj.getSymbolName(Symbol, Name);
+    StringRef Name = Err(Obj.getSymbolName(Obj.getCOFFSymbol(S)));
     StringMap<bool>::iterator It;
     bool Inserted;
     std::tie(It, Inserted) = SymbolUnique.insert(std::make_pair(Name, true));
     if (!Inserted)
       It->second = false;
   }
 
   for (const auto &ObjSection : Obj.sections()) {
dumpCLRTokenDefinition(COFFYAML::Symbol *Sym,
   COFF::AuxiliaryCLRToken YAMLCLRToken;
   YAMLCLRToken.AuxType = ObjCLRToken->AuxType;
   YAMLCLRToken.SymbolTableIndex = ObjCLRToken->SymbolTableIndex;
 
   Sym->CLRToken = YAMLCLRToken;
 }
 
 void COFFDumper::dumpSymbols(unsigned NumSymbols) {
+  ExitOnError Err("invalid symbol table");
+
   std::vector<COFFYAML::Symbol> &Symbols = YAMLObj.Symbols;
   for (const auto &S : Obj.symbols()) {
     object::COFFSymbolRef Symbol = Obj.getCOFFSymbol(S);
     COFFYAML::Symbol Sym;
-    Obj.getSymbolName(Symbol, Sym.Name);
+    Sym.Name = Err(Obj.getSymbolName(Symbol));
     Sym.SimpleType = COFF::SymbolBaseType(Symbol.getBaseType());
     Sym.ComplexType = COFF::SymbolComplexType(Symbol.getComplexType());
     Sym.Header.StorageClass = Symbol.getStorageClass();
     Sym.Header.Value = Symbol.getValue();
     Sym.Header.SectionNumber = Symbol.getSectionNumber();
     Sym.Header.NumberOfAuxSymbols = Symbol.getNumberOfAuxSymbols();
 
     if (Symbol.getNumberOfAuxSymbols() > 0) {