[lld][COFF] Fix lld-link crash when several .obj files built with /Zi refer to a...
authorSylvain Audi <sylvain.audi@ubisoft.com>
Thu, 15 Dec 2022 15:56:47 +0000 (10:56 -0500)
committerSylvain Audi <sylvain.audi@ubisoft.com>
Wed, 21 Dec 2022 21:13:46 +0000 (16:13 -0500)
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
lld/COFF/InputFiles.cpp
lld/COFF/InputFiles.h
lld/test/COFF/Inputs/pdb-type-server-corrupted-a.yaml [new file with mode: 0644]
lld/test/COFF/Inputs/pdb-type-server-corrupted-b.yaml [new file with mode: 0644]
lld/test/COFF/invalid-input-pdb.ll [new file with mode: 0644]
lld/test/COFF/pdb-type-server-corrupted.test [new file with mode: 0644]

index d723e48..66b9b99 100644 (file)
@@ -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<TypeServerSource *> 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<StringError>(*pdb->loadErrorStr,
+                                          llvm::inconvertibleErrorCode()));
 
     tsSrc = (TypeServerSource *)pdb->debugTypesObj;
 
index 3e9b48b..84920ae 100644 (file)
@@ -876,11 +876,13 @@ 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()));
 
@@ -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);
index e7e3fdc..75c6ee5 100644 (file)
@@ -319,7 +319,7 @@ public:
                                           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;
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 (file)
index 0000000..7d8a13d
--- /dev/null
@@ -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 (file)
index 0000000..08c896b
--- /dev/null
@@ -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 (file)
index 0000000..4592f38
--- /dev/null
@@ -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 (file)
index 0000000..6c55368
--- /dev/null
@@ -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