[clang] [DependencyScanningFilesystem] Make sure the local/shared cache filename lookups use only absolute paths (PR #66122)
Ben Langmuir via cfe-commits
cfe-commits at lists.llvm.org
Fri Sep 15 11:34:28 PDT 2023
================
@@ -330,3 +353,20 @@ DependencyScanningWorkerFilesystem::openFileForRead(const Twine &Path) {
return Result.getError();
return DepScanFile::create(Result.get());
}
+
+std::error_code DependencyScanningWorkerFilesystem::setCurrentWorkingDirectory(
+ const Twine &Path) {
+ updateWorkingDirForCacheLookup(Path.str());
+ return ProxyFileSystem::setCurrentWorkingDirectory(Path);
+}
+
+void DependencyScanningWorkerFilesystem::updateWorkingDirForCacheLookup(
+ llvm::ErrorOr<std::string> CWD) {
+ if (CWD && !CWD->empty()) {
+ WorkingDirForCacheLookup = *CWD;
+ } else {
+ // The cache lookup functions will not accept relative paths for safety, so
+ // at least make it absolute from a "root".
+ WorkingDirForCacheLookup = "/";
----------------
benlangmuir wrote:
> What is the specific concern? If multiple TUs got errors for working directory and subsequently used the same standard prefix then they may resolve to same path for a relative filename but these TUs are already in an erroneous state already, it's not invalidating their correctness.
The dummy path you're generating can overlap with a real file path. E.g. failing on "include/foo.h" poisons "/include/foo.h".
> My general point is that I don't think we should make the assumption that if a VFS implementation emits an error for setCurrentWorkingDirectory() then that VFS instance should not ever get used again subsequently.
I think this is fine as long as our cache cannot poison the value for a real path.
https://github.com/llvm/llvm-project/pull/66122
More information about the cfe-commits
mailing list