[CodeView] Properly align symbol records on read/write.

Object files have symbol records not aligned to any particular
boundary (e.g. 1-byte aligned), while PDB files have symbol
records padded to 4-byte aligned boundaries.  Since they share
the same reading / writing code, we have to provide an option to
specify the alignment and propagate it up to the producer or
consumer who knows what the alignment is supposed to be for the
given container type.

Added a test for this by modifying the existing PDB -> YAML -> PDB
round-tripping code to round trip symbol records as well as types.

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

llvm-svn: 304484
diff --git a/llvm/lib/DebugInfo/CodeView/CodeViewRecordIO.cpp b/llvm/lib/DebugInfo/CodeView/CodeViewRecordIO.cpp
index 282e310..711144f 100644
--- a/llvm/lib/DebugInfo/CodeView/CodeViewRecordIO.cpp
+++ b/llvm/lib/DebugInfo/CodeView/CodeViewRecordIO.cpp
@@ -27,6 +27,14 @@
 Error CodeViewRecordIO::endRecord() {
   assert(!Limits.empty() && "Not in a record!");
   Limits.pop_back();
+  // We would like to assert that we actually read / wrote all the bytes that we
+  // expected to for this record, but unfortunately we can't do this.  Some
+  // producers such as MASM over-allocate for certain types of records and
+  // commit the extraneous data, so when reading we can't be sure every byte
+  // will have been read.  And when writing we over-allocate temporarily since
+  // we don't know how big the record is until we're finished writing it, so
+  // even though we don't commit the extraneous data, we still can't guarantee
+  // we're at the end of the allocated data.
   return Error::success();
 }
 
@@ -49,6 +57,12 @@
   return *Min;
 }
 
+Error CodeViewRecordIO::padToAlignment(uint32_t Align) {
+  if (isReading())
+    return Reader->padToAlignment(Align);
+  return Writer->padToAlignment(Align);
+}
+
 Error CodeViewRecordIO::skipPadding() {
   assert(!isWriting() && "Cannot skip padding while writing!");
 
diff --git a/llvm/lib/DebugInfo/CodeView/DebugSubsectionRecord.cpp b/llvm/lib/DebugInfo/CodeView/DebugSubsectionRecord.cpp
index 511f36d0..3d1510c 100644
--- a/llvm/lib/DebugInfo/CodeView/DebugSubsectionRecord.cpp
+++ b/llvm/lib/DebugInfo/CodeView/DebugSubsectionRecord.cpp
@@ -16,14 +16,17 @@
 using namespace llvm::codeview;
 
 DebugSubsectionRecord::DebugSubsectionRecord()
-    : Kind(DebugSubsectionKind::None) {}
+    : Kind(DebugSubsectionKind::None),
+      Container(CodeViewContainer::ObjectFile) {}
 
 DebugSubsectionRecord::DebugSubsectionRecord(DebugSubsectionKind Kind,
-                                             BinaryStreamRef Data)
-    : Kind(Kind), Data(Data) {}
+                                             BinaryStreamRef Data,
+                                             CodeViewContainer Container)
+    : Kind(Kind), Data(Data), Container(Container) {}
 
 Error DebugSubsectionRecord::initialize(BinaryStreamRef Stream,
-                                        DebugSubsectionRecord &Info) {
+                                        DebugSubsectionRecord &Info,
+                                        CodeViewContainer Container) {
   const DebugSubsectionHeader *Header;
   BinaryStreamReader Reader(Stream);
   if (auto EC = Reader.readObject(Header))
@@ -41,13 +44,14 @@
   }
   if (auto EC = Reader.readStreamRef(Info.Data, Header->Length))
     return EC;
+  Info.Container = Container;
   Info.Kind = Kind;
   return Error::success();
 }
 
 uint32_t DebugSubsectionRecord::getRecordLength() const {
   uint32_t Result = sizeof(DebugSubsectionHeader) + Data.getLength();
-  assert(Result % 4 == 0);
+  assert(Result % alignOf(Container) == 0);
   return Result;
 }
 
@@ -56,16 +60,20 @@
 BinaryStreamRef DebugSubsectionRecord::getRecordData() const { return Data; }
 
 DebugSubsectionRecordBuilder::DebugSubsectionRecordBuilder(
-    DebugSubsectionKind Kind, DebugSubsection &Frag)
-    : Kind(Kind), Frag(Frag) {}
+    DebugSubsectionKind Kind, DebugSubsection &Frag,
+    CodeViewContainer Container)
+    : Kind(Kind), Frag(Frag), Container(Container) {}
 
 uint32_t DebugSubsectionRecordBuilder::calculateSerializedLength() {
   uint32_t Size = sizeof(DebugSubsectionHeader) +
-                  alignTo(Frag.calculateSerializedSize(), 4);
+                  alignTo(Frag.calculateSerializedSize(), alignOf(Container));
   return Size;
 }
 
 Error DebugSubsectionRecordBuilder::commit(BinaryStreamWriter &Writer) {
+  assert(Writer.getOffset() % alignOf(Container) == 0 &&
+         "Debug Subsection not properly aligned");
+
   DebugSubsectionHeader Header;
   Header.Kind = uint32_t(Kind);
   Header.Length = calculateSerializedLength() - sizeof(DebugSubsectionHeader);
@@ -74,7 +82,7 @@
     return EC;
   if (auto EC = Frag.commit(Writer))
     return EC;
-  if (auto EC = Writer.padToAlignment(4))
+  if (auto EC = Writer.padToAlignment(alignOf(Container)))
     return EC;
 
   return Error::success();
diff --git a/llvm/lib/DebugInfo/CodeView/SymbolDumper.cpp b/llvm/lib/DebugInfo/CodeView/SymbolDumper.cpp
index 3d49a71..6604593 100644
--- a/llvm/lib/DebugInfo/CodeView/SymbolDumper.cpp
+++ b/llvm/lib/DebugInfo/CodeView/SymbolDumper.cpp
@@ -668,7 +668,7 @@
 
 Error CVSymbolDumper::dump(CVRecord<SymbolKind> &Record) {
   SymbolVisitorCallbackPipeline Pipeline;
-  SymbolDeserializer Deserializer(ObjDelegate.get());
+  SymbolDeserializer Deserializer(ObjDelegate.get(), Container);
   CVSymbolDumperImpl Dumper(Types, ObjDelegate.get(), W, PrintRecordBytes);
 
   Pipeline.addCallbackToPipeline(Deserializer);
@@ -679,7 +679,7 @@
 
 Error CVSymbolDumper::dump(const CVSymbolArray &Symbols) {
   SymbolVisitorCallbackPipeline Pipeline;
-  SymbolDeserializer Deserializer(ObjDelegate.get());
+  SymbolDeserializer Deserializer(ObjDelegate.get(), Container);
   CVSymbolDumperImpl Dumper(Types, ObjDelegate.get(), W, PrintRecordBytes);
 
   Pipeline.addCallbackToPipeline(Deserializer);
diff --git a/llvm/lib/DebugInfo/CodeView/SymbolRecordMapping.cpp b/llvm/lib/DebugInfo/CodeView/SymbolRecordMapping.cpp
index bb17314..ea46841 100644
--- a/llvm/lib/DebugInfo/CodeView/SymbolRecordMapping.cpp
+++ b/llvm/lib/DebugInfo/CodeView/SymbolRecordMapping.cpp
@@ -40,6 +40,7 @@
 }
 
 Error SymbolRecordMapping::visitSymbolEnd(CVSymbol &Record) {
+  error(IO.padToAlignment(alignOf(Container)));
   error(IO.endRecord());
   return Error::success();
 }
diff --git a/llvm/lib/DebugInfo/CodeView/SymbolSerializer.cpp b/llvm/lib/DebugInfo/CodeView/SymbolSerializer.cpp
index 251cc43..9f2d619 100644
--- a/llvm/lib/DebugInfo/CodeView/SymbolSerializer.cpp
+++ b/llvm/lib/DebugInfo/CodeView/SymbolSerializer.cpp
@@ -12,9 +12,11 @@
 using namespace llvm;
 using namespace llvm::codeview;
 
-SymbolSerializer::SymbolSerializer(BumpPtrAllocator &Allocator)
-  : Storage(Allocator), RecordBuffer(MaxRecordLength), Stream(RecordBuffer, llvm::support::little),
-  Writer(Stream), Mapping(Writer) { }
+SymbolSerializer::SymbolSerializer(BumpPtrAllocator &Allocator,
+                                   CodeViewContainer Container)
+    : Storage(Allocator), RecordBuffer(MaxRecordLength),
+      Stream(RecordBuffer, llvm::support::little), Writer(Stream),
+      Mapping(Writer, Container) {}
 
 Error SymbolSerializer::visitSymbolBegin(CVSymbol &Record) {
   assert(!CurrentSymbol.hasValue() && "Already in a symbol mapping!");
diff --git a/llvm/lib/DebugInfo/PDB/Native/DbiModuleDescriptorBuilder.cpp b/llvm/lib/DebugInfo/PDB/Native/DbiModuleDescriptorBuilder.cpp
index b28ec2f..bf3f837 100644
--- a/llvm/lib/DebugInfo/PDB/Native/DbiModuleDescriptorBuilder.cpp
+++ b/llvm/lib/DebugInfo/PDB/Native/DbiModuleDescriptorBuilder.cpp
@@ -66,7 +66,11 @@
 
 void DbiModuleDescriptorBuilder::addSymbol(CVSymbol Symbol) {
   Symbols.push_back(Symbol);
-  SymbolByteSize += Symbol.data().size();
+  // Symbols written to a PDB file are required to be 4 byte aligned.  The same
+  // is not true of object files.
+  assert(Symbol.length() % alignOf(CodeViewContainer::Pdb) == 0 &&
+         "Invalid Symbol alignment!");
+  SymbolByteSize += Symbol.length();
 }
 
 void DbiModuleDescriptorBuilder::addSourceFile(StringRef Path) {
@@ -153,7 +157,8 @@
     if (auto EC = SymbolWriter.writeStreamRef(RecordsRef))
       return EC;
     // TODO: Write C11 Line data
-
+    assert(SymbolWriter.getOffset() % alignOf(CodeViewContainer::Pdb) == 0 &&
+           "Invalid debug section alignment!");
     for (const auto &Builder : C13Builders) {
       assert(Builder && "Empty C13 Fragment Builder!");
       if (auto EC = Builder->commit(SymbolWriter))
@@ -179,8 +184,8 @@
     C13Builders.push_back(nullptr);
 
   this->LineInfo.push_back(std::move(Lines));
-  C13Builders.push_back(
-      llvm::make_unique<DebugSubsectionRecordBuilder>(Frag.kind(), Frag));
+  C13Builders.push_back(llvm::make_unique<DebugSubsectionRecordBuilder>(
+      Frag.kind(), Frag, CodeViewContainer::Pdb));
 }
 
 void DbiModuleDescriptorBuilder::addC13Fragment(
@@ -193,8 +198,8 @@
     C13Builders.push_back(nullptr);
 
   this->Inlinees.push_back(std::move(Inlinees));
-  C13Builders.push_back(
-      llvm::make_unique<DebugSubsectionRecordBuilder>(Frag.kind(), Frag));
+  C13Builders.push_back(llvm::make_unique<DebugSubsectionRecordBuilder>(
+      Frag.kind(), Frag, CodeViewContainer::Pdb));
 }
 
 void DbiModuleDescriptorBuilder::setC13FileChecksums(
@@ -206,5 +211,5 @@
 
   ChecksumInfo = std::move(Checksums);
   C13Builders[0] = llvm::make_unique<DebugSubsectionRecordBuilder>(
-      ChecksumInfo->kind(), *ChecksumInfo);
+      ChecksumInfo->kind(), *ChecksumInfo, CodeViewContainer::Pdb);
 }