labath created this revision. labath added reviewers: JDevlieghere, clayborg, aprantl. Herald added a subscriber: jdoerfert.
This patch refactors the CompileUnit class to store the support files in a shared_ptr, and changes the SymbolFiles to hand them out as such. This allows the file lists to be shared between compile units, or between other objects. The motivation for this is to enable SymbolFileDWARF to keep a handle on the file lists it parses, so that it can use it when processing type units (which will not be handed out as lldb_private::CompileUnits). This will be implemented in follow-up patch(es). Since I was already changing the signature of this function, I made it return an Expected<shared_ptr> and changed all the places that were silently doing nothing to return an error. The CompileUnit class still returns an empty file list in case the parsing failed, but it first makes sure any errors get reported. https://reviews.llvm.org/D62649 Files: include/lldb/Symbol/CompileUnit.h include/lldb/Symbol/SymbolFile.h include/lldb/Symbol/SymbolVendor.h source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp source/Plugins/SymbolFile/PDB/SymbolFilePDB.h source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h source/Symbol/CompileUnit.cpp source/Symbol/SymbolVendor.cpp
Index: source/Symbol/SymbolVendor.cpp =================================================================== --- source/Symbol/SymbolVendor.cpp +++ source/Symbol/SymbolVendor.cpp @@ -155,15 +155,19 @@ } return false; } -bool SymbolVendor::ParseSupportFiles(CompileUnit &comp_unit, - FileSpecList &support_files) { + +llvm::Expected<std::shared_ptr<const FileSpecList>> +SymbolVendor::ParseSupportFiles(CompileUnit &comp_unit) { ModuleSP module_sp(GetModule()); if (module_sp) { std::lock_guard<std::recursive_mutex> guard(module_sp->GetMutex()); if (m_sym_file_up) - return m_sym_file_up->ParseSupportFiles(comp_unit, support_files); + return m_sym_file_up->ParseSupportFiles(comp_unit); + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "No symbol file."); } - return false; + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "No module file."); } bool SymbolVendor::ParseIsOptimized(CompileUnit &comp_unit) { Index: source/Symbol/CompileUnit.cpp =================================================================== --- source/Symbol/CompileUnit.cpp +++ source/Symbol/CompileUnit.cpp @@ -22,8 +22,7 @@ lldb_private::LazyBool is_optimized) : ModuleChild(module_sp), FileSpec(pathname), UserID(cu_sym_id), m_user_data(user_data), m_language(language), m_flags(0), - m_support_files(), m_line_table_up(), m_variables(), - m_is_optimized(is_optimized) { + m_line_table_up(), m_variables(), m_is_optimized(is_optimized) { if (language != eLanguageTypeUnknown) m_flags.Set(flagsParsedLanguage); assert(module_sp); @@ -35,8 +34,7 @@ lldb_private::LazyBool is_optimized) : ModuleChild(module_sp), FileSpec(fspec), UserID(cu_sym_id), m_user_data(user_data), m_language(language), m_flags(0), - m_support_files(), m_line_table_up(), m_variables(), - m_is_optimized(is_optimized) { + m_line_table_up(), m_variables(), m_is_optimized(is_optimized) { if (language != eLanguageTypeUnknown) m_flags.Set(flagsParsedLanguage); assert(module_sp); @@ -398,16 +396,24 @@ } const FileSpecList &CompileUnit::GetSupportFiles() { - if (m_support_files.GetSize() == 0) { - if (m_flags.IsClear(flagsParsedSupportFiles)) { - m_flags.Set(flagsParsedSupportFiles); - SymbolVendor *symbol_vendor = GetModule()->GetSymbolVendor(); - if (symbol_vendor) { - symbol_vendor->ParseSupportFiles(*this, m_support_files); - } + if (!m_support_files_sp) { + SymbolVendor *symbol_vendor = GetModule()->GetSymbolVendor(); + llvm::Expected<std::shared_ptr<const FileSpecList>> expected_list( + symbol_vendor ? symbol_vendor->ParseSupportFiles(*this) + : llvm::createStringError(llvm::inconvertibleErrorCode(), + "No symbol vendor.")); + + if (expected_list) { + assert(*expected_list); + m_support_files_sp = *expected_list; + } else { + GetModule()->ReportError( + "Parsing support files for compile unit `%s` failed: %s", + GetCString(), toString(expected_list.takeError()).c_str()); + m_support_files_sp = std::make_shared<FileSpecList>(); } } - return m_support_files; + return *m_support_files_sp; } void *CompileUnit::GetUserData() const { return m_user_data; } Index: source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h =================================================================== --- source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h +++ source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.h @@ -50,8 +50,10 @@ bool ParseDebugMacros(lldb_private::CompileUnit &comp_unit) override; - bool ParseSupportFiles(lldb_private::CompileUnit &comp_unit, - lldb_private::FileSpecList &support_files) override; + llvm::Expected<std::shared_ptr<const lldb_private::FileSpecList>> + ParseSupportFiles(lldb_private::CompileUnit &comp_unit) override { + return std::make_shared<lldb_private::FileSpecList>(); + } size_t ParseTypes(lldb_private::CompileUnit &comp_unit) override; Index: source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp =================================================================== --- source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp +++ source/Plugins/SymbolFile/Symtab/SymbolFileSymtab.cpp @@ -213,11 +213,6 @@ return false; } -bool SymbolFileSymtab::ParseSupportFiles(CompileUnit &comp_unit, - FileSpecList &support_files) { - return false; -} - bool SymbolFileSymtab::ParseImportedModules( const SymbolContext &sc, std::vector<SourceModule> &imported_modules) { return false; Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.h =================================================================== --- source/Plugins/SymbolFile/PDB/SymbolFilePDB.h +++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.h @@ -61,8 +61,8 @@ bool ParseDebugMacros(lldb_private::CompileUnit &comp_unit) override; - bool ParseSupportFiles(lldb_private::CompileUnit &comp_unit, - lldb_private::FileSpecList &support_files) override; + llvm::Expected<std::shared_ptr<const lldb_private::FileSpecList>> + ParseSupportFiles(lldb_private::CompileUnit &comp_unit) override; size_t ParseTypes(lldb_private::CompileUnit &comp_unit) override; Index: source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp =================================================================== --- source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp +++ source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp @@ -343,8 +343,8 @@ return false; } -bool SymbolFilePDB::ParseSupportFiles( - CompileUnit &comp_unit, lldb_private::FileSpecList &support_files) { +llvm::Expected<std::shared_ptr<const FileSpecList>> +SymbolFilePDB::ParseSupportFiles(CompileUnit &comp_unit) { // In theory this is unnecessary work for us, because all of this information // is easily (and quickly) accessible from DebugInfoPDB, so caching it a @@ -353,21 +353,24 @@ // to cache this list. auto compiland_up = GetPDBCompilandByUID(comp_unit.GetID()); if (!compiland_up) - return false; + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "No compiland."); auto files = m_session_up->getSourceFilesForCompiland(*compiland_up); if (!files || files->getChildCount() == 0) - return false; + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "No source files in compiland."); + auto files_sp = std::make_shared<FileSpecList>(); while (auto file = files->getNext()) { FileSpec spec(file->getFileName(), FileSpec::Style::windows); - support_files.AppendIfUnique(spec); + files_sp->AppendIfUnique(spec); } // LLDB uses the DWARF-like file numeration (one based), // the zeroth file is the compile unit itself - support_files.Insert(0, comp_unit); + files_sp->Insert(0, comp_unit); - return true; + return files_sp; } bool SymbolFilePDB::ParseImportedModules( Index: source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h =================================================================== --- source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h +++ source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.h @@ -84,8 +84,9 @@ bool ParseDebugMacros(lldb_private::CompileUnit &comp_unit) override; - bool ParseSupportFiles(lldb_private::CompileUnit &comp_unit, - FileSpecList &support_files) override; + llvm::Expected<std::shared_ptr<const FileSpecList>> + ParseSupportFiles(lldb_private::CompileUnit &comp_unit) override; + size_t ParseTypes(lldb_private::CompileUnit &comp_unit) override; bool ParseImportedModules( Index: source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp =================================================================== --- source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp +++ source/Plugins/SymbolFile/NativePDB/SymbolFileNativePDB.cpp @@ -1128,19 +1128,20 @@ return false; } -bool SymbolFileNativePDB::ParseSupportFiles(CompileUnit &comp_unit, - FileSpecList &support_files) { +llvm::Expected<std::shared_ptr<const FileSpecList>> +SymbolFileNativePDB::ParseSupportFiles(CompileUnit &comp_unit) { PdbSymUid cu_id(comp_unit.GetID()); lldbassert(cu_id.kind() == PdbSymUidKind::Compiland); CompilandIndexItem *cci = m_index->compilands().GetCompiland(cu_id.asCompiland().modi); lldbassert(cci); + auto files_sp = std::make_shared<FileSpecList>(); for (llvm::StringRef f : cci->m_file_list) { FileSpec::Style style = f.startswith("/") ? FileSpec::Style::posix : FileSpec::Style::windows; FileSpec spec(f, style); - support_files.Append(spec); + files_sp->Append(spec); } llvm::SmallString<64> main_source_file = @@ -1149,8 +1150,8 @@ ? FileSpec::Style::posix : FileSpec::Style::windows; FileSpec spec(main_source_file, style); - support_files.Insert(0, spec); - return true; + files_sp->Insert(0, spec); + return files_sp; } bool SymbolFileNativePDB::ParseImportedModules( Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h =================================================================== --- source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h +++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.h @@ -56,8 +56,8 @@ bool ParseDebugMacros(lldb_private::CompileUnit &comp_unit) override; - bool ParseSupportFiles(lldb_private::CompileUnit &comp_unit, - lldb_private::FileSpecList &support_files) override; + llvm::Expected<std::shared_ptr<const lldb_private::FileSpecList>> + ParseSupportFiles(lldb_private::CompileUnit &comp_unit) override; bool ParseIsOptimized(lldb_private::CompileUnit &comp_unit) override; Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp =================================================================== --- source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp +++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp @@ -652,12 +652,12 @@ return false; } -bool SymbolFileDWARFDebugMap::ParseSupportFiles(CompileUnit &comp_unit, - FileSpecList &support_files) { - SymbolFileDWARF *oso_dwarf = GetSymbolFile(comp_unit); - if (oso_dwarf) - return oso_dwarf->ParseSupportFiles(comp_unit, support_files); - return false; +llvm::Expected<std::shared_ptr<const FileSpecList>> +SymbolFileDWARFDebugMap::ParseSupportFiles(CompileUnit &comp_unit) { + if (SymbolFileDWARF *oso_dwarf = GetSymbolFile(comp_unit)) + return oso_dwarf->ParseSupportFiles(comp_unit); + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "No OSO dwarf."); } bool SymbolFileDWARFDebugMap::ParseIsOptimized(CompileUnit &comp_unit) { Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h =================================================================== --- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h +++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h @@ -108,8 +108,8 @@ bool ParseDebugMacros(lldb_private::CompileUnit &comp_unit) override; - bool ParseSupportFiles(lldb_private::CompileUnit &comp_unit, - lldb_private::FileSpecList &support_files) override; + llvm::Expected<std::shared_ptr<const lldb_private::FileSpecList>> + ParseSupportFiles(lldb_private::CompileUnit &comp_unit) override; bool ParseIsOptimized(lldb_private::CompileUnit &comp_unit) override; Index: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp =================================================================== --- source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -772,8 +772,8 @@ return functions_added; } -bool SymbolFileDWARF::ParseSupportFiles(CompileUnit &comp_unit, - FileSpecList &support_files) { +llvm::Expected<std::shared_ptr<const FileSpecList>> +SymbolFileDWARF::ParseSupportFiles(CompileUnit &comp_unit) { ASSERT_MODULE_LOCK(this); DWARFUnit *dwarf_cu = GetDWARFCompileUnit(&comp_unit); if (dwarf_cu) { @@ -785,14 +785,17 @@ if (stmt_list != DW_INVALID_OFFSET) { // All file indexes in DWARF are one based and a file of index zero is // supposed to be the compile unit itself. - support_files.Append(comp_unit); - return DWARFDebugLine::ParseSupportFiles( + auto files_sp = std::make_shared<FileSpecList>(); + files_sp->Append(comp_unit); + DWARFDebugLine::ParseSupportFiles( comp_unit.GetModule(), m_context.getOrLoadLineData(), stmt_list, - support_files, dwarf_cu); + *files_sp, dwarf_cu); + return files_sp; } } } - return false; + return llvm::createStringError(llvm::inconvertibleErrorCode(), + "No DWARF Compile unit"); } bool SymbolFileDWARF::ParseIsOptimized(CompileUnit &comp_unit) { Index: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h =================================================================== --- source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h +++ source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.h @@ -60,8 +60,9 @@ bool ParseDebugMacros(CompileUnit &comp_unit) override { return false; } - bool ParseSupportFiles(CompileUnit &comp_unit, - FileSpecList &support_files) override; + llvm::Expected<std::shared_ptr<const FileSpecList>> + ParseSupportFiles(CompileUnit &comp_unit) override; + size_t ParseTypes(CompileUnit &cu) override { return 0; } bool ParseImportedModules( Index: source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp =================================================================== --- source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp +++ source/Plugins/SymbolFile/Breakpad/SymbolFileBreakpad.cpp @@ -237,14 +237,13 @@ return true; } -bool SymbolFileBreakpad::ParseSupportFiles(CompileUnit &comp_unit, - FileSpecList &support_files) { +llvm::Expected<std::shared_ptr<const FileSpecList>> +SymbolFileBreakpad::ParseSupportFiles(CompileUnit &comp_unit) { CompUnitData &data = m_cu_data->GetEntryRef(comp_unit.GetID()).data; if (!data.support_files) ParseLineTableAndSupportFiles(comp_unit, data); - support_files = std::move(*data.support_files); - return true; + return std::make_shared<FileSpecList>(std::move(*data.support_files)); } uint32_t Index: include/lldb/Symbol/SymbolVendor.h =================================================================== --- include/lldb/Symbol/SymbolVendor.h +++ include/lldb/Symbol/SymbolVendor.h @@ -50,8 +50,8 @@ virtual bool ParseDebugMacros(CompileUnit &comp_unit); - virtual bool ParseSupportFiles(CompileUnit &comp_unit, - FileSpecList &support_files); + virtual llvm::Expected<std::shared_ptr<const FileSpecList>> + ParseSupportFiles(CompileUnit &comp_unit); virtual bool ParseIsOptimized(CompileUnit &comp_unit); Index: include/lldb/Symbol/SymbolFile.h =================================================================== --- include/lldb/Symbol/SymbolFile.h +++ include/lldb/Symbol/SymbolFile.h @@ -117,8 +117,10 @@ 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 llvm::Expected<std::shared_ptr<const FileSpecList>> + ParseSupportFiles(CompileUnit &comp_unit) = 0; + virtual size_t ParseTypes(CompileUnit &comp_unit) = 0; virtual bool ParseIsOptimized(CompileUnit &comp_unit) { return false; } Index: include/lldb/Symbol/CompileUnit.h =================================================================== --- include/lldb/Symbol/CompileUnit.h +++ include/lldb/Symbol/CompileUnit.h @@ -386,7 +386,7 @@ std::vector<SourceModule> m_imported_modules; /// Files associated with this compile unit's line table and /// declarations. - FileSpecList m_support_files; + std::shared_ptr<const FileSpecList> m_support_files_sp; /// Line table that will get parsed on demand. std::unique_ptr<LineTable> m_line_table_up; /// Debug macros that will get parsed on demand. @@ -403,15 +403,13 @@ (1u << 0), ///< Have we already parsed all our functions flagsParsedVariables = (1u << 1), ///< Have we already parsed globals and statics? - flagsParsedSupportFiles = (1u << 2), ///< Have we already parsed the support - ///files for this compile unit? flagsParsedLineTable = - (1u << 3), ///< Have we parsed the line table already? - flagsParsedLanguage = (1u << 4), ///< Have we parsed the language already? + (1u << 2), ///< Have we parsed the line table already? + flagsParsedLanguage = (1u << 3), ///< Have we parsed the language already? flagsParsedImportedModules = - (1u << 5), ///< Have we parsed the imported modules already? + (1u << 4), ///< Have we parsed the imported modules already? flagsParsedDebugMacros = - (1u << 6) ///< Have we parsed the debug macros already? + (1u << 5) ///< Have we parsed the debug macros already? }; DISALLOW_COPY_AND_ASSIGN(CompileUnit);
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits