[Object] Change ObjectFile::getSectionContents to use Expected

[MaskRay]

Summary

Change
std::error_code getSectionContents(DataRefImpl, StringRef &) const;
to
Expected<ArrayRef<uint8_t>> getSectionContents(DataRefImpl) const;

Many object formats use ArrayRef<uint8_t> as the underlying type, which
is generally better than StringRef to represent binary data, so change
the type to decrease the number of type conversions.

[sbc100]

llvm/include/llvm/Object/ObjectFile.h

458

Looks like an opportunity for a followup change to propagate the use of ArrayRef here too?

[MaskRay]

llvm/include/llvm/Object/ObjectFile.h

458

Thanks for review! Yes, a few more functions to refactor. I didn't change this function in this patch because it has 48 references...

[MaskRay]

lld/COFF/Driver.cpp

866

@ruiu What I am less confident is whether I should use cantFail here. If the error checking has been done before, cantFail is the best.

[ruiu]

LGTM

lld/COFF/Driver.cpp

866

When we reach here, we are sure that Addrsig section exists. If we for some reason fail to get that section at this point, that indicates that there's a bug in lld or an input file is corrupted. In either case, aborting using cantFail is fine.

lld/COFF/Chunks.cpp

 StringRef SectionChunk::getDebugName() {
   if (Sym)
     return Sym->getName();
   return "";
 }
 
 ArrayRef<uint8_t> SectionChunk::getContents() const {
   ArrayRef<uint8_t> A;
-  File->getCOFFObj()->getSectionContents(Header, A);
+  cantFail(File->getCOFFObj()->getSectionContents(Header, A));
   return A;
 }
 
 ArrayRef<uint8_t> SectionChunk::consumeDebugMagic() {
   assert(isCodeView());
   return consumeDebugMagic(getContents(), getSectionName());
 }
 

lld/COFF/Driver.cpp

for (Export &R : Config->Exports)
     markAddrsig(R.Sym);
 
   // Visit the address-significance table in each object file and mark each
   // referenced symbol as address-significant.
   for (ObjFile *Obj : ObjFile::Instances) {
     ArrayRef<Symbol *> Syms = Obj->getSymbols();
     if (Obj->AddrsigSec) {
       ArrayRef<uint8_t> Contents;
-      Obj->getCOFFObj()->getSectionContents(Obj->AddrsigSec, Contents);
+      cantFail(Obj->getCOFFObj()->getSectionContents(Obj->AddrsigSec, Contents));
       const uint8_t *Cur = Contents.begin();
       while (Cur != Contents.end()) {
         unsigned Size;
         const char *Err;
         uint64_t SymIndex = decodeULEB128(Cur, &Size, Contents.end(), &Err);
         if (Err)
           fatal(toString(Obj) + ": could not decode addrsig section: " + Err);
         if (SymIndex >= Syms.size())

llvm/include/llvm/Object/COFF.h

protected:
   uint32_t getSymbolFlags(DataRefImpl Symb) const override;
   Expected<SymbolRef::Type> getSymbolType(DataRefImpl Symb) const override;
   Expected<section_iterator> getSymbolSection(DataRefImpl Symb) const override;
   void moveSectionNext(DataRefImpl &Sec) const override;
   Expected<StringRef> getSectionName(DataRefImpl Sec) const override;
   uint64_t getSectionAddress(DataRefImpl Sec) const override;
   uint64_t getSectionIndex(DataRefImpl Sec) const override;
   uint64_t getSectionSize(DataRefImpl Sec) const override;
-  std::error_code getSectionContents(DataRefImpl Sec,
-                                     StringRef &Res) const override;
+  Expected<ArrayRef<uint8_t>>
+  getSectionContents(DataRefImpl Sec) const override;
   uint64_t getSectionAlignment(DataRefImpl Sec) const override;
   bool isSectionCompressed(DataRefImpl Sec) const override;
   bool isSectionText(DataRefImpl Sec) const override;
   bool isSectionData(DataRefImpl Sec) const override;
   bool isSectionBSS(DataRefImpl Sec) const override;
   bool isSectionVirtual(DataRefImpl Sec) const override;
   relocation_iterator section_rel_begin(DataRefImpl Sec) const override;
   relocation_iterator section_rel_end(DataRefImpl Sec) const override;
if (COFFBigObjHeader)
       return sizeof(coff_symbol32);
     llvm_unreachable("null symbol table pointer!");
   }
 
   ArrayRef<coff_relocation> getRelocations(const coff_section *Sec) const;
 
   Expected<StringRef> getSectionName(const coff_section *Sec) const;
   uint64_t getSectionSize(const coff_section *Sec) const;
-  std::error_code getSectionContents(const coff_section *Sec,
-                                     ArrayRef<uint8_t> &Res) const;
+  Error getSectionContents(const coff_section *Sec,
 
   uint64_t getImageBase() const;
   std::error_code getVaPtr(uint64_t VA, uintptr_t &Res) const;
   std::error_code getRvaPtr(uint32_t Rva, uintptr_t &Res) const;
 
   /// Given an RVA base and size, returns a valid array of bytes or an error
   /// code if the RVA and size is not contained completely within a valid
   /// section.

llvm/include/llvm/Object/ELFObjectFile.h

Expected<section_iterator> getSymbolSection(const Elf_Sym *Symb,
                                               const Elf_Shdr *SymTab) const;
   Expected<section_iterator> getSymbolSection(DataRefImpl Symb) const override;
 
   void moveSectionNext(DataRefImpl &Sec) const override;
   Expected<StringRef> getSectionName(DataRefImpl Sec) const override;
   uint64_t getSectionAddress(DataRefImpl Sec) const override;
   uint64_t getSectionIndex(DataRefImpl Sec) const override;
   uint64_t getSectionSize(DataRefImpl Sec) const override;
-  std::error_code getSectionContents(DataRefImpl Sec,
-                                     StringRef &Res) const override;
+  Expected<ArrayRef<uint8_t>>
+  getSectionContents(DataRefImpl Sec) const override;
   uint64_t getSectionAlignment(DataRefImpl Sec) const override;
   bool isSectionCompressed(DataRefImpl Sec) const override;
   bool isSectionText(DataRefImpl Sec) const override;
   bool isSectionData(DataRefImpl Sec) const override;
   bool isSectionBSS(DataRefImpl Sec) const override;
   bool isSectionVirtual(DataRefImpl Sec) const override;
   bool isBerkeleyText(DataRefImpl Sec) const override;
   bool isBerkeleyData(DataRefImpl Sec) const override;

 }
 
 template <class ELFT>
 uint64_t ELFObjectFile<ELFT>::getSectionSize(DataRefImpl Sec) const {
   return getSection(Sec)->sh_size;
 }
 
 template <class ELFT>
-std::error_code
-ELFObjectFile<ELFT>::getSectionContents(DataRefImpl Sec,
-                                        StringRef &Result) const {
+Expected<ArrayRef<uint8_t>>
+ELFObjectFile<ELFT>::getSectionContents(DataRefImpl Sec) const {
   const Elf_Shdr *EShdr = getSection(Sec);
   if (std::error_code EC =
           checkOffset(getMemoryBufferRef(),
                       (uintptr_t)base() + EShdr->sh_offset, EShdr->sh_size))
-    return EC;
-  Result = StringRef((const char *)base() + EShdr->sh_offset, EShdr->sh_size);
-  return std::error_code();
+    return errorCodeToError(EC);
+  return makeArrayRef((const uint8_t *)base() + EShdr->sh_offset,
+                      EShdr->sh_size);
 }
 
 template <class ELFT>
 uint64_t ELFObjectFile<ELFT>::getSectionAlignment(DataRefImpl Sec) const {
   return getSection(Sec)->sh_addralign;
 }
 
 template <class ELFT>

llvm/include/llvm/Object/MachO.h

public:
   unsigned getSymbolSectionID(SymbolRef Symb) const;
   unsigned getSectionID(SectionRef Sec) const;
 
   void moveSectionNext(DataRefImpl &Sec) const override;
   Expected<StringRef> getSectionName(DataRefImpl Sec) const override;
   uint64_t getSectionAddress(DataRefImpl Sec) const override;
   uint64_t getSectionIndex(DataRefImpl Sec) const override;
   uint64_t getSectionSize(DataRefImpl Sec) const override;
-  std::error_code getSectionContents(DataRefImpl Sec,
-                                     StringRef &Res) const override;
+  Expected<ArrayRef<uint8_t>>
+  getSectionContents(DataRefImpl Sec) const override;
   uint64_t getSectionAlignment(DataRefImpl Sec) const override;
   Expected<SectionRef> getSection(unsigned SectionIndex) const;
   Expected<SectionRef> getSection(StringRef SectionName) const;
   bool isSectionCompressed(DataRefImpl Sec) const override;
   bool isSectionText(DataRefImpl Sec) const override;
   bool isSectionData(DataRefImpl Sec) const override;
   bool isSectionBSS(DataRefImpl Sec) const override;
   bool isSectionVirtual(DataRefImpl Sec) const override;

llvm/include/llvm/Object/ObjectFile.h

protected:
   // Same as above for SectionRef.
   friend class SectionRef;
 
   virtual void moveSectionNext(DataRefImpl &Sec) const = 0;
   virtual Expected<StringRef> getSectionName(DataRefImpl Sec) const = 0;
   virtual uint64_t getSectionAddress(DataRefImpl Sec) const = 0;
   virtual uint64_t getSectionIndex(DataRefImpl Sec) const = 0;
   virtual uint64_t getSectionSize(DataRefImpl Sec) const = 0;
-  virtual std::error_code getSectionContents(DataRefImpl Sec,
-                                             StringRef &Res) const = 0;
+  virtual Expected<ArrayRef<uint8_t>>
+  getSectionContents(DataRefImpl Sec) const = 0;
   virtual uint64_t getSectionAlignment(DataRefImpl Sec) const = 0;
   virtual bool isSectionCompressed(DataRefImpl Sec) const = 0;
   virtual bool isSectionText(DataRefImpl Sec) const = 0;
   virtual bool isSectionData(DataRefImpl Sec) const = 0;
   virtual bool isSectionBSS(DataRefImpl Sec) const = 0;
   // A section is 'virtual' if its contents aren't present in the object image.
   virtual bool isSectionVirtual(DataRefImpl Sec) const = 0;
   virtual bool isSectionBitcode(DataRefImpl Sec) const;
inline uint64_t SectionRef::getIndex() const {
   return OwningObject->getSectionIndex(SectionPimpl);
 }
 
 inline uint64_t SectionRef::getSize() const {
   return OwningObject->getSectionSize(SectionPimpl);
 }
 
 inline std::error_code SectionRef::getContents(StringRef &Result) const {
-  return OwningObject->getSectionContents(SectionPimpl, Result);
+  Expected<ArrayRef<uint8_t>> Res =
+      OwningObject->getSectionContents(SectionPimpl);
+  if (!Res)
+    return errorToErrorCode(Res.takeError());
+  Result = StringRef(reinterpret_cast<const char *>(Res->data()), Res->size());
+  return std::error_code();
 }
 
 inline uint64_t SectionRef::getAlignment() const {
   return OwningObject->getSectionAlignment(SectionPimpl);
 }
 
 inline bool SectionRef::isCompressed() const {
   return OwningObject->isSectionCompressed(SectionPimpl);

llvm/include/llvm/Object/Wasm.h

public:
   Expected<section_iterator> getSymbolSection(DataRefImpl Symb) const override;
 
   // Overrides from SectionRef.
   void moveSectionNext(DataRefImpl &Sec) const override;
   Expected<StringRef> getSectionName(DataRefImpl Sec) const override;
   uint64_t getSectionAddress(DataRefImpl Sec) const override;
   uint64_t getSectionIndex(DataRefImpl Sec) const override;
   uint64_t getSectionSize(DataRefImpl Sec) const override;
-  std::error_code getSectionContents(DataRefImpl Sec,
-                                     StringRef &Res) const override;
+  Expected<ArrayRef<uint8_t>>
+  getSectionContents(DataRefImpl Sec) const override;
   uint64_t getSectionAlignment(DataRefImpl Sec) const override;
   bool isSectionCompressed(DataRefImpl Sec) const override;
   bool isSectionText(DataRefImpl Sec) const override;
   bool isSectionData(DataRefImpl Sec) const override;
   bool isSectionBSS(DataRefImpl Sec) const override;
   bool isSectionVirtual(DataRefImpl Sec) const override;
   bool isSectionBitcode(DataRefImpl Sec) const override;
   relocation_iterator section_rel_begin(DataRefImpl Sec) const override;

llvm/include/llvm/Object/XCOFFObjectFile.h

public:
   Expected<SymbolRef::Type> getSymbolType(DataRefImpl Symb) const override;
   Expected<section_iterator> getSymbolSection(DataRefImpl Symb) const override;
 
   void moveSectionNext(DataRefImpl &Sec) const override;
   Expected<StringRef> getSectionName(DataRefImpl Sec) const override;
   uint64_t getSectionAddress(DataRefImpl Sec) const override;
   uint64_t getSectionIndex(DataRefImpl Sec) const override;
   uint64_t getSectionSize(DataRefImpl Sec) const override;
-  std::error_code getSectionContents(DataRefImpl Sec,
-                                     StringRef &Res) const override;
+  Expected<ArrayRef<uint8_t>>
+  getSectionContents(DataRefImpl Sec) const override;
   uint64_t getSectionAlignment(DataRefImpl Sec) const override;
   bool isSectionCompressed(DataRefImpl Sec) const override;
   bool isSectionText(DataRefImpl Sec) const override;
   bool isSectionData(DataRefImpl Sec) const override;
   bool isSectionBSS(DataRefImpl Sec) const override;
 
   bool isSectionVirtual(DataRefImpl Sec) const override;
   relocation_iterator section_rel_begin(DataRefImpl Sec) const override;

llvm/lib/Object/COFFObjectFile.cpp

 uint64_t COFFObjectFile::getSectionIndex(DataRefImpl Sec) const {
   return toSec(Sec) - SectionTable;
 }
 
 uint64_t COFFObjectFile::getSectionSize(DataRefImpl Ref) const {
   return getSectionSize(toSec(Ref));
 }
 
-std::error_code COFFObjectFile::getSectionContents(DataRefImpl Ref,
-                                                   StringRef &Result) const {
+Expected<ArrayRef<uint8_t>>
+COFFObjectFile::getSectionContents(DataRefImpl Ref) const {
   const coff_section *Sec = toSec(Ref);
   ArrayRef<uint8_t> Res;
-  std::error_code EC = getSectionContents(Sec, Res);
-  Result = StringRef(reinterpret_cast<const char*>(Res.data()), Res.size());
-  return EC;
+  if (Error E = getSectionContents(Sec, Res))
+    return std::move(E);
+  return Res;
 }
 
 uint64_t COFFObjectFile::getSectionAlignment(DataRefImpl Ref) const {
   const coff_section *Sec = toSec(Ref);
   return Sec->getAlignment();
 }
 
 bool COFFObjectFile::isSectionCompressed(DataRefImpl Sec) const {
uint64_t COFFObjectFile::getSectionSize(const coff_section *Sec) const {
   // actual section size is in VirtualSize.  It is possible for VirtualSize to
   // be greater than SizeOfRawData; the contents past that point should be
   // considered to be zero.
   if (getDOSHeader())
     return std::min(Sec->VirtualSize, Sec->SizeOfRawData);
   return Sec->SizeOfRawData;
 }
 
-std::error_code
-COFFObjectFile::getSectionContents(const coff_section *Sec,
+Error COFFObjectFile::getSectionContents(const coff_section *Sec,
                                          ArrayRef<uint8_t> &Res) const {
   // In COFF, a virtual section won't have any in-file
   // content, so the file pointer to the content will be zero.
   if (Sec->PointerToRawData == 0)
-    return std::error_code();
+    return Error::success();
   // The only thing that we need to verify is that the contents is contained
   // within the file bounds. We don't need to make sure it doesn't cover other
   // data, as there's nothing that says that is not allowed.
   uintptr_t ConStart = uintptr_t(base()) + Sec->PointerToRawData;
   uint32_t SectionSize = getSectionSize(Sec);
   if (checkOffset(Data, ConStart, SectionSize))
-    return object_error::parse_failed;
+    return make_error<BinaryError>();
   Res = makeArrayRef(reinterpret_cast<const uint8_t *>(ConStart), SectionSize);
-  return std::error_code();
+  return Error::success();
 }
 
 const coff_relocation *COFFObjectFile::toRel(DataRefImpl Rel) const {
   return reinterpret_cast<const coff_relocation*>(Rel.p);
 }
 
 void COFFObjectFile::moveRelocationNext(DataRefImpl &Rel) const {
   Rel.p = reinterpret_cast<uintptr_t>(

llvm/lib/Object/MachOObjectFile.cpp

uint64_t MachOObjectFile::getSectionSize(DataRefImpl Sec) const {
   uint64_t FileSize = getData().size();
   if (SectOffset > FileSize)
     return 0;
   if (FileSize - SectOffset < SectSize)
     return FileSize - SectOffset;
   return SectSize;
 }
 
-std::error_code MachOObjectFile::getSectionContents(DataRefImpl Sec,
-                                                    StringRef &Res) const {
+Expected<ArrayRef<uint8_t>>
+MachOObjectFile::getSectionContents(DataRefImpl Sec) const {
   uint32_t Offset;
   uint64_t Size;
 
   if (is64Bit()) {
     MachO::section_64 Sect = getSection64(Sec);
     Offset = Sect.offset;
     Size = Sect.size;
   } else {
     MachO::section Sect = getSection(Sec);
     Offset = Sect.offset;
     Size = Sect.size;
   }
 
-  Res = this->getData().substr(Offset, Size);
-  return std::error_code();
+  return arrayRefFromStringRef(getData().substr(Offset, Size));
 }
 
 uint64_t MachOObjectFile::getSectionAlignment(DataRefImpl Sec) const {
   uint32_t Align;
   if (is64Bit()) {
     MachO::section_64 Sect = getSection64(Sec);
     Align = Sect.align;
   } else {

llvm/lib/Object/WasmObjectFile.cpp

uint64_t WasmObjectFile::getSectionIndex(DataRefImpl Sec) const {
   return Sec.d.a;
 }
 
 uint64_t WasmObjectFile::getSectionSize(DataRefImpl Sec) const {
   const WasmSection &S = Sections[Sec.d.a];
   return S.Content.size();
 }
 
-std::error_code WasmObjectFile::getSectionContents(DataRefImpl Sec,
-                                                   StringRef &Res) const {
+Expected<ArrayRef<uint8_t>> WasmObjectFile::getSectionContents(DataRefImpl Sec) const {
   const WasmSection &S = Sections[Sec.d.a];
   // This will never fail since wasm sections can never be empty (user-sections
   // must have a name and non-user sections each have a defined structure).
-  Res = StringRef(reinterpret_cast<const char *>(S.Content.data()),
-                  S.Content.size());
-  return std::error_code();
+  return S.Content;
 }
 
 uint64_t WasmObjectFile::getSectionAlignment(DataRefImpl Sec) const {
   return 1;
 }
 
 bool WasmObjectFile::isSectionCompressed(DataRefImpl Sec) const {
   return false;

llvm/lib/Object/XCOFFObjectFile.cpp

uint64_t XCOFFObjectFile::getSectionIndex(DataRefImpl Sec) const {
   // zero is used to indicate that a symbol is being imported or is undefined.
   return toSection(Sec) - SectionHdrTablePtr + 1;
 }
 
 uint64_t XCOFFObjectFile::getSectionSize(DataRefImpl Sec) const {
   return toSection(Sec)->SectionSize;
 }
 
-std::error_code XCOFFObjectFile::getSectionContents(DataRefImpl Sec,
-                                                    StringRef &Res) const {
+Expected<ArrayRef<uint8_t>>
+XCOFFObjectFile::getSectionContents(DataRefImpl Sec) const {
   llvm_unreachable("Not yet implemented!");
-  return std::error_code();
 }
 
 uint64_t XCOFFObjectFile::getSectionAlignment(DataRefImpl Sec) const {
   uint64_t Result = 0;
   llvm_unreachable("Not yet implemented!");
   return Result;
 }
 

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

Error COFFReader::readSections(Object &Obj) const {
   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);
     Sections.push_back(Section());
     Section &S = Sections.back();
     S.Header = *Sec;
     ArrayRef<uint8_t> Contents;
-    if (auto EC = COFFObj.getSectionContents(Sec, Contents))
-      return errorCodeToError(EC);
+    if (Error E = COFFObj.getSectionContents(Sec, Contents))
+      return E;
     S.setContentsRef(Contents);
     ArrayRef<coff_relocation> Relocs = COFFObj.getRelocations(Sec);
     for (const coff_relocation &R : Relocs)
       S.Relocs.push_back(R);
     if (Expected<StringRef> NameOrErr = COFFObj.getSectionName(Sec))
       S.Name = *NameOrErr;
     else
       return NameOrErr.takeError();

llvm/tools/llvm-objcopy/MachO/MachOReader.cpp

for (; reinterpret_cast<const void *>(Curr) < End; Curr++) {
     Section &S = Sections.back();
 
     StringRef SectName(S.Sectname);
     Expected<object::SectionRef> SecRef =
         MachOObj.getSection(NextSectionIndex++);
     if (!SecRef)
       reportError(MachOObj.getFileName(), SecRef.takeError());
 
-    StringRef Content;
-    if (auto EC =
-            MachOObj.getSectionContents(SecRef->getRawDataRefImpl(), Content))
-      reportError(MachOObj.getFileName(), std::move(EC));
-    S.Content = Content;
+    if (Expected<ArrayRef<uint8_t>> E =
+            MachOObj.getSectionContents(SecRef->getRawDataRefImpl()))
+      S.Content =
+          StringRef(reinterpret_cast<const char *>(E->data()), E->size());
+    else
+      reportError(MachOObj.getFileName(), E.takeError());
 
     S.Relocations.reserve(S.NReloc);
     for (auto RI = MachOObj.section_rel_begin(SecRef->getRawDataRefImpl()),
               RE = MachOObj.section_rel_end(SecRef->getRawDataRefImpl());
          RI != RE; ++RI)
       S.Relocations.push_back(MachOObj.getRelocation(RI->getRawDataRefImpl()));
     assert(S.NReloc == S.Relocations.size() &&
            "Incorrect number of relocations");

llvm/tools/llvm-objdump/COFFDump.cpp

getSectionContents(const COFFObjectFile *Obj,
                    const std::vector<RelocationRef> &Rels, uint64_t Offset,
                    ArrayRef<uint8_t> &Contents, uint64_t &Addr) {
   SymbolRef Sym;
   if (Error E = resolveSymbol(Rels, Offset, Sym))
     return E;
   const coff_section *Section;
   if (Error E = resolveSectionAndAddress(Obj, Sym, Section, Addr))
     return E;
-  if (std::error_code EC = Obj->getSectionContents(Section, Contents))
-    return errorCodeToError(EC);
-  return Error::success();
+  return Obj->getSectionContents(Section, Contents);
 }
 
 // Given a vector of relocations for a section and an offset into this section
 // the function returns the name of the symbol used for the relocation at the
 // offset.
 static Error resolveSymbolName(const std::vector<RelocationRef> &Rels,
                                uint64_t Offset, StringRef &Name) {
   SymbolRef Sym;