This is an archive of the discontinued LLVM Phabricator instance.

[DebugInfo] Fix register variables not showing up in pdb
ClosedPublic

Authored by aganea on Dec 6 2017, 1:48 PM.

Details

Summary

Previously, when linking against "C:/Program Files (x86)/Microsoft Visual Studio 14.0/VC/lib/libcmt.lib", lld-link.exe /verbose shows the following warning multiple times:

"ignoring unknown symbol record with kind 0x1106"

With the enclosed patch the warning is gone and pdb2yaml now properly shows S_REGISTER entries.

Diff Detail

Repository
rL LLVM

Event Timeline

aganea created this revision.Dec 6 2017, 1:48 PM
aganea edited the summary of this revision. (Show Details)Dec 6 2017, 1:48 PM
zturner edited edge metadata.Dec 6 2017, 1:58 PM

Thanks for finding this! Can you add update TypeIndexDiscoveryTest.cpp in unittests/DebugInfo/CodeView to contain a RegisterRecord so that we can make sure this stays working?

aganea updated this revision to Diff 126003.Dec 7 2017, 11:20 AM
aganea edited the summary of this revision. (Show Details)
zturner accepted this revision.Dec 7 2017, 11:22 AM

lgtm with one small nit. If you have commit access, can you fix that before submitting? If you don't have commit access, you don't need to do anything, I will fix it for you before submitting.

TypeIndexDiscoveryTest.cpp
556

This should be called Reg.

This revision is now accepted and ready to land.Dec 7 2017, 11:22 AM
zturner added inline comments.Dec 7 2017, 11:23 AM
TypeIndexDiscoveryTest.cpp
556

To elaborate, we use CamelCase, but all caps is acceptable when abbreviating a CamelCase word. For example, DataSym -> DS is ok, but DATASYM is not ok. Similarly, since Reg isn't abbreviate,d it should be Reg, not REG.

aganea updated this revision to Diff 126008.EditedDec 7 2017, 11:31 AM
aganea marked 2 inline comments as done.

Thanks for the review Zachary. I don't have commit access - I'll ask. I'm currently switching our (PC) game projects from MSVC to use Clang+LLD. I have the feeling I'll have other fixes coming ;-)

Sounds good, let me know if I can be of any help!

aganea added a comment.Dec 7 2017, 2:38 PM

Sounds good, let me know if I can be of any help!

Could you submit the patch Zachary please? From the developer rules, it seems commit access is only granted along with a track of submitted changes ;)

Sure, I'll submit it. By the way, how are you generating the diff to upload? It looks like TypeIndexDiscovery.cpp and TypeIndexDiscoveryTest.cpp are in the same folder, which makes the diff not apply cleanly on a standard repository layout. I can fix it up locally since the patch is small, but if you're going to submit other, potentially larger patches, it might be worth figuring this out.

aganea added a comment.Dec 7 2017, 2:45 PM

Sure, I'll submit it. By the way, how are you generating the diff to upload? It looks like TypeIndexDiscovery.cpp and TypeIndexDiscoveryTest.cpp are in the same folder, which makes the diff not apply cleanly on a standard repository layout. I can fix it up locally since the patch is small, but if you're going to submit other, potentially larger patches, it might be worth figuring this out.

Through TortoiseSVN - my bad I created single patches directly for each file. It seems Tortoise doesn't add the complete repository path if you do that. I'll resubmit a patch from the base.

aganea updated this revision to Diff 126054.Dec 7 2017, 2:46 PM
This revision was automatically updated to reflect the committed changes.