[LLD][COFF] Fix file/line retrieval when a undefined symbol is to be printed

[aganea]

Summary

Previously, if a symbol function was undefined, LLD was trying to print its referenced locations, and was failing with two different errors:

1. Stream Error: The specified offset is invalid for the current stream
2. CodeView Error: There are no records

Nothing else was printed.

The fix covers these two cases: one is a Lines subsection without anything in it; the other is when the provided Addr param is <= Line.Offset.

The original OBJ was compiled with Clang. The provided test is simply a reduction of our case.
The source of these two bugs seems to lie in how Clang emits Lines subsections; however I wanted to ensure LLD handles these correctly first.

[rnk]

COFF/PDB.cpp

1739 ↗

(On Diff #179131)

I think zero initializing these both is better than using Optional. The string table index 0 gives the empty string, right? It seems nicer to use that and avoid the extra conditional branches. I guess you'd still need the branch in the loop to say, if we don't have a filename so far but we have an entry, use the next entry. Although, the branchless way to write that is to pull the first filename string table index from the first line table entry if it exists.

[aganea]

COFF/PDB.cpp

1739 ↗

(On Diff #179131)

Unfourtunately it's a 0-based index in the checksums table:

*** FILECHKSUMS

FileId  St.Offset  Cb  Type  ChksumBytes
     0  00000001   10  MD5   D72EDEF8B8E50C364A330F9CB3CD904B
    18  00000065   10  MD5   D72EDEF8B8E50C364A330F9CB3CD904B
    30  000000A5   10  MD5   D72EDEF8B8E50C364A330F9CB3CD904B
    48  000000E8   10  MD5   D72EDEF8B8E50C364A330F9CB3CD904B

The string table indeed starts with an empty string:

*** STRINGTABLE

00000000 
00000001 C:\fastbuild-work\.fbuild.tmp\source\(edited).h
00000065 C:\fastbuild-work\.fbuild.tmp\source\(edited).h
000000a5 C:\fastbuild-work\.fbuild.tmp\source\(edited).h
000000e8 C:\fastbuild-work\.fbuild.tmp\source\(edited).h
0000014a C:\fastbuild-work\.fbuild.tmp\source\(edited).h

[aganea]

COFF/PDB.cpp

1739 ↗

(On Diff #179131)

Essentially, if you zero-out NameIndex (which I did at first), you end up with the wrong error message (the first filename, instead of nothing)

[rnk]

lgtm

COFF/PDB.cpp

1739 ↗

(On Diff #179131)

Right, wrong index.

[aganea]

Thank you! And happy holidays!

lld/trunk/COFF/PDB.cpp

std::pair<StringRef, uint32_t> coff::getFileLine(const SectionChunk *C,
   DebugStringTableSubsectionRef CVStrTab;
   DebugChecksumsSubsectionRef Checksums;
   DebugLinesSubsectionRef Lines;
   uint32_t OffsetInLinetable;
 
   if (!findLineTable(C, Addr, CVStrTab, Checksums, Lines, OffsetInLinetable))
     return {"", 0};
 
-  uint32_t NameIndex;
-  uint32_t LineNumber;
+  Optional<uint32_t> NameIndex;
+  Optional<uint32_t> LineNumber;
   for (LineColumnEntry &Entry : Lines) {
     for (const LineNumberEntry &LN : Entry.LineNumbers) {
+      LineInfo LI(LN.Flags);
       if (LN.Offset > OffsetInLinetable) {
+        if (!NameIndex) {
+          NameIndex = Entry.NameIndex;
+          LineNumber = LI.getStartLine();
+        }
         StringRef Filename =
-            ExitOnErr(getFileName(CVStrTab, Checksums, NameIndex));
-        return {Filename, LineNumber};
+            ExitOnErr(getFileName(CVStrTab, Checksums, *NameIndex));
+        return {Filename, *LineNumber};
       }
-      LineInfo LI(LN.Flags);
       NameIndex = Entry.NameIndex;
       LineNumber = LI.getStartLine();
     }
   }
-  StringRef Filename = ExitOnErr(getFileName(CVStrTab, Checksums, NameIndex));
-  return {Filename, LineNumber};
+  if (!NameIndex)
+    return {"", 0};
+  StringRef Filename = ExitOnErr(getFileName(CVStrTab, Checksums, *NameIndex));
+  return {Filename, *LineNumber};
 }

lld/trunk/test/COFF/line-error.yaml

+# RUN: yaml2obj %s -o %t.obj
+# RUN: not lld-link %t.obj /subsystem:console 2>&1 | FileCheck %s
+
+# CHECK: lld-link: error: undefined symbol: function
+# CHECK-NEXT: >>> referenced by {{.*}}line-error.yaml.tmp.obj:(caller1)
+# CHECK-NEXT: >>> referenced by E:\file.cpp:1935
+# CHECK-NEXT: >>>               {{.*}}line-error.yaml.tmp.obj:(caller22)
+
+--- !COFF
+header:          
+  Machine:         IMAGE_FILE_MACHINE_AMD64
+  Characteristics: [  ]
+sections:        
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       16
+    SectionData:     488B0500000000488B51284881C2D80000004889C148FF2500000000
+    Relocations:     
+      - VirtualAddress:  3
+        SymbolName:      function
+        Type:            IMAGE_REL_AMD64_REL32
+  - Name:            .text
+    Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_LNK_COMDAT, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+    Alignment:       16
+    SectionData:     488B0500000000488B51084881C2D80000004889C148FF2500000000
+    Relocations:     
+      - VirtualAddress:  3
+        SymbolName:      function
+        Type:            IMAGE_REL_AMD64_REL32
+  - Name:            '.debug$S'
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
+    Alignment:       4
+    Subsections:     
+      - !FileChecksums
+        Checksums:       
+          - FileName:        'E:\file.cpp'
+            Kind:            MD5
+            Checksum:        D72EDEF8B8E50C364A330F9CB3CD904B
+      - !StringTable
+        Strings:         
+          - 'E:\file.cpp'
+  - Name:            '.debug$S'
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
+    Alignment:       4
+    Subsections:     
+      - !Lines
+        CodeSize:        28
+        Flags:           [  ]
+        RelocOffset:     0
+        RelocSegment:    0
+        Blocks:          []
+    Relocations:     
+      - VirtualAddress:  12
+        SymbolName:      caller1
+        Type:            IMAGE_REL_AMD64_SECREL
+  - Name:            '.debug$S'
+    Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_LNK_COMDAT, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
+    Alignment:       4
+    Subsections:     
+      - !Lines
+        CodeSize:        28
+        Flags:           [  ]
+        RelocOffset:     0
+        RelocSegment:    0
+        Blocks:          
+          - FileName:        'E:\file.cpp'
+            Lines:           
+              - Offset:          11
+                LineStart:       1935
+                IsStatement:     false
+                EndDelta:        0
+            Columns:         []
+    Relocations:     
+      - VirtualAddress:  12
+        SymbolName:      caller22
+        Type:            IMAGE_REL_AMD64_SECREL
+symbols:
+  - Name:            .text
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition: 
+      Length:          28
+      NumberOfRelocations: 2
+      NumberOfLinenumbers: 0
+      CheckSum:        2430089736
+      Number:          1
+      Selection:       IMAGE_COMDAT_SELECT_NODUPLICATES
+  - Name:            caller1
+    Value:           0
+    SectionNumber:   1
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL    
+  - Name:            .text
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition: 
+      Length:          28
+      NumberOfRelocations: 2
+      NumberOfLinenumbers: 0
+      CheckSum:        3449717304
+      Number:          2
+      Selection:       IMAGE_COMDAT_SELECT_NODUPLICATES
+  - Name:            caller22
+    Value:           0
+    SectionNumber:   2
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_FUNCTION
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL    
+  - Name:            '.debug$S'
+    Value:           0
+    SectionNumber:   3
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition: 
+      Length:          767204
+      NumberOfRelocations: 0
+      NumberOfLinenumbers: 0
+      CheckSum:        4280093374
+      Number:          3
+  - Name:            '.debug$S'
+    Value:           0
+    SectionNumber:   4
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition: 
+      Length:          296
+      NumberOfRelocations: 1
+      NumberOfLinenumbers: 0
+      CheckSum:        1957793731
+      Number:          1
+      Selection:       IMAGE_COMDAT_SELECT_ASSOCIATIVE
+  - Name:            '.debug$S'
+    Value:           0
+    SectionNumber:   5
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_STATIC
+    SectionDefinition: 
+      Length:          276
+      NumberOfRelocations: 1
+      NumberOfLinenumbers: 0
+      CheckSum:        1957793731
+      Number:          2
+      Selection:       IMAGE_COMDAT_SELECT_ASSOCIATIVE
+  - Name:            function
+    Value:           0
+    SectionNumber:   0
+    SimpleType:      IMAGE_SYM_TYPE_NULL
+    ComplexType:     IMAGE_SYM_DTYPE_NULL
+    StorageClass:    IMAGE_SYM_CLASS_EXTERNAL
+...