https://github.com/ellishg updated https://github.com/llvm/llvm-project/pull/90925
>From 4760ebce0ff7725f4bb75f5107f551d867e4db6d Mon Sep 17 00:00:00 2001 From: Ellis Hoag <ellis.sparky.h...@gmail.com> Date: Thu, 2 May 2024 17:47:38 -0700 Subject: [PATCH 1/3] [modules] Accept equivalent module caches from different symlink Use `fs::equivalent()`, which follows symlinks, to check if two module cache paths are equivalent. This prevents a PCH error when building from a different path that is a symlink of the original. ``` error: PCH was compiled with module cache path '/home/foo/blah/ModuleCache/2IBP1TNT8OR8D', but the path is currently '/data/users/foo/blah/ModuleCache/2IBP1TNT8OR8D' 1 error generated. ``` --- clang/lib/Serialization/ASTReader.cpp | 20 +++++++++----------- clang/test/Modules/module-symlink.m | 11 +++++++++++ 2 files changed, 20 insertions(+), 11 deletions(-) create mode 100644 clang/test/Modules/module-symlink.m diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 0ef57a3ea804ef..c20ead8b865692 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -839,17 +839,15 @@ static bool checkHeaderSearchOptions(const HeaderSearchOptions &HSOpts, DiagnosticsEngine *Diags, const LangOptions &LangOpts, const PreprocessorOptions &PPOpts) { - if (LangOpts.Modules) { - if (SpecificModuleCachePath != ExistingModuleCachePath && - !PPOpts.AllowPCHWithDifferentModulesCachePath) { - if (Diags) - Diags->Report(diag::err_pch_modulecache_mismatch) - << SpecificModuleCachePath << ExistingModuleCachePath; - return true; - } - } - - return false; + if (!LangOpts.Modules || PPOpts.AllowPCHWithDifferentModulesCachePath || + SpecificModuleCachePath == ExistingModuleCachePath || + llvm::sys::fs::equivalent(SpecificModuleCachePath, + ExistingModuleCachePath)) + return false; + if (Diags) + Diags->Report(diag::err_pch_modulecache_mismatch) + << SpecificModuleCachePath << ExistingModuleCachePath; + return true; } bool PCHValidator::ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts, diff --git a/clang/test/Modules/module-symlink.m b/clang/test/Modules/module-symlink.m new file mode 100644 index 00000000000000..be447449a0e81e --- /dev/null +++ b/clang/test/Modules/module-symlink.m @@ -0,0 +1,11 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -fmodules-cache-path=%t/modules -fmodules -fimplicit-module-maps -I %S/Inputs -emit-pch -o %t.pch %s -verify + +// RUN: ln -s %t/modules %t/modules.symlink +// RUN: %clang_cc1 -fmodules-cache-path=%t/modules.symlink -fmodules -fimplicit-module-maps -I %S/Inputs -include-pch %t.pch %s -verify + +// expected-no-diagnostics + +@import ignored_macros; + +struct Point p; >From 490eefe98e3dd020ff3e51c7f817ec2b3d3a2663 Mon Sep 17 00:00:00 2001 From: Ellis Hoag <ellis.sparky.h...@gmail.com> Date: Fri, 3 May 2024 09:50:11 -0700 Subject: [PATCH 2/3] Require shell to fix windows test --- clang/test/Modules/module-symlink.m | 2 ++ 1 file changed, 2 insertions(+) diff --git a/clang/test/Modules/module-symlink.m b/clang/test/Modules/module-symlink.m index be447449a0e81e..9a69186c5ea28f 100644 --- a/clang/test/Modules/module-symlink.m +++ b/clang/test/Modules/module-symlink.m @@ -1,3 +1,5 @@ +// REQUIRES: shell + // RUN: rm -rf %t // RUN: %clang_cc1 -fmodules-cache-path=%t/modules -fmodules -fimplicit-module-maps -I %S/Inputs -emit-pch -o %t.pch %s -verify >From 6e58177107f854f42d3cdc70e796c425a1797798 Mon Sep 17 00:00:00 2001 From: Ellis Hoag <ellis.sparky.h...@gmail.com> Date: Fri, 3 May 2024 10:34:35 -0700 Subject: [PATCH 3/3] Use VFS to check if files are equal --- clang/lib/Serialization/ASTReader.cpp | 25 +++++++++++-------- clang/test/Modules/module-symlink.m | 1 + llvm/include/llvm/Support/VirtualFileSystem.h | 4 +++ llvm/lib/Support/VirtualFileSystem.cpp | 10 ++++++++ 4 files changed, 29 insertions(+), 11 deletions(-) diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index c20ead8b865692..d35c870926f96e 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -833,16 +833,18 @@ bool SimpleASTReaderListener::ReadPreprocessorOptions( /// against the header search options in an existing preprocessor. /// /// \param Diags If non-null, produce diagnostics for any mismatches incurred. -static bool checkHeaderSearchOptions(const HeaderSearchOptions &HSOpts, +static bool checkHeaderSearchOptions(llvm::vfs::FileSystem &VFS, StringRef SpecificModuleCachePath, StringRef ExistingModuleCachePath, DiagnosticsEngine *Diags, const LangOptions &LangOpts, const PreprocessorOptions &PPOpts) { if (!LangOpts.Modules || PPOpts.AllowPCHWithDifferentModulesCachePath || - SpecificModuleCachePath == ExistingModuleCachePath || - llvm::sys::fs::equivalent(SpecificModuleCachePath, - ExistingModuleCachePath)) + SpecificModuleCachePath == ExistingModuleCachePath) + return false; + auto EqualOrErr = + VFS.equivalent(SpecificModuleCachePath, ExistingModuleCachePath); + if (EqualOrErr && *EqualOrErr) return false; if (Diags) Diags->Report(diag::err_pch_modulecache_mismatch) @@ -853,10 +855,11 @@ static bool checkHeaderSearchOptions(const HeaderSearchOptions &HSOpts, bool PCHValidator::ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts, StringRef SpecificModuleCachePath, bool Complain) { - return checkHeaderSearchOptions(HSOpts, SpecificModuleCachePath, - PP.getHeaderSearchInfo().getModuleCachePath(), - Complain ? &Reader.Diags : nullptr, - PP.getLangOpts(), PP.getPreprocessorOpts()); + return checkHeaderSearchOptions( + Reader.getFileManager().getVirtualFileSystem(), SpecificModuleCachePath, + PP.getHeaderSearchInfo().getModuleCachePath(), + Complain ? &Reader.Diags : nullptr, PP.getLangOpts(), + PP.getPreprocessorOpts()); } void PCHValidator::ReadCounter(const ModuleFile &M, unsigned Value) { @@ -5387,9 +5390,9 @@ namespace { bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts, StringRef SpecificModuleCachePath, bool Complain) override { - return checkHeaderSearchOptions(HSOpts, SpecificModuleCachePath, - ExistingModuleCachePath, nullptr, - ExistingLangOpts, ExistingPPOpts); + return checkHeaderSearchOptions( + FileMgr.getVirtualFileSystem(), SpecificModuleCachePath, + ExistingModuleCachePath, nullptr, ExistingLangOpts, ExistingPPOpts); } bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts, diff --git a/clang/test/Modules/module-symlink.m b/clang/test/Modules/module-symlink.m index 9a69186c5ea28f..efdaf3db0dfef2 100644 --- a/clang/test/Modules/module-symlink.m +++ b/clang/test/Modules/module-symlink.m @@ -5,6 +5,7 @@ // RUN: ln -s %t/modules %t/modules.symlink // RUN: %clang_cc1 -fmodules-cache-path=%t/modules.symlink -fmodules -fimplicit-module-maps -I %S/Inputs -include-pch %t.pch %s -verify +// RUN: not %clang_cc1 -fmodules-cache-path=%t/modules.dne -fmodules -fimplicit-module-maps -I %S/Inputs -include-pch %t.pch %s -verify // expected-no-diagnostics diff --git a/llvm/include/llvm/Support/VirtualFileSystem.h b/llvm/include/llvm/Support/VirtualFileSystem.h index 0113d6b7da25d3..cea99e20eb29a5 100644 --- a/llvm/include/llvm/Support/VirtualFileSystem.h +++ b/llvm/include/llvm/Support/VirtualFileSystem.h @@ -320,6 +320,10 @@ class FileSystem : public llvm::ThreadSafeRefCountedBase<FileSystem>, /// platform-specific error_code. virtual std::error_code makeAbsolute(SmallVectorImpl<char> &Path) const; + /// \returns true if \p A and \p B represent the same file, or an error or + /// false if they do not. + llvm::ErrorOr<bool> equivalent(const Twine &A, const Twine &B); + enum class PrintType { Summary, Contents, RecursiveContents }; void print(raw_ostream &OS, PrintType Type = PrintType::Contents, unsigned IndentLevel = 0) const { diff --git a/llvm/lib/Support/VirtualFileSystem.cpp b/llvm/lib/Support/VirtualFileSystem.cpp index 921af30bfcde9f..f5074930718fe0 100644 --- a/llvm/lib/Support/VirtualFileSystem.cpp +++ b/llvm/lib/Support/VirtualFileSystem.cpp @@ -151,6 +151,16 @@ bool FileSystem::exists(const Twine &Path) { return Status && Status->exists(); } +llvm::ErrorOr<bool> FileSystem::equivalent(const Twine &A, const Twine &B) { + auto StatusA = status(A); + if (!StatusA) + return StatusA.getError(); + auto StatusB = status(B); + if (!StatusB) + return StatusB.getError(); + return StatusA->equivalent(*StatusB); +} + #if !defined(NDEBUG) || defined(LLVM_ENABLE_DUMP) void FileSystem::dump() const { print(dbgs(), PrintType::RecursiveContents); } #endif _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits