[PDB] Fix reading of clang-generated PDBs by CVDump.

[zturner]

Summary

If you dump a pdb to yaml, and then round-trip it back to a pdb, and run cvdump -l <file> on the new pdb, cvdump will generate output such as this.

Microsoft (R) Debugging Information Dumper  Version 14.00.23611
Copyright (C) Microsoft Corporation.  All rights reserved.


*** LINES

** Module: "d:\src\llvm\test\DebugInfo\PDB\Inputs\empty.obj"

Error: Line number corrupted: invalid file id 0
  <Unknown> (MD5), 0001:00000010-0000001A, line/addr pairs = 3

      5 00000010      6 00000013      7 00000018

Note the error message about the corrupted line number.

It turns out that the problem is that cvdump cannot find the /names stream (e.g. the global string table), and the reason it can't find the /names stream is because it doesn't understand the NameMap that we serialize which tells pdb consumers which stream has the string table.

Some experimentation shows that if we add items to the hash table in a specific order before serializing it, cvdump can read it. This suggests that we're using the wrong hash function, but it will take some deeper investigation to figure out how / why. For now, this at least allows cvdump to read our line information.

[rnk]

lgtm =/

[amccarth]

llvm/lib/DebugInfo/PDB/Native/NamedStreamMap.cpp

87 ↗

(On Diff #103503)

Just to make sure I understand ... the old way iterated through everything in Mapping, and the new way iterates only through the three streams in OrderedStreamNames. Will there ever be anything in the Mapping besides those three streams?

[zturner]

llvm/lib/DebugInfo/PDB/Native/NamedStreamMap.cpp

87 ↗

(On Diff #103503)

Not quite right. The new way still iterates through everything in Mapping. These 3 strings are the only 3 strings that were ever there to begin with. The difference is that when iterating Mapping, you can't guarantee the order the results will be returned in, so they were being returned in a different order. We still end up with the same hash table, we've just inserted the items in a determined ordering.

[amccarth]

Thanks for the clarification.

llvm/trunk/lib/DebugInfo/PDB/Native/NamedStreamMap.cpp

 #include "llvm/Support/BinaryStreamReader.h"
 #include "llvm/Support/Error.h"
 #include <algorithm>
 #include <cstdint>
 
 using namespace llvm;
 using namespace llvm::pdb;
 
+// FIXME: This shouldn't be necessary, but if we insert the strings in any
+// other order, cvdump cannot read the generated name map.  This suggests that
+// we may be using the wrong hash function.  A closer inspection of the cvdump
+// source code may reveal something, but for now this at least makes us work,
+// even if only by accident.
+static constexpr const char *OrderedStreamNames[] = {"/LinkInfo", "/names",
+                                                     "/src/headerblock"};
+
 NamedStreamMap::NamedStreamMap() = default;
 
 Error NamedStreamMap::load(BinaryStreamReader &Stream) {
   Mapping.clear();
   FinalizedHashTable.clear();
   FinalizedInfo.reset();
 
   uint32_t StringBufferSize;

 
 Error NamedStreamMap::commit(BinaryStreamWriter &Writer) const {
   assert(FinalizedInfo.hasValue());
 
   // The first field is the number of bytes of string data.
   if (auto EC = Writer.writeInteger(FinalizedInfo->StringDataBytes))
     return EC;
 
-  // Now all of the string data itself.
-  for (const auto &Item : Mapping) {
-    if (auto EC = Writer.writeCString(Item.getKey()))
+  for (const auto &Name : OrderedStreamNames) {
+    auto Item = Mapping.find(Name);
+    assert(Item != Mapping.end());
+    if (auto EC = Writer.writeCString(Item->getKey()))
       return EC;
   }
 
   // And finally the Offset Index map.
   if (auto EC = FinalizedHashTable.commit(Writer))
     return EC;
 
   return Error::success();
 }
 
 uint32_t NamedStreamMap::finalize() {
   if (FinalizedInfo.hasValue())
     return FinalizedInfo->SerializedLength;
 
   // Build the finalized hash table.
   FinalizedHashTable.clear();
   FinalizedInfo.emplace();
-  for (const auto &Item : Mapping) {
-    FinalizedHashTable.set(FinalizedInfo->StringDataBytes, Item.getValue());
-    FinalizedInfo->StringDataBytes += Item.getKeyLength() + 1;
+
+  for (const auto &Name : OrderedStreamNames) {
+    auto Item = Mapping.find(Name);
+    assert(Item != Mapping.end());
+    FinalizedHashTable.set(FinalizedInfo->StringDataBytes, Item->getValue());
+    FinalizedInfo->StringDataBytes += Item->getKeyLength() + 1;
   }
 
   // Number of bytes of string data.
   FinalizedInfo->SerializedLength += sizeof(support::ulittle32_t);
   // Followed by that many actual bytes of string data.
   FinalizedInfo->SerializedLength += FinalizedInfo->StringDataBytes;
   // Followed by the mapping from Offset to Index.
   FinalizedInfo->SerializedLength +=