[PDB] Enable NativeSession to create symbols for built-in types on demand

[amccarth]

Summary

There is a reserved range of type indexes for built-in types (like integers).
This will create a symbol for a built-in type if the caller askes for one by
type index. This is also plumbing for being able to recall symbols by type
index in general, but user-defined types will come in subsequent patches.

[rnk]

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

42

Any reason not to use codeview::TypeIndex for this parameter? Just trying to avoid exposing internal libraries out through this interface?

86

Generally, LLVM uses bare uint32_t rather than std::uint32_t. You could argue it's C legacy, but that's currently the norm, so I'd rather stick with it.

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

44

It seems unlikely that a builtin type will grow beyond 4GB.

[zturner]

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

13

Can you use llvm::DenseMap instead (and delete this #include)

42

+1

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

107–108

I can think of a couple of issues with this function.

1. I might be misunderstanding the purpose of findSymbolByTypeIndex, but doesn't TypeIndex imply that this function *only* works for types? What about symbols, which don't have a type index? Would those get a different function?

2. What about id types (e.g. types from the IPI string instead of the TPI stream)? Would we eventually add a findSymbolByIdIndex too?

3. Why even have a map? Why not just have a std::vector<std::unique_ptr<NativeRawSymbol>> Types; and then in the constructor you resize it to Tpi.getNumTypeRecords(), and in this function if it's simple you create one on the fly, otherwise you access Types[TI.toArrayIndex()], and if it's nullptr you create a new one otherwise you return the existing one? This would be O(1) instead of doing the hash computation.

[zturner]

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

107–108

Alternatively, maybe we don't even need a cache? LazyRandomTypeCollection already has O(1) amortized random access, as it is effectively already the cache. So maybe you can create everything on the fly, and not even have a cache?

[zturner]

Ok I think I get it. There's a distinction between "sym index id" (DIA terminology) and "type index" (codeview terminology) that I think is getting lost here. Can we make a typedef for SymIndexId (call it something else if you need to, since that would conflict with DIA typedefs. Like PDB_SymId. Then have this function return a PDB_SymId and accept a codeview::TypeIndex?

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

114–115

Can you add a fixme to handle this? Simple Types include things like pointers to other simple types, which we'll need to handle.

[amccarth]

I've got one more suggestion from Zach to take care of, then I'll upload an updated patch.

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

42

I tried TypeIndex originally, but there were two issues:

1. Users of the API generally get the type indexes from symbol methods like getTypeId, which (like DIA), return a uint32_t. Using a TypeIndex seemed to introduce conversions without much value
2. There's no standard hash function for a TypeIndex, and defining one seemed gratuitous.

I've gone ahead and switched back to TypeIndex.

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

44

Sure, but, again, this was a matter of conforming to the types used by the already-defined API. The getLength() method for a symbol returns a uint64_t, so I was trying to avoid unnecessary conversions. I've switched to uint32_t.

107–108

1. No, this is specifically for looking up a type symbol by its type index, not the unique symbol index that the API hands out. The motivation here is, for example, a record for an enum has a method called getUnderlyingType which returns a TypeIndex, not a unique symbol index, so we need to be able to find a type symbol by its type index.

2. I'm still fuzzy on what the IPI stream contains versus the TPI. What is an id type?

3. The symbol cache, which is indexed by symbol IDs is just a vector.

107–108

Re: LazyRandomTypeCollection. Yes, I think that makes sense to do this for the user-defined types, so that seems like it would replace the need for caching those. But findSymbolByTypeIndex needs to work for both user-defined types and built-in types.

The only trick is that the cached objects are stamped with the symbol's unique ID, so somewhere I'm going to have to keep track of those so that re-visiting a type gets the original ID.

[amccarth]

Addressed feedback from zturner and rnk.

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

13

Sure, but the TypeIndexToSymbolId map won't be dense in most cases. There will likely be a relatively small number of entries with scattered key values. What (where?) are the guidelines for when to use DenseMap?

[amccarth]

Switched from std::unordered_map to llvm::DenseMap per zturner's suggestion.

[rnk]

lgtm

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

+//===- NativeBuiltinSymbol.h -------------------------------------- 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_NATIVE_NATIVEBUILTINSYMBOL_H
+#define LLVM_DEBUGINFO_PDB_NATIVE_NATIVEBUILTINSYMBOL_H
+
+#include "llvm/DebugInfo/PDB/Native/NativeRawSymbol.h"
+
+#include "llvm/DebugInfo/PDB/PDBTypes.h"
+
+namespace llvm {
+namespace pdb {
+
+class NativeSession;
+
+class NativeBuiltinSymbol : public NativeRawSymbol {
+public:
+  NativeBuiltinSymbol(NativeSession &PDBSession, uint32_t Id, PDB_BuiltinType T,
+                      uint64_t L);
+  ~NativeBuiltinSymbol() override;
+
+  virtual std::unique_ptr<NativeRawSymbol> clone() const override;
+
+  void dump(raw_ostream &OS, int Indent) const override;
+
+  PDB_SymType NativeBuiltinSymbol::getSymTag() const override;
+
+  PDB_BuiltinType getBuiltinType() const override;
+  bool isConstType() const override;
+  uint64_t getLength() const override;
+  bool isUnalignedType() const override;
+  bool isVolatileType() const override;
+
+protected:
+  NativeSession &Session;
+  PDB_BuiltinType Type;
+  uint64_t Length;
+};
+
+} // namespace pdb
+} // namespace llvm
+
+#endif

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_NATIVE_NATIVESESSION_H
 #define LLVM_DEBUGINFO_PDB_NATIVE_NATIVESESSION_H
 
+#include <unordered_map>
+
 #include "llvm/ADT/StringRef.h"
+#include "llvm/DebugInfo/PDB/IPDBRawSymbol.h"
 #include "llvm/DebugInfo/PDB/IPDBSession.h"
 #include "llvm/DebugInfo/PDB/Native/DbiModuleDescriptor.h"
+#include "llvm/DebugInfo/PDB/Native/NativeBuiltinSymbol.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);
 
+  uint32_t findSymbolByTypeIndex(uint32_t TI);
+
   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:
 
   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;
+  std::unordered_map<std::uint32_t, std::uint32_t> TypeIndexToSymbolId;
 };
 }
 }
 
 #endif

llvm/lib/DebugInfo/PDB/CMakeLists.txt

add_pdb_impl_folder(Native
   Native/EnumTables.cpp
   Native/GlobalsStream.cpp
   Native/GSI.cpp
   Native/Hash.cpp
   Native/HashTable.cpp
   Native/InfoStream.cpp
   Native/InfoStreamBuilder.cpp
   Native/ModuleDebugStream.cpp
+  Native/NativeBuiltinSymbol.cpp
   Native/NativeCompilandSymbol.cpp
   Native/NativeEnumModules.cpp
   Native/NativeExeSymbol.cpp
   Native/NativeRawSymbol.cpp
   Native/NamedStreamMap.cpp
   Native/NativeSession.cpp
   Native/PDBFile.cpp
   Native/PDBFileBuilder.cpp

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

+//===- NativeBuiltinSymbol.cpp ------------------------------------ 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/NativeBuiltinSymbol.h"
+
+#include "llvm/DebugInfo/PDB/Native/NativeSession.h"
+
+namespace llvm {
+namespace pdb {
+
+NativeBuiltinSymbol::NativeBuiltinSymbol(NativeSession &PDBSession, uint32_t Id,
+                                         PDB_BuiltinType T, uint64_t L)
+    : NativeRawSymbol(PDBSession, Id), Session(PDBSession), Type(T), Length(L) {
+}
+
+NativeBuiltinSymbol::~NativeBuiltinSymbol() {}
+
+std::unique_ptr<NativeRawSymbol> NativeBuiltinSymbol::clone() const {
+  return std::make_unique<NativeBuiltinSymbol>(Session, SymbolId, Type, Length);
+}
+
+void NativeBuiltinSymbol::dump(raw_ostream &OS, int Indent) const {
+  // TODO:  Apparently nothing needs this yet.
+}
+
+PDB_SymType NativeBuiltinSymbol::getSymTag() const {
+  return PDB_SymType::BuiltinType;
+}
+
+PDB_BuiltinType NativeBuiltinSymbol::getBuiltinType() const { return Type; }
+
+bool NativeBuiltinSymbol::isConstType() const { return false; }
+
+uint64_t NativeBuiltinSymbol::getLength() const { return Length; }
+
+bool NativeBuiltinSymbol::isUnalignedType() const { return false; }
+
+bool NativeBuiltinSymbol::isVolatileType() const { return false; }
+
+} // namespace pdb
+} // namespace llvm

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 "llvm/ADT/STLExtras.h"
+#include "llvm/DebugInfo/CodeView/TypeIndex.h"
 #include "llvm/DebugInfo/PDB/GenericError.h"
 #include "llvm/DebugInfo/PDB/IPDBEnumChildren.h"
 #include "llvm/DebugInfo/PDB/IPDBSourceFile.h"
+#include "llvm/DebugInfo/PDB/Native/NativeBuiltinSymbol.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>
 #include <utility>
 
 using namespace llvm;
 using namespace llvm::msf;
 using namespace llvm::pdb;
 
+namespace {
+// Maps codeview::SimpleTypeKind of a built-in type to the parameters necessary
+// to instantiate a NativeBuiltinSymbol for that type.
+static const struct {
+  codeview::SimpleTypeKind Kind;
+  PDB_BuiltinType Type;
+  uint64_t Size;
+} BuiltinTypes[] = {
+    {codeview::SimpleTypeKind::Int32, PDB_BuiltinType::Int, 4},
+    {codeview::SimpleTypeKind::UInt32, PDB_BuiltinType::UInt, 4},
+    {codeview::SimpleTypeKind::UInt32Long, PDB_BuiltinType::UInt, 4},
+    {codeview::SimpleTypeKind::UInt64Quad, PDB_BuiltinType::UInt, 8},
+    {codeview::SimpleTypeKind::NarrowCharacter, PDB_BuiltinType::Char, 1},
+    {codeview::SimpleTypeKind::SignedCharacter, PDB_BuiltinType::Char, 1},
+    {codeview::SimpleTypeKind::UnsignedCharacter, PDB_BuiltinType::UInt, 1},
+    {codeview::SimpleTypeKind::UInt16Short, PDB_BuiltinType::UInt, 2},
+    {codeview::SimpleTypeKind::Boolean8, PDB_BuiltinType::Bool, 1}
+    // This table can be grown as necessary, but these are the only types we've
+    // needed so far.
+};
+} // namespace
+
 NativeSession::NativeSession(std::unique_ptr<PDBFile> PdbFile,
                              std::unique_ptr<BumpPtrAllocator> Allocator)
     : Pdb(std::move(PdbFile)), Allocator(std::move(Allocator)) {}
 
 NativeSession::~NativeSession() = default;
 
 Error NativeSession::createFromPdb(StringRef Path,
                                    std::unique_ptr<IPDBSession> &Session) {

 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()));
 }
 
+uint32_t NativeSession::findSymbolByTypeIndex(uint32_t Index) {
+  // First see if it's already in our cache.
+  const auto Entry = TypeIndexToSymbolId.find(Index);
+  if (Entry != TypeIndexToSymbolId.end())
+    return Entry->second;
+
+  // Symbols for built-in types are created on the fly.
+  if (Index < codeview::TypeIndex::FirstNonSimpleIndex) {
+    const codeview::TypeIndex TI(Index);
+    if (TI.getSimpleMode() != codeview::SimpleTypeMode::Direct)
+      return 0;
+    const auto Kind = TI.getSimpleKind();
+    const auto It = std::find_if(
+        std::begin(BuiltinTypes), std::end(BuiltinTypes),
+        [Kind](const auto &Builtin) { return Builtin.Kind == Kind; });
+    if (It == std::end(BuiltinTypes))
+      return 0;
+    uint32_t Id = SymbolCache.size();
+    SymbolCache.emplace_back(
+        std::make_unique<NativeBuiltinSymbol>(*this, Id, It->Type, It->Size));
+    TypeIndexToSymbolId[Index] = Id;
+    return Id;
+  }
+
+  // TODO:  Look up PDB type by type index
+
+  return 0;
+}
+
 uint64_t NativeSession::getLoadAddress() const { return 0; }
 
 void NativeSession::setLoadAddress(uint64_t Address) {}
 
 std::unique_ptr<PDBSymbolExe> NativeSession::getGlobalScope() {
   const auto Id = static_cast<uint32_t>(SymbolCache.size());
   SymbolCache.push_back(llvm::make_unique<NativeExeSymbol>(*this, Id));
   auto RawSymbol = SymbolCache[Id]->clone();