[lld][COFF] Fix lld-link crash when several .obj files built with /Zi refer to a .pdb file that failed to load

[saudi]

Summary

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:

• making it 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.

[saudi]

Fixed clang-format

[aganea]

lld/COFF/InputFiles.cpp

899

I wouldn't add those two accessor functions, and would keep loadErrorStr public. The intent is already clear enough by accessing std::optional and you seem to only use getLoadError once. What about moving the make_error.. line to the call site and compose it with createFileError?

[saudi]

Followed @aganea's suggestion, it's indeed clearer, thanks!

[aganea]

Thanks for making the changes! LGTM.

[thieta]

Oh sorry for the late reply. I think removing the getter makes sense as well - it worked better when we had the earlier revision that copied string out of the Error etc.

Anyway LGTM and this hopefully fixes the crash we got reported internally.

lld/COFF/DebugTypes.cpp

 // is discovered from the dependent OBJ's debug type stream. The
 // TypeServerSource object is then queued and loaded by the COFF Driver. The
 // debug type stream for such PDB files will be merged first in the final PDB,
 // before any dependent OBJ.
 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();
     if (!expectedInfo)
       return;
     Guid = expectedInfo->getGuid();
     auto it = ctx.typeServerSourceMappings.emplace(Guid, this);
     if (!it.second) {
Expected<TypeServerSource *> UseTypeServerSource::getTypeServerSource() {
   }
   if (tsSrc == nullptr) {
     // The file failed to load, lookup by name
     PDBInputFile *pdb = PDBInputFile::findFromRecordPath(ctx, tsPath, file);
     if (!pdb)
       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<StringError>(*pdb->loadErrorStr,
+                                          llvm::inconvertibleErrorCode()));
 
     tsSrc = (TypeServerSource *)pdb->debugTypesObj;
 
     // Just because a file with a matching name was found and it was an actual
     // PDB file doesn't mean it matches.  For it to match the InfoStream's GUID
     // must match the GUID specified in the TypeServer2 record.
     if (tsSrc->Guid != tsId) {
       return createFileError(tsPath,

lld/COFF/InputFiles.h

public:
   ~PDBInputFile();
   static bool classof(const InputFile *f) { return f->kind() == PDBKind; }
   void parse() override;
 
   static PDBInputFile *findFromRecordPath(const COFFLinkerContext &ctx,
                                           StringRef path, ObjFile *fromFile);
 
   // Record possible errors while opening the PDB file
-  std::optional<Error> loadErr;
+  std::optional<std::string> loadErrorStr;
 
   // This is the actual interface to the PDB (if it was opened successfully)
   std::unique_ptr<llvm::pdb::NativeSession> session;
 
   // If the PDB has a .debug$T stream, this tells how it will be handled.
   TpiSource *debugTypesObj = nullptr;
 };
 

lld/COFF/InputFiles.cpp

if (it != ctx.pdbInputFileInstances.end())
     return it->second;
   return nullptr;
 }
 
 void PDBInputFile::parse() {
   ctx.pdbInputFileInstances[mb.getBufferIdentifier().str()] = this;
 
   std::unique_ptr<pdb::IPDBSession> 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<pdb::NativeSession *>(thisSession.release()));
 
   pdb::PDBFile &pdbFile = session->getPDBFile();
   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);
 }
 
 // Used only for DWARF debug info, which is not common (except in MinGW
 // environments). This returns an optional pair of file name and line
 // number for where the variable was defined.
 std::optional<std::pair<StringRef, uint32_t>>
 ObjFile::getVariableLocation(StringRef var) {
   if (!dwarf) {
     dwarf = make<DWARFCache>(DWARFContext::create(*getCOFFObj()));
     if (!dwarf)
       return std::nullopt;

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

+--- !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
+...

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

+--- !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
+...

lld/test/COFF/invalid-input-pdb.ll

+; 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
+}
+
+

lld/test/COFF/pdb-type-server-corrupted.test

+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