[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 ↗

(On Diff #193163)

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

722 ↗

(On Diff #193163)

const ObjFile *File ?

745 ↗

(On Diff #193163)

can be const ObjFile * here too?

758 ↗

(On Diff #193163)

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

33–36 ↗

(On Diff #196643)

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.

149–150 ↗

(On Diff #196643)

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

[aganea]

COFF/DebugTypes.cpp

33–36 ↗

(On Diff #196643)

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.

lld/trunk/COFF/DebugTypes.h

 //===- DebugTypes.h ---------------------------------------------*- C++ -*-===//
 //
 // 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
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLD_COFF_DEBUGTYPES_H
 #define LLD_COFF_DEBUGTYPES_H
 
 #include "llvm/Support/Error.h"
+#include "llvm/Support/MemoryBuffer.h"
 
 namespace llvm {
 namespace codeview {
 class PrecompRecord;
 class TypeServer2Record;
 } // namespace codeview
+namespace pdb {
+class NativeSession;
+}
 } // namespace llvm
 
 namespace lld {
 namespace coff {
 
 class ObjFile;
 
 class TpiSource {
 public:
   enum TpiKind { Regular, PCH, UsingPCH, PDB, UsingPDB };
 
-  TpiSource(TpiKind K, ObjFile *F);
+  TpiSource(TpiKind K, const ObjFile *F);
   virtual ~TpiSource() {}
 
   const TpiKind Kind;
-  ObjFile *File;
+  const ObjFile *File;
 };
 
-TpiSource *makeTpiSource(ObjFile *F);
-TpiSource *makeTypeServerSource(ObjFile *F);
-TpiSource *makeUseTypeServerSource(ObjFile *F,
-                                   llvm::codeview::TypeServer2Record *TS);
-TpiSource *makePrecompSource(ObjFile *F);
-TpiSource *makeUsePrecompSource(ObjFile *F,
-                                llvm::codeview::PrecompRecord *Precomp);
+TpiSource *makeTpiSource(const ObjFile *F);
+TpiSource *makeUseTypeServerSource(const ObjFile *F,
+                                   const llvm::codeview::TypeServer2Record *TS);
+TpiSource *makePrecompSource(const ObjFile *F);
+TpiSource *makeUsePrecompSource(const ObjFile *F,
+                                const llvm::codeview::PrecompRecord *Precomp);
+
+void loadTypeServerSource(llvm::MemoryBufferRef M);
 
 // Temporary interface to get the dependency
-template <typename T> const T &retrieveDependencyInfo(TpiSource *Source);
+template <typename T> const T &retrieveDependencyInfo(const TpiSource *Source);
+
+// Temporary interface until we move PDBLinker::maybeMergeTypeServerPDB here
+llvm::Expected<llvm::pdb::NativeSession *>
+findTypeServerSource(const ObjFile *F);
 
 } // namespace coff
 } // namespace lld
 
 #endif
 No newline at end of file

lld/trunk/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 "Driver.h"
 #include "InputFiles.h"
+#include "lld/Common/ErrorHandler.h"
 #include "llvm/DebugInfo/CodeView/TypeRecord.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/Support/Path.h"
 
 using namespace lld;
 using namespace lld::coff;
 using namespace llvm;
 using namespace llvm::codeview;
 
 namespace {
+// The TypeServerSource class represents a PDB type server, a file referenced by
+// OBJ files compiled with MSVC /Zi. A single PDB can be shared by several OBJ
+// files, therefore there must be only once instance per OBJ lot. The file path
+// is discovered from the dependent OBJ's debug type stream. The
+// TypeServerSource object is then queued and loaded by the COFF Driver. The
+// debug type stream for such PDB files will be merged first in the final PDB,
+// before any dependent OBJ.
 class TypeServerSource : public TpiSource {
 public:
-  TypeServerSource(ObjFile *F) : TpiSource(PDB, F) {}
+  explicit TypeServerSource(MemoryBufferRef M, llvm::pdb::NativeSession *S)
+      : TpiSource(PDB, nullptr), Session(S), MB(M) {}
+
+  // Queue a PDB type server for loading in the COFF Driver
+  static void enqueue(const ObjFile *DependentFile,
+                      const TypeServer2Record &TS);
+
+  // Create an instance
+  static Expected<TypeServerSource *> getInstance(MemoryBufferRef M);
+
+  // Fetch the PDB instance loaded for a corresponding dependent OBJ.
+  static Expected<TypeServerSource *>
+  findFromFile(const ObjFile *DependentFile);
+
+  static std::map<std::string, std::pair<std::string, TypeServerSource *>>
+      Instances;
+
+  // The interface to the PDB (if it was opened successfully)
+  std::unique_ptr<llvm::pdb::NativeSession> Session;
+
+private:
+  MemoryBufferRef MB;
 };
 
+// This class represents the debug type stream of an OBJ file that depends on a
+// PDB type server (see TypeServerSource).
 class UseTypeServerSource : public TpiSource {
 public:
-  UseTypeServerSource(ObjFile *F, TypeServer2Record *TS)
+  UseTypeServerSource(const ObjFile *F, const TypeServer2Record *TS)
       : TpiSource(UsingPDB, F), TypeServerDependency(*TS) {}
 
   // Information about the PDB type server dependency, that needs to be loaded
   // in before merging this OBJ.
   TypeServer2Record TypeServerDependency;
 };
 
+// This class represents the debug type stream of a Microsoft precompiled
+// headers OBJ (PCH OBJ). This OBJ kind needs to be merged first in the output
+// PDB, before any other OBJs that depend on this. Note that only MSVC generate
+// such files, clang does not.
 class PrecompSource : public TpiSource {
 public:
-  PrecompSource(ObjFile *F) : TpiSource(PCH, F) {}
+  PrecompSource(const ObjFile *F) : TpiSource(PCH, F) {}
 };
 
+// This class represents the debug type stream of an OBJ file that depends on a
+// Microsoft precompiled headers OBJ (see PrecompSource).
 class UsePrecompSource : public TpiSource {
 public:
-  UsePrecompSource(ObjFile *F, PrecompRecord *Precomp)
+  UsePrecompSource(const ObjFile *F, const PrecompRecord *Precomp)
       : TpiSource(UsingPCH, F), PrecompDependency(*Precomp) {}
 
   // Information about the Precomp OBJ dependency, that needs to be loaded in
   // before merging this OBJ.
   PrecompRecord PrecompDependency;
 };
 } // namespace
 
 static std::vector<std::unique_ptr<TpiSource>> GC;
 
-TpiSource::TpiSource(TpiKind K, ObjFile *F) : Kind(K), File(F) {
+TpiSource::TpiSource(TpiKind K, const ObjFile *F) : Kind(K), File(F) {
   GC.push_back(std::unique_ptr<TpiSource>(this));
 }
 
-TpiSource *coff::makeTpiSource(ObjFile *F) {
+TpiSource *lld::coff::makeTpiSource(const ObjFile *F) {
   return new TpiSource(TpiSource::Regular, F);
 }
 
-TpiSource *coff::makeTypeServerSource(ObjFile *F) {
-  return new TypeServerSource(F);
-}
-
-TpiSource *coff::makeUseTypeServerSource(ObjFile *F, TypeServer2Record *TS) {
+TpiSource *lld::coff::makeUseTypeServerSource(const ObjFile *F,
+                                              const TypeServer2Record *TS) {
+  TypeServerSource::enqueue(F, *TS);
   return new UseTypeServerSource(F, TS);
 }
 
-TpiSource *coff::makePrecompSource(ObjFile *F) { return new PrecompSource(F); }
+TpiSource *lld::coff::makePrecompSource(const ObjFile *F) {
+  return new PrecompSource(F);
+}
 
-TpiSource *coff::makeUsePrecompSource(ObjFile *F, PrecompRecord *Precomp) {
+TpiSource *lld::coff::makeUsePrecompSource(const ObjFile *F,
+                                           const PrecompRecord *Precomp) {
   return new UsePrecompSource(F, Precomp);
 }
 
 namespace lld {
 namespace coff {
 template <>
-const PrecompRecord &retrieveDependencyInfo(TpiSource *Source) {
+const PrecompRecord &retrieveDependencyInfo(const TpiSource *Source) {
   assert(Source->Kind == TpiSource::UsingPCH);
-  return ((UsePrecompSource *)Source)->PrecompDependency;
+  return ((const UsePrecompSource *)Source)->PrecompDependency;
 }
 
 template <>
-const TypeServer2Record &retrieveDependencyInfo(TpiSource *Source) {
+const TypeServer2Record &retrieveDependencyInfo(const TpiSource *Source) {
   assert(Source->Kind == TpiSource::UsingPDB);
-  return ((UseTypeServerSource *)Source)->TypeServerDependency;
+  return ((const UseTypeServerSource *)Source)->TypeServerDependency;
 }
 } // namespace coff
 } // namespace lld
+
+std::map<std::string, std::pair<std::string, TypeServerSource *>>
+    TypeServerSource::Instances;
+
+// Make a PDB path assuming the PDB is in the same folder as the OBJ
+static std::string getPdbBaseName(const 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 MSVC cl, which only runs
+  // on Windows, so we can assume type server paths are Windows style.
+  return Path + sys::path::filename(TSPath, sys::path::Style::windows).str();
+}
+
+// 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,
+                                         const ObjFile *DependentFile) {
+  // Ensure the file exists before anything else. In some cases, if the path
+  // points to a removable device, Driver::enqueuePath() would fail with an
+  // error (EAGAIN, "resource unavailable try again") which we want to skip
+  // silently.
+  if (llvm::sys::fs::exists(PDBPath))
+    return normalizePdbPath(PDBPath);
+  std::string Ret = getPdbBaseName(DependentFile, PDBPath);
+  if (llvm::sys::fs::exists(Ret))
+    return normalizePdbPath(Ret);
+  return None;
+}
+
+// Fetch the PDB instance that was already loaded by the COFF Driver.
+Expected<TypeServerSource *>
+TypeServerSource::findFromFile(const 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 = TypeServerSource::Instances.find(*P);
+  // The PDB file exists on disk, at this point we expect it to have been
+  // inserted in the map by TypeServerSource::loadPDB()
+  assert(It != TypeServerSource::Instances.end());
+
+  std::pair<std::string, TypeServerSource *> &PDB = It->second;
+
+  if (!PDB.second)
+    return createFileError(
+        *P, createStringError(inconvertibleErrorCode(), PDB.first.c_str()));
+
+  pdb::PDBFile &PDBFile = (PDB.second)->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. The 'Age' is used by MSVC incremental compilation.
+  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.second;
+}
+
+// FIXME: Temporary interface until PDBLinker::maybeMergeTypeServerPDB() is
+// moved here.
+Expected<llvm::pdb::NativeSession *>
+lld::coff::findTypeServerSource(const ObjFile *F) {
+  Expected<TypeServerSource *> TS = TypeServerSource::findFromFile(F);
+  if (!TS)
+    return TS.takeError();
+  return TS.get()->Session.get();
+}
+
+// Queue a PDB type server for loading in the COFF Driver
+void TypeServerSource::enqueue(const ObjFile *DependentFile,
+                               const TypeServer2Record &TS) {
+  // Start by finding where the PDB is located (either the record path or next
+  // to the OBJ file)
+  Optional<std::string> P = findPdbPath(TS.Name, DependentFile);
+  if (!P)
+    return;
+  auto It = TypeServerSource::Instances.emplace(
+      *P, std::pair<std::string, TypeServerSource *>{});
+  if (!It.second)
+    return; // another OBJ already scheduled this PDB for load
+
+  Driver->enqueuePath(*P, false);
+}
+
+// Create an instance of TypeServerSource or an error string if the PDB couldn't
+// be loaded. The error message will be displayed later, when the referring OBJ
+// will be merged in. NOTE - a PDB load failure is not a link error: some
+// debug info will simply be missing from the final PDB - that is the default
+// accepted behavior.
+void lld::coff::loadTypeServerSource(llvm::MemoryBufferRef M) {
+  std::string Path = normalizePdbPath(M.getBufferIdentifier());
+
+  Expected<TypeServerSource *> TS = TypeServerSource::getInstance(M);
+  if (!TS)
+    TypeServerSource::Instances[Path] = {toString(TS.takeError()), nullptr};
+  else
+    TypeServerSource::Instances[Path] = {{}, *TS};
+}
+
+Expected<TypeServerSource *> TypeServerSource::getInstance(MemoryBufferRef M) {
+  std::unique_ptr<llvm::pdb::IPDBSession> ISession;
+  Error Err = pdb::NativeSession::createFromPdb(
+      MemoryBuffer::getMemBuffer(M, false), ISession);
+  if (Err)
+    return std::move(Err);
+
+  std::unique_ptr<llvm::pdb::NativeSession> Session(
+      static_cast<pdb::NativeSession *>(ISession.release()));
+
+  pdb::PDBFile &PDBFile = Session->getPDBFile();
+  Expected<pdb::InfoStream &> Info = PDBFile.getPDBInfoStream();
+  // All PDB Files should have an Info stream.
+  if (!Info)
+    return Info.takeError();
+  return new TypeServerSource(M, Session.release());
+}

lld/trunk/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, uint64_t OffsetInArchive);
 
-  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;

lld/trunk/COFF/Driver.cpp

 //===- Driver.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 "Driver.h"
 #include "Config.h"
 #include "ICF.h"
 #include "InputFiles.h"
 #include "MarkLive.h"
 #include "MinGW.h"
 #include "SymbolTable.h"
 #include "Symbols.h"
case file_magic::archive:
     break;
   case file_magic::bitcode:
     Symtab->addFile(make<BitcodeFile>(MBRef, "", 0));
     break;
   case file_magic::coff_object:
   case file_magic::coff_import_library:
     Symtab->addFile(make<ObjFile>(MBRef));
     break;
+  case file_magic::pdb:
+    loadTypeServerSource(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;

lld/trunk/COFF/PDB.cpp

private:
   TypeMerger TMerger;
 
   /// 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(TMerger.IDTable, TMerger.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<llvm::pdb::NativeSession *> PDBSession = findTypeServerSource(File);
+  if (!PDBSession)
+    return PDBSession.takeError();
+
+  pdb::PDBFile &PDBFile = PDBSession.get()->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

lld/trunk/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

lld/trunk/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'