[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 ↗

(On Diff #199004)

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

[MaskRay]

llvm/include/llvm/Object/ObjectFile.h

458 ↗

(On Diff #199004)

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 ↗

(On Diff #199004)

@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 ↗

(On Diff #199004)

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.

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());
 }
 

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())

COFF/InputFiles.cpp

SectionChunk *ObjFile::readSection(uint32_t SectionNumber,
   if (Expected<StringRef> E = COFFObj->getSectionName(Sec))
     Name = *E;
   else
     fatal("getSectionName failed: #" + Twine(SectionNumber) + ": " +
           toString(E.takeError()));
 
   if (Name == ".drectve") {
     ArrayRef<uint8_t> Data;
-    COFFObj->getSectionContents(Sec, Data);
+    cantFail(COFFObj->getSectionContents(Sec, Data));
     Directives = StringRef((const char *)Data.data(), Data.size());
     return nullptr;
   }
 
   if (Name == ".llvm_addrsig") {
     AddrsigSec = Sec;
     return nullptr;
   }