Introduce symbol cache to PDB NativeSession

[amccarth]

Summary

Instead of creating symbols directly in the findChildren methods of the native symbol implementations, they will rely on the NativeSession to act as a factory for these types. This lets NativeSession cache the NativeRawSymbols in its new symbol cache and makes that cache the source of unique IDs for the symbols.

Right now, this affects only NativeCompilandSymbols. There's no external change yet, so I think the existing tests are still sufficient. Coming soon are patches to extend this to built-in types and enums.

[amccarth]

ping

[zturner]

llvm/include/llvm/DebugInfo/PDB/Native/NativeSession.h

35

Should this return a PDBSymbolCompiland?

llvm/lib/DebugInfo/PDB/Native/NativeSession.cpp

75–76

push_back instead of emplace_back (same as below)

77–78

make_unique again.

88

make_unique? Also, since you're passing the value type directly, you should use push_back instead of emplace_back

[amccarth]

llvm/include/llvm/DebugInfo/PDB/Native/NativeSession.h

35

That's a fair question. This factory method (and the others that are in the pipeline) are typically invoked in implementations of IPDBSymbol::getChildAtIndex, which return std::unique_ptr<PDBSymbol>, so I was going with the base type, but I suppose there's no harm in returning the derived type and letting the implicit conversion happen.

llvm/lib/DebugInfo/PDB/Native/NativeSession.cpp

77–78

I need to construct an instance of the derived type but return it as a unique_ptr to the base type. I've had trouble getting VS to accept that with std::make_unique, which is why I was using new directly. But it seems to work with llvm::make_unique, so I guess that'll do.

[amccarth]

Addresses feedback from zturner.

[zturner]

llvm/lib/DebugInfo/PDB/Native/NativeSession.cpp

12–14

I think these are supposed to go below the llvm includes based on the style guide.

[amccarth]

llvm/lib/DebugInfo/PDB/Native/NativeSession.cpp

12–14

You're right, of course. I spent too long in Google style to keep them straight now.

[amccarth]

Fixed order of includes to comply with LLVM style

llvm/include/llvm/DebugInfo/PDB/Native/NativeSession.h

 //===- NativeSession.h - Native implementation of IPDBSession ---*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
-#ifndef LLVM_DEBUGINFO_PDB_RAW_RAWSESSION_H
-#define LLVM_DEBUGINFO_PDB_RAW_RAWSESSION_H
+#ifndef LLVM_DEBUGINFO_PDB_NATIVE_NATIVESESSION_H
+#define LLVM_DEBUGINFO_PDB_NATIVE_NATIVESESSION_H
 
 #include "llvm/ADT/StringRef.h"
 #include "llvm/DebugInfo/PDB/IPDBSession.h"
+#include "llvm/DebugInfo/PDB/Native/DbiModuleDescriptor.h"
+#include "llvm/DebugInfo/PDB/Native/NativeRawSymbol.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/Error.h"
 
 namespace llvm {
 namespace pdb {
 class PDBFile;
 
 class NativeSession : public IPDBSession {
 public:
   NativeSession(std::unique_ptr<PDBFile> PdbFile,
                 std::unique_ptr<BumpPtrAllocator> Allocator);
   ~NativeSession() override;
 
   static Error createFromPdb(StringRef Path,
                              std::unique_ptr<IPDBSession> &Session);
   static Error createFromExe(StringRef Path,
                              std::unique_ptr<IPDBSession> &Session);
 
+  std::unique_ptr<PDBSymbolCompiland>
+  createCompilandSymbol(DbiModuleDescriptor MI);
+
   uint64_t getLoadAddress() const override;
   void setLoadAddress(uint64_t Address) override;
   std::unique_ptr<PDBSymbolExe> getGlobalScope() override;
   std::unique_ptr<PDBSymbol> getSymbolById(uint32_t SymbolId) const override;
 
   std::unique_ptr<PDBSymbol>
   findSymbolByAddress(uint64_t Address, PDB_SymType Type) const override;
 
public:
   std::unique_ptr<IPDBEnumDataStreams> getDebugStreams() const override;
 
   PDBFile &getPDBFile() { return *Pdb; }
   const PDBFile &getPDBFile() const { return *Pdb; }
 
 private:
   std::unique_ptr<PDBFile> Pdb;
   std::unique_ptr<BumpPtrAllocator> Allocator;
+  std::vector<std::unique_ptr<NativeRawSymbol>> SymbolCache;
 };
 }
 }
 
 #endif

llvm/lib/DebugInfo/PDB/Native/NativeEnumModules.cpp

 uint32_t NativeEnumModules::getChildCount() const {
   return static_cast<uint32_t>(Modules.getModuleCount());
 }
 
 std::unique_ptr<PDBSymbol>
 NativeEnumModules::getChildAtIndex(uint32_t Index) const {
   if (Index >= Modules.getModuleCount())
     return nullptr;
-  return std::unique_ptr<PDBSymbol>(new PDBSymbolCompiland(
-      Session, std::unique_ptr<IPDBRawSymbol>(new NativeCompilandSymbol(
-                   Session, 0, Modules.getModuleDescriptor(Index)))));
+  return Session.createCompilandSymbol(Modules.getModuleDescriptor(Index));
 }
 
 std::unique_ptr<PDBSymbol> NativeEnumModules::getNext() {
   if (Index >= Modules.getModuleCount())
     return nullptr;
   return getChildAtIndex(Index++);
 }
 
 void NativeEnumModules::reset() { Index = 0; }
 
 NativeEnumModules *NativeEnumModules::clone() const {
   return new NativeEnumModules(Session, Modules, Index);
 }
 
 }
 }

llvm/lib/DebugInfo/PDB/Native/NativeSession.cpp

 //===- NativeSession.cpp - Native implementation of IPDBSession -*- C++ -*-===//
 //
 //                     The LLVM Compiler Infrastructure
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/DebugInfo/PDB/Native/NativeSession.h"
 
+#include <algorithm>
+#include <memory>
+#include <utility>
+
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/DebugInfo/PDB/GenericError.h"
 #include "llvm/DebugInfo/PDB/IPDBEnumChildren.h"
 #include "llvm/DebugInfo/PDB/IPDBSourceFile.h"
+#include "llvm/DebugInfo/PDB/Native/NativeCompilandSymbol.h"
 #include "llvm/DebugInfo/PDB/Native/NativeExeSymbol.h"
 #include "llvm/DebugInfo/PDB/Native/PDBFile.h"
 #include "llvm/DebugInfo/PDB/Native/RawError.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolCompiland.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolExe.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/BinaryByteStream.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/MemoryBuffer.h"
-#include <algorithm>
-#include <memory>
 
 using namespace llvm;
 using namespace llvm::msf;
 using namespace llvm::pdb;
 
 NativeSession::NativeSession(std::unique_ptr<PDBFile> PdbFile,
                              std::unique_ptr<BumpPtrAllocator> Allocator)
     : Pdb(std::move(PdbFile)), Allocator(std::move(Allocator)) {}
Error NativeSession::createFromPdb(StringRef Path,
   return Error::success();
 }
 
 Error NativeSession::createFromExe(StringRef Path,
                                    std::unique_ptr<IPDBSession> &Session) {
   return make_error<RawError>(raw_error_code::feature_unsupported);
 }
 
+std::unique_ptr<PDBSymbolCompiland>
+NativeSession::createCompilandSymbol(DbiModuleDescriptor MI) {
+  const auto Id = static_cast<uint32_t>(SymbolCache.size());
+  SymbolCache.push_back(
+      llvm::make_unique<NativeCompilandSymbol>(*this, Id, MI));
+  return llvm::make_unique<PDBSymbolCompiland>(
+      *this, std::unique_ptr<IPDBRawSymbol>(SymbolCache[Id]->clone()));
+}
+
 uint64_t NativeSession::getLoadAddress() const { return 0; }
 
 void NativeSession::setLoadAddress(uint64_t Address) {}
 
 std::unique_ptr<PDBSymbolExe> NativeSession::getGlobalScope() {
-  auto RawSymbol = llvm::make_unique<NativeExeSymbol>(*this, 0);
+  const auto Id = static_cast<uint32_t>(SymbolCache.size());
+  SymbolCache.push_back(llvm::make_unique<NativeExeSymbol>(*this, Id));
+  auto RawSymbol = SymbolCache[Id]->clone();
   auto PdbSymbol(PDBSymbol::create(*this, std::move(RawSymbol)));
   std::unique_ptr<PDBSymbolExe> ExeSymbol(
       static_cast<PDBSymbolExe *>(PdbSymbol.release()));
   return ExeSymbol;
 }
 
 std::unique_ptr<PDBSymbol>
 NativeSession::getSymbolById(uint32_t SymbolId) const {
-  return nullptr;
+  // If the caller has a SymbolId, it'd better be in our SymbolCache.
+  return SymbolId < SymbolCache.size()
+             ? PDBSymbol::create(*this, SymbolCache[SymbolId]->clone())
+             : nullptr;
 }
 
 std::unique_ptr<PDBSymbol>
 NativeSession::findSymbolByAddress(uint64_t Address, PDB_SymType Type) const {
   return nullptr;
 }
 
 std::unique_ptr<IPDBEnumLineNumbers>