Skip to content

Commit 84038cf

Browse files
sylvain-audisylvain-audi
committed
[lld][COFF] Fix lld-link crash when several .obj files built with /Zi refer to a .pdb file that failed to load
This patch relaxes the constraints on the error message saved in PDBInputFile when failing to load a pdb file. Storing an `Error` member infers that it must be accessed exactly once, which doesn't fit in several scenarios: - If an invalid PDB file is provided as input file but never used, a loading error is created but never handled, causing an assert at shutdown. - PDB file created using MSVC's `/Zi` option : The loading error message must be displayed once per obj file. Also, the state of `PDBInputFile` was altered when reading (taking) the `Error` member, causing issues: - accessing it (taking the `Error`) makes the object look valid whereas it's not properly initialized - read vs write concurrency on a same `PDBInputFile` in the ghash parallel algorithm The solution adopted here was to instead store an optional error string, and generate Error objects from it on demand. Differential Revision: https://reviews.llvm.org/D140333
1 parent b5415f3 commit 84038cf

File tree

7 files changed

+468
-8
lines changed

7 files changed

+468
-8
lines changed

‎lld/COFF/DebugTypes.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ class TypeServerSource : public TpiSource {
4848
public:
4949
explicit TypeServerSource(COFFLinkerContext &ctx, PDBInputFile *f)
5050
: TpiSource(ctx, PDB, nullptr), pdbInputFile(f) {
51-
if (f->loadErr && *f->loadErr)
51+
if (f->loadErrorStr)
5252
return;
5353
pdb::PDBFile &file = f->session->getPDBFile();
5454
auto expectedInfo = file.getPDBInfoStream();
@@ -424,8 +424,10 @@ Expected<TypeServerSource *> UseTypeServerSource::getTypeServerSource() {
424424
return createFileError(tsPath, errorCodeToError(std::error_code(
425425
ENOENT, std::generic_category())));
426426
// If an error occurred during loading, throw it now
427-
if (pdb->loadErr && *pdb->loadErr)
428-
return createFileError(tsPath, std::move(*pdb->loadErr));
427+
if (pdb->loadErrorStr)
428+
return createFileError(
429+
tsPath, make_error<StringError>(*pdb->loadErrorStr,
430+
llvm::inconvertibleErrorCode()));
429431

430432
tsSrc = (TypeServerSource *)pdb->debugTypesObj;
431433

‎lld/COFF/InputFiles.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -876,19 +876,21 @@ void PDBInputFile::parse() {
876876
ctx.pdbInputFileInstances[mb.getBufferIdentifier().str()] = this;
877877

878878
std::unique_ptr<pdb::IPDBSession> thisSession;
879-
loadErr.emplace(pdb::NativeSession::createFromPdb(
880-
MemoryBuffer::getMemBuffer(mb, false), thisSession));
881-
if (*loadErr)
879+
Error E = pdb::NativeSession::createFromPdb(
880+
MemoryBuffer::getMemBuffer(mb, false), thisSession);
881+
if (E) {
882+
loadErrorStr.emplace(toString(std::move(E)));
882883
return; // fail silently at this point - the error will be handled later,
883884
// when merging the debug type stream
885+
}
884886

885887
session.reset(static_cast<pdb::NativeSession *>(thisSession.release()));
886888

887889
pdb::PDBFile &pdbFile = session->getPDBFile();
888890
auto expectedInfo = pdbFile.getPDBInfoStream();
889891
// All PDB Files should have an Info stream.
890892
if (!expectedInfo) {
891-
loadErr.emplace(expectedInfo.takeError());
893+
loadErrorStr.emplace(toString(expectedInfo.takeError()));
892894
return;
893895
}
894896
debugTypesObj = makeTypeServerSource(ctx, this);

‎lld/COFF/InputFiles.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ class PDBInputFile : public InputFile {
319319
StringRef path, ObjFile *fromFile);
320320

321321
// Record possible errors while opening the PDB file
322-
std::optional<Error> loadErr;
322+
std::optional<std::string> loadErrorStr;
323323

324324
// This is the actual interface to the PDB (if it was opened successfully)
325325
std::unique_ptr<llvm::pdb::NativeSession> session;

‎lld/test/COFF/Inputs/pdb-type-server-corrupted-a.yaml

Lines changed: 255 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,255 @@
1+
--- !COFF
2+
header:
3+
Machine: IMAGE_FILE_MACHINE_AMD64
4+
Characteristics: [ ]
5+
sections:
6+
- Name: .drectve
7+
Characteristics: [ IMAGE_SCN_LNK_INFO, IMAGE_SCN_LNK_REMOVE ]
8+
Alignment: 1
9+
SectionData: 2020202F44454641554C544C49423A224C4942434D5422202F44454641554C544C49423A224F4C444E414D45532220
10+
- Name: '.debug$S'
11+
Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
12+
Alignment: 1
13+
Subsections:
14+
- !Symbols
15+
Records:
16+
- Kind: S_OBJNAME
17+
ObjNameSym:
18+
Signature: 0
19+
ObjectName: 'C:\src\llvm-project\build\a.obj'
20+
- Kind: S_COMPILE3
21+
Compile3Sym:
22+
Flags: [ SecurityChecks, HotPatch ]
23+
Machine: X64
24+
FrontendMajor: 19
25+
FrontendMinor: 0
26+
FrontendBuild: 24215
27+
FrontendQFE: 1
28+
BackendMajor: 19
29+
BackendMinor: 0
30+
BackendBuild: 24215
31+
BackendQFE: 1
32+
Version: 'Microsoft (R) Optimizing Compiler'
33+
- !Symbols
34+
Records:
35+
- Kind: S_GPROC32_ID
36+
ProcSym:
37+
CodeSize: 27
38+
DbgStart: 4
39+
DbgEnd: 22
40+
FunctionType: 4098
41+
Flags: [ ]
42+
DisplayName: main
43+
- Kind: S_FRAMEPROC
44+
FrameProcSym:
45+
TotalFrameBytes: 56
46+
PaddingFrameBytes: 0
47+
OffsetToPadding: 0
48+
BytesOfCalleeSavedRegisters: 0
49+
OffsetOfExceptionHandler: 0
50+
SectionIdOfExceptionHandler: 0
51+
Flags: [ AsynchronousExceptionHandling, OptimizedForSpeed ]
52+
- Kind: S_REGREL32
53+
RegRelativeSym:
54+
Offset: 32
55+
Type: 4102
56+
Register: RSP
57+
VarName: f
58+
- Kind: S_PROC_ID_END
59+
ScopeEndSym:
60+
- !Lines
61+
CodeSize: 27
62+
Flags: [ ]
63+
RelocOffset: 0
64+
RelocSegment: 0
65+
Blocks:
66+
- FileName: 'c:\src\llvm-project\build\a.c'
67+
Lines:
68+
- Offset: 0
69+
LineStart: 3
70+
IsStatement: true
71+
EndDelta: 0
72+
- Offset: 4
73+
LineStart: 4
74+
IsStatement: true
75+
EndDelta: 0
76+
- Offset: 12
77+
LineStart: 5
78+
IsStatement: true
79+
EndDelta: 0
80+
- Offset: 22
81+
LineStart: 6
82+
IsStatement: true
83+
EndDelta: 0
84+
Columns:
85+
- !Symbols
86+
Records:
87+
- Kind: S_UDT
88+
UDTSym:
89+
Type: 4102
90+
UDTName: Foo
91+
- !FileChecksums
92+
Checksums:
93+
- FileName: 'c:\src\llvm-project\build\a.c'
94+
Kind: MD5
95+
Checksum: BF69E7E933074E1B7ED1FE8FB395965B
96+
- !StringTable
97+
Strings:
98+
- 'c:\src\llvm-project\build\a.c'
99+
- !Symbols
100+
Records:
101+
- Kind: S_BUILDINFO
102+
BuildInfoSym:
103+
BuildId: 4107
104+
Relocations:
105+
- VirtualAddress: 152
106+
SymbolName: main
107+
Type: IMAGE_REL_AMD64_SECREL
108+
- VirtualAddress: 156
109+
SymbolName: main
110+
Type: IMAGE_REL_AMD64_SECTION
111+
- VirtualAddress: 224
112+
SymbolName: main
113+
Type: IMAGE_REL_AMD64_SECREL
114+
- VirtualAddress: 228
115+
SymbolName: main
116+
Type: IMAGE_REL_AMD64_SECTION
117+
- Name: '.debug$T'
118+
Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
119+
Alignment: 1
120+
Types:
121+
- Kind: LF_TYPESERVER2
122+
TypeServer2:
123+
Guid: '{41414141-4141-4141-4141-414141414141}'
124+
Age: 1
125+
Name: 'C:\src\llvm-project\build\bad-block-size.pdb'
126+
- Name: '.text$mn'
127+
Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
128+
Alignment: 16
129+
SectionData: 4883EC38C74424202A000000488D4C2420E8000000004883C438C3
130+
Relocations:
131+
- VirtualAddress: 18
132+
SymbolName: g
133+
Type: IMAGE_REL_AMD64_REL32
134+
- Name: .xdata
135+
Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
136+
Alignment: 4
137+
SectionData: '0104010004620000'
138+
- Name: .pdata
139+
Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
140+
Alignment: 4
141+
SectionData: 000000001B00000000000000
142+
Relocations:
143+
- VirtualAddress: 0
144+
SymbolName: '$LN3'
145+
Type: IMAGE_REL_AMD64_ADDR32NB
146+
- VirtualAddress: 4
147+
SymbolName: '$LN3'
148+
Type: IMAGE_REL_AMD64_ADDR32NB
149+
- VirtualAddress: 8
150+
SymbolName: '$unwind$main'
151+
Type: IMAGE_REL_AMD64_ADDR32NB
152+
symbols:
153+
- Name: .drectve
154+
Value: 0
155+
SectionNumber: 1
156+
SimpleType: IMAGE_SYM_TYPE_NULL
157+
ComplexType: IMAGE_SYM_DTYPE_NULL
158+
StorageClass: IMAGE_SYM_CLASS_STATIC
159+
SectionDefinition:
160+
Length: 47
161+
NumberOfRelocations: 0
162+
NumberOfLinenumbers: 0
163+
CheckSum: 0
164+
Number: 0
165+
- Name: '.debug$S'
166+
Value: 0
167+
SectionNumber: 2
168+
SimpleType: IMAGE_SYM_TYPE_NULL
169+
ComplexType: IMAGE_SYM_DTYPE_NULL
170+
StorageClass: IMAGE_SYM_CLASS_STATIC
171+
SectionDefinition:
172+
Length: 388
173+
NumberOfRelocations: 4
174+
NumberOfLinenumbers: 0
175+
CheckSum: 0
176+
Number: 0
177+
- Name: '.debug$T'
178+
Value: 0
179+
SectionNumber: 3
180+
SimpleType: IMAGE_SYM_TYPE_NULL
181+
ComplexType: IMAGE_SYM_DTYPE_NULL
182+
StorageClass: IMAGE_SYM_CLASS_STATIC
183+
SectionDefinition:
184+
Length: 64
185+
NumberOfRelocations: 0
186+
NumberOfLinenumbers: 0
187+
CheckSum: 0
188+
Number: 0
189+
- Name: '.text$mn'
190+
Value: 0
191+
SectionNumber: 4
192+
SimpleType: IMAGE_SYM_TYPE_NULL
193+
ComplexType: IMAGE_SYM_DTYPE_NULL
194+
StorageClass: IMAGE_SYM_CLASS_STATIC
195+
SectionDefinition:
196+
Length: 27
197+
NumberOfRelocations: 1
198+
NumberOfLinenumbers: 0
199+
CheckSum: 1939996292
200+
Number: 0
201+
- Name: g
202+
Value: 0
203+
SectionNumber: 0
204+
SimpleType: IMAGE_SYM_TYPE_NULL
205+
ComplexType: IMAGE_SYM_DTYPE_FUNCTION
206+
StorageClass: IMAGE_SYM_CLASS_EXTERNAL
207+
- Name: main
208+
Value: 0
209+
SectionNumber: 4
210+
SimpleType: IMAGE_SYM_TYPE_NULL
211+
ComplexType: IMAGE_SYM_DTYPE_FUNCTION
212+
StorageClass: IMAGE_SYM_CLASS_EXTERNAL
213+
- Name: '$LN3'
214+
Value: 0
215+
SectionNumber: 4
216+
SimpleType: IMAGE_SYM_TYPE_NULL
217+
ComplexType: IMAGE_SYM_DTYPE_NULL
218+
StorageClass: IMAGE_SYM_CLASS_LABEL
219+
- Name: .xdata
220+
Value: 0
221+
SectionNumber: 5
222+
SimpleType: IMAGE_SYM_TYPE_NULL
223+
ComplexType: IMAGE_SYM_DTYPE_NULL
224+
StorageClass: IMAGE_SYM_CLASS_STATIC
225+
SectionDefinition:
226+
Length: 8
227+
NumberOfRelocations: 0
228+
NumberOfLinenumbers: 0
229+
CheckSum: 931692337
230+
Number: 0
231+
- Name: '$unwind$main'
232+
Value: 0
233+
SectionNumber: 5
234+
SimpleType: IMAGE_SYM_TYPE_NULL
235+
ComplexType: IMAGE_SYM_DTYPE_NULL
236+
StorageClass: IMAGE_SYM_CLASS_STATIC
237+
- Name: .pdata
238+
Value: 0
239+
SectionNumber: 6
240+
SimpleType: IMAGE_SYM_TYPE_NULL
241+
ComplexType: IMAGE_SYM_DTYPE_NULL
242+
StorageClass: IMAGE_SYM_CLASS_STATIC
243+
SectionDefinition:
244+
Length: 12
245+
NumberOfRelocations: 3
246+
NumberOfLinenumbers: 0
247+
CheckSum: 567356797
248+
Number: 0
249+
- Name: '$pdata$main'
250+
Value: 0
251+
SectionNumber: 6
252+
SimpleType: IMAGE_SYM_TYPE_NULL
253+
ComplexType: IMAGE_SYM_DTYPE_NULL
254+
StorageClass: IMAGE_SYM_CLASS_STATIC
255+
...

0 commit comments

Comments
 (0)