[dsymutil] Make the CachedBinaryHolder the default.

[JDevlieghere]

Summary

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

[friss]

llvm/tools/dsymutil/BinaryHolder.h

100 ↗

(On Diff #153484)

Why does this have to change in this patch? This looks like it's addressing a lifetime issue, but I'm not sure which one (and wouldn't it be needed in D48501?)

[JDevlieghere]

llvm/tools/dsymutil/BinaryHolder.h

100 ↗

(On Diff #153484)

Correct. I'll move into the first patch.

[JDevlieghere]

• Move lifetime issue into parent patch.

[friss]

LGTM

llvm/trunk/test/tools/dsymutil/debug-map-parsing.test

 RUN: dsymutil -dump-debug-map -oso-prepend-path=%p %p/Inputs/basic.macho.x86_64 | FileCheck %s
 RUN: dsymutil -dump-debug-map -oso-prepend-path=%p %p/Inputs/basic-lto.macho.x86_64 | FileCheck %s --check-prefix=CHECK-LTO
-RUN: dsymutil -verbose -dump-debug-map -oso-prepend-path=%p %p/Inputs/basic-archive.macho.x86_64 | FileCheck %s --check-prefix=CHECK-ARCHIVE
+RUN: dsymutil -verbose -dump-debug-map -oso-prepend-path=%p %p/Inputs/basic-archive.macho.x86_64 2>&1 | FileCheck %s --check-prefix=CHECK-ARCHIVE
 RUN: dsymutil -dump-debug-map %p/Inputs/basic.macho.x86_64 2>&1 | FileCheck %s --check-prefix=NOT-FOUND
 RUN: not dsymutil -dump-debug-map %p/Inputs/inexistant 2>&1 | FileCheck %s --check-prefix=NO-EXECUTABLE
 
 
 Check that We can parse the debug map of the basic executable.
 
 CHECK-NOT: error
 CHECK: ---

 CHECK-LTO-DAG:  sym: _private_int, objAddr: 0x00000000000008E8, binAddr: 0x0000000100001008, size: 0x00000000
 CHECK-LTO-DAG:  sym: _val, objAddr: 0x00000000000008EC, binAddr: 0x0000000100001004, size: 0x00000000
 CHECK-LTO: ...
 
 Check thet we correctly handle debug maps with archive members (including only
 opening the archive once if mulitple of its members are used).
 
 CHECK-ARCHIVE:      trying to open {{.*}}basic-archive.macho.x86_64'
-CHECK-ARCHIVE-NEXT:   loaded file.
+CHECK-ARCHIVE-NEXT:   loaded object.
 CHECK-ARCHIVE-NEXT: trying to open {{.*}}/Inputs/basic1.macho.x86_64.o'
-CHECK-ARCHIVE-NEXT:   loaded file.
+CHECK-ARCHIVE-NEXT:   loaded object.
 CHECK-ARCHIVE-NEXT: trying to open {{.*}}/libbasic.a(basic2.macho.x86_64.o)'
-CHECK-ARCHIVE-NEXT:   opened new archive {{.*}}/libbasic.a'
-CHECK-ARCHIVE-NEXT:   found member in current archive.
+CHECK-ARCHIVE-NEXT:   loaded archive {{.*}}/libbasic.a'
+CHECK-ARCHIVE-NEXT:   found member in archive.
 CHECK-ARCHIVE-NEXT: trying to open {{.*}}/libbasic.a(basic3.macho.x86_64.o)'
-CHECK-ARCHIVE-NEXT:   found member in current archive.
+CHECK-ARCHIVE-NEXT:   found member in archive.
 CHECK-ARCHIVE: ---
 CHECK-ARCHIVE: triple: 'x86_64-apple-darwin'
 CHECK-ARCHIVE: binary-path:{{.*}}/Inputs/basic-archive.macho.x86_64
 CHECK-ARCHIVE: /Inputs/basic1.macho.x86_64.o
 CHECK-ARCHIVE-DAG:  sym: _main, objAddr: 0x0000000000000000, binAddr: 0x0000000100000EA0, size: 0x00000024
 CHECK-ARCHIVE: /Inputs/./libbasic.a(basic2.macho.x86_64.o)
 CHECK-ARCHIVE-DAG:  sym: _baz, objAddr: 0x0000000000000310, binAddr: 0x0000000100001000, size: 0x00000000
 CHECK-ARCHIVE-DAG:  sym: _foo, objAddr: 0x0000000000000020, binAddr: 0x0000000100000ED0, size: 0x00000050

llvm/trunk/tools/dsymutil/BinaryHolder.h

 #include "llvm/Support/Errc.h"
 #include "llvm/Support/ErrorOr.h"
 
 #include <mutex>
 
 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;
   class ArchiveEntry;
 
   /// Base class shared by cached entries, representing objects and archives.
   class EntryBase {
   protected:
public:
 
     /// Access all owned ObjectFiles.
     std::vector<const object::ObjectFile *> getObjects() const;
 
     /// Access to a derived version of all the currently owned ObjectFiles. The
     /// 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());
         if (!Derived)
           return errorCodeToError(object::object_error::invalid_file_type);
         Result.push_back(Derived);
       }
       return Result;
Expected<const ObjectEntry &> getObjectEntry(StringRef Filename,
                                                  bool Verbose = false);
 
   private:
     std::vector<std::unique_ptr<object::Archive>> Archives;
     DenseMap<KeyTy, ObjectEntry> MemberCache;
     std::mutex MemberCacheMutex;
   };
 
-  Expected<const ObjectEntry &> getObjectEntry(StringRef Filename,
-                                               TimestampTy Timestamp);
+  Expected<const ObjectEntry &>
+  getObjectEntry(StringRef Filename, TimestampTy Timestamp = TimestampTy());
 
   void clear();
 
 private:
   /// Cache of static archives. Objects that are part of a static archive are
   /// stored under this object, rather than in the map below.
   StringMap<ArchiveEntry> ArchiveCache;
   std::mutex ArchiveCacheMutex;
 
   /// Object entries for objects that are not in a static archive.
   StringMap<ObjectEntry> ObjectCache;
   std::mutex ObjectCacheMutex;
 
   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;
   }
 };
 
 } // namespace llvm
 #endif

llvm/trunk/tools/dsymutil/BinaryHolder.cpp

getMachOFatMemoryBuffers(StringRef Filename, MemoryBuffer &Mem,
   for (auto It = Fat.begin_objects(), End = Fat.end_objects(); It != End;
        ++It) {
     StringRef ObjData = FatData.substr(It->getOffset(), It->getSize());
     Buffers.emplace_back(ObjData, Filename);
   }
   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.
   auto ErrOrBuff = MemoryBuffer::getFileOrSTDIN(ArchiveFilename, -1, false);
   if (auto Err = ErrOrBuff.getError())
     return errorCodeToError(Err);
 
   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.
   std::vector<MemoryBufferRef> ArchiveBuffers;
 
   auto ErrOrFat =
       object::MachOUniversalBinary::create(MemoryBuffer->getMemBufferRef());
   if (!ErrOrFat) {
for (auto MemRef : ArchiveBuffers) {
     if (!ErrOrArchive)
       return ErrOrArchive.takeError();
     Archives.push_back(std::move(*ErrOrArchive));
   }
 
   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())
     return errorCodeToError(Err);
 
   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.
   std::vector<MemoryBufferRef> ObjectBuffers;
 
   auto ErrOrFat =
       object::MachOUniversalBinary::create(MemoryBuffer->getMemBufferRef());
   if (!ErrOrFat) {
if (!ErrOrObjectFile)
       return ErrOrObjectFile.takeError();
     Objects.push_back(std::move(*ErrOrObjectFile));
   }
 
   return Error::success();
 }
 
 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) {
     Result.push_back(Object.get());
   }
   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())
         return *MachO;
     } else if (Obj->getArch() == T.getArch())
       return *Obj;
   }
   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,
   StringRef ArchiveFilename;
   StringRef ObjectFilename;
   std::tie(ArchiveFilename, ObjectFilename) = getArchiveAndObjectName(Filename);
 
   // Try the cache first.
   KeyTy Key = {ObjectFilename, Timestamp};
 
   {
for (auto Child : Archive->children(Err)) {
           if (Timestamp != sys::TimePoint<>() &&
               Timestamp != ModTimeOrErr.get()) {
             if (Verbose)
               WithColor::warning() << "member has timestamp mismatch.\n";
             continue;
           }
 
           if (Verbose)
-            WithColor::note() << "found member in current archive.\n";
+            WithColor::note() << "found member in archive.\n";
 
           auto ErrOrMem = Child.getMemoryBufferRef();
           if (!ErrOrMem)
             return ErrOrMem.takeError();
 
           auto ErrOrObjectFile =
               object::ObjectFile::createObjectFile(*ErrOrMem);
           if (!ErrOrObjectFile)
BinaryHolder::ArchiveEntry::getObjectEntry(StringRef Filename,
   if (OE.Objects.empty())
     return errorCodeToError(errc::no_such_file_or_directory);
 
   std::lock_guard<std::mutex> Lock(MemberCacheMutex);
   MemberCache.try_emplace(Key, std::move(OE));
   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";
 
   // If this is an archive, we might have either the object or the archive
   // cached. In this case we can load it without accessing the file system.
   if (isArchive(Filename)) {
     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);
       if (Err) {
         ArchiveCache.erase(ArchiveFilename);
         // 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);
       }
     }
   }
 
   // If this is an object, we might have it cached. If not we'll have to load
   // it from the file system and cache it now.
   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);
     }
   }
 
   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

llvm/trunk/tools/dsymutil/DebugMap.cpp

 dsymutil::DebugMapObject
 MappingTraits<dsymutil::DebugMapObject>::YamlDMO::denormalize(IO &IO) {
   BinaryHolder BinHolder(/* Verbose =*/false);
   const auto &Ctxt = *reinterpret_cast<YAMLContext *>(IO.getContext());
   SmallString<80> Path(Ctxt.PrependPath);
   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()) {
+
+  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))) {
+        if (!Name || (Sym.getFlags() &
+                      (SymbolRef::SF_Absolute | SymbolRef::SF_Common))) {
           // TODO: Actually report errors helpfully.
           if (!Name)
             consumeError(Name.takeError());
           continue;
         }
         SymbolAddresses[*Name] = Address;
       }
     }
+  }
 
   dsymutil::DebugMapObject Res(Path, sys::toTimePoint(Timestamp), MachO::N_OSO);
   for (auto &Entry : Entries) {
     auto &Mapping = Entry.second;
     Optional<uint64_t> ObjAddress;
     if (Mapping.ObjectAddress)
       ObjAddress = *Mapping.ObjectAddress;
     auto AddressIt = SymbolAddresses.find(Entry.first);

llvm/trunk/tools/dsymutil/DwarfLinker.h

 ///
 /// The root DIEs are identified because they contain relocations that
 /// correspond to a debug map entry at specific places (the low_pc for
 /// a function, the location for a variable). These relocations are
 /// called ValidRelocs in the DwarfLinker and are gathered as a very
 /// 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) {}
 
   /// Link the contents of the DebugMap.
   bool link(const DebugMap &);
 
   void reportWarning(const Twine &Warning, const DebugMapObject &DMO,
                      const DWARFDie *DIE = nullptr) const;
private:
   bool createStreamer(const Triple &TheTriple, raw_fd_ostream &OutFile);
 
   /// Attempt to load a debug object from disk.
   ErrorOr<const object::ObjectFile &> loadObject(const DebugMapObject &Obj,
                                                  const DebugMap &Map);
   /// @}
 
   raw_fd_ostream &OutFile;
-  CachedBinaryHolder &BinHolder;
+  BinaryHolder &BinHolder;
   LinkOptions Options;
   std::unique_ptr<DwarfStreamer> Streamer;
   uint64_t OutputDebugInfoSize;
   unsigned MaxDwarfVersion = 0;
 
   /// The CIEs that have been emitted in the output section. The actual CIE
   /// data serves a the key to this StringMap, this takes care of comparing the
   /// semantics of CIEs defined in different object files.

llvm/trunk/tools/dsymutil/DwarfLinker.cpp

if (Options.Threads == 1) {
     pool.async(AnalyzeLambda);
     pool.async(CloneLambda);
     pool.wait();
   }
 
   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);
 }
 
 } // namespace dsymutil
 } // namespace llvm

llvm/trunk/tools/dsymutil/MachODebugMapParser.cpp

 
 class MachODebugMapParser {
 public:
   MachODebugMapParser(StringRef BinaryPath, ArrayRef<std::string> Archs,
                       StringRef PathPrefix = "",
                       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.
   /// \returns an error in case the provided BinaryPath doesn't exist
   /// or isn't of a supported type.
   ErrorOr<std::vector<std::unique_ptr<DebugMap>>> parse();
 
   /// Walk the symbol table and dump it.
   bool dumpStab();
 
 private:
   std::string BinaryPath;
   SmallVector<StringRef, 1> Archs;
   std::string PathPrefix;
   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.
   DebugMapObject *CurrentDebugMapObject;
 
   /// Holds function info while function scope processing.
   const char *CurrentFunctionName;
   uint64_t CurrentFunctionAddress;

 /// everything up to add symbols to the new one.
 void MachODebugMapParser::switchToNewDebugMapObject(
     StringRef Filename, sys::TimePoint<std::chrono::seconds> Timestamp) {
   resetParserState();
 
   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) {
   Triple T = Obj.getArchTriple();
   return T.getArchName();
 }
 
 std::unique_ptr<DebugMap>
static bool shouldLinkArch(SmallVectorImpl<StringRef> &Archs, StringRef Arch) {
   SmallString<16> ArchName = Arch;
   if (Arch.startswith("thumb"))
     ArchName = ("arm" + Arch.substr(5)).str();
 
   return is_contained(Archs, ArchName);
 }
 
 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;
 }
 
 /// This main parsing routine tries to open the main binary and if
 /// 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);
 }
 
 /// Interpret the STAB entries to fill the DebugMap.
 void MachODebugMapParser::handleStabSymbolTableEntry(uint32_t StringIndex,
                                                      uint8_t Type,
                                                      uint8_t SectionIndex,

llvm/trunk/tools/dsymutil/MachOUtils.cpp

bool generateDsymCompanion(const DebugMap &DM, MCStreamer &MS,
   auto &ObjectStreamer = static_cast<MCObjectStreamer &>(MS);
   MCAssembler &MCAsm = ObjectStreamer.getAssembler();
   auto &Writer = static_cast<MachObjectWriter &>(MCAsm.getWriter());
   MCAsmLayout Layout(MCAsm);
 
   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();
 
   // Get UUID.
   MachO::uuid_command UUIDCmd;
   memset(&UUIDCmd, 0, sizeof(UUIDCmd));
   UUIDCmd.cmd = MachO::LC_UUID;

llvm/trunk/tools/dsymutil/dsymutil.h

 #include "llvm/Support/ErrorOr.h"
 #include <memory>
 #include <string>
 #include <vector>
 
 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
 /// returned when the file is universal (aka fat) binary.
 ErrorOr<std::vector<std::unique_ptr<DebugMap>>>
 parseDebugMap(StringRef InputFile, ArrayRef<std::string> Archs,
               StringRef PrependPath, bool PaperTrailWarnings, bool Verbose,
               bool InputIsYAML);
 
 /// Dump the symbol table
 bool dumpStab(StringRef InputFile, ArrayRef<std::string> Archs,
               StringRef PrependPath = "");
 
 /// 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
 } // end namespace llvm
 
 #endif // LLVM_TOOLS_DSYMUTIL_DSYMUTIL_H

llvm/trunk/tools/dsymutil/dsymutil.cpp

for (auto &InputFile : *InputsOrErr) {
 
     // Ensure that the debug map is not empty (anymore).
     if (DebugMapPtrsOrErr->empty()) {
       WithColor::error() << "no architecture to link\n";
       return 1;
     }
 
     // Shared a single binary holder for all the link steps.
-    CachedBinaryHolder BinHolder;
+    BinaryHolder BinHolder;
 
     NumThreads =
         std::min<unsigned>(OptionsOrErr->Threads, DebugMapPtrsOrErr->size());
     llvm::ThreadPool Threads(NumThreads);
 
     // If there is more than one link to execute, we need to generate
     // temporary files.
     bool NeedsTempFiles =