[llvm-branch-commits] [llvm] 6196695 - [CodeView] Align type records on 4-bytes when emitting PDBs
Tom Stellard via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Thu Apr 16 12:05:40 PDT 2020
Author: Alexandre Ganea
Date: 2020-04-16T12:05:11-07:00
New Revision: 6196695ec5819c0df7efe3fecca5c4ef9ea80b1c
URL: https://github.com/llvm/llvm-project/commit/6196695ec5819c0df7efe3fecca5c4ef9ea80b1c
DIFF: https://github.com/llvm/llvm-project/commit/6196695ec5819c0df7efe3fecca5c4ef9ea80b1c.diff
LOG: [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)
Added:
lld/test/COFF/pdb-tpi-aligned-records.test
Modified:
llvm/include/llvm/DebugInfo/CodeView/GlobalTypeTableBuilder.h
llvm/lib/DebugInfo/CodeView/MergingTypeTableBuilder.cpp
llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp
llvm/lib/DebugInfo/PDB/Native/TpiStreamBuilder.cpp
Removed:
################################################################################
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 000000000000..cb3e412a8c20
--- /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 3b103c227708..bb8cc032e28d 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 <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*/ ||
diff --git a/llvm/lib/DebugInfo/CodeView/MergingTypeTableBuilder.cpp b/llvm/lib/DebugInfo/CodeView/MergingTypeTableBuilder.cpp
index 4d7cd468f3ee..6924b0e0ca02 100644
--- a/llvm/lib/DebugInfo/CodeView/MergingTypeTableBuilder.cpp
+++ b/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());
diff --git a/llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp b/llvm/lib/DebugInfo/CodeView/TypeStreamMerger.cpp
index f9fca74a2199..c233db5c1d06 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<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;
}
diff --git a/llvm/lib/DebugInfo/PDB/Native/TpiStreamBuilder.cpp b/llvm/lib/DebugInfo/PDB/Native/TpiStreamBuilder.cpp
index 4f10f8524a9b..51a1f0a544e3 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<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;
}
More information about the llvm-branch-commits
mailing list