[pdb] Change type visitor pattern to be dynamic.

This allows better catching of compiler errors since we can use
the override keyword to verify that methods are actually
overridden.

Also in this patch I've changed from storing a boolean Error
code everywhere to returning an llvm::Error, to propagate richer
error information up the call stack.

Reviewed By: ruiu, rnk
Differential Revision: http://reviews.llvm.org/D21410

llvm-svn: 272926

llvm/include/llvm/DebugInfo/CodeView/CVTypeVisitor.h

@@ -11,151 +11,31 @@
 #define LLVM_DEBUGINFO_CODEVIEW_CVTYPEVISITOR_H
 
 #include "llvm/DebugInfo/CodeView/CVRecord.h"
-#include "llvm/DebugInfo/CodeView/CodeView.h"
-#include "llvm/DebugInfo/CodeView/TypeIndex.h"
 #include "llvm/DebugInfo/CodeView/TypeRecord.h"
-#include "llvm/Support/ErrorOr.h"
+#include "llvm/DebugInfo/CodeView/TypeVisitorCallbacks.h"
+#include "llvm/Support/Error.h"
 
 namespace llvm {
 namespace codeview {
 
-template <typename Derived>
 class CVTypeVisitor {
 public:
-  CVTypeVisitor() {}
+  explicit CVTypeVisitor(TypeVisitorCallbacks &Callbacks);
 
-  bool hadError() const { return HadError; }
-
-  template <typename T>
-  bool consumeObject(ArrayRef<uint8_t> &Data, const T *&Res) {
-    if (Data.size() < sizeof(*Res)) {
-      HadError = true;
-      return false;
-    }
-    Res = reinterpret_cast<const T *>(Data.data());
-    Data = Data.drop_front(sizeof(*Res));
-    return true;
-  }
-
-  /// Actions to take on known types. By default, they do nothing. Visit methods
-  /// for member records take the FieldData by non-const reference and are
-  /// expected to consume the trailing bytes used by the field.
-  /// FIXME: Make the visitor interpret the trailing bytes so that clients don't
-  /// need to.
-#define TYPE_RECORD(EnumName, EnumVal, Name)                                   \
-  void visit##Name(Name##Record &Record) {}
-#define TYPE_RECORD_ALIAS(EnumName, EnumVal, Name, AliasName)
-#define MEMBER_RECORD(EnumName, EnumVal, Name)                                 \
-  void visit##Name(Name##Record &Record) {}
-#define MEMBER_RECORD_ALIAS(EnumName, EnumVal, Name, AliasName)
-#include "TypeRecords.def"
-
-  void visitTypeRecord(const CVRecord<TypeLeafKind> &Record) {
-    ArrayRef<uint8_t> LeafData = Record.Data;
-    auto *DerivedThis = static_cast<Derived *>(this);
-    DerivedThis->visitTypeBegin(Record);
-    switch (Record.Type) {
-    default:
-      DerivedThis->visitUnknownType(Record);
-      break;
-    case LF_FIELDLIST:
-      DerivedThis->visitFieldList(Record.Type, LeafData);
-      break;
-#define TYPE_RECORD(EnumName, EnumVal, Name)                                   \
-  case EnumName: {                                                             \
-    TypeRecordKind RK = static_cast<TypeRecordKind>(EnumName);                 \
-    auto Result = Name##Record::deserialize(RK, LeafData);                     \
-    if (Result.getError())                                                     \
-      return parseError();                                                     \
-    DerivedThis->visit##Name(*Result);                                         \
-    break;                                                                     \
-  }
-#define TYPE_RECORD_ALIAS(EnumName, EnumVal, Name, AliasName)                  \
-  TYPE_RECORD(EnumVal, EnumVal, AliasName)
-#define MEMBER_RECORD(EnumName, EnumVal, Name)
-#include "TypeRecords.def"
-      }
-      DerivedThis->visitTypeEnd(Record);
-  }
+  Error visitTypeRecord(const CVRecord<TypeLeafKind> &Record);
 
   /// Visits the type records in Data. Sets the error flag on parse failures.
-  void visitTypeStream(const CVTypeArray &Types) {
-    for (const auto &I : Types) {
-      visitTypeRecord(I);
-      if (hadError())
-        break;
-    }
-  }
-
-  /// Action to take on unknown types. By default, they are ignored.
-  void visitUnknownType(const CVRecord<TypeLeafKind> &Record) {}
+  Error visitTypeStream(const CVTypeArray &Types);
 
-  /// Paired begin/end actions for all types. Receives all record data,
-  /// including the fixed-length record prefix.
-  void visitTypeBegin(const CVRecord<TypeLeafKind> &Record) {}
-  void visitTypeEnd(const CVRecord<TypeLeafKind> &Record) {}
-
-  ArrayRef<uint8_t> skipPadding(ArrayRef<uint8_t> Data) {
-    if (Data.empty())
-      return Data;
-    uint8_t Leaf = Data.front();
-    if (Leaf < LF_PAD0)
-      return Data;
-    // Leaf is greater than 0xf0. We should advance by the number of bytes in
-    // the low 4 bits.
-    unsigned BytesToAdvance = Leaf & 0x0F;
-    if (Data.size() < BytesToAdvance) {
-      parseError();
-      return None;
-    }
-    return Data.drop_front(BytesToAdvance);
-  }
+  Error skipPadding(ArrayRef<uint8_t> &Data);
 
   /// Visits individual member records of a field list record. Member records do
   /// not describe their own length, and need special handling.
-  void visitFieldList(TypeLeafKind Leaf, ArrayRef<uint8_t> FieldData) {
-    auto *DerivedThis = static_cast<Derived *>(this);
-    while (!FieldData.empty()) {
-      const ulittle16_t *LeafPtr;
-      if (!CVTypeVisitor::consumeObject(FieldData, LeafPtr))
-        return;
-      TypeLeafKind Leaf = TypeLeafKind(unsigned(*LeafPtr));
-      switch (Leaf) {
-      default:
-        // Field list records do not describe their own length, so we cannot
-        // continue parsing past an unknown member type.
-        DerivedThis->visitUnknownMember(Leaf);
-        return parseError();
-#define MEMBER_RECORD(EnumName, EnumVal, Name)                                 \
-  case EnumName: {                                                             \
-    TypeRecordKind RK = static_cast<TypeRecordKind>(EnumName);                 \
-    auto Result = Name##Record::deserialize(RK, FieldData);                    \
-    if (Result.getError())                                                     \
-      return parseError();                                                     \
-    DerivedThis->visit##Name(*Result);                                         \
-    break;                                                                     \
-  }
-#define MEMBER_RECORD_ALIAS(EnumName, EnumVal, Name, AliasName)                \
-  MEMBER_RECORD(EnumVal, EnumVal, AliasName)
-#include "TypeRecords.def"
-      }
-      FieldData = skipPadding(FieldData);
-      if (hadError())
-        break;
-    }
-  }
-
-  /// Action to take on unknown members. By default, they are ignored. Member
-  /// record parsing cannot recover from an unknown member record, so this
-  /// method is only called at most once per field list record.
-  void visitUnknownMember(TypeLeafKind Leaf) {}
-
-  /// Helper for returning from a void function when the stream is corrupted.
-  void parseError() { HadError = true; }
+  Error visitFieldList(const CVRecord<TypeLeafKind> &Record);
 
 private:
-  /// Whether a type stream parsing error was encountered.
-  bool HadError = false;
+  /// The interface to the class that gets notified of each visitation.
+  TypeVisitorCallbacks &Callbacks;
 };
 
 } // end namespace codeview

llvm/include/llvm/DebugInfo/CodeView/TypeDumper.h

@@ -14,14 +14,15 @@
 #include "llvm/ADT/StringSet.h"
 #include "llvm/DebugInfo/CodeView/TypeIndex.h"
 #include "llvm/DebugInfo/CodeView/TypeRecord.h"
+#include "llvm/DebugInfo/CodeView/TypeVisitorCallbacks.h"
 
 namespace llvm {
 class ScopedPrinter;
 
 namespace codeview {
 
 /// Dumper for CodeView type streams found in COFF object files and PDB files.
-class CVTypeDumper {
+class CVTypeDumper : public TypeVisitorCallbacks {
 public:
   CVTypeDumper(ScopedPrinter *W, bool PrintRecordBytes)
       : W(W), PrintRecordBytes(PrintRecordBytes) {}
@@ -33,17 +34,17 @@ class CVTypeDumper {
   /// and true otherwise.  This should be called in order, since the dumper
   /// maintains state about previous records which are necessary for cross
   /// type references.
-  bool dump(const CVRecord<TypeLeafKind> &Record);
+  Error dump(const CVRecord<TypeLeafKind> &Record);
 
   /// Dumps the type records in Types. Returns false if there was a type stream
   /// parse error, and true otherwise.
-  bool dump(const CVTypeArray &Types);
+  Error dump(const CVTypeArray &Types);
 
   /// Dumps the type records in Data. Returns false if there was a type stream
   /// parse error, and true otherwise. Use this method instead of the
   /// CVTypeArray overload when type records are laid out contiguously in
   /// memory.
-  bool dump(ArrayRef<uint8_t> Data);
+  Error dump(ArrayRef<uint8_t> Data);
 
   /// Gets the type index for the next type record.
   unsigned getNextTypeIndex() const {
@@ -61,11 +62,35 @@ class CVTypeDumper {
   void setPrinter(ScopedPrinter *P);
   ScopedPrinter *getPrinter() { return W; }
 
+  /// Action to take on unknown types. By default, they are ignored.
+  Error visitUnknownType(const CVRecord<TypeLeafKind> &Record) override;
+  Error visitUnknownMember(const CVRecord<TypeLeafKind> &Record) override;
+
+  /// Paired begin/end actions for all types. Receives all record data,
+  /// including the fixed-length record prefix.
+  Error visitTypeBegin(const CVRecord<TypeLeafKind> &Record) override;
+  Error visitTypeEnd(const CVRecord<TypeLeafKind> &Record) override;
+
+#define TYPE_RECORD(EnumName, EnumVal, Name)                                   \
+  Error visit##Name(Name##Record &Record) override;
+#define MEMBER_RECORD(EnumName, EnumVal, Name)                                 \
+  TYPE_RECORD(EnumName, EnumVal, Name)
+#define TYPE_RECORD_ALIAS(EnumName, EnumVal, Name, AliasName)
+#define MEMBER_RECORD_ALIAS(EnumName, EnumVal, Name, AliasName)
+#include "TypeRecords.def"
+
 private:
+  void printMemberAttributes(MemberAttributes Attrs);
+  void printMemberAttributes(MemberAccess Access, MethodKind Kind,
+                             MethodOptions Options);
+
   ScopedPrinter *W;
 
   bool PrintRecordBytes = false;
 
+  /// Name of the current type. Only valid before visitTypeEnd.
+  StringRef Name;
+
   /// 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.

llvm/include/llvm/DebugInfo/CodeView/TypeRecords.def

@@ -83,6 +83,7 @@ TYPE_RECORD(LF_STRING_ID, 0x1605, StringId)
 TYPE_RECORD(LF_UDT_SRC_LINE, 0x1606, UdtSourceLine)
 TYPE_RECORD(LF_UDT_MOD_SRC_LINE, 0x1607, UdtModSourceLine)
 
+
 TYPE_RECORD(LF_METHODLIST, 0x1206, MethodOverloadList)
 
 
@@ -195,6 +196,8 @@ CV_TYPE(LF_MODIFIER_EX, 0x1518)
 CV_TYPE(LF_VECTOR, 0x151b)
 CV_TYPE(LF_MATRIX, 0x151c)
 
+// ID leaf records. Subsequent leaf types may be referenced from .debug$S.
+
 // Numeric leaf types. These are generally contained in other records, and not
 // encountered in the main type stream.
 

llvm/include/llvm/DebugInfo/CodeView/TypeVisitorCallbacks.h

@@ -0,0 +1,53 @@
+//===- TypeVisitorCallbacks.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_TYPEVISITORCALLBACKS_H
+#define LLVM_DEBUGINFO_CODEVIEW_TYPEVISITORCALLBACKS_H
+
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/DebugInfo/CodeView/CodeView.h"
+#include "llvm/Support/Error.h"
+
+namespace llvm {
+namespace codeview {
+class TypeVisitorCallbacks {
+  friend class CVTypeVisitor;
+
+public:
+  virtual ~TypeVisitorCallbacks() {}
+
+  /// Action to take on unknown types. By default, they are ignored.
+  virtual Error visitUnknownType(const CVRecord<TypeLeafKind> &Record) {
+    return Error::success();
+  }
+  virtual Error visitUnknownMember(const CVRecord<TypeLeafKind> &Record) {
+    return Error::success();
+  }
+
+  /// Paired begin/end actions for all types. Receives all record data,
+  /// including the fixed-length record prefix.
+  virtual Error visitTypeBegin(const CVRecord<TypeLeafKind> &Record) {
+    return Error::success();
+  }
+  virtual Error visitTypeEnd(const CVRecord<TypeLeafKind> &Record) {
+    return Error::success();
+  }
+
+#define TYPE_RECORD(EnumName, EnumVal, Name)                                   \
+  virtual Error visit##Name(Name##Record &Record) { return Error::success(); }
+#define MEMBER_RECORD(EnumName, EnumVal, Name)                                 \
+  TYPE_RECORD(EnumName, EnumVal, Name)
+#define TYPE_RECORD_ALIAS(EnumName, EnumVal, Name, AliasName)
+#define MEMBER_RECORD_ALIAS(EnumName, EnumVal, Name, AliasName)
+#include "TypeRecords.def"
+};
+}
+}
+
+#endif

llvm/include/llvm/DebugInfo/PDB/Raw/RawError.h

@@ -25,6 +25,7 @@ enum class raw_error_code {
   index_out_of_bounds,
   invalid_block_address,
   not_writable,
+  invalid_tpi_hash,
 };
 
 /// Base class for errors originating when parsing raw PDB files

llvm/lib/CodeGen/AsmPrinter/CodeViewDebug.cpp

@@ -322,10 +322,9 @@ void CodeViewDebug::emitTypeInformation() {
           ScopedPrinter SP(CommentOS);
           SP.setPrefix(CommentPrefix);
           CVTD.setPrinter(&SP);
-          bool DumpSuccess =
-              CVTD.dump({Record.bytes_begin(), Record.bytes_end()});
-          (void)DumpSuccess;
-          assert(DumpSuccess && "produced malformed type record");
+          Error EC = CVTD.dump({Record.bytes_begin(), Record.bytes_end()});
+          assert(!EC && "produced malformed type record");
+          consumeError(std::move(EC));
           // emitRawComment will insert its own tab and comment string before
           // the first line, so strip off our first one. It also prints its own
           // newline.

llvm/lib/DebugInfo/CodeView/CMakeLists.txt

@@ -1,6 +1,7 @@
 add_llvm_library(LLVMDebugInfoCodeView
   ByteStream.cpp
   CodeViewError.cpp
+  CVTypeVisitor.cpp
   EnumTables.cpp
   FieldListRecordBuilder.cpp
   Line.cpp