[codeview] Remove Type member from CVRecord

Summary:
Now CVType and CVSymbol are effectively type-safe wrappers around
ArrayRef<uint8_t>. Make the kind() accessor load it from the
RecordPrefix, which is the same for types and symbols.

Reviewers: zturner, aganea

Subscribers: hiraditya, llvm-commits

Tags: #llvm

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

llvm-svn: 357658

include/llvm/DebugInfo/CodeView/CVRecord.h

@@ -24,17 +24,31 @@ namespace llvm {
 
 namespace codeview {
 
+/// CVRecord is a fat pointer (base + size pair) to a symbol or type record.
+/// Carrying the size separately instead of trusting the size stored in the
+/// record prefix provides some extra safety and flexibility.
 template <typename Kind> class CVRecord {
 public:
-  CVRecord() : Type(static_cast<Kind>(0)) {}
+  CVRecord() = default;
 
-  CVRecord(Kind K, ArrayRef<uint8_t> Data) : Type(K), RecordData(Data) {}
+  CVRecord(ArrayRef<uint8_t> Data) : RecordData(Data) {}
 
-  bool valid() const { return Type != static_cast<Kind>(0); }
+  CVRecord(const RecordPrefix *P, size_t Size)
+      : RecordData(reinterpret_cast<const uint8_t *>(P), Size) {}
+
+  bool valid() const { return kind() != Kind(0); }
 
   uint32_t length() const { return RecordData.size(); }
-  Kind kind() const { return Type; }
+
+  Kind kind() const {
+    if (RecordData.size() < sizeof(RecordPrefix))
+      return Kind(0);
+    return static_cast<Kind>(static_cast<uint16_t>(
+        reinterpret_cast<const RecordPrefix *>(RecordData.data())->RecordKind));
+  }
+
   ArrayRef<uint8_t> data() const { return RecordData; }
+
   StringRef str_data() const {
     return StringRef(reinterpret_cast<const char *>(RecordData.data()),
                      RecordData.size());
@@ -44,7 +58,6 @@ public:
     return RecordData.drop_front(sizeof(RecordPrefix));
   }
 
-  Kind Type;
   ArrayRef<uint8_t> RecordData;
 };
 
@@ -71,8 +84,7 @@ Error forEachCodeViewRecord(ArrayRef<uint8_t> StreamBuffer, Func F) {
     ArrayRef<uint8_t> Data = StreamBuffer.take_front(RealLen);
     StreamBuffer = StreamBuffer.drop_front(RealLen);
 
-    Record R(static_cast<decltype(Record::Type)>((uint16_t)Prefix->RecordKind),
-             Data);
+    Record R(Data);
     if (auto EC = F(R))
       return EC;
   }
@@ -91,13 +103,12 @@ inline Expected<CVRecord<Kind>> readCVRecordFromStream(BinaryStreamRef Stream,
     return std::move(EC);
   if (Prefix->RecordLen < 2)
     return make_error<CodeViewError>(cv_error_code::corrupt_record);
-  Kind K = static_cast<Kind>(uint16_t(Prefix->RecordKind));
 
   Reader.setOffset(Offset);
   ArrayRef<uint8_t> RawData;
   if (auto EC = Reader.readBytes(RawData, Prefix->RecordLen + sizeof(uint16_t)))
     return std::move(EC);
-  return codeview::CVRecord<Kind>(K, RawData);
+  return codeview::CVRecord<Kind>(RawData);
 }
 
 } // end namespace codeview

include/llvm/DebugInfo/CodeView/RecordSerialization.h

@@ -31,6 +31,9 @@ using llvm::support::ulittle32_t;
 enum : unsigned { MaxRecordLength = 0xFF00 };
 
 struct RecordPrefix {
+  RecordPrefix() = default;
+  explicit RecordPrefix(uint16_t Kind) : RecordLen(2), RecordKind(Kind) {}
+
   ulittle16_t RecordLen;  // Record length, starting from &RecordKind.
   ulittle16_t RecordKind; // Record kind enum (SymRecordKind or TypeRecordKind)
 };

include/llvm/DebugInfo/CodeView/SymbolSerializer.h

@@ -51,8 +51,8 @@ public:
   template <typename SymType>
   static CVSymbol writeOneSymbol(SymType &Sym, BumpPtrAllocator &Storage,
                                  CodeViewContainer Container) {
-    CVSymbol Result;
-    Result.Type = static_cast<SymbolKind>(Sym.Kind);
+    RecordPrefix Prefix{uint16_t(Sym.Kind)};
+    CVSymbol Result(&Prefix, sizeof(Prefix));
     SymbolSerializer Serializer(Storage, Container);
     consumeError(Serializer.visitSymbolBegin(Result));
     consumeError(Serializer.visitKnownRecord(Result, Sym));

include/llvm/DebugInfo/CodeView/TypeDeserializer.h

@@ -58,7 +58,7 @@ public:
     TypeRecordKind K =
         static_cast<TypeRecordKind>(uint16_t(Prefix->RecordKind));
     T Record(K);
-    CVType CVT(static_cast<TypeLeafKind>(K), Data);
+    CVType CVT(Data);
     if (auto EC = deserializeAs<T>(CVT, Record))
       return std::move(EC);
     return Record;
@@ -111,14 +111,14 @@ class FieldListDeserializer : public TypeVisitorCallbacks {
 
 public:
   explicit FieldListDeserializer(BinaryStreamReader &Reader) : Mapping(Reader) {
-    CVType FieldList;
-    FieldList.Type = TypeLeafKind::LF_FIELDLIST;
+    RecordPrefix Pre(static_cast<uint16_t>(TypeLeafKind::LF_FIELDLIST));
+    CVType FieldList(&Pre, sizeof(Pre));
     consumeError(Mapping.Mapping.visitTypeBegin(FieldList));
   }
 
   ~FieldListDeserializer() override {
-    CVType FieldList;
-    FieldList.Type = TypeLeafKind::LF_FIELDLIST;
+    RecordPrefix Pre(static_cast<uint16_t>(TypeLeafKind::LF_FIELDLIST));
+    CVType FieldList(&Pre, sizeof(Pre));
     consumeError(Mapping.Mapping.visitTypeEnd(FieldList));
   }
 

lib/DebugInfo/CodeView/AppendingTypeTableBuilder.cpp

@@ -49,13 +49,8 @@ Optional<TypeIndex> AppendingTypeTableBuilder::getNext(TypeIndex Prev) {
   return Prev;
 }
 
-CVType AppendingTypeTableBuilder::getType(TypeIndex Index) {
-  CVType Type;
-  Type.RecordData = SeenRecords[Index.toArrayIndex()];
-  const RecordPrefix *P =
-      reinterpret_cast<const RecordPrefix *>(Type.RecordData.data());
-  Type.Type = static_cast<TypeLeafKind>(uint16_t(P->RecordKind));
-  return Type;
+CVType AppendingTypeTableBuilder::getType(TypeIndex Index){
+  return CVType(SeenRecords[Index.toArrayIndex()]);
 }
 
 StringRef AppendingTypeTableBuilder::getTypeName(TypeIndex Index) {

lib/DebugInfo/CodeView/CVSymbolVisitor.cpp

@@ -20,7 +20,7 @@ CVSymbolVisitor::CVSymbolVisitor(SymbolVisitorCallbacks &Callbacks)
 template <typename T>
 static Error visitKnownRecord(CVSymbol &Record,
                               SymbolVisitorCallbacks &Callbacks) {
-  SymbolRecordKind RK = static_cast<SymbolRecordKind>(Record.Type);
+  SymbolRecordKind RK = static_cast<SymbolRecordKind>(Record.kind());
   T KnownRecord(RK);
   if (auto EC = Callbacks.visitKnownRecord(Record, KnownRecord))
     return EC;
@@ -29,7 +29,7 @@ static Error visitKnownRecord(CVSymbol &Record,
 
 static Error finishVisitation(CVSymbol &Record,
                               SymbolVisitorCallbacks &Callbacks) {
-  switch (Record.Type) {
+  switch (Record.kind()) {
   default:
     if (auto EC = Callbacks.visitUnknownSymbol(Record))
       return EC;

lib/DebugInfo/CodeView/CVTypeVisitor.cpp

@@ -22,7 +22,7 @@ using namespace llvm::codeview;
 
 template <typename T>
 static Error visitKnownRecord(CVType &Record, TypeVisitorCallbacks &Callbacks) {
-  TypeRecordKind RK = static_cast<TypeRecordKind>(Record.Type);
+  TypeRecordKind RK = static_cast<TypeRecordKind>(Record.kind());
   T KnownRecord(RK);
   if (auto EC = Callbacks.visitKnownRecord(Record, KnownRecord))
     return EC;
@@ -96,7 +96,7 @@ CVTypeVisitor::CVTypeVisitor(TypeVisitorCallbacks &Callbacks)
     : Callbacks(Callbacks) {}
 
 Error CVTypeVisitor::finishVisitation(CVType &Record) {
-  switch (Record.Type) {
+  switch (Record.kind()) {
   default:
     if (auto EC = Callbacks.visitUnknownType(Record))
       return EC;

lib/DebugInfo/CodeView/ContinuationRecordBuilder.cpp

@@ -66,14 +66,11 @@ void ContinuationRecordBuilder::begin(ContinuationRecordKind RecordKind) {
   InjectedSegmentBytes =
       ArrayRef<uint8_t>(FLIB, FLIB + sizeof(SegmentInjection));
 
-  CVType Type;
-  Type.Type = getTypeLeafKind(RecordKind);
+  // Seed the first record with an appropriate record prefix.
+  RecordPrefix Prefix(getTypeLeafKind(RecordKind));
+  CVType Type(&Prefix, sizeof(Prefix));
   cantFail(Mapping.visitTypeBegin(Type));
 
-  // Seed the first trecord with an appropriate record prefix.
-  RecordPrefix Prefix;
-  Prefix.RecordLen = 0;
-  Prefix.RecordKind = Type.Type;
   cantFail(SegmentWriter.writeObject(Prefix));
 }
 
@@ -156,14 +153,9 @@ CVType ContinuationRecordBuilder::createSegmentRecord(
   MutableArrayRef<uint8_t> Data = Buffer.data();
   Data = Data.slice(OffBegin, OffEnd - OffBegin);
 
-  CVType Type;
-  Type.Type = getTypeLeafKind(*Kind);
-  Type.RecordData = Data;
-
   // Write the length to the RecordPrefix, making sure it does not include
   // sizeof(RecordPrefix.Length)
   RecordPrefix *Prefix = reinterpret_cast<RecordPrefix *>(Data.data());
-  assert(Prefix->RecordKind == Type.Type);
   Prefix->RecordLen = Data.size() - sizeof(RecordPrefix::RecordLen);
 
   if (RefersTo.hasValue()) {
@@ -175,12 +167,12 @@ CVType ContinuationRecordBuilder::createSegmentRecord(
     CR->IndexRef = RefersTo->getIndex();
   }
 
-  return Type;
+  return CVType(Data);
 }
 
 std::vector<CVType> ContinuationRecordBuilder::end(TypeIndex Index) {
-  CVType Type;
-  Type.Type = getTypeLeafKind(*Kind);
+  RecordPrefix Prefix(getTypeLeafKind(*Kind));
+  CVType Type(&Prefix, sizeof(Prefix));
   cantFail(Mapping.visitTypeEnd(Type));
 
   // We're now done, and we have a series of segments each beginning at an

lib/DebugInfo/CodeView/GlobalTypeTableBuilder.cpp

@@ -52,14 +52,7 @@ Optional<TypeIndex> GlobalTypeTableBuilder::getNext(TypeIndex Prev) {
 }
 
 CVType GlobalTypeTableBuilder::getType(TypeIndex Index) {
-  CVType Type;
-  Type.RecordData = SeenRecords[Index.toArrayIndex()];
-  if (!Type.RecordData.empty()) {
-    assert(Type.RecordData.size() >= sizeof(RecordPrefix));
-    const RecordPrefix *P =
-        reinterpret_cast<const RecordPrefix *>(Type.RecordData.data());
-    Type.Type = static_cast<TypeLeafKind>(uint16_t(P->RecordKind));
-  }
+  CVType Type(SeenRecords[Index.toArrayIndex()]);
   return Type;
 }
 

lib/DebugInfo/CodeView/MergingTypeTableBuilder.cpp

@@ -52,11 +52,7 @@ Optional<TypeIndex> MergingTypeTableBuilder::getNext(TypeIndex Prev) {
 }
 
 CVType MergingTypeTableBuilder::getType(TypeIndex Index) {
-  CVType Type;
-  Type.RecordData = SeenRecords[Index.toArrayIndex()];
-  const RecordPrefix *P =
-      reinterpret_cast<const RecordPrefix *>(Type.RecordData.data());
-  Type.Type = static_cast<TypeLeafKind>(uint16_t(P->RecordKind));
+  CVType Type(SeenRecords[Index.toArrayIndex()]);
   return Type;
 }
 

lib/DebugInfo/CodeView/SimpleTypeSerializer.cpp

@@ -3,13 +3,6 @@
 using namespace llvm;
 using namespace llvm::codeview;
 
-static void writeRecordPrefix(BinaryStreamWriter &Writer, TypeLeafKind Kind) {
-  RecordPrefix Prefix;
-  Prefix.RecordKind = Kind;
-  Prefix.RecordLen = 0;
-  cantFail(Writer.writeObject(Prefix));
-}
-
 static void addPadding(BinaryStreamWriter &Writer) {
   uint32_t Align = Writer.getOffset() % 4;
   if (Align == 0)
@@ -32,10 +25,12 @@ ArrayRef<uint8_t> SimpleTypeSerializer::serialize(T &Record) {
   BinaryStreamWriter Writer(ScratchBuffer, support::little);
   TypeRecordMapping Mapping(Writer);
 
-  CVType CVT;
-  CVT.Type = static_cast<TypeLeafKind>(Record.getKind());
+  // Write the record prefix first with a dummy length but real kind.
+  RecordPrefix DummyPrefix(uint16_t(Record.getKind()));
+  cantFail(Writer.writeObject(DummyPrefix));
 
-  writeRecordPrefix(Writer, CVT.Type);
+  RecordPrefix *Prefix = reinterpret_cast<RecordPrefix *>(ScratchBuffer.data());
+  CVType CVT(Prefix, sizeof(RecordPrefix));
 
   cantFail(Mapping.visitTypeBegin(CVT));
   cantFail(Mapping.visitKnownRecord(CVT, Record));
@@ -43,8 +38,7 @@ ArrayRef<uint8_t> SimpleTypeSerializer::serialize(T &Record) {
 
   addPadding(Writer);
 
-  RecordPrefix *Prefix = reinterpret_cast<RecordPrefix *>(ScratchBuffer.data());
-
+  // Update the size and kind after serialization.
   Prefix->RecordKind = CVT.kind();
   Prefix->RecordLen = Writer.getOffset() - sizeof(uint16_t);
 

lib/DebugInfo/CodeView/SymbolDumper.cpp

@@ -101,10 +101,10 @@ void CVSymbolDumperImpl::printTypeIndex(StringRef FieldName, TypeIndex TI) {
 }
 
 Error CVSymbolDumperImpl::visitSymbolBegin(CVSymbol &CVR) {
-  W.startLine() << getSymbolKindName(CVR.Type);
+  W.startLine() << getSymbolKindName(CVR.kind());
   W.getOStream() << " {\n";
   W.indent();
-  W.printEnum("Kind", unsigned(CVR.Type), getSymbolTypeNames());
+  W.printEnum("Kind", unsigned(CVR.kind()), getSymbolTypeNames());
   return Error::success();
 }
 

lib/DebugInfo/CodeView/TypeDumpVisitor.cpp

@@ -171,11 +171,11 @@ Error TypeDumpVisitor::visitTypeBegin(CVType &Record) {
 }
 
 Error TypeDumpVisitor::visitTypeBegin(CVType &Record, TypeIndex Index) {
-  W->startLine() << getLeafTypeName(Record.Type);
+  W->startLine() << getLeafTypeName(Record.kind());
   W->getOStream() << " (" << HexNumber(Index.getIndex()) << ")";
   W->getOStream() << " {\n";
   W->indent();
-  W->printEnum("TypeLeafKind", unsigned(Record.Type),
+  W->printEnum("TypeLeafKind", unsigned(Record.kind()),
                makeArrayRef(LeafTypeNames));
   return Error::success();
 }

lib/DebugInfo/CodeView/TypeRecordMapping.cpp

@@ -88,11 +88,11 @@ Error TypeRecordMapping::visitTypeBegin(CVType &CVR) {
   // split with continuation records.  All other record types cannot be
   // longer than the maximum record length.
   Optional<uint32_t> MaxLen;
-  if (CVR.Type != TypeLeafKind::LF_FIELDLIST &&
-      CVR.Type != TypeLeafKind::LF_METHODLIST)
+  if (CVR.kind() != TypeLeafKind::LF_FIELDLIST &&
+      CVR.kind() != TypeLeafKind::LF_METHODLIST)
     MaxLen = MaxRecordLength - sizeof(RecordPrefix);
   error(IO.beginRecord(MaxLen));
-  TypeKind = CVR.Type;
+  TypeKind = CVR.kind();
   return Error::success();
 }
 
@@ -211,9 +211,9 @@ Error TypeRecordMapping::visitKnownRecord(CVType &CVR, ArrayRecord &Record) {
 }
 
 Error TypeRecordMapping::visitKnownRecord(CVType &CVR, ClassRecord &Record) {
-  assert((CVR.Type == TypeLeafKind::LF_STRUCTURE) ||
-         (CVR.Type == TypeLeafKind::LF_CLASS) ||
-         (CVR.Type == TypeLeafKind::LF_INTERFACE));
+  assert((CVR.kind() == TypeLeafKind::LF_STRUCTURE) ||
+         (CVR.kind() == TypeLeafKind::LF_CLASS) ||
+         (CVR.kind() == TypeLeafKind::LF_INTERFACE));
 
   error(IO.mapInteger(Record.MemberCount));
   error(IO.mapEnum(Record.Options));

lib/DebugInfo/CodeView/TypeTableCollection.cpp

@@ -36,11 +36,7 @@ Optional<TypeIndex> TypeTableCollection::getNext(TypeIndex Prev) {
 
 CVType TypeTableCollection::getType(TypeIndex Index) {
   assert(Index.toArrayIndex() < Records.size());
-  ArrayRef<uint8_t> Bytes = Records[Index.toArrayIndex()];
-  const RecordPrefix *Prefix =
-      reinterpret_cast<const RecordPrefix *>(Bytes.data());
-  TypeLeafKind Kind = static_cast<TypeLeafKind>(uint16_t(Prefix->RecordKind));
-  return CVType(Kind, Bytes);
+  return CVType(Records[Index.toArrayIndex()]);
 }
 
 StringRef TypeTableCollection::getTypeName(TypeIndex Index) {

lib/DebugInfo/PDB/Native/GSIStreamBuilder.cpp

@@ -36,8 +36,8 @@ struct llvm::pdb::GSIHashStreamBuilder {
       return Empty;
     }
     static inline CVSymbol getTombstoneKey() {
-      static CVSymbol Tombstone(static_cast<SymbolKind>(-1),
-                                ArrayRef<uint8_t>());
+      static CVSymbol Tombstone(
+          DenseMapInfo<ArrayRef<uint8_t>>::getTombstoneKey());
       return Tombstone;
     }
     static unsigned getHashValue(const CVSymbol &Val) {

lib/ObjectYAML/CodeViewYAMLSymbols.cpp

@@ -248,7 +248,7 @@ struct UnknownSymbolRecord : public SymbolRecordBase {
     uint8_t *Buffer = Allocator.Allocate<uint8_t>(TotalLen);
     ::memcpy(Buffer, &Prefix, sizeof(RecordPrefix));
     ::memcpy(Buffer + sizeof(RecordPrefix), Data.data(), Data.size());
-    return CVSymbol(Kind, ArrayRef<uint8_t>(Buffer, TotalLen));
+    return CVSymbol(ArrayRef<uint8_t>(Buffer, TotalLen));
   }
 
   Error fromCodeViewSymbol(CVSymbol CVS) override {

lib/ObjectYAML/CodeViewYAMLTypes.cpp

@@ -98,7 +98,7 @@ template <typename T> struct LeafRecordImpl : public LeafRecordBase {
 
   CVType toCodeViewRecord(AppendingTypeTableBuilder &TS) const override {
     TS.writeLeafType(Record);
-    return CVType(Kind, TS.records().back());
+    return CVType(TS.records().back());
   }
 
   mutable T Record;
@@ -496,7 +496,7 @@ CVType LeafRecordImpl<FieldListRecord>::toCodeViewRecord(
     Member.Member->writeTo(CRB);
   }
   TS.insertRecord(CRB);
-  return CVType(Kind, TS.records().back());
+  return CVType(TS.records().back());
 }
 
 void MappingTraits<OneMethodRecord>::mapping(IO &io, OneMethodRecord &Record) {

tools/llvm-pdbutil/MinimalSymbolDumper.cpp

@@ -331,7 +331,7 @@ Error MinimalSymbolDumper::visitSymbolBegin(codeview::CVSymbol &Record,
   // append to the existing line.
   P.formatLine("{0} | {1} [size = {2}]",
                fmt_align(Offset, AlignStyle::Right, 6),
-               formatSymbolKind(Record.Type), Record.length());
+               formatSymbolKind(Record.kind()), Record.length());
   P.Indent();
   return Error::success();
 }

tools/llvm-pdbutil/MinimalTypeDumper.cpp

@@ -224,7 +224,7 @@ Error MinimalTypeDumpVisitor::visitTypeBegin(CVType &Record, TypeIndex Index) {
   // append to the existing line.
   P.formatLine("{0} | {1} [size = {2}",
                fmt_align(Index, AlignStyle::Right, Width),
-               formatTypeLeafKind(Record.Type), Record.length());
+               formatTypeLeafKind(Record.kind()), Record.length());
   if (Hashes) {
     std::string H;
     if (Index.toArrayIndex() >= HashValues.size()) {

unittests/DebugInfo/CodeView/RandomAccessVisitorTest.cpp

@@ -42,8 +42,6 @@ inline bool operator!=(const ArrayRecord &R1, const ArrayRecord &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;
@@ -107,7 +105,7 @@ public:
       GlobalState->Records.push_back(AR);
       GlobalState->Indices.push_back(Builder.writeLeafType(AR));
 
-      CVType Type(TypeLeafKind::LF_ARRAY, Builder.records().back());
+      CVType Type(Builder.records().back());
       GlobalState->TypeVector.push_back(Type);
 
       GlobalState->AllOffsets.push_back(
@@ -369,11 +367,10 @@ TEST_F(RandomAccessVisitorTest, CrossChunkName) {
   TypeIndex IndexOne = Builder.writeLeafType(Modifier);
 
   // set up a type stream that refers to the above two serialized records.
-  std::vector<CVType> TypeArray;
-  TypeArray.push_back(
-      CVType(static_cast<TypeLeafKind>(Class.Kind), Builder.records()[0]));
-  TypeArray.push_back(
-      CVType(static_cast<TypeLeafKind>(Modifier.Kind), Builder.records()[1]));
+  std::vector<CVType> TypeArray = {
+      {Builder.records()[0]},
+      {Builder.records()[1]},
+  };
   BinaryItemStream<CVType> ItemStream(llvm::support::little);
   ItemStream.setItems(TypeArray);
   VarStreamArray<CVType> TypeStream(ItemStream);