Make CodeView TypeIterator generic so that it can iterate symbols too

[zturner]

Summary

Symbol streams and type streams have the same format, and it's possible we might find other types of "things" that have similar formats as well. So rather than reproduce the code, we templatize TypeIterator on the type enumeration kind, and allow it to iterate over arbitrary streams of similarly laid out records.

[amccarth]

Nice refactor.

[rnk]

lgtm

include/llvm/DebugInfo/CodeView/RecordIterator.h

20

Maybe call the template parameter RecordKind or KindType so you can call the Record field Kind instead of Type?

113–114

// end namespace codeview / llvm?

include/llvm/DebugInfo/CodeView/TypeRecord.h

275

Once upon a type, TypeRecord was an actual class. With TypeRecordPrefix gone, the last vestiges of it are gone. Can you replace TypeRecord with "type record" here?

[ovyalov]

Looks like this CL broke CMake buildbot - http://lab.llvm.org:8011/builders/lldb-x86_64-ubuntu-14.04-cmake/builds/14464, could you take a look?

[zturner]

Someone beat me to the fix, case insensitive include path problem.

[seurer]

I just noticed this problem on powerpc as well:

In file included from /home/seurer/llvm/llvm-test/include/llvm/DebugInfo/CodeView/SymbolRecord.h:14:0,

from /home/seurer/llvm/llvm-test/lib/MC/MCCodeView.cpp:19:

/home/seurer/llvm/llvm-test/include/llvm/DebugInfo/CodeView/RecordIterator.h:15:33: fatal error: llvm/support/Endian.h: No such file or directory

seurer@makalu-lp1:~/llvm/build/llvm-test$ find $LLVM_SRC -name Endian.h
/home/seurer/llvm/llvm-test/include/llvm/Support/Endian.h

Note "support" (incorrect) versus "Support" (correct).

[zturner]

Yea, sorry should be fixed in r268943. Let me know if it's not.

include/llvm/DebugInfo/CodeView/CVTypeVisitor.h

 //===- CVTypeVisitor.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_CVTYPEVISITOR_H
 #define LLVM_DEBUGINFO_CODEVIEW_CVTYPEVISITOR_H
 
 #include "llvm/DebugInfo/CodeView/CodeView.h"
+#include "llvm/DebugInfo/CodeView/RecordIterator.h"
 #include "llvm/DebugInfo/CodeView/TypeIndex.h"
 #include "llvm/DebugInfo/CodeView/TypeRecord.h"
-#include "llvm/DebugInfo/CodeView/TypeStream.h"
 
 namespace llvm {
 namespace codeview {
 
 template <typename Derived>
 class CVTypeVisitor {
 public:
   CVTypeVisitor() {}
void visit##ClassName(TypeLeafKind LeafType, const ClassName *Record,        \
                         ArrayRef<uint8_t> LeafData) {}
 #define TYPE_RECORD_ALIAS(ClassName, LeafEnum)
 #define MEMBER_RECORD(ClassName, LeafEnum)                                     \
   void visit##ClassName(TypeLeafKind LeafType, const ClassName *Record,        \
                         ArrayRef<uint8_t> &FieldData) {}
 #define MEMBER_RECORD_ALIAS(ClassName, LeafEnum)
 #include "TypeRecords.def"
 
-  void visitTypeRecord(const TypeIterator::TypeRecord &Record) {
-    ArrayRef<uint8_t> LeafData = Record.LeafData;
+  void visitTypeRecord(const TypeIterator::Record &Record) {
+    ArrayRef<uint8_t> LeafData = Record.Data;
     ArrayRef<uint8_t> RecordData = LeafData;
     auto *DerivedThis = static_cast<Derived *>(this);
-    DerivedThis->visitTypeBegin(Record.Leaf, RecordData);
-    switch (Record.Leaf) {
+    DerivedThis->visitTypeBegin(Record.Type, RecordData);
+    switch (Record.Type) {
     default:
-      DerivedThis->visitUnknownType(Record.Leaf);
+      DerivedThis->visitUnknownType(Record.Type);
       break;
     case LF_FIELDLIST:
-      DerivedThis->visitFieldList(Record.Leaf, LeafData);
+      DerivedThis->visitFieldList(Record.Type, LeafData);
       break;
     case LF_METHODLIST:
-      DerivedThis->visitMethodList(Record.Leaf, LeafData);
+      DerivedThis->visitMethodList(Record.Type, LeafData);
       break;
 #define TYPE_RECORD(ClassName, LeafEnum)                                       \
   case LeafEnum: {                                                             \
     const ClassName *Rec;                                                      \
     if (!CVTypeVisitor::consumeObject(LeafData, Rec))                          \
       return;                                                                  \
-    DerivedThis->visit##ClassName(Record.Leaf, Rec, LeafData);                 \
+    DerivedThis->visit##ClassName(Record.Type, Rec, LeafData);                 \
     break;                                                                     \
   }
 #include "TypeRecords.def"
       }
-      DerivedThis->visitTypeEnd(Record.Leaf, RecordData);
+      DerivedThis->visitTypeEnd(Record.Type, RecordData);
   }
 
   /// Visits the type records in Data. Sets the error flag on parse failures.
   void visitTypeStream(ArrayRef<uint8_t> Data) {
     for (const auto &I : makeTypeRange(Data))
       visitTypeRecord(I);
   }
 

include/llvm/DebugInfo/CodeView/RecordIterator.h

+//===- RecordIterator.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_RECORDITERATOR_H
+#define LLVM_DEBUGINFO_CODEVIEW_RECORDITERATOR_H
+
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/iterator_range.h"
+#include "llvm/support/Endian.h"
+
+namespace llvm {
+namespace codeview {
+// A const input iterator interface to the CodeView record stream.
+template <typename Kind> class RecordIterator {
+private:
+  struct RecordPrefix {
+    support::ulittle16_t Len;  // Record length, starting from &Leaf
+    support::ulittle16_t Kind; // Record kind (a value from the `Kind` enum).
+  };
+
+public:
+  struct Record {
+    std::size_t Length;
+    Kind Type;
+    ArrayRef<uint8_t> Data;
+  };
+
+  explicit RecordIterator(const ArrayRef<uint8_t> &RecordBytes)
+      : Data(RecordBytes), AtEnd(false) {
+    next(); // Prime the pump
+  }
+
+  RecordIterator() : AtEnd(true) {}
+
+  // For iterators to compare equal, they must both point at the same record
+  // in the same data stream, or they must both be at the end of a stream.
+  friend bool operator==(const RecordIterator<Kind> &lhs,
+                         const RecordIterator<Kind> &rhs) {
+    return (lhs.Data.begin() == rhs.Data.begin()) || (lhs.AtEnd && rhs.AtEnd);
+  }
+
+  friend bool operator!=(const RecordIterator<Kind> &lhs,
+                         const RecordIterator<Kind> &rhs) {
+    return !(lhs == rhs);
+  }
+
+  const Record &operator*() const {
+    assert(!AtEnd);
+    return Current;
+  }
+
+  const Record *operator->() const {
+    assert(!AtEnd);
+    return &Current;
+  }
+
+  RecordIterator<Kind> &operator++() {
+    next();
+    return *this;
+  }
+
+  RecordIterator<Kind> operator++(int) {
+    RecordIterator<Kind> Original = *this;
+    ++*this;
+    return Original;
+  }
+
+private:
+  void next() {
+    assert(!AtEnd && "Attempted to advance more than one past the last rec");
+    if (Data.empty()) {
+      // We've advanced past the last record.
+      AtEnd = true;
+      return;
+    }
+
+    // FIXME: Use consumeObject when it deals in ArrayRef<uint8_t>.
+    if (Data.size() < sizeof(RecordPrefix))
+      return;
+    const auto *Rec = reinterpret_cast<const RecordPrefix *>(Data.data());
+    Data = Data.drop_front(sizeof(RecordPrefix));
+
+    Current.Length = Rec->Len;
+    Current.Type = static_cast<Kind>(uint16_t(Rec->Kind));
+    Current.Data = Data.slice(0, Current.Length - 2);
+
+    // The next record starts immediately after this one.
+    Data = Data.drop_front(Current.Data.size());
+
+    // FIXME: The stream contains LF_PAD bytes that we need to ignore, but those
+    // are typically included in LeafData. We may need to call skipPadding() if
+    // we ever find a record that doesn't count those bytes.
+
+    return;
+  }
+
+  ArrayRef<uint8_t> Data;
+  Record Current;
+  bool AtEnd;
+};
+
+template <typename Kind>
+inline iterator_range<RecordIterator<Kind>>
+makeRecordRange(ArrayRef<uint8_t> Data) {
+  return make_range(RecordIterator<Kind>(Data), RecordIterator<Kind>());
+}
+}
+}
+
+#endif

include/llvm/DebugInfo/CodeView/SymbolRecord.h

 //===- SymbolRecord.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_SYMBOLRECORD_H
 #define LLVM_DEBUGINFO_CODEVIEW_SYMBOLRECORD_H
 
 #include "llvm/DebugInfo/CodeView/CodeView.h"
+#include "llvm/DebugInfo/CodeView/RecordIterator.h"
 #include "llvm/DebugInfo/CodeView/TypeIndex.h"
 #include "llvm/Support/Endian.h"
 
 namespace llvm {
 namespace codeview {
 
 using llvm::support::ulittle16_t;
 using llvm::support::ulittle32_t;

 // S_LTHREAD32, S_GTHREAD32
 struct ThreadLocalDataSym {
   TypeIndex Type;
   ulittle32_t DataOffset;
   ulittle16_t Segment;
   // Name: The null-terminated name follows.
 };
 
+typedef RecordIterator<SymbolRecordKind> SymbolIterator;
+
+inline iterator_range<SymbolIterator> makeSymbolRange(ArrayRef<uint8_t> Data) {
+  return make_range(SymbolIterator(Data), SymbolIterator());
+}
+
 } // namespace codeview
 } // namespace llvm
 
 #endif

include/llvm/DebugInfo/CodeView/TypeDumper.h

public:
 
   StringRef getTypeName(TypeIndex TI);
   void printTypeIndex(StringRef FieldName, TypeIndex TI);
 
   /// Dumps one type record.  Returns false if there was a type parsing error,
   /// 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 TypeIterator::TypeRecord &Record);
+  bool dump(const TypeIterator::Record &Record);
 
   /// Dumps the type records in Data. Returns false if there was a type stream
   /// parse error, and true otherwise.
   bool dump(ArrayRef<uint8_t> Data);
 
   /// Gets the type index for the next type record.
   unsigned getNextTypeIndex() const {
     return 0x1000 + CVUDTNames.size();

include/llvm/DebugInfo/CodeView/TypeRecord.h

 
 private:
   ArrayRef<VirtualTableSlotKind> Slots;
 };
 
 //===----------------------------------------------------------------------===//
 // On-disk representation of type information
 
 // A CodeView type stream is a sequence of TypeRecords. Records larger than
 // 65536 must chain on to a second record. Each TypeRecord is followed by one of
 // the leaf types described below.
-struct TypeRecordPrefix {
-  ulittle16_t Len;  // Type record length, starting from &Leaf.
-  ulittle16_t Leaf; // Type record kind (TypeLeafKind)
-};
 
 // LF_TYPESERVER2
 struct TypeServer2 {
   char Signature[16];  // GUID
   ulittle32_t Age;
   // Name: Name of the PDB as a null-terminated string
 };
 

include/llvm/DebugInfo/CodeView/TypeStream.h

 //===- TypeStream.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_TYPESTREAM_H
 #define LLVM_DEBUGINFO_CODEVIEW_TYPESTREAM_H
 
-#include "llvm/ADT/iterator_range.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/DebugInfo/CodeView/CodeView.h"
-#include "llvm/DebugInfo/CodeView/TypeRecord.h"
+#include "llvm/DebugInfo/CodeView/RecordIterator.h"
 #include "llvm/Object/Error.h"
 #include "llvm/Support/Endian.h"
-#include "llvm/Support/ErrorOr.h"
 
 #include <stdint.h>
 
 namespace llvm {
 class APSInt;
 
 namespace codeview {
 
inline bool decodeNumericLeaf(StringRef &Data, APSInt &Num) {
   bool Success = decodeNumericLeaf(Bytes, Num);
   Data = StringRef(reinterpret_cast<const char *>(Bytes.data()), Bytes.size());
   return Success;
 }
 
 /// Decode a numeric leaf value that is known to be a uint32_t.
 bool decodeUIntLeaf(ArrayRef<uint8_t> &Data, uint64_t &Num);
 
-// A const input iterator interface to the CodeView type stream.
-class TypeIterator {
-public:
-  struct TypeRecord {
-    std::size_t Length;
-    TypeLeafKind Leaf;
-    ArrayRef<uint8_t> LeafData;
-  };
-
-  explicit TypeIterator(const ArrayRef<uint8_t> &SectionData)
-      : Data(SectionData), AtEnd(false) {
-    next(); // Prime the pump
-  }
-
-  TypeIterator() : AtEnd(true) {}
-
-  // For iterators to compare equal, they must both point at the same record
-  // in the same data stream, or they must both be at the end of a stream.
-  friend bool operator==(const TypeIterator &lhs, const TypeIterator &rhs) {
-    return (lhs.Data.begin() == rhs.Data.begin()) || (lhs.AtEnd && rhs.AtEnd);
-  }
-
-  friend bool operator!=(const TypeIterator &lhs, const TypeIterator &rhs) {
-    return !(lhs == rhs);
-  }
-
-  const TypeRecord &operator*() const {
-    assert(!AtEnd);
-    return Current;
-  }
-
-  const TypeRecord *operator->() const {
-    assert(!AtEnd);
-    return &Current;
-  }
-
-  TypeIterator operator++() {
-    next();
-    return *this;
-  }
-
-  TypeIterator operator++(int) {
-    TypeIterator Original = *this;
-    ++*this;
-    return Original;
-  }
-
-private:
-  void next() {
-    assert(!AtEnd && "Attempted to advance more than one past the last rec");
-    if (Data.empty()) {
-      // We've advanced past the last record.
-      AtEnd = true;
-      return;
-    }
-
-    // FIXME: Use consumeObject when it deals in ArrayRef<uint8_t>.
-    if (Data.size() < sizeof(TypeRecordPrefix))
-      return;
-    const auto *Rec = reinterpret_cast<const TypeRecordPrefix *>(Data.data());
-    Data = Data.drop_front(sizeof(TypeRecordPrefix));
-
-    Current.Length = Rec->Len;
-    Current.Leaf = static_cast<TypeLeafKind>(uint16_t(Rec->Leaf));
-    Current.LeafData = Data.slice(0, Current.Length - 2);
-
-    // The next record starts immediately after this one.
-    Data = Data.drop_front(Current.LeafData.size());
-
-    // FIXME: The stream contains LF_PAD bytes that we need to ignore, but those
-    // are typically included in LeafData. We may need to call skipPadding() if
-    // we ever find a record that doesn't count those bytes.
-
-    return;
-  }
-
-  ArrayRef<uint8_t> Data;
-  TypeRecord Current;
-  bool AtEnd;
-};
+typedef RecordIterator<TypeLeafKind> TypeIterator;
 
 inline iterator_range<TypeIterator> makeTypeRange(ArrayRef<uint8_t> Data) {
   return make_range(TypeIterator(Data), TypeIterator());
 }
 
 } // end namespace codeview
 } // end namespace llvm
 
 #endif

lib/DebugInfo/CodeView/TypeDumper.cpp

void CVTypeDumper::printTypeIndex(StringRef FieldName, TypeIndex TI) {
   if (!TI.isNoType())
     TypeName = getTypeName(TI);
   if (!TypeName.empty())
     W.printHex(FieldName, TypeName, TI.getIndex());
   else
     W.printHex(FieldName, TI.getIndex());
 }
 
-bool CVTypeDumper::dump(const TypeIterator::TypeRecord &Record) {
+bool CVTypeDumper::dump(const TypeIterator::Record &Record) {
   CVTypeDumperImpl Dumper(*this, W, PrintRecordBytes);
   Dumper.visitTypeRecord(Record);
   return !Dumper.hadError();
 }
 
 bool CVTypeDumper::dump(ArrayRef<uint8_t> Data) {
   CVTypeDumperImpl Dumper(*this, W, PrintRecordBytes);
   Dumper.visitTypeStream(Data);
   return !Dumper.hadError();
 }

tools/llvm-pdbdump/llvm-pdbdump.cpp

if (opts::DumpTpiRecordBytes || opts::DumpTpiRecords) {
 
     for (auto &Type : Tpi.types()) {
       DictScope DD(P, "");
 
       if (opts::DumpTpiRecords)
         TD.dump(Type);
 
       if (opts::DumpTpiRecordBytes)
-        P.printBinaryBlock("Bytes", Type.LeafData);
+        P.printBinaryBlock("Bytes", Type.Data);
     }
   }
   return Error::success();
 }
 
 static Error dumpStructure(RawSession &RS) {
   PDBFile &File = RS.getPDBFile();
   ScopedPrinter P(outs());