From 84038cf914f6a0060477ba35bafbff2fd0c49fe0 Mon Sep 17 00:00:00 2001 From: Sylvain Audi Date: Thu, 15 Dec 2022 10:56:47 -0500 Subject: [PATCH] [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 --- lld/COFF/DebugTypes.cpp | 8 +- lld/COFF/InputFiles.cpp | 10 +- lld/COFF/InputFiles.h | 2 +- .../COFF/Inputs/pdb-type-server-corrupted-a.yaml | 255 +++++++++++++++++++++ .../COFF/Inputs/pdb-type-server-corrupted-b.yaml | 173 ++++++++++++++ lld/test/COFF/invalid-input-pdb.ll | 14 ++ lld/test/COFF/pdb-type-server-corrupted.test | 14 ++ 7 files changed, 468 insertions(+), 8 deletions(-) create mode 100644 lld/test/COFF/Inputs/pdb-type-server-corrupted-a.yaml create mode 100644 lld/test/COFF/Inputs/pdb-type-server-corrupted-b.yaml create mode 100644 lld/test/COFF/invalid-input-pdb.ll create mode 100644 lld/test/COFF/pdb-type-server-corrupted.test diff --git a/lld/COFF/DebugTypes.cpp b/lld/COFF/DebugTypes.cpp index d723e48..66b9b99 100644 --- a/lld/COFF/DebugTypes.cpp +++ b/lld/COFF/DebugTypes.cpp @@ -48,7 +48,7 @@ class TypeServerSource : public TpiSource { public: explicit TypeServerSource(COFFLinkerContext &ctx, PDBInputFile *f) : TpiSource(ctx, PDB, nullptr), pdbInputFile(f) { - if (f->loadErr && *f->loadErr) + if (f->loadErrorStr) return; pdb::PDBFile &file = f->session->getPDBFile(); auto expectedInfo = file.getPDBInfoStream(); @@ -424,8 +424,10 @@ Expected UseTypeServerSource::getTypeServerSource() { return createFileError(tsPath, errorCodeToError(std::error_code( ENOENT, std::generic_category()))); // If an error occurred during loading, throw it now - if (pdb->loadErr && *pdb->loadErr) - return createFileError(tsPath, std::move(*pdb->loadErr)); + if (pdb->loadErrorStr) + return createFileError( + tsPath, make_error(*pdb->loadErrorStr, + llvm::inconvertibleErrorCode())); tsSrc = (TypeServerSource *)pdb->debugTypesObj; diff --git a/lld/COFF/InputFiles.cpp b/lld/COFF/InputFiles.cpp index 3e9b48b..84920ae 100644 --- a/lld/COFF/InputFiles.cpp +++ b/lld/COFF/InputFiles.cpp @@ -876,11 +876,13 @@ void PDBInputFile::parse() { ctx.pdbInputFileInstances[mb.getBufferIdentifier().str()] = this; std::unique_ptr thisSession; - loadErr.emplace(pdb::NativeSession::createFromPdb( - MemoryBuffer::getMemBuffer(mb, false), thisSession)); - if (*loadErr) + Error E = pdb::NativeSession::createFromPdb( + MemoryBuffer::getMemBuffer(mb, false), thisSession); + if (E) { + loadErrorStr.emplace(toString(std::move(E))); return; // fail silently at this point - the error will be handled later, // when merging the debug type stream + } session.reset(static_cast(thisSession.release())); @@ -888,7 +890,7 @@ void PDBInputFile::parse() { auto expectedInfo = pdbFile.getPDBInfoStream(); // All PDB Files should have an Info stream. if (!expectedInfo) { - loadErr.emplace(expectedInfo.takeError()); + loadErrorStr.emplace(toString(expectedInfo.takeError())); return; } debugTypesObj = makeTypeServerSource(ctx, this); diff --git a/lld/COFF/InputFiles.h b/lld/COFF/InputFiles.h index e7e3fdc..75c6ee5 100644 --- a/lld/COFF/InputFiles.h +++ b/lld/COFF/InputFiles.h @@ -319,7 +319,7 @@ public: StringRef path, ObjFile *fromFile); // Record possible errors while opening the PDB file - std::optional loadErr; + std::optional loadErrorStr; // This is the actual interface to the PDB (if it was opened successfully) std::unique_ptr session; diff --git a/lld/test/COFF/Inputs/pdb-type-server-corrupted-a.yaml b/lld/test/COFF/Inputs/pdb-type-server-corrupted-a.yaml new file mode 100644 index 0000000..7d8a13d --- /dev/null +++ b/lld/test/COFF/Inputs/pdb-type-server-corrupted-a.yaml @@ -0,0 +1,255 @@ +--- !COFF +header: + Machine: IMAGE_FILE_MACHINE_AMD64 + Characteristics: [ ] +sections: + - Name: .drectve + Characteristics: [ IMAGE_SCN_LNK_INFO, IMAGE_SCN_LNK_REMOVE ] + Alignment: 1 + SectionData: 2020202F44454641554C544C49423A224C4942434D5422202F44454641554C544C49423A224F4C444E414D45532220 + - Name: '.debug$S' + Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ] + Alignment: 1 + Subsections: + - !Symbols + Records: + - Kind: S_OBJNAME + ObjNameSym: + Signature: 0 + ObjectName: 'C:\src\llvm-project\build\a.obj' + - Kind: S_COMPILE3 + Compile3Sym: + Flags: [ SecurityChecks, HotPatch ] + Machine: X64 + FrontendMajor: 19 + FrontendMinor: 0 + FrontendBuild: 24215 + FrontendQFE: 1 + BackendMajor: 19 + BackendMinor: 0 + BackendBuild: 24215 + BackendQFE: 1 + Version: 'Microsoft (R) Optimizing Compiler' + - !Symbols + Records: + - Kind: S_GPROC32_ID + ProcSym: + CodeSize: 27 + DbgStart: 4 + DbgEnd: 22 + FunctionType: 4098 + Flags: [ ] + DisplayName: main + - Kind: S_FRAMEPROC + FrameProcSym: + TotalFrameBytes: 56 + PaddingFrameBytes: 0 + OffsetToPadding: 0 + BytesOfCalleeSavedRegisters: 0 + OffsetOfExceptionHandler: 0 + SectionIdOfExceptionHandler: 0 + Flags: [ AsynchronousExceptionHandling, OptimizedForSpeed ] + - Kind: S_REGREL32 + RegRelativeSym: + Offset: 32 + Type: 4102 + Register: RSP + VarName: f + - Kind: S_PROC_ID_END + ScopeEndSym: + - !Lines + CodeSize: 27 + Flags: [ ] + RelocOffset: 0 + RelocSegment: 0 + Blocks: + - FileName: 'c:\src\llvm-project\build\a.c' + Lines: + - Offset: 0 + LineStart: 3 + IsStatement: true + EndDelta: 0 + - Offset: 4 + LineStart: 4 + IsStatement: true + EndDelta: 0 + - Offset: 12 + LineStart: 5 + IsStatement: true + EndDelta: 0 + - Offset: 22 + LineStart: 6 + IsStatement: true + EndDelta: 0 + Columns: + - !Symbols + Records: + - Kind: S_UDT + UDTSym: + Type: 4102 + UDTName: Foo + - !FileChecksums + Checksums: + - FileName: 'c:\src\llvm-project\build\a.c' + Kind: MD5 + Checksum: BF69E7E933074E1B7ED1FE8FB395965B + - !StringTable + Strings: + - 'c:\src\llvm-project\build\a.c' + - !Symbols + Records: + - Kind: S_BUILDINFO + BuildInfoSym: + BuildId: 4107 + Relocations: + - VirtualAddress: 152 + SymbolName: main + Type: IMAGE_REL_AMD64_SECREL + - VirtualAddress: 156 + SymbolName: main + Type: IMAGE_REL_AMD64_SECTION + - VirtualAddress: 224 + SymbolName: main + Type: IMAGE_REL_AMD64_SECREL + - VirtualAddress: 228 + SymbolName: main + Type: IMAGE_REL_AMD64_SECTION + - Name: '.debug$T' + Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ] + Alignment: 1 + Types: + - Kind: LF_TYPESERVER2 + TypeServer2: + Guid: '{41414141-4141-4141-4141-414141414141}' + Age: 1 + Name: 'C:\src\llvm-project\build\bad-block-size.pdb' + - Name: '.text$mn' + Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ] + Alignment: 16 + SectionData: 4883EC38C74424202A000000488D4C2420E8000000004883C438C3 + Relocations: + - VirtualAddress: 18 + SymbolName: g + Type: IMAGE_REL_AMD64_REL32 + - Name: .xdata + Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ] + Alignment: 4 + SectionData: '0104010004620000' + - Name: .pdata + Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ] + Alignment: 4 + SectionData: 000000001B00000000000000 + Relocations: + - VirtualAddress: 0 + SymbolName: '$LN3' + Type: IMAGE_REL_AMD64_ADDR32NB + - VirtualAddress: 4 + SymbolName: '$LN3' + Type: IMAGE_REL_AMD64_ADDR32NB + - VirtualAddress: 8 + SymbolName: '$unwind$main' + Type: IMAGE_REL_AMD64_ADDR32NB +symbols: + - Name: .drectve + Value: 0 + SectionNumber: 1 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_STATIC + SectionDefinition: + Length: 47 + NumberOfRelocations: 0 + NumberOfLinenumbers: 0 + CheckSum: 0 + Number: 0 + - Name: '.debug$S' + Value: 0 + SectionNumber: 2 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_STATIC + SectionDefinition: + Length: 388 + NumberOfRelocations: 4 + NumberOfLinenumbers: 0 + CheckSum: 0 + Number: 0 + - Name: '.debug$T' + Value: 0 + SectionNumber: 3 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_STATIC + SectionDefinition: + Length: 64 + NumberOfRelocations: 0 + NumberOfLinenumbers: 0 + CheckSum: 0 + Number: 0 + - Name: '.text$mn' + Value: 0 + SectionNumber: 4 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_STATIC + SectionDefinition: + Length: 27 + NumberOfRelocations: 1 + NumberOfLinenumbers: 0 + CheckSum: 1939996292 + Number: 0 + - Name: g + Value: 0 + SectionNumber: 0 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_FUNCTION + StorageClass: IMAGE_SYM_CLASS_EXTERNAL + - Name: main + Value: 0 + SectionNumber: 4 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_FUNCTION + StorageClass: IMAGE_SYM_CLASS_EXTERNAL + - Name: '$LN3' + Value: 0 + SectionNumber: 4 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_LABEL + - Name: .xdata + Value: 0 + SectionNumber: 5 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_STATIC + SectionDefinition: + Length: 8 + NumberOfRelocations: 0 + NumberOfLinenumbers: 0 + CheckSum: 931692337 + Number: 0 + - Name: '$unwind$main' + Value: 0 + SectionNumber: 5 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_STATIC + - Name: .pdata + Value: 0 + SectionNumber: 6 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_STATIC + SectionDefinition: + Length: 12 + NumberOfRelocations: 3 + NumberOfLinenumbers: 0 + CheckSum: 567356797 + Number: 0 + - Name: '$pdata$main' + Value: 0 + SectionNumber: 6 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_STATIC +... diff --git a/lld/test/COFF/Inputs/pdb-type-server-corrupted-b.yaml b/lld/test/COFF/Inputs/pdb-type-server-corrupted-b.yaml new file mode 100644 index 0000000..08c896b --- /dev/null +++ b/lld/test/COFF/Inputs/pdb-type-server-corrupted-b.yaml @@ -0,0 +1,173 @@ +--- !COFF +header: + Machine: IMAGE_FILE_MACHINE_AMD64 + Characteristics: [ ] +sections: + - Name: .drectve + Characteristics: [ IMAGE_SCN_LNK_INFO, IMAGE_SCN_LNK_REMOVE ] + Alignment: 1 + SectionData: 2020202F44454641554C544C49423A224C4942434D5422202F44454641554C544C49423A224F4C444E414D45532220 + - Name: '.debug$S' + Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ] + Alignment: 1 + Subsections: + - !Symbols + Records: + - Kind: S_OBJNAME + ObjNameSym: + Signature: 0 + ObjectName: 'C:\src\llvm-project\build\b.obj' + - Kind: S_COMPILE3 + Compile3Sym: + Flags: [ SecurityChecks, HotPatch ] + Machine: X64 + FrontendMajor: 19 + FrontendMinor: 0 + FrontendBuild: 24215 + FrontendQFE: 1 + BackendMajor: 19 + BackendMinor: 0 + BackendBuild: 24215 + BackendQFE: 1 + Version: 'Microsoft (R) Optimizing Compiler' + - !Symbols + Records: + - Kind: S_GPROC32_ID + ProcSym: + CodeSize: 13 + DbgStart: 5 + DbgEnd: 12 + FunctionType: 4099 + Flags: [ ] + DisplayName: g + - Kind: S_FRAMEPROC + FrameProcSym: + TotalFrameBytes: 0 + PaddingFrameBytes: 0 + OffsetToPadding: 0 + BytesOfCalleeSavedRegisters: 0 + OffsetOfExceptionHandler: 0 + SectionIdOfExceptionHandler: 0 + Flags: [ AsynchronousExceptionHandling, OptimizedForSpeed ] + - Kind: S_REGREL32 + RegRelativeSym: + Offset: 8 + Type: 4097 + Register: RSP + VarName: p + - Kind: S_PROC_ID_END + ScopeEndSym: + - !Lines + CodeSize: 13 + Flags: [ ] + RelocOffset: 0 + RelocSegment: 0 + Blocks: + - FileName: 'c:\src\llvm-project\build\b.c' + Lines: + - Offset: 0 + LineStart: 2 + IsStatement: true + EndDelta: 0 + Columns: + - !Symbols + Records: + - Kind: S_UDT + UDTSym: + Type: 4102 + UDTName: Foo + - !FileChecksums + Checksums: + - FileName: 'c:\src\llvm-project\build\b.c' + Kind: MD5 + Checksum: DDF8FD35CD67990C5D4147516BE10D0C + - !StringTable + Strings: + - 'c:\src\llvm-project\build\b.c' + - !Symbols + Records: + - Kind: S_BUILDINFO + BuildInfoSym: + BuildId: 4111 + Relocations: + - VirtualAddress: 152 + SymbolName: g + Type: IMAGE_REL_AMD64_SECREL + - VirtualAddress: 156 + SymbolName: g + Type: IMAGE_REL_AMD64_SECTION + - VirtualAddress: 220 + SymbolName: g + Type: IMAGE_REL_AMD64_SECREL + - VirtualAddress: 224 + SymbolName: g + Type: IMAGE_REL_AMD64_SECTION + - Name: '.debug$T' + Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ] + Alignment: 1 + Types: + - Kind: LF_TYPESERVER2 + TypeServer2: + Guid: '{41414141-4141-4141-4141-414141414141}' + Age: 1 + Name: 'C:\src\llvm-project\build\bad-block-size.pdb' + - Name: '.text$mn' + Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ] + Alignment: 16 + SectionData: 48894C2408488B4424088B00C3 +symbols: + - Name: .drectve + Value: 0 + SectionNumber: 1 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_STATIC + SectionDefinition: + Length: 47 + NumberOfRelocations: 0 + NumberOfLinenumbers: 0 + CheckSum: 0 + Number: 0 + - Name: '.debug$S' + Value: 0 + SectionNumber: 2 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_STATIC + SectionDefinition: + Length: 360 + NumberOfRelocations: 4 + NumberOfLinenumbers: 0 + CheckSum: 0 + Number: 0 + - Name: '.debug$T' + Value: 0 + SectionNumber: 3 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_STATIC + SectionDefinition: + Length: 64 + NumberOfRelocations: 0 + NumberOfLinenumbers: 0 + CheckSum: 0 + Number: 0 + - Name: '.text$mn' + Value: 0 + SectionNumber: 4 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_NULL + StorageClass: IMAGE_SYM_CLASS_STATIC + SectionDefinition: + Length: 13 + NumberOfRelocations: 0 + NumberOfLinenumbers: 0 + CheckSum: 3246683207 + Number: 0 + - Name: g + Value: 0 + SectionNumber: 4 + SimpleType: IMAGE_SYM_TYPE_NULL + ComplexType: IMAGE_SYM_DTYPE_FUNCTION + StorageClass: IMAGE_SYM_CLASS_EXTERNAL +... diff --git a/lld/test/COFF/invalid-input-pdb.ll b/lld/test/COFF/invalid-input-pdb.ll new file mode 100644 index 0000000..4592f38 --- /dev/null +++ b/lld/test/COFF/invalid-input-pdb.ll @@ -0,0 +1,14 @@ +; REQUIRES: x86 +; RUN: llvm-as -o %t.obj %s + +; Verify that an invalid but unused input pdb file doesn't trigger any warning or error. +; RUN: lld-link /out:%t.exe %t.obj %S/Inputs/bad-block-size.pdb /entry:main /subsystem:console /WX + +target datalayout = "e-m:w-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128" +target triple = "x86_64-pc-windows-msvc" + +define void @main() { + ret void +} + + diff --git a/lld/test/COFF/pdb-type-server-corrupted.test b/lld/test/COFF/pdb-type-server-corrupted.test new file mode 100644 index 0000000..6c55368 --- /dev/null +++ b/lld/test/COFF/pdb-type-server-corrupted.test @@ -0,0 +1,14 @@ +RUN: rm -rf %t && mkdir -p %t && cd %t +RUN: yaml2obj %S/Inputs/pdb-type-server-corrupted-a.yaml -o a.obj +RUN: yaml2obj %S/Inputs/pdb-type-server-corrupted-b.yaml -o b.obj +RUN: cp %S/Inputs/bad-block-size.pdb ./bad-block-size.pdb +RUN: lld-link a.obj b.obj -entry:main -debug:noghash -out:t.exe -pdb:t.pdb -nodefaultlib 2>&1 | FileCheck %s + +Re-run with /DEBUG:GHASH +RUN: lld-link a.obj b.obj -entry:main -debug:ghash -out:t.exe -pdb:t.pdb -nodefaultlib 2>&1 | FileCheck %s + +# CHECK: warning: Cannot use debug info for {{.*}}a.obj +# CHECK-NEXT: failed to load reference '{{.*}}bad-block-size.pdb': The PDB file is corrupt. MSF superblock is missing + +# CHECK: warning: Cannot use debug info for {{.*}}b.obj +# CHECK-NEXT: failed to load reference '{{.*}}bad-block-size.pdb': The PDB file is corrupt. MSF superblock is missing -- 2.7.4