From 2897e0306efe62703bad8e1234334fefccf2ae37 Mon Sep 17 00:00:00 2001 From: Zachary Turner Date: Thu, 25 May 2017 21:16:03 +0000 Subject: [PATCH] [lld] Fix a bug where we continually re-follow type servers. Originally this was intended to be set up so that when linking a PDB which refers to a type server, it would only visit the PDB once, and on subsequent visitations it would just skip it since all the records had already been added. Due to some C++ scoping issues, this was not occurring and it was revisiting the type server every time, which caused every record to end up being thrown away on all subsequent visitations. This doesn't affect the performance of linking clang-cl generated object files because we don't use type servers, but when linking object files and libraries generated with /Zi via MSVC, this means only 1 object file has to be linked instead of N object files, so the speedup is quite large. llvm-svn: 303920 --- lld/COFF/PDB.cpp | 11 ++++++----- llvm/include/llvm/DebugInfo/PDB/Native/PDBTypeServerHandler.h | 5 ++--- llvm/lib/DebugInfo/PDB/Native/PDBTypeServerHandler.cpp | 11 ++++++----- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/lld/COFF/PDB.cpp b/lld/COFF/PDB.cpp index a1b0291..a3b3ab7 100644 --- a/lld/COFF/PDB.cpp +++ b/lld/COFF/PDB.cpp @@ -99,6 +99,12 @@ static void addTypeInfo(pdb::TpiStreamBuilder &TpiBuilder, static void mergeDebugT(SymbolTable *Symtab, pdb::PDBFileBuilder &Builder, codeview::TypeTableBuilder &TypeTable, codeview::TypeTableBuilder &IDTable) { + // Follow type servers. If the same type server is encountered more than + // once for this instance of `PDBTypeServerHandler` (for example if many + // object files reference the same TypeServer), the types from the + // TypeServer will only be visited once. + pdb::PDBTypeServerHandler Handler; + // Visit all .debug$T sections to add them to Builder. for (ObjectFile *File : Symtab->ObjectFiles) { ArrayRef Data = getDebugSection(File, ".debug$T"); @@ -109,11 +115,6 @@ static void mergeDebugT(SymbolTable *Symtab, pdb::PDBFileBuilder &Builder, codeview::CVTypeArray Types; BinaryStreamReader Reader(Stream); SmallVector SourceToDest; - // Follow type servers. If the same type server is encountered more than - // once for this instance of `PDBTypeServerHandler` (for example if many - // object files reference the same TypeServer), the types from the - // TypeServer will only be visited once. - pdb::PDBTypeServerHandler Handler; Handler.addSearchPath(llvm::sys::path::parent_path(File->getName())); if (auto EC = Reader.readArray(Types, Reader.getLength())) fatal(EC, "Reader::readArray failed"); diff --git a/llvm/include/llvm/DebugInfo/PDB/Native/PDBTypeServerHandler.h b/llvm/include/llvm/DebugInfo/PDB/Native/PDBTypeServerHandler.h index bfd38b6..196ba4d 100644 --- a/llvm/include/llvm/DebugInfo/PDB/Native/PDBTypeServerHandler.h +++ b/llvm/include/llvm/DebugInfo/PDB/Native/PDBTypeServerHandler.h @@ -11,8 +11,7 @@ #define LLVM_DEBUGINFO_PDB_PDBTYPESERVERHANDLER_H #include "llvm/ADT/SmallString.h" -#include "llvm/ADT/SmallVector.h" -#include "llvm/ADT/StringMap.h" +#include "llvm/ADT/StringSet.h" #include "llvm/DebugInfo/CodeView/TypeRecord.h" #include "llvm/DebugInfo/CodeView/TypeServerHandler.h" #include "llvm/DebugInfo/PDB/Native/NativeSession.h" @@ -39,7 +38,7 @@ private: bool RevisitAlways; std::unique_ptr Session; - SmallVector, 4> SearchPaths; + StringSet<> SearchPaths; }; } } diff --git a/llvm/lib/DebugInfo/PDB/Native/PDBTypeServerHandler.cpp b/llvm/lib/DebugInfo/PDB/Native/PDBTypeServerHandler.cpp index 6ef9873..9fd9010 100644 --- a/llvm/lib/DebugInfo/PDB/Native/PDBTypeServerHandler.cpp +++ b/llvm/lib/DebugInfo/PDB/Native/PDBTypeServerHandler.cpp @@ -47,7 +47,7 @@ void PDBTypeServerHandler::addSearchPath(StringRef Path) { if (Path.empty() || !sys::fs::is_directory(Path)) return; - SearchPaths.push_back(Path); + SearchPaths.insert(Path); } Expected @@ -86,13 +86,14 @@ Expected PDBTypeServerHandler::handle(TypeServer2Record &TS, cv_error_code::corrupt_record, "TypeServer2Record does not contain filename!"); - for (auto Path : SearchPaths) { - sys::path::append(Path, File); - if (!sys::fs::exists(Path)) + for (auto &Path : SearchPaths) { + SmallString<64> PathStr = Path.getKey(); + sys::path::append(PathStr, File); + if (!sys::fs::exists(PathStr)) continue; std::unique_ptr ThisSession; - if (auto EC = loadDataForPDB(PDB_ReaderType::Native, Path, ThisSession)) { + if (auto EC = loadDataForPDB(PDB_ReaderType::Native, PathStr, ThisSession)) { // It is not an error if this PDB fails to load, it just means that it // doesn't match and we should continue searching. ignoreErrors(std::move(EC)); -- 2.7.4