This is an archive of the discontinued LLVM Phabricator instance.

Differential D55951

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

Authored by aganea on Dec 20 2018, 12:54 PM.

Details

Reviewers
pcc
rnk
Commits
rG79d485167879: [LLD][COFF] Fix file/line retrieval when a undefined symbol is to be printed
rLLD350438: [LLD][COFF] Fix file/line retrieval when a undefined symbol is to be printed
rL350438: [LLD][COFF] Fix file/line retrieval when a undefined symbol is to be printed
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.

Diff Detail

Repository
rLLD LLVM Linker

Event Timeline

aganea created this revision.Dec 20 2018, 12:54 PM
aganea added a subscriber: belkiss.Dec 20 2018, 12:57 PM
rnk added inline comments.Dec 21 2018, 12:13 PM
COFF/PDB.cpp
1739

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 marked an inline comment as done.Dec 21 2018, 12:28 PM
aganea added inline comments.
COFF/PDB.cpp
1739

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 marked an inline comment as done.Dec 21 2018, 12:33 PM
aganea added inline comments.
COFF/PDB.cpp
1739

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 accepted this revision.Dec 21 2018, 1:34 PM

lgtm

COFF/PDB.cpp
1739

Right, wrong index.

This revision is now accepted and ready to land.Dec 21 2018, 1:34 PM
aganea added a comment.Dec 21 2018, 1:36 PM

Thank you! And happy holidays!

Closed by commit rL350438: [LLD][COFF] Fix file/line retrieval when a undefined symbol is to be printed (authored by aganea). · Explain WhyJan 4 2019, 1:53 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents

PathSize
COFF/
20 lines
test/
COFF/
160 lines

Diff 179131

COFF/PDB.cpp

Show First 20 Lines • Show All 1,729 Lines • ▼ Show 20 Linesstd::pair<StringRef, uint32_t> coff::getFileLine(const SectionChunk *C,
DebugStringTableSubsectionRef CVStrTab; DebugStringTableSubsectionRef CVStrTab;
DebugChecksumsSubsectionRef Checksums; DebugChecksumsSubsectionRef Checksums;
DebugLinesSubsectionRef Lines; DebugLinesSubsectionRef Lines;
uint32_t OffsetInLinetable; uint32_t OffsetInLinetable;
if (!findLineTable(C, Addr, CVStrTab, Checksums, Lines, OffsetInLinetable)) if (!findLineTable(C, Addr, CVStrTab, Checksums, Lines, OffsetInLinetable))
return {"", 0}; return {"", 0};
uint32_t NameIndex; Optional<uint32_t> NameIndex;
uint32_t LineNumber; Optional<uint32_t> LineNumber;
rnkUnsubmitted
Not Done
ReplyInline Actions

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.

rnk: I think zero initializing these both is better than using Optional. The string table index 0…
aganeaAuthorUnsubmitted
Done
ReplyInline Actions

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: Unfourtunately it's a 0-based index in the checksums table: ``` *** FILECHKSUMS FileId St.
aganeaAuthorUnsubmitted
Done
ReplyInline Actions

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)

aganea: Essentially, if you zero-out `NameIndex` (which I did at first), you end up with the wrong…
rnkUnsubmitted
Not Done
ReplyInline Actions

Right, wrong index.

rnk: Right, wrong index.
for (LineColumnEntry &Entry : Lines) { for (LineColumnEntry &Entry : Lines) {
for (const LineNumberEntry &LN : Entry.LineNumbers) { for (const LineNumberEntry &LN : Entry.LineNumbers) {
LineInfo LI(LN.Flags);
if (LN.Offset > OffsetInLinetable) { if (LN.Offset > OffsetInLinetable) {
if (!NameIndex) {
NameIndex = Entry.NameIndex;
LineNumber = LI.getStartLine();
}
StringRef Filename = StringRef Filename =
ExitOnErr(getFileName(CVStrTab, Checksums, NameIndex)); ExitOnErr(getFileName(CVStrTab, Checksums, *NameIndex));
return {Filename, LineNumber}; return {Filename, *LineNumber};
} }
LineInfo LI(LN.Flags);
NameIndex = Entry.NameIndex; NameIndex = Entry.NameIndex;
LineNumber = LI.getStartLine(); LineNumber = LI.getStartLine();
} }
} }
StringRef Filename = ExitOnErr(getFileName(CVStrTab, Checksums, NameIndex)); if (!NameIndex)
return {Filename, LineNumber}; return {"", 0};
StringRef Filename = ExitOnErr(getFileName(CVStrTab, Checksums, *NameIndex));
return {Filename, *LineNumber};
} }

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