[lldb][COFF] Improve info of symbols from export table

[alvinhochun]

Summary

• Skip dummy/invalid export symbols.
• Make the export ordinal of export symbols visible when dumping the symtab.
• Stop setting the 'Debug' flag and set the 'External' flag instead to better match the meaning of export symbols.
• Try to guess the type (code vs data) of the symbol from section flags.

Depends on D134196

[mstorsjo]

Looks good to me in general. Do any of the changes make a functional difference, or is it just improved (and less misleading) display to the user of the debugger?

[alvinhochun]

This comment was removed by alvinhochun.

[alvinhochun]

I think marking the symbol as Data instead of Code does have an effect on commands that look for functions (e.g. "disassemble" will not disassemble a Data symbol, which makes sense). For the rest of the changes I think they are only visible in the symtab dump.

(Sorry, I got confused and made a mistake in my previous comment.)

[alvinhochun]

Updated to new test from parent

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

if (auto err = entry.getSymbolName(sym_name)) {
                "table entry name: {0}",
                llvm::fmt_consume(std::move(err)));
       continue;
     }
     Symbol symbol;
     // Note: symbol name may be empty if it is only exported by ordinal.
     symbol.GetMangled().SetValue(ConstString(sym_name));
 
+    uint32_t ordinal;
+    llvm::cantFail(entry.getOrdinal(ordinal));
+    symbol.SetID(ordinal);
+
     uint32_t function_rva;
     if (auto err = entry.getExportRVA(function_rva)) {
       LLDB_LOG(log,
                "ObjectFilePECOFF::AppendFromExportTable - failed to get "
                "address of export entry '{0}': {1}",
                sym_name, llvm::fmt_consume(std::move(err)));
       continue;
     }
+    // Skip the symbol if it doesn't look valid.
+    if (function_rva == 0 && sym_name.empty())
+      continue;
     symbol.GetAddressRef() =
         Address(m_coff_header_opt.image_base + function_rva, sect_list);
+
+    // An exported symbol may be either code or data. Guess by checking whether
+    // the section containing the symbol is executable.
+    symbol.SetType(lldb::eSymbolTypeData);
+    if (auto section_sp = symbol.GetAddressRef().GetSection())
+      if (section_sp->GetPermissions() & ePermissionsExecutable)
         symbol.SetType(lldb::eSymbolTypeCode);
-    symbol.SetDebug(true);
+    symbol.SetExternal(true);
     symtab.AddSymbol(symbol);
   }
 }
 
 std::unique_ptr<CallFrameInfo> ObjectFilePECOFF::CreateCallFrameInfo() {
   if (llvm::COFF::EXCEPTION_TABLE >= m_coff_header_opt.data_dirs.size())
     return {};
 

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

 # RUN: yaml2obj %s -o %t
 # RUN: lldb-test symbols %t | FileCheck %s
 
 # Checks that the symtab contains both symbols from the export table and the
 # COFF symbol table.
 
 # CHECK:          UserID DSX Type    File Address/Value {{.*}} Size            Flags           Name
 # CHECK-NEXT:     ------
-# CHECK-NEXT: 4294967295 D   Code    0x0000000180001020        0x{{[0-9a-f]+}} 0x{{[0-9a-f]+}} exportFnAlias
-# CHECK-NEXT: 4294967295 D   Code    0x0000000180001010        0x{{[0-9a-f]+}} 0x{{[0-9a-f]+}} exportFunc
-# CHECK-NEXT: 4294967295 D   Code    0x0000000180003000        0x{{[0-9a-f]+}} 0x{{[0-9a-f]+}} exportInt
-# CHECK-NEXT: 4294967295 D   Code    0x0000000180003004        0x{{[0-9a-f]+}} 0x{{[0-9a-f]+}} exportIntAlias
+# CHECK-NEXT:          1   X Code    0x0000000180001020        0x{{[0-9a-f]+}} 0x{{[0-9a-f]+}} exportFnAlias
+# 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     Code    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     Invalid 0x0000000180003000        0x{{[0-9a-f]+}} 0x{{[0-9a-f]+}} exportInt
 # CHECK-NEXT: 4294967295     Invalid 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-EMPTY: