[CodeView] Align type records on 4-bytes when emitting PDBs

When emitting PDBs, the TypeStreamMerger class is used to merge .debug$T records from the input .OBJ files into the output .PDB stream.
Records in .OBJs are not required to be aligned on 4-bytes, and "The Netwide Assembler 2.14" generates non-aligned records.

When compiling with -DLLVM_ENABLE_ASSERTIONS=ON, an assert was triggered in MergingTypeTableBuilder when non-ghash merging was used.
With ghash merging there was no assert.
As a result, LLD could potentially generate a non-aligned TPI stream.

We now align records on 4-bytes when record indices are remapped, in TypeStreamMerger::remapIndices().

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

(cherry picked from commit a732529)

Author: aganea

lld/test/COFF/pdb-tpi-aligned-records.test

@@ -0,0 +1,46 @@
+# RUN: yaml2obj < %s > %t.obj
+# RUN: yaml2obj %p/Inputs/generic.yaml > %t2.obj
+
+# RUN: lld-link /out:%t.exe /debug /entry:main %t.obj %t2.obj /nodefaultlib
+# RUN: llvm-pdbutil dump --types --type-data %t.pdb | FileCheck %s
+# RUN: lld-link /out:%t.exe /debug:ghash /entry:main %t.obj %t2.obj /nodefaultlib
+# RUN: llvm-pdbutil dump --types --type-data %t.pdb | FileCheck %s
+
+# CHECK: 0000: 12000810 03000000 00000000 00000000 0000F2F1
+
+--- !COFF
+header:
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: []
+sections:
+  - Name:            '.debug$T'
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
+    Alignment:       1
+    # It is important to keep the 'SectionData' since the .OBJ is reconstructed from it,
+    # and that triggers an alignement bug in the output .PDB.
+    SectionData:     '040000001000081003000000000000000000000000000600011200000000'
+    Types:
+      - Kind:            LF_PROCEDURE
+        Procedure:
+          ReturnType:      3
+          CallConv:        NearC
+          Options:         [ None ]
+          ParameterCount:  0
+          ArgumentList:    0
+      - Kind:            LF_ARGLIST
+        ArgList:
+          ArgIndices:      [  ]
+symbols:
+  - Name:            '.debug$T'
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition:
+      Length:          30
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        0
+      Number:          0
+...

llvm/include/llvm/DebugInfo/CodeView/GlobalTypeTableBuilder.h

@@ -71,6 +71,11 @@ class GlobalTypeTableBuilder : public TypeCollection {
   template <typename CreateFunc>
   TypeIndex insertRecordAs(GloballyHashedType Hash, size_t RecordSize,
                            CreateFunc Create) {
+    assert(RecordSize < UINT32_MAX && "Record too big");
+    assert(RecordSize % 4 == 0 &&
+           "RecordSize is not a multiple of 4 bytes which will cause "
+           "misalignment in the output TPI stream!");
+
     auto Result = HashedRecords.try_emplace(Hash, nextTypeIndex());
 
     if (LLVM_UNLIKELY(Result.second /*inserted*/ ||

llvm/lib/DebugInfo/CodeView/MergingTypeTableBuilder.cpp

@@ -90,7 +90,9 @@ static inline ArrayRef<uint8_t> stabilize(BumpPtrAllocator &Alloc,
 TypeIndex MergingTypeTableBuilder::insertRecordAs(hash_code Hash,
                                                   ArrayRef<uint8_t> &Record) {
   assert(Record.size() < UINT32_MAX && "Record too big");
-  assert(Record.size() % 4 == 0 && "Record is not aligned to 4 bytes!");
+  assert(Record.size() % 4 == 0 &&
+         "The type record size is not a multiple of 4 bytes which will cause "
+         "misalignment in the output TPI stream!");
 
   LocallyHashedType WeakHash{Hash, Record};
   auto Result = HashedRecords.try_emplace(WeakHash, nextTypeIndex());

llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp

@@ -360,16 +360,18 @@ Error TypeStreamMerger::remapType(const CVType &Type) {
         [this, Type](MutableArrayRef<uint8_t> Storage) -> ArrayRef<uint8_t> {
       return remapIndices(Type, Storage);
     };
+    unsigned AlignedSize = alignTo(Type.RecordData.size(), 4);
+
     if (LLVM_LIKELY(UseGlobalHashes)) {
       GlobalTypeTableBuilder &Dest =
           isIdRecord(Type.kind()) ? *DestGlobalIdStream : *DestGlobalTypeStream;
       GloballyHashedType H = GlobalHashes[CurIndex.toArrayIndex()];
-      DestIdx = Dest.insertRecordAs(H, Type.RecordData.size(), DoSerialize);
+      DestIdx = Dest.insertRecordAs(H, AlignedSize, DoSerialize);
     } else {
       MergingTypeTableBuilder &Dest =
           isIdRecord(Type.kind()) ? *DestIdStream : *DestTypeStream;
 
-      RemapStorage.resize(Type.RecordData.size());
+      RemapStorage.resize(AlignedSize);
       ArrayRef<uint8_t> Result = DoSerialize(RemapStorage);
       if (!Result.empty())
         DestIdx = Dest.insertRecordBytes(Result);
@@ -386,9 +388,15 @@ Error TypeStreamMerger::remapType(const CVType &Type) {
 ArrayRef<uint8_t>
 TypeStreamMerger::remapIndices(const CVType &OriginalType,
                                MutableArrayRef<uint8_t> Storage) {
+  unsigned Align = OriginalType.RecordData.size() & 3;
+  unsigned AlignedSize = alignTo(OriginalType.RecordData.size(), 4);
+  assert(Storage.size() == AlignedSize &&
+         "The storage buffer size is not a multiple of 4 bytes which will "
+         "cause misalignment in the output TPI stream!");
+
   SmallVector<TiReference, 4> Refs;
   discoverTypeIndices(OriginalType.RecordData, Refs);
-  if (Refs.empty())
+  if (Refs.empty() && Align == 0)
     return OriginalType.RecordData;
 
   ::memcpy(Storage.data(), OriginalType.RecordData.data(),
@@ -408,6 +416,16 @@ TypeStreamMerger::remapIndices(const CVType &OriginalType,
         return {};
     }
   }
+
+  if (Align > 0) {
+    RecordPrefix *StorageHeader =
+        reinterpret_cast<RecordPrefix *>(Storage.data());
+    StorageHeader->RecordLen += 4 - Align;
+
+    DestContent = Storage.data() + OriginalType.RecordData.size();
+    for (; Align < 4; ++Align)
+      *DestContent++ = LF_PAD4 - Align;
+  }
   return Storage;
 }
 

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

@@ -44,6 +44,9 @@ void TpiStreamBuilder::setVersionHeader(PdbRaw_TpiVer Version) {
 void TpiStreamBuilder::addTypeRecord(ArrayRef<uint8_t> Record,
                                      Optional<uint32_t> Hash) {
   // If we just crossed an 8KB threshold, add a type index offset.
+  assert(((Record.size() & 3) == 0) &&
+         "The type record's size is not a multiple of 4 bytes which will "
+         "cause misalignment in the output TPI stream!");
   size_t NewSize = TypeRecordBytes + Record.size();
   constexpr size_t EightKB = 8 * 1024;
   if (NewSize / EightKB > TypeRecordBytes / EightKB || TypeRecords.empty()) {
@@ -153,8 +156,11 @@ Error TpiStreamBuilder::commit(const msf::MSFLayout &Layout,
     return EC;
 
   for (auto Rec : TypeRecords) {
-    assert(!Rec.empty()); // An empty record will not write anything, but it
-                          // would shift all offsets from here on.
+    assert(!Rec.empty() && "Attempting to write an empty type record shifts "
+                           "all offsets in the TPI stream!");
+    assert(((Rec.size() & 3) == 0) &&
+           "The type record's size is not a multiple of 4 bytes which will "
+           "cause misalignment in the output TPI stream!");
     if (auto EC = Writer.writeBytes(Rec))
       return EC;
   }