diff --git a/lldb/include/lldb/Core/SourceManager.h b/lldb/include/lldb/Core/SourceManager.h --- a/lldb/include/lldb/Core/SourceManager.h +++ b/lldb/include/lldb/Core/SourceManager.h @@ -68,6 +68,9 @@ llvm::sys::TimePoint<> GetTimestamp() const { return m_mod_time; } protected: + /// Set file and update modification time. + void SetFileSpec(FileSpec file_spec); + bool CalculateLineOffsets(uint32_t line = UINT32_MAX); FileSpec m_file_spec_orig; // The original file spec that was used (can be @@ -101,7 +104,7 @@ SourceFileCache() = default; ~SourceFileCache() = default; - void AddSourceFile(const FileSP &file_sp); + void AddSourceFile(const FileSpec &file_spec, FileSP file_sp); FileSP FindSourceFile(const FileSpec &file_spec) const; // Removes all elements from the cache. @@ -109,7 +112,9 @@ void Dump(Stream &stream) const; - protected: + private: + void AddSourceFileImpl(const FileSpec &file_spec, FileSP file_sp); + typedef std::map FileCache; FileCache m_file_cache; }; diff --git a/lldb/include/lldb/Host/FileSystem.h b/lldb/include/lldb/Host/FileSystem.h --- a/lldb/include/lldb/Host/FileSystem.h +++ b/lldb/include/lldb/Host/FileSystem.h @@ -12,7 +12,9 @@ #include "lldb/Host/File.h" #include "lldb/Utility/DataBuffer.h" #include "lldb/Utility/FileSpec.h" +#include "lldb/Utility/LLDBAssert.h" #include "lldb/Utility/Status.h" +#include "lldb/Utility/TildeExpressionResolver.h" #include "llvm/Support/Chrono.h" #include "llvm/Support/VirtualFileSystem.h" @@ -30,17 +32,25 @@ static const char *DEV_NULL; static const char *PATH_CONVERSION_ERROR; - FileSystem() : m_fs(llvm::vfs::getRealFileSystem()) {} + FileSystem() + : m_fs(llvm::vfs::getRealFileSystem()), + m_tilde_resolver(std::make_unique()) {} FileSystem(llvm::IntrusiveRefCntPtr fs) - : m_fs(std::move(fs)) {} + : m_fs(std::move(fs)), + m_tilde_resolver(std::make_unique()) {} + FileSystem(std::unique_ptr tilde_resolver) + : m_fs(llvm::vfs::getRealFileSystem()), + m_tilde_resolver(std::move(tilde_resolver)) {} FileSystem(const FileSystem &fs) = delete; FileSystem &operator=(const FileSystem &fs) = delete; static FileSystem &Instance(); - static void Initialize(); - static void Initialize(llvm::IntrusiveRefCntPtr fs); + template static void Initialize(T &&...t) { + lldbassert(!InstanceImpl() && "Already initialized."); + InstanceImpl().emplace(std::forward(t)...); + } static void Terminate(); Status Symlink(const FileSpec &src, const FileSpec &dst); @@ -197,6 +207,7 @@ private: static std::optional &InstanceImpl(); llvm::IntrusiveRefCntPtr m_fs; + std::unique_ptr m_tilde_resolver; std::string m_home_directory; }; } // namespace lldb_private diff --git a/lldb/source/Core/SourceManager.cpp b/lldb/source/Core/SourceManager.cpp --- a/lldb/source/Core/SourceManager.cpp +++ b/lldb/source/Core/SourceManager.cpp @@ -75,13 +75,10 @@ if (!file_spec) return nullptr; - FileSpec resolved_fspec = file_spec; - resolve_tilde(resolved_fspec); - DebuggerSP debugger_sp(m_debugger_wp.lock()); FileSP file_sp; if (debugger_sp && debugger_sp->GetUseSourceCache()) - file_sp = debugger_sp->GetSourceFileCache().FindSourceFile(resolved_fspec); + file_sp = debugger_sp->GetSourceFileCache().FindSourceFile(file_spec); TargetSP target_sp(m_target_wp.lock()); @@ -99,12 +96,12 @@ // If file_sp is no good or it points to a non-existent file, reset it. if (!file_sp || !FileSystem::Instance().Exists(file_sp->GetFileSpec())) { if (target_sp) - file_sp = std::make_shared(resolved_fspec, target_sp.get()); + file_sp = std::make_shared(file_spec, target_sp.get()); else - file_sp = std::make_shared(resolved_fspec, debugger_sp); + file_sp = std::make_shared(file_spec, debugger_sp); if (debugger_sp && debugger_sp->GetUseSourceCache()) - debugger_sp->GetSourceFileCache().AddSourceFile(file_sp); + debugger_sp->GetSourceFileCache().AddSourceFile(file_spec, file_sp); } return file_sp; } @@ -393,15 +390,13 @@ SourceManager::File::File(const FileSpec &file_spec, lldb::DebuggerSP debugger_sp) - : m_file_spec_orig(file_spec), m_file_spec(file_spec), - m_mod_time(FileSystem::Instance().GetModificationTime(file_spec)), + : m_file_spec_orig(file_spec), m_file_spec(), m_mod_time(), m_debugger_wp(debugger_sp) { CommonInitializer(file_spec, nullptr); } SourceManager::File::File(const FileSpec &file_spec, Target *target) - : m_file_spec_orig(file_spec), m_file_spec(file_spec), - m_mod_time(FileSystem::Instance().GetModificationTime(file_spec)), + : m_file_spec_orig(file_spec), m_file_spec(), m_mod_time(), m_debugger_wp(target ? target->GetDebugger().shared_from_this() : DebuggerSP()) { CommonInitializer(file_spec, target); @@ -409,13 +404,16 @@ void SourceManager::File::CommonInitializer(const FileSpec &file_spec, Target *target) { + // Set the file and update the modification time. + SetFileSpec(file_spec); + + // File doesn't exist. if (m_mod_time == llvm::sys::TimePoint<>()) { if (target) { m_source_map_mod_id = target->GetSourcePathMap().GetModificationID(); + // If this is just a file name, try finding it in the target. if (!file_spec.GetDirectory() && file_spec.GetFilename()) { - // If this is just a file name, lets see if we can find it in the - // target: bool check_inlines = false; SymbolContextList sc_list; size_t num_matches = @@ -443,13 +441,12 @@ SymbolContext sc; sc_list.GetContextAtIndex(0, sc); if (sc.comp_unit) - m_file_spec = sc.comp_unit->GetPrimaryFile(); - m_mod_time = FileSystem::Instance().GetModificationTime(m_file_spec); + SetFileSpec(sc.comp_unit->GetPrimaryFile()); } } } - resolve_tilde(m_file_spec); - // Try remapping if m_file_spec does not correspond to an existing file. + + // Try remapping the file if it doesn't exist. if (!FileSystem::Instance().Exists(m_file_spec)) { // Check target specific source remappings (i.e., the // target.source-map setting), then fall back to the module @@ -460,18 +457,23 @@ if (target->GetImages().FindSourceFile(m_file_spec, new_spec)) remapped = new_spec; } - if (remapped) { - m_file_spec = *remapped; - m_mod_time = FileSystem::Instance().GetModificationTime(m_file_spec); - } + if (remapped) + SetFileSpec(*remapped); } } } + // If the file exists, read in the data. if (m_mod_time != llvm::sys::TimePoint<>()) m_data_sp = FileSystem::Instance().CreateDataBuffer(m_file_spec); } +void SourceManager::File::SetFileSpec(FileSpec file_spec) { + resolve_tilde(file_spec); + m_file_spec = std::move(file_spec); + m_mod_time = FileSystem::Instance().GetModificationTime(m_file_spec); +} + uint32_t SourceManager::File::GetLineOffset(uint32_t line) { if (line == 0) return UINT32_MAX; @@ -708,12 +710,22 @@ return true; } -void SourceManager::SourceFileCache::AddSourceFile(const FileSP &file_sp) { - FileSpec file_spec = file_sp->GetFileSpec(); +void SourceManager::SourceFileCache::AddSourceFile(const FileSpec &file_spec, + FileSP file_sp) { + assert(file_sp && "invalid FileSP"); + + AddSourceFileImpl(file_spec, file_sp); + const FileSpec &resolved_file_spec = file_sp->GetFileSpec(); + if (file_spec != resolved_file_spec) + AddSourceFileImpl(file_sp->GetFileSpec(), file_sp); +} + +void SourceManager::SourceFileCache::AddSourceFileImpl( + const FileSpec &file_spec, FileSP file_sp) { FileCache::iterator pos = m_file_cache.find(file_spec); - if (pos == m_file_cache.end()) + if (pos == m_file_cache.end()) { m_file_cache[file_spec] = file_sp; - else { + } else { if (file_sp != pos->second) m_file_cache[file_spec] = file_sp; } diff --git a/lldb/source/Host/common/FileSystem.cpp b/lldb/source/Host/common/FileSystem.cpp --- a/lldb/source/Host/common/FileSystem.cpp +++ b/lldb/source/Host/common/FileSystem.cpp @@ -9,8 +9,6 @@ #include "lldb/Host/FileSystem.h" #include "lldb/Utility/DataBufferLLVM.h" -#include "lldb/Utility/LLDBAssert.h" -#include "lldb/Utility/TildeExpressionResolver.h" #include "llvm/Support/Errc.h" #include "llvm/Support/Errno.h" @@ -46,16 +44,6 @@ FileSystem &FileSystem::Instance() { return *InstanceImpl(); } -void FileSystem::Initialize() { - lldbassert(!InstanceImpl() && "Already initialized."); - InstanceImpl().emplace(); -} - -void FileSystem::Initialize(IntrusiveRefCntPtr fs) { - lldbassert(!InstanceImpl() && "Already initialized."); - InstanceImpl().emplace(fs); -} - void FileSystem::Terminate() { lldbassert(InstanceImpl() && "Already terminated."); InstanceImpl().reset(); @@ -239,9 +227,9 @@ // Resolve tilde in path. SmallString<128> resolved(path.begin(), path.end()); - StandardTildeExpressionResolver Resolver; - Resolver.ResolveFullPath(llvm::StringRef(path.begin(), path.size()), - resolved); + assert(m_tilde_resolver && "must initialize tilde resolver in constructor"); + m_tilde_resolver->ResolveFullPath(llvm::StringRef(path.begin(), path.size()), + resolved); // Try making the path absolute if it exists. SmallString<128> absolute(resolved.begin(), resolved.end()); diff --git a/lldb/unittests/Core/SourceManagerTest.cpp b/lldb/unittests/Core/SourceManagerTest.cpp --- a/lldb/unittests/Core/SourceManagerTest.cpp +++ b/lldb/unittests/Core/SourceManagerTest.cpp @@ -10,12 +10,17 @@ #include "lldb/Host/FileSystem.h" #include "gtest/gtest.h" +#include "TestingSupport/MockTildeExpressionResolver.h" + using namespace lldb; using namespace lldb_private; class SourceFileCache : public ::testing::Test { public: - void SetUp() override { FileSystem::Initialize(); } + void SetUp() override { + FileSystem::Initialize(std::unique_ptr( + new MockTildeExpressionResolver("Jonas", "/jonas"))); + } void TearDown() override { FileSystem::Terminate(); } }; @@ -26,7 +31,7 @@ FileSpec foo_file_spec("foo"); auto foo_file_sp = std::make_shared(foo_file_spec, nullptr); - cache.AddSourceFile(foo_file_sp); + cache.AddSourceFile(foo_file_spec, foo_file_sp); // Query: foo, expect found. FileSpec another_foo_file_spec("foo"); @@ -40,9 +45,32 @@ FileSpec foo_file_spec("foo"); auto foo_file_sp = std::make_shared(foo_file_spec, nullptr); - cache.AddSourceFile(foo_file_sp); + cache.AddSourceFile(foo_file_spec, foo_file_sp); // Query: bar, expect not found. FileSpec bar_file_spec("bar"); ASSERT_EQ(cache.FindSourceFile(bar_file_spec), nullptr); } + +TEST_F(SourceFileCache, FindSourceFileByUnresolvedPath) { + SourceManager::SourceFileCache cache; + + FileSpec foo_file_spec("~/foo"); + + // Mimic the resolution in SourceManager::GetFile. + FileSpec resolved_foo_file_spec = foo_file_spec; + FileSystem::Instance().Resolve(resolved_foo_file_spec); + + // Create the file with the resolved file spec. + auto foo_file_sp = + std::make_shared(resolved_foo_file_spec, nullptr); + + // Cache the result with the unresolved file spec. + cache.AddSourceFile(foo_file_spec, foo_file_sp); + + // Query the unresolved path. + EXPECT_EQ(cache.FindSourceFile(FileSpec("~/foo")), foo_file_sp); + + // Query the resolved path. + EXPECT_EQ(cache.FindSourceFile(FileSpec("/jonas/foo")), foo_file_sp); +}