From e3f3a43807d2edcadd60ad333713206ed8fdd74e Mon Sep 17 00:00:00 2001 From: Alexandre Ganea Date: Fri, 13 Mar 2020 12:22:09 -0400 Subject: [PATCH] [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 a7325298e1f311b383b8ce5ba8e2d3698fef472a) --- lld/test/COFF/pdb-tpi-aligned-records.test | 46 +++++++++++++++++++ .../CodeView/GlobalTypeTableBuilder.h | 5 ++ .../CodeView/MergingTypeTableBuilder.cpp | 4 +- .../DebugInfo/CodeView/TypeStreamMerger.cpp | 24 ++++++++-- .../DebugInfo/PDB/Native/TpiStreamBuilder.cpp | 10 +++- 5 files changed, 83 insertions(+), 6 deletions(-) create mode 100644 lld/test/COFF/pdb-tpi-aligned-records.test diff --git a/lld/test/COFF/pdb-tpi-aligned-records.test b/lld/test/COFF/pdb-tpi-aligned-records.test new file mode 100644 index 0000000000000..cb3e412a8c202 --- /dev/null +++ b/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 +... diff --git a/llvm/include/llvm/DebugInfo/CodeView/GlobalTypeTableBuilder.h b/llvm/include/llvm/DebugInfo/CodeView/GlobalTypeTableBuilder.h index 3b103c227708a..bb8cc032e28da 100644 --- a/llvm/include/llvm/DebugInfo/CodeView/GlobalTypeTableBuilder.h +++ b/llvm/include/llvm/DebugInfo/CodeView/GlobalTypeTableBuilder.h @@ -71,6 +71,11 @@ class GlobalTypeTableBuilder : public TypeCollection { template 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*/ || diff --git a/llvm/lib/DebugInfo/CodeView/MergingTypeTableBuilder.cpp b/llvm/lib/DebugInfo/CodeView/MergingTypeTableBuilder.cpp index 4d7cd468f3ee6..6924b0e0ca02f 100644 --- a/llvm/lib/DebugInfo/CodeView/MergingTypeTableBuilder.cpp +++ b/llvm/lib/DebugInfo/CodeView/MergingTypeTableBuilder.cpp @@ -90,7 +90,9 @@ static inline ArrayRef stabilize(BumpPtrAllocator &Alloc, TypeIndex MergingTypeTableBuilder::insertRecordAs(hash_code Hash, ArrayRef &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()); diff --git a/llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp b/llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp index f9fca74a2199a..c233db5c1d06a 100644 --- a/llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp +++ b/llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp @@ -360,16 +360,18 @@ Error TypeStreamMerger::remapType(const CVType &Type) { [this, Type](MutableArrayRef Storage) -> ArrayRef { 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 Result = DoSerialize(RemapStorage); if (!Result.empty()) DestIdx = Dest.insertRecordBytes(Result); @@ -386,9 +388,15 @@ Error TypeStreamMerger::remapType(const CVType &Type) { ArrayRef TypeStreamMerger::remapIndices(const CVType &OriginalType, MutableArrayRef 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 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(Storage.data()); + StorageHeader->RecordLen += 4 - Align; + + DestContent = Storage.data() + OriginalType.RecordData.size(); + for (; Align < 4; ++Align) + *DestContent++ = LF_PAD4 - Align; + } return Storage; } diff --git a/llvm/lib/DebugInfo/PDB/Native/TpiStreamBuilder.cpp b/llvm/lib/DebugInfo/PDB/Native/TpiStreamBuilder.cpp index 4f10f8524a9b1..51a1f0a544e3c 100644 --- a/llvm/lib/DebugInfo/PDB/Native/TpiStreamBuilder.cpp +++ b/llvm/lib/DebugInfo/PDB/Native/TpiStreamBuilder.cpp @@ -44,6 +44,9 @@ void TpiStreamBuilder::setVersionHeader(PdbRaw_TpiVer Version) { void TpiStreamBuilder::addTypeRecord(ArrayRef Record, Optional 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; }