[Symbolize] Improve the ownership of parsed objects.

This code changes the way Symbolize handles parsed binaries: now
parsed OwningBinary<Binary> is not broken into (binary, memory buffer)
pair, and is just stored as-is in a cache. ObjectFile components
of Mach-O universal binaries are also stored explicitly in a
separate cache.

Additionally, this change:
* simplifies the code that parses/caches binaries: it's now done
  in a single place, not three different functions.
* makes flush() method behave as expected, and actually clear
  the cached parsed binaries and objects.
* fixes a dangling pointer issue described in
  http://reviews.llvm.org/D15638

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@256041 91177308-0d34-0410-b5e6-96231b3b80d8

1 files changed, 45 insertions, 46 deletions

diff --git a/lib/DebugInfo/Symbolize/Symbolize.cpp b/lib/DebugInfo/Symbolize/Symbolize.cpp
index ba7d39cbdb3..3da1963bb79 100644
--- a/lib/DebugInfo/Symbolize/Symbolize.cpp
+++ b/lib/DebugInfo/Symbolize/Symbolize.cpp
@@ -22,6 +22,7 @@
 #include "llvm/DebugInfo/PDB/PDBContext.h"
 #include "llvm/Object/ELFObjectFile.h"
 #include "llvm/Object/MachO.h"
+#include "llvm/Object/MachOUniversal.h"
 #include "llvm/Support/COFF.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Compression.h"
@@ -109,9 +110,10 @@ ErrorOr<DIGlobal> LLVMSymbolizer::symbolizeData(const std::string &ModuleName,
 }
 
 void LLVMSymbolizer::flush() {
-  Modules.clear();
+  ObjectForUBPathAndArch.clear();
+  BinaryForPath.clear();
   ObjectPairForPathArch.clear();
-  ObjectFileForArch.clear();
+  Modules.clear();
 }
 
 // For Path="/path/to/foo" and Basename="foo" assume that debug info is in
@@ -223,23 +225,16 @@ ObjectFile *LLVMSymbolizer::lookUpDsymFile(const std::string &ExePath,
   for (const auto &Path : Opts.DsymHints) {
     DsymPaths.push_back(getDarwinDWARFResourceForPath(Path, Filename));
   }
-  for (const auto &path : DsymPaths) {
-    ErrorOr<OwningBinary<Binary>> BinaryOrErr = createBinary(path);
-    if (!BinaryOrErr)
-      continue;
-    OwningBinary<Binary> &B = BinaryOrErr.get();
-    auto DbgObjOrErr = getObjectFileFromBinary(B.getBinary(), ArchName);
+  for (const auto &Path : DsymPaths) {
+    auto DbgObjOrErr = getOrCreateObject(Path, ArchName);
     if (!DbgObjOrErr)
       continue;
     ObjectFile *DbgObj = DbgObjOrErr.get();
-    const MachOObjectFile *MachDbgObj =
-        dyn_cast<const MachOObjectFile>(DbgObj);
+    const MachOObjectFile *MachDbgObj = dyn_cast<const MachOObjectFile>(DbgObj);
     if (!MachDbgObj)
       continue;
-    if (darwinDsymMatchesBinary(MachDbgObj, MachExeObj)) {
-      addOwningBinary(std::move(B));
+    if (darwinDsymMatchesBinary(MachDbgObj, MachExeObj))
       return DbgObj;
-    }
   }
   return nullptr;
 }
@@ -254,40 +249,25 @@ ObjectFile *LLVMSymbolizer::lookUpDebuglinkObject(const std::string &Path,
     return nullptr;
   if (!findDebugBinary(Path, DebuglinkName, CRCHash, DebugBinaryPath))
     return nullptr;
-  ErrorOr<OwningBinary<Binary>> DebugBinaryOrErr =
-      createBinary(DebugBinaryPath);
-  if (!DebugBinaryOrErr)
-    return nullptr;
-  OwningBinary<Binary> &DB = DebugBinaryOrErr.get();
-  auto DbgObjOrErr = getObjectFileFromBinary(DB.getBinary(), ArchName);
+  auto DbgObjOrErr = getOrCreateObject(DebugBinaryPath, ArchName);
   if (!DbgObjOrErr)
     return nullptr;
-  addOwningBinary(std::move(DB));
   return DbgObjOrErr.get();
 }
 
 ErrorOr<LLVMSymbolizer::ObjectPair>
-LLVMSymbolizer::getOrCreateObjects(const std::string &Path,
-                                   const std::string &ArchName) {
+LLVMSymbolizer::getOrCreateObjectPair(const std::string &Path,
+                                      const std::string &ArchName) {
   const auto &I = ObjectPairForPathArch.find(std::make_pair(Path, ArchName));
   if (I != ObjectPairForPathArch.end())
     return I->second;
 
-  ErrorOr<OwningBinary<Binary>> BinaryOrErr = createBinary(Path);
-  if (auto EC = BinaryOrErr.getError()) {
-    ObjectPairForPathArch.insert(
-        std::make_pair(std::make_pair(Path, ArchName), EC));
-    return EC;
-  }
-  OwningBinary<Binary> &B = BinaryOrErr.get();
-
-  auto ObjOrErr = getObjectFileFromBinary(B.getBinary(), ArchName);
+  auto ObjOrErr = getOrCreateObject(Path, ArchName);
   if (auto EC = ObjOrErr.getError()) {
     ObjectPairForPathArch.insert(
         std::make_pair(std::make_pair(Path, ArchName), EC));
     return EC;
   }
-  addOwningBinary(std::move(B));
 
   ObjectFile *Obj = ObjOrErr.get();
   assert(Obj != nullptr);
@@ -306,24 +286,43 @@ LLVMSymbolizer::getOrCreateObjects(const std::string &Path,
 }
 
 ErrorOr<ObjectFile *>
-LLVMSymbolizer::getObjectFileFromBinary(Binary *Bin,
-                                        const std::string &ArchName) {
+LLVMSymbolizer::getOrCreateObject(const std::string &Path,
+                                  const std::string &ArchName) {
+  const auto &I = BinaryForPath.find(Path);
+  Binary *Bin = nullptr;
+  if (I == BinaryForPath.end()) {
+    ErrorOr<OwningBinary<Binary>> BinOrErr = createBinary(Path);
+    if (auto EC = BinOrErr.getError()) {
+      BinaryForPath.insert(std::make_pair(Path, EC));
+      return EC;
+    }
+    Bin = BinOrErr->getBinary();
+    BinaryForPath.insert(std::make_pair(Path, std::move(BinOrErr.get())));
+  } else if (auto EC = I->second.getError()) {
+    return EC;
+  } else {
+    Bin = I->second->getBinary();
+  }
+
   assert(Bin != nullptr);
+
   if (MachOUniversalBinary *UB = dyn_cast<MachOUniversalBinary>(Bin)) {
-    const auto &I = ObjectFileForArch.find(
-        std::make_pair(UB, ArchName));
-    if (I != ObjectFileForArch.end())
-      return I->second;
-    ErrorOr<std::unique_ptr<ObjectFile>> ParsedObj =
+    const auto &I = ObjectForUBPathAndArch.find(std::make_pair(Path, ArchName));
+    if (I != ObjectForUBPathAndArch.end()) {
+      if (auto EC = I->second.getError())
+        return EC;
+      return I->second->get();
+    }
+    ErrorOr<std::unique_ptr<ObjectFile>> ObjOrErr =
         UB->getObjectForArch(ArchName);
-    if (auto EC = ParsedObj.getError()) {
-      ObjectFileForArch.insert(
-          std::make_pair(std::make_pair(UB, ArchName), EC));
+    if (auto EC = ObjOrErr.getError()) {
+      ObjectForUBPathAndArch.insert(
+          std::make_pair(std::make_pair(Path, ArchName), EC));
       return EC;
     }
-    ObjectFile *Res = ParsedObj.get().get();
-    ParsedBinariesAndObjects.push_back(std::move(ParsedObj.get()));
-    ObjectFileForArch.insert(std::make_pair(std::make_pair(UB, ArchName), Res));
+    ObjectFile *Res = ObjOrErr->get();
+    ObjectForUBPathAndArch.insert(std::make_pair(std::make_pair(Path, ArchName),
+                                                 std::move(ObjOrErr.get())));
     return Res;
   }
   if (Bin->isObject()) {
@@ -352,7 +351,7 @@ LLVMSymbolizer::getOrCreateModuleInfo(const std::string &ModuleName) {
       ArchName = ArchStr;
     }
   }
-  auto ObjectsOrErr = getOrCreateObjects(BinaryName, ArchName);
+  auto ObjectsOrErr = getOrCreateObjectPair(BinaryName, ArchName);
   if (auto EC = ObjectsOrErr.getError()) {
     // Failed to find valid object file.
     Modules.insert(std::make_pair(ModuleName, EC));