[lldb] Remerge #136236 (Avoid force loading symbols in statistics collection

lldb/include/lldb/Symbol/ObjectFile.h

@@ -319,7 +319,7 @@ class ObjectFile : public std::enable_shared_from_this<ObjectFile>,
   ///
   /// \return
   ///     The symbol table for this object file.
-  Symtab *GetSymtab();
+  Symtab *GetSymtab(bool can_create = true);
 
   /// Parse the symbol table into the provides symbol table object.
   ///

lldb/include/lldb/Symbol/SymbolFile.h

@@ -144,7 +144,7 @@ class SymbolFile : public PluginInterface {
   virtual uint32_t GetNumCompileUnits() = 0;
   virtual lldb::CompUnitSP GetCompileUnitAtIndex(uint32_t idx) = 0;
 
-  virtual Symtab *GetSymtab() = 0;
+  virtual Symtab *GetSymtab(bool can_create = true) = 0;
 
   virtual lldb::LanguageType ParseLanguage(CompileUnit &comp_unit) = 0;
   /// Return the Xcode SDK comp_unit was compiled against.
@@ -533,7 +533,7 @@ class SymbolFileCommon : public SymbolFile {
     return m_abilities;
   }
 
-  Symtab *GetSymtab() override;
+  Symtab *GetSymtab(bool can_create = true) override;
 
   ObjectFile *GetObjectFile() override { return m_objfile_sp.get(); }
   const ObjectFile *GetObjectFile() const override {

lldb/include/lldb/Symbol/SymbolFileOnDemand.h

@@ -186,7 +186,9 @@ class SymbolFileOnDemand : public lldb_private::SymbolFile {
 
   uint32_t GetAbilities() override;
 
-  Symtab *GetSymtab() override { return m_sym_file_impl->GetSymtab(); }
+  Symtab *GetSymtab(bool can_create = true) override {
+    return m_sym_file_impl->GetSymtab(can_create);
+  }
 
   ObjectFile *GetObjectFile() override {
     return m_sym_file_impl->GetObjectFile();

lldb/include/lldb/Target/Statistics.h

@@ -120,7 +120,7 @@ struct ModuleStats {
   llvm::StringMap<llvm::json::Value> type_system_stats;
   double symtab_parse_time = 0.0;
   double symtab_index_time = 0.0;
-  uint32_t num_symbols_loaded = 0;
+  uint32_t symtab_symbol_count = 0;
   double debug_parse_time = 0.0;
   double debug_index_time = 0.0;
   uint64_t debug_info_size = 0;

lldb/source/Core/Module.cpp

@@ -997,7 +997,7 @@ SymbolFile *Module::GetSymbolFile(bool can_create, Stream *feedback_strm) {
 
 Symtab *Module::GetSymtab(bool can_create) {
   if (SymbolFile *symbols = GetSymbolFile(can_create))
-    return symbols->GetSymtab();
+    return symbols->GetSymtab(can_create);
   return nullptr;
 }
 

lldb/source/Symbol/ObjectFile.cpp

@@ -734,10 +734,9 @@ void llvm::format_provider<ObjectFile::Strata>::format(
   }
 }
 
-
-Symtab *ObjectFile::GetSymtab() {
+Symtab *ObjectFile::GetSymtab(bool can_create) {
   ModuleSP module_sp(GetModule());
-  if (module_sp) {
+  if (module_sp && can_create) {
     // We can't take the module lock in ObjectFile::GetSymtab() or we can
     // deadlock in DWARF indexing when any file asks for the symbol table from
     // an object file. This currently happens in the preloading of symbols in

lldb/source/Symbol/SymbolFile.cpp

@@ -152,10 +152,10 @@ void SymbolFile::AssertModuleLock() {
 
 SymbolFile::RegisterInfoResolver::~RegisterInfoResolver() = default;
 
-Symtab *SymbolFileCommon::GetSymtab() {
+Symtab *SymbolFileCommon::GetSymtab(bool can_create) {
   std::lock_guard<std::recursive_mutex> guard(GetModuleMutex());
   // Fetch the symtab from the main object file.
-  auto *symtab = GetMainObjectFile()->GetSymtab();
+  auto *symtab = GetMainObjectFile()->GetSymtab(can_create);
   if (m_symtab != symtab) {
     m_symtab = symtab;
 

lldb/source/Target/Statistics.cpp

@@ -71,7 +71,7 @@ json::Value ModuleStats::ToJSON() const {
   module.try_emplace("debugInfoHadIncompleteTypes",
                      debug_info_had_incomplete_types);
   module.try_emplace("symbolTableStripped", symtab_stripped);
-  module.try_emplace("symbolsLoaded", num_symbols_loaded);
+  module.try_emplace("symbolTableSymbolCount", symtab_symbol_count);
   if (!symfile_path.empty())
     module.try_emplace("symbolFilePath", symfile_path);
 
@@ -311,7 +311,7 @@ llvm::json::Value DebuggerStats::ReportStatistics(
   uint32_t num_modules_with_variable_errors = 0;
   uint32_t num_modules_with_incomplete_types = 0;
   uint32_t num_stripped_modules = 0;
-  uint32_t num_symbols_loaded = 0;
+  uint32_t symtab_symbol_count = 0;
   for (size_t image_idx = 0; image_idx < num_modules; ++image_idx) {
     Module *module = target != nullptr
                          ? target->GetImages().GetModuleAtIndex(image_idx).get()
@@ -321,8 +321,8 @@ llvm::json::Value DebuggerStats::ReportStatistics(
     module_stat.symtab_index_time = module->GetSymtabIndexTime().get().count();
     Symtab *symtab = module->GetSymtab(/*can_create=*/false);
     if (symtab) {
-      module_stat.num_symbols_loaded = symtab->GetNumSymbols();
-      num_symbols_loaded += module_stat.num_symbols_loaded;
+      module_stat.symtab_symbol_count = symtab->GetNumSymbols();
+      symtab_symbol_count += module_stat.symtab_symbol_count;
       ++symtabs_loaded;
       module_stat.symtab_loaded_from_cache = symtab->GetWasLoadedFromCache();
       if (module_stat.symtab_loaded_from_cache)
@@ -414,7 +414,7 @@ llvm::json::Value DebuggerStats::ReportStatistics(
        num_modules_with_incomplete_types},
       {"totalDebugInfoEnabled", num_debug_info_enabled_modules},
       {"totalSymbolTableStripped", num_stripped_modules},
-      {"totalSymbolsLoaded", num_symbols_loaded},
+      {"totalSymbolTableSymbolCount", symtab_symbol_count},
   };
 
   if (include_targets) {

lldb/test/API/commands/statistics/basic/TestStats.py

@@ -170,7 +170,7 @@ def test_default_no_run(self):
             "totalSymbolTableIndexTime",
             "totalSymbolTablesLoadedFromCache",
             "totalSymbolTablesSavedToCache",
-            "totalSymbolsLoaded",
+            "totalSymbolTableSymbolCount",
             "totalSymbolTablesLoaded",
             "totalDebugInfoByteSize",
             "totalDebugInfoIndexTime",
@@ -180,7 +180,7 @@ def test_default_no_run(self):
         ]
         self.verify_keys(debug_stats, '"debug_stats"', debug_stat_keys, None)
         if self.getPlatform() != "windows":
-            self.assertGreater(debug_stats["totalSymbolsLoaded"], 0)
+            self.assertGreater(debug_stats["totalSymbolTableSymbolCount"], 0)
             self.assertGreater(debug_stats["totalSymbolTablesLoaded"], 0)
 
         # Verify target stats keys.
@@ -203,13 +203,34 @@ def test_default_no_run(self):
         # Verify module stats keys.
         for module_stats in debug_stats["modules"]:
             module_stat_keys_exist = [
-                "symbolsLoaded",
+                "symbolTableSymbolCount",
             ]
             self.verify_keys(
                 module_stats, '"module_stats"', module_stat_keys_exist, None
             )
             if self.getPlatform() != "windows":
-                self.assertGreater(module_stats["symbolsLoaded"], 0)
+                self.assertGreater(module_stats["symbolTableSymbolCount"], 0)
+
+    def test_default_no_run_no_preload_symbols(self):
+        """Test "statistics dump" without running the target and without
+        preloading symbols.
+
+        Checks that symbol count are zero.
+        """
+        # Make sure symbols will not be preloaded.
+        self.runCmd("settings set target.preload-symbols false")
+
+        # Build and load the target
+        self.build()
+        self.createTestTarget()
+
+        # Get statistics
+        debug_stats = self.get_stats()
+
+        # No symbols should be loaded in the main module.
+        main_module_stats = debug_stats["modules"][0]
+        if self.getPlatform() != "windows":
+            self.assertEqual(main_module_stats["symbolTableSymbolCount"], 0)
 
     def test_default_with_run(self):
         """Test "statistics dump" when running the target to a breakpoint.

lldb/unittests/Symbol/LineTableTest.cpp

@@ -58,7 +58,7 @@ class FakeSymbolFile : public SymbolFile {
   uint32_t CalculateAbilities() override { return UINT32_MAX; }
   uint32_t GetNumCompileUnits() override { return 1; }
   CompUnitSP GetCompileUnitAtIndex(uint32_t) override { return m_cu_sp; }
-  Symtab *GetSymtab() override { return nullptr; }
+  Symtab *GetSymtab(bool can_create = true) override { return nullptr; }
   LanguageType ParseLanguage(CompileUnit &) override { return eLanguageTypeC; }
   size_t ParseFunctions(CompileUnit &) override { return 0; }
   bool ParseLineTable(CompileUnit &) override { return true; }

lldb/unittests/Symbol/SymtabTest.cpp

@@ -721,7 +721,7 @@ TEST_F(SymtabTest, TestDecodeCStringMaps) {
   ASSERT_NE(symbol, nullptr);
 }
 
-TEST_F(SymtabTest, TestSymtabCreatedOnDemand) {
+TEST_F(SymtabTest, TestSymbolFileCreatedOnDemand) {
   auto ExpectedFile = TestFile::fromYaml(R"(
 --- !ELF
 FileHeader:
@@ -749,7 +749,7 @@ TEST_F(SymtabTest, TestSymtabCreatedOnDemand) {
   ASSERT_THAT_EXPECTED(ExpectedFile, llvm::Succeeded());
   auto module_sp = std::make_shared<Module>(ExpectedFile->moduleSpec());
 
-  // The symbol table should not be loaded by default.
+  // The symbol file should not be created by default.
   Symtab *module_symtab = module_sp->GetSymtab(/*can_create=*/false);
   ASSERT_EQ(module_symtab, nullptr);
 
@@ -761,3 +761,77 @@ TEST_F(SymtabTest, TestSymtabCreatedOnDemand) {
   Symtab *cached_module_symtab = module_sp->GetSymtab(/*can_create=*/false);
   ASSERT_EQ(module_symtab, cached_module_symtab);
 }
+
+TEST_F(SymtabTest, TestSymbolTableCreatedOnDemand) {
+  const char *yamldata = R"(
+--- !ELF
+FileHeader:
+  Class:   ELFCLASS64
+  Data:    ELFDATA2LSB
+  Type:    ET_EXEC
+  Machine: EM_386
+DWARF:
+  debug_abbrev:
+    - Table:
+        - Code:            0x00000001
+          Tag:             DW_TAG_compile_unit
+          Children:        DW_CHILDREN_no
+          Attributes:
+            - Attribute:       DW_AT_addr_base
+              Form:            DW_FORM_sec_offset
+  debug_info:
+    - Version:         5
+      AddrSize:        4
+      UnitType:        DW_UT_compile
+      Entries:
+        - AbbrCode:        0x00000001
+          Values:
+            - Value:           0x8 # Offset of the first Address past the header
+        - AbbrCode:        0x0
+  debug_addr:
+    - Version: 5
+      AddressSize: 4
+      Entries:
+        - Address: 0x1234
+        - Address: 0x5678
+  debug_line:
+    - Length:          42
+      Version:         2
+      PrologueLength:  36
+      MinInstLength:   1
+      DefaultIsStmt:   1
+      LineBase:        251
+      LineRange:       14
+      OpcodeBase:      13
+      StandardOpcodeLengths: [ 0, 1, 1, 1, 1, 0, 0, 0, 1, 0, 0, 1 ]
+      IncludeDirs:
+        - '/tmp'
+      Files:
+        - Name:            main.cpp
+          DirIdx:          1
+          ModTime:         0
+          Length:          0
+)";
+  llvm::Expected<TestFile> file = TestFile::fromYaml(yamldata);
+  EXPECT_THAT_EXPECTED(file, llvm::Succeeded());
+  auto module_sp = std::make_shared<Module>(file->moduleSpec());
+
+  SymbolFile *symbol_file = module_sp->GetSymbolFile();
+  // At this point, the symbol table is not created. This is because the above
+  // yaml data contains the necessary sections in order for
+  // SymbolFileDWARF::CalculateAbilities() to identify all abilities, saving it
+  // from calling SymbolFileDWARFDebugMap::CalculateAbilities(), which
+  // eventually loads the symbol table, which we don't want.
+
+  // The symbol table should not be created if asked not to.
+  Symtab *symtab = symbol_file->GetSymtab(/*can_create=*/false);
+  ASSERT_EQ(symtab, nullptr);
+
+  // But it should be created on demand.
+  symtab = symbol_file->GetSymtab(/*can_create=*/true);
+  ASSERT_NE(symtab, nullptr);
+
+  // And we should be able to get it again once it has been created.
+  Symtab *cached_symtab = symbol_file->GetSymtab(/*can_create=*/false);
+  ASSERT_EQ(symtab, cached_symtab);
+}