[lldb][COFF] Map symbols without base+complex type as 'Data' type

[alvinhochun]

Summary

Both LLD and GNU ld write global/static variables to the COFF symbol
table with IMAGE_SYM_TYPE_NULL and IMAGE_SYM_DTYPE_NULL type. Map
these symbols as 'Data' type in the symtab to allow these symbols to be
used in expressions and printable.

Depends on D134426

[mstorsjo]

This LGTM, but I'll leave it to some LLDB maintainer to formally approve.

[DavidSpickett]

You're the expert on what the linker does and the code looks fine.

Is it possible that msvc uses these IMAGE_SYM_TYPE_NULL in a different way that could cause a problem? Worst that happens is we get a bunch of symbols available in expressions that shouldn't be?

[mstorsjo]

In D134585#3814458, @DavidSpickett wrote:

You're the expert on what the linker does and the code looks fine.

Is it possible that msvc uses these IMAGE_SYM_TYPE_NULL in a different way that could cause a problem? Worst that happens is we get a bunch of symbols available in expressions that shouldn't be?

MSVC/link.exe doesn't write symbols into linked PE images at all. But at the object file stage, they do express data symbols in the same way (which is the default marking for symbols that have no other types set).

[DavidSpickett]

MSVC/link.exe doesn't write symbols into linked PE images at all.

So by the time we get to a debugger, it's not an issue anyway.

Then this LGTM.

Thanks for all these improvements btw, I expect Linaro's Windows on Arm efforts appreciate them too.

[mstorsjo]

In D134585#3814463, @DavidSpickett wrote:

MSVC/link.exe doesn't write symbols into linked PE images at all.

So by the time we get to a debugger, it's not an issue anyway.

Yep, most of these patches about symbol table handling doesn't make any difference for the MSVC ecosystem usage at all (although some of these patches fix other generic windows-specific bugs noticed too).

Then this LGTM.

Thanks! Can you mark it formally approved too? :-)

[DavidSpickett]

Would help if I actually clicked the buttons yes :)

lldb/source/Plugins/ObjectFile/PECOFF/ObjectFilePECOFF.cpp

 lldb::SymbolType ObjectFilePECOFF::MapSymbolType(uint16_t coff_symbol_type) {
   // TODO:  We need to complete this mapping of COFF symbol types to LLDB ones.
   // For now, here's a hack to make sure our function have types.
   const auto complex_type =
       coff_symbol_type >> llvm::COFF::SCT_COMPLEX_TYPE_SHIFT;
   if (complex_type == llvm::COFF::IMAGE_SYM_DTYPE_FUNCTION) {
     return lldb::eSymbolTypeCode;
   }
+  const auto base_type = coff_symbol_type & 0xff;
+  if (base_type == llvm::COFF::IMAGE_SYM_TYPE_NULL &&
+      complex_type == llvm::COFF::IMAGE_SYM_DTYPE_NULL) {
+    // Unknown type. LLD and GNU ld uses this for variables on MinGW, so
+    // consider these symbols to be data to enable printing.
+    return lldb::eSymbolTypeData;
+  }
   return lldb::eSymbolTypeInvalid;
 }
 
 bool ObjectFilePECOFF::CreateBinary() {
   if (m_binary)
     return true;
 
   Log *log = GetLog(LLDBLog::Object);

lldb/test/Shell/ObjectFile/PECOFF/symbol.yaml

 # RUN: yaml2obj %s -o %t
 # RUN: lldb-test symbols %t | FileCheck %s
 
 ## The .file symbol isn't checked, but is included to test that the symbol
 ## table iteration handles cases with a symbol with more than one aux symbol.
 
 # CHECK: Type     File Address/Value {{.*}} Size            Flags           Name
 # CHECK: Code     0x0000000040001000        0x{{[0-9a-f]+}} 0x{{[0-9a-f]+}} entry
-# CHECK:          0x0000000040002000        0x{{[0-9a-f]+}} 0x{{[0-9a-f]+}} variable
+# CHECK: Data     0x0000000040002000        0x{{[0-9a-f]+}} 0x{{[0-9a-f]+}} variable
 # CHECK: Absolute 0x00000000deadbeef        0x{{[0-9a-f]+}} 0x{{[0-9a-f]+}} absolute_symbol
 
 --- !COFF
 OptionalHeader:
   AddressOfEntryPoint: 4096
   ImageBase:       1073741824
   SectionAlignment: 4096
   FileAlignment:   512

lldb/test/Shell/ObjectFile/PECOFF/symbols-export-table.yaml

 # CHECK-NEXT:          2   X Code       0x0000000180001010        0x{{[0-9a-f]+}} 0x{{[0-9a-f]+}} exportFunc
 # CHECK-NEXT:          3   X Data       0x0000000180003000        0x{{[0-9a-f]+}} 0x{{[0-9a-f]+}} exportInt
 # CHECK-NEXT:          4   X Data       0x0000000180003004        0x{{[0-9a-f]+}} 0x{{[0-9a-f]+}} exportIntAlias
 # CHECK-NEXT: 4294967295     Code       0x0000000180001000        0x{{[0-9a-f]+}} 0x{{[0-9a-f]+}} entry
 # CHECK-NEXT: 4294967295   X Additional 0x0000000180001010        0x{{[0-9a-f]+}} 0x{{[0-9a-f]+}} exportFunc
 # CHECK-NEXT: 4294967295     Code       0x0000000180001020        0x{{[0-9a-f]+}} 0x{{[0-9a-f]+}} aliasFunc
 # CHECK-NEXT: 4294967295   X Additional 0x0000000180003000        0x{{[0-9a-f]+}} 0x{{[0-9a-f]+}} exportInt
 # CHECK-NEXT: 4294967295     Data       0x0000000180003004        0x{{[0-9a-f]+}} 0x{{[0-9a-f]+}} aliasInt
-# CHECK-NEXT: 4294967295     Invalid    0x0000000180003008        0x{{[0-9a-f]+}} 0x{{[0-9a-f]+}} internalInt
+# CHECK-NEXT: 4294967295     Data       0x0000000180003008        0x{{[0-9a-f]+}} 0x{{[0-9a-f]+}} internalInt
 # CHECK-EMPTY:
 
 # Test file generated with:
 #   clang -O2 --target=x86_64-windows-msvc test.c -nostdlib -c -o test.obj
 #   lld-link -debug:symtab -dll -out:test.dll -entry:entry -export:exportFnAlias=aliasFunc -export:exportIntAlias=aliasInt test.obj
 # test.c:
 #   __declspec(dllexport) int exportInt;
 #   int aliasInt;