[dsymutil] Make the CachedBinaryHolder the default

Replaces all uses of the old binary holder with its cached variant.

Differential revision: https://reviews.llvm.org/D48770

llvm-svn: 335991

9 files changed, 120 insertions, 377 deletions

diff --git a/llvm/tools/dsymutil/BinaryHolder.cpp b/llvm/tools/dsymutil/BinaryHolder.cpp
index 8264f465a5d..72a4e865092 100644
--- a/llvm/tools/dsymutil/BinaryHolder.cpp
+++ b/llvm/tools/dsymutil/BinaryHolder.cpp
@@ -42,9 +42,8 @@ getMachOFatMemoryBuffers(StringRef Filename, MemoryBuffer &Mem,
   return Buffers;
 }
 
-Error CachedBinaryHolder::ArchiveEntry::load(StringRef Filename,
-                                             TimestampTy Timestamp,
-                                             bool Verbose) {
+Error BinaryHolder::ArchiveEntry::load(StringRef Filename,
+                                       TimestampTy Timestamp, bool Verbose) {
   StringRef ArchiveFilename = getArchiveAndObjectName(Filename).first;
 
   // Try to load archive and force it to be memory mapped.
@@ -55,7 +54,7 @@ Error CachedBinaryHolder::ArchiveEntry::load(StringRef Filename,
   MemoryBuffer = std::move(*ErrOrBuff);
 
   if (Verbose)
-    WithColor::note() << "opened archive '" << ArchiveFilename << "'\n";
+    WithColor::note() << "loaded archive '" << ArchiveFilename << "'\n";
 
   // Load one or more archive buffers, depending on whether we're dealing with
   // a fat binary.
@@ -85,7 +84,7 @@ Error CachedBinaryHolder::ArchiveEntry::load(StringRef Filename,
   return Error::success();
 }
 
-Error CachedBinaryHolder::ObjectEntry::load(StringRef Filename, bool Verbose) {
+Error BinaryHolder::ObjectEntry::load(StringRef Filename, bool Verbose) {
   // Try to load regular binary and force it to be memory mapped.
   auto ErrOrBuff = MemoryBuffer::getFileOrSTDIN(Filename, -1, false);
   if (auto Err = ErrOrBuff.getError())
@@ -94,7 +93,7 @@ Error CachedBinaryHolder::ObjectEntry::load(StringRef Filename, bool Verbose) {
   MemoryBuffer = std::move(*ErrOrBuff);
 
   if (Verbose)
-    WithColor::note() << "opened object.\n";
+    WithColor::note() << "loaded object.\n";
 
   // Load one or more object buffers, depending on whether we're dealing with a
   // fat binary.
@@ -124,7 +123,7 @@ Error CachedBinaryHolder::ObjectEntry::load(StringRef Filename, bool Verbose) {
 }
 
 std::vector<const object::ObjectFile *>
-CachedBinaryHolder::ObjectEntry::getObjects() const {
+BinaryHolder::ObjectEntry::getObjects() const {
   std::vector<const object::ObjectFile *> Result;
   Result.reserve(Objects.size());
   for (auto &Object : Objects) {
@@ -133,7 +132,7 @@ CachedBinaryHolder::ObjectEntry::getObjects() const {
   return Result;
 }
 Expected<const object::ObjectFile &>
-CachedBinaryHolder::ObjectEntry::getObject(const Triple &T) const {
+BinaryHolder::ObjectEntry::getObject(const Triple &T) const {
   for (const auto &Obj : Objects) {
     if (const auto *MachO = dyn_cast<object::MachOObjectFile>(Obj.get())) {
       if (MachO->getArchTriple().str() == T.str())
@@ -144,10 +143,10 @@ CachedBinaryHolder::ObjectEntry::getObject(const Triple &T) const {
   return errorCodeToError(object::object_error::arch_not_found);
 }
 
-Expected<const CachedBinaryHolder::ObjectEntry &>
-CachedBinaryHolder::ArchiveEntry::getObjectEntry(StringRef Filename,
-                                                 TimestampTy Timestamp,
-                                                 bool Verbose) {
+Expected<const BinaryHolder::ObjectEntry &>
+BinaryHolder::ArchiveEntry::getObjectEntry(StringRef Filename,
+                                           TimestampTy Timestamp,
+                                           bool Verbose) {
   StringRef ArchiveFilename;
   StringRef ObjectFilename;
   std::tie(ArchiveFilename, ObjectFilename) = getArchiveAndObjectName(Filename);
@@ -183,7 +182,7 @@ CachedBinaryHolder::ArchiveEntry::getObjectEntry(StringRef Filename,
           }
 
           if (Verbose)
-            WithColor::note() << "found member in current archive.\n";
+            WithColor::note() << "found member in archive.\n";
 
           auto ErrOrMem = Child.getMemoryBufferRef();
           if (!ErrOrMem)
@@ -210,8 +209,8 @@ CachedBinaryHolder::ArchiveEntry::getObjectEntry(StringRef Filename,
   return MemberCache[Key];
 }
 
-Expected<const CachedBinaryHolder::ObjectEntry &>
-CachedBinaryHolder::getObjectEntry(StringRef Filename, TimestampTy Timestamp) {
+Expected<const BinaryHolder::ObjectEntry &>
+BinaryHolder::getObjectEntry(StringRef Filename, TimestampTy Timestamp) {
   if (Verbose)
     WithColor::note() << "trying to open '" << Filename << "'\n";
 
@@ -221,7 +220,8 @@ CachedBinaryHolder::getObjectEntry(StringRef Filename, TimestampTy Timestamp) {
     StringRef ArchiveFilename = getArchiveAndObjectName(Filename).first;
     std::lock_guard<std::mutex> Lock(ArchiveCacheMutex);
     if (ArchiveCache.count(ArchiveFilename)) {
-      return ArchiveCache[ArchiveFilename].getObjectEntry(Filename, Timestamp);
+      return ArchiveCache[ArchiveFilename].getObjectEntry(Filename, Timestamp,
+                                                          Verbose);
     } else {
       ArchiveEntry &AE = ArchiveCache[ArchiveFilename];
       auto Err = AE.load(Filename, Timestamp, Verbose);
@@ -230,8 +230,8 @@ CachedBinaryHolder::getObjectEntry(StringRef Filename, TimestampTy Timestamp) {
         // Don't return the error here: maybe the file wasn't an archive.
         llvm::consumeError(std::move(Err));
       } else {
-        return ArchiveCache[ArchiveFilename].getObjectEntry(Filename,
-                                                            Timestamp);
+        return ArchiveCache[ArchiveFilename].getObjectEntry(Filename, Timestamp,
+                                                            Verbose);
       }
     }
   }
@@ -241,7 +241,7 @@ CachedBinaryHolder::getObjectEntry(StringRef Filename, TimestampTy Timestamp) {
   std::lock_guard<std::mutex> Lock(ObjectCacheMutex);
   if (!ObjectCache.count(Filename)) {
     ObjectEntry &OE = ObjectCache[Filename];
-    auto Err = OE.load(Filename);
+    auto Err = OE.load(Filename, Verbose);
     if (Err) {
       ObjectCache.erase(Filename);
       return std::move(Err);
@@ -251,182 +251,12 @@ CachedBinaryHolder::getObjectEntry(StringRef Filename, TimestampTy Timestamp) {
   return ObjectCache[Filename];
 }
 
-void CachedBinaryHolder::clear() {
+void BinaryHolder::clear() {
   std::lock_guard<std::mutex> ArchiveLock(ArchiveCacheMutex);
   std::lock_guard<std::mutex> ObjectLock(ObjectCacheMutex);
   ArchiveCache.clear();
   ObjectCache.clear();
 }
 
-void BinaryHolder::changeBackingMemoryBuffer(
-    std::unique_ptr<MemoryBuffer> &&Buf) {
-  CurrentArchives.clear();
-  CurrentObjectFiles.clear();
-  CurrentFatBinary.reset();
-
-  CurrentMemoryBuffer = std::move(Buf);
-}
-
-ErrorOr<std::vector<MemoryBufferRef>> BinaryHolder::GetMemoryBuffersForFile(
-    StringRef Filename, sys::TimePoint<std::chrono::seconds> Timestamp) {
-  if (Verbose)
-    outs() << "trying to open '" << Filename << "'\n";
-
-  // Try that first as it doesn't involve any filesystem access.
-  if (auto ErrOrArchiveMembers = GetArchiveMemberBuffers(Filename, Timestamp))
-    return *ErrOrArchiveMembers;
-
-  // If the name ends with a closing paren, there is a huge chance
-  // it is an archive member specification.
-  if (Filename.endswith(")"))
-    if (auto ErrOrArchiveMembers =
-            MapArchiveAndGetMemberBuffers(Filename, Timestamp))
-      return *ErrOrArchiveMembers;
-
-  // Otherwise, just try opening a standard file. If this is an
-  // archive member specifiaction and any of the above didn't handle it
-  // (either because the archive is not there anymore, or because the
-  // archive doesn't contain the requested member), this will still
-  // provide a sensible error message.
-  auto ErrOrFile = MemoryBuffer::getFileOrSTDIN(Filename, -1, false);
-  if (auto Err = ErrOrFile.getError())
-    return Err;
-
-  changeBackingMemoryBuffer(std::move(*ErrOrFile));
-  if (Verbose)
-    outs() << "\tloaded file.\n";
-
-  auto ErrOrFat = object::MachOUniversalBinary::create(
-      CurrentMemoryBuffer->getMemBufferRef());
-  if (!ErrOrFat) {
-    consumeError(ErrOrFat.takeError());
-    // Not a fat binary must be a standard one. Return a one element vector.
-    return std::vector<MemoryBufferRef>{CurrentMemoryBuffer->getMemBufferRef()};
-  }
-
-  CurrentFatBinary = std::move(*ErrOrFat);
-  CurrentFatBinaryName = Filename;
-  return getMachOFatMemoryBuffers(CurrentFatBinaryName, *CurrentMemoryBuffer,
-                                  *CurrentFatBinary);
-}
-
-ErrorOr<std::vector<MemoryBufferRef>> BinaryHolder::GetArchiveMemberBuffers(
-    StringRef Filename, sys::TimePoint<std::chrono::seconds> Timestamp) {
-  if (CurrentArchives.empty())
-    return make_error_code(errc::no_such_file_or_directory);
-
-  StringRef CurArchiveName = CurrentArchives.front()->getFileName();
-  if (!Filename.startswith(Twine(CurArchiveName, "(").str()))
-    return make_error_code(errc::no_such_file_or_directory);
-
-  // Remove the archive name and the parens around the archive member name.
-  Filename = Filename.substr(CurArchiveName.size() + 1).drop_back();
-
-  std::vector<MemoryBufferRef> Buffers;
-  Buffers.reserve(CurrentArchives.size());
-
-  for (const auto &CurrentArchive : CurrentArchives) {
-    Error Err = Error::success();
-    for (auto Child : CurrentArchive->children(Err)) {
-      if (auto NameOrErr = Child.getName()) {
-        if (*NameOrErr == Filename) {
-          auto ModTimeOrErr = Child.getLastModified();
-          if (!ModTimeOrErr)
-            return errorToErrorCode(ModTimeOrErr.takeError());
-          if (Timestamp != sys::TimePoint<>() &&
-              Timestamp != ModTimeOrErr.get()) {
-            if (Verbose)
-              outs() << "\tmember had timestamp mismatch.\n";
-            continue;
-          }
-          if (Verbose)
-            outs() << "\tfound member in current archive.\n";
-          auto ErrOrMem = Child.getMemoryBufferRef();
-          if (!ErrOrMem)
-            return errorToErrorCode(ErrOrMem.takeError());
-          Buffers.push_back(*ErrOrMem);
-        }
-      }
-    }
-    if (Err)
-      return errorToErrorCode(std::move(Err));
-  }
-
-  if (Buffers.empty())
-    return make_error_code(errc::no_such_file_or_directory);
-  return Buffers;
-}
-
-ErrorOr<std::vector<MemoryBufferRef>>
-BinaryHolder::MapArchiveAndGetMemberBuffers(
-    StringRef Filename, sys::TimePoint<std::chrono::seconds> Timestamp) {
-  StringRef ArchiveFilename = Filename.substr(0, Filename.find('('));
-
-  auto ErrOrBuff = MemoryBuffer::getFileOrSTDIN(ArchiveFilename, -1, false);
-  if (auto Err = ErrOrBuff.getError())
-    return Err;
-
-  if (Verbose)
-    outs() << "\topened new archive '" << ArchiveFilename << "'\n";
-
-  changeBackingMemoryBuffer(std::move(*ErrOrBuff));
-  std::vector<MemoryBufferRef> ArchiveBuffers;
-  auto ErrOrFat = object::MachOUniversalBinary::create(
-      CurrentMemoryBuffer->getMemBufferRef());
-  if (!ErrOrFat) {
-    consumeError(ErrOrFat.takeError());
-    // Not a fat binary must be a standard one.
-    ArchiveBuffers.push_back(CurrentMemoryBuffer->getMemBufferRef());
-  } else {
-    CurrentFatBinary = std::move(*ErrOrFat);
-    CurrentFatBinaryName = ArchiveFilename;
-    ArchiveBuffers = getMachOFatMemoryBuffers(
-        CurrentFatBinaryName, *CurrentMemoryBuffer, *CurrentFatBinary);
-  }
-
-  for (auto MemRef : ArchiveBuffers) {
-    auto ErrOrArchive = object::Archive::create(MemRef);
-    if (!ErrOrArchive)
-      return errorToErrorCode(ErrOrArchive.takeError());
-    CurrentArchives.push_back(std::move(*ErrOrArchive));
-  }
-  return GetArchiveMemberBuffers(Filename, Timestamp);
-}
-
-ErrorOr<const object::ObjectFile &>
-BinaryHolder::getObjfileForArch(const Triple &T) {
-  for (const auto &Obj : CurrentObjectFiles) {
-    if (const auto *MachO = dyn_cast<object::MachOObjectFile>(Obj.get())) {
-      if (MachO->getArchTriple().str() == T.str())
-        return *MachO;
-    } else if (Obj->getArch() == T.getArch())
-      return *Obj;
-  }
-
-  return make_error_code(object::object_error::arch_not_found);
-}
-
-ErrorOr<std::vector<const object::ObjectFile *>>
-BinaryHolder::GetObjectFiles(StringRef Filename,
-                             sys::TimePoint<std::chrono::seconds> Timestamp) {
-  auto ErrOrMemBufferRefs = GetMemoryBuffersForFile(Filename, Timestamp);
-  if (auto Err = ErrOrMemBufferRefs.getError())
-    return Err;
-
-  std::vector<const object::ObjectFile *> Objects;
-  Objects.reserve(ErrOrMemBufferRefs->size());
-
-  CurrentObjectFiles.clear();
-  for (auto MemBuf : *ErrOrMemBufferRefs) {
-    auto ErrOrObjectFile = object::ObjectFile::createObjectFile(MemBuf);
-    if (!ErrOrObjectFile)
-      return errorToErrorCode(ErrOrObjectFile.takeError());
-
-    Objects.push_back(ErrOrObjectFile->get());
-    CurrentObjectFiles.push_back(std::move(*ErrOrObjectFile));
-  }
-
-  return std::move(Objects);
-}
 } // namespace dsymutil
 } // namespace llvm
diff --git a/llvm/tools/dsymutil/BinaryHolder.h b/llvm/tools/dsymutil/BinaryHolder.h
index c4eef47ac65..9cc79415f17 100644
--- a/llvm/tools/dsymutil/BinaryHolder.h
+++ b/llvm/tools/dsymutil/BinaryHolder.h
@@ -29,15 +29,15 @@
 namespace llvm {
 namespace dsymutil {
 
-/// The CachedBinaryHolder class is responsible for creating and owning
+/// The BinaryHolder class is responsible for creating and owning
 /// ObjectFiles and their underlying MemoryBuffers. It differs from a simple
 /// OwningBinary in that it handles accessing and caching of archives and its
 /// members.
-class CachedBinaryHolder {
+class BinaryHolder {
 public:
   using TimestampTy = sys::TimePoint<std::chrono::seconds>;
 
-  CachedBinaryHolder(bool Verbose = false) : Verbose(Verbose) {}
+  BinaryHolder(bool Verbose = false) : Verbose(Verbose) {}
 
   // Forward declarations for friend declaration.
   class ObjectEntry;
@@ -64,7 +64,7 @@ public:
     /// conversion might be invalid, in which case an Error is returned.
     template <typename ObjectFileType>
     Expected<std::vector<const ObjectFileType *>> getObjectsAs() const {
-      std::vector<const object::ObjectFile *> Result;
+      std::vector<const ObjectFileType *> Result;
       Result.reserve(Objects.size());
       for (auto &Object : Objects) {
         const auto *Derived = dyn_cast<ObjectFileType>(Object.get());
@@ -118,8 +118,8 @@ public:
     std::mutex MemberCacheMutex;
   };
 
-  Expected<const ObjectEntry &> getObjectEntry(StringRef Filename,
-                                               TimestampTy Timestamp);
+  Expected<const ObjectEntry &>
+  getObjectEntry(StringRef Filename, TimestampTy Timestamp = TimestampTy());
 
   void clear();
 
@@ -136,138 +136,27 @@ private:
   bool Verbose;
 };
 
-/// The BinaryHolder class is responsible for creating and owning ObjectFile
-/// objects and their underlying MemoryBuffer. This is different from a simple
-/// OwningBinary in that it handles accessing to archive members.
-///
-/// As an optimization, this class will reuse an already mapped and parsed
-/// Archive object if 2 successive requests target the same archive file (Which
-/// is always the case in debug maps).
-/// Currently it only owns one memory buffer at any given time, meaning that a
-/// mapping request will invalidate the previous memory mapping.
-class BinaryHolder {
-  std::vector<std::unique_ptr<object::Archive>> CurrentArchives;
-  std::unique_ptr<MemoryBuffer> CurrentMemoryBuffer;
-  std::vector<std::unique_ptr<object::ObjectFile>> CurrentObjectFiles;
-  std::unique_ptr<object::MachOUniversalBinary> CurrentFatBinary;
-  std::string CurrentFatBinaryName;
-  bool Verbose;
-
-  /// Get the MemoryBufferRefs for the file specification in \p
-  /// Filename from the current archive. Multiple buffers are returned
-  /// when there are multiple architectures available for the
-  /// requested file.
-  ///
-  /// This function performs no system calls, it just looks up a
-  /// potential match for the given \p Filename in the currently
-  /// mapped archive if there is one.
-  ErrorOr<std::vector<MemoryBufferRef>>
-  GetArchiveMemberBuffers(StringRef Filename,
-                          sys::TimePoint<std::chrono::seconds> Timestamp);
-
-  /// Interpret Filename as an archive member specification map the
-  /// corresponding archive to memory and return the MemoryBufferRefs
-  /// corresponding to the described member. Multiple buffers are
-  /// returned when there are multiple architectures available for the
-  /// requested file.
-  ErrorOr<std::vector<MemoryBufferRef>>
-  MapArchiveAndGetMemberBuffers(StringRef Filename,
-                                sys::TimePoint<std::chrono::seconds> Timestamp);
-
-  /// Return the MemoryBufferRef that holds the memory mapping for the
-  /// given \p Filename. This function will try to parse archive
-  /// member specifications of the form /path/to/archive.a(member.o).
-  ///
-  /// The returned MemoryBufferRefs points to a buffer owned by this
-  /// object. The buffer is valid until the next call to
-  /// GetMemoryBufferForFile() on this object.
-  /// Multiple buffers are returned when there are multiple
-  /// architectures available for the requested file.
-  ErrorOr<std::vector<MemoryBufferRef>>
-  GetMemoryBuffersForFile(StringRef Filename,
-                          sys::TimePoint<std::chrono::seconds> Timestamp);
-
-  void changeBackingMemoryBuffer(std::unique_ptr<MemoryBuffer> &&MemBuf);
-  ErrorOr<const object::ObjectFile &> getObjfileForArch(const Triple &T);
-
-public:
-  BinaryHolder(bool Verbose) : Verbose(Verbose) {}
-
-  /// Get the ObjectFiles designated by the \p Filename. This
-  /// might be an archive member specification of the form
-  /// /path/to/archive.a(member.o).
-  ///
-  /// Calling this function invalidates the previous mapping owned by
-  /// the BinaryHolder. Multiple buffers are returned when there are
-  /// multiple architectures available for the requested file.
-  ErrorOr<std::vector<const object::ObjectFile *>>
-  GetObjectFiles(StringRef Filename,
-                 sys::TimePoint<std::chrono::seconds> Timestamp =
-                     sys::TimePoint<std::chrono::seconds>());
-
-  /// Wraps GetObjectFiles() to return a derived ObjectFile type.
-  template <typename ObjectFileType>
-  ErrorOr<std::vector<const ObjectFileType *>>
-  GetFilesAs(StringRef Filename,
-             sys::TimePoint<std::chrono::seconds> Timestamp =
-                 sys::TimePoint<std::chrono::seconds>()) {
-    auto ErrOrObjFile = GetObjectFiles(Filename, Timestamp);
-    if (auto Err = ErrOrObjFile.getError())
-      return Err;
-
-    std::vector<const ObjectFileType *> Objects;
-    Objects.reserve((*ErrOrObjFile).size());
-    for (const auto &Obj : *ErrOrObjFile) {
-      const auto *Derived = dyn_cast<ObjectFileType>(Obj);
-      if (!Derived)
-        return make_error_code(object::object_error::invalid_file_type);
-      Objects.push_back(Derived);
-    }
-    return std::move(Objects);
-  }
-
-  /// Access the currently owned ObjectFile with architecture \p T. As
-  /// successful call to GetObjectFiles() or GetFilesAs() must have
-  /// been performed before calling this.
-  ErrorOr<const object::ObjectFile &> Get(const Triple &T) {
-    return getObjfileForArch(T);
-  }
-
-  /// Get and cast to a subclass of the currently owned ObjectFile. The
-  /// conversion must be known to be valid.
-  template <typename ObjectFileType>
-  ErrorOr<const ObjectFileType &> GetAs(const Triple &T) {
-    auto ErrOrObj = Get(T);
-    if (auto Err = ErrOrObj.getError())
-      return Err;
-    return cast<ObjectFileType>(*ErrOrObj);
-  }
-};
 } // namespace dsymutil
 
-template <>
-struct DenseMapInfo<dsymutil::CachedBinaryHolder::ArchiveEntry::KeyTy> {
+template <> struct DenseMapInfo<dsymutil::BinaryHolder::ArchiveEntry::KeyTy> {
 
-  static inline dsymutil::CachedBinaryHolder::ArchiveEntry::KeyTy
-  getEmptyKey() {
-    return dsymutil::CachedBinaryHolder::ArchiveEntry::KeyTy();
+  static inline dsymutil::BinaryHolder::ArchiveEntry::KeyTy getEmptyKey() {
+    return dsymutil::BinaryHolder::ArchiveEntry::KeyTy();
   }
 
-  static inline dsymutil::CachedBinaryHolder::ArchiveEntry::KeyTy
-  getTombstoneKey() {
-    return dsymutil::CachedBinaryHolder::ArchiveEntry::KeyTy("/", {});
+  static inline dsymutil::BinaryHolder::ArchiveEntry::KeyTy getTombstoneKey() {
+    return dsymutil::BinaryHolder::ArchiveEntry::KeyTy("/", {});
   }
 
   static unsigned
-  getHashValue(const dsymutil::CachedBinaryHolder::ArchiveEntry::KeyTy &K) {
+  getHashValue(const dsymutil::BinaryHolder::ArchiveEntry::KeyTy &K) {
     return hash_combine(DenseMapInfo<StringRef>::getHashValue(K.Filename),
                         DenseMapInfo<unsigned>::getHashValue(
                             K.Timestamp.time_since_epoch().count()));
   }
 
-  static bool
-  isEqual(const dsymutil::CachedBinaryHolder::ArchiveEntry::KeyTy &LHS,
-          const dsymutil::CachedBinaryHolder::ArchiveEntry::KeyTy &RHS) {
+  static bool isEqual(const dsymutil::BinaryHolder::ArchiveEntry::KeyTy &LHS,
+                      const dsymutil::BinaryHolder::ArchiveEntry::KeyTy &RHS) {
     return LHS.Filename == RHS.Filename && LHS.Timestamp == RHS.Timestamp;
   }
 };
diff --git a/llvm/tools/dsymutil/DebugMap.cpp b/llvm/tools/dsymutil/DebugMap.cpp
index d4828171b2b..26137773a4e 100644
--- a/llvm/tools/dsymutil/DebugMap.cpp
+++ b/llvm/tools/dsymutil/DebugMap.cpp
@@ -240,26 +240,31 @@ MappingTraits<dsymutil::DebugMapObject>::YamlDMO::denormalize(IO &IO) {
   StringMap<uint64_t> SymbolAddresses;
 
   sys::path::append(Path, Filename);
-  auto ErrOrObjectFiles = BinHolder.GetObjectFiles(Path);
-  if (auto EC = ErrOrObjectFiles.getError()) {
-    WithColor::warning() << "Unable to open " << Path << " " << EC.message()
-                         << '\n';
-  } else if (auto ErrOrObjectFile = BinHolder.Get(Ctxt.BinaryTriple)) {
-    // Rewrite the object file symbol addresses in the debug map. The YAML
-    // input is mainly used to test dsymutil without requiring binaries
-    // checked-in. If we generate the object files during the test, we can't
-    // hard-code the symbols addresses, so look them up here and rewrite them.
-    for (const auto &Sym : ErrOrObjectFile->symbols()) {
-      uint64_t Address = Sym.getValue();
-      Expected<StringRef> Name = Sym.getName();
-      if (!Name ||
-          (Sym.getFlags() & (SymbolRef::SF_Absolute | SymbolRef::SF_Common))) {
-        // TODO: Actually report errors helpfully.
-        if (!Name)
-          consumeError(Name.takeError());
-        continue;
+
+  auto ObjectEntry = BinHolder.getObjectEntry(Path);
+  if (!ObjectEntry) {
+    auto Err = ObjectEntry.takeError();
+    WithColor::warning() << "Unable to open " << Path << " "
+                         << toString(std::move(Err)) << '\n';
+  } else {
+    auto Object = ObjectEntry->getObject(Ctxt.BinaryTriple);
+    if (!Object) {
+      auto Err = Object.takeError();
+      WithColor::warning() << "Unable to open " << Path << " "
+                           << toString(std::move(Err)) << '\n';
+    } else {
+      for (const auto &Sym : Object->symbols()) {
+        uint64_t Address = Sym.getValue();
+        Expected<StringRef> Name = Sym.getName();
+        if (!Name || (Sym.getFlags() &
+                      (SymbolRef::SF_Absolute | SymbolRef::SF_Common))) {
+          // TODO: Actually report errors helpfully.
+          if (!Name)
+            consumeError(Name.takeError());
+          continue;
+        }
+        SymbolAddresses[*Name] = Address;
       }
-      SymbolAddresses[*Name] = Address;
     }
   }
 
diff --git a/llvm/tools/dsymutil/DwarfLinker.cpp b/llvm/tools/dsymutil/DwarfLinker.cpp
index 48248dc3c33..4e3457ded30 100644
--- a/llvm/tools/dsymutil/DwarfLinker.cpp
+++ b/llvm/tools/dsymutil/DwarfLinker.cpp
@@ -2467,7 +2467,7 @@ bool DwarfLinker::link(const DebugMap &Map) {
   return Options.NoOutput ? true : Streamer->finish(Map);
 }
 
-bool linkDwarf(raw_fd_ostream &OutFile, CachedBinaryHolder &BinHolder,
+bool linkDwarf(raw_fd_ostream &OutFile, BinaryHolder &BinHolder,
                const DebugMap &DM, const LinkOptions &Options) {
   DwarfLinker Linker(OutFile, BinHolder, Options);
   return Linker.link(DM);
diff --git a/llvm/tools/dsymutil/DwarfLinker.h b/llvm/tools/dsymutil/DwarfLinker.h
index 618b1ae9bd7..ef82911fabc 100644
--- a/llvm/tools/dsymutil/DwarfLinker.h
+++ b/llvm/tools/dsymutil/DwarfLinker.h
@@ -56,7 +56,7 @@ using UnitListTy = std::vector<std::unique_ptr<CompileUnit>>;
 /// first step when we start processing a DebugMapObject.
 class DwarfLinker {
 public:
-  DwarfLinker(raw_fd_ostream &OutFile, CachedBinaryHolder &BinHolder,
+  DwarfLinker(raw_fd_ostream &OutFile, BinaryHolder &BinHolder,
               const LinkOptions &Options)
       : OutFile(OutFile), BinHolder(BinHolder), Options(Options) {}
 
@@ -445,7 +445,7 @@ private:
   /// @}
 
   raw_fd_ostream &OutFile;
-  CachedBinaryHolder &BinHolder;
+  BinaryHolder &BinHolder;
   LinkOptions Options;
   std::unique_ptr<DwarfStreamer> Streamer;
   uint64_t OutputDebugInfoSize;
diff --git a/llvm/tools/dsymutil/MachODebugMapParser.cpp b/llvm/tools/dsymutil/MachODebugMapParser.cpp
index 13416097ca3..48155b40204 100644
--- a/llvm/tools/dsymutil/MachODebugMapParser.cpp
+++ b/llvm/tools/dsymutil/MachODebugMapParser.cpp
@@ -28,8 +28,7 @@ public:
                       bool PaperTrailWarnings = false, bool Verbose = false)
       : BinaryPath(BinaryPath), Archs(Archs.begin(), Archs.end()),
         PathPrefix(PathPrefix), PaperTrailWarnings(PaperTrailWarnings),
-        MainBinaryHolder(Verbose), CurrentObjectHolder(Verbose),
-        CurrentDebugMapObject(nullptr) {}
+        BinHolder(Verbose), CurrentDebugMapObject(nullptr) {}
 
   /// Parses and returns the DebugMaps of the input binary. The binary contains
   /// multiple maps in case it is a universal binary.
@@ -47,15 +46,13 @@ private:
   bool PaperTrailWarnings;
 
   /// Owns the MemoryBuffer for the main binary.
-  BinaryHolder MainBinaryHolder;
+  BinaryHolder BinHolder;
   /// Map of the binary symbol addresses.
   StringMap<uint64_t> MainBinarySymbolAddresses;
   StringRef MainBinaryStrings;
   /// The constructed DebugMap.
   std::unique_ptr<DebugMap> Result;
 
-  /// Owns the MemoryBuffer for the currently handled object file.
-  BinaryHolder CurrentObjectHolder;
   /// Map of the currently processed object file symbol addresses.
   StringMap<Optional<uint64_t>> CurrentObjectAddresses;
   /// Element of the debug map corresponding to the current object file.
@@ -136,23 +133,25 @@ void MachODebugMapParser::switchToNewDebugMapObject(
   SmallString<80> Path(PathPrefix);
   sys::path::append(Path, Filename);
 
-  auto MachOOrError =
-      CurrentObjectHolder.GetFilesAs<MachOObjectFile>(Path, Timestamp);
-  if (auto Error = MachOOrError.getError()) {
-    Warning("unable to open object file: " + Error.message(), Path.str());
+  auto ObjectEntry = BinHolder.getObjectEntry(Path, Timestamp);
+  if (!ObjectEntry) {
+    auto Err = ObjectEntry.takeError();
+    Warning("unable to open object file: " + toString(std::move(Err)),
+            Path.str());
     return;
   }
 
-  auto ErrOrAchObj =
-      CurrentObjectHolder.GetAs<MachOObjectFile>(Result->getTriple());
-  if (auto Error = ErrOrAchObj.getError()) {
-    Warning("unable to open object file: " + Error.message(), Path.str());
+  auto Object = ObjectEntry->getObjectAs<MachOObjectFile>(Result->getTriple());
+  if (!Object) {
+    auto Err = Object.takeError();
+    Warning("unable to open object file: " + toString(std::move(Err)),
+            Path.str());
     return;
   }
 
   CurrentDebugMapObject =
       &Result->addDebugMapObject(Path, Timestamp, MachO::N_OSO);
-  loadCurrentObjectFileSymbols(*ErrOrAchObj);
+  loadCurrentObjectFileSymbols(*Object);
 }
 
 static std::string getArchName(const object::MachOObjectFile &Obj) {
@@ -322,17 +321,26 @@ static bool shouldLinkArch(SmallVectorImpl<StringRef> &Archs, StringRef Arch) {
 }
 
 bool MachODebugMapParser::dumpStab() {
-  auto MainBinOrError =
-      MainBinaryHolder.GetFilesAs<MachOObjectFile>(BinaryPath);
-  if (auto Error = MainBinOrError.getError()) {
-    llvm::errs() << "Cannot get '" << BinaryPath
-                 << "' as MachO file: " << Error.message() << "\n";
+  auto ObjectEntry = BinHolder.getObjectEntry(BinaryPath);
+  if (!ObjectEntry) {
+    auto Err = ObjectEntry.takeError();
+    WithColor::error() << "cannot load '" << BinaryPath
+                       << "': " << toString(std::move(Err)) << '\n';
     return false;
   }
 
-  for (const auto *Binary : *MainBinOrError)
-    if (shouldLinkArch(Archs, Binary->getArchTriple().getArchName()))
-      dumpOneBinaryStab(*Binary, BinaryPath);
+  auto Objects = ObjectEntry->getObjectsAs<MachOObjectFile>();
+  if (!Objects) {
+    auto Err = Objects.takeError();
+    WithColor::error() << "cannot get '" << BinaryPath
+                       << "' as MachO file: " << toString(std::move(Err))
+                       << "\n";
+    return false;
+  }
+
+  for (const auto *Object : *Objects)
+    if (shouldLinkArch(Archs, Object->getArchTriple().getArchName()))
+      dumpOneBinaryStab(*Object, BinaryPath);
 
   return true;
 }
@@ -341,15 +349,20 @@ bool MachODebugMapParser::dumpStab() {
 /// successful iterates over the STAB entries. The real parsing is
 /// done in handleStabSymbolTableEntry.
 ErrorOr<std::vector<std::unique_ptr<DebugMap>>> MachODebugMapParser::parse() {
-  auto MainBinOrError =
-      MainBinaryHolder.GetFilesAs<MachOObjectFile>(BinaryPath);
-  if (auto Error = MainBinOrError.getError())
-    return Error;
+  auto ObjectEntry = BinHolder.getObjectEntry(BinaryPath);
+  if (!ObjectEntry) {
+    return errorToErrorCode(ObjectEntry.takeError());
+  }
+
+  auto Objects = ObjectEntry->getObjectsAs<MachOObjectFile>();
+  if (!Objects) {
+    return errorToErrorCode(ObjectEntry.takeError());
+  }
 
   std::vector<std::unique_ptr<DebugMap>> Results;
-  for (const auto *Binary : *MainBinOrError)
-    if (shouldLinkArch(Archs, Binary->getArchTriple().getArchName()))
-      Results.push_back(parseOneBinary(*Binary, BinaryPath));
+  for (const auto *Object : *Objects)
+    if (shouldLinkArch(Archs, Object->getArchTriple().getArchName()))
+      Results.push_back(parseOneBinary(*Object, BinaryPath));
 
   return std::move(Results);
 }
diff --git a/llvm/tools/dsymutil/MachOUtils.cpp b/llvm/tools/dsymutil/MachOUtils.cpp
index cd99d720225..5d70afdc9f7 100644
--- a/llvm/tools/dsymutil/MachOUtils.cpp
+++ b/llvm/tools/dsymutil/MachOUtils.cpp
@@ -327,19 +327,25 @@ bool generateDsymCompanion(const DebugMap &DM, MCStreamer &MS,
   MCAsm.layout(Layout);
 
   BinaryHolder InputBinaryHolder(false);
-  auto ErrOrObjs = InputBinaryHolder.GetObjectFiles(DM.getBinaryPath());
-  if (auto Error = ErrOrObjs.getError())
+
+  auto ObjectEntry = InputBinaryHolder.getObjectEntry(DM.getBinaryPath());
+  if (!ObjectEntry) {
+    auto Err = ObjectEntry.takeError();
     return error(Twine("opening ") + DM.getBinaryPath() + ": " +
-                     Error.message(),
+                     toString(std::move(Err)),
                  "output file streaming");
+  }
 
-  auto ErrOrInputBinary =
-      InputBinaryHolder.GetAs<object::MachOObjectFile>(DM.getTriple());
-  if (auto Error = ErrOrInputBinary.getError())
+  auto Object =
+      ObjectEntry->getObjectAs<object::MachOObjectFile>(DM.getTriple());
+  if (!Object) {
+    auto Err = Object.takeError();
     return error(Twine("opening ") + DM.getBinaryPath() + ": " +
-                     Error.message(),
+                     toString(std::move(Err)),
                  "output file streaming");
-  auto &InputBinary = *ErrOrInputBinary;
+  }
+
+  auto &InputBinary = *Object;
 
   bool Is64Bit = Writer.is64Bit();
   MachO::symtab_command SymtabCmd = InputBinary.getSymtabLoadCommand();
diff --git a/llvm/tools/dsymutil/dsymutil.cpp b/llvm/tools/dsymutil/dsymutil.cpp
index 6effa1f6a98..2e633fcac6a 100644
--- a/llvm/tools/dsymutil/dsymutil.cpp
+++ b/llvm/tools/dsymutil/dsymutil.cpp
@@ -496,7 +496,7 @@ int main(int argc, char **argv) {
     }
 
     // Shared a single binary holder for all the link steps.
-    CachedBinaryHolder BinHolder;
+    BinaryHolder BinHolder;
 
     NumThreads =
         std::min<unsigned>(OptionsOrErr->Threads, DebugMapPtrsOrErr->size());
diff --git a/llvm/tools/dsymutil/dsymutil.h b/llvm/tools/dsymutil/dsymutil.h
index d54b46cde56..556b2d65e1a 100644
--- a/llvm/tools/dsymutil/dsymutil.h
+++ b/llvm/tools/dsymutil/dsymutil.h
@@ -30,7 +30,7 @@
 namespace llvm {
 namespace dsymutil {
 
-class CachedBinaryHolder;
+class BinaryHolder;
 
 /// Extract the DebugMaps from the given file.
 /// The file has to be a MachO object file. Multiple debug maps can be
@@ -46,7 +46,7 @@ bool dumpStab(StringRef InputFile, ArrayRef<std::string> Archs,
 
 /// Link the Dwarf debug info as directed by the passed DebugMap \p DM into a
 /// DwarfFile named \p OutputFilename. \returns false if the link failed.
-bool linkDwarf(raw_fd_ostream &OutFile, CachedBinaryHolder &BinHolder,
+bool linkDwarf(raw_fd_ostream &OutFile, BinaryHolder &BinHolder,
                const DebugMap &DM, const LinkOptions &Options);
 
 } // end namespace dsymutil