Enable more abilities in SymbolFilePDB

[asmith]

Summary

1. Finding symbols through --symfile
2. More abilities: Functions, Blocks, GlobalVariables, LocalVariables, VariableTypes

[clayborg]

source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp

127

I am assuming this assert won't fire if we give this an ELF or MachO file or any file that doesn't contain PDB info? Every SymbolFile subclass gets to calculate its abilities on each file until on of them returns that they can handle all abilities, or until all plug-ins have had a chance to answer and then the best one is picked. Seems like this shouldn't be here? I can't remember what checks get run before SymbolFile::CalculateAbilities() is called...

[zturner]

source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp

108–125

LLVM coding style dictates the use of early returns whenever possible. So this block should be changed to something like.

auto module_sp = m_obj_file->GetModule();
if (!module_sp)
  return 0;

FileSpec symfile = module_sp->GetSymbolFileSpec();
if (!symfile)
  return 0;

error = loadDataForPDB(...);
if (error) {
  llvm::consumeError(error);
  return 0;
}

// etc

141

Similar thing here. Assign first, and then add:

if (!enum_dbg_streams_up)
  break;

144

if (stream_name != "SECTIONHEADERS")
  continue;

151–157

Is this correct? If *any* of these 3 sections of present, then *all* of these capabilities are present?

Shouldn't we be actually trying to query DIA for a global, local, or variable and then seeing what it returns?

(Incidentally, it's easy to figure this out with the native PDB reader, since you can just look for the presence of a module symbol stream, globals stream, and/or TPI stream).

[asmith]

source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp

127

Any ELF or MachO other than PDB or PECOFF will error when calling loadDataForEXE or loadDataForPDB and return before reaching the assertion.

[asmith]

If a Symbols table is present then lldb can retrieve symbols for the types listed in PDBSym_Type. In other words, if we want symbolic information for a function then checking for the Symbols table is sufficient.

[zturner]

This looks better. Technically it's possible to break this with some weird PDBs but I don't think it's possible to do better without using the native reader.

source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp

 #include "lldb/Symbol/TypeMap.h"
 
 #include "llvm/DebugInfo/PDB/GenericError.h"
+#include "llvm/DebugInfo/PDB/IPDBDataStream.h"
 #include "llvm/DebugInfo/PDB/IPDBEnumChildren.h"
 #include "llvm/DebugInfo/PDB/IPDBLineNumber.h"
 #include "llvm/DebugInfo/PDB/IPDBSourceFile.h"
+#include "llvm/DebugInfo/PDB/IPDBTable.h"
 #include "llvm/DebugInfo/PDB/PDBSymbol.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolCompiland.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolCompilandDetails.h"
+#include "llvm/DebugInfo/PDB/PDBSymbolData.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolExe.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolFunc.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolFuncDebugEnd.h"

 SymbolFilePDB::~SymbolFilePDB() {}
 
 uint32_t SymbolFilePDB::CalculateAbilities() {
+  uint32_t abilities = 0;
+  if (!m_obj_file)
+    return 0;
+
   if (!m_session_up) {
     // Lazily load and match the PDB file, but only do this once.
     std::string exePath = m_obj_file->GetFileSpec().GetPath();

                                 m_session_up);
     if (error) {
       llvm::consumeError(std::move(error));
-      return 0;
+      auto module_sp = m_obj_file->GetModule();
+      if (!module_sp)
+        return 0;
+      // See if any symbol file is specified through `--symfile` option.
+      FileSpec symfile = module_sp->GetSymbolFileFileSpec();
+      if (!symfile)
+        return 0;
+      error = loadDataForPDB(PDB_ReaderType::DIA,
+                             llvm::StringRef(symfile.GetPath()),
+                             m_session_up);
+      if (error) {
+        llvm::consumeError(std::move(error));
+        return 0;
+      }
+    }
+  }
+  if (!m_session_up.get())
+    return 0;
+
+  auto enum_tables_up = m_session_up->getEnumTables();
+  if (!enum_tables_up)
+    return 0;
+  while (auto table_up = enum_tables_up->getNext()) {
+    if (table_up->getItemCount() == 0)
+      continue;
+    auto type = table_up->getTableType();
+    switch (type) {
+    case PDB_TableType::Symbols:
+      // This table represents a store of symbols with types listed in
+      // PDBSym_Type
+      abilities |= (CompileUnits | Functions | Blocks |
+                    GlobalVariables | LocalVariables | VariableTypes);
+      break;
+    case PDB_TableType::LineNumbers:
+      abilities |= LineTables;
+      break;
+    default: break;
     }
   }
-  return CompileUnits | LineTables;
+  return abilities;
 }
 
 void SymbolFilePDB::InitializeObject() {

unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp

   EXPECT_NE(nullptr, symfile);
   EXPECT_EQ(symfile->GetPluginName(), SymbolFilePDB::GetPluginNameStatic());
 
-  uint32_t expected_abilities =
-      SymbolFile::CompileUnits | SymbolFile::LineTables;
+  uint32_t expected_abilities = SymbolFile::kAllAbilities;
   EXPECT_EQ(expected_abilities, symfile->CalculateAbilities());
 }