Enable llvm-pdbutil to list enumerations using native PDB reader

[amccarth]

Summary

This extends the native reader to enable llvm-pdbutil to list the enums in a
PDB and it includes a simple test. It does not yet list the values in the
enumerations, which requires an actual implementation of
NativeEnumSymbol::FindChildren.

To exercise this code, use a command like:

llvm-pdbutil pretty -native -enums foo.pdb

[zturner]

Is it that much extra work to implement children? It would be nice if this could just be feature complete. It seems like all you have to do is read the FieldList member of the CodeView record, load that item, and then create an enumerator that iterates each item and returns a NativeConstantSymbol or whatever we're calling it

[rnk]

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

22

I'm wondering if we can get away with implementing these findChildren methods by building a vector of PDBSymbols up front, and then this class becomes a simple adapter from vector to IPDBEnumChildren. It seems to me if the caller calls findChildren, they're going to walk them all, so we can do O(n) work without slowing things down asymptotically.

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

48–49

This check will never fail. The optional HadError parameter is essentially an awkward way to thread error handling out of a range-based-for. The idea is that if the iterator encounters an error, it halts iteration and sets *HadError to true. The caller can check for failure outside the loop. We don't want to eagerly check for errors because that would require iterating over all the data.

Does this method get called often or early? I'm wondering if it would be better to iterate and filter the type stream up front and make a vector.

[amccarth]

In D35738#817991, @zturner wrote:

Is it that much extra work to implement children? It would be nice if this could just be feature complete. It seems like all you have to do is read the FieldList member of the CodeView record, load that item, and then create an enumerator that iterates each item and returns a NativeConstantSymbol or whatever we're calling it

I think I see how to do this, but I'd feel better doing it as a subsequent patch. I'd need to make the NativeSession be the owner of the Tpi stream.

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

22

Am I missing something? I think this is still O(n), as calls to getNext continue through the types to the next one that matches. It may be a small speedup in cases where the caller is planning to iterate over the entire range. But it's overkill if the caller isn't going to iterate the entire list. And I don't think it changes the complexity.

And a vector would require more memory, which may or may not be a big deal.

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

48–49

OK, I get what you've said about the error, and I can remove the unnecessary check.

In a dumper, this method is called once for the enums, so, no, it's not called frequently. In a random-access scenario, I'd expect we'd be following TypeIndexes rather than asking the Exe scope for all of the enums it contains.

[zturner]

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

39–64

Can we get rid of the ifdefs?

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

23–24

How about cantFail(visitTypeRecord(CV, *this));

61

Remove ifdefs?

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

27–29

This is incorrect. getChildCount() is supposed to return the number of values that the enumerator is going to enumerate. The value being returned here doesn't account for the fact that not everything is an enum. Also, I'm pretty sure we use this method in llvm-pdbutil pretty. In any case, it seems pretty important to me.

34

Not crazy about leaving this unsupported, but if we're going to do it, at the very least it needs a llvm_unreachable.

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

47

If you call Tpi->typeCollection() instead of Tpi->types(), then you can store a LazyRandomTypeCollection in the NativeEnumTypes, which should be more efficient. This also solves the problem of making the session own the TpiStream, and should enable you to easily create an enumerator for the values of the enum.

In fact, I'd like to eventually delete the types() method now that we have the random access type collection.

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

109

Should we be asserting that I.Kind == LF_ENUM?

[amccarth]

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

27–29

Yeah, the "doesn't really return what we want" in the comment was intended to capture that this isn't right.

You and I had a discussion a while back about removing this functionality because it requires enumerating all the types an extra time just to count them, for rather minimal value. But we didn't really reach a conclusion.

As far as I can tell, pretty is the only user, and it seems like it adds only minimal value. So my preference would be to remove the getChildCount method from the enumerators altogether.

34

This is related to the previous comment.

On the one hand, the interface defined in IPDBEnumChildren suggests it's a simple forward iterator (getNext, reset). On the other hand, it's an indexed collection (getChildCount, getChildAtIndex). It kind of violates the single responsibility principle.

We could enumerate everything once, essentially building a container, which would let us implement all the methods efficiently at the cost of more memory and moving a bunch of the cost to object creation time.

It isn't a big deal for a dumper that's going through everything once, but it's not hard to imagine other users that would prefer the slim iterator idea.

[amccarth]

I'm reworking NativeEnumTypes per the discussion. I'll have an update on Monday.

[amccarth]

Enable llvm-pdbutil to list enumerations using native PDB reader

Per suggestions, changed NativeEnumTypes to be both a container and an
iterator, which allows correct implementation of counting, etc.

Implemented most of the NativeEnumSymbol methods that had been #if'ed out. I
stopped overriding the rest of them, as the default handling of the
NativeRawSymbol base class is appropriate.

Still planning to list the actual members of the numeration in a separate
patch.

[amccarth]

I'll ping when it's ready for another look.

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

47

Working on this.

[amccarth]

Simplified NativeEnumTypes by moving work to NativeSession which already had
access to Tpi and thus LazyRandomTypeCollection. NativeEnumTypes can now
exactly answer how many objects is knows about because it scans the types
when its constructed and maintains a vector of the relevant TypeIndexes.

[amccarth]

PTAL.

[zturner]

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

18–19

Can you do

using namespace llvm;
using namespace llvm::pdb;

instead of putting the namespace inline? That's the preferred approach for LLVM style.

22

Can you choose a better name than fooCV? :)

25

should we assert that fooCV.type() == TypeLeafKind::LV_ENUM? Ideally it would be nice if this constructor were private and there was a single method that would create a Native symbol from a CVType that would create the right concrete type. But we can save that for later.

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

110

early return?

115

It looks like this will create a new instance every time someone calls this method? I thought it was going to do a scan up front and cache everything, and then subsequent calls would just return directly from the cache?

[amccarth]

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

18–19

Um. OK, but that seems an odd style in general, since it would be prone to name collisions.

25

Sure.

Now they are constructed only by NativeSession, which has such an assertion. I guess all these NativeXXXSymbol classes should have private constructors, and the NativeSession could be a friend to them all.

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

115

The NativeEnumTypes "iterator" caches just the TypeIndexes and lazily calls this method to create the actual symbols when they're needed.

I think you're right that there's a hole in the logic here that could cause it to create the same symbol multiple times, which is a problem. I'll fix that by moving this to findSymbolByTypeIndex. (See the TODO below.)

[amccarth]

Addressed additional feedback.

Also made createEnumSymbol go through findSymbolByTypeIndex to avoid making
duplicate symbols.

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

+//===- NativeEnumSymbol.h - info about enum type ----------------*- 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_NATIVEENUMSYMBOL_H
+#define LLVM_DEBUGINFO_PDB_NATIVE_NATIVEENUMSYMBOL_H
+
+#include "llvm/DebugInfo/CodeView/CodeView.h"
+#include "llvm/DebugInfo/CodeView/TypeVisitorCallbacks.h"
+#include "llvm/DebugInfo/PDB/Native/NativeRawSymbol.h"
+#include "llvm/DebugInfo/PDB/Native/NativeSession.h"
+
+namespace llvm {
+namespace pdb {
+
+class NativeEnumSymbol : public NativeRawSymbol,
+                         public codeview::TypeVisitorCallbacks {
+public:
+  NativeEnumSymbol(NativeSession &Session, SymIndexId Id,
+                   const codeview::CVType &CV);
+  ~NativeEnumSymbol() override;
+
+  std::unique_ptr<NativeRawSymbol> clone() const override;
+
+  std::unique_ptr<IPDBEnumSymbols>
+  findChildren(PDB_SymType Type) const override;
+
+  Error visitKnownRecord(codeview::CVType &CVR,
+                         codeview::EnumRecord &Record) override;
+  Error visitKnownMember(codeview::CVMemberRecord &CVM,
+                         codeview::EnumeratorRecord &Record) override;
+
+  PDB_SymType getSymTag() const override;
+#if 0
+  PDB_BuiltinType getBuiltinType() const override;
+#endif
+  uint32_t getClassParentId() const override;
+#if 0
+  uint32_t getUnmodifiedTypeId() const override;
+  bool hasConstructor() const override;
+  bool isConstType() const override;
+  bool hasAssignmentOperator() const override;
+  bool hasCastOperator() const override;
+  bool hasNestedTypes() const override;
+  uint64_t getLength() const override;
+  uint32_t getLexicalParentId() const override;
+#endif
+  std::string getName() const override;
+#if 0
+  bool isNested() const override;
+  bool hasOverloadedOperator() const override;
+  bool isPacked() const override;
+  bool isScoped() const override;
+#endif
+  uint32_t getTypeId() const override;
+#if 0
+  bool isUnalignedType() const override;
+  bool isVolatileType() const override;
+#endif
+
+protected:
+  codeview::CVType CV;
+  codeview::EnumRecord Record;
+};
+
+} // namespace pdb
+} // namespace llvm
+
+#endif // LLVM_DEBUGINFO_PDB_NATIVE_NATIVEENUMSYMBOL_H

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

+//==- NativeEnumTypes.h - Native Type Enumerator impl ------------*- 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_NATIVEENUMTYPES_H
+#define LLVM_DEBUGINFO_PDB_NATIVE_NATIVEENUMTYPES_H
+
+#include "llvm/DebugInfo/CodeView/TypeRecord.h"
+#include "llvm/DebugInfo/PDB/IPDBEnumChildren.h"
+#include "llvm/DebugInfo/PDB/PDBSymbol.h"
+
+namespace llvm {
+namespace pdb {
+
+class NativeSession;
+
+class NativeEnumTypes : public IPDBEnumChildren<PDBSymbol> {
+public:
+  NativeEnumTypes(NativeSession &Session, codeview::CVTypeRange Range,
+                  codeview::TypeLeafKind Kind);
+
+  uint32_t getChildCount() const override;
+  std::unique_ptr<PDBSymbol> getChildAtIndex(uint32_t Index) const override;
+  std::unique_ptr<PDBSymbol> getNext() override;
+  void reset() override;
+  NativeEnumTypes *clone() const override;
+
+private:
+  NativeSession &Session;
+  codeview::CVTypeRange Range;
+  codeview::TypeLeafKind Kind;
+  codeview::CVTypeArray::Iterator I;
+};
+
+} // namespace pdb
+} // namespace llvm
+
+#endif

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

public:
   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);
 
+  std::unique_ptr<PDBSymbolTypeEnum>
+  createEnumSymbol(const codeview::CVType &E);
+
   SymIndexId findSymbolByTypeIndex(codeview::TypeIndex 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>

llvm/lib/DebugInfo/PDB/CMakeLists.txt

add_pdb_impl_folder(Native
   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/NativeEnumSymbol.cpp
+  Native/NativeEnumTypes.cpp
   Native/NativeExeSymbol.cpp
   Native/NativeRawSymbol.cpp
   Native/NamedStreamMap.cpp
   Native/NativeSession.cpp
   Native/PDBFile.cpp
   Native/PDBFileBuilder.cpp
   Native/PDBStringTable.cpp
   Native/PDBStringTableBuilder.cpp

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

+//===- NativeEnumSymbol.cpp - info about enum type --------------*- 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/NativeEnumSymbol.h"
+
+#include "llvm/DebugInfo/CodeView/CVTypeVisitor.h"
+#include "llvm/DebugInfo/CodeView/TypeRecord.h"
+#include "llvm/DebugInfo/PDB/Native/NativeEnumTypes.h"
+
+namespace llvm {
+namespace pdb {
+
+NativeEnumSymbol::NativeEnumSymbol(NativeSession &Session, SymIndexId Id,
+                                   const codeview::CVType &fooCV)
+    : NativeRawSymbol(Session, Id), CV(fooCV),
+      Record(codeview::TypeRecordKind::Enum) {
+  if (auto EC = visitTypeRecord(CV, *this))
+    (void)EC;
+}
+
+NativeEnumSymbol::~NativeEnumSymbol() {}
+
+std::unique_ptr<NativeRawSymbol> NativeEnumSymbol::clone() const {
+  return llvm::make_unique<NativeEnumSymbol>(Session, SymbolId, CV);
+}
+
+std::unique_ptr<IPDBEnumSymbols>
+NativeEnumSymbol::findChildren(PDB_SymType Type) const {
+  switch (Type) {
+  case PDB_SymType::Data: {
+    // TODO(amccarth):  Provide an actual implementation.
+    codeview::CVTypeArray Array;
+    codeview::CVTypeRange Types(Array);
+    return std::unique_ptr<IPDBEnumSymbols>(
+        new NativeEnumTypes(Session, Types, codeview::LF_ENUMERATE));
+  }
+  default:
+    return nullptr;
+  }
+}
+
+Error NativeEnumSymbol::visitKnownRecord(codeview::CVType &CVR,
+                                         codeview::EnumRecord &ER) {
+  Record = ER;
+  return Error::success();
+}
+
+Error NativeEnumSymbol::visitKnownMember(codeview::CVMemberRecord &CVM,
+                                         codeview::EnumeratorRecord &R) {
+  return Error::success();
+}
+
+PDB_SymType NativeEnumSymbol::getSymTag() const { return PDB_SymType::Enum; }
+
+#if 0
+PDB_BuiltinType NativeEnumSymbol::getBuiltinType() const {
+  return PDB_BuiltinType::None;
+}
+#endif
+
+uint32_t NativeEnumSymbol::getClassParentId() const { return 0xFFFFFFFF; }
+
+#if 0
+uint32_t NativeEnumSymbol::getUnmodifiedTypeId() const {
+  return 0;
+}
+
+bool NativeEnumSymbol::hasConstructor() const {
+}
+
+bool NativeEnumSymbol::isConstType() const {}
+
+bool NativeEnumSymbol::hasAssignmentOperator() const {}
+
+bool NativeEnumSymbol::hasCastOperator() const {}
+
+bool NativeEnumSymbol::hasNestedTypes() const {}
+
+uint64_t NativeEnumSymbol::getLength() const {}
+
+uint32_t NativeEnumSymbol::getLexicalParentId() const {}
+#endif
+
+std::string NativeEnumSymbol::getName() const { return Record.getName(); }
+
+#if 0
+
+bool NativeEnumSymbol::isNested() const {}
+
+bool NativeEnumSymbol::hasOverloadedOperator() const {}
+
+bool NativeEnumSymbol::isPacked() const {}
+
+bool NativeEnumSymbol::isScoped() const {}
+#endif
+
+uint32_t NativeEnumSymbol::getTypeId() const {
+  return Session.findSymbolByTypeIndex(Record.getUnderlyingType());
+}
+
+#if 0
+bool NativeEnumSymbol::isUnalignedType() const {}
+
+bool NativeEnumSymbol::isVolatileType() const {}
+#endif
+
+} // namespace pdb
+} // namespace llvm

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

+//==- NativeEnumTypes.cpp - Native Type Enumerator impl ----------*- 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/NativeEnumTypes.h"
+
+#include "llvm/DebugInfo/PDB/IPDBEnumChildren.h"
+#include "llvm/DebugInfo/PDB/Native/NativeEnumSymbol.h"
+#include "llvm/DebugInfo/PDB/Native/NativeSession.h"
+#include "llvm/DebugInfo/PDB/PDBSymbol.h"
+#include "llvm/DebugInfo/PDB/PDBSymbolTypeEnum.h"
+
+namespace llvm {
+namespace pdb {
+
+NativeEnumTypes::NativeEnumTypes(NativeSession &PDBSession,
+                                 codeview::CVTypeRange Range,
+                                 codeview::TypeLeafKind Kind)
+    : Session(PDBSession), Range(Range), Kind(Kind), I(Range.begin()) {}
+
+uint32_t NativeEnumTypes::getChildCount() const {
+  // This is expensive and doesn't really return what we want.  Do we need
+  // this functionality?
+  return static_cast<uint32_t>(std::distance(Range.begin(), Range.end()));
+}
+
+std::unique_ptr<PDBSymbol>
+NativeEnumTypes::getChildAtIndex(uint32_t Index) const {
+  // Not supported.  Random access is slow and currently unimportant.
+  return nullptr;
+}
+
+std::unique_ptr<PDBSymbol> NativeEnumTypes::getNext() {
+  while (I != Range.end() && I->kind() != Kind) {
+    ++I;
+  }
+  if (I == Range.end())
+    return nullptr;
+  return Session.createEnumSymbol(*I++);
+}
+
+void NativeEnumTypes::reset() { I = Range.begin(); }
+
+NativeEnumTypes *NativeEnumTypes::clone() const {
+  return new NativeEnumTypes(Session, Range, Kind);
+}
+
+} // namespace pdb
+} // namespace llvm

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

 //===- NativeExeSymbol.cpp - native impl for PDBSymbolExe -------*- 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/NativeExeSymbol.h"
 
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/DebugInfo/PDB/Native/DbiStream.h"
 #include "llvm/DebugInfo/PDB/Native/InfoStream.h"
 #include "llvm/DebugInfo/PDB/Native/NativeEnumModules.h"
+#include "llvm/DebugInfo/PDB/Native/NativeEnumTypes.h"
 #include "llvm/DebugInfo/PDB/Native/PDBFile.h"
+#include "llvm/DebugInfo/PDB/Native/TpiStream.h"
 
 namespace llvm {
 namespace pdb {
 
 NativeExeSymbol::NativeExeSymbol(NativeSession &Session, SymIndexId SymbolId)
     : NativeRawSymbol(Session, SymbolId), File(Session.getPDBFile()) {}
 
 std::unique_ptr<NativeRawSymbol> NativeExeSymbol::clone() const {
   return llvm::make_unique<NativeExeSymbol>(Session, SymbolId);
 }
 
 std::unique_ptr<IPDBEnumSymbols>
 NativeExeSymbol::findChildren(PDB_SymType Type) const {
   switch (Type) {
   case PDB_SymType::Compiland: {
     auto Dbi = File.getPDBDbiStream();
     if (Dbi) {
       const DbiModuleList &Modules = Dbi->modules();
       return std::unique_ptr<IPDBEnumSymbols>(
           new NativeEnumModules(Session, Modules));
     }
     consumeError(Dbi.takeError());
     break;
   }
+  case PDB_SymType::Enum: {
+    auto Tpi = File.getPDBTpiStream();
+    if (Tpi) {
+      bool HadError = false;
+      const auto &Types = Tpi->types(&HadError);
+      if (HadError)
+        return nullptr;
+      return std::unique_ptr<IPDBEnumSymbols>(
+          new NativeEnumTypes(Session, Types, llvm::codeview::LF_ENUM));
+    }
+    consumeError(Tpi.takeError());
+    break;
+  }
   default:
     break;
   }
   return nullptr;
 }
 
 uint32_t NativeExeSymbol::getAge() const {
   auto IS = File.getPDBInfoStream();

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

 
 #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/NativeEnumSymbol.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/DebugInfo/PDB/PDBSymbolTypeEnum.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>

 NativeSession::createCompilandSymbol(DbiModuleDescriptor MI) {
   const auto Id = static_cast<SymIndexId>(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()));
 }
 
+std::unique_ptr<PDBSymbolTypeEnum>
+NativeSession::createEnumSymbol(const codeview::CVType &I) {
+  const auto Id = static_cast<SymIndexId>(SymbolCache.size());
+  SymbolCache.push_back(llvm::make_unique<NativeEnumSymbol>(*this, Id, I));
+  return llvm::make_unique<PDBSymbolTypeEnum>(
+      *this, std::unique_ptr<IPDBRawSymbol>(SymbolCache[Id]->clone()));
+}
+
 SymIndexId NativeSession::findSymbolByTypeIndex(codeview::TypeIndex 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.isSimple()) {
     // FIXME:  We will eventually need to handle pointers to other simple types,
if (It == std::end(BuiltinTypes))
       return 0;
     SymIndexId Id = SymbolCache.size();
     SymbolCache.emplace_back(
         llvm::make_unique<NativeBuiltinSymbol>(*this, Id, It->Type, It->Size));
     TypeIndexToSymbolId[Index] = Id;
     return Id;
   }
 
-  // TODO:  Look up PDB type by type index
+  // TODO(amccarth):  Look up PDB type by type index
 
   return 0;
 }
 
 uint64_t NativeSession::getLoadAddress() const { return 0; }
 
 void NativeSession::setLoadAddress(uint64_t Address) {}
 

llvm/test/DebugInfo/PDB/Native/pdb-native-enums.test

+; Test that the native PDB reader can enumerate the enum types.
+; RUN: llvm-pdbutil pretty -native -enums %p/../Inputs/every-type.pdb \
+; RUN:   | FileCheck -check-prefix=ENUMS %s
+
+ENUMS:  enum FooClass::NestedEnum {
+ENUMS-NEXT:  }

llvm/tools/llvm-pdbutil/PrettyEnumDumper.cpp

 using namespace llvm::pdb;
 
 EnumDumper::EnumDumper(LinePrinter &P) : PDBSymDumper(true), Printer(P) {}
 
 void EnumDumper::start(const PDBSymbolTypeEnum &Symbol) {
   WithColor(Printer, PDB_ColorItem::Keyword).get() << "enum ";
   WithColor(Printer, PDB_ColorItem::Type).get() << Symbol.getName();
   if (!opts::pretty::NoEnumDefs) {
-    auto BuiltinType = Symbol.getUnderlyingType();
-    if (BuiltinType->getBuiltinType() != PDB_BuiltinType::Int ||
-        BuiltinType->getLength() != 4) {
+    auto UnderlyingType = Symbol.getUnderlyingType();
+    if (!UnderlyingType)
+      return;
+    if (UnderlyingType->getBuiltinType() != PDB_BuiltinType::Int ||
+        UnderlyingType->getLength() != 4) {
       Printer << " : ";
       BuiltinDumper Dumper(Printer);
-      Dumper.start(*BuiltinType);
+      Dumper.start(*UnderlyingType);
     }
+    auto EnumValues = Symbol.findAllChildren<PDBSymbolData>();
     Printer << " {";
     Printer.Indent();
-    auto EnumValues = Symbol.findAllChildren<PDBSymbolData>();
     while (auto EnumValue = EnumValues->getNext()) {
       if (EnumValue->getDataKind() != PDB_DataKind::Constant)
         continue;
       Printer.NewLine();
       WithColor(Printer, PDB_ColorItem::Identifier).get()
           << EnumValue->getName();
       Printer << " = ";
       WithColor(Printer, PDB_ColorItem::LiteralValue).get()
           << EnumValue->getValue();
     }
     Printer.Unindent();
     Printer.NewLine();
     Printer << "}";
   }
 }