Skip to content

Commit 242a9cf

Browse files
aganeaaganea
committed
[LLD][COFF] Survive empty and invalid PCH signature
Solve two issues that showed up when using LLD with Unreal Engine & FASTBuild: 1. It seems the S_OBJNAME record doesn't always record the "precomp signature". We were relying on that to match the PCH.OBJ with their dependent-OBJ. 2. MSVC link.exe is able to link a PCH.OBJ when the "precomp signatureÈ doesn't match, but LLD was failing. This was occuring since the Unreal Engine Build Tool was compiling the PCH.OBJ, but the dependent-OBJ were compiled & cached through FASTBuild. Upon a clean rebuild, the PCH.OBJs were recompiled by the Unreal Build Tool, thus the "precomp signatures" were changing; however the OBJs were already cached by FASTBuild, thus having an old "precomp signatures". We now ignore "precomp signatures" and properly fallback to cmd-line name lookup, like MSVC link.exe does, and only fail if the PCH.OBJ type stream doesn't match the count expected by the dependent-OBJ. Differential Revision: https://reviews.llvm.org/D136762
1 parent e2bff1e commit 242a9cf

File tree

8 files changed

+128
-72
lines changed

8 files changed

+128
-72
lines changed

‎lld/COFF/DebugTypes.cpp

Lines changed: 62 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -125,18 +125,23 @@ class UseTypeServerSource : public TpiSource {
125125
class PrecompSource : public TpiSource {
126126
public:
127127
PrecompSource(COFFLinkerContext &ctx, ObjFile *f) : TpiSource(ctx, PCH, f) {
128-
if (!f->pchSignature || !*f->pchSignature)
129-
fatal(toString(f) +
130-
" claims to be a PCH object, but does not have a valid signature");
131-
auto it = ctx.precompSourceMappings.emplace(*f->pchSignature, this);
132-
if (!it.second)
133-
fatal("a PCH object with the same signature has already been provided (" +
134-
toString(it.first->second->file) + " and " + toString(file) + ")");
128+
// If the S_OBJNAME record contains the PCH signature, we'll register this
129+
// source file right away.
130+
registerMapping();
135131
}
136132

133+
Error mergeDebugT(TypeMerger *m) override;
134+
137135
void loadGHashes() override;
138136

139137
bool isDependency() const override { return true; }
138+
139+
private:
140+
void registerMapping();
141+
142+
// Whether this precomp OBJ was recorded in the precompSourceMappings map.
143+
// Only happens if the file->pchSignature is valid.
144+
bool registered = false;
140145
};
141146

142147
// This class represents the debug type stream of an OBJ file that depends on a
@@ -310,10 +315,15 @@ Error TpiSource::mergeDebugT(TypeMerger *m) {
310315
// When dealing with PCH.OBJ, some indices were already merged.
311316
unsigned nbHeadIndices = indexMapStorage.size();
312317

313-
if (auto err = mergeTypeAndIdRecords(
314-
m->idTable, m->typeTable, indexMapStorage, types, file->pchSignature))
318+
Optional<PCHMergerInfo> pchInfo;
319+
if (auto err = mergeTypeAndIdRecords(m->idTable, m->typeTable,
320+
indexMapStorage, types, pchInfo))
315321
fatal("codeview::mergeTypeAndIdRecords failed: " +
316322
toString(std::move(err)));
323+
if (pchInfo) {
324+
file->pchSignature = pchInfo->PCHSignature;
325+
endPrecompIdx = pchInfo->EndPrecompIndex;
326+
}
317327

318328
// In an object, there is only one mapping for both types and items.
319329
tpiMap = indexMapStorage;
@@ -494,16 +504,15 @@ Expected<PrecompSource *> UsePrecompSource::findPrecompMap(ObjFile *file,
494504
pr.getPrecompFilePath(),
495505
make_error<pdb::PDBError>(pdb::pdb_error_code::no_matching_pch));
496506

497-
if (pr.getSignature() != file->pchSignature)
507+
// Don't rely on the PCH signature to validate the concordance between the PCH
508+
// and the OBJ that uses it. However we do validate here that the
509+
// LF_ENDPRECOMP record index lines up with the number of type records
510+
// LF_PRECOMP is expecting.
511+
if (precomp->endPrecompIdx != pr.getTypesCount())
498512
return createFileError(
499513
toString(file),
500514
make_error<pdb::PDBError>(pdb::pdb_error_code::no_matching_pch));
501515

502-
if (pr.getSignature() != *precomp->file->pchSignature)
503-
return createFileError(
504-
toString(precomp->file),
505-
make_error<pdb::PDBError>(pdb::pdb_error_code::no_matching_pch));
506-
507516
return precomp;
508517
}
509518

@@ -541,6 +550,30 @@ Error UsePrecompSource::mergeDebugT(TypeMerger *m) {
541550
return TpiSource::mergeDebugT(m);
542551
}
543552

553+
Error PrecompSource::mergeDebugT(TypeMerger *m) {
554+
// In some cases, the S_OBJNAME record doesn't contain the PCH signature.
555+
// The signature comes later with the LF_ENDPRECOMP record, so we first need
556+
// to merge in all the .PCH.OBJ file type records, before registering below.
557+
if (Error e = TpiSource::mergeDebugT(m))
558+
return e;
559+
560+
registerMapping();
561+
562+
return Error::success();
563+
}
564+
565+
void PrecompSource::registerMapping() {
566+
if (registered)
567+
return;
568+
if (file->pchSignature && *file->pchSignature) {
569+
auto it = ctx.precompSourceMappings.emplace(*file->pchSignature, this);
570+
if (!it.second)
571+
fatal("a PCH object with the same signature has already been provided (" +
572+
toString(it.first->second->file) + " and " + toString(file) + ")");
573+
registered = true;
574+
}
575+
}
576+
544577
//===----------------------------------------------------------------------===//
545578
// Parellel GHash type merging implementation.
546579
//===----------------------------------------------------------------------===//
@@ -808,8 +841,14 @@ void PrecompSource::loadGHashes() {
808841
// Remember the index of the LF_ENDPRECOMP record so it can be excluded from
809842
// the PDB. There must be an entry in the list of ghashes so that the type
810843
// indexes of the following records in the /Yc PCH object line up.
811-
if (ty.kind() == LF_ENDPRECOMP)
812-
endPrecompGHashIdx = ghashIdx;
844+
if (ty.kind() == LF_ENDPRECOMP) {
845+
EndPrecompRecord endPrecomp;
846+
cantFail(TypeDeserializer::deserializeAs<EndPrecompRecord>(
847+
const_cast<CVType &>(ty), endPrecomp));
848+
file->pchSignature = endPrecomp.getSignature();
849+
registerMapping();
850+
endPrecompIdx = ghashIdx;
851+
}
813852

814853
hashVec.push_back(GloballyHashedType::hashType(ty, hashVec, hashVec));
815854
isItemIndex.push_back(isIdRecord(ty.kind()));
@@ -819,9 +858,13 @@ void PrecompSource::loadGHashes() {
819858
}
820859

821860
void UsePrecompSource::loadGHashes() {
822-
PrecompSource *pchSrc = findPrecompSource(file, precompDependency);
823-
if (!pchSrc)
861+
auto e = findPrecompMap(file, precompDependency);
862+
if (!e) {
863+
warn(toString(e.takeError()));
824864
return;
865+
}
866+
867+
PrecompSource *pchSrc = *e;
825868

826869
// To compute ghashes of a /Yu object file, we need to build on the ghashes of
827870
// the /Yc PCH object. After we are done hashing, discard the ghashes from the

‎lld/COFF/DebugTypes.h

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -106,18 +106,18 @@ class TpiSource {
106106
/// it is unique. This prevents a record from being added to the input ghash
107107
/// table.
108108
bool shouldOmitFromPdb(uint32_t ghashIdx) {
109-
return ghashIdx == endPrecompGHashIdx;
109+
return ghashIdx == endPrecompIdx;
110110
}
111111

112112
const TpiKind kind;
113113
bool ownedGHashes = true;
114114
uint32_t tpiSrcIdx = 0;
115115

116-
protected:
117-
/// The ghash index (zero based, not 0x1000-based) of the LF_ENDPRECOMP record
118-
/// in this object, if one exists. This is the all ones value otherwise. It is
119-
/// recorded here so that it can be omitted from the final ghash table.
120-
uint32_t endPrecompGHashIdx = ~0U;
116+
/// The index (zero based, not 0x1000-based) of the LF_ENDPRECOMP record in
117+
/// this object, if one exists. This is the all ones value otherwise. It is
118+
/// recorded here for validation, and so that it can be omitted from the final
119+
/// ghash table.
120+
uint32_t endPrecompIdx = ~0U;
121121

122122
public:
123123
ObjFile *file;

‎lld/COFF/InputFiles.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -734,7 +734,8 @@ void ObjFile::initializeFlags() {
734734
if (sym->kind() == SymbolKind::S_OBJNAME) {
735735
auto objName = cantFail(SymbolDeserializer::deserializeAs<ObjNameSym>(
736736
sym.get()));
737-
pchSignature = objName.Signature;
737+
if (objName.Signature)
738+
pchSignature = objName.Signature;
738739
}
739740
offset += sym->length();
740741
}
@@ -800,6 +801,10 @@ void ObjFile::initializeDependencies() {
800801
if (firstType->kind() == LF_PRECOMP) {
801802
PrecompRecord precomp = cantFail(
802803
TypeDeserializer::deserializeAs<PrecompRecord>(firstType->data()));
804+
// We're better off trusting the LF_PRECOMP signature. In some cases the
805+
// S_OBJNAME record doesn't contain a valid PCH signature.
806+
if (precomp.Signature)
807+
pchSignature = precomp.Signature;
803808
debugTypesObj = makeUsePrecompSource(ctx, this, precomp);
804809
// Drop the LF_PRECOMP record from the input stream.
805810
debugTypes = debugTypes.drop_front(firstType->RecordData.size());

‎lld/docs/ReleaseNotes.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,9 @@ COFF Improvements
4545
(`D137723 <https://reviews.llvm.org/D137723>`_)
4646
* Switched from SHA1 to BLAKE3 for PDB type hashing / ``-gcodeview-ghash``
4747
(`D137101 <https://reviews.llvm.org/D137101>`_)
48+
* Improvements to the PCH.OBJ files handling. Now LLD behaves the same as MSVC
49+
link.exe when merging PCH.OBJ files that don't have the same signature.
50+
(`D136762 <https://reviews.llvm.org/D136762>`_)
4851

4952
MinGW Improvements
5053
------------------

‎lld/test/COFF/precomp-link.test

Lines changed: 18 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
1-
RUN: lld-link %S/Inputs/precomp-a.obj %S/Inputs/precomp-b.obj %S/Inputs/precomp.obj /nodefaultlib /entry:main /debug /pdb:%t.pdb /out:%t.exe /opt:ref /opt:icf /summary | FileCheck %s -check-prefix SUMMARY
1+
RUN: lld-link %S/Inputs/precomp-a.obj %S/Inputs/precomp-b.obj %S/Inputs/precomp.obj /nodefaultlib /entry:main /debug:noghash /pdb:%t.pdb /out:%t.exe /opt:ref /opt:icf /summary | FileCheck %s -check-prefix SUMMARY
2+
RUN: llvm-pdbutil dump -types %t.pdb | FileCheck %s
3+
RUN: lld-link %S/Inputs/precomp-a.obj %S/Inputs/precomp-b.obj %S/Inputs/precomp.obj /nodefaultlib /entry:main /debug:ghash /pdb:%t.pdb /out:%t.exe /opt:ref /opt:icf /summary | FileCheck %s -check-prefix SUMMARY
24
RUN: llvm-pdbutil dump -types %t.pdb | FileCheck %s
35

46
RUN: lld-link %S/Inputs/precomp.obj %S/Inputs/precomp-a.obj %S/Inputs/precomp-b.obj /nodefaultlib /entry:main /debug /pdb:%t.pdb /out:%t.exe /opt:ref /opt:icf
57
RUN: llvm-pdbutil dump -types %t.pdb | FileCheck %s
68

7-
RUN: lld-link %S/Inputs/precomp-a.obj %S/Inputs/precomp-invalid.obj %S/Inputs/precomp.obj /nodefaultlib /entry:main /debug /pdb:%t.pdb /out:%t.exe /opt:ref /opt:icf 2>&1 | FileCheck %s -check-prefix FAILURE
8-
RUN: lld-link %S/Inputs/precomp-a.obj %S/Inputs/precomp-invalid.obj %S/Inputs/precomp.obj /nodefaultlib /entry:main /debug:ghash /pdb:%t.pdb /out:%t.exe /opt:ref /opt:icf 2>&1 | FileCheck %s -check-prefix FAILURE
9+
RUN: lld-link %S/Inputs/precomp-a.obj %S/Inputs/precomp-invalid.obj %S/Inputs/precomp.obj /nodefaultlib /entry:main /debug:noghash /pdb:%t.pdb /out:%t.exe /opt:ref /opt:icf 2>&1
10+
RUN: lld-link %S/Inputs/precomp-a.obj %S/Inputs/precomp-invalid.obj %S/Inputs/precomp.obj /nodefaultlib /entry:main /debug:ghash /pdb:%t.pdb /out:%t.exe /opt:ref /opt:icf 2>&1
911

1012
FIXME: The following RUN line should fail, regardless of whether debug info is
1113
enabled or not. Normally this would result in an error due to missing _PchSym_
@@ -14,51 +16,46 @@ special case for those symbols and it emits the LNK2011 error.
1416

1517
RUN: lld-link %S/Inputs/precomp-a.obj %S/Inputs/precomp-b.obj /nodefaultlib /entry:main /debug /pdb:%t.pdb /out:%t.exe /opt:ref /opt:icf 2>&1 | FileCheck %s -check-prefix FAILURE-MISSING-PRECOMPOBJ
1618

17-
FAILURE: warning: Cannot use debug info for '{{.*}}precomp-invalid.obj' [LNK4099]
18-
FAILURE-NEXT: failed to load reference '{{.*}}precomp.obj': No matching precompiled header could be located.
19-
2019
FAILURE-MISSING-PRECOMPOBJ: warning: Cannot use debug info for '{{.*}}precomp-a.obj' [LNK4099]
2120
FAILURE-MISSING-PRECOMPOBJ-NEXT: failed to load reference '{{.*}}precomp.obj': No matching precompiled header could be located.
2221

23-
Check that a PCH object file with a missing S_OBJNAME record results in an
24-
error. Edit out this record from the yaml-ified object:
22+
Check that a PCH object file with an empty S_OBJNAME record works fine.
23+
Edit out this record from the yaml-ified object:
2524
- Kind: S_OBJNAME
2625
ObjNameSym:
2726
Signature: 545589255
2827
ObjectName: 'F:\svn\lld\test\COFF\precomp\precomp.obj'
2928

3029
RUN: obj2yaml %S/Inputs/precomp.obj | grep -v 'SectionData: *04000000' > %t.precomp.yaml
3130
RUN: sed '/S_OBJNAME/,/ObjectName:/d' < %t.precomp.yaml > precomp-no-objname.yaml
32-
RUN: sed 's/Signature: *545589255/Signature: 0/' < %t.precomp.yaml > precomp-zero-sig.yaml
31+
RUN: sed '16,19s/Signature: *545589255/Signature: 0/' < %t.precomp.yaml > precomp-zero-sig.yaml
3332
RUN: yaml2obj precomp-no-objname.yaml -o %t.precomp-no-objname.obj
3433
RUN: yaml2obj precomp-zero-sig.yaml -o %t.precomp-zero-sig.obj
34+
RUN: env LLD_IN_TEST=1 lld-link %t.precomp-no-objname.obj %S/Inputs/precomp-a.obj %S/Inputs/precomp-b.obj /nodefaultlib /entry:main /debug:noghash /pdb:%t.pdb /out:%t.exe 2>&1 | FileCheck %s -check-prefix WARNING --allow-empty
35+
RUN: env LLD_IN_TEST=1 lld-link %t.precomp-no-objname.obj %S/Inputs/precomp-a.obj %S/Inputs/precomp-b.obj /nodefaultlib /entry:main /debug:ghash /pdb:%t.pdb /out:%t.exe 2>&1 | FileCheck %s -check-prefix WARNING --allow-empty
36+
RUN: env LLD_IN_TEST=1 lld-link %t.precomp-zero-sig.obj %S/Inputs/precomp-a.obj %S/Inputs/precomp-b.obj /nodefaultlib /entry:main /debug:noghash /pdb:%t.pdb /out:%t.exe 2>&1 | FileCheck %s -check-prefix WARNING --allow-empty
37+
RUN: env LLD_IN_TEST=1 lld-link %t.precomp-zero-sig.obj %S/Inputs/precomp-a.obj %S/Inputs/precomp-b.obj /nodefaultlib /entry:main /debug:ghash /pdb:%t.pdb /out:%t.exe 2>&1 | FileCheck %s -check-prefix WARNING --allow-empty
3538

36-
RUN: env LLD_IN_TEST=1 not lld-link %t.precomp-no-objname.obj %S/Inputs/precomp-a.obj %S/Inputs/precomp-b.obj /nodefaultlib /entry:main /debug /pdb:%t.pdb /out:%t.exe 2>&1 | FileCheck %s -check-prefix FAILURE-NO-SIGNATURE
39+
WARNING-NOT: warning
3740

38-
RUN: env LLD_IN_TEST=1 not lld-link %t.precomp-zero-sig.obj %S/Inputs/precomp-a.obj %S/Inputs/precomp-b.obj /nodefaultlib /entry:main /debug /pdb:%t.pdb /out:%t.exe 2>&1 | FileCheck %s -check-prefix FAILURE-NO-SIGNATURE
41+
Check that a PCH with wrong signature, but with right LF_PRECOMP records count, works.
3942

40-
FAILURE-NO-SIGNATURE: error: {{.*}}.obj claims to be a PCH object, but does not have a valid signature
43+
RUN: sed '16,19s/Signature: *545589255/Signature: 123456789/' < %t.precomp.yaml > precomp-wrong-sig.yaml
44+
RUN: yaml2obj precomp-wrong-sig.yaml -o %T/precomp.obj
45+
RUN: env LLD_IN_TEST=1 lld-link %T/precomp.obj %S/Inputs/precomp-a.obj %S/Inputs/precomp-b.obj /nodefaultlib /entry:main /debug:noghash /pdb:%t.pdb /out:%t.exe 2>&1 | FileCheck %s -check-prefix WARNING --allow-empty
46+
RUN: env LLD_IN_TEST=1 lld-link %T/precomp.obj %S/Inputs/precomp-a.obj %S/Inputs/precomp-b.obj /nodefaultlib /entry:main /debug:ghash /pdb:%t.pdb /out:%t.exe 2>&1 | FileCheck %s -check-prefix WARNING --allow-empty
4147

4248
Check that two PCH objs with duplicate signatures are an error.
4349

4450
RUN: cp %S/Inputs/precomp.obj %t.precomp-dup.obj
45-
4651
RUN: env LLD_IN_TEST=1 not lld-link %S/Inputs/precomp.obj %t.precomp-dup.obj %S/Inputs/precomp-a.obj %S/Inputs/precomp-b.obj /nodefaultlib /entry:main /debug /pdb:%t.pdb /out:%t.exe 2>&1 | FileCheck %s -check-prefix FAILURE-DUP-SIGNATURE
47-
4852
FAILURE-DUP-SIGNATURE: error: a PCH object with the same signature has already been provided ({{.*precomp.obj and .*precomp-dup.obj.*}})
4953

5054

5155
CHECK: Types (TPI Stream)
5256
CHECK-NOT: LF_PRECOMP
5357
CHECK-NOT: LF_ENDPRECOMP
5458

55-
56-
Re-run with ghash. Eventually, perhaps this will be the default.
57-
58-
RUN: lld-link %S/Inputs/precomp-a.obj %S/Inputs/precomp-b.obj %S/Inputs/precomp.obj /nodefaultlib /entry:main /debug /debug:ghash /pdb:%t.pdb /out:%t.exe /opt:ref /opt:icf /summary | FileCheck %s -check-prefix SUMMARY
59-
RUN: llvm-pdbutil dump -types %t.pdb | FileCheck %s
60-
61-
6259
SUMMARY: Summary
6360
SUMMARY-NEXT: --------------------------------------------------------------------------------
6461
SUMMARY-NEXT: 3 Input OBJ files (expanded from all cmd-line inputs)

‎llvm/include/llvm/DebugInfo/CodeView/TypeStreamMerger.h

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,13 @@ struct GloballyHashedType;
2323
class GlobalTypeTableBuilder;
2424
class MergingTypeTableBuilder;
2525

26+
/// Used to forward information about PCH.OBJ (precompiled) files, when
27+
/// applicable.
28+
struct PCHMergerInfo {
29+
uint32_t PCHSignature{};
30+
uint32_t EndPrecompIndex = ~0U;
31+
};
32+
2633
/// Merge one set of type records into another. This method assumes
2734
/// that all records are type records, and there are no Id records present.
2835
///
@@ -84,20 +91,20 @@ Error mergeTypeAndIdRecords(MergingTypeTableBuilder &DestIds,
8491
MergingTypeTableBuilder &DestTypes,
8592
SmallVectorImpl<TypeIndex> &SourceToDest,
8693
const CVTypeArray &IdsAndTypes,
87-
Optional<uint32_t> &PCHSignature);
94+
Optional<PCHMergerInfo> &PCHInfo);
8895

8996
Error mergeTypeAndIdRecords(GlobalTypeTableBuilder &DestIds,
9097
GlobalTypeTableBuilder &DestTypes,
9198
SmallVectorImpl<TypeIndex> &SourceToDest,
9299
const CVTypeArray &IdsAndTypes,
93100
ArrayRef<GloballyHashedType> Hashes,
94-
Optional<uint32_t> &PCHSignature);
101+
Optional<PCHMergerInfo> &PCHInfo);
95102

96103
Error mergeTypeRecords(GlobalTypeTableBuilder &Dest,
97104
SmallVectorImpl<TypeIndex> &SourceToDest,
98105
const CVTypeArray &Types,
99106
ArrayRef<GloballyHashedType> Hashes,
100-
Optional<uint32_t> &PCHSignature);
107+
Optional<PCHMergerInfo> &PCHInfo);
101108

102109
Error mergeIdRecords(GlobalTypeTableBuilder &Dest, ArrayRef<TypeIndex> Types,
103110
SmallVectorImpl<TypeIndex> &SourceToDest,

0 commit comments

Comments
 (0)