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/ClangASTContext.h"
 #include "lldb/Symbol/CompileUnit.h"
 #include "lldb/Symbol/LineTable.h"
 #include "lldb/Symbol/ObjectFile.h"
 #include "lldb/Symbol/SymbolContext.h"
 #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/PDBSymbolExe.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolFunc.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolFuncDebugEnd.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolFuncDebugStart.h"
 #include "llvm/DebugInfo/PDB/PDBSymbolTypeEnum.h"

 }
 
 SymbolFilePDB::SymbolFilePDB(lldb_private::ObjectFile *object_file)
     : SymbolFile(object_file), m_cached_compile_unit_count(0) {}
 
 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();
     auto error = loadDataForEXE(PDB_ReaderType::DIA, llvm::StringRef(exePath),
                                 m_session_up);
     if (error) {
       llvm::consumeError(std::move(error));
+      if (auto module_sp = m_obj_file->GetModule()) {
+        // See if a symbol file was specified through the `--symfile` option.
+        FileSpec symfile = module_sp->GetSymbolFileFileSpec();
+        if (symfile) {
+          error = loadDataForPDB(PDB_ReaderType::DIA,
+                                 llvm::StringRef(symfile.GetPath()),
+                                 m_session_up);
+          if (error) {
+            llvm::consumeError(std::move(error));
+            return 0;
+          }
+        } else {
           return 0;
         }
+      } else {
+        return 0;
+      }
+    }
+  }
+  lldbassert(m_session_up.get());
+  if (auto enum_tables_up = m_session_up->getEnumTables()) {
+    while (auto table_up = enum_tables_up->getNext()) {
+      if (table_up->getItemCount()) {
+        auto type = table_up->getTableType();
+        switch (type) {
+        case PDB_TableType::SectionContribs:
+          // This table contains module/compiland information.
+          abilities |= CompileUnits;
+          break;
+        case PDB_TableType::Symbols:
+          abilities |= (Functions | Blocks);
+          // Check if there are sections of raw data for any variables
+          // in the `SECTIONHEADERS` data stream.
+          if (auto enum_dbg_streams_up = m_session_up->getDebugStreams()) {
+            while (auto dbg_stream_up = enum_dbg_streams_up->getNext()) {
+              auto stream_name = dbg_stream_up->getName();
+              if (stream_name == "SECTIONHEADERS") {
+                for (int rec_idx = 0;
+                     rec_idx < dbg_stream_up->getRecordCount();
+                     rec_idx++) {
+                  auto record = dbg_stream_up->getItemAtIndex(rec_idx);
+                  auto record_pointer =
+                      reinterpret_cast<char*>(record->data());
+                  if (strncmp(record_pointer, ".data", 5) == 0 ||
+                      strncmp(record_pointer, ".bss", 4) == 0 ||
+                      strncmp(record_pointer, ".rdata", 6) == 0) {
+                    abilities |= (GlobalVariables | LocalVariables |
+                                  VariableTypes);
+                    break;
+                  }
+                }
+              }
+            }
+          }
+          break;
+        case PDB_TableType::LineNumbers:
+          abilities |= LineTables;
+          break;
+        default:
+          break;
+        }
+      }
+    }
   }
-  return CompileUnits | LineTables;
+  return abilities;
 }
 
 void SymbolFilePDB::InitializeObject() {
   lldb::addr_t obj_load_address = m_obj_file->GetFileOffset();
   m_session_up->setLoadAddress(obj_load_address);
 
   TypeSystem *type_system =
       GetTypeSystemForLanguage(lldb::eLanguageTypeC_plus_plus);

unittests/SymbolFile/PDB/SymbolFilePDBTests.cpp

TEST_F(SymbolFilePDBTests, TestAbilitiesForPDB) {
   lldb::ModuleSP module = std::make_shared<Module>(fspec, aspec);
 
   SymbolVendor *plugin = module->GetSymbolVendor();
   EXPECT_NE(nullptr, plugin);
   SymbolFile *symfile = plugin->GetSymbolFile();
   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());
 }
 
 TEST_F(SymbolFilePDBTests, TestResolveSymbolContextBasename) {
   // Test that attempting to call ResolveSymbolContext with only a basename
   // finds all full paths
   // with the same basename
   FileSpec fspec(m_pdb_test_exe.c_str(), false);