JDevlieghere created this revision. JDevlieghere added a reviewer: jasonmolenda. Herald added a subscriber: mgrang. JDevlieghere requested review of this revision.
When replaying a reproducer captured from a core file, we always use dsymForUUID for the kernel binary. When enabled, we also use it to find kexts. Since these files are already contained in the reproducer, there's no reason to call out to an external tool. If the tool returns a different result, e.g. because the dSYM got garbage collected, it will break reproducer replay. The SymbolFileProvider solves the issue by mapping UUIDs to module and symbol paths in the reproducer. https://reviews.llvm.org/D86389 Files: lldb/include/lldb/Utility/Reproducer.h lldb/source/Symbol/LocateSymbolFile.cpp lldb/source/Symbol/LocateSymbolFileMacOSX.cpp lldb/source/Utility/Reproducer.cpp
Index: lldb/source/Utility/Reproducer.cpp =================================================================== --- lldb/source/Utility/Reproducer.cpp +++ lldb/source/Utility/Reproducer.cpp @@ -8,6 +8,7 @@ #include "lldb/Utility/Reproducer.h" #include "lldb/Utility/LLDBAssert.h" +#include "lldb/Utility/UUID.h" #include "llvm/Support/FileSystem.h" #include "llvm/Support/Threading.h" @@ -301,6 +302,77 @@ m_collector->addDirectory(dir); } +LLVM_YAML_IS_SEQUENCE_VECTOR(lldb_private::repro::SymbolFileProvider::Entry) + +namespace llvm { +namespace yaml { +template <> +struct MappingTraits<lldb_private::repro::SymbolFileProvider::Entry> { + static void mapping(IO &io, + lldb_private::repro::SymbolFileProvider::Entry &entry) { + io.mapRequired("uuid", entry.uuid); + io.mapRequired("module-path", entry.module_path); + io.mapRequired("symbol-path", entry.symbol_path); + } +}; +} // namespace yaml +} // namespace llvm + +void SymbolFileProvider::AddSymbolFile(const UUID *uuid, + const FileSpec &module_file, + const FileSpec &symbol_file) { + if (!uuid || (!module_file && !symbol_file)) + return; + m_symbol_files.emplace_back(uuid->GetAsString(), module_file.GetPath(), + symbol_file.GetPath()); +} + +void SymbolFileProvider::Keep() { + FileSpec file = this->GetRoot().CopyByAppendingPathComponent(Info::file); + std::error_code ec; + llvm::raw_fd_ostream os(file.GetPath(), ec, llvm::sys::fs::OF_Text); + if (ec) + return; + + // Remove duplicates. + llvm::sort(m_symbol_files.begin(), m_symbol_files.end()); + m_symbol_files.erase( + std::unique(m_symbol_files.begin(), m_symbol_files.end()), + m_symbol_files.end()); + + llvm::yaml::Output yout(os); + yout << m_symbol_files; +} + +SymbolFileLoader::SymbolFileLoader(Loader *loader) { + if (!loader) + return; + + FileSpec file = loader->GetFile<SymbolFileProvider::Info>(); + if (!file) + return; + + auto error_or_file = llvm::MemoryBuffer::getFile(file.GetPath()); + if (auto err = error_or_file.getError()) + return; + + llvm::yaml::Input yin((*error_or_file)->getBuffer()); + yin >> m_symbol_files; +} + +std::pair<FileSpec, FileSpec> +SymbolFileLoader::GetPaths(const UUID *uuid) const { + if (!uuid) + return {}; + + auto it = std::lower_bound(m_symbol_files.begin(), m_symbol_files.end(), + SymbolFileProvider::Entry(uuid->GetAsString())); + if (it == m_symbol_files.end()) + return {}; + return std::make_pair<FileSpec, FileSpec>(FileSpec(it->module_path), + FileSpec(it->symbol_path)); +} + void ProviderBase::anchor() {} char CommandProvider::ID = 0; char FileProvider::ID = 0; @@ -308,6 +380,7 @@ char VersionProvider::ID = 0; char WorkingDirectoryProvider::ID = 0; char HomeDirectoryProvider::ID = 0; +char SymbolFileProvider::ID = 0; const char *CommandProvider::Info::file = "command-interpreter.yaml"; const char *CommandProvider::Info::name = "command-interpreter"; const char *FileProvider::Info::file = "files.yaml"; @@ -318,3 +391,5 @@ const char *WorkingDirectoryProvider::Info::name = "cwd"; const char *HomeDirectoryProvider::Info::file = "home.txt"; const char *HomeDirectoryProvider::Info::name = "home"; +const char *SymbolFileProvider::Info::file = "symbol-files.yaml"; +const char *SymbolFileProvider::Info::name = "symbol-files"; Index: lldb/source/Symbol/LocateSymbolFileMacOSX.cpp =================================================================== --- lldb/source/Symbol/LocateSymbolFileMacOSX.cpp +++ lldb/source/Symbol/LocateSymbolFileMacOSX.cpp @@ -27,6 +27,7 @@ #include "lldb/Utility/DataExtractor.h" #include "lldb/Utility/Endian.h" #include "lldb/Utility/Log.h" +#include "lldb/Utility/Reproducer.h" #include "lldb/Utility/StreamString.h" #include "lldb/Utility/Timer.h" #include "lldb/Utility/UUID.h" @@ -53,6 +54,17 @@ return_module_spec.GetFileSpec().Clear(); return_module_spec.GetSymbolFileSpec().Clear(); + const UUID *uuid = module_spec.GetUUIDPtr(); + const ArchSpec *arch = module_spec.GetArchitecturePtr(); + + if (repro::Loader *l = repro::Reproducer::Instance().GetLoader()) { + static repro::SymbolFileLoader symbol_file_loader(l); + std::pair<FileSpec, FileSpec> paths = symbol_file_loader.GetPaths(uuid); + return_module_spec.GetFileSpec() = paths.first; + return_module_spec.GetSymbolFileSpec() = paths.second; + return 1; + } + int items_found = 0; if (g_dlsym_DBGCopyFullDSYMURLForUUID == nullptr || @@ -69,9 +81,6 @@ return items_found; } - const UUID *uuid = module_spec.GetUUIDPtr(); - const ArchSpec *arch = module_spec.GetArchitecturePtr(); - if (uuid && uuid->IsValid()) { // Try and locate the dSYM file using DebugSymbols first llvm::ArrayRef<uint8_t> module_uuid = uuid->GetBytes(); @@ -247,6 +256,12 @@ } } + if (repro::Generator *g = repro::Reproducer::Instance().GetGenerator()) { + g->GetOrCreate<repro::SymbolFileProvider>().AddSymbolFile( + uuid, return_module_spec.GetFileSpec(), + return_module_spec.GetSymbolFileSpec()); + } + return items_found; } @@ -464,6 +479,25 @@ const UUID *uuid_ptr = module_spec.GetUUIDPtr(); const FileSpec *file_spec_ptr = module_spec.GetFileSpecPtr(); + if (repro::Loader *l = repro::Reproducer::Instance().GetLoader()) { + static repro::SymbolFileLoader symbol_file_loader(l); + std::pair<FileSpec, FileSpec> paths = symbol_file_loader.GetPaths(uuid_ptr); + if (paths.first) + module_spec.GetFileSpec() = paths.first; + if (paths.second) + module_spec.GetSymbolFileSpec() = paths.second; + return true; + } + + // Lambda to capture the state of module_spec before returning from this + // function. + auto RecordResult = [&]() { + if (repro::Generator *g = repro::Reproducer::Instance().GetGenerator()) { + g->GetOrCreate<repro::SymbolFileProvider>().AddSymbolFile( + uuid_ptr, module_spec.GetFileSpec(), module_spec.GetSymbolFileSpec()); + } + }; + // It's expensive to check for the DBGShellCommands defaults setting, only do // it once per lldb run and cache the result. static bool g_have_checked_for_dbgshell_command = false; @@ -489,6 +523,7 @@ // When g_dbgshell_command is NULL, the user has not enabled the use of an // external program to find the symbols, don't run it for them. if (!force_lookup && g_dbgshell_command == NULL) { + RecordResult(); return false; } @@ -613,8 +648,10 @@ ::CFDictionaryGetKeysAndValues(plist.get(), NULL, (const void **)&values[0]); if (num_values == 1) { - return GetModuleSpecInfoFromUUIDDictionary(values[0], - module_spec); + success = GetModuleSpecInfoFromUUIDDictionary(values[0], + module_spec); + RecordResult(); + return success; } else { for (CFIndex i = 0; i < num_values; ++i) { ModuleSpec curr_module_spec; @@ -623,6 +660,7 @@ if (module_spec.GetArchitecture().IsCompatibleMatch( curr_module_spec.GetArchitecture())) { module_spec = curr_module_spec; + RecordResult(); return true; } } @@ -644,5 +682,6 @@ } } } + RecordResult(); return success; } Index: lldb/source/Symbol/LocateSymbolFile.cpp =================================================================== --- lldb/source/Symbol/LocateSymbolFile.cpp +++ lldb/source/Symbol/LocateSymbolFile.cpp @@ -16,6 +16,7 @@ #include "lldb/Utility/DataBuffer.h" #include "lldb/Utility/DataExtractor.h" #include "lldb/Utility/Log.h" +#include "lldb/Utility/Reproducer.h" #include "lldb/Utility/StreamString.h" #include "lldb/Utility/Timer.h" #include "lldb/Utility/UUID.h" @@ -225,6 +226,7 @@ } else { dsym_module_spec.GetSymbolFileSpec() = symbol_fspec; } + return dsym_module_spec.GetSymbolFileSpec(); } @@ -248,6 +250,7 @@ } else { LocateMacOSXFilesUsingDebugSymbols(module_spec, result); } + return result; } Index: lldb/include/lldb/Utility/Reproducer.h =================================================================== --- lldb/include/lldb/Utility/Reproducer.h +++ lldb/include/lldb/Utility/Reproducer.h @@ -22,6 +22,7 @@ #include <vector> namespace lldb_private { +class UUID; namespace repro { class Reproducer; @@ -200,6 +201,40 @@ static char ID; }; +class SymbolFileProvider : public Provider<SymbolFileProvider> { +public: + SymbolFileProvider(const FileSpec &directory) + : Provider(directory), m_symbol_files() {} + + void AddSymbolFile(const UUID *uuid, const FileSpec &module_path, + const FileSpec &symbol_path); + void Keep() override; + + struct Entry { + Entry() = default; + Entry(std::string uuid) : uuid(std::move(uuid)) {} + Entry(std::string uuid, std::string module_path, std::string symbol_path) + : uuid(std::move(uuid)), module_path(std::move(module_path)), + symbol_path(std::move(symbol_path)) {} + + bool operator==(const Entry &rhs) const { return uuid == rhs.uuid; } + bool operator<(const Entry &rhs) const { return uuid < rhs.uuid; } + + std::string uuid; + std::string module_path; + std::string symbol_path; + }; + + struct Info { + static const char *name; + static const char *file; + }; + static char ID; + +private: + std::vector<Entry> m_symbol_files; +}; + /// The recorder is a small object handed out by a provider to record data. It /// is commonly used in combination with a MultiProvider which is meant to /// record information for multiple instances of the same source of data. @@ -514,6 +549,16 @@ unsigned m_index = 0; }; +class SymbolFileLoader { +public: + SymbolFileLoader(Loader *loader); + std::pair<FileSpec, FileSpec> GetPaths(const UUID *uuid) const; + +private: + // Sorted list of UUID to path mappings. + std::vector<SymbolFileProvider::Entry> m_symbol_files; +}; + /// Helper to read directories written by the DirectoryProvider. template <typename T> llvm::Expected<std::string> GetDirectoryFrom(repro::Loader *loader) {
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits