[PATCH] D33748: [PDB] Fix alignment of symbol record string

[Zachary Turner via Phabricator via llvm-commits]

zturner created this revision.

The symbol record substream needs to be written at a multiple of 4 bytes.  This is because the next field that is read is the beginning of the module debug subsections (checksums, lines, etc) and the first field is a uint32.

I found this by round-tripping empty.pdb from the test folder and seeing it give me an error.  To determine the correct fix, I stepped through the reading code of empty.pdb and saw that the Symbol Record Substream consisted of 208 bytes, a multiple of 4.  This is the value that the header reported for the "symbol record substream size", as well as the number of subsequent bytes we read and attributed to the symbol record substream.  When I converted this file to yaml, and then converted it back to pdb, we were writing 206 bytes for this value, and then writing 206 bytes of data.


https://reviews.llvm.org/D33748

Files:
  llvm/lib/DebugInfo/PDB/Native/DbiModuleDescriptorBuilder.cpp


Index: llvm/lib/DebugInfo/PDB/Native/DbiModuleDescriptorBuilder.cpp
===================================================================
--- llvm/lib/DebugInfo/PDB/Native/DbiModuleDescriptorBuilder.cpp
+++ llvm/lib/DebugInfo/PDB/Native/DbiModuleDescriptorBuilder.cpp
@@ -38,12 +38,12 @@
 
 static uint32_t calculateDiSymbolStreamSize(uint32_t SymbolByteSize,
                                             uint32_t C13Size) {
-  uint32_t Size = sizeof(uint32_t); // Signature
-  Size += SymbolByteSize;           // Symbol Data
-  Size += 0;                        // TODO: Layout.C11Bytes
-  Size += C13Size;                  // C13 Debug Info Size
-  Size += sizeof(uint32_t);         // GlobalRefs substream size (always 0)
-  Size += 0;                        // GlobalRefs substream bytes
+  uint32_t Size = sizeof(uint32_t);   // Signature
+  Size += alignTo(SymbolByteSize, 4); // Symbol Data
+  Size += 0;                          // TODO: Layout.C11Bytes
+  Size += C13Size;                    // C13 Debug Info Size
+  Size += sizeof(uint32_t);           // GlobalRefs substream size (always 0)
+  Size += 0;                          // GlobalRefs substream bytes
   return Size;
 }
 
@@ -110,7 +110,7 @@
 
   // This value includes both the signature field as well as the record bytes
   // from the symbol stream.
-  Layout.SymBytes = SymbolByteSize + sizeof(uint32_t);
+  Layout.SymBytes = alignTo(SymbolByteSize + sizeof(uint32_t), 4);
 }
 
 Error DbiModuleDescriptorBuilder::finalizeMsfLayout() {
@@ -152,13 +152,17 @@
     BinaryStreamRef RecordsRef(Records);
     if (auto EC = SymbolWriter.writeStreamRef(RecordsRef))
       return EC;
+    if (auto EC = SymbolWriter.padToAlignment(4))
+      return EC;
     // TODO: Write C11 Line data
 
     for (const auto &Builder : C13Builders) {
+      assert(SymbolWriter.getOffset() % 4 == 0);
       assert(Builder && "Empty C13 Fragment Builder!");
       if (auto EC = Builder->commit(SymbolWriter))
         return EC;
     }
+    assert(SymbolWriter.getOffset() % 4 == 0);
 
     // TODO: Figure out what GlobalRefs substream actually is and populate it.
     if (auto EC = SymbolWriter.writeInteger<uint32_t>(0))


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D33748.100942.patch
Type: text/x-patch
Size: 2178 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170601/40da8efb/attachment.bin>