[clang] [llvm] [modules] Accept equivalent module caches from different symlink (PR #90925)
Ellis Hoag via cfe-commits
cfe-commits at lists.llvm.org
Tue May 7 12:18:16 PDT 2024
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.hoag at gmail.com>
Date: Thu, 2 May 2024 17:47:38 -0700
Subject: [PATCH 1/4] [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
+
+ at import ignored_macros;
+
+struct Point p;
>From 490eefe98e3dd020ff3e51c7f817ec2b3d3a2663 Mon Sep 17 00:00:00 2001
From: Ellis Hoag <ellis.sparky.hoag at gmail.com>
Date: Fri, 3 May 2024 09:50:11 -0700
Subject: [PATCH 2/4] 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.hoag at gmail.com>
Date: Fri, 3 May 2024 10:34:35 -0700
Subject: [PATCH 3/4] 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
>From f9f3fcdf350e1a8381caea3e6d2cf776f26ba6a2 Mon Sep 17 00:00:00 2001
From: Ellis Hoag <ellis.sparky.hoag at gmail.com>
Date: Tue, 7 May 2024 12:17:43 -0700
Subject: [PATCH 4/4] Rename to checkModuleCachePath
---
clang/lib/Serialization/ASTReader.cpp | 28 +++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index d35c870926f96e..0ad254184aa0fe 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -829,16 +829,16 @@ bool SimpleASTReaderListener::ReadPreprocessorOptions(
OptionValidateNone);
}
-/// Check the header search options deserialized from the control block
-/// against the header search options in an existing preprocessor.
+/// Check that the specified and the existing module cache paths are equivalent.
///
/// \param Diags If non-null, produce diagnostics for any mismatches incurred.
-static bool checkHeaderSearchOptions(llvm::vfs::FileSystem &VFS,
- StringRef SpecificModuleCachePath,
- StringRef ExistingModuleCachePath,
- DiagnosticsEngine *Diags,
- const LangOptions &LangOpts,
- const PreprocessorOptions &PPOpts) {
+/// \returns true when the module cache paths differ.
+static bool checkModuleCachePath(llvm::vfs::FileSystem &VFS,
+ StringRef SpecificModuleCachePath,
+ StringRef ExistingModuleCachePath,
+ DiagnosticsEngine *Diags,
+ const LangOptions &LangOpts,
+ const PreprocessorOptions &PPOpts) {
if (!LangOpts.Modules || PPOpts.AllowPCHWithDifferentModulesCachePath ||
SpecificModuleCachePath == ExistingModuleCachePath)
return false;
@@ -855,11 +855,11 @@ static bool checkHeaderSearchOptions(llvm::vfs::FileSystem &VFS,
bool PCHValidator::ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
StringRef SpecificModuleCachePath,
bool Complain) {
- return checkHeaderSearchOptions(
- Reader.getFileManager().getVirtualFileSystem(), SpecificModuleCachePath,
- PP.getHeaderSearchInfo().getModuleCachePath(),
- Complain ? &Reader.Diags : nullptr, PP.getLangOpts(),
- PP.getPreprocessorOpts());
+ return checkModuleCachePath(Reader.getFileManager().getVirtualFileSystem(),
+ SpecificModuleCachePath,
+ PP.getHeaderSearchInfo().getModuleCachePath(),
+ Complain ? &Reader.Diags : nullptr,
+ PP.getLangOpts(), PP.getPreprocessorOpts());
}
void PCHValidator::ReadCounter(const ModuleFile &M, unsigned Value) {
@@ -5390,7 +5390,7 @@ namespace {
bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts,
StringRef SpecificModuleCachePath,
bool Complain) override {
- return checkHeaderSearchOptions(
+ return checkModuleCachePath(
FileMgr.getVirtualFileSystem(), SpecificModuleCachePath,
ExistingModuleCachePath, nullptr, ExistingLangOpts, ExistingPPOpts);
}
More information about the cfe-commits
mailing list