llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-lldb Author: None (royitaqi) <details> <summary>Changes</summary> # The change Currently, `DebuggerStats::ReportStatistics()` calls `Module::GetSymtab(/* can_create */ false)`, but then the latter calls `SymbolFile::GetSymtab()`. This will load symbols if haven't yet. The problem is that `DebuggerStats::ReportStatistics` should be read-only. This is especially important because it reports stats for symtab parsing/indexing time, which could be affected by the reporting itself if it's not read-only. This patch fixes this problem by adding an optional parameter `SymbolFile::GetSymtab(can_create = true)` and receive the `false` value passed down from `Module::GetSymtab()` when the call was initiated from `DebuggerStats::ReportStatistics()`. # Tests **Manually tested** with a helloworld program, that: 1. Without this patch, a breakpoint in `ObjectFileMachO::ParseSymtab()` is hit when we do `lldb a.out` then `statistics dump`. 2. With this patch, said breakpoint is not hit. **Unit test**: Still looking for a good unit test file to add a new test. Will update here. --- Full diff: https://github.com/llvm/llvm-project/pull/136236.diff 6 Files Affected: - (modified) lldb/include/lldb/Symbol/ObjectFile.h (+1-1) - (modified) lldb/include/lldb/Symbol/SymbolFile.h (+2-2) - (modified) lldb/include/lldb/Symbol/SymbolFileOnDemand.h (+1-1) - (modified) lldb/source/Core/Module.cpp (+1-1) - (modified) lldb/source/Symbol/ObjectFile.cpp (+2-2) - (modified) lldb/source/Symbol/SymbolFile.cpp (+2-2) ``````````diff diff --git a/lldb/include/lldb/Symbol/ObjectFile.h b/lldb/include/lldb/Symbol/ObjectFile.h index cfcca04a76de8..7fca6383fa9f3 100644 --- a/lldb/include/lldb/Symbol/ObjectFile.h +++ b/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. /// diff --git a/lldb/include/lldb/Symbol/SymbolFile.h b/lldb/include/lldb/Symbol/SymbolFile.h index f35d3ee9f22ae..df2f263b18e17 100644 --- a/lldb/include/lldb/Symbol/SymbolFile.h +++ b/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 { diff --git a/lldb/include/lldb/Symbol/SymbolFileOnDemand.h b/lldb/include/lldb/Symbol/SymbolFileOnDemand.h index 7a366bfabec86..ce9fc8626ac34 100644 --- a/lldb/include/lldb/Symbol/SymbolFileOnDemand.h +++ b/lldb/include/lldb/Symbol/SymbolFileOnDemand.h @@ -186,7 +186,7 @@ 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(); diff --git a/lldb/source/Core/Module.cpp b/lldb/source/Core/Module.cpp index ad7046e596278..625c14e4a2153 100644 --- a/lldb/source/Core/Module.cpp +++ b/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; } diff --git a/lldb/source/Symbol/ObjectFile.cpp b/lldb/source/Symbol/ObjectFile.cpp index 2f2c59d6af620..292c00b9ac166 100644 --- a/lldb/source/Symbol/ObjectFile.cpp +++ b/lldb/source/Symbol/ObjectFile.cpp @@ -735,9 +735,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 diff --git a/lldb/source/Symbol/SymbolFile.cpp b/lldb/source/Symbol/SymbolFile.cpp index 94e32b55572dd..870d778dca740 100644 --- a/lldb/source/Symbol/SymbolFile.cpp +++ b/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; `````````` </details> https://github.com/llvm/llvm-project/pull/136236 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
