Recommit [Object] Change object::SectionRef::getContents() to return Expected<StringRef>

MaskRay · MaskRay · commit e183340c29db · 2019-05-16T13:24:04.000Z
r360876 didn't fix 2 call sites in clang.

Expected&lt;ArrayRef&lt;uint8_t&gt;&gt; may be better but use Expected&lt;StringRef&gt; for now.

Follow-up of D61781.

llvm-svn: 360892
diff --git a/clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp b/clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
@@ -337,8 +337,14 @@ ObjectFilePCHContainerReader::ExtractPCH(llvm::MemoryBufferRef Buffer) const {
       StringRef Name;
       Section.getName(Name);
       if ((!IsCOFF && Name == "__clangast") || (IsCOFF && Name == "clangast")) {
-        Section.getContents(PCH);
-        return PCH;
+        if (Expected<StringRef> E = Section.getContents())
+          return *E;
+        else {
+          handleAllErrors(E.takeError(), [&](const llvm::ErrorInfoBase &EIB) {
+            EIB.log(llvm::errs());
+          });
+          return "";
+        }
       }
     }
   }
diff --git a/clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp b/clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -462,13 +462,16 @@ class ObjectFileHandler final : public FileHandler {
     // TODO: Instead of copying the input file as is, deactivate the section
     // that is no longer needed.
 
-    StringRef Content;
-    CurrentSection->getContents(Content);
+    Expected<StringRef> Content = CurrentSection->getContents();
+    if (!Content) {
+      consumeError(Content.takeError());
+      return;
+    }
 
-    if (Content.size() < 2)
+    if (Content->size() < 2)
       OS.write(Input.getBufferStart(), Input.getBufferSize());
     else
-      OS.write(Content.data(), Content.size());
+      OS.write(Content->data(), Content->size());
   }
 
   void WriteHeader(raw_fd_ostream &OS,
diff --git a/llvm/include/llvm/Object/ObjectFile.h b/llvm/include/llvm/Object/ObjectFile.h
@@ -98,7 +98,7 @@ class SectionRef {
   uint64_t getAddress() const;
   uint64_t getIndex() const;
   uint64_t getSize() const;
-  std::error_code getContents(StringRef &Result) const;
+  Expected<StringRef> getContents() const;
 
   /// Get the alignment of this section as the actual value (not log 2).
   uint64_t getAlignment() const;
@@ -454,13 +454,12 @@ inline uint64_t SectionRef::getSize() const {
   return OwningObject->getSectionSize(SectionPimpl);
 }
 
-inline std::error_code SectionRef::getContents(StringRef &Result) const {
+inline Expected<StringRef> SectionRef::getContents() const {
   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();
+    return Res.takeError();
+  return StringRef(reinterpret_cast<const char *>(Res->data()), Res->size());
 }
 
 inline uint64_t SectionRef::getAlignment() const {
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp b/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp
@@ -1410,8 +1410,14 @@ class DWARFObjInMemory final : public DWARFObject {
       // Try to obtain an already relocated version of this section.
       // Else use the unrelocated section from the object file. We'll have to
       // apply relocations ourselves later.
-      if (!L || !L->getLoadedSectionContents(*RelocatedSection, Data))
-        Section.getContents(Data);
+      if (!L || !L->getLoadedSectionContents(*RelocatedSection, Data)) {
+        Expected<StringRef> E = Section.getContents();
+        if (E)
+          Data = *E;
+        else
+          // maybeDecompress below will error.
+          consumeError(E.takeError());
+      }
 
       if (auto Err = maybeDecompress(Section, Name, Data)) {
         ErrorPolicy EP = HandleError(createError(
diff --git a/llvm/lib/DebugInfo/Symbolize/SymbolizableObjectFile.cpp b/llvm/lib/DebugInfo/Symbolize/SymbolizableObjectFile.cpp
@@ -53,13 +53,13 @@ SymbolizableObjectFile::create(object::ObjectFile *Obj,
   if (Obj->getArch() == Triple::ppc64) {
     for (section_iterator Section : Obj->sections()) {
       StringRef Name;
-      StringRef Data;
       if (auto EC = Section->getName(Name))
         return EC;
       if (Name == ".opd") {
-        if (auto EC = Section->getContents(Data))
-          return EC;
-        OpdExtractor.reset(new DataExtractor(Data, Obj->isLittleEndian(),
+        Expected<StringRef> E = Section->getContents();
+        if (!E)
+          return errorToErrorCode(E.takeError());
+        OpdExtractor.reset(new DataExtractor(*E, Obj->isLittleEndian(),
                                              Obj->getBytesInAddress()));
         OpdAddress = Section->getAddress();
         break;
diff --git a/llvm/lib/DebugInfo/Symbolize/Symbolize.cpp b/llvm/lib/DebugInfo/Symbolize/Symbolize.cpp
@@ -221,9 +221,12 @@ bool getGNUDebuglinkContents(const ObjectFile *Obj, std::string &DebugName,
     Section.getName(Name);
     Name = Name.substr(Name.find_first_not_of("._"));
     if (Name == "gnu_debuglink") {
-      StringRef Data;
-      Section.getContents(Data);
-      DataExtractor DE(Data, Obj->isLittleEndian(), 0);
+      Expected<StringRef> ContentsOrErr = Section.getContents();
+      if (!ContentsOrErr) {
+        consumeError(ContentsOrErr.takeError());
+        return false;
+      }
+      DataExtractor DE(*ContentsOrErr, Obj->isLittleEndian(), 0);
       uint32_t Offset = 0;
       if (const char *DebugNameStr = DE.getCStr(&Offset)) {
         // 4-byte align the offset.
diff --git a/llvm/lib/ExecutionEngine/JITLink/MachOAtomGraphBuilder.cpp b/llvm/lib/ExecutionEngine/JITLink/MachOAtomGraphBuilder.cpp
@@ -136,14 +136,14 @@ Error MachOAtomGraphBuilder::parseSections() {
 
     if (!SecRef.isVirtual()) {
       // If this section has content then record it.
-      StringRef Content;
-      if (auto EC = SecRef.getContents(Content))
-        return errorCodeToError(EC);
-      if (Content.size() != SecRef.getSize())
+      Expected<StringRef> Content = SecRef.getContents();
+      if (!Content)
+        return Content.takeError();
+      if (Content->size() != SecRef.getSize())
         return make_error<JITLinkError>("Section content size does not match "
                                         "declared size for " +
                                         Name);
-      MachOSec.setContent(Content);
+      MachOSec.setContent(*Content);
     } else {
       // If this is a zero-fill section then just record the size.
       MachOSec.setZeroFill(SecRef.getSize());
diff --git a/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp b/llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp
@@ -792,8 +792,10 @@ RuntimeDyldImpl::emitSection(const ObjectFile &Obj,
   if (!IsVirtual && !IsZeroInit) {
     // In either case, set the location of the unrelocated section in memory,
     // since we still process relocations for it even if we're not applying them.
-    if (auto EC = Section.getContents(data))
-      return errorCodeToError(EC);
+    if (Expected<StringRef> E = Section.getContents())
+      data = *E;
+    else
+      return E.takeError();
     pData = data.data();
   }
 
diff --git a/llvm/lib/Object/ELFObjectFile.cpp b/llvm/lib/Object/ELFObjectFile.cpp
@@ -377,12 +377,13 @@ ELFObjectFileBase::getPltAddresses() const {
   }
   if (!Plt || !RelaPlt || !GotPlt)
     return {};
-  StringRef PltContents;
-  if (Plt->getContents(PltContents))
+  Expected<StringRef> PltContents = Plt->getContents();
+  if (!PltContents) {
+    consumeError(PltContents.takeError());
     return {};
-  ArrayRef<uint8_t> PltBytes((const uint8_t *)PltContents.data(),
-                             Plt->getSize());
-  auto PltEntries = MIA->findPltEntries(Plt->getAddress(), PltBytes,
+  }
+  auto PltEntries = MIA->findPltEntries(Plt->getAddress(),
+                                        arrayRefFromStringRef(*PltContents),
                                         GotPlt->getAddress(), Triple);
   // Build a map from GOT entry virtual address to PLT entry virtual address.
   DenseMap<uint64_t, uint64_t> GotToPlt;
diff --git a/llvm/lib/Object/IRObjectFile.cpp b/llvm/lib/Object/IRObjectFile.cpp
@@ -74,12 +74,12 @@ Expected<MemoryBufferRef>
 IRObjectFile::findBitcodeInObject(const ObjectFile &Obj) {
   for (const SectionRef &Sec : Obj.sections()) {
     if (Sec.isBitcode()) {
-      StringRef SecContents;
-      if (std::error_code EC = Sec.getContents(SecContents))
-        return errorCodeToError(EC);
-      if (SecContents.size() <= 1)
+      Expected<StringRef> Contents = Sec.getContents();
+      if (!Contents)
+        return Contents.takeError();
+      if (Contents->size() <= 1)
         return errorCodeToError(object_error::bitcode_section_not_found);
-      return MemoryBufferRef(SecContents, Obj.getFileName());
+      return MemoryBufferRef(*Contents, Obj.getFileName());
     }
   }
 
diff --git a/llvm/lib/Object/Object.cpp b/llvm/lib/Object/Object.cpp
@@ -247,10 +247,10 @@ uint64_t LLVMGetSectionSize(LLVMSectionIteratorRef SI) {
 }
 
 const char *LLVMGetSectionContents(LLVMSectionIteratorRef SI) {
-  StringRef ret;
-  if (std::error_code ec = (*unwrap(SI))->getContents(ret))
-    report_fatal_error(ec.message());
-  return ret.data();
+  if (Expected<StringRef> E = (*unwrap(SI))->getContents())
+    return E->data();
+  else
+    report_fatal_error(E.takeError());
 }
 
 uint64_t LLVMGetSectionAddress(LLVMSectionIteratorRef SI) {
diff --git a/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp b/llvm/lib/ProfileData/Coverage/CoverageMappingReader.cpp
@@ -348,8 +348,10 @@ Expected<bool> RawCoverageMappingDummyChecker::isDummy() {
 }
 
 Error InstrProfSymtab::create(SectionRef &Section) {
-  if (auto EC = Section.getContents(Data))
-    return errorCodeToError(EC);
+  Expected<StringRef> DataOrErr = Section.getContents();
+  if (!DataOrErr)
+    return DataOrErr.takeError();
+  Data = *DataOrErr;
   Address = Section.getAddress();
 
   // If this is a linked PE/COFF file, then we have to skip over the null byte
@@ -687,8 +689,11 @@ static Error loadBinaryFormat(MemoryBufferRef ObjectBuffer,
     return E;
 
   // Get the contents of the given sections.
-  if (auto EC = CoverageSection->getContents(CoverageMapping))
-    return errorCodeToError(EC);
+  if (Expected<StringRef> E = CoverageSection->getContents())
+    CoverageMapping = *E;
+  else
+    return E.takeError();
+
   if (Error E = ProfileNames.create(*NamesSection))
     return E;
 
diff --git a/llvm/lib/XRay/InstrumentationMap.cpp b/llvm/lib/XRay/InstrumentationMap.cpp
@@ -78,9 +78,10 @@ loadObj(StringRef Filename, object::OwningBinary<object::ObjectFile> &ObjFile,
         "Failed to find XRay instrumentation map.",
         std::make_error_code(std::errc::executable_format_error));
 
-  if (I->getContents(Contents))
-    return errorCodeToError(
-        std::make_error_code(std::errc::executable_format_error));
+  if (Expected<StringRef> E = I->getContents())
+    Contents = *E;
+  else
+    return E.takeError();
 
   RelocMap Relocs;
   if (ObjFile.getBinary()->isELF()) {
diff --git a/llvm/tools/dsymutil/DwarfLinker.cpp b/llvm/tools/dsymutil/DwarfLinker.cpp
@@ -436,9 +436,13 @@ static bool isMachOPairedReloc(uint64_t RelocType, uint64_t Arch) {
 void DwarfLinker::RelocationManager::findValidRelocsMachO(
     const object::SectionRef &Section, const object::MachOObjectFile &Obj,
     const DebugMapObject &DMO) {
-  StringRef Contents;
-  Section.getContents(Contents);
-  DataExtractor Data(Contents, Obj.isLittleEndian(), 0);
+  Expected<StringRef> ContentsOrErr = Section.getContents();
+  if (!ContentsOrErr) {
+    consumeError(ContentsOrErr.takeError());
+    Linker.reportWarning("error reading section", DMO);
+    return;
+  }
+  DataExtractor Data(*ContentsOrErr, Obj.isLittleEndian(), 0);
   bool SkipNext = false;
 
   for (const object::RelocationRef &Reloc : Section.relocations()) {
diff --git a/llvm/tools/dsymutil/DwarfStreamer.cpp b/llvm/tools/dsymutil/DwarfStreamer.cpp
@@ -667,10 +667,12 @@ void DwarfStreamer::translateLineTable(DataExtractor Data, uint32_t Offset,
 
 static void emitSectionContents(const object::ObjectFile &Obj,
                                 StringRef SecName, MCStreamer *MS) {
-  StringRef Contents;
-  if (auto Sec = getSectionByName(Obj, SecName))
-    if (!Sec->getContents(Contents))
-      MS->EmitBytes(Contents);
+  if (auto Sec = getSectionByName(Obj, SecName)) {
+    if (Expected<StringRef> E = Sec->getContents())
+      MS->EmitBytes(*E);
+    else
+      consumeError(E.takeError());
+  }
 }
 
 void DwarfStreamer::copyInvariantDebugSection(const object::ObjectFile &Obj) {
diff --git a/llvm/tools/llvm-cfi-verify/lib/FileAnalysis.cpp b/llvm/tools/llvm-cfi-verify/lib/FileAnalysis.cpp
@@ -453,13 +453,11 @@ Error FileAnalysis::parseCodeSections() {
     if (!Section.getName(SectionName) && SectionName == ".plt")
       continue;
 
-    StringRef SectionContents;
-    if (Section.getContents(SectionContents))
-      return make_error<StringError>("Failed to retrieve section contents",
-                                     inconvertibleErrorCode());
+    Expected<StringRef> Contents = Section.getContents();
+    if (!Contents)
+      return Contents.takeError();
+    ArrayRef<uint8_t> SectionBytes = arrayRefFromStringRef(*Contents);
 
-    ArrayRef<uint8_t> SectionBytes((const uint8_t *)SectionContents.data(),
-                                   Section.getSize());
     parseSectionContents(SectionBytes,
                          {Section.getAddress(), Section.getIndex()});
   }
diff --git a/llvm/tools/llvm-cov/TestingSupport.cpp b/llvm/tools/llvm-cov/TestingSupport.cpp
@@ -69,9 +69,18 @@ int convertForTestingMain(int argc, const char *argv[]) {
   uint64_t ProfileNamesAddress = ProfileNames.getAddress();
   StringRef CoverageMappingData;
   StringRef ProfileNamesData;
-  if (CoverageMapping.getContents(CoverageMappingData) ||
-      ProfileNames.getContents(ProfileNamesData))
+  if (Expected<StringRef> E = CoverageMapping.getContents())
+    CoverageMappingData = *E;
+  else {
+    consumeError(E.takeError());
     return 1;
+  }
+  if (Expected<StringRef> E = ProfileNames.getContents())
+    ProfileNamesData = *E;
+  else {
+    consumeError(E.takeError());
+    return 1;
+  }
 
   int FD;
   if (auto Err = sys::fs::openFileForWrite(OutputFilename, FD)) {
diff --git a/llvm/tools/llvm-cxxdump/llvm-cxxdump.cpp b/llvm/tools/llvm-cxxdump/llvm-cxxdump.cpp
@@ -48,15 +48,20 @@ static void error(std::error_code EC) {
   exit(1);
 }
 
-static void error(Error Err) {
-  if (!Err)
-    return;
+LLVM_ATTRIBUTE_NORETURN static void error(Error Err) {
   logAllUnhandledErrors(std::move(Err), WithColor::error(outs()),
                         "reading file: ");
   outs().flush();
   exit(1);
 }
 
+template <typename T>
+T unwrapOrError(Expected<T> EO) {
+  if (!EO)
+    error(EO.takeError());
+  return std::move(*EO);
+}
+
 } // namespace llvm
 
 static void reportError(StringRef Input, StringRef Message) {
@@ -195,8 +200,7 @@ static void dumpCXXData(const ObjectFile *Obj) {
     // Skip virtual or BSS sections.
     if (Sec.isBSS() || Sec.isVirtual())
       continue;
-    StringRef SecContents;
-    error(Sec.getContents(SecContents));
+    StringRef SecContents = unwrapOrError(Sec.getContents());
     Expected<uint64_t> SymAddressOrErr = Sym.getAddress();
     error(errorToErrorCode(SymAddressOrErr.takeError()));
     uint64_t SymAddress = *SymAddressOrErr;
@@ -510,7 +514,8 @@ static void dumpArchive(const Archive *Arc) {
     else
       reportError(Arc->getFileName(), cxxdump_error::unrecognized_file_format);
   }
-  error(std::move(Err));
+  if (Err)
+    error(std::move(Err));
 }
 
 static void dumpInput(StringRef File) {
diff --git a/llvm/tools/llvm-dwp/llvm-dwp.cpp b/llvm/tools/llvm-dwp/llvm-dwp.cpp
@@ -410,9 +410,10 @@ static Error handleSection(
   if (std::error_code Err = Section.getName(Name))
     return errorCodeToError(Err);
 
-  StringRef Contents;
-  if (auto Err = Section.getContents(Contents))
-    return errorCodeToError(Err);
+  Expected<StringRef> ContentsOrErr = Section.getContents();
+  if (!ContentsOrErr)
+    return ContentsOrErr.takeError();
+  StringRef Contents = *ContentsOrErr;
 
   if (auto Err = handleCompressedSection(UncompressedSections, Name, Contents))
     return Err;
diff --git a/llvm/tools/llvm-objdump/MachODump.cpp b/llvm/tools/llvm-objdump/MachODump.cpp
diff --git a/llvm/tools/llvm-objdump/llvm-objdump.cpp b/llvm/tools/llvm-objdump/llvm-objdump.cpp
diff --git a/llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp b/llvm/tools/llvm-pdbutil/DumpOutputStyle.cpp
diff --git a/llvm/tools/llvm-pdbutil/InputFile.cpp b/llvm/tools/llvm-pdbutil/InputFile.cpp
diff --git a/llvm/tools/llvm-readobj/COFFDumper.cpp b/llvm/tools/llvm-readobj/COFFDumper.cpp
diff --git a/llvm/tools/llvm-readobj/MachODumper.cpp b/llvm/tools/llvm-readobj/MachODumper.cpp
diff --git a/llvm/tools/llvm-readobj/ObjDumper.cpp b/llvm/tools/llvm-readobj/ObjDumper.cpp
diff --git a/llvm/tools/sancov/sancov.cpp b/llvm/tools/sancov/sancov.cpp

Original file line number	Diff line number	Diff line change
`@@ -337,8 +337,14 @@ ObjectFilePCHContainerReader::ExtractPCH(llvm::MemoryBufferRef Buffer) const {`
`337`	`337`	`StringRef Name;`
`338`	`338`	`Section.getName(Name);`
`339`	`339`	`if ((!IsCOFF && Name == "__clangast") \|\| (IsCOFF && Name == "clangast")) {`
`340`		`- Section.getContents(PCH);`
`341`		`- return PCH;`
	`340`	`+ if (Expected<StringRef> E = Section.getContents())`
	`341`	`+ return *E;`
	`342`	`+ else {`
	`343`	`+ handleAllErrors(E.takeError(), [&](const llvm::ErrorInfoBase &EIB) {`
	`344`	`+ EIB.log(llvm::errs());`
	`345`	`+ });`
	`346`	`+ return "";`
	`347`	`+ }`
`342`	`348`	`}`
`343`	`349`	`}`
`344`	`350`	`}`