SymbolVendor: Move Symtab construction into the SymbolFile

[labath]

Summary

Instead of having SymbolVendor coordinate Symtab construction between
Symbol and Object files, make the SymbolVendor function a passthrough,
and put all of the logic into the SymbolFile.

[clayborg]

So the original idea behind the symbol vendor was that if you have more than one binary: stripped installed binary, and unstripped debug info binary with symbols, that the symbol table could be generated by using one or more object files. It would be nice to ensure we can still do this somehow.

[labath]

In D65208#1599648, @clayborg wrote:

So the original idea behind the symbol vendor was that if you have more than one binary: stripped installed binary, and unstripped debug info binary with symbols, that the symbol table could be generated by using one or more object files. It would be nice to ensure we can still do this somehow.

This functionality still exists/works (we even have tests for it :P), and I have no intention of removing it. The merging of sections from the split binaries happens in SymbolVendorELF::CreateInstance and ObjectFileMachO::CreateSections. This patch merely changes where the Symtab created from these merged sections ends up being stored.

include/lldb/Symbol/SymbolFile.h

public:
   /// prior to any other functions in the SymbolFile protocol.
   virtual void InitializeObject() {}
 
   // Compile Unit function calls
   // Approach 1 - iterator
   uint32_t GetNumCompileUnits();
   lldb::CompUnitSP GetCompileUnitAtIndex(uint32_t idx);
 
+  Symtab *GetSymtab();
+
   virtual lldb::LanguageType ParseLanguage(CompileUnit &comp_unit) = 0;
   virtual size_t ParseFunctions(CompileUnit &comp_unit) = 0;
   virtual bool ParseLineTable(CompileUnit &comp_unit) = 0;
   virtual bool ParseDebugMacros(CompileUnit &comp_unit) = 0;
   virtual bool ParseSupportFiles(CompileUnit &comp_unit,
                                  FileSpecList &support_files) = 0;
   virtual size_t ParseTypes(CompileUnit &comp_unit) = 0;
   virtual bool ParseIsOptimized(CompileUnit &comp_unit) { return false; }
protected:
   virtual lldb::CompUnitSP ParseCompileUnitAtIndex(uint32_t idx) = 0;
   virtual TypeList &GetTypeList() { return m_type_list; }
 
   void SetCompileUnitAtIndex(uint32_t idx, const lldb::CompUnitSP &cu_sp);
 
   ObjectFile *m_obj_file; // The object file that symbols can be extracted from.
   llvm::Optional<std::vector<lldb::CompUnitSP>> m_compile_units;
   TypeList m_type_list;
+  Symtab *m_symtab = nullptr;
   uint32_t m_abilities;
   bool m_calculated_abilities;
 
 private:
   DISALLOW_COPY_AND_ASSIGN(SymbolFile);
 };
 
 } // namespace lldb_private
 
 #endif // liblldb_SymbolFile_h_

include/lldb/Symbol/SymbolVendor.h

public:
 
   SymbolFile *GetSymbolFile() { return m_sym_file_up.get(); }
 
   FileSpec GetMainFileSpec() const;
 
   // Get module unified section list symbol table.
   virtual Symtab *GetSymtab();
 
-  // Clear module unified section list symbol table.
-  virtual void ClearSymtab();
-
   /// Notify the SymbolVendor that the file addresses in the Sections
   /// for this module have been changed.
   virtual void SectionFileAddressesChanged();
 
   // PluginInterface protocol
   ConstString GetPluginName() override;
 
   uint32_t GetPluginVersion() override;
 
 protected:
   lldb::ObjectFileSP m_objfile_sp; // Keep a reference to the object file in
                                    // case it isn't the same as the module
                                    // object file (debug symbols in a separate
                                    // file)
   std::unique_ptr<SymbolFile> m_sym_file_up; // A single symbol file. Subclasses
                                              // can add more of these if needed.
-  Symtab *m_symtab; // Save a symtab once to not pass it through `AddSymbols` of
-                    // the symbol file each time when it is needed
 
 private:
   // For SymbolVendor only
   DISALLOW_COPY_AND_ASSIGN(SymbolVendor);
 };
 
 } // namespace lldb_private
 
 #endif // liblldb_SymbolVendor_h_

source/Symbol/SymbolFile.cpp

void SymbolFile::SetCompileUnitAtIndex(uint32_t idx, const CompUnitSP &cu_sp) {
   // partial parsing should take care of only setting the compile unit
   // once, so if this assertion fails, we need to make sure that we don't
   // have a race condition, or have a second parse of the same compile
   // unit.
   assert((*m_compile_units)[idx] == nullptr);
   (*m_compile_units)[idx] = cu_sp;
 }
 
+Symtab *SymbolFile::GetSymtab() {
+  std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
+  if (m_symtab)
+    return m_symtab;
+
+  // Fetch the symtab from the main object file.
+  m_symtab = m_obj_file->GetModule()->GetObjectFile()->GetSymtab();
+
+  // Then add our symbols to it.
+  if (m_symtab)
+    AddSymbols(*m_symtab);
+
+  return m_symtab;
+}
+
 void SymbolFile::Dump(Stream &s) {
   s.PutCString("Types:\n");
   m_type_list.Dump(&s, /*show_context*/ false);
   s.PutChar('\n');
 
   s.PutCString("Compile units:\n");
   if (m_compile_units) {
     for (const CompUnitSP &cu_sp : *m_compile_units) {

source/Symbol/SymbolVendor.cpp

if (!sym_objfile_sp)
     sym_objfile_sp = module_sp->GetObjectFile()->shared_from_this();
   instance_up.reset(new SymbolVendor(module_sp));
   instance_up->AddSymbolFileRepresentation(sym_objfile_sp);
   return instance_up.release();
 }
 
 // SymbolVendor constructor
 SymbolVendor::SymbolVendor(const lldb::ModuleSP &module_sp)
-    : ModuleChild(module_sp), m_sym_file_up(), m_symtab() {}
+    : ModuleChild(module_sp), m_sym_file_up() {}
 
 // Destructor
 SymbolVendor::~SymbolVendor() {}
 
 // Add a representation given an object file.
 void SymbolVendor::AddSymbolFileRepresentation(const ObjectFileSP &objfile_sp) {
   ModuleSP module_sp(GetModule());
   if (module_sp) {
if (m_sym_file_up) {
     if (symfile_objfile)
       return symfile_objfile->GetFileSpec();
   }
 
   return FileSpec();
 }
 
 Symtab *SymbolVendor::GetSymtab() {
-  ModuleSP module_sp(GetModule());
-  if (!module_sp)
-    return nullptr;
-
-  std::lock_guard<std::recursive_mutex> guard(module_sp->GetMutex());
-
-  if (m_symtab)
-    return m_symtab;
-
-  ObjectFile *objfile = module_sp->GetObjectFile();
-  if (!objfile)
-    return nullptr;
-
-  m_symtab = objfile->GetSymtab();
-  if (m_symtab && m_sym_file_up)
-    m_sym_file_up->AddSymbols(*m_symtab);
-
-  return m_symtab;
-}
-
-void SymbolVendor::ClearSymtab() {
-  ModuleSP module_sp(GetModule());
-  if (module_sp) {
-    ObjectFile *objfile = module_sp->GetObjectFile();
-    if (objfile) {
-      // Clear symbol table from unified section list.
-      objfile->ClearSymtab();
-    }
-  }
+  if (m_sym_file_up)
+    return m_sym_file_up->GetSymtab();
 }
 
 void SymbolVendor::SectionFileAddressesChanged() {
   ModuleSP module_sp(GetModule());
   if (module_sp) {
     ObjectFile *module_objfile = module_sp->GetObjectFile();
     if (m_sym_file_up) {
       ObjectFile *symfile_objfile = m_sym_file_up->GetObjectFile();

unittests/Core/CMakeLists.txt

 add_lldb_unittest(LLDBCoreTests
   MangledTest.cpp
   RichManglingContextTest.cpp
   StreamCallbackTest.cpp
   UniqueCStringMapTest.cpp
 
   LINK_LIBS
     lldbCore
     lldbHost
     lldbSymbol
     lldbPluginObjectFileELF
-    lldbPluginSymbolVendorELF
+    lldbPluginSymbolFileSymtab
     lldbUtilityHelpers
     LLVMTestingSupport
   LINK_COMPONENTS
     Support
   )
 
 set(test_inputs
   mangled-function-names.yaml
   )
 add_unittest_inputs(LLDBCoreTests "${test_inputs}")

unittests/Core/MangledTest.cpp

 //===-- MangledTest.cpp -----------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
-#include "Plugins/SymbolVendor/ELF/SymbolVendorELF.h"
+#include "Plugins/SymbolFile/Symtab/SymbolFileSymtab.h"
 #include "TestingSupport/TestUtilities.h"
 
 #include "lldb/Core/Mangled.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Symbol/SymbolContext.h"
TEST(MangledTest, EmptyForInvalidName) {
 
   EXPECT_STREQ("", TheDemangled.GetCString());
 }
 
 TEST(MangledTest, NameIndexes_FindFunctionSymbols) {
   FileSystem::Initialize();
   HostInfo::Initialize();
   ObjectFileELF::Initialize();
-  SymbolVendorELF::Initialize();
+  SymbolFileSymtab::Initialize();
 
   llvm::SmallString<128> Obj;
   ASSERT_NO_ERROR(llvm::sys::fs::createTemporaryFile(
       "mangled-function-names-%%%%%%", "obj", Obj));
   llvm::FileRemover Deleter(Obj);
   ASSERT_THAT_ERROR(ReadYAMLObjectFile("mangled-function-names.yaml", Obj),
                     llvm::Succeeded());
 
TEST(MangledTest, NameIndexes_FindFunctionSymbols) {
 
   // Invalid things: unable to decode but still possible to find by full name
   EXPECT_EQ(1, Count("_Z12undemangableEvx42", eFunctionNameTypeFull));
   EXPECT_EQ(1, Count("_Z12undemangableEvx42", eFunctionNameTypeBase));
   EXPECT_EQ(0, Count("_Z12undemangableEvx42", eFunctionNameTypeMethod));
   EXPECT_EQ(0, Count("undemangable", eFunctionNameTypeBase));
   EXPECT_EQ(0, Count("undemangable", eFunctionNameTypeMethod));
 
-  SymbolVendorELF::Terminate();
+  SymbolFileSymtab::Terminate();
   ObjectFileELF::Terminate();
   HostInfo::Terminate();
   FileSystem::Terminate();
 }

unittests/ObjectFile/ELF/CMakeLists.txt

 add_lldb_unittest(ObjectFileELFTests
   TestObjectFileELF.cpp
 
   LINK_LIBS
     lldbPluginObjectFileELF
-    lldbPluginSymbolVendorELF
+    lldbPluginSymbolFileSymtab
     lldbCore
     lldbUtilityHelpers
     LLVMTestingSupport
   )
 
 set(test_inputs
   early-section-headers.so
   debug-info-relocations.pcm.yaml
   sections-resolve-consistently.yaml
   )
 add_unittest_inputs(ObjectFileELFTests "${test_inputs}")

unittests/ObjectFile/ELF/TestObjectFileELF.cpp

 //===-- TestObjectFileELF.cpp -----------------------------------*- C++ -*-===//
 //
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
-#include "Plugins/SymbolVendor/ELF/SymbolVendorELF.h"
+#include "Plugins/SymbolFile/Symtab/SymbolFileSymtab.h"
 #include "TestingSupport/TestUtilities.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Utility/DataBufferHeap.h"
 #include "llvm/ADT/Optional.h"

 using namespace lldb;
 
 class ObjectFileELFTest : public testing::Test {
 public:
   void SetUp() override {
     FileSystem::Initialize();
     HostInfo::Initialize();
     ObjectFileELF::Initialize();
-    SymbolVendorELF::Initialize();
+    SymbolFileSymtab::Initialize();
   }
 
   void TearDown() override {
-    SymbolVendorELF::Terminate();
+    SymbolFileSymtab::Terminate();
     ObjectFileELF::Terminate();
     HostInfo::Terminate();
     FileSystem::Terminate();
   }
 
 protected:
 };
 

unittests/Symbol/CMakeLists.txt

 add_lldb_unittest(SymbolTests
   LocateSymbolFileTest.cpp
   PostfixExpressionTest.cpp
   TestClangASTContext.cpp
   TestDWARFCallFrameInfo.cpp
   TestType.cpp
   TestLineEntry.cpp
 
   LINK_LIBS
     lldbHost
     lldbSymbol
     lldbUtilityHelpers
     lldbPluginObjectFileELF
     lldbPluginObjectFileMachO
     lldbPluginSymbolFileDWARF
+    lldbPluginSymbolFileSymtab
     LLVMTestingSupport
   )
 
 set(test_inputs
   basic-call-frame-info.yaml
   inlined-functions.yaml
   )
 add_unittest_inputs(SymbolTests "${test_inputs}")

unittests/Symbol/TestDWARFCallFrameInfo.cpp

 //===-- TestDWARFCallFrameInfo.cpp ------------------------------*- C++ -*-===//
 //
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "gtest/gtest.h"
 
 #include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
 #include "Plugins/Process/Utility/RegisterContext_x86.h"
+#include "Plugins/SymbolFile/Symtab/SymbolFileSymtab.h"
 #include "TestingSupport/TestUtilities.h"
 
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Core/Section.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Symbol/DWARFCallFrameInfo.h"

 using namespace lldb;
 
 class DWARFCallFrameInfoTest : public testing::Test {
 public:
   void SetUp() override {
     FileSystem::Initialize();
     HostInfo::Initialize();
     ObjectFileELF::Initialize();
+    SymbolFileSymtab::Initialize();
   }
 
   void TearDown() override {
+    SymbolFileSymtab::Terminate();
     ObjectFileELF::Terminate();
     HostInfo::Terminate();
     FileSystem::Terminate();
   }
 
 protected:
   void TestBasic(DWARFCallFrameInfo::Type type, llvm::StringRef symbol);
 };

unittests/Target/CMakeLists.txt

 add_lldb_unittest(TargetTests
   MemoryRegionInfoTest.cpp
   ModuleCacheTest.cpp
   PathMappingListTest.cpp
   ProcessInstanceInfoTest.cpp
 
   LINK_LIBS
       lldbCore
       lldbHost
       lldbSymbol
       lldbUtility
       lldbPluginObjectFileELF
+      lldbPluginSymbolFileSymtab
       lldbUtilityHelpers
     LINK_COMPONENTS
       Support
   )
 
 add_unittest_inputs(TargetTests TestModule.so)

unittests/Target/ModuleCacheTest.cpp

 #include "gtest/gtest.h"
 
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 
 #include "Plugins/ObjectFile/ELF/ObjectFileELF.h"
+#include "Plugins/SymbolFile/Symtab/SymbolFileSymtab.h"
 #include "TestingSupport/TestUtilities.h"
 #include "lldb/Core/Module.h"
 #include "lldb/Core/ModuleSpec.h"
 #include "lldb/Host/FileSystem.h"
 #include "lldb/Host/HostInfo.h"
 #include "lldb/Symbol/SymbolContext.h"
 #include "lldb/Target/ModuleCache.h"
 
static FileSpec GetSysrootView(FileSpec spec, const char *hostname) {
   spec.AppendPathComponent(module_name);
   return spec;
 }
 
 void ModuleCacheTest::SetUpTestCase() {
   FileSystem::Initialize();
   HostInfo::Initialize();
   ObjectFileELF::Initialize();
+  SymbolFileSymtab::Initialize();
 
   s_cache_dir = HostInfo::GetProcessTempDir();
   s_test_executable = GetInputFilePath(module_name);
 }
 
 void ModuleCacheTest::TearDownTestCase() {
+  SymbolFileSymtab::Terminate();
   ObjectFileELF::Terminate();
   HostInfo::Terminate();
   FileSystem::Terminate();
 }
 
 static void VerifyDiskState(const FileSpec &cache_dir, const char *hostname) {
   FileSpec uuid_view = GetUuidView(cache_dir);
   EXPECT_TRUE(FileSystem::Instance().Exists(uuid_view))