Resubmit "[pdb] Change type visitor pattern to be dynamic."

There was a regression introduced during type stream merging when
visiting a field list record.  This has been fixed in this patch.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@272929 91177308-0d34-0410-b5e6-96231b3b80d8

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

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.

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.
 

include/llvm/DebugInfo/CodeView/TypeVisitorCallbacks.h

@@ -0,0 +1,61 @@
+//===- 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();
+  }
+
+  virtual Error visitFieldListBegin(const CVRecord<TypeLeafKind> &Record) {
+    return Error::success();
+  }
+
+  virtual Error visitFieldListEnd(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

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

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.

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