[DebugInfo] Fix register variables not showing up in pdb

[aganea]

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.

[zturner]

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?

[zturner]

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.

[zturner]

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]

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

[zturner]

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

[aganea]

In D40919#948592, @zturner wrote:

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

[zturner]

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]

In D40919#948898, @zturner wrote:

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.

TypeIndexDiscovery.cpp

   case SymbolKind::S_LOCAL:
     Refs.push_back({TiRefKind::TypeRef, 0, 1}); // Type
     break;
+  case SymbolKind::S_REGISTER:
+    Refs.push_back({TiRefKind::TypeRef, 0, 1}); // Type
+    break;
   case SymbolKind::S_CONSTANT:
     Refs.push_back({TiRefKind::TypeRef, 0, 1}); // Type
     break;

TypeIndexDiscoveryTest.cpp

 
 TEST_F(TypeIndexIteratorTest, ProcSym) {
   ProcSym GS(SymbolRecordKind::GlobalProcSym);
-  GS.FunctionType = TypeIndex(0x40);
+  GS.FunctionType = TypeIndex::Float32();
   ProcSym LS(SymbolRecordKind::ProcSym);
-  LS.FunctionType = TypeIndex(0x41);
+  LS.FunctionType = TypeIndex::Float64();
   writeSymbolRecords(GS, LS);
   checkTypeReferences(0, GS.FunctionType);
   checkTypeReferences(1, LS.FunctionType);

 
 TEST_F(TypeIndexIteratorTest, DataSym) {
   DataSym DS(SymbolRecordKind::GlobalData);
-  DS.Type = TypeIndex(0x40);
+  DS.Type = TypeIndex::Float32();
   writeSymbolRecords(DS);
   checkTypeReferences(0, DS.Type);
 }
 
+TEST_F(TypeIndexIteratorTest, RegisterSym) {
+    RegisterSym Reg(SymbolRecordKind::RegisterSym);
+    Reg.Index = TypeIndex::UInt32();
+    Reg.Register = RegisterId::EAX;
+    Reg.Name = "Target";
+    writeSymbolRecords(Reg);
+    checkTypeReferences(0, Reg.Index);
+}
+
 TEST_F(TypeIndexIteratorTest, CallerSym) {
   CallerSym Callees(SymbolRecordKind::CalleeSym);
   Callees.Indices.push_back(TypeIndex(1));