From: Alexey Samsonov Date: Fri, 18 Dec 2015 22:02:14 +0000 (+0000) Subject: [Symbolize] Improve the ownership of parsed objects. X-Git-Url: http://plrg.eecs.uci.edu/git/?p=oota-llvm.git;a=commitdiff_plain;h=9dcc66e06e5d5a2830a93aad80d5fa1e8343927a;hp=a2d9f5fdfeb2d6810eefbd6339157da2b217de39 [Symbolize] Improve the ownership of parsed objects. This code changes the way Symbolize handles parsed binaries: now parsed OwningBinary 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 --- diff --git a/include/llvm/DebugInfo/Symbolize/Symbolize.h b/include/llvm/DebugInfo/Symbolize/Symbolize.h index 230dc759a97..ec3ae002659 100644 --- a/include/llvm/DebugInfo/Symbolize/Symbolize.h +++ b/include/llvm/DebugInfo/Symbolize/Symbolize.h @@ -13,13 +13,9 @@ #ifndef LLVM_DEBUGINFO_SYMBOLIZE_SYMBOLIZE_H #define LLVM_DEBUGINFO_SYMBOLIZE_SYMBOLIZE_H -#include "llvm/ADT/SmallVector.h" -#include "llvm/DebugInfo/DIContext.h" #include "llvm/DebugInfo/Symbolize/SymbolizableModule.h" -#include "llvm/Object/MachOUniversal.h" #include "llvm/Object/ObjectFile.h" #include "llvm/Support/ErrorOr.h" -#include "llvm/Support/MemoryBuffer.h" #include #include #include @@ -63,6 +59,8 @@ public: const SymbolizableModule *ModInfo); private: + // Bundles together object file with code/data and object file with + // corresponding debug info. These objects can be the same. typedef std::pair ObjectPair; ErrorOr @@ -75,30 +73,29 @@ private: const std::string &ArchName); /// \brief Returns pair of pointers to object and debug object. - ErrorOr getOrCreateObjects(const std::string &Path, - const std::string &ArchName); - /// \brief Returns a parsed object file for a given architecture in a - /// universal binary (or the binary itself if it is an object file). - ErrorOr getObjectFileFromBinary(Binary *Bin, - const std::string &ArchName); - - // Owns all the parsed binaries and object files. - SmallVector, 4> ParsedBinariesAndObjects; - SmallVector, 4> MemoryBuffers; - void addOwningBinary(OwningBinary OwningBin) { - std::unique_ptr Bin; - std::unique_ptr MemBuf; - std::tie(Bin, MemBuf) = OwningBin.takeBinary(); - ParsedBinariesAndObjects.push_back(std::move(Bin)); - MemoryBuffers.push_back(std::move(MemBuf)); - } + ErrorOr getOrCreateObjectPair(const std::string &Path, + const std::string &ArchName); + + /// \brief Return a pointer to object file at specified path, for a specified + /// architecture (e.g. if path refers to a Mach-O universal binary, only one + /// object file from it will be returned). + ErrorOr getOrCreateObject(const std::string &Path, + const std::string &ArchName); std::map>> Modules; - std::map, - ErrorOr> ObjectFileForArch; + + /// \brief Contains cached results of getOrCreateObjectPair(). std::map, ErrorOr> ObjectPairForPathArch; + /// \brief Contains parsed binary for each path, or parsing error. + std::map>> BinaryForPath; + + /// \brief Parsed object file for path/architecture pair, where "path" refers + /// to Mach-O universal binary. + std::map, ErrorOr>> + ObjectForUBPathAndArch; + Options Opts; }; 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 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> BinaryOrErr = createBinary(path); - if (!BinaryOrErr) - continue; - OwningBinary &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(DbgObj); + const MachOObjectFile *MachDbgObj = dyn_cast(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> DebugBinaryOrErr = - createBinary(DebugBinaryPath); - if (!DebugBinaryOrErr) - return nullptr; - OwningBinary &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::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> BinaryOrErr = createBinary(Path); - if (auto EC = BinaryOrErr.getError()) { - ObjectPairForPathArch.insert( - std::make_pair(std::make_pair(Path, ArchName), EC)); - return EC; - } - OwningBinary &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 -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> 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(Bin)) { - const auto &I = ObjectFileForArch.find( - std::make_pair(UB, ArchName)); - if (I != ObjectFileForArch.end()) - return I->second; - ErrorOr> 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> 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));