[Lldb-commits] [lldb] 8b3b66e - [lldb] Remove FileSystem::Initialize from FileCollector
Jonas Devlieghere via lldb-commits
lldb-commits at lists.llvm.org
Thu Mar 3 13:22:56 PST 2022
Author: Jonas Devlieghere
Date: 2022-03-03T13:22:38-08:00
New Revision: 8b3b66ea63d6469e5a3842627d538805173bda05
URL: https://github.com/llvm/llvm-project/commit/8b3b66ea63d6469e5a3842627d538805173bda05
DIFF: https://github.com/llvm/llvm-project/commit/8b3b66ea63d6469e5a3842627d538805173bda05.diff
LOG: [lldb] Remove FileSystem::Initialize from FileCollector
This patch removes the ability to instantiate the LLDB FileSystem class
with a FileCollector. It keeps the ability to collect files, but uses
the FileCollectorFileSystem to do that transparently.
Because the two are intertwined, this patch also removes the
finalization logic which copied the files over out of process.
Added:
Modified:
lldb/include/lldb/Host/FileSystem.h
lldb/include/lldb/Utility/Reproducer.h
lldb/include/lldb/Utility/ReproducerProvider.h
lldb/source/API/SBReproducer.cpp
lldb/source/Commands/CommandObjectReproducer.cpp
lldb/source/Host/common/FileSystem.cpp
lldb/source/Host/posix/FileSystemPosix.cpp
lldb/source/Initialization/SystemInitializerCommon.cpp
lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
lldb/source/Utility/Reproducer.cpp
lldb/source/Utility/ReproducerProvider.cpp
Removed:
################################################################################
diff --git a/lldb/include/lldb/Host/FileSystem.h b/lldb/include/lldb/Host/FileSystem.h
index 50c48648fecda..efa77cbbbcb96 100644
--- a/lldb/include/lldb/Host/FileSystem.h
+++ b/lldb/include/lldb/Host/FileSystem.h
@@ -16,7 +16,6 @@
#include "llvm/ADT/Optional.h"
#include "llvm/Support/Chrono.h"
-#include "llvm/Support/FileCollector.h"
#include "llvm/Support/VirtualFileSystem.h"
#include "lldb/lldb-types.h"
@@ -31,12 +30,9 @@ class FileSystem {
static const char *DEV_NULL;
static const char *PATH_CONVERSION_ERROR;
- FileSystem() : m_fs(llvm::vfs::getRealFileSystem()), m_collector(nullptr) {}
- FileSystem(std::shared_ptr<llvm::FileCollectorBase> collector)
- : m_fs(llvm::vfs::getRealFileSystem()),
- m_collector(std::move(collector)) {}
+ FileSystem() : m_fs(llvm::vfs::getRealFileSystem()) {}
FileSystem(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs)
- : m_fs(std::move(fs)), m_collector(nullptr) {}
+ : m_fs(std::move(fs)) {}
FileSystem(const FileSystem &fs) = delete;
FileSystem &operator=(const FileSystem &fs) = delete;
@@ -44,7 +40,6 @@ class FileSystem {
static FileSystem &Instance();
static void Initialize();
- static void Initialize(std::shared_ptr<llvm::FileCollectorBase> collector);
static void Initialize(llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> fs);
static void Terminate();
@@ -191,15 +186,11 @@ class FileSystem {
return m_fs;
}
- void Collect(const FileSpec &file_spec);
- void Collect(const llvm::Twine &file);
-
void SetHomeDirectory(std::string home_directory);
private:
static llvm::Optional<FileSystem> &InstanceImpl();
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> m_fs;
- std::shared_ptr<llvm::FileCollectorBase> m_collector;
std::string m_home_directory;
};
} // namespace lldb_private
diff --git a/lldb/include/lldb/Utility/Reproducer.h b/lldb/include/lldb/Utility/Reproducer.h
index 672f82fcb9565..e81abad2ec320 100644
--- a/lldb/include/lldb/Utility/Reproducer.h
+++ b/lldb/include/lldb/Utility/Reproducer.h
@@ -225,9 +225,6 @@ struct ReplayOptions {
bool check_version = true;
};
-llvm::Error Finalize(Loader *loader);
-llvm::Error Finalize(const FileSpec &root);
-
} // namespace repro
} // namespace lldb_private
diff --git a/lldb/include/lldb/Utility/ReproducerProvider.h b/lldb/include/lldb/Utility/ReproducerProvider.h
index 56aadf92369e2..72ddf64f7ebc1 100644
--- a/lldb/include/lldb/Utility/ReproducerProvider.h
+++ b/lldb/include/lldb/Utility/ReproducerProvider.h
@@ -91,23 +91,6 @@ class YamlRecorder : public AbstractRecorder {
}
};
-class FlushingFileCollector : public llvm::FileCollectorBase {
-public:
- FlushingFileCollector(llvm::StringRef files_path, llvm::StringRef dirs_path,
- std::error_code &ec);
-
-protected:
- void addFileImpl(llvm::StringRef file) override;
-
- llvm::vfs::directory_iterator
- addDirectoryImpl(const llvm::Twine &dir,
- llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> vfs,
- std::error_code &dir_ec) override;
-
- llvm::Optional<llvm::raw_fd_ostream> m_files_os;
- llvm::Optional<llvm::raw_fd_ostream> m_dirs_os;
-};
-
class FileProvider : public Provider<FileProvider> {
public:
struct Info {
@@ -116,25 +99,24 @@ class FileProvider : public Provider<FileProvider> {
};
FileProvider(const FileSpec &directory) : Provider(directory) {
- std::error_code ec;
- m_collector = std::make_shared<FlushingFileCollector>(
- directory.CopyByAppendingPathComponent("files.txt").GetPath(),
- directory.CopyByAppendingPathComponent("dirs.txt").GetPath(), ec);
- if (ec)
- m_collector.reset();
+ m_collector = std::make_shared<llvm::FileCollector>(
+ directory.CopyByAppendingPathComponent("root").GetPath(),
+ directory.GetPath());
}
- std::shared_ptr<llvm::FileCollectorBase> GetFileCollector() {
+ std::shared_ptr<llvm::FileCollector> GetFileCollector() {
return m_collector;
}
+ void Keep() override;
+
void RecordInterestingDirectory(const llvm::Twine &dir);
void RecordInterestingDirectoryRecursive(const llvm::Twine &dir);
static char ID;
private:
- std::shared_ptr<FlushingFileCollector> m_collector;
+ std::shared_ptr<llvm::FileCollector> m_collector;
};
/// Provider for the LLDB version number.
diff --git a/lldb/source/API/SBReproducer.cpp b/lldb/source/API/SBReproducer.cpp
index d3d27cc577480..7431b49d3ea62 100644
--- a/lldb/source/API/SBReproducer.cpp
+++ b/lldb/source/API/SBReproducer.cpp
@@ -110,20 +110,7 @@ const char *SBReproducer::Replay(const char *path,
const char *SBReproducer::Finalize(const char *path) {
LLDB_INSTRUMENT_VA(path)
- static std::string error;
-
- repro::Loader *loader = repro::Reproducer::Instance().GetLoader();
- if (!loader) {
- error = "unable to get replay loader.";
- return error.c_str();
- }
-
- if (auto e = repro::Finalize(loader)) {
- error = llvm::toString(std::move(e));
- return error.c_str();
- }
-
- return nullptr;
+ return "Reproducer finalize has been removed";
}
bool SBReproducer::Generate() {
diff --git a/lldb/source/Commands/CommandObjectReproducer.cpp b/lldb/source/Commands/CommandObjectReproducer.cpp
index 8b67b27c10f95..143712134feeb 100644
--- a/lldb/source/Commands/CommandObjectReproducer.cpp
+++ b/lldb/source/Commands/CommandObjectReproducer.cpp
@@ -191,10 +191,6 @@ class CommandObjectReproducerGenerate : public CommandObjectParsed {
auto &r = Reproducer::Instance();
if (auto generator = r.GetGenerator()) {
generator->Keep();
- if (llvm::Error e = repro::Finalize(r.GetReproducerPath())) {
- SetError(result, std::move(e));
- return result.Succeeded();
- }
} else {
result.AppendErrorWithFormat("Unable to get the reproducer generator");
return false;
diff --git a/lldb/source/Host/common/FileSystem.cpp b/lldb/source/Host/common/FileSystem.cpp
index 4a8cc3a21ea43..2367a641e9350 100644
--- a/lldb/source/Host/common/FileSystem.cpp
+++ b/lldb/source/Host/common/FileSystem.cpp
@@ -49,11 +49,6 @@ void FileSystem::Initialize() {
InstanceImpl().emplace();
}
-void FileSystem::Initialize(std::shared_ptr<FileCollectorBase> collector) {
- lldbassert(!InstanceImpl() && "Already initialized.");
- InstanceImpl().emplace(collector);
-}
-
void FileSystem::Initialize(IntrusiveRefCntPtr<vfs::FileSystem> fs) {
lldbassert(!InstanceImpl() && "Already initialized.");
InstanceImpl().emplace(fs);
@@ -281,8 +276,6 @@ void FileSystem::Resolve(FileSpec &file_spec) {
std::shared_ptr<DataBufferLLVM>
FileSystem::CreateDataBuffer(const llvm::Twine &path, uint64_t size,
uint64_t offset) {
- Collect(path);
-
const bool is_volatile = !IsLocal(path);
std::unique_ptr<llvm::WritableMemoryBuffer> buffer;
if (size == 0) {
@@ -430,8 +423,6 @@ static mode_t GetOpenMode(uint32_t permissions) {
Expected<FileUP> FileSystem::Open(const FileSpec &file_spec,
File::OpenOptions options,
uint32_t permissions, bool should_close_fd) {
- Collect(file_spec.GetPath());
-
const int open_flags = GetOpenFlags(options);
const mode_t open_mode =
(open_flags & O_CREAT) ? GetOpenMode(permissions) : 0;
@@ -451,20 +442,6 @@ Expected<FileUP> FileSystem::Open(const FileSpec &file_spec,
return std::move(file);
}
-void FileSystem::Collect(const FileSpec &file_spec) {
- Collect(file_spec.GetPath());
-}
-
-void FileSystem::Collect(const llvm::Twine &file) {
- if (!m_collector)
- return;
-
- if (llvm::sys::fs::is_directory(file))
- m_collector->addDirectory(file);
- else
- m_collector->addFile(file);
-}
-
void FileSystem::SetHomeDirectory(std::string home_directory) {
m_home_directory = std::move(home_directory);
}
diff --git a/lldb/source/Host/posix/FileSystemPosix.cpp b/lldb/source/Host/posix/FileSystemPosix.cpp
index 0aa34bc594359..3660f67895a4f 100644
--- a/lldb/source/Host/posix/FileSystemPosix.cpp
+++ b/lldb/source/Host/posix/FileSystemPosix.cpp
@@ -72,11 +72,9 @@ Status FileSystem::ResolveSymbolicLink(const FileSpec &src, FileSpec &dst) {
}
FILE *FileSystem::Fopen(const char *path, const char *mode) {
- Collect(path);
return llvm::sys::RetryAfterSignal(nullptr, ::fopen, path, mode);
}
int FileSystem::Open(const char *path, int flags, int mode) {
- Collect(path);
return llvm::sys::RetryAfterSignal(-1, ::open, path, flags, mode);
}
diff --git a/lldb/source/Initialization/SystemInitializerCommon.cpp b/lldb/source/Initialization/SystemInitializerCommon.cpp
index bb44d9116717c..b14218442cec8 100644
--- a/lldb/source/Initialization/SystemInitializerCommon.cpp
+++ b/lldb/source/Initialization/SystemInitializerCommon.cpp
@@ -43,12 +43,15 @@ SystemInitializerCommon::~SystemInitializerCommon() = default;
/// Initialize the FileSystem based on the current reproducer mode.
static llvm::Error InitializeFileSystem() {
auto &r = repro::Reproducer::Instance();
+
if (repro::Generator *g = r.GetGenerator()) {
repro::VersionProvider &vp = g->GetOrCreate<repro::VersionProvider>();
vp.SetVersion(lldb_private::GetVersion());
repro::FileProvider &fp = g->GetOrCreate<repro::FileProvider>();
- FileSystem::Initialize(fp.GetFileCollector());
+
+ FileSystem::Initialize(llvm::FileCollector::createCollectorVFS(
+ llvm::vfs::getRealFileSystem(), fp.GetFileCollector()));
fp.RecordInterestingDirectory(
g->GetOrCreate<repro::WorkingDirectoryProvider>().GetDirectory());
diff --git a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
index 544a7b7670577..d9943f0dddd5d 100644
--- a/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
+++ b/lldb/source/Plugins/ScriptInterpreter/Python/ScriptInterpreterPython.cpp
@@ -2723,7 +2723,6 @@ bool ScriptInterpreterPythonImpl::LoadScriptingModule(
} else {
FileSpec module_file(pathname);
FileSystem::Instance().Resolve(module_file);
- FileSystem::Instance().Collect(module_file);
fs::file_status st;
std::error_code ec = status(module_file.GetPath(), st);
diff --git a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
index 31224ecb18c67..72ae44f043634 100644
--- a/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
+++ b/lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARFDebugMap.cpp
@@ -409,7 +409,6 @@ Module *SymbolFileDWARFDebugMap::GetModuleByCompUnitInfo(
FileSpec oso_file(oso_path);
ConstString oso_object;
if (FileSystem::Instance().Exists(oso_file)) {
- FileSystem::Instance().Collect(oso_file);
// The modification time returned by the FS can have a higher precision
// than the one from the CU.
auto oso_mod_time = std::chrono::time_point_cast<std::chrono::seconds>(
diff --git a/lldb/source/Utility/Reproducer.cpp b/lldb/source/Utility/Reproducer.cpp
index a2ada12140cf6..3641c933c38a1 100644
--- a/lldb/source/Utility/Reproducer.cpp
+++ b/lldb/source/Utility/Reproducer.cpp
@@ -136,7 +136,6 @@ Generator::~Generator() {
if (!m_done) {
if (m_auto_generate) {
Keep();
- llvm::cantFail(Finalize(GetRoot()));
} else {
Discard();
}
@@ -229,58 +228,3 @@ bool Loader::HasFile(StringRef file) {
auto it = std::lower_bound(m_files.begin(), m_files.end(), file.str());
return (it != m_files.end()) && (*it == file);
}
-
-static llvm::Error addPaths(StringRef path,
- function_ref<void(StringRef)> callback) {
- auto buffer = llvm::MemoryBuffer::getFile(path);
- if (!buffer)
- return errorCodeToError(buffer.getError());
-
- SmallVector<StringRef, 0> paths;
- (*buffer)->getBuffer().split(paths, '\0');
- for (StringRef p : paths) {
- if (!p.empty() && llvm::sys::fs::exists(p))
- callback(p);
- }
-
- return errorCodeToError(llvm::sys::fs::remove(path));
-}
-
-llvm::Error repro::Finalize(Loader *loader) {
- if (!loader)
- return make_error<StringError>("invalid loader",
- llvm::inconvertibleErrorCode());
-
- FileSpec reproducer_root = loader->GetRoot();
- std::string files_path =
- reproducer_root.CopyByAppendingPathComponent("files.txt").GetPath();
- std::string dirs_path =
- reproducer_root.CopyByAppendingPathComponent("dirs.txt").GetPath();
-
- FileCollector collector(
- reproducer_root.CopyByAppendingPathComponent("root").GetPath(),
- reproducer_root.GetPath());
-
- if (Error e =
- addPaths(files_path, [&](StringRef p) { collector.addFile(p); }))
- return e;
-
- if (Error e =
- addPaths(dirs_path, [&](StringRef p) { collector.addDirectory(p); }))
- return e;
-
- FileSpec mapping =
- reproducer_root.CopyByAppendingPathComponent(FileProvider::Info::file);
- if (auto ec = collector.copyFiles(/*StopOnError=*/false))
- return errorCodeToError(ec);
- collector.writeMapping(mapping.GetPath());
-
- return llvm::Error::success();
-}
-
-llvm::Error repro::Finalize(const FileSpec &root) {
- Loader loader(root);
- if (Error e = loader.LoadIndex())
- return e;
- return Finalize(&loader);
-}
diff --git a/lldb/source/Utility/ReproducerProvider.cpp b/lldb/source/Utility/ReproducerProvider.cpp
index 5145819b717c7..0d1581abda645 100644
--- a/lldb/source/Utility/ReproducerProvider.cpp
+++ b/lldb/source/Utility/ReproducerProvider.cpp
@@ -45,40 +45,6 @@ void VersionProvider::Keep() {
os << m_version << "\n";
}
-FlushingFileCollector::FlushingFileCollector(llvm::StringRef files_path,
- llvm::StringRef dirs_path,
- std::error_code &ec) {
- auto clear = llvm::make_scope_exit([this]() {
- m_files_os.reset();
- m_dirs_os.reset();
- });
- m_files_os.emplace(files_path, ec, llvm::sys::fs::OF_Append);
- if (ec)
- return;
- m_dirs_os.emplace(dirs_path, ec, llvm::sys::fs::OF_Append);
- if (ec)
- return;
- clear.release();
-}
-
-void FlushingFileCollector::addFileImpl(StringRef file) {
- if (m_files_os) {
- *m_files_os << file << '\0';
- m_files_os->flush();
- }
-}
-
-llvm::vfs::directory_iterator
-FlushingFileCollector::addDirectoryImpl(const Twine &dir,
- IntrusiveRefCntPtr<vfs::FileSystem> vfs,
- std::error_code &dir_ec) {
- if (m_dirs_os) {
- *m_dirs_os << dir << '\0';
- m_dirs_os->flush();
- }
- return vfs->dir_begin(dir, dir_ec);
-}
-
void FileProvider::RecordInterestingDirectory(const llvm::Twine &dir) {
if (m_collector)
m_collector->addFile(dir);
@@ -89,6 +55,13 @@ void FileProvider::RecordInterestingDirectoryRecursive(const llvm::Twine &dir) {
m_collector->addDirectory(dir);
}
+void FileProvider::Keep() {
+ if (m_collector) {
+ FileSpec file = GetRoot().CopyByAppendingPathComponent(Info::file);
+ m_collector->writeMapping(file.GetPath());
+ }
+}
+
llvm::Expected<std::unique_ptr<ProcessInfoRecorder>>
ProcessInfoRecorder::Create(const FileSpec &filename) {
std::error_code ec;
More information about the lldb-commits
mailing list