This is an archive of the discontinued LLVM Phabricator instance.

[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

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

Thank you! And happy holidays!

This revision was automatically updated to reflect the committed changes.