[LLD][COFF] Early load PDB type server files

[aganea]

Summary

In order to have all inputs loaded & available upfront when starting to merge types, we need to open PDB type server files early in the process.

PDB type servers are created when using MSVC /Zi. In that case, the debug info for a given project is stored in a single PDB type server file. All OBJ files in that project will refer to their corresponding PDB through a TypeServer2Record.

This patch also fixes a tiny issue where the PDB "Age" wasn't validated against the OBJ's TypeServer2Record.Age. The age is a concept used by MSVC for incremental compilation: the PDB's Age is incremented by MSVC on each incremental build, but the GUID remains the same.

This patch is step 2. in "Proposed commit strategy" in D59226

[aganea]

Ping! I know Reid is OOO this week and Zachary might not be available for reviewing. @ruiu could you please take a quick look at this? Thank you in advance for your time.

[takuto.ikuta]

COFF/InputFiles.cpp

44

better to not have llvm:: prefix for type in this code when we have this?

722

const ObjFile *File ?

745

can be const ObjFile * here too?

758

same here?

[ruiu]

InputFile abstracts the concept of a file containing symbols. In each parse() function, we parse a symbol table and then insert symbols to the symbol table. parse() is part of a symbol resolution process step.

Since PDBInputFile doesn't contain any symbols, it doesn't fit to that concept, and it doesn't seem you need to parse them while we parse other types of input files.

So, why don't you just add a pdb file to some global list, and after resolving all symbols, parse them in a simple for loop? I guess that's conceptually much simpler.

[rnk]

In D60095#1471063, @ruiu wrote:

InputFile abstracts the concept of a file containing symbols. In each parse() function, we parse a symbol table and then insert symbols to the symbol table. parse() is part of a symbol resolution process step.

Since PDBInputFile doesn't contain any symbols, it doesn't fit to that concept, and it doesn't seem you need to parse them while we parse other types of input files.

So, why don't you just add a pdb file to some global list, and after resolving all symbols, parse them in a simple for loop? I guess that's conceptually much simpler.

IIUC, the point of this change is to start loading types in parallel as early as possible. The suggestion to accumulate the inputs on the side and wait until after symbol resolution won't exploit all the parallelism that's available.

[aganea]

In D60095#1472111, @rnk wrote:

In D60095#1471063, @ruiu wrote:

InputFile abstracts the concept of a file containing symbols. In each parse() function, we parse a symbol table and then insert symbols to the symbol table. parse() is part of a symbol resolution process step.

Since PDBInputFile doesn't contain any symbols, it doesn't fit to that concept, and it doesn't seem you need to parse them while we parse other types of input files.

So, why don't you just add a pdb file to some global list, and after resolving all symbols, parse them in a simple for loop? I guess that's conceptually much simpler.

IIUC, the point of this change is to start loading types in parallel as early as possible. The suggestion to accumulate the inputs on the side and wait until after symbol resolution won't exploit all the parallelism that's available.

As long as this goes in the LinkerDriver's TaskQueue, it's fine, no? I could simply move PDBInputFile out of the InputFile hierarchy, I think that's what @ruiu was proposing?
It wasn't my goal to parallelize the TaskQueue just yet, rather introduce the ghash parallelism first, then parallelize the Type merging.

[aganea]

I simplfied the patch by moving the PDBInputFile behavior into the existing TypeServerSource class. This makes things more elegant and keeps everything related to debug types into one place.

Tested (check-lld) on three different configurations:

1. MSBuild + MSVC 15.9.11 with LLVM_ENABLE_ASSERTIONS on, in a Debug target, under Windows 10 1803.
2. Ninja + Clang 8.0 in a Release target, under Windows 10 1803.
3. Ninja + GCC 7.3 in a Release target, under WSL/Ubuntu 18.04.

[aganea]

Ping! Any further suggestions? Thanks in advance for your time.

[aganea]

Ping!

[ruiu]

Overall looking good.

COFF/DebugTypes.cpp

23–26

If a constructor can fail, one pattern to handle that is to make the constructor private and instead provide static ErrorOr<TypeServerSource> getInstance() function. In that function, you can load a PDB and then instantiate TypeServerSouce only when the file loading succeeded. If failed, you can return an error.

100–101

nit: return Path + sys::path::...

[aganea]

COFF/DebugTypes.cpp

23–26

As suggested, added static Expected<TypeServerSource *> TypeServerSource::getInstance() and replaced previous lld::coff::makeTypeServerSource() by lld::coff::loadTypeServerSource() to be consistent.

Note - PDB load errors are simply queued in TypeServerSource::Instance, and displayed later by TypeServerSource::findFromFile() which is called by PDBLinker::maybeMergeTypeServerPDB().
I've tried displaying the errors on the spot, but at that point we don't know who requested loading of that PDB, and it feels wrong to decorelate the PDB errors from the requesting OBJ:

lld-link: warning: 'bad-block-size.pdb': The PDB file is corrupt. MSF superblock is missing
...
... (various other warnings)
...
lld-link: warning: Cannot use debug info for 'F:\svn\build\tools\lld\test\COFF\Output\pdb-type-server-native-errors.yaml.tmp.obj' [LNK4099]
>>> failed to load reference 'bad-block-size.pdb': (see above)

Given it's not a fatal error, I prefer the current behavior, it feels better from a user perspective:

ld-link: warning: Cannot use debug info for 'F:\svn\build\tools\lld\test\COFF\Output\pdb-type-server-native-errors.yaml.tmp.obj' [LNK4099]
>>> failed to load reference 'bad-block-size.pdb': The PDB file is corrupt. MSF superblock is missing

[aganea]

Ping @ruiu

[ruiu]

LGTM

[aganea]

Thank you!

[stella.stamenova]

This change broke one of the tests on the Windows LLDB bot:

http://lab.llvm.org:8011/builders/lldb-x64-windows-ninja/builds/5209

Could you please have a look and revert it if you don't have a quick fix?

[aganea]

Looking now.

[aganea]

@stella.stamenova : I'll have to revert it, I'm not sure what's going on. When I run the tests on my Windows PC, many tests are tagged as 'unsupported', I don't know why:

Unsupported Tests  : 796

Whereas on your build bot, there are no 'unsupported' tests:

Expected Passes    : 1596
Expected Failures  : 11
Unsupported Tests  : 46

If I try to manually run the test that fails on the bot: C:\Users\aganea\AppData\Local\Programs\Python\Python37-32\python.exe F:/svn/buildninja/./bin/llvm-lit.py -sv -vv -a F:\svn\lldb\lit\tools\lldb-mi\exec\exec-finish.test, I also get:

UNSUPPORTED: LLDB :: tools/lldb-mi/exec/exec-finish.test (1 of 1)
Test is unsupported

If I run the commands in the test manually, I get to a point where lldb-mi doesn't want to run:

F:\svn\buildninja\tools\lldb\lit>f:\svn\buildninja\bin\lldb-mi.exe
MI: Error: Driver. LLDB Debugger.
MI: Error: Driver Manager. Driver 'Machine Interface Driver Version: 1.0.0.9' (ID:'MIDriver') initialise failed. Driver. LLDB Debugger.

Any suggestions? I'll revert the patch in the meanwhile.

[stella.stamenova]

I am not very familiar with the lldb-mi tests specifically, but it looks like they require python to run (and are marked as unsupported otherwise):

# lldb-mi always fails without Python support
if lldbmi.was_resolved and not config.lldb_disable_python:
    config.available_features.add('lldb-mi')

My guess is that the difference comes from the options you passed to cmake at configuration time. This is what the Buildbot does:

cmake -G Ninja ../llvm -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=../install -DPYTHON_HOME="C:\Program Files (x86)\Microsoft Visual Studio\Shared\Python36_64" -DLLVM_ENABLE_ASSERTIONS=OFF

On Windows, I've noticed that if we don't correctly configure PYTHON_HOME, there are a variety of issues that pop up.

In D60095#1519986, @aganea wrote:

@stella.stamenova : I'll have to revert it, I'm not sure what's going on. When I run the tests on my Windows PC, many tests are tagged as 'unsupported', I don't know why:

Unsupported Tests  : 796

Whereas on your build bot, there are no 'unsupported' tests:

Expected Passes    : 1596
Expected Failures  : 11
Unsupported Tests  : 46

If I try to manually run the test that fails on the bot: C:\Users\aganea\AppData\Local\Programs\Python\Python37-32\python.exe F:/svn/buildninja/./bin/llvm-lit.py -sv -vv -a F:\svn\lldb\lit\tools\lldb-mi\exec\exec-finish.test, I also get:

UNSUPPORTED: LLDB :: tools/lldb-mi/exec/exec-finish.test (1 of 1)
Test is unsupported

If I run the commands in the test manually, I get to a point where lldb-mi doesn't want to run:

F:\svn\buildninja\tools\lldb\lit>f:\svn\buildninja\bin\lldb-mi.exe
MI: Error: Driver. LLDB Debugger.
MI: Error: Driver Manager. Driver 'Machine Interface Driver Version: 1.0.0.9' (ID:'MIDriver') initialise failed. Driver. LLDB Debugger.

Any suggestions? I'll revert the patch in the meanwhile.

[aganea]

Just for the record, the lldb tests issues above were caused first by a missing SWIG installation on my PC, then by using wrong SWIG version (4 instead if 3), and then my locale being fr_CA (which still causes a few tests to fail).

I've added a test in LLD to cover the issue raised by LLDB :: tools/lldb-mi/exec/exec-finish.test. See lld/trunk/test/COFF/pdb-type-server-invalid-signature.yaml L20.

Relanded as rL362393.

[rnk]

I'm seeing a lot of LNK4099 warnings after this change, and Chromium links with -Werror. I also saw a fair number while doing some local testing with asan. I'll collect some more information, but I wanted to give a heads up that this might be problematic.

[aganea]

In D60095#1528125, @rnk wrote:

I'm seeing a lot of LNK4099 warnings after this change, and Chromium links with -Werror. I also saw a fair number while doing some local testing with asan. I'll collect some more information, but I wanted to give a heads up that this might be problematic.

I can take a look asap if you can provide a complete warning, along with the incriminated OBJ path and corresponding PDB?

[aganea]

In D60095#1528125, @rnk wrote:

I'm seeing a lot of LNK4099 warnings after this change, and Chromium links with -Werror. I also saw a fair number while doing some local testing with asan. I'll collect some more information, but I wanted to give a heads up that this might be problematic.

How do you test locally with asan? ninja check-asan?

[aganea]

You're right, there's indeed an issue, checking now.

[rnk]

Looks like I wrote the following comment, but didn't submit it yesterday. Maybe I felt it didn't have enough information. Anyway, I see the proposed fix, I'll try it out.

In D60095#1528125, @rnk wrote:

I'm seeing a lot of LNK4099 warnings after this change, and Chromium links with -Werror. I also saw a fair number while doing some local testing with asan. I'll collect some more information, but I wanted to give a heads up that this might be problematic.

This happens when linking hello world:

$ clang-cl -c t.cpp && lld-link t.obj -debug
lld-link: warning: Cannot use debug info for 'libcmt.lib(gs_cookie.obj)' [LNK4099]
>>> failed to load reference 'd:\agent\_work\1\s\binaries\amd64ret\lib\amd64\libcmt.amd64.pdb': The signature does not match; the file(s) might be out of date.

lld-link: warning: Cannot use debug info for 'libcmt.lib(gs_report.obj)' [LNK4099]
>>> failed to load reference 'd:\agent\_work\1\s\binaries\amd64ret\lib\amd64\libcmt.amd64.pdb': The signature does not match; the file(s) might be out of date.

lld-link: warning: Cannot use debug info for 'libcmt.lib(loadcfg.obj)' [LNK4099]
>>> failed to load reference 'd:\agent\_work\1\s\binaries\amd64ret\lib\amd64\libcmt.amd64.pdb': The signature does not match; the file(s) might be out of date.
...

[aganea]

This comment was removed by aganea.

COFF/DebugTypes.h

 class TypeServer2Record;
 } // namespace codeview
 } // namespace llvm
 
 namespace lld {
 namespace coff {
 
 class ObjFile;
+class PDBInputFile;
 
 class TpiSource {
 public:
   enum TpiKind { Regular, PCH, UsingPCH, PDB, UsingPDB };
 
   TpiSource(TpiKind K, ObjFile *F);
   virtual ~TpiSource() {}
 
   const TpiKind Kind;
   ObjFile *File;
 };
 
 TpiSource *makeTpiSource(ObjFile *F);
-TpiSource *makeTypeServerSource(ObjFile *F);
+TpiSource *makeTypeServerSource(PDBInputFile *F);
 TpiSource *makeUseTypeServerSource(ObjFile *F,
                                    llvm::codeview::TypeServer2Record *TS);
 TpiSource *makePrecompSource(ObjFile *F);
 TpiSource *makeUsePrecompSource(ObjFile *F,
                                 llvm::codeview::PrecompRecord *Precomp);
 
 // Temporary interface to get the dependency
 template <typename T> const T &retrieveDependencyInfo(TpiSource *Source);
 
 } // namespace coff
 } // namespace lld
 
 #endif
 No newline at end of file

COFF/DebugTypes.cpp

 //===- DebugTypes.cpp -----------------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "DebugTypes.h"
 #include "InputFiles.h"
 #include "llvm/DebugInfo/CodeView/TypeRecord.h"
+#include "llvm/DebugInfo/PDB/Native/PDBFile.h"
+#include "llvm/DebugInfo/PDB/Native/InfoStream.h"
 
 using namespace lld;
 using namespace lld::coff;
 using namespace llvm;
 using namespace llvm::codeview;
 
 namespace {
 class TypeServerSource : public TpiSource {
 public:
-  TypeServerSource(ObjFile *F) : TpiSource(PDB, F) {}
+  TypeServerSource(PDBInputFile *F) : TpiSource(PDB, nullptr), PDBFile(F) {}
+
+  PDBInputFile *PDBFile;
 };
 
 class UseTypeServerSource : public TpiSource {
 public:
   UseTypeServerSource(ObjFile *F, TypeServer2Record *TS)
       : TpiSource(UsingPDB, F), TypeServerDependency(*TS) {}
 
   // Information about the PDB type server dependency, that needs to be loaded
   // in before merging this OBJ.

 TpiSource::TpiSource(TpiKind K, ObjFile *F) : Kind(K), File(F) {
   GC.push_back(std::unique_ptr<TpiSource>(this));
 }
 
 TpiSource *coff::makeTpiSource(ObjFile *F) {
   return new TpiSource(TpiSource::Regular, F);
 }
 
-TpiSource *coff::makeTypeServerSource(ObjFile *F) {
+TpiSource *coff::makeTypeServerSource(PDBInputFile *F) {
   return new TypeServerSource(F);
 }
 
 TpiSource *coff::makeUseTypeServerSource(ObjFile *F, TypeServer2Record *TS) {
   return new UseTypeServerSource(F, TS);
 }
 
 TpiSource *coff::makePrecompSource(ObjFile *F) { return new PrecompSource(F); }
 
 TpiSource *coff::makeUsePrecompSource(ObjFile *F, PrecompRecord *Precomp) {
   return new UsePrecompSource(F, Precomp);
 }
 
 template <>
 const PrecompRecord &lld::coff::retrieveDependencyInfo(TpiSource *Source) {
   assert(Source->Kind == TpiSource::UsingPCH);
   return ((UsePrecompSource *)Source)->PrecompDependency;
 }
 
 template <>
 const TypeServer2Record &lld::coff::retrieveDependencyInfo(TpiSource *Source) {
   assert(Source->Kind == TpiSource::UsingPDB);
   return ((UseTypeServerSource *)Source)->TypeServerDependency;
 }
 No newline at end of file

COFF/Driver.h

public:
   void parseDirectives(InputFile *File);
 
   // Used by ArchiveFile to enqueue members.
   void enqueueArchiveMember(const Archive::Child &C, StringRef SymName,
                             StringRef ParentName);
 
   MemoryBufferRef takeBuffer(std::unique_ptr<MemoryBuffer> MB);
 
+  void enqueuePath(StringRef Path, bool WholeArchive);
+
 private:
   std::unique_ptr<llvm::TarWriter> Tar; // for /linkrepro
 
   // Opens a file. Path has to be resolved already.
   MemoryBufferRef openFile(StringRef Path);
 
   // Searches a file from search paths.
   Optional<StringRef> findFile(StringRef Filename);
private:
   // entry point name.
   StringRef findDefaultEntry();
   WindowsSubsystem inferSubsystem();
 
   void addBuffer(std::unique_ptr<MemoryBuffer> MB, bool WholeArchive);
   void addArchiveBuffer(MemoryBufferRef MBRef, StringRef SymName,
                         StringRef ParentName);
 
-  void enqueuePath(StringRef Path, bool WholeArchive);
-
   void enqueueTask(std::function<void()> Task);
   bool run();
 
   std::list<std::function<void()>> TaskQueue;
   std::vector<StringRef> FilePaths;
   std::vector<MemoryBufferRef> Resources;
 
   llvm::StringSet<> DirectivesExports;

COFF/Driver.cpp

case file_magic::archive:
     break;
   case file_magic::bitcode:
     Symtab->addFile(make<BitcodeFile>(MBRef));
     break;
   case file_magic::coff_object:
   case file_magic::coff_import_library:
     Symtab->addFile(make<ObjFile>(MBRef));
     break;
+  case file_magic::pdb:
+    Symtab->addFile(make<PDBInputFile>(MBRef));
+    break;
   case file_magic::coff_cl_gl_object:
     error(Filename + ": is not a native COFF file. Recompile without /GL");
     break;
   case file_magic::pecoff_executable:
     if (Filename.endswith_lower(".dll")) {
       error(Filename + ": bad file type. Did you specify a DLL instead of an "
                        "import library?");
       break;

COFF/InputFiles.h

 #include "llvm/Support/StringSaver.h"
 #include <memory>
 #include <set>
 #include <vector>
 
 namespace llvm {
 namespace pdb {
 class DbiModuleDescriptorBuilder;
+class NativeSession;
 }
 }
 
 namespace lld {
 namespace coff {
 
 std::vector<MemoryBufferRef> getArchiveMembers(llvm::object::Archive *File);
 

 class SectionChunk;
 class Symbol;
 class Undefined;
 class TpiSource;
 
 // The root class of input files.
 class InputFile {
 public:
-  enum Kind { ArchiveKind, ObjectKind, ImportKind, BitcodeKind };
+  enum Kind { ArchiveKind, ObjectKind, PDBKind, ImportKind, BitcodeKind };
   Kind kind() const { return FileKind; }
   virtual ~InputFile() {}
 
   // Returns the filename.
   StringRef getName() const { return MB.getBufferIdentifier(); }
 
   // Reads a file (the constructor doesn't do that).
   virtual void parse() = 0;
private:
 
   // This vector contains a list of all symbols defined or referenced by this
   // file. They are indexed such that you can get a Symbol by symbol
   // index. Nonexistent indices (which are occupied by auxiliary
   // symbols in the real symbol table) are filled with null pointers.
   std::vector<Symbol *> Symbols;
 };
 
+// A PDB type server dependency that is not a cmd-line input file per se, but
+// needs to be treated like one. A PDBInputFile is discovered from the debug
+// type stream from dependent ObjFiles.
+class PDBInputFile : public InputFile {
+public:
+  explicit PDBInputFile(MemoryBufferRef M) : InputFile(PDBKind, M) {}
+  static bool classof(const InputFile *F) { return F->kind() == PDBKind; }
+  void parse() override;
+
+  static void enqueue(ObjFile *DependentFile);
+
+  // Fetch the PDB instance loaded for a corresponding dependent OBJ.
+  static Expected<PDBInputFile> findFromFile(ObjFile *DependentFile);
+
+  static std::map<std::string, PDBInputFile *> Instances;
+
+  // The actual interface to the PDB (if it was opened successfully)
+  llvm::pdb::NativeSession *Session;
+
+  // If the PDB has a .debug$T stream, this tells how it will be handled.
+  TpiSource *DebugTypesObj;
+
+private:
+  // Record possible errors while opening the PDB file
+  std::string ErrMsg;
+};
+
 // This type represents import library members that contain DLL names
 // and symbols exported from the DLLs. See Microsoft PE/COFF spec. 7
 // for details about the format.
 class ImportFile : public InputFile {
 public:
   explicit ImportFile(MemoryBufferRef M) : InputFile(ImportKind, M) {}
 
   static bool classof(const InputFile *F) { return F->kind() == ImportKind; }

COFF/InputFiles.cpp

 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/BinaryFormat/COFF.h"
 #include "llvm/DebugInfo/CodeView/DebugSubsectionRecord.h"
 #include "llvm/DebugInfo/CodeView/SymbolDeserializer.h"
 #include "llvm/DebugInfo/CodeView/SymbolRecord.h"
 #include "llvm/DebugInfo/CodeView/TypeDeserializer.h"
+#include "llvm/DebugInfo/PDB/GenericError.h"
+#include "llvm/DebugInfo/PDB/Native/InfoStream.h"
+#include "llvm/DebugInfo/PDB/Native/NativeSession.h"
+#include "llvm/DebugInfo/PDB/Native/PDBFile.h"
 #include "llvm/Object/Binary.h"
 #include "llvm/Object/COFF.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Endian.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Target/TargetOptions.h"
 #include <cstring>
 #include <system_error>
 #include <utility>
 
 using namespace llvm;
 using namespace llvm::COFF;
 using namespace llvm::codeview;
 using namespace llvm::object;
 using namespace llvm::support::endian;
 
 using llvm::Triple;
 using llvm::support::ulittle32_t;
 
 namespace lld {
 namespace coff {
 
 std::vector<ObjFile *> ObjFile::Instances;
+std::map<std::string, PDBInputFile *> PDBInputFile::Instances;
 std::vector<ImportFile *> ImportFile::Instances;
 std::vector<BitcodeFile *> BitcodeFile::Instances;
 
 /// Checks that Source is compatible with being a weak alias to Target.
 /// If Source is Undefined and has no weak alias set, makes it a weak
 /// alias to Target.
 static void checkAndSetWeakAlias(SymbolTable *Symtab, InputFile *F,
                                  Symbol *Source, Symbol *Target) {
if (IsPCH) {
     DebugTypesObj = makePrecompSource(this);
     return;
   }
 
   if (FirstType->kind() == LF_TYPESERVER2) {
     TypeServer2Record TS = cantFail(
         TypeDeserializer::deserializeAs<TypeServer2Record>(FirstType->data()));
     DebugTypesObj = makeUseTypeServerSource(this, &TS);
+    PDBInputFile::enqueue(this);
     return;
   }
 
   if (FirstType->kind() == LF_PRECOMP) {
     PrecompRecord Precomp = cantFail(
         TypeDeserializer::deserializeAs<PrecompRecord>(FirstType->data()));
     DebugTypesObj = makeUsePrecompSource(this, &Precomp);
     return;
   }
 
   DebugTypesObj = makeTpiSource(this);
 }
 
+// Make a PDB path assuming the PDB is in the same folder as the OBJ
+static std::string getPdbBaseName(ObjFile *File, StringRef TSPath) {
+  StringRef LocalPath =
+      !File->ParentName.empty() ? File->ParentName : File->getName();
+  std::string Path = sys::path::parent_path(LocalPath);
+  // Currently, type server PDBs are only created by cl, which only runs
+  // on Windows, so we can assume type server paths are Windows style.
+  Path += sys::path::filename(TSPath, sys::path::Style::windows);
+  return Path;
+}
+
+// The casing of the PDB path stamped in the OBJ can differ from the actual path
+// on disk. With this, we ensure to always use lowercase as a key for the
+// PDBInputFile::Instances map, at least on Windows.
+static std::string normalizePdbPath(StringRef path) {
+#if defined(_WIN32)
+  return path.lower();
+#else // LINUX
+  return path;
+#endif
+}
+
+// If existing, return the actual PDB path on disk.
+static Optional<std::string> findPdbPath(StringRef PDBPath,
+                                         ObjFile *DependentFile) {
+  // Ensure the file exists before anything else. In some cases, if the path
+  // points to a removable device, enqueuePath() would fail with an error
+  // (EAGAIN, "resource unavailable try again") which we want to skip
+  // silently.
+  if (llvm::sys::fs::exists(PDBPath))
+    return PDBPath.str();
+  std::string Ret = getPdbBaseName(DependentFile, PDBPath);
+  if (llvm::sys::fs::exists(Ret))
+    return Ret;
+  return None;
+}
+
+Expected<PDBInputFile> PDBInputFile::findFromFile(ObjFile *DependentFile) {
+  const TypeServer2Record &TS =
+      retrieveDependencyInfo<TypeServer2Record>(DependentFile->DebugTypesObj);
+
+  Optional<std::string> P = findPdbPath(TS.Name, DependentFile);
+  if (!P)
+    return createFileError(TS.Name, errorCodeToError(std::error_code(
+                                        ENOENT, std::generic_category())));
+
+  auto It = PDBInputFile::Instances.find(*P);
+  // The PDB file exists on disk, we expect it to have been inserted in the map
+  // in PDBInputFile::parse()
+  assert(It != PDBInputFile::Instances.end());
+
+  PDBInputFile *PDB = It->second;
+
+  if (!PDB->ErrMsg.empty())
+    return createFileError(TS.Name, make_error<StringError>(
+                                        PDB->ErrMsg, inconvertibleErrorCode()));
+
+  pdb::PDBFile &PDBFile = PDB->Session->getPDBFile();
+  pdb::InfoStream &Info = cantFail(PDBFile.getPDBInfoStream());
+
+  // Just because a file with a matching name was found doesn't mean it can be
+  // used. The GUID and Age must match between the PDB header and the OBJ
+  // TypeServer2 record.
+  if (Info.getGuid() != TS.getGuid() || Info.getAge() != TS.getAge())
+    return createFileError(
+        TS.Name,
+        make_error<pdb::PDBError>(pdb::pdb_error_code::signature_out_of_date));
+
+  return *PDB;
+}
+
+void PDBInputFile::enqueue(ObjFile *DependentFile) {
+  const TypeServer2Record &TS =
+      retrieveDependencyInfo<TypeServer2Record>(DependentFile->DebugTypesObj);
+
+  Optional<std::string> P = findPdbPath(TS.Name, DependentFile);
+  if (!P)
+    return;
+  auto It = PDBInputFile::Instances.emplace(*P, nullptr);
+  if (!It.second)
+    return; // already scheduled for load
+  Driver->enqueuePath(*P, false);
+}
+
+void PDBInputFile::parse() {
+  PDBInputFile::Instances[normalizePdbPath(MB.getBufferIdentifier())] = this;
+
+  std::unique_ptr<llvm::pdb::IPDBSession> ISession;
+  Error Err = pdb::NativeSession::createFromPdb(
+      MemoryBuffer::getMemBuffer(MB, false), ISession);
+  if (Err) {
+    ErrMsg = toString(std::move(Err));
+    return; // fail silently at this point - the error will be handled later,
+            // when merging the debug type stream
+  }
+
+  static std::vector<std::unique_ptr<llvm::pdb::NativeSession>> SessionGC;
+  SessionGC.emplace_back(static_cast<pdb::NativeSession *>(ISession.release()));
+  Session = SessionGC.rbegin()->get();
+
+  pdb::PDBFile &PDBFile = Session->getPDBFile();
+  Expected<pdb::InfoStream &> Info = PDBFile.getPDBInfoStream();
+  if (!Info) {
+    // All PDB Files should have an Info stream.
+    ErrMsg = toString(Info.takeError());
+    return;
+  }
+  DebugTypesObj = makeTypeServerSource(this);
+}
+
 StringRef ltrim1(StringRef S, const char *Chars) {
   if (!S.empty() && strchr(Chars, S[0]))
     return S.substr(1);
   return S;
 }
 
 void ImportFile::parse() {
   const char *Buf = MB.getBufferStart();

COFF/PDB.cpp

private:
   GlobalTypeTableBuilder GlobalIDTable;
 
   /// PDBs use a single global string table for filenames in the file checksum
   /// table.
   DebugStringTableSubsection PDBStrTab;
 
   llvm::SmallString<128> NativePath;
 
-  /// A list of other PDBs which are loaded during the linking process and which
-  /// we need to keep around since the linking operation may reference pointers
-  /// inside of these PDBs.
-  llvm::SmallVector<std::unique_ptr<pdb::NativeSession>, 2> LoadedPDBs;
-
   std::vector<pdb::SecMapEntry> SectionMap;
 
   /// Type index mappings of type server PDBs that we've loaded so far.
   std::map<codeview::GUID, CVIndexMap> TypeServerIndexMappings;
 
   /// Type index mappings of precompiled objects type map that we've loaded so
   /// far.
   std::map<uint32_t, CVIndexMap> PrecompTypeIndexMappings;
 
-  /// List of TypeServer PDBs which cannot be loaded.
-  /// Cached to prevent repeated load attempts.
-  std::map<codeview::GUID, std::string> MissingTypeServerPDBs;
-
   // For statistics
   uint64_t GlobalSymbols = 0;
   uint64_t ModuleSymbols = 0;
   uint64_t PublicSymbols = 0;
 };
 
 class DebugSHandler {
   PDBLinker &Linker;
if (auto Err =
             mergeTypeAndIdRecords(IDTable, TypeTable, ObjectIndexMap->TPIMap,
                                   Types, File->PCHSignature))
       fatal("codeview::mergeTypeAndIdRecords failed: " +
             toString(std::move(Err)));
   }
   return *ObjectIndexMap;
 }
 
-static Expected<std::unique_ptr<pdb::NativeSession>>
-tryToLoadPDB(const codeview::GUID &GuidFromObj, StringRef TSPath) {
-  // Ensure the file exists before anything else. We want to return ENOENT,
-  // "file not found", even if the path points to a removable device (in which
-  // case the return message would be EAGAIN, "resource unavailable try again")
-  if (!llvm::sys::fs::exists(TSPath))
-    return errorCodeToError(std::error_code(ENOENT, std::generic_category()));
-
-  ErrorOr<std::unique_ptr<MemoryBuffer>> MBOrErr = MemoryBuffer::getFile(
-      TSPath, /*FileSize=*/-1, /*RequiresNullTerminator=*/false);
-  if (!MBOrErr)
-    return errorCodeToError(MBOrErr.getError());
-
-  std::unique_ptr<pdb::IPDBSession> ThisSession;
-  if (auto EC = pdb::NativeSession::createFromPdb(
-          MemoryBuffer::getMemBuffer(Driver->takeBuffer(std::move(*MBOrErr)),
-                                     /*RequiresNullTerminator=*/false),
-          ThisSession))
-    return std::move(EC);
-
-  std::unique_ptr<pdb::NativeSession> NS(
-      static_cast<pdb::NativeSession *>(ThisSession.release()));
-  pdb::PDBFile &File = NS->getPDBFile();
-  auto ExpectedInfo = File.getPDBInfoStream();
-  // All PDB Files should have an Info stream.
-  if (!ExpectedInfo)
-    return ExpectedInfo.takeError();
-
-  // Just because a file with a matching name was found and it was an actual
-  // PDB file doesn't mean it matches.  For it to match the InfoStream's GUID
-  // must match the GUID specified in the TypeServer2 record.
-  if (ExpectedInfo->getGuid() != GuidFromObj)
-    return make_error<pdb::PDBError>(pdb::pdb_error_code::signature_out_of_date);
-
-  return std::move(NS);
-}
-
 Expected<const CVIndexMap &> PDBLinker::maybeMergeTypeServerPDB(ObjFile *File) {
-  const TypeServer2Record &TS =
-      retrieveDependencyInfo<TypeServer2Record>(File->DebugTypesObj);
-
-  const codeview::GUID &TSId = TS.getGuid();
-  StringRef TSPath = TS.getName();
-
-  // First, check if the PDB has previously failed to load.
-  auto PrevErr = MissingTypeServerPDBs.find(TSId);
-  if (PrevErr != MissingTypeServerPDBs.end())
-    return createFileError(
-        TSPath,
-        make_error<StringError>(PrevErr->second, inconvertibleErrorCode()));
-
-  // Second, check if we already loaded a PDB with this GUID. Return the type
-  // index mapping if we have it.
-  auto Insertion = TypeServerIndexMappings.insert({TSId, CVIndexMap()});
-  CVIndexMap &IndexMap = Insertion.first->second;
-  if (!Insertion.second)
-    return IndexMap;
+  Expected<PDBInputFile> PDB = PDBInputFile::findFromFile(File);
+  if (!PDB)
+    return PDB.takeError();
+
+  pdb::PDBFile &PDBFile = PDB->Session->getPDBFile();
+  pdb::InfoStream &Info = cantFail(PDBFile.getPDBInfoStream());
+
+  auto It = TypeServerIndexMappings.emplace(Info.getGuid(), CVIndexMap());
+  CVIndexMap &IndexMap = It.first->second;
+  if (!It.second)
+    return IndexMap; // already merged
 
   // Mark this map as a type server map.
   IndexMap.IsTypeServerMap = true;
 
-  // Check for a PDB at:
-  // 1. The given file path
-  // 2. Next to the object file or archive file
-  auto ExpectedSession = handleExpected(
-      tryToLoadPDB(TSId, TSPath),
-      [&]() {
-        StringRef LocalPath =
-            !File->ParentName.empty() ? File->ParentName : File->getName();
-        SmallString<128> Path = sys::path::parent_path(LocalPath);
-        // Currently, type server PDBs are only created by cl, which only runs
-        // on Windows, so we can assume type server paths are Windows style.
-        sys::path::append(
-            Path, sys::path::filename(TSPath, sys::path::Style::windows));
-        return tryToLoadPDB(TSId, Path);
-      },
-      [&](std::unique_ptr<ECError> EC) -> Error {
-        auto SysErr = EC->convertToErrorCode();
-        // Only re-try loading if the previous error was "No such file or
-        // directory"
-        if (SysErr.category() == std::generic_category() &&
-            SysErr.value() == ENOENT)
-          return Error::success();
-        return Error(std::move(EC));
-      });
-
-  if (auto E = ExpectedSession.takeError()) {
-    TypeServerIndexMappings.erase(TSId);
-
-    // Flatten the error to a string, for later display, if the error occurs
-    // again on the same PDB.
-    std::string ErrMsg;
-    raw_string_ostream S(ErrMsg);
-    S << E;
-    MissingTypeServerPDBs.emplace(TSId, S.str());
-
-    return createFileError(TSPath, std::move(E));
-  }
-
-  pdb::NativeSession *Session = ExpectedSession->get();
-
-  // Keep a strong reference to this PDB, so that it's safe to hold pointers
-  // into the file.
-  LoadedPDBs.push_back(std::move(*ExpectedSession));
-
-  auto ExpectedTpi = Session->getPDBFile().getPDBTpiStream();
+  Expected<pdb::TpiStream &> ExpectedTpi = PDBFile.getPDBTpiStream();
   if (auto E = ExpectedTpi.takeError())
     fatal("Type server does not have TPI stream: " + toString(std::move(E)));
-  auto ExpectedIpi = Session->getPDBFile().getPDBIpiStream();
+  Expected<pdb::TpiStream &> ExpectedIpi = PDBFile.getPDBIpiStream();
   if (auto E = ExpectedIpi.takeError())
     fatal("Type server does not have TPI stream: " + toString(std::move(E)));
 
   if (Config->DebugGHashes) {
     // PDBs do not actually store global hashes, so when merging a type server
     // PDB we have to synthesize global hashes.  To do this, we first synthesize
     // global hashes for the TPI stream, since it is independent, then we
     // synthesize hashes for the IPI stream, using the hashes for the TPI stream

test/COFF/Inputs/pdb-type-server-valid-signature.yaml

Relocations:
         Type:            IMAGE_REL_AMD64_SECTION
   - Name:            '.debug$T'
     Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
     Alignment:       1
     Types:
       - Kind:            LF_TYPESERVER2
         TypeServer2:
           Guid:            '{8DABD2A0-28FF-CB43-9BAF-175B77B76414}'
-          Age:             18
+          Age:             1
           Name:            'pdb-diff-cl.pdb'
   - Name:            '.text$mn'
     Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
     Alignment:       16
     SectionData:     33C0C3
 symbols:
   - Name:            '.debug$S'
     Value:           0

test/COFF/pdb-type-server-invalid-signature.yaml

 
 # RUN: cd %S/Inputs
 # RUN: yaml2obj %s -o %t.obj
 # RUN: lld-link %t.obj -out:%t.exe -debug -pdb:%t.pdb -nodefaultlib -entry:main 2>&1 | FileCheck %s
 # RUN: cd %S
 
-# CHECK: warning: Cannot use debug info for {{.*}}.obj
+# CHECK: warning: Cannot use debug info for '{{.*}}.obj'
 # CHECK-NEXT: The signature does not match; the file(s) might be out of date
 
 # Also test a valid match
 
 # RUN: cd %S/Inputs
 # RUN: yaml2obj %S/Inputs/pdb-type-server-valid-signature.yaml -o %t2.obj
 # RUN: lld-link %t2.obj -out:%t2.exe -debug -pdb:%t2.pdb -nodefaultlib -entry:main 2>&1 | FileCheck %s -check-prefix=VALID-SIGNATURE -allow-empty
 # RUN: cd %S
 
-# VALID-SIGNATURE-NOT: warning: Cannot use debug info for {{.*}}.obj
+# VALID-SIGNATURE-NOT: warning: Cannot use debug info for '{{.*}}.obj'
 # VALID-SIGNATURE-NOT: The signature does not match; the file(s) might be out of date
 
 --- !COFF
 header:
   Machine:         IMAGE_FILE_MACHINE_AMD64
   Characteristics: [  ]
 sections:
   - Name:            '.debug$S'