diff --git a/llvm/include/llvm/DWARFLinker/DWARFLinker.h b/llvm/include/llvm/DWARFLinker/DWARFLinker.h --- a/llvm/include/llvm/DWARFLinker/DWARFLinker.h +++ b/llvm/include/llvm/DWARFLinker/DWARFLinker.h @@ -467,7 +467,6 @@ bool registerModuleReference(DWARFDie CUDie, const DWARFUnit &Unit, const DWARFFile &File, OffsetsStringPool &OffsetsStringPool, - UniquingStringPool &UniquingStringPoolStringPool, DeclContextTree &ODRContexts, uint64_t ModulesEndOffset, unsigned &UnitID, bool IsLittleEndian, unsigned Indent = 0, @@ -480,7 +479,6 @@ StringRef ModuleName, uint64_t DwoId, const DWARFFile &File, OffsetsStringPool &OffsetsStringPool, - UniquingStringPool &UniquingStringPool, DeclContextTree &ODRContexts, uint64_t ModulesEndOffset, unsigned &UnitID, bool IsLittleEndian, unsigned Indent = 0, bool Quiet = false); diff --git a/llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h b/llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h --- a/llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h +++ b/llvm/include/llvm/DWARFLinker/DWARFLinkerCompileUnit.h @@ -11,6 +11,7 @@ #include "llvm/ADT/IntervalMap.h" #include "llvm/CodeGen/DIE.h" +#include "llvm/CodeGen/NonRelocatableStringpool.h" #include "llvm/DebugInfo/DWARF/DWARFUnit.h" #include "llvm/Support/DataExtractor.h" @@ -237,21 +238,6 @@ const std::vector &getNamespaces() const { return Namespaces; } const std::vector &getObjC() const { return ObjC; } - /// Get the full path for file \a FileNum in the line table - StringRef getResolvedPath(unsigned FileNum) { - if (FileNum >= ResolvedPaths.size()) - return StringRef(); - return ResolvedPaths[FileNum]; - } - - /// Set the fully resolved path for the line-table's file \a FileNum - /// to \a Path. - void setResolvedPath(unsigned FileNum, StringRef Path) { - if (ResolvedPaths.size() <= FileNum) - ResolvedPaths.resize(FileNum + 1); - ResolvedPaths[FileNum] = Path; - } - MCSymbol *getLabelBegin() { return LabelBegin; } void setLabelBegin(MCSymbol *S) { LabelBegin = S; } @@ -310,12 +296,6 @@ std::vector ObjC; /// @} - /// Cached resolved paths from the line table. - /// Note, the StringRefs here point in to the intern (uniquing) string pool. - /// This means that a StringRef returned here doesn't need to then be uniqued - /// for the purposes of getting a unique address for each string. - std::vector ResolvedPaths; - /// Is this unit subject to the ODR rule? bool HasODR; diff --git a/llvm/include/llvm/DWARFLinker/DWARFLinkerDeclContext.h b/llvm/include/llvm/DWARFLinker/DWARFLinkerDeclContext.h --- a/llvm/include/llvm/DWARFLinker/DWARFLinkerDeclContext.h +++ b/llvm/include/llvm/DWARFLinker/DWARFLinkerDeclContext.h @@ -15,6 +15,7 @@ #include "llvm/ADT/StringRef.h" #include "llvm/CodeGen/NonRelocatableStringpool.h" #include "llvm/DWARFLinker/DWARFLinkerCompileUnit.h" +#include "llvm/DebugInfo/DWARF/DWARFDebugLine.h" #include "llvm/DebugInfo/DWARF/DWARFDie.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Path.h" @@ -31,16 +32,18 @@ public: /// Resolve a path by calling realpath and cache its result. The returned /// StringRef is interned in the given \p StringPool. - StringRef resolve(std::string Path, NonRelocatableStringpool &StringPool) { + StringRef resolve(const std::string &Path, + NonRelocatableStringpool &StringPool) { StringRef FileName = sys::path::filename(Path); - SmallString<256> ParentPath = sys::path::parent_path(Path); + StringRef ParentPath = sys::path::parent_path(Path); // If the ParentPath has not yet been resolved, resolve and cache it for // future look-ups. if (!ResolvedPaths.count(ParentPath)) { SmallString<256> RealPath; sys::fs::real_path(ParentPath, RealPath); - ResolvedPaths.insert({ParentPath, StringRef(RealPath).str()}); + ResolvedPaths.insert( + {ParentPath, std::string(RealPath.c_str(), RealPath.size())}); } // Join the file name again with the resolved path. @@ -95,7 +98,6 @@ void setDefinedInClangModule(bool Val) { DefinedInClangModule = Val; } uint16_t getTag() const { return Tag; } - StringRef getName() const { return Name; } private: friend DeclMapInfo; @@ -129,10 +131,10 @@ /// /// FIXME: The invalid bit along the return value is to emulate some /// dsymutil-classic functionality. - PointerIntPair - getChildDeclContext(DeclContext &Context, const DWARFDie &DIE, - CompileUnit &Unit, UniquingStringPool &StringPool, - bool InClangModule); + PointerIntPair getChildDeclContext(DeclContext &Context, + const DWARFDie &DIE, + CompileUnit &Unit, + bool InClangModule); DeclContext &getRoot() { return Root; } @@ -141,8 +143,40 @@ DeclContext Root; DeclContext::Map Contexts; - /// Cache resolved paths from the line table. + /// Cached resolved paths from the line table. + /// The key is . + using ResolvedPathsMap = DenseMap, StringRef>; + ResolvedPathsMap ResolvedPaths; + + /// Helper that resolves and caches fragments of file paths. CachedPathResolver PathResolver; + + /// String pool keeping real path bodies. + NonRelocatableStringpool StringPool; + + StringRef getResolvedPath(CompileUnit &CU, unsigned FileNum, + const DWARFDebugLine::LineTable &LineTable) { + std::pair Key = {CU.getUniqueID(), FileNum}; + + ResolvedPathsMap::const_iterator it = ResolvedPaths.find(Key); + + if (it == ResolvedPaths.end()) { + std::string FileName; + bool FoundFileName = LineTable.getFileNameByIndex( + FileNum, CU.getOrigUnit().getCompilationDir(), + DILineInfoSpecifier::FileLineInfoKind::AbsoluteFilePath, FileName); + (void)FoundFileName; + assert(FoundFileName && "Must get file name from line table"); + + // Second level of caching, this time based on the file's parent + // path. + StringRef ResolvedPath = PathResolver.resolve(FileName, StringPool); + + it = ResolvedPaths.insert(std::make_pair(Key, ResolvedPath)).first; + } + + return it->second; + } }; /// Info type for the DenseMap storing the DeclContext pointers. diff --git a/llvm/lib/DWARFLinker/DWARFLinker.cpp b/llvm/lib/DWARFLinker/DWARFLinker.cpp --- a/llvm/lib/DWARFLinker/DWARFLinker.cpp +++ b/llvm/lib/DWARFLinker/DWARFLinker.cpp @@ -313,9 +313,8 @@ /// (i.e., forward declarations that are children of a DW_TAG_module). static bool analyzeContextInfo( const DWARFDie &DIE, unsigned ParentIdx, CompileUnit &CU, - DeclContext *CurrentDeclContext, UniquingStringPool &StringPool, - DeclContextTree &Contexts, uint64_t ModulesEndOffset, - swiftInterfacesMap *ParseableSwiftInterfaces, + DeclContext *CurrentDeclContext, DeclContextTree &Contexts, + uint64_t ModulesEndOffset, swiftInterfacesMap *ParseableSwiftInterfaces, std::function ReportWarning, bool InImportedModule = false) { // LIFO work list. @@ -366,7 +365,7 @@ if (CU.hasODR() || InClangModule) { if (Current.Context) { auto PtrInvalidPair = Contexts.getChildDeclContext( - *Current.Context, Current.Die, CU, StringPool, InClangModule); + *Current.Context, Current.Die, CU, InClangModule); Current.Context = PtrInvalidPair.getPointer(); Info.Ctxt = PtrInvalidPair.getInt() ? nullptr : PtrInvalidPair.getPointer(); @@ -1977,11 +1976,13 @@ return p.str().str(); } -bool DWARFLinker::registerModuleReference( - DWARFDie CUDie, const DWARFUnit &Unit, const DWARFFile &File, - OffsetsStringPool &StringPool, UniquingStringPool &UniquingStringPool, - DeclContextTree &ODRContexts, uint64_t ModulesEndOffset, unsigned &UnitID, - bool IsLittleEndian, unsigned Indent, bool Quiet) { +bool DWARFLinker::registerModuleReference(DWARFDie CUDie, const DWARFUnit &Unit, + const DWARFFile &File, + OffsetsStringPool &StringPool, + DeclContextTree &ODRContexts, + uint64_t ModulesEndOffset, + unsigned &UnitID, bool IsLittleEndian, + unsigned Indent, bool Quiet) { std::string PCMfile = dwarf::toString( CUDie.find({dwarf::DW_AT_dwo_name, dwarf::DW_AT_GNU_dwo_name}), ""); if (PCMfile.empty()) @@ -2025,10 +2026,9 @@ // shouldn't run into an infinite loop, so mark it as processed now. ClangModules.insert({PCMfile, DwoId}); - if (Error E = - loadClangModule(CUDie, PCMfile, Name, DwoId, File, StringPool, - UniquingStringPool, ODRContexts, ModulesEndOffset, - UnitID, IsLittleEndian, Indent + 2, Quiet)) { + if (Error E = loadClangModule(CUDie, PCMfile, Name, DwoId, File, StringPool, + ODRContexts, ModulesEndOffset, UnitID, + IsLittleEndian, Indent + 2, Quiet)) { consumeError(std::move(E)); return false; } @@ -2038,9 +2038,8 @@ Error DWARFLinker::loadClangModule( DWARFDie CUDie, StringRef Filename, StringRef ModuleName, uint64_t DwoId, const DWARFFile &File, OffsetsStringPool &StringPool, - UniquingStringPool &UniquingStringPool, DeclContextTree &ODRContexts, - uint64_t ModulesEndOffset, unsigned &UnitID, bool IsLittleEndian, - unsigned Indent, bool Quiet) { + DeclContextTree &ODRContexts, uint64_t ModulesEndOffset, unsigned &UnitID, + bool IsLittleEndian, unsigned Indent, bool Quiet) { /// Using a SmallString<0> because loadClangModule() is recursive. SmallString<0> Path(Options.PrependPath); if (sys::path::is_relative(Filename)) @@ -2064,9 +2063,9 @@ auto CUDie = CU->getUnitDIE(false); if (!CUDie) continue; - if (!registerModuleReference( - CUDie, *CU, File, StringPool, UniquingStringPool, ODRContexts, - ModulesEndOffset, UnitID, IsLittleEndian, Indent, Quiet)) { + if (!registerModuleReference(CUDie, *CU, File, StringPool, ODRContexts, + ModulesEndOffset, UnitID, IsLittleEndian, + Indent, Quiet)) { if (Unit) { std::string Err = (Filename + @@ -2094,9 +2093,8 @@ Unit = std::make_unique(*CU, UnitID++, !Options.NoODR, ModuleName); Unit->setHasInterestingContent(); - analyzeContextInfo(CUDie, 0, *Unit, &ODRContexts.getRoot(), - UniquingStringPool, ODRContexts, ModulesEndOffset, - Options.ParseableSwiftInterfaces, + analyzeContextInfo(CUDie, 0, *Unit, &ODRContexts.getRoot(), ODRContexts, + ModulesEndOffset, Options.ParseableSwiftInterfaces, [&](const Twine &Warning, const DWARFDie &DIE) { reportWarning(Warning, File, &DIE); }); @@ -2310,10 +2308,6 @@ // parallel loop. unsigned NumObjects = ObjectContexts.size(); - // This Dwarf string pool which is only used for uniquing. This one should - // never be used for offsets as its not thread-safe or predictable. - UniquingStringPool UniquingStringPool(nullptr, true); - // This Dwarf string pool which is used for emission. It must be used // serially as the order of calling getStringOffset matters for // reproducibility. @@ -2383,7 +2377,7 @@ } if (CUDie && !LLVM_UNLIKELY(Options.Update)) registerModuleReference(CUDie, *CU, OptContext.File, OffsetsStringPool, - UniquingStringPool, ODRContexts, 0, UnitID, + ODRContexts, 0, UnitID, OptContext.File.Dwarf->isLittleEndian()); } } @@ -2425,8 +2419,8 @@ auto CUDie = CU->getUnitDIE(false); if (!CUDie || LLVM_UNLIKELY(Options.Update) || !registerModuleReference(CUDie, *CU, Context.File, OffsetsStringPool, - UniquingStringPool, ODRContexts, - ModulesEndOffset, UnitID, Quiet)) { + ODRContexts, ModulesEndOffset, UnitID, + Quiet)) { Context.CompileUnits.push_back(std::make_unique( *CU, UnitID++, !Options.NoODR && !Options.Update, "")); } @@ -2438,9 +2432,8 @@ if (!CUDie) continue; analyzeContextInfo(CurrentUnit->getOrigUnit().getUnitDIE(), 0, - *CurrentUnit, &ODRContexts.getRoot(), - UniquingStringPool, ODRContexts, ModulesEndOffset, - Options.ParseableSwiftInterfaces, + *CurrentUnit, &ODRContexts.getRoot(), ODRContexts, + ModulesEndOffset, Options.ParseableSwiftInterfaces, [&](const Twine &Warning, const DWARFDie &DIE) { reportWarning(Warning, Context.File, &DIE); }); diff --git a/llvm/lib/DWARFLinker/DWARFLinkerDeclContext.cpp b/llvm/lib/DWARFLinker/DWARFLinkerDeclContext.cpp --- a/llvm/lib/DWARFLinker/DWARFLinkerDeclContext.cpp +++ b/llvm/lib/DWARFLinker/DWARFLinkerDeclContext.cpp @@ -40,9 +40,9 @@ return true; } -PointerIntPair DeclContextTree::getChildDeclContext( - DeclContext &Context, const DWARFDie &DIE, CompileUnit &U, - UniquingStringPool &StringPool, bool InClangModule) { +PointerIntPair +DeclContextTree::getChildDeclContext(DeclContext &Context, const DWARFDie &DIE, + CompileUnit &U, bool InClangModule) { unsigned Tag = DIE.getTag(); // FIXME: dsymutil-classic compat: We should bail out here if we @@ -80,27 +80,22 @@ break; } - const char *Name = DIE.getLinkageName(); - const char *ShortName = DIE.getShortName(); - - if (!Name) - Name = ShortName; - StringRef NameRef; - StringRef ShortNameRef; StringRef FileRef; - if (Name) - NameRef = StringPool.internString(Name); - else if (Tag == dwarf::DW_TAG_namespace) + if (const char *LinkageName = DIE.getLinkageName()) + NameRef = StringPool.internString(LinkageName); + else if (const char *ShortName = DIE.getShortName()) + NameRef = StringPool.internString(ShortName); + + bool IsAnonimousNamespace = false; + + if (NameRef.empty() && Tag == dwarf::DW_TAG_namespace) { // FIXME: For dsymutil-classic compatibility. I think uniquing within // anonymous namespaces is wrong. There is no ODR guarantee there. - NameRef = StringPool.internString("(anonymous namespace)"); - - if (ShortName && ShortName != Name) - ShortNameRef = StringPool.internString(ShortName); - else - ShortNameRef = NameRef; + NameRef = "(anonymous namespace)"; + IsAnonimousNamespace = true; + } if (Tag != dwarf::DW_TAG_class_type && Tag != dwarf::DW_TAG_structure_type && Tag != dwarf::DW_TAG_union_type && @@ -121,7 +116,7 @@ // module-defined types do not have a file and line. ByteSize = dwarf::toUnsigned(DIE.find(dwarf::DW_AT_byte_size), std::numeric_limits::max()); - if (Tag != dwarf::DW_TAG_namespace || !Name) { + if (Tag != dwarf::DW_TAG_namespace || IsAnonimousNamespace) { if (unsigned FileNum = dwarf::toUnsigned(DIE.find(dwarf::DW_AT_decl_file), 0)) { if (const auto *LT = U.getOrigUnit().getContext().getLineTableForUnit( @@ -129,29 +124,14 @@ // FIXME: dsymutil-classic compatibility. I'd rather not // unique anything in anonymous namespaces, but if we do, then // verify that the file and line correspond. - if (!Name && Tag == dwarf::DW_TAG_namespace) + if (IsAnonimousNamespace) FileNum = 1; if (LT->hasFileAtIndex(FileNum)) { Line = dwarf::toUnsigned(DIE.find(dwarf::DW_AT_decl_line), 0); // Cache the resolved paths based on the index in the line table, - // because calling realpath is expansive. - StringRef ResolvedPath = U.getResolvedPath(FileNum); - if (!ResolvedPath.empty()) { - FileRef = ResolvedPath; - } else { - std::string File; - bool FoundFileName = LT->getFileNameByIndex( - FileNum, U.getOrigUnit().getCompilationDir(), - DILineInfoSpecifier::FileLineInfoKind::AbsoluteFilePath, - File); - (void)FoundFileName; - assert(FoundFileName && "Must get file name from line table"); - // Second level of caching, this time based on the file's parent - // path. - FileRef = PathResolver.resolve(File, StringPool); - U.setResolvedPath(FileNum, FileRef); - } + // because calling realpath is expensive. + FileRef = getResolvedPath(U, FileNum, *LT); } } } @@ -175,7 +155,7 @@ // FIXME: dsymutil-classic compatibility: when we don't have a name, // use the filename. - if (Tag == dwarf::DW_TAG_namespace && NameRef == "(anonymous namespace)") + if (IsAnonimousNamespace) Hash = hash_combine(Hash, FileRef); // Now look if this context already exists.