[PATCH] D37157: Fix Bug 30978 by emitting cv file checksums.

[Zachary Turner via Phabricator via llvm-commits]

zturner added a comment.

Try to be more careful so as not to submit testing code / comments etc in uploaded patches.  Reasons like this are why I prefer using a debugger instead of printf debugging :)



================
Comment at: llvm/include/llvm/IR/DebugInfoMetadata.h:476-481
+  enum class ChecksumKind : uint8_t {
     CSK_None,
     CSK_MD5,
     CSK_SHA1,
     CSK_Last = CSK_SHA1 // Should be last enumeration.
   };
----------------
If you're going to make it an enum class, then it doesn't need the `CSK` prefix in the name, since you have to write `ChecksumKind` anyway.  I wouldn't suggest churning the code to change the names everywhere it's used, so instead maybe just leave this as a regular enum.  It's already scoped to `DIFile` anyway, so there's no risk of namespace pollution.


================
Comment at: llvm/include/llvm/MC/MCCodeView.h:297-300
   SmallVector<StringRef, 4> Filenames;
 
+  // Map from filename to checksum.
+  StringMap<std::pair<StringRef, uint8_t>> Checksums;
----------------
Any reason this isn't just:

```
struct FileInfo {
  StringRef Name;
  StringRef Checksum;
  ChecksumKind Kind;
};

SmallVector<FileInfo> Files;
```

having a `StringMap` seems unnecessary if we're already iterating a linear container anyway 


================
Comment at: llvm/lib/DebugInfo/CodeView/DebugChecksumsSubsection.cpp:53
 Error DebugChecksumsSubsectionRef::initialize(BinaryStreamReader Reader) {
+  //errs() << "checksum bytes remaining " << Reader.bytesRemaining() << "\n";
   if (auto EC = Reader.readArray(Checksums, Reader.bytesRemaining()))
----------------
Delete


================
Comment at: llvm/lib/DebugInfo/CodeView/DebugStringTableSubsection.cpp:32
 Error DebugStringTableSubsectionRef::initialize(BinaryStreamReader &Reader) {
+  errs() << "String table bytes remaining " << Reader.bytesRemaining() << "\n";
   return Reader.readStreamRef(Stream);
----------------
Delete


================
Comment at: llvm/lib/DebugInfo/CodeView/DebugStringTableSubsection.cpp:38
 DebugStringTableSubsectionRef::getString(uint32_t Offset) const {
+  //errs() << "DebugStringTableSubsectionRef getString\n";
   BinaryStreamReader Reader(Stream);
----------------
Delete


================
Comment at: llvm/lib/DebugInfo/CodeView/DebugStringTableSubsection.cpp:44
     return std::move(EC);
+  //errs() << "leaving getString\n";
   return Result;
----------------
Delete


================
Comment at: llvm/lib/MC/MCCodeView.cpp:188-200
+    // std::pair<StringRef, uint8_t> Checksum = Checksums[Filename];
+    // if (!Checksum.second) {
+    //   // There is no checksum.
+    //   OS.EmitIntValue(0, 4);
+    //   continue;
+    // }
+    //OS.EmitIntValue(static_cast<uint8_t>(Checksum.first.size()), 1);
----------------
What is this?  A bunch of code commented out, and then emitting a bunch of magic numbers?


================
Comment at: llvm/tools/llvm-readobj/COFFDumper.cpp:927
     case DebugSubsectionKind::StringTable:
+      errs() << "initializing string table\n";
       error(CVStringTable.initialize(ST));
----------------
Delete


================
Comment at: llvm/tools/llvm-readobj/COFFDumper.cpp:929
       error(CVStringTable.initialize(ST));
+      errs() << "initialized string table\n";
       break;
----------------
Delete


================
Comment at: llvm/tools/llvm-readobj/COFFDumper.cpp:1088
       ListScope S(W, "FilenameSegment");
+      //errs() << "about to pring filename with offset " << Entry.NameIndex << "\n";
       printFileNameForOffset("Filename", Entry.NameIndex);
----------------
Delete


================
Comment at: llvm/tools/llvm-readobj/COFFDumper.cpp:1090
       printFileNameForOffset("Filename", Entry.NameIndex);
+      //errs() << "printed filename\n";
       uint32_t ColumnIndex = 0;
----------------
Delete


https://reviews.llvm.org/D37157