[PATCH] D21393: [pdbdump] Verify LF_{CLASS, ENUM, INTERFACE, STRUCTURE, UNION} records with different hash function.

Zachary Turner via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 16 11:12:41 PDT 2016

zturner added inline comments.

Comment at: lib/DebugInfo/PDB/Raw/TpiStream.cpp:69
@@ -69,1 +68,3 @@
+template <typename T>
+static uint32_t getTpiHash(T &Rec, const CVRecord<TypeLeafKind> &RawRec) {
   auto Opts = static_cast<uint16_t>(Rec.getOptions());
I was imagining that we wouldn't even have this `TpiHashVerifier` class, and that everything would just go in `TypeDumper`.  Does this not work?  For example, imagine you delete all of this code, and delete the `TpiHashVerifier` class, and delete lines 205-209.  Then, in `TypeDumper.cpp`, you change `CVTypeDumper::visitClass` to look like this:

Error CVTypeDumper::visitClass(ClassRecord &Class) {
  uint16_t Props = static_cast<uint16_t>(Class.getOptions());
  W->printNumber("MemberCount", Class.getMemberCount());
  W->printFlags("Properties", Props, makeArrayRef(ClassOptionNames));
  printTypeIndex("FieldList", Class.getFieldList());
  printTypeIndex("DerivedFrom", Class.getDerivationList());
  printTypeIndex("VShape", Class.getVTableShape());
  W->printNumber("SizeOf", Class.getSize());
  W->printString("Name", Class.getName());
  if (Props & uint16_t(ClassOptions::HasUniqueName))
    W->printString("LinkageName", Class.getUniqueName());
  Name = Class.getName();

  if (auto EC = verifyHash(Class))
    return EC;
  return Error::success();


With the current implementation, we walk the entire type array twice.  Once to verify hashes, and once to dump them.  So if we could do it all in one pass, it seems better.


More information about the llvm-commits mailing list