[CodeView] Add a random access type visitor.

This adds a visitor that is capable of accessing type
records randomly and caching intermediate results that it
learns about during partial linear scans.  This yields
amortized O(1) access to a type stream even though type
streams cannot normally be indexed.

Differential Revision: https://reviews.llvm.org/D33009

llvm-svn: 302936

include/llvm/DebugInfo/CodeView/RandomAccessTypeVisitor.h

@@ -0,0 +1,103 @@
+//===- RandomAccessTypeVisitor.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_CODEVIEW_RANDOMACCESSTYPEVISITOR_H
+#define LLVM_DEBUGINFO_CODEVIEW_RANDOMACCESSTYPEVISITOR_H
+
+#include "llvm/ADT/TinyPtrVector.h"
+#include "llvm/DebugInfo/CodeView/CVTypeVisitor.h"
+#include "llvm/DebugInfo/CodeView/TypeDatabase.h"
+#include "llvm/DebugInfo/CodeView/TypeDatabaseVisitor.h"
+#include "llvm/DebugInfo/CodeView/TypeDeserializer.h"
+#include "llvm/DebugInfo/CodeView/TypeIndex.h"
+#include "llvm/DebugInfo/CodeView/TypeRecord.h"
+#include "llvm/DebugInfo/CodeView/TypeVisitorCallbackPipeline.h"
+#include "llvm/Support/Error.h"
+
+namespace llvm {
+namespace codeview {
+
+class TypeDatabase;
+class TypeServerHandler;
+class TypeVisitorCallbacks;
+
+/// \brief Provides amortized O(1) random access to a CodeView type stream.
+/// Normally to access a type from a type stream, you must know its byte
+/// offset into the type stream, because type records are variable-lengthed.
+/// However, this is not the way we prefer to access them.  For example, given
+/// a symbol record one of the fields may be the TypeIndex of the symbol's
+/// type record.  Or given a type record such as an array type, there might
+/// be a TypeIndex for the element type.  Sequential access is perfect when
+/// we're just dumping every entry, but it's very poor for real world usage.
+///
+/// Type streams in PDBs contain an additional field which is a list of pairs
+/// containing indices and their corresponding offsets, roughly every ~8KB of
+/// record data.  This general idea need not be confined to PDBs though.  By
+/// supplying such an array, the producer of a type stream can allow the
+/// consumer much better access time, because the consumer can find the nearest
+/// index in this array, and do a linear scan forward only from there.
+///
+/// RandomAccessTypeVisitor implements this algorithm, but additionally goes one
+/// step further by caching offsets of every record that has been visited at
+/// least once.  This way, even repeated visits of the same record will never
+/// require more than one linear scan.  For a type stream of N elements divided
+/// into M chunks of roughly equal size, this yields a worst case lookup time
+/// of O(N/M) and an amortized time of O(1).
+class RandomAccessTypeVisitor {
+  typedef FixedStreamArray<TypeIndexOffset> PartialOffsetArray;
+
+public:
+  RandomAccessTypeVisitor(const CVTypeArray &Types, uint32_t NumRecords,
+                          PartialOffsetArray PartialOffsets);
+
+  Error visitTypeIndex(TypeIndex Index, TypeVisitorCallbacks &Callbacks);
+
+  const TypeDatabase &database() const { return Database; }
+
+private:
+  Error visitRangeForType(TypeIndex TI);
+  Error visitRange(TypeIndex Begin, uint32_t BeginOffset, TypeIndex End);
+
+  /// Visited records get automatically added to the type database.
+  TypeDatabase Database;
+
+  /// The type array to allow random access visitation of.
+  const CVTypeArray &Types;
+
+  /// The database visitor which adds new records to the database.
+  TypeDatabaseVisitor DatabaseVisitor;
+
+  /// The deserializer which deserializes new records.
+  TypeDeserializer Deserializer;
+
+  /// The visitation callback pipeline to use.  By default this contains a
+  /// deserializer and a type database visitor.  But the callback specified
+  /// in the constructor is also added.
+  TypeVisitorCallbackPipeline Pipeline;
+
+  /// The visitor used to visit the internal pipeline for deserialization and
+  /// database maintenance.
+  CVTypeVisitor InternalVisitor;
+
+  /// A vector mapping type indices to type offset.  For every record that has
+  /// been visited, contains the absolute offset of that record in the record
+  /// array.
+  std::vector<uint32_t> KnownOffsets;
+
+  /// An array of index offsets for the given type stream, allowing log(N)
+  /// lookups of a type record by index.  Similar to KnownOffsets but only
+  /// contains offsets for some type indices, some of which may not have
+  /// ever been visited.
+  PartialOffsetArray PartialOffsets;
+};
+
+} // end namespace codeview
+} // end namespace llvm
+
+#endif // LLVM_DEBUGINFO_CODEVIEW_RANDOMACCESSTYPEVISITOR_H

include/llvm/DebugInfo/CodeView/TypeDatabase.h

@@ -24,13 +24,13 @@ class TypeDatabase {
   friend class RandomAccessTypeVisitor;
 
 public:
-  explicit TypeDatabase(uint32_t ExpectedSize);
-
-  /// Gets the type index for the next type record.
-  TypeIndex getNextTypeIndex() const;
+  explicit TypeDatabase(uint32_t Capacity);
 
   /// Records the name of a type, and reserves its type index.
-  void recordType(StringRef Name, const CVType &Data);
+  TypeIndex appendType(StringRef Name, const CVType &Data);
+
+  /// Records the name of a type, and reserves its type index.
+  void recordType(StringRef Name, TypeIndex Index, const CVType &Data);
 
   /// Saves the name in a StringSet and creates a stable StringRef.
   StringRef saveTypeName(StringRef TypeName);
@@ -40,15 +40,21 @@ public:
   const CVType &getTypeRecord(TypeIndex Index) const;
   CVType &getTypeRecord(TypeIndex Index);
 
-  bool containsTypeIndex(TypeIndex Index) const;
+  bool contains(TypeIndex Index) const;
 
   uint32_t size() const;
+  uint32_t capacity() const;
+  bool empty() const;
 
-protected:
-  uint32_t toArrayIndex(TypeIndex Index) const;
+  TypeIndex getAppendIndex() const;
+
+private:
+  void grow();
 
   BumpPtrAllocator Allocator;
 
+  uint32_t Count = 0;
+
   /// All user defined type records in .debug$T live in here. Type indices
   /// greater than 0x1000 are user defined. Subtract 0x1000 from the index to
   /// index into this vector.
@@ -56,27 +62,7 @@ protected:
   SmallVector<CVType, 10> TypeRecords;
 
   StringSaver TypeNameStorage;
-};
 
-class RandomAccessTypeDatabase : private TypeDatabase {
-public:
-  explicit RandomAccessTypeDatabase(uint32_t ExpectedSize);
-
-  /// Records the name of a type, and reserves its type index.
-  void recordType(StringRef Name, TypeIndex Index, const CVType &Data);
-
-  using TypeDatabase::saveTypeName;
-
-  StringRef getTypeName(TypeIndex Index) const;
-
-  const CVType &getTypeRecord(TypeIndex Index) const;
-  CVType &getTypeRecord(TypeIndex Index);
-
-  bool containsTypeIndex(TypeIndex Index) const;
-
-  uint32_t size() const;
-
-private:
   BitVector ValidRecords;
 };
 }

include/llvm/DebugInfo/CodeView/TypeDatabaseVisitor.h

@@ -24,8 +24,6 @@ namespace codeview {
 class TypeDatabaseVisitor : public TypeVisitorCallbacks {
 public:
   explicit TypeDatabaseVisitor(TypeDatabase &TypeDB) : TypeDB(&TypeDB) {}
-  explicit TypeDatabaseVisitor(RandomAccessTypeDatabase &TypeDB)
-      : TypeDB(&TypeDB) {}
 
   /// Paired begin/end actions for all types. Receives all record data,
   /// including the fixed-length record prefix.
@@ -53,9 +51,9 @@ private:
   StringRef Name;
   /// Current type index.  Only valid before visitTypeEnd, and if we are
   /// visiting a random access type database.
-  TypeIndex CurrentTypeIndex;
+  Optional<TypeIndex> CurrentTypeIndex;
 
-  PointerUnion<TypeDatabase *, RandomAccessTypeDatabase *> TypeDB;
+  TypeDatabase *TypeDB;
 };
 
 } // end namespace codeview

include/llvm/DebugInfo/CodeView/TypeDeserializer.h

@@ -46,6 +46,10 @@ public:
     return Mapping->Mapping.visitTypeBegin(Record);
   }
 
+  Error visitTypeBegin(CVType &Record, TypeIndex Index) override {
+    return visitTypeBegin(Record);
+  }
+
   Error visitTypeEnd(CVType &Record) override {
     assert(Mapping && "Not in a type mapping!");
     auto EC = Mapping->Mapping.visitTypeEnd(Record);

include/llvm/DebugInfo/CodeView/TypeDumpVisitor.h

@@ -45,6 +45,7 @@ public:
   /// Paired begin/end actions for all types. Receives all record data,
   /// including the fixed-length record prefix.
   Error visitTypeBegin(CVType &Record) override;
+  Error visitTypeBegin(CVType &Record, TypeIndex Index) override;
   Error visitTypeEnd(CVType &Record) override;
   Error visitMemberBegin(CVMemberRecord &Record) override;
   Error visitMemberEnd(CVMemberRecord &Record) override;

include/llvm/DebugInfo/CodeView/TypeIndex.h

@@ -242,6 +242,13 @@ private:
   support::ulittle32_t Index;
 };
 
+// Used for pseudo-indexing an array of type records.  An array of such records
+// sorted by TypeIndex can allow log(N) lookups even though such a type record
+// stream does not provide random access.
+struct TypeIndexOffset {
+  TypeIndex Type;
+  support::ulittle32_t Offset;
+};
 }
 }
 

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

@@ -73,13 +73,6 @@ struct SecMapEntry {
   support::ulittle32_t SecByteLength; // Byte count of the segment or group.
 };
 
-// Used for serialized hash table in TPI stream.
-// In the reference, it is an array of TI and cbOff pair.
-struct TypeIndexOffset {
-  codeview::TypeIndex Type;
-  support::ulittle32_t Offset;
-};
-
 /// Some of the values are stored in bitfields.  Since this needs to be portable
 /// across compilers and architectures (big / little endian in particular) we
 /// can't use the actual structures below, but must instead do the shifting

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

@@ -47,7 +47,7 @@ public:
   uint32_t getHashKeySize() const;
   uint32_t getNumHashBuckets() const;
   FixedStreamArray<support::ulittle32_t> getHashValues() const;
-  FixedStreamArray<TypeIndexOffset> getTypeIndexOffsets() const;
+  FixedStreamArray<codeview::TypeIndexOffset> getTypeIndexOffsets() const;
   HashTable &getHashAdjusters();
 
   codeview::CVTypeRange types(bool *HadError) const;
@@ -62,7 +62,7 @@ private:
 
   std::unique_ptr<BinaryStream> HashStream;
   FixedStreamArray<support::ulittle32_t> HashValues;
-  FixedStreamArray<TypeIndexOffset> TypeIndexOffsets;
+  FixedStreamArray<codeview::TypeIndexOffset> TypeIndexOffsets;
   HashTable HashAdjusters;
 
   const TpiStreamHeader *Header;

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

@@ -75,7 +75,7 @@ private:
   Optional<PdbRaw_TpiVer> VerHeader;
   std::vector<ArrayRef<uint8_t>> TypeRecords;
   std::vector<uint32_t> TypeHashes;
-  std::vector<TypeIndexOffset> TypeIndexOffsets;
+  std::vector<codeview::TypeIndexOffset> TypeIndexOffsets;
   uint32_t HashStreamIndex = kInvalidStreamIndex;
   std::unique_ptr<BinaryByteStream> HashValueStream;
 

include/llvm/Support/BinaryStreamArray.h

@@ -139,6 +139,7 @@ public:
   }
 
   uint32_t offset() const { return AbsOffset; }
+  uint32_t getRecordLength() const { return ThisLen; }
 
 private:
   void moveToEnd() {
@@ -294,6 +295,8 @@ template <typename T> class FixedStreamArray {
   friend class FixedStreamArrayIterator<T>;
 
 public:
+  typedef FixedStreamArrayIterator<T> Iterator;
+
   FixedStreamArray() = default;
   explicit FixedStreamArray(BinaryStreamRef Stream) : Stream(Stream) {
     assert(Stream.getLength() % sizeof(T) == 0);
@@ -371,7 +374,7 @@ public:
   }
 
   FixedStreamArrayIterator<T> &operator-=(std::ptrdiff_t N) {
-    assert(Index >= N);
+    assert(std::ptrdiff_t(Index) >= N);
     Index -= N;
     return *this;
   }

lib/DebugInfo/CodeView/CMakeLists.txt

@@ -13,6 +13,7 @@ add_llvm_library(LLVMDebugInfoCodeView
   ModuleDebugFragmentVisitor.cpp
   ModuleDebugInlineeLinesFragment.cpp
   ModuleDebugLineFragment.cpp
+  RandomAccessTypeVisitor.cpp
   RecordSerialization.cpp
   StringTable.cpp
   SymbolRecordMapping.cpp

lib/DebugInfo/CodeView/CVTypeVisitor.cpp

@@ -26,8 +26,7 @@ CVTypeVisitor::CVTypeVisitor(TypeVisitorCallbacks &Callbacks)
     : Callbacks(Callbacks) {}
 
 template <typename T>
-static Error visitKnownRecord(CVTypeVisitor &Visitor, CVType &Record,
-                              TypeVisitorCallbacks &Callbacks) {
+static Error visitKnownRecord(CVType &Record, TypeVisitorCallbacks &Callbacks) {
   TypeRecordKind RK = static_cast<TypeRecordKind>(Record.Type);
   T KnownRecord(RK);
   if (auto EC = Callbacks.visitKnownRecord(Record, KnownRecord))
@@ -107,7 +106,7 @@ Error CVTypeVisitor::finishVisitation(CVType &Record) {
     break;
 #define TYPE_RECORD(EnumName, EnumVal, Name)                                   \
   case EnumName: {                                                             \
-    if (auto EC = visitKnownRecord<Name##Record>(*this, Record, Callbacks))    \
+    if (auto EC = visitKnownRecord<Name##Record>(Record, Callbacks))           \
       return EC;                                                               \
     break;                                                                     \
   }

lib/DebugInfo/CodeView/RandomAccessTypeVisitor.cpp

@@ -0,0 +1,91 @@
+//===- RandomAccessTypeVisitor.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/CodeView/RandomAccessTypeVisitor.h"
+
+#include "llvm/DebugInfo/CodeView/TypeDatabase.h"
+#include "llvm/DebugInfo/CodeView/TypeServerHandler.h"
+#include "llvm/DebugInfo/CodeView/TypeVisitorCallbacks.h"
+
+using namespace llvm;
+using namespace llvm::codeview;
+
+RandomAccessTypeVisitor::RandomAccessTypeVisitor(
+    const CVTypeArray &Types, uint32_t NumRecords,
+    PartialOffsetArray PartialOffsets)
+    : Database(NumRecords), Types(Types), DatabaseVisitor(Database),
+      InternalVisitor(Pipeline), PartialOffsets(PartialOffsets) {
+  Pipeline.addCallbackToPipeline(Deserializer);
+  Pipeline.addCallbackToPipeline(DatabaseVisitor);
+
+  KnownOffsets.resize(Database.capacity());
+}
+
+Error RandomAccessTypeVisitor::visitTypeIndex(TypeIndex TI,
+                                              TypeVisitorCallbacks &Callbacks) {
+  assert(TI.toArrayIndex() < Database.capacity());
+
+  if (!Database.contains(TI)) {
+    if (auto EC = visitRangeForType(TI))
+      return EC;
+  }
+
+  assert(Database.contains(TI));
+  auto &Record = Database.getTypeRecord(TI);
+  CVTypeVisitor V(Callbacks);
+  return V.visitTypeRecord(Record, TI);
+}
+
+Error RandomAccessTypeVisitor::visitRangeForType(TypeIndex TI) {
+  if (PartialOffsets.empty()) {
+    TypeIndex TIB(TypeIndex::FirstNonSimpleIndex);
+    TypeIndex TIE = TIB + Database.capacity();
+    return visitRange(TIB, 0, TIE);
+  }
+
+  auto Next = std::upper_bound(PartialOffsets.begin(), PartialOffsets.end(), TI,
+                               [](TypeIndex Value, const TypeIndexOffset &IO) {
+                                 return Value < IO.Type;
+                               });
+
+  assert(Next != PartialOffsets.begin());
+  auto Prev = std::prev(Next);
+
+  TypeIndex TIB = Prev->Type;
+  TypeIndex TIE;
+  if (Next == PartialOffsets.end()) {
+    TIE = TypeIndex::fromArrayIndex(Database.capacity());
+  } else {
+    TIE = Next->Type;
+  }
+
+  if (auto EC = visitRange(TIB, Prev->Offset, TIE))
+    return EC;
+  return Error::success();
+}
+
+Error RandomAccessTypeVisitor::visitRange(TypeIndex Begin, uint32_t BeginOffset,
+                                          TypeIndex End) {
+
+  auto RI = Types.at(BeginOffset);
+  assert(RI != Types.end());
+
+  while (Begin != End) {
+    assert(!Database.contains(Begin));
+    if (auto EC = InternalVisitor.visitTypeRecord(*RI, Begin))
+      return EC;
+    KnownOffsets[Begin.toArrayIndex()] = BeginOffset;
+
+    BeginOffset += RI.getRecordLength();
+    ++Begin;
+    ++RI;
+  }
+
+  return Error::success();
+}

lib/DebugInfo/CodeView/TypeDatabase.cpp

@@ -65,20 +65,32 @@ static const SimpleTypeEntry SimpleTypeNames[] = {
     {"__bool64*", SimpleTypeKind::Boolean64},
 };
 
-TypeDatabase::TypeDatabase(uint32_t ExpectedSize) : TypeNameStorage(Allocator) {
-  CVUDTNames.reserve(ExpectedSize);
-  TypeRecords.reserve(ExpectedSize);
+TypeDatabase::TypeDatabase(uint32_t Capacity) : TypeNameStorage(Allocator) {
+  CVUDTNames.resize(Capacity);
+  TypeRecords.resize(Capacity);
+  ValidRecords.resize(Capacity);
 }
 
-/// Gets the type index for the next type record.
-TypeIndex TypeDatabase::getNextTypeIndex() const {
-  return TypeIndex(TypeIndex::FirstNonSimpleIndex + CVUDTNames.size());
+TypeIndex TypeDatabase::appendType(StringRef Name, const CVType &Data) {
+  TypeIndex TI;
+  TI = getAppendIndex();
+  if (TI.toArrayIndex() >= capacity())
+    grow();
+  recordType(Name, TI, Data);
+  return TI;
 }
 
-/// Records the name of a type, and reserves its type index.
-void TypeDatabase::recordType(StringRef Name, const CVType &Data) {
-  CVUDTNames.push_back(Name);
-  TypeRecords.push_back(Data);
+void TypeDatabase::recordType(StringRef Name, TypeIndex Index,
+                              const CVType &Data) {
+  uint32_t AI = Index.toArrayIndex();
+
+  assert(!contains(Index));
+  assert(AI < capacity());
+
+  CVUDTNames[AI] = Name;
+  TypeRecords[AI] = Data;
+  ValidRecords.set(AI);
+  ++Count;
 }
 
 /// Saves the name in a StringSet and creates a stable StringRef.
@@ -104,69 +116,47 @@ StringRef TypeDatabase::getTypeName(TypeIndex Index) const {
     return "<unknown simple type>";
   }
 
-  uint32_t I = Index.getIndex() - TypeIndex::FirstNonSimpleIndex;
-  if (I < CVUDTNames.size())
-    return CVUDTNames[I];
+  if (contains(Index))
+    return CVUDTNames[Index.toArrayIndex()];
 
   return "<unknown UDT>";
 }
 
 const CVType &TypeDatabase::getTypeRecord(TypeIndex Index) const {
-  return TypeRecords[toArrayIndex(Index)];
+  assert(contains(Index));
+  return TypeRecords[Index.toArrayIndex()];
 }
 
 CVType &TypeDatabase::getTypeRecord(TypeIndex Index) {
-  return TypeRecords[toArrayIndex(Index)];
+  assert(contains(Index));
+  return TypeRecords[Index.toArrayIndex()];
 }
 
-bool TypeDatabase::containsTypeIndex(TypeIndex Index) const {
-  return toArrayIndex(Index) < CVUDTNames.size();
+bool TypeDatabase::contains(TypeIndex Index) const {
+  uint32_t AI = Index.toArrayIndex();
+  if (AI >= capacity())
+    return false;
+
+  return ValidRecords.test(AI);
 }
 
-uint32_t TypeDatabase::size() const { return CVUDTNames.size(); }
+uint32_t TypeDatabase::size() const { return Count; }
 
-uint32_t TypeDatabase::toArrayIndex(TypeIndex Index) const {
-  assert(Index.getIndex() >= TypeIndex::FirstNonSimpleIndex);
-  return Index.getIndex() - TypeIndex::FirstNonSimpleIndex;
+uint32_t TypeDatabase::capacity() const { return TypeRecords.size(); }
+
+void TypeDatabase::grow() {
+  TypeRecords.emplace_back();
+  CVUDTNames.emplace_back();
+  ValidRecords.resize(ValidRecords.size() + 1);
 }
 
-RandomAccessTypeDatabase::RandomAccessTypeDatabase(uint32_t ExpectedSize)
-    : TypeDatabase(ExpectedSize) {
-  ValidRecords.resize(ExpectedSize);
-  CVUDTNames.resize(ExpectedSize);
-  TypeRecords.resize(ExpectedSize);
+bool TypeDatabase::empty() const { return size() == 0; }
+
+TypeIndex TypeDatabase::getAppendIndex() const {
+  if (empty())
+    return TypeIndex::fromArrayIndex(0);
+
+  int Index = ValidRecords.find_last();
+  assert(Index != -1);
+  return TypeIndex::fromArrayIndex(Index) + 1;
 }
-
-void RandomAccessTypeDatabase::recordType(StringRef Name, TypeIndex Index,
-                                          const CVType &Data) {
-  assert(!containsTypeIndex(Index));
-  uint32_t ZI = Index.getIndex() - TypeIndex::FirstNonSimpleIndex;
-
-  CVUDTNames[ZI] = Name;
-  TypeRecords[ZI] = Data;
-  ValidRecords.set(ZI);
-}
-
-StringRef RandomAccessTypeDatabase::getTypeName(TypeIndex Index) const {
-  assert(containsTypeIndex(Index));
-  return TypeDatabase::getTypeName(Index);
-}
-
-const CVType &RandomAccessTypeDatabase::getTypeRecord(TypeIndex Index) const {
-  assert(containsTypeIndex(Index));
-  return TypeDatabase::getTypeRecord(Index);
-}
-
-CVType &RandomAccessTypeDatabase::getTypeRecord(TypeIndex Index) {
-  assert(containsTypeIndex(Index));
-  return TypeDatabase::getTypeRecord(Index);
-}
-
-bool RandomAccessTypeDatabase::containsTypeIndex(TypeIndex Index) const {
-  if (Index.isSimple())
-    return true;
-
-  return ValidRecords.test(toArrayIndex(Index));
-}
-
-uint32_t RandomAccessTypeDatabase::size() const { return ValidRecords.count(); }

lib/DebugInfo/CodeView/TypeDatabaseVisitor.cpp

@@ -16,8 +16,6 @@ using namespace llvm;
 using namespace llvm::codeview;
 
 Error TypeDatabaseVisitor::visitTypeBegin(CVType &Record) {
-  assert(TypeDB.is<TypeDatabase *>());
-
   assert(!IsInFieldList);
   // Reset Name to the empty string. If the visitor sets it, we know it.
   Name = "";
@@ -30,27 +28,7 @@ Error TypeDatabaseVisitor::visitTypeBegin(CVType &Record) {
   return Error::success();
 }
 
-StringRef TypeDatabaseVisitor::getTypeName(TypeIndex Index) const {
-  if (auto DB = TypeDB.get<TypeDatabase *>())
-    return DB->getTypeName(Index);
-  else if (auto DB = TypeDB.get<RandomAccessTypeDatabase *>())
-    return DB->getTypeName(Index);
-
-  llvm_unreachable("Invalid TypeDB Kind!");
-}
-
-StringRef TypeDatabaseVisitor::saveTypeName(StringRef Name) {
-  if (auto DB = TypeDB.get<TypeDatabase *>())
-    return DB->saveTypeName(Name);
-  else if (auto DB = TypeDB.get<RandomAccessTypeDatabase *>())
-    return DB->saveTypeName(Name);
-
-  llvm_unreachable("Invalid TypeDB Kind!");
-}
-
 Error TypeDatabaseVisitor::visitTypeBegin(CVType &Record, TypeIndex Index) {
-  assert(TypeDB.is<RandomAccessTypeDatabase *>());
-
   if (auto EC = visitTypeBegin(Record))
     return EC;
 
@@ -58,6 +36,14 @@ Error TypeDatabaseVisitor::visitTypeBegin(CVType &Record, TypeIndex Index) {
   return Error::success();
 }
 
+StringRef TypeDatabaseVisitor::getTypeName(TypeIndex Index) const {
+  return TypeDB->getTypeName(Index);
+}
+
+StringRef TypeDatabaseVisitor::saveTypeName(StringRef Name) {
+  return TypeDB->saveTypeName(Name);
+}
+
 Error TypeDatabaseVisitor::visitTypeEnd(CVType &CVR) {
   if (CVR.Type == LF_FIELDLIST) {
     assert(IsInFieldList);
@@ -69,11 +55,12 @@ Error TypeDatabaseVisitor::visitTypeEnd(CVType &CVR) {
   // CVUDTNames is indexed by type index, and must have one entry for every
   // type.  Field list members are not recorded, and are only referenced by
   // their containing field list record.
-  if (auto DB = TypeDB.get<TypeDatabase *>())
-    DB->recordType(Name, CVR);
-  else if (auto DB = TypeDB.get<RandomAccessTypeDatabase *>())
-    DB->recordType(Name, CurrentTypeIndex, CVR);
+  if (CurrentTypeIndex)
+    TypeDB->recordType(Name, *CurrentTypeIndex, CVR);
+  else
+    TypeDB->appendType(Name, CVR);
 
+  CurrentTypeIndex.reset();
   return Error::success();
 }
 

lib/DebugInfo/CodeView/TypeDumpVisitor.cpp

@@ -173,10 +173,13 @@ void TypeDumpVisitor::printItemIndex(StringRef FieldName, TypeIndex TI) const {
 }
 
 Error TypeDumpVisitor::visitTypeBegin(CVType &Record) {
+  TypeIndex TI = getSourceDB().getAppendIndex();
+  return visitTypeBegin(Record, TI);
+}
+
+Error TypeDumpVisitor::visitTypeBegin(CVType &Record, TypeIndex Index) {
   W->startLine() << getLeafTypeName(Record.Type);
-  W->getOStream() << " ("
-                  << HexNumber(getSourceDB().getNextTypeIndex().getIndex())
-                  << ")";
+  W->getOStream() << " (" << HexNumber(Index.getIndex()) << ")";
   W->getOStream() << " {\n";
   W->indent();
   W->printEnum("TypeLeafKind", unsigned(Record.Type),

lib/DebugInfo/PDB/Native/TpiStreamBuilder.cpp

@@ -109,7 +109,7 @@ uint32_t TpiStreamBuilder::calculateHashBufferSize() const {
 }
 
 uint32_t TpiStreamBuilder::calculateIndexOffsetSize() const {
-  return TypeIndexOffsets.size() * sizeof(TypeIndexOffset);
+  return TypeIndexOffsets.size() * sizeof(codeview::TypeIndexOffset);
 }
 
 Error TpiStreamBuilder::finalizeMsfLayout() {

tools/llvm-pdbdump/LLVMOutputStyle.cpp

@@ -180,7 +180,7 @@ private:
     CompactTypeDumpVisitor CTDV(DB, Index, &P);
     CVTypeVisitor Visitor(CTDV);
     DictScope D(P, Label);
-    if (DB.containsTypeIndex(Index)) {
+    if (DB.contains(Index)) {
       CVType &Type = DB.getTypeRecord(Index);
       if (auto EC = Visitor.visitTypeRecord(Type))
         return EC;

unittests/DebugInfo/CMakeLists.txt

@@ -1,3 +1,3 @@
-
+add_subdirectory(CodeView)
 add_subdirectory(DWARF)
 add_subdirectory(PDB)

unittests/DebugInfo/CodeView/CMakeLists.txt

@@ -0,0 +1,11 @@
+set(LLVM_LINK_COMPONENTS
+  DebugInfoCodeView
+  )
+
+set(DebugInfoCodeViewSources
+  RandomAccessVisitorTest.cpp
+  )
+
+add_llvm_unittest(DebugInfoCodeViewTests
+  ${DebugInfoCodeViewSources}
+  )

unittests/DebugInfo/CodeView/ErrorChecking.h

@@ -0,0 +1,61 @@
+//===- ErrorChecking.h - Helpers for verifying llvm::Errors -----*- C++ -*-===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_UNITTESTS_DEBUGINFO_CODEVIEW_ERRORCHECKING_H
+#define LLVM_UNITTESTS_DEBUGINFO_CODEVIEW_ERRORCHECKING_H
+
+#define EXPECT_NO_ERROR(Err)                                                   \
+  {                                                                            \
+    auto E = Err;                                                              \
+    EXPECT_FALSE(static_cast<bool>(E));                                        \
+    if (E)                                                                     \
+      consumeError(std::move(E));                                              \
+  }
+
+#define EXPECT_ERROR(Err)                                                      \
+  {                                                                            \
+    auto E = Err;                                                              \
+    EXPECT_TRUE(static_cast<bool>(E));                                         \
+    if (E)                                                                     \
+      consumeError(std::move(E));                                              \
+  }
+
+#define EXPECT_EXPECTED(Exp)                                                   \
+  {                                                                            \
+    auto E = Exp.takeError();                                                  \
+    EXPECT_FALSE(static_cast<bool>(E));                                        \
+    if (E) {                                                                   \
+      consumeError(std::move(E));                                              \
+      return;                                                                  \
+    }                                                                          \
+  }
+
+#define EXPECT_EXPECTED_EQ(Val, Exp)                                           \
+  {                                                                            \
+    auto Result = Exp;                                                         \
+    auto E = Result.takeError();                                               \
+    EXPECT_FALSE(static_cast<bool>(E));                                        \
+    if (E) {                                                                   \
+      consumeError(std::move(E));                                              \
+      return;                                                                  \
+    }                                                                          \
+    EXPECT_EQ(Val, *Result);                                                   \
+  }
+
+#define EXPECT_UNEXPECTED(Exp)                                                 \
+  {                                                                            \
+    auto E = Exp.takeError();                                                  \
+    EXPECT_TRUE(static_cast<bool>(E));                                         \
+    if (E) {                                                                   \
+      consumeError(std::move(E));                                              \
+      return;                                                                  \
+    }                                                                          \
+  }
+
+#endif

unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp

@@ -0,0 +1,353 @@
+//===- llvm/unittest/DebugInfo/CodeView/RandomAccessVisitorTest.cpp -------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+#include "ErrorChecking.h"
+
+#include "llvm/ADT/SmallBitVector.h"
+#include "llvm/DebugInfo/CodeView/CVTypeVisitor.h"
+#include "llvm/DebugInfo/CodeView/RandomAccessTypeVisitor.h"
+#include "llvm/DebugInfo/CodeView/TypeRecord.h"
+#include "llvm/DebugInfo/CodeView/TypeRecordMapping.h"
+#include "llvm/DebugInfo/CodeView/TypeSerializer.h"
+#include "llvm/DebugInfo/CodeView/TypeServerHandler.h"
+#include "llvm/DebugInfo/CodeView/TypeTableBuilder.h"
+#include "llvm/DebugInfo/CodeView/TypeVisitorCallbackPipeline.h"
+#include "llvm/DebugInfo/CodeView/TypeVisitorCallbacks.h"
+#include "llvm/DebugInfo/PDB/Native/RawTypes.h"
+#include "llvm/Support/Allocator.h"
+#include "llvm/Support/BinaryItemStream.h"
+#include "llvm/Support/Error.h"
+
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace llvm::codeview;
+using namespace llvm::pdb;
+
+namespace llvm {
+namespace codeview {
+inline bool operator==(const ArrayRecord &R1, const ArrayRecord &R2) {
+  if (R1.ElementType != R2.ElementType)
+    return false;
+  if (R1.IndexType != R2.IndexType)
+    return false;
+  if (R1.Name != R2.Name)
+    return false;
+  if (R1.Size != R2.Size)
+    return false;
+  return true;
+}
+inline bool operator!=(const ArrayRecord &R1, const ArrayRecord &R2) {
+  return !(R1 == R2);
+}
+
+inline bool operator==(const CVType &R1, const CVType &R2) {
+  if (R1.Type != R2.Type)
+    return false;
+  if (R1.RecordData != R2.RecordData)
+    return false;
+  return true;
+}
+inline bool operator!=(const CVType &R1, const CVType &R2) {
+  return !(R1 == R2);
+}
+}
+}
+
+namespace llvm {
+template <> struct BinaryItemTraits<CVType> {
+  static size_t length(const CVType &Item) { return Item.length(); }
+  static ArrayRef<uint8_t> bytes(const CVType &Item) { return Item.data(); }
+};
+}
+
+namespace {
+
+class MockCallbacks : public TypeVisitorCallbacks {
+public:
+  virtual Error visitTypeBegin(CVType &CVR, TypeIndex Index) {
+    Indices.push_back(Index);
+    return Error::success();
+  }
+  virtual Error visitKnownRecord(CVType &CVR, ArrayRecord &AR) {
+    VisitedRecords.push_back(AR);
+    RawRecords.push_back(CVR);
+    return Error::success();
+  }
+
+  uint32_t count() const {
+    assert(Indices.size() == RawRecords.size());
+    assert(Indices.size() == VisitedRecords.size());
+    return Indices.size();
+  }
+  std::vector<TypeIndex> Indices;
+  std::vector<CVType> RawRecords;
+  std::vector<ArrayRecord> VisitedRecords;
+};
+
+class RandomAccessVisitorTest : public testing::Test {
+public:
+  RandomAccessVisitorTest() {}
+
+  static void SetUpTestCase() {
+    GlobalState = llvm::make_unique<GlobalTestState>();
+
+    TypeTableBuilder Builder(GlobalState->Allocator);
+
+    uint32_t Offset = 0;
+    for (int I = 0; I < 11; ++I) {
+      ArrayRecord AR(TypeRecordKind::Array);
+      AR.ElementType = TypeIndex::Int32();
+      AR.IndexType = TypeIndex::UInt32();
+      AR.Size = I;
+      std::string Name;
+      raw_string_ostream Stream(Name);
+      Stream << "Array [" << I << "]";
+      AR.Name = GlobalState->Strings.save(Stream.str());
+      GlobalState->Records.push_back(AR);
+      GlobalState->Indices.push_back(Builder.writeKnownType(AR));
+
+      CVType Type(TypeLeafKind::LF_ARRAY, Builder.records().back());
+      GlobalState->TypeVector.push_back(Type);
+
+      GlobalState->AllOffsets.push_back(
+          {GlobalState->Indices.back(), ulittle32_t(Offset)});
+      Offset += Type.length();
+    }
+
+    GlobalState->ItemStream.setItems(GlobalState->TypeVector);
+    GlobalState->TypeArray = VarStreamArray<CVType>(GlobalState->ItemStream);
+  }
+
+  static void TearDownTestCase() { GlobalState.reset(); }
+
+  void SetUp() override {
+    TestState = llvm::make_unique<PerTestState>();
+
+    TestState->Pipeline.addCallbackToPipeline(TestState->Deserializer);
+    TestState->Pipeline.addCallbackToPipeline(TestState->Callbacks);
+  }
+
+  void TearDown() override { TestState.reset(); }
+
+protected:
+  bool ValidateDatabaseRecord(const RandomAccessTypeVisitor &Visitor,
+                              uint32_t Index) {
+    TypeIndex TI = TypeIndex::fromArrayIndex(Index);
+    if (!Visitor.database().contains(TI))
+      return false;
+    if (GlobalState->TypeVector[Index] != Visitor.database().getTypeRecord(TI))
+      return false;
+    return true;
+  }
+
+  bool ValidateVisitedRecord(uint32_t VisitationOrder,
+                             uint32_t GlobalArrayIndex) {
+    TypeIndex TI = TypeIndex::fromArrayIndex(GlobalArrayIndex);
+    if (TI != TestState->Callbacks.Indices[VisitationOrder])
+      return false;
+
+    if (GlobalState->TypeVector[TI.toArrayIndex()] !=
+        TestState->Callbacks.RawRecords[VisitationOrder])
+      return false;
+
+    if (GlobalState->Records[TI.toArrayIndex()] !=
+        TestState->Callbacks.VisitedRecords[VisitationOrder])
+      return false;
+
+    return true;
+  }
+
+  struct GlobalTestState {
+    GlobalTestState() : Strings(Allocator), ItemStream(llvm::support::little) {}
+
+    BumpPtrAllocator Allocator;
+    StringSaver Strings;
+
+    std::vector<ArrayRecord> Records;
+    std::vector<TypeIndex> Indices;
+    std::vector<TypeIndexOffset> AllOffsets;
+    std::vector<CVType> TypeVector;
+    BinaryItemStream<CVType> ItemStream;
+    VarStreamArray<CVType> TypeArray;
+
+    MutableBinaryByteStream Stream;
+  };
+
+  struct PerTestState {
+    FixedStreamArray<TypeIndexOffset> Offsets;
+
+    TypeVisitorCallbackPipeline Pipeline;
+    TypeDeserializer Deserializer;
+    MockCallbacks Callbacks;
+  };
+
+  FixedStreamArray<TypeIndexOffset>
+  createPartialOffsets(MutableBinaryByteStream &Storage,
+                       std::initializer_list<uint32_t> Indices) {
+
+    uint32_t Count = Indices.size();
+    uint32_t Size = Count * sizeof(TypeIndexOffset);
+    uint8_t *Buffer = GlobalState->Allocator.Allocate<uint8_t>(Size);
+    MutableArrayRef<uint8_t> Bytes(Buffer, Size);
+    Storage = MutableBinaryByteStream(Bytes, support::little);
+    BinaryStreamWriter Writer(Storage);
+    for (const auto I : Indices)
+      consumeError(Writer.writeObject(GlobalState->AllOffsets[I]));
+
+    BinaryStreamReader Reader(Storage);
+    FixedStreamArray<TypeIndexOffset> Result;
+    consumeError(Reader.readArray(Result, Count));
+    return Result;
+  }
+
+  static std::unique_ptr<GlobalTestState> GlobalState;
+  std::unique_ptr<PerTestState> TestState;
+};
+
+std::unique_ptr<RandomAccessVisitorTest::GlobalTestState>
+    RandomAccessVisitorTest::GlobalState;
+}
+
+TEST_F(RandomAccessVisitorTest, MultipleVisits) {
+  TestState->Offsets = createPartialOffsets(GlobalState->Stream, {0, 8});
+  RandomAccessTypeVisitor Visitor(GlobalState->TypeArray,
+                                  GlobalState->TypeVector.size(),
+                                  TestState->Offsets);
+
+  std::vector<uint32_t> IndicesToVisit = {5, 5, 5};
+
+  for (uint32_t I : IndicesToVisit) {
+    TypeIndex TI = TypeIndex::fromArrayIndex(I);
+    EXPECT_NO_ERROR(Visitor.visitTypeIndex(TI, TestState->Pipeline));
+  }
+
+  // [0,8) should be present
+  EXPECT_EQ(8, Visitor.database().size());
+  for (uint32_t I = 0; I < 8; ++I)
+    EXPECT_TRUE(ValidateDatabaseRecord(Visitor, I));
+
+  // 5, 5, 5
+  EXPECT_EQ(3, TestState->Callbacks.count());
+  for (auto I : enumerate(IndicesToVisit))
+    EXPECT_TRUE(ValidateVisitedRecord(I.index(), I.value()));
+}
+
+TEST_F(RandomAccessVisitorTest, DescendingWithinChunk) {
+  // Visit multiple items from the same "chunk" in reverse order.  In this
+  // example, it's 7 then 4 then 2.  At the end, all records from 0 to 7 should
+  // be known by the database, but only 2, 4, and 7 should have been visited.
+  TestState->Offsets = createPartialOffsets(GlobalState->Stream, {0, 8});
+
+  std::vector<uint32_t> IndicesToVisit = {7, 4, 2};
+
+  RandomAccessTypeVisitor Visitor(GlobalState->TypeArray,
+                                  GlobalState->TypeVector.size(),
+                                  TestState->Offsets);
+
+  for (uint32_t I : IndicesToVisit) {
+    TypeIndex TI = TypeIndex::fromArrayIndex(I);
+    EXPECT_NO_ERROR(Visitor.visitTypeIndex(TI, TestState->Pipeline));
+  }
+
+  // [0, 7]
+  EXPECT_EQ(8, Visitor.database().size());
+  for (uint32_t I = 0; I < 8; ++I)
+    EXPECT_TRUE(ValidateDatabaseRecord(Visitor, I));
+
+  // 2, 4, 7
+  EXPECT_EQ(3, TestState->Callbacks.count());
+  for (auto I : enumerate(IndicesToVisit))
+    EXPECT_TRUE(ValidateVisitedRecord(I.index(), I.value()));
+}
+
+TEST_F(RandomAccessVisitorTest, AscendingWithinChunk) {
+  // * Visit multiple items from the same chunk in ascending order, ensuring
+  //   that intermediate items are not visited.  In the below example, it's
+  //   5 -> 6 -> 7 which come from the [4,8) chunk.
+  TestState->Offsets = createPartialOffsets(GlobalState->Stream, {0, 8});
+
+  std::vector<uint32_t> IndicesToVisit = {2, 4, 7};
+
+  RandomAccessTypeVisitor Visitor(GlobalState->TypeArray,
+                                  GlobalState->TypeVector.size(),
+                                  TestState->Offsets);
+
+  for (uint32_t I : IndicesToVisit) {
+    TypeIndex TI = TypeIndex::fromArrayIndex(I);
+    EXPECT_NO_ERROR(Visitor.visitTypeIndex(TI, TestState->Pipeline));
+  }
+
+  // [0, 7]
+  EXPECT_EQ(8, Visitor.database().size());
+  for (uint32_t I = 0; I < 8; ++I)
+    EXPECT_TRUE(ValidateDatabaseRecord(Visitor, I));
+
+  // 2, 4, 7
+  EXPECT_EQ(3, TestState->Callbacks.count());
+  for (auto &I : enumerate(IndicesToVisit))
+    EXPECT_TRUE(ValidateVisitedRecord(I.index(), I.value()));
+}
+
+TEST_F(RandomAccessVisitorTest, StopPrematurelyInChunk) {
+  // * Don't visit the last item in one chunk, ensuring that visitation stops
+  //   at the record you specify, and the chunk is only partially visited.
+  //   In the below example, this is tested by visiting 0 and 1 but not 2,
+  //   all from the [0,3) chunk.
+  TestState->Offsets = createPartialOffsets(GlobalState->Stream, {0, 8});
+
+  std::vector<uint32_t> IndicesToVisit = {0, 1, 2};
+
+  RandomAccessTypeVisitor Visitor(GlobalState->TypeArray,
+                                  GlobalState->TypeVector.size(),
+                                  TestState->Offsets);
+
+  for (uint32_t I : IndicesToVisit) {
+    TypeIndex TI = TypeIndex::fromArrayIndex(I);
+    EXPECT_NO_ERROR(Visitor.visitTypeIndex(TI, TestState->Pipeline));
+  }
+
+  // [0, 8) should be visited.
+  EXPECT_EQ(8, Visitor.database().size());
+  for (uint32_t I = 0; I < 8; ++I)
+    EXPECT_TRUE(ValidateDatabaseRecord(Visitor, I));
+
+  // [0, 2]
+  EXPECT_EQ(3, TestState->Callbacks.count());
+  for (auto I : enumerate(IndicesToVisit))
+    EXPECT_TRUE(ValidateVisitedRecord(I.index(), I.value()));
+}
+
+TEST_F(RandomAccessVisitorTest, InnerChunk) {
+  // Test that when a request comes from a chunk in the middle of the partial
+  // offsets array, that items from surrounding chunks are not visited or
+  // added to the database.
+  TestState->Offsets = createPartialOffsets(GlobalState->Stream, {0, 4, 9});
+
+  std::vector<uint32_t> IndicesToVisit = {5, 7};
+
+  RandomAccessTypeVisitor Visitor(GlobalState->TypeArray,
+                                  GlobalState->TypeVector.size(),
+                                  TestState->Offsets);
+
+  for (uint32_t I : IndicesToVisit) {
+    TypeIndex TI = TypeIndex::fromArrayIndex(I);
+    EXPECT_NO_ERROR(Visitor.visitTypeIndex(TI, TestState->Pipeline));
+  }
+
+  // [4, 9)
+  EXPECT_EQ(5, Visitor.database().size());
+  for (uint32_t I = 4; I < 9; ++I)
+    EXPECT_TRUE(ValidateDatabaseRecord(Visitor, I));
+
+  // 5, 7
+  EXPECT_EQ(2, TestState->Callbacks.count());
+  for (auto &I : enumerate(IndicesToVisit))
+    EXPECT_TRUE(ValidateVisitedRecord(I.index(), I.value()));
+}