https://github.com/jansvoboda11 created https://github.com/llvm/llvm-project/pull/185765
This PR changes how `ModuleManager` deduplicates module files. Previously, `ModuleManager` used `FileEntry` for assigning unique identity to module files. This works fine for explicitly-built modules because they don't change during the lifetime of a single Clang instance. For implicitly-built modules however, there are two issues: 1. The `FileEntry` objects are deduplicated by `FileManager` based on the inode number. Some file systems reuse inode numbers of previously removed files. Because implicitly-built module files are rapidly removed and created, this deduplication breaks and compilations may fail spuriously when inode numbers are recycled during the lifetime of a single Clang instance. 2. The first thing `ModuleManager` does when loading a module file is consulting the `FileManager` and checking the file size and modification time match the expectation of the importer. This is done even when such module file already lives in the `InMemoryModuleCache`. This introduces racy behavior into the mechanism that explicitly tries to solve race conditions, and may lead into spurious compilation failures. This PR identifies implicitly-built module files by a pair of `DirectoryEntry` of the module cache path and the path suffix `<context-hash>/<module-name>-<module-map-path-hash>.pcm`. This gives us canonicalization of the user-provided module cache path without turning to `FileEntry` for the PCM file. The path suffix is Clang-generated and is already canonical. Some tests needed to be updated because the module cache path directory was also used as an include directory. This PR relies on not caching the non-existence of the module cache directory in the `FileManager`. When other parts of Clang are trying to look up the same path and cache its non-existence, things break. This is probably very specific to some of our tests and not how users are setting up their compilations. >From c599af32a82eb924c88fb9114a0df082e48e1117 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Tue, 10 Mar 2026 13:17:39 -0700 Subject: [PATCH 1/3] [clang][modules] Provide normalized module cache path from `HeaderSearch` --- .../include/clang/Frontend/CompilerInstance.h | 5 --- clang/include/clang/Lex/HeaderSearch.h | 25 +++++++++---- clang/include/clang/Serialization/ASTReader.h | 9 ++--- .../DependencyScannerImpl.cpp | 3 +- clang/lib/Frontend/ASTUnit.cpp | 23 ++++++------ clang/lib/Frontend/CompilerInstance.cpp | 22 ++--------- clang/lib/Frontend/FrontendAction.cpp | 5 ++- clang/lib/Frontend/FrontendActions.cpp | 6 ++- clang/lib/Lex/HeaderSearch.cpp | 28 ++++++++++++++ clang/lib/Serialization/ASTReader.cpp | 37 ++++++++++--------- clang/lib/Serialization/ASTWriter.cpp | 10 ++--- 11 files changed, 95 insertions(+), 78 deletions(-) diff --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h index 266e0826b38f4..f7a0d1064110a 100644 --- a/clang/include/clang/Frontend/CompilerInstance.h +++ b/clang/include/clang/Frontend/CompilerInstance.h @@ -746,11 +746,6 @@ class CompilerInstance : public ModuleLoader { GetDependencyDirectives = std::move(Getter); } - std::string getSpecificModuleCachePath(StringRef ContextHash); - std::string getSpecificModuleCachePath() { - return getSpecificModuleCachePath(getInvocation().computeContextHash()); - } - /// Create the AST context. void createASTContext(); diff --git a/clang/include/clang/Lex/HeaderSearch.h b/clang/include/clang/Lex/HeaderSearch.h index d7000da682c6e..2bb78fbf608f3 100644 --- a/clang/include/clang/Lex/HeaderSearch.h +++ b/clang/include/clang/Lex/HeaderSearch.h @@ -282,6 +282,10 @@ class HeaderSearch { /// The specific module cache path containing ContextHash (unless suppressed). std::string SpecificModuleCachePath; + /// The length of the normalized module cache path at the start of \c + /// SpecificModuleCachePath. + size_t NormalizedModuleCachePathLen = 0; + /// All of the preprocessor-specific data about files that are /// included, indexed by the FileEntry's UID. mutable std::vector<HeaderFileInfo> FileInfo; @@ -467,20 +471,20 @@ class HeaderSearch { return {}; } - /// Set the context hash to use for module cache paths. - void setContextHash(StringRef Hash) { ContextHash = std::string(Hash); } + /// Initialize the module cache path. + void initializeModuleCachePath(std::string ContextHash); - /// Set the module cache path with the context hash (unless suppressed). - void setSpecificModuleCachePath(StringRef Path) { - SpecificModuleCachePath = std::string(Path); + /// Retrieve the module cache path with the context hash (unless suppressed). + StringRef getSpecificModuleCachePath() const { + return SpecificModuleCachePath; } /// Retrieve the context hash. StringRef getContextHash() const { return ContextHash; } - /// Retrieve the module cache path with the context hash (unless suppressed). - StringRef getSpecificModuleCachePath() const { - return SpecificModuleCachePath; + /// Retrieve the normalized module cache path. + StringRef getNormalizedModuleCachePath() const { + return getSpecificModuleCachePath().substr(0, NormalizedModuleCachePathLen); } /// Forget everything we know about headers so far. @@ -1042,6 +1046,11 @@ void ApplyHeaderSearchOptions(HeaderSearch &HS, void normalizeModuleCachePath(FileManager &FileMgr, StringRef Path, SmallVectorImpl<char> &NormalizedPath); +std::string createSpecificModuleCachePath(FileManager &FileMgr, + StringRef ModuleCachePath, + bool DisableModuleHash, + std::string ContextHash); + } // namespace clang #endif // LLVM_CLANG_LEX_HEADERSEARCH_H diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index f254459ce933d..14f7d8a69a1b3 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -185,8 +185,7 @@ class ASTReaderListener { /// otherwise. virtual bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts, StringRef ModuleFilename, - StringRef SpecificModuleCachePath, - bool Complain) { + StringRef ContextHash, bool Complain) { return false; } @@ -304,8 +303,7 @@ class ChainedASTReaderListener : public ASTReaderListener { bool Complain) override; bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts, - StringRef ModuleFilename, - StringRef SpecificModuleCachePath, + StringRef ModuleFilename, StringRef ContextHash, bool Complain) override; bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts, StringRef ModuleFilename, bool ReadMacros, @@ -349,8 +347,7 @@ class PCHValidator : public ASTReaderListener { bool Complain, std::string &SuggestedPredefines) override; bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts, - StringRef ModuleFilename, - StringRef SpecificModuleCachePath, + StringRef ModuleFilename, StringRef ContextHash, bool Complain) override; void ReadCounter(const serialization::ModuleFile &M, uint32_t Value) override; }; diff --git a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp index 0e345af8817ae..a38ee690d42de 100644 --- a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp +++ b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp @@ -150,8 +150,7 @@ class PrebuiltModuleListener : public ASTReaderListener { /// Check the header search options for a given module when considering /// if the module comes from stable directories. bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts, - StringRef ModuleFilename, - StringRef SpecificModuleCachePath, + StringRef ModuleFilename, StringRef ContextHash, bool Complain) override { auto PrebuiltMapEntry = PrebuiltModulesASTMap.try_emplace(CurrentFile); diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp index 9fcaf1806fcb1..1d249ebaa1492 100644 --- a/clang/lib/Frontend/ASTUnit.cpp +++ b/clang/lib/Frontend/ASTUnit.cpp @@ -505,7 +505,7 @@ namespace { /// a Preprocessor. class ASTInfoCollector : public ASTReaderListener { HeaderSearchOptions &HSOpts; - std::string &SpecificModuleCachePath; + std::string &ContextHash; PreprocessorOptions &PPOpts; LangOptions &LangOpts; CodeGenOptions &CodeGenOpts; @@ -513,14 +513,13 @@ class ASTInfoCollector : public ASTReaderListener { uint32_t &Counter; public: - ASTInfoCollector(HeaderSearchOptions &HSOpts, - std::string &SpecificModuleCachePath, + ASTInfoCollector(HeaderSearchOptions &HSOpts, std::string &ContextHash, PreprocessorOptions &PPOpts, LangOptions &LangOpts, CodeGenOptions &CodeGenOpts, TargetOptions &TargetOpts, uint32_t &Counter) - : HSOpts(HSOpts), SpecificModuleCachePath(SpecificModuleCachePath), - PPOpts(PPOpts), LangOpts(LangOpts), CodeGenOpts(CodeGenOpts), - TargetOpts(TargetOpts), Counter(Counter) {} + : HSOpts(HSOpts), ContextHash(ContextHash), PPOpts(PPOpts), + LangOpts(LangOpts), CodeGenOpts(CodeGenOpts), TargetOpts(TargetOpts), + Counter(Counter) {} bool ReadLanguageOptions(const LangOptions &NewLangOpts, StringRef ModuleFilename, bool Complain, @@ -538,10 +537,10 @@ class ASTInfoCollector : public ASTReaderListener { bool ReadHeaderSearchOptions(const HeaderSearchOptions &NewHSOpts, StringRef ModuleFilename, - StringRef NewSpecificModuleCachePath, + StringRef NewContextHash, bool Complain) override { HSOpts = NewHSOpts; - SpecificModuleCachePath = NewSpecificModuleCachePath; + ContextHash = NewContextHash; return false; } @@ -733,13 +732,13 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromASTFile( AST->ModCache = createCrossProcessModuleCache(); // Gather info for preprocessor construction later on. - std::string SpecificModuleCachePath; + std::string ContextHash; unsigned Counter = 0; // Using a temporary FileManager since the AST file might specify custom // HeaderSearchOptions::VFSOverlayFiles that affect the underlying VFS. FileManager TmpFileMgr(FileSystemOpts, VFS); - ASTInfoCollector Collector(*AST->HSOpts, SpecificModuleCachePath, - *AST->PPOpts, *AST->LangOpts, *AST->CodeGenOpts, + ASTInfoCollector Collector(*AST->HSOpts, ContextHash, *AST->PPOpts, + *AST->LangOpts, *AST->CodeGenOpts, *AST->TargetOpts, Counter); if (ASTReader::readASTFileControlBlock( Filename, TmpFileMgr, *AST->ModCache, PCHContainerRdr, @@ -763,7 +762,7 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromASTFile( AST->getHeaderSearchOpts(), AST->getSourceManager(), AST->getDiagnostics(), AST->getLangOpts(), /*Target=*/nullptr); - AST->HeaderInfo->setSpecificModuleCachePath(SpecificModuleCachePath); + AST->HeaderInfo->initializeModuleCachePath(std::move(ContextHash)); AST->PP = std::make_shared<Preprocessor>( *AST->PPOpts, AST->getDiagnostics(), *AST->LangOpts, diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 60914d9b2cbc7..4c7a5a66254a5 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -486,12 +486,9 @@ void CompilerInstance::createPreprocessor(TranslationUnitKind TUKind) { PP->setPreprocessedOutput(getPreprocessorOutputOpts().ShowCPP); - if (PP->getLangOpts().Modules && PP->getLangOpts().ImplicitModules) { - std::string ContextHash = getInvocation().computeContextHash(); - PP->getHeaderSearchInfo().setContextHash(ContextHash); - PP->getHeaderSearchInfo().setSpecificModuleCachePath( - getSpecificModuleCachePath(ContextHash)); - } + if (PP->getLangOpts().Modules && PP->getLangOpts().ImplicitModules) + PP->getHeaderSearchInfo().initializeModuleCachePath( + getInvocation().computeContextHash()); // Handle generating dependencies, if requested. const DependencyOutputOptions &DepOpts = getDependencyOutputOpts(); @@ -546,19 +543,6 @@ void CompilerInstance::createPreprocessor(TranslationUnitKind TUKind) { PP->setDependencyDirectivesGetter(*GetDependencyDirectives); } -std::string -CompilerInstance::getSpecificModuleCachePath(StringRef ContextHash) { - assert(FileMgr && "Specific module cache path requires a FileManager"); - - // Set up the module path, including the hash for the module-creation options. - SmallString<256> SpecificModuleCache; - normalizeModuleCachePath(*FileMgr, getHeaderSearchOpts().ModuleCachePath, - SpecificModuleCache); - if (!SpecificModuleCache.empty() && !getHeaderSearchOpts().DisableModuleHash) - llvm::sys::path::append(SpecificModuleCache, ContextHash); - return std::string(SpecificModuleCache); -} - // ASTContext void CompilerInstance::createASTContext() { diff --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp index 73f092521546f..84ceb22208801 100644 --- a/clang/lib/Frontend/FrontendAction.cpp +++ b/clang/lib/Frontend/FrontendAction.cpp @@ -1040,7 +1040,10 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI, llvm::sys::path::native(PCHDir->getName(), DirNative); bool Found = false; llvm::vfs::FileSystem &FS = FileMgr.getVirtualFileSystem(); - std::string SpecificModuleCachePath = CI.getSpecificModuleCachePath(); + std::string SpecificModuleCachePath = createSpecificModuleCachePath( + CI.getFileManager(), CI.getHeaderSearchOpts().ModuleCachePath, + CI.getHeaderSearchOpts().DisableModuleHash, + CI.getInvocation().computeContextHash()); for (llvm::vfs::directory_iterator Dir = FS.dir_begin(DirNative, EC), DirEnd; Dir != DirEnd && !EC; Dir.increment(EC)) { diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp index 492f7b1742bee..15d19dae966ac 100644 --- a/clang/lib/Frontend/FrontendActions.cpp +++ b/clang/lib/Frontend/FrontendActions.cpp @@ -716,8 +716,12 @@ namespace { bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts, StringRef ModuleFilename, - StringRef SpecificModuleCachePath, + StringRef ContextHash, bool Complain) override { + SmallString<128> SpecificModuleCachePath(HSOpts.ModuleCachePath); + if (!SpecificModuleCachePath.empty() && !HSOpts.DisableModuleHash) + llvm::sys::path::append(SpecificModuleCachePath, ContextHash); + Out.indent(2) << "Header search options:\n"; Out.indent(4) << "System root [-isysroot=]: '" << HSOpts.Sysroot << "'\n"; Out.indent(4) << "Resource dir [ -resource-dir=]: '" << HSOpts.ResourceDir << "'\n"; diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp index 5aee19cd14c51..de26b999683d1 100644 --- a/clang/lib/Lex/HeaderSearch.cpp +++ b/clang/lib/Lex/HeaderSearch.cpp @@ -2471,3 +2471,31 @@ void clang::normalizeModuleCachePath(FileManager &FileMgr, StringRef Path, llvm::sys::path::remove_dots(NormalizedPath); } } + +static std::string createSpecificModuleCachePathImpl( + FileManager &FileMgr, StringRef ModuleCachePath, bool DisableModuleHash, + std::string ContextHash, size_t &NormalizedModuleCachePathLen) { + SmallString<256> SpecificModuleCachePath; + normalizeModuleCachePath(FileMgr, ModuleCachePath, SpecificModuleCachePath); + NormalizedModuleCachePathLen = SpecificModuleCachePath.size(); + if (!SpecificModuleCachePath.empty() && !DisableModuleHash) + llvm::sys::path::append(SpecificModuleCachePath, ContextHash); + return std::string(SpecificModuleCachePath); +} + +void HeaderSearch::initializeModuleCachePath(std::string NewContextHash) { + ContextHash = std::move(NewContextHash); + SpecificModuleCachePath = createSpecificModuleCachePathImpl( + FileMgr, HSOpts.ModuleCachePath, HSOpts.DisableModuleHash, ContextHash, + NormalizedModuleCachePathLen); +} + +std::string clang::createSpecificModuleCachePath(FileManager &FileMgr, + StringRef ModuleCachePath, + bool DisableModuleHash, + std::string ContextHash) { + size_t NormalizedModuleCachePathLen; + return createSpecificModuleCachePathImpl( + FileMgr, ModuleCachePath, DisableModuleHash, std::move(ContextHash), + NormalizedModuleCachePathLen); +} diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index b82ae971bc84d..812f27989c129 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -208,11 +208,11 @@ ChainedASTReaderListener::ReadFileSystemOptions(const FileSystemOptions &FSOpts, bool ChainedASTReaderListener::ReadHeaderSearchOptions( const HeaderSearchOptions &HSOpts, StringRef ModuleFilename, - StringRef SpecificModuleCachePath, bool Complain) { - return First->ReadHeaderSearchOptions(HSOpts, ModuleFilename, - SpecificModuleCachePath, Complain) || - Second->ReadHeaderSearchOptions(HSOpts, ModuleFilename, - SpecificModuleCachePath, Complain); + StringRef ContextHash, bool Complain) { + return First->ReadHeaderSearchOptions(HSOpts, ModuleFilename, ContextHash, + Complain) || + Second->ReadHeaderSearchOptions(HSOpts, ModuleFilename, ContextHash, + Complain); } bool ChainedASTReaderListener::ReadPreprocessorOptions( @@ -961,11 +961,15 @@ bool SimpleASTReaderListener::ReadPreprocessorOptions( /// \param Diags If non-null, produce diagnostics for any mismatches incurred. /// \returns true when the module cache paths differ. static bool checkModuleCachePath( - llvm::vfs::FileSystem &VFS, StringRef SpecificModuleCachePath, + llvm::vfs::FileSystem &VFS, StringRef ContextHash, StringRef ExistingSpecificModuleCachePath, StringRef ASTFilename, DiagnosticsEngine *Diags, const LangOptions &LangOpts, const PreprocessorOptions &PPOpts, const HeaderSearchOptions &HSOpts, const HeaderSearchOptions &ASTFileHSOpts) { + SmallString<128> SpecificModuleCachePath(ASTFileHSOpts.ModuleCachePath); + if (!SpecificModuleCachePath.empty() && !ASTFileHSOpts.DisableModuleHash) + llvm::sys::path::append(SpecificModuleCachePath, ContextHash); + if (!LangOpts.Modules || PPOpts.AllowPCHWithDifferentModulesCachePath || SpecificModuleCachePath == ExistingSpecificModuleCachePath) return false; @@ -991,11 +995,11 @@ static bool checkModuleCachePath( bool PCHValidator::ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts, StringRef ASTFilename, - StringRef SpecificModuleCachePath, + StringRef ContextHash, bool Complain) { const HeaderSearch &HeaderSearchInfo = PP.getHeaderSearchInfo(); return checkModuleCachePath( - Reader.getFileManager().getVirtualFileSystem(), SpecificModuleCachePath, + Reader.getFileManager().getVirtualFileSystem(), ContextHash, HeaderSearchInfo.getSpecificModuleCachePath(), ASTFilename, Complain ? &Reader.Diags : nullptr, PP.getLangOpts(), PP.getPreprocessorOpts(), HeaderSearchInfo.getHeaderSearchOpts(), HSOpts); @@ -5877,13 +5881,12 @@ namespace { } bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts, - StringRef ASTFilename, - StringRef SpecificModuleCachePath, + StringRef ASTFilename, StringRef ContextHash, bool Complain) override { - return checkModuleCachePath( - FileMgr.getVirtualFileSystem(), SpecificModuleCachePath, - ExistingSpecificModuleCachePath, ASTFilename, nullptr, - ExistingLangOpts, ExistingPPOpts, ExistingHSOpts, HSOpts); + return checkModuleCachePath(FileMgr.getVirtualFileSystem(), ContextHash, + ExistingSpecificModuleCachePath, ASTFilename, + nullptr, ExistingLangOpts, ExistingPPOpts, + ExistingHSOpts, HSOpts); } bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts, @@ -6705,10 +6708,10 @@ bool ASTReader::ParseHeaderSearchOptions(const RecordData &Record, HSOpts.UseStandardSystemIncludes = Record[Idx++]; HSOpts.UseStandardCXXIncludes = Record[Idx++]; HSOpts.UseLibcxx = Record[Idx++]; - std::string SpecificModuleCachePath = ReadString(Record, Idx); + std::string ContextHash = ReadString(Record, Idx); - return Listener.ReadHeaderSearchOptions(HSOpts, ModuleFilename, - SpecificModuleCachePath, Complain); + return Listener.ReadHeaderSearchOptions(HSOpts, ModuleFilename, ContextHash, + Complain); } bool ASTReader::ParseHeaderSearchPaths(const RecordData &Record, bool Complain, diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index ec718169550aa..2744d70b89aac 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -1694,9 +1694,8 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, StringRef isysroot) { const HeaderSearchOptions &HSOpts = PP.getHeaderSearchInfo().getHeaderSearchOpts(); - SmallString<256> HSOpts_ModuleCachePath; - normalizeModuleCachePath(PP.getFileManager(), HSOpts.ModuleCachePath, - HSOpts_ModuleCachePath); + StringRef HSOpts_ModuleCachePath = + PP.getHeaderSearchInfo().getNormalizedModuleCachePath(); AddString(HSOpts.Sysroot, Record); AddString(HSOpts.ResourceDir, Record); @@ -1710,10 +1709,7 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, StringRef isysroot) { Record.push_back(HSOpts.UseStandardSystemIncludes); Record.push_back(HSOpts.UseStandardCXXIncludes); Record.push_back(HSOpts.UseLibcxx); - // Write out the specific module cache path that contains the module files. - // FIXME: We already wrote out the normalized cache path. Just write the - // context hash (unless suppressed). - AddString(PP.getHeaderSearchInfo().getSpecificModuleCachePath(), Record); + AddString(PP.getHeaderSearchInfo().getContextHash(), Record); Stream.EmitRecord(HEADER_SEARCH_OPTIONS, Record); // Preprocessor options. >From 5a48f462ab14af12783bf0f0c575127f3e5d4e84 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Tue, 10 Mar 2026 13:26:58 -0700 Subject: [PATCH 2/3] Bump AST file version --- clang/include/clang/Serialization/ASTBitCodes.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h index 752e7fd288aa6..5db0b08f877ce 100644 --- a/clang/include/clang/Serialization/ASTBitCodes.h +++ b/clang/include/clang/Serialization/ASTBitCodes.h @@ -44,7 +44,7 @@ namespace serialization { /// Version 4 of AST files also requires that the version control branch and /// revision match exactly, since there is no backward compatibility of /// AST files at this time. -const unsigned VERSION_MAJOR = 35; +const unsigned VERSION_MAJOR = 36; /// AST file minor version number supported by this version of /// Clang. >From 164206bdc541f465acdf7afeb74aceb68687f1c1 Mon Sep 17 00:00:00 2001 From: Jan Svoboda <[email protected]> Date: Mon, 9 Mar 2026 14:38:24 -0700 Subject: [PATCH 3/3] [clang][modules] Stop uniquing implicit modules via `FileEntry` --- clang-tools-extra/clangd/ModulesBuilder.cpp | 4 +- clang/include/clang/Basic/Module.h | 114 ++++++++++ .../include/clang/Frontend/CompilerInstance.h | 3 +- clang/include/clang/Lex/HeaderSearch.h | 20 +- .../include/clang/Serialization/ASTBitCodes.h | 2 +- clang/include/clang/Serialization/ASTReader.h | 4 +- .../include/clang/Serialization/ModuleFile.h | 3 + .../clang/Serialization/ModuleManager.h | 44 ++-- clang/lib/Basic/Module.cpp | 17 ++ .../DependencyScannerImpl.cpp | 2 +- clang/lib/Frontend/ASTUnit.cpp | 5 +- clang/lib/Frontend/ChainedIncludesSource.cpp | 3 +- clang/lib/Frontend/CompilerInstance.cpp | 43 ++-- clang/lib/Frontend/FrontendAction.cpp | 2 +- clang/lib/Frontend/FrontendActions.cpp | 14 +- clang/lib/Lex/HeaderSearch.cpp | 37 ++-- clang/lib/Serialization/ASTReader.cpp | 43 ++-- clang/lib/Serialization/ASTWriter.cpp | 12 +- clang/lib/Serialization/GlobalModuleIndex.cpp | 1 + clang/lib/Serialization/ModuleManager.cpp | 209 ++++++++++-------- clang/test/Modules/DebugInfoNamespace.cpp | 2 +- clang/test/Modules/ExtDebugInfo.cpp | 2 +- clang/test/Modules/ExtDebugInfo.m | 2 +- clang/test/Modules/ModuleDebugInfo.cpp | 2 +- clang/test/Modules/ModuleDebugInfo.m | 2 +- clang/test/Modules/ModuleModuleDebugInfo.cpp | 2 +- clang/test/Modules/cxx20-hu-04.cpp | 2 +- clang/test/Modules/debug-info-moduleimport.m | 8 +- clang/test/Modules/global_index.m | 10 +- clang/test/Modules/load-after-failure.m | 4 +- clang/test/Modules/module-debuginfo-compdir.m | 2 +- clang/test/Modules/module-debuginfo-prefix.m | 2 +- clang/test/Modules/relocatable-modules.cpp | 8 +- .../Serialization/ForceCheckFileInputTest.cpp | 6 +- 34 files changed, 395 insertions(+), 241 deletions(-) diff --git a/clang-tools-extra/clangd/ModulesBuilder.cpp b/clang-tools-extra/clangd/ModulesBuilder.cpp index 524ec620c4076..b5a0127d48b7e 100644 --- a/clang-tools-extra/clangd/ModulesBuilder.cpp +++ b/clang-tools-extra/clangd/ModulesBuilder.cpp @@ -275,8 +275,8 @@ bool IsModuleFileUpToDate(PathRef ModuleFilePath, // listener. Reader.setListener(nullptr); - if (Reader.ReadAST(ModuleFilePath, serialization::MK_MainFile, - SourceLocation(), + if (Reader.ReadAST(ModuleFileName::make_explicit(ModuleFilePath), + serialization::MK_MainFile, SourceLocation(), ASTReader::ARR_None) != ASTReader::Success) return false; diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h index 69a1de6f79b35..c512249e5e945 100644 --- a/clang/include/clang/Basic/Module.h +++ b/clang/include/clang/Basic/Module.h @@ -54,6 +54,101 @@ class TargetInfo; /// Describes the name of a module. using ModuleId = SmallVector<std::pair<std::string, SourceLocation>, 2>; +namespace serialization { +class ModuleManager; +} // namespace serialization + +/// Deduplication key for a loaded module file in \c ModuleManager. +/// +/// For implicitly-built modules, this is the \c DirectoryEntry of the module +/// cache and the module file name with the (optional) context hash. +/// This enables using \c FileManager's inode-based canonicalization of the +/// user-provided module cache path without hitting issues on file systems that +/// recycle inodes for recompiled module files. +/// +/// For explicitly-built modules, this is \c FileEntry. +/// This uses \c FileManager's inode-based canonicalization of the user-provided +/// module file path. Because input explicitly-built modules do not change +/// during the lifetime of the compiler, inode recycling is not of concern here. +class ModuleFileKey { + /// The FileManager entity used for deduplication. + const void *Ptr; + /// The path relative to the module cache path for implicit module file, empty + /// for other kinds of module files. + std::string PathSuffix; + + friend class serialization::ModuleManager; + friend class ModuleFileName; + friend llvm::DenseMapInfo<ModuleFileKey>; + + ModuleFileKey(const void *Ptr) : Ptr(Ptr) {} + + ModuleFileKey(const FileEntry *ModuleFile) : Ptr(ModuleFile) {} + + ModuleFileKey(const DirectoryEntry *ModuleCacheDir, StringRef PathSuffix) + : Ptr(ModuleCacheDir), PathSuffix(PathSuffix) {} + +public: + bool operator==(const ModuleFileKey &Other) const { + return Ptr == Other.Ptr && PathSuffix == Other.PathSuffix; + } + + bool operator!=(const ModuleFileKey &Other) const { + return !operator==(Other); + } +}; + +/// Identifies a module file to be loaded. +/// +/// For implicitly-built module files, the path is split into the module cache +/// path and the module file name with the (optional) context hash. For all +/// other types of module files, this is just the file system path. +class ModuleFileName { + std::string Path; + std::optional<uint32_t> Separator; + +public: + /// Creates an empty module file name. + ModuleFileName() = default; + + /// Creates a file name for an explicit module. + static ModuleFileName make_explicit(std::string Name) { + ModuleFileName File; + File.Path = std::move(Name); + return File; + } + + /// Creates a file name for an explicit module. + static ModuleFileName make_explicit(StringRef Name) { + return make_explicit(Name.str()); + } + + /// Creates a file name for an implicit module. + static ModuleFileName make_implicit(std::string Name, uint32_t Separator) { + ModuleFileName File; + File.Path = std::move(Name); + File.Separator = Separator; + return File; + } + + /// Creates a file name for an implicit module. + static ModuleFileName make_implicit(StringRef Name, uint32_t Separator) { + return make_implicit(Name.str(), Separator); + } + + /// Returns the plain module file name. + StringRef str() const { return Path; } + + /// Converts to StringRef representing the plain module file name. + operator StringRef() const { return Path; } + + /// Checks whether the module file name is empty. + bool empty() const { return Path.empty(); } + + /// Creates the deduplication key for use in \c ModuleManager. + std::optional<ModuleFileKey> makeKey(FileManager &FileMgr) const; +}; + /// The signature of a module, which is a hash of the AST content. struct ASTFileSignature : std::array<uint8_t, 20> { using BaseT = std::array<uint8_t, 20>; @@ -926,4 +1021,23 @@ class VisibleModuleSet { } // namespace clang +template <> struct llvm::DenseMapInfo<clang::ModuleFileKey> { + static clang::ModuleFileKey getEmptyKey() { + return DenseMapInfo<const void *>::getEmptyKey(); + } + + static clang::ModuleFileKey getTombstoneKey() { + return DenseMapInfo<const void *>::getTombstoneKey(); + } + + static unsigned getHashValue(const clang::ModuleFileKey &Val) { + return hash_combine(Val.Ptr, Val.PathSuffix); + } + + static bool isEqual(const clang::ModuleFileKey &LHS, + const clang::ModuleFileKey &RHS) { + return LHS == RHS; + } +}; + #endif // LLVM_CLANG_BASIC_MODULE_H diff --git a/clang/include/clang/Frontend/CompilerInstance.h b/clang/include/clang/Frontend/CompilerInstance.h index f7a0d1064110a..0d684d5c7f9fe 100644 --- a/clang/include/clang/Frontend/CompilerInstance.h +++ b/clang/include/clang/Frontend/CompilerInstance.h @@ -17,6 +17,7 @@ #include "clang/Frontend/PCHContainerOperations.h" #include "clang/Frontend/Utils.h" #include "clang/Lex/DependencyDirectivesScanner.h" +#include "clang/Lex/HeaderSearch.h" #include "clang/Lex/HeaderSearchOptions.h" #include "clang/Lex/ModuleLoader.h" #include "llvm/ADT/ArrayRef.h" @@ -867,7 +868,7 @@ class CompilerInstance : public ModuleLoader { void createASTReader(); - bool loadModuleFile(StringRef FileName, + bool loadModuleFile(ModuleFileName FileName, serialization::ModuleFile *&LoadedModuleFile); /// Configuration object for making the result of \c cloneForModuleCompile() diff --git a/clang/include/clang/Lex/HeaderSearch.h b/clang/include/clang/Lex/HeaderSearch.h index 2bb78fbf608f3..fd52a0ad5b007 100644 --- a/clang/include/clang/Lex/HeaderSearch.h +++ b/clang/include/clang/Lex/HeaderSearch.h @@ -661,7 +661,7 @@ class HeaderSearch { /// /// \returns The name of the module file that corresponds to this module, /// or an empty string if this module does not correspond to any module file. - std::string getCachedModuleFileName(Module *Module); + ModuleFileName getCachedModuleFileName(Module *Module); /// Retrieve the name of the prebuilt module file that should be used /// to load a module with the given name. @@ -673,8 +673,8 @@ class HeaderSearch { /// /// \returns The name of the module file that corresponds to this module, /// or an empty string if this module does not correspond to any module file. - std::string getPrebuiltModuleFileName(StringRef ModuleName, - bool FileMapOnly = false); + ModuleFileName getPrebuiltModuleFileName(StringRef ModuleName, + bool FileMapOnly = false); /// Retrieve the name of the prebuilt module file that should be used /// to load the given module. @@ -683,7 +683,7 @@ class HeaderSearch { /// /// \returns The name of the module file that corresponds to this module, /// or an empty string if this module does not correspond to any module file. - std::string getPrebuiltImplicitModuleFileName(Module *Module); + ModuleFileName getPrebuiltImplicitModuleFileName(Module *Module); /// Retrieve the name of the (to-be-)cached module file that should /// be used to load a module with the given name. @@ -695,8 +695,8 @@ class HeaderSearch { /// /// \returns The name of the module file that corresponds to this module, /// or an empty string if this module does not correspond to any module file. - std::string getCachedModuleFileName(StringRef ModuleName, - StringRef ModuleMapPath); + ModuleFileName getCachedModuleFileName(StringRef ModuleName, + StringRef ModuleMapPath); /// Lookup a module Search for a module with the given name. /// @@ -812,13 +812,13 @@ class HeaderSearch { /// \param ModuleMapPath A path that when combined with \c ModuleName /// uniquely identifies this module. See Module::ModuleMap. /// - /// \param CachePath A path to the module cache. + /// \param NormalizedCachePath The normalized path to the module cache. /// /// \returns The name of the module file that corresponds to this module, /// or an empty string if this module does not correspond to any module file. - std::string getCachedModuleFileNameImpl(StringRef ModuleName, - StringRef ModuleMapPath, - StringRef CachePath); + ModuleFileName getCachedModuleFileNameImpl(StringRef ModuleName, + StringRef ModuleMapPath, + StringRef NormalizedCachePath); /// Retrieve a module with the given name, which may be part of the /// given framework. diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h index 5db0b08f877ce..4e8fe1d32d42e 100644 --- a/clang/include/clang/Serialization/ASTBitCodes.h +++ b/clang/include/clang/Serialization/ASTBitCodes.h @@ -44,7 +44,7 @@ namespace serialization { /// Version 4 of AST files also requires that the version control branch and /// revision match exactly, since there is no backward compatibility of /// AST files at this time. -const unsigned VERSION_MAJOR = 36; +const unsigned VERSION_MAJOR = 37; /// AST file minor version number supported by this version of /// Clang. diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index 14f7d8a69a1b3..e9706d0ea2f2b 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -1549,7 +1549,7 @@ class ASTReader : Mod(Mod), ImportedBy(ImportedBy), ImportLoc(ImportLoc) {} }; - ASTReadResult ReadASTCore(StringRef FileName, ModuleKind Type, + ASTReadResult ReadASTCore(ModuleFileName FileName, ModuleKind Type, SourceLocation ImportLoc, ModuleFile *ImportedBy, SmallVectorImpl<ImportedModule> &Loaded, off_t ExpectedSize, time_t ExpectedModTime, @@ -1885,7 +1885,7 @@ class ASTReader /// NewLoadedModuleFile would refer to the address of the new loaded top level /// module. The state of NewLoadedModuleFile is unspecified if the AST file /// isn't loaded successfully. - ASTReadResult ReadAST(StringRef FileName, ModuleKind Type, + ASTReadResult ReadAST(ModuleFileName FileName, ModuleKind Type, SourceLocation ImportLoc, unsigned ClientLoadCapabilities, ModuleFile **NewLoadedModuleFile = nullptr); diff --git a/clang/include/clang/Serialization/ModuleFile.h b/clang/include/clang/Serialization/ModuleFile.h index 519a74d920129..4915ad0634fcd 100644 --- a/clang/include/clang/Serialization/ModuleFile.h +++ b/clang/include/clang/Serialization/ModuleFile.h @@ -159,6 +159,9 @@ class ModuleFile { /// The file name of the module file. std::string FileName; + /// All keys ModuleManager used for the module file. + llvm::DenseSet<ModuleFileKey> Keys; + /// The name of the module. std::string ModuleName; diff --git a/clang/include/clang/Serialization/ModuleManager.h b/clang/include/clang/Serialization/ModuleManager.h index 8ab70b6630f47..162856f2f14c0 100644 --- a/clang/include/clang/Serialization/ModuleManager.h +++ b/clang/include/clang/Serialization/ModuleManager.h @@ -16,6 +16,7 @@ #include "clang/Basic/LLVM.h" #include "clang/Basic/SourceLocation.h" +#include "clang/Lex/HeaderSearch.h" #include "clang/Serialization/ModuleFile.h" #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/STLExtras.h" @@ -56,8 +57,8 @@ class ModuleManager { // to implement short-circuiting logic when running DFS over the dependencies. SmallVector<ModuleFile *, 2> Roots; - /// All loaded modules, indexed by name. - llvm::DenseMap<const FileEntry *, ModuleFile *> Modules; + /// All loaded modules. + llvm::DenseMap<ModuleFileKey, ModuleFile *> Modules; /// FileManager that handles translating between filenames and /// FileEntry *. @@ -172,14 +173,22 @@ class ModuleManager { ModuleFile &operator[](unsigned Index) const { return *Chain[Index]; } /// Returns the module associated with the given file name. + /// Soon to be removed. ModuleFile *lookupByFileName(StringRef FileName) const; /// Returns the module associated with the given module name. ModuleFile *lookupByModuleName(StringRef ModName) const; /// Returns the module associated with the given module file. + /// Soon to be removed. ModuleFile *lookup(const FileEntry *File) const; + /// Returns the module associated with the given module file name. + ModuleFile *lookupByFileName(ModuleFileName FileName) const; + + /// Returns the module associated with the given module file key. + ModuleFile *lookup(ModuleFileKey Key) const; + /// Returns the in-memory (virtual file) buffer with the given name std::unique_ptr<llvm::MemoryBuffer> lookupBuffer(StringRef Name); @@ -237,14 +246,13 @@ class ModuleManager { /// /// \return A pointer to the module that corresponds to this file name, /// and a value indicating whether the module was loaded. - AddModuleResult addModule(StringRef FileName, ModuleKind Type, - SourceLocation ImportLoc, - ModuleFile *ImportedBy, unsigned Generation, - off_t ExpectedSize, time_t ExpectedModTime, + AddModuleResult addModule(ModuleFileName FileName, ModuleKind Type, + SourceLocation ImportLoc, ModuleFile *ImportedBy, + unsigned Generation, off_t ExpectedSize, + time_t ExpectedModTime, ASTFileSignature ExpectedSignature, ASTFileSignatureReader ReadSignature, - ModuleFile *&Module, - std::string &ErrorStr); + ModuleFile *&Module, std::string &ErrorStr); /// Remove the modules starting from First (to the end). void removeModules(ModuleIterator First); @@ -282,26 +290,6 @@ class ModuleManager { void visit(llvm::function_ref<bool(ModuleFile &M)> Visitor, llvm::SmallPtrSetImpl<ModuleFile *> *ModuleFilesHit = nullptr); - /// Attempt to resolve the given module file name to a file entry. - /// - /// \param FileName The name of the module file. - /// - /// \param ExpectedSize The size that the module file is expected to have. - /// If the actual size differs, the resolver should return \c true. - /// - /// \param ExpectedModTime The modification time that the module file is - /// expected to have. If the actual modification time differs, the resolver - /// should return \c true. - /// - /// \param File Will be set to the file if there is one, or null - /// otherwise. - /// - /// \returns True if a file exists but does not meet the size/ - /// modification time criteria, false if the file is either available and - /// suitable, or is missing. - bool lookupModuleFile(StringRef FileName, off_t ExpectedSize, - time_t ExpectedModTime, OptionalFileEntryRef &File); - /// View the graphviz representation of the module graph. void viewGraph(); diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp index 97f742d292224..bfc29e32c3672 100644 --- a/clang/lib/Basic/Module.cpp +++ b/clang/lib/Basic/Module.cpp @@ -33,6 +33,23 @@ using namespace clang; +std::optional<ModuleFileKey> +ModuleFileName::makeKey(FileManager &FileMgr) const { + if (Separator) { + StringRef ModuleCachePath = StringRef(Path).substr(0, *Separator); + StringRef ModuleFilePathSuffix = StringRef(Path).substr(*Separator); + if (auto ModuleCache = FileMgr.getOptionalDirectoryRef( + ModuleCachePath, /*CacheFailure=*/false)) + return ModuleFileKey(*ModuleCache, ModuleFilePathSuffix); + } else { + if (auto ModuleFile = FileMgr.getOptionalFileRef(Path, /*OpenFile=*/true, + /*CacheFailure=*/false)) + return ModuleFileKey(*ModuleFile); + } + + return std::nullopt; +} + Module::Module(ModuleConstructorTag, StringRef Name, SourceLocation DefinitionLoc, Module *Parent, bool IsFramework, bool IsExplicit, unsigned VisibilityID) diff --git a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp index a38ee690d42de..f982e2328537c 100644 --- a/clang/lib/DependencyScanning/DependencyScannerImpl.cpp +++ b/clang/lib/DependencyScanning/DependencyScannerImpl.cpp @@ -607,7 +607,7 @@ struct AsyncModuleCompile : PPCallbacks { HeaderSearch &HS = CI.getPreprocessor().getHeaderSearchInfo(); ModuleCache &ModCache = CI.getModuleCache(); - std::string ModuleFileName = HS.getCachedModuleFileName(M); + ModuleFileName ModuleFileName = HS.getCachedModuleFileName(M); uint64_t Timestamp = ModCache.getModuleTimestamp(ModuleFileName); // Someone else already built/validated the PCM. diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp index 1d249ebaa1492..32769e14a3fcc 100644 --- a/clang/lib/Frontend/ASTUnit.cpp +++ b/clang/lib/Frontend/ASTUnit.cpp @@ -835,8 +835,9 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromASTFile( AST->HSOpts->ForceCheckCXX20ModulesInputFiles = HSOpts.ForceCheckCXX20ModulesInputFiles; - switch (AST->Reader->ReadAST(Filename, serialization::MK_MainFile, - SourceLocation(), ASTReader::ARR_None)) { + switch (AST->Reader->ReadAST(ModuleFileName::make_explicit(Filename), + serialization::MK_MainFile, SourceLocation(), + ASTReader::ARR_None)) { case ASTReader::Success: break; diff --git a/clang/lib/Frontend/ChainedIncludesSource.cpp b/clang/lib/Frontend/ChainedIncludesSource.cpp index 2338698cee9d1..1d853da65f1ca 100644 --- a/clang/lib/Frontend/ChainedIncludesSource.cpp +++ b/clang/lib/Frontend/ChainedIncludesSource.cpp @@ -69,7 +69,8 @@ createASTReader(CompilerInstance &CI, StringRef pchFile, Reader->addInMemoryBuffer(sr, std::move(MemBufs[ti])); } Reader->setDeserializationListener(deserialListener); - switch (Reader->ReadAST(pchFile, serialization::MK_PCH, SourceLocation(), + switch (Reader->ReadAST(ModuleFileName::make_explicit(pchFile), + serialization::MK_PCH, SourceLocation(), ASTReader::ARR_None)) { case ASTReader::Success: // Set the predefines buffer as suggested by the PCH reader. diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 4c7a5a66254a5..6aa24d380031e 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -655,11 +655,10 @@ IntrusiveRefCntPtr<ASTReader> CompilerInstance::createPCHExternalASTSource( ASTReader::ListenerScope ReadModuleNamesListener(*Reader, std::move(Listener)); - switch (Reader->ReadAST(Path, + switch (Reader->ReadAST(ModuleFileName::make_explicit(Path), Preamble ? serialization::MK_Preamble : serialization::MK_PCH, - SourceLocation(), - ASTReader::ARR_None)) { + SourceLocation(), ASTReader::ARR_None)) { case ASTReader::Success: // Set the predefines buffer as suggested by the PCH reader. Typically, the // predefines buffer will be empty. @@ -1393,7 +1392,8 @@ std::unique_ptr<CompilerInstance> CompilerInstance::cloneForModuleCompile( static bool readASTAfterCompileModule(CompilerInstance &ImportingInstance, SourceLocation ImportLoc, SourceLocation ModuleNameLoc, - Module *Module, StringRef ModuleFileName, + Module *Module, + ModuleFileName ModuleFileName, bool *OutOfDate, bool *Missing) { DiagnosticsEngine &Diags = ImportingInstance.getDiagnostics(); @@ -1435,7 +1435,7 @@ static bool compileModuleAndReadASTImpl(CompilerInstance &ImportingInstance, SourceLocation ImportLoc, SourceLocation ModuleNameLoc, Module *Module, - StringRef ModuleFileName) { + ModuleFileName ModuleFileName) { { auto Instance = ImportingInstance.cloneForModuleCompile( ModuleNameLoc, Module, ModuleFileName); @@ -1471,9 +1471,11 @@ static bool compileModuleAndReadASTImpl(CompilerInstance &ImportingInstance, /// multiple instances will compete to create the same module. On timeout, /// deletes the lock file in order to avoid deadlock from crashing processes or /// bugs in the lock file manager. -static bool compileModuleAndReadASTBehindLock( - CompilerInstance &ImportingInstance, SourceLocation ImportLoc, - SourceLocation ModuleNameLoc, Module *Module, StringRef ModuleFileName) { +static bool +compileModuleAndReadASTBehindLock(CompilerInstance &ImportingInstance, + SourceLocation ImportLoc, + SourceLocation ModuleNameLoc, Module *Module, + ModuleFileName ModuleFileName) { DiagnosticsEngine &Diags = ImportingInstance.getDiagnostics(); Diags.Report(ModuleNameLoc, diag::remark_module_lock) @@ -1543,7 +1545,8 @@ static bool compileModuleAndReadASTBehindLock( static bool compileModuleAndReadAST(CompilerInstance &ImportingInstance, SourceLocation ImportLoc, SourceLocation ModuleNameLoc, - Module *Module, StringRef ModuleFileName) { + Module *Module, + ModuleFileName ModuleFileName) { return ImportingInstance.getInvocation() .getFrontendOpts() .BuildingImplicitModuleUsesLock @@ -1682,11 +1685,11 @@ void CompilerInstance::createASTReader() { } bool CompilerInstance::loadModuleFile( - StringRef FileName, serialization::ModuleFile *&LoadedModuleFile) { + ModuleFileName FileName, serialization::ModuleFile *&LoadedModuleFile) { llvm::Timer Timer; if (timerGroup) - Timer.init("preloading." + FileName.str(), "Preloading " + FileName.str(), - *timerGroup); + Timer.init("preloading." + std::string(FileName.str()), + "Preloading " + std::string(FileName.str()), *timerGroup); llvm::TimeRegion TimeLoading(timerGroup ? &Timer : nullptr); // If we don't already have an ASTReader, create one now. @@ -1743,7 +1746,7 @@ enum ModuleSource { /// Select a source for loading the named module and compute the filename to /// load it from. static ModuleSource selectModuleSource( - Module *M, StringRef ModuleName, std::string &ModuleFilename, + Module *M, StringRef ModuleName, ModuleFileName &ModuleFilename, const std::map<std::string, std::string, std::less<>> &BuiltModules, HeaderSearch &HS) { assert(ModuleFilename.empty() && "Already has a module source?"); @@ -1752,7 +1755,7 @@ static ModuleSource selectModuleSource( // via a module build pragma. auto BuiltModuleIt = BuiltModules.find(ModuleName); if (BuiltModuleIt != BuiltModules.end()) { - ModuleFilename = BuiltModuleIt->second; + ModuleFilename = ModuleFileName::make_explicit(BuiltModuleIt->second); return MS_ModuleBuildPragma; } @@ -1760,9 +1763,9 @@ static ModuleSource selectModuleSource( const HeaderSearchOptions &HSOpts = HS.getHeaderSearchOpts(); if (!HSOpts.PrebuiltModuleFiles.empty() || !HSOpts.PrebuiltModulePaths.empty()) { - ModuleFilename = HS.getPrebuiltModuleFileName(ModuleName); + ModuleFilename = ModuleFileName(HS.getPrebuiltModuleFileName(ModuleName)); if (HSOpts.EnablePrebuiltImplicitModules && ModuleFilename.empty()) - ModuleFilename = HS.getPrebuiltImplicitModuleFileName(M); + ModuleFilename = ModuleFileName(HS.getPrebuiltImplicitModuleFileName(M)); if (!ModuleFilename.empty()) return MS_PrebuiltModulePath; } @@ -1792,7 +1795,7 @@ ModuleLoadResult CompilerInstance::findOrCompileModuleAndReadAST( checkConfigMacros(getPreprocessor(), M, ImportLoc); // Select the source and filename for loading the named module. - std::string ModuleFilename; + ModuleFileName ModuleFilename; ModuleSource Source = selectModuleSource(M, ModuleName, ModuleFilename, BuiltModules, HS); SourceLocation ModuleNameLoc = ModuleNameRange.getBegin(); @@ -1821,8 +1824,8 @@ ModuleLoadResult CompilerInstance::findOrCompileModuleAndReadAST( // Time how long it takes to load the module. llvm::Timer Timer; if (timerGroup) - Timer.init("loading." + ModuleFilename, "Loading " + ModuleFilename, - *timerGroup); + Timer.init("loading." + std::string(ModuleFilename.str()), + "Loading " + std::string(ModuleFilename.str()), *timerGroup); llvm::TimeRegion TimeLoading(timerGroup ? &Timer : nullptr); llvm::TimeTraceScope TimeScope("Module Load", ModuleName); @@ -2042,7 +2045,7 @@ CompilerInstance::loadModule(SourceLocation ImportLoc, PrivateModule, PP->getIdentifierInfo(Module->Name)->getTokenID()); PrivPath.emplace_back(Path[0].getLoc(), &II); - std::string FileName; + ModuleFileName FileName; // If there is a modulemap module or prebuilt module, load it. if (PP->getHeaderSearchInfo().lookupModule(PrivateModule, ImportLoc, true, !IsInclusionDirective) || diff --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp index 84ceb22208801..b82296c9c52f9 100644 --- a/clang/lib/Frontend/FrontendAction.cpp +++ b/clang/lib/Frontend/FrontendAction.cpp @@ -1288,7 +1288,7 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI, // If we were asked to load any module files, do so now. for (const auto &ModuleFile : CI.getFrontendOpts().ModuleFiles) { serialization::ModuleFile *Loaded = nullptr; - if (!CI.loadModuleFile(ModuleFile, Loaded)) + if (!CI.loadModuleFile(ModuleFileName::make_explicit(ModuleFile), Loaded)) return false; if (Loaded && Loaded->StandardCXXModule) diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp index 15d19dae966ac..e371c68162b31 100644 --- a/clang/lib/Frontend/FrontendActions.cpp +++ b/clang/lib/Frontend/FrontendActions.cpp @@ -251,9 +251,9 @@ GenerateModuleFromModuleMapAction::CreateOutputFile(CompilerInstance &CI, ModuleMapFile = InFile; HeaderSearch &HS = CI.getPreprocessor().getHeaderSearchInfo(); - CI.getFrontendOpts().OutputFile = - HS.getCachedModuleFileName(CI.getLangOpts().CurrentModule, - ModuleMapFile); + ModuleFileName FileName = HS.getCachedModuleFileName( + CI.getLangOpts().CurrentModule, ModuleMapFile); + CI.getFrontendOpts().OutputFile = FileName.str(); } // Because this is exposed via libclang we must disable RemoveFileOnSignal. @@ -367,11 +367,9 @@ void VerifyPCHAction::ExecuteAction() { /*AllowConfigurationMismatch*/ true, /*ValidateSystemInputs*/ true, /*ForceValidateUserInputs*/ true)); - Reader->ReadAST(getCurrentFile(), - Preamble ? serialization::MK_Preamble - : serialization::MK_PCH, - SourceLocation(), - ASTReader::ARR_ConfigurationMismatch); + Reader->ReadAST(ModuleFileName::make_explicit(getCurrentFile()), + Preamble ? serialization::MK_Preamble : serialization::MK_PCH, + SourceLocation(), ASTReader::ARR_ConfigurationMismatch); } namespace { diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp index de26b999683d1..8d6bed45f5e45 100644 --- a/clang/lib/Lex/HeaderSearch.cpp +++ b/clang/lib/Lex/HeaderSearch.cpp @@ -197,7 +197,7 @@ void HeaderSearch::getHeaderMapFileNames( Names.push_back(std::string(HM.first.getName())); } -std::string HeaderSearch::getCachedModuleFileName(Module *Module) { +ModuleFileName HeaderSearch::getCachedModuleFileName(Module *Module) { OptionalFileEntryRef ModuleMap = getModuleMap().getModuleMapFileForUniquing(Module); // The ModuleMap maybe a nullptr, when we load a cached C++ module without @@ -207,12 +207,12 @@ std::string HeaderSearch::getCachedModuleFileName(Module *Module) { return getCachedModuleFileName(Module->Name, ModuleMap->getNameAsRequested()); } -std::string HeaderSearch::getPrebuiltModuleFileName(StringRef ModuleName, - bool FileMapOnly) { +ModuleFileName HeaderSearch::getPrebuiltModuleFileName(StringRef ModuleName, + bool FileMapOnly) { // First check the module name to pcm file map. auto i(HSOpts.PrebuiltModuleFiles.find(ModuleName)); if (i != HSOpts.PrebuiltModuleFiles.end()) - return i->second; + return ModuleFileName::make_explicit(i->second); if (FileMapOnly || HSOpts.PrebuiltModulePaths.empty()) return {}; @@ -232,49 +232,52 @@ std::string HeaderSearch::getPrebuiltModuleFileName(StringRef ModuleName, else llvm::sys::path::append(Result, ModuleName + ".pcm"); if (getFileMgr().getOptionalFileRef(Result)) - return std::string(Result); + return ModuleFileName::make_explicit(Result); } return {}; } -std::string HeaderSearch::getPrebuiltImplicitModuleFileName(Module *Module) { +ModuleFileName HeaderSearch::getPrebuiltImplicitModuleFileName(Module *Module) { OptionalFileEntryRef ModuleMap = getModuleMap().getModuleMapFileForUniquing(Module); StringRef ModuleName = Module->Name; StringRef ModuleMapPath = ModuleMap->getName(); - StringRef ContextHash = HSOpts.DisableModuleHash ? "" : getContextHash(); for (const std::string &Dir : HSOpts.PrebuiltModulePaths) { SmallString<256> CachePath(Dir); FileMgr.makeAbsolutePath(CachePath); - llvm::sys::path::append(CachePath, ContextHash); - std::string FileName = + ModuleFileName FileName = getCachedModuleFileNameImpl(ModuleName, ModuleMapPath, CachePath); if (!FileName.empty() && getFileMgr().getOptionalFileRef(FileName)) - return FileName; + return ModuleFileName::make_explicit(FileName); } return {}; } -std::string HeaderSearch::getCachedModuleFileName(StringRef ModuleName, - StringRef ModuleMapPath) { +ModuleFileName HeaderSearch::getCachedModuleFileName(StringRef ModuleName, + StringRef ModuleMapPath) { return getCachedModuleFileNameImpl(ModuleName, ModuleMapPath, - getSpecificModuleCachePath()); + getNormalizedModuleCachePath()); } -std::string HeaderSearch::getCachedModuleFileNameImpl(StringRef ModuleName, - StringRef ModuleMapPath, - StringRef CachePath) { +ModuleFileName HeaderSearch::getCachedModuleFileNameImpl( + StringRef ModuleName, StringRef ModuleMapPath, StringRef CachePath) { // If we don't have a module cache path or aren't supposed to use one, we // can't do anything. if (CachePath.empty()) return {}; + // Note: This re-implements part of createSpecificModuleCachePathImpl() in + // order to be able to correctly construct ModuleFileName. + SmallString<256> Result(CachePath); + unsigned Separator = CachePath.size(); if (HSOpts.DisableModuleHash) { llvm::sys::path::append(Result, ModuleName + ".pcm"); } else { + llvm::sys::path::append(Result, ContextHash); + // Construct the name <ModuleName>-<hash of ModuleMapPath>.pcm which should // ideally be globally unique to this particular module. Name collisions // in the hash are safe (because any translation unit can only import one @@ -292,7 +295,7 @@ std::string HeaderSearch::getCachedModuleFileNameImpl(StringRef ModuleName, llvm::APInt(64, Hash).toStringUnsigned(HashStr, /*Radix*/36); llvm::sys::path::append(Result, ModuleName + "-" + HashStr + ".pcm"); } - return Result.str().str(); + return ModuleFileName::make_implicit(Result, Separator); } Module *HeaderSearch::lookupModule(StringRef ModuleName, diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 812f27989c129..637cbc02c085d 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -3475,8 +3475,9 @@ ASTReader::ReadControlBlock(ModuleFile &F, off_t StoredSize = 0; time_t StoredModTime = 0; + unsigned ModuleCacheLen = 0; ASTFileSignature StoredSignature; - std::string ImportedFile; + ModuleFileName ImportedFile; std::string StoredFile; bool IgnoreImportedByNote = false; @@ -3498,17 +3499,23 @@ ASTReader::ReadControlBlock(ModuleFile &F, if (!IsImportingStdCXXModule) { StoredSize = (off_t)Record[Idx++]; StoredModTime = (time_t)Record[Idx++]; + ModuleCacheLen = (unsigned)Record[Idx++]; StringRef SignatureBytes = Blob.substr(0, ASTFileSignature::size); StoredSignature = ASTFileSignature::create(SignatureBytes.begin(), SignatureBytes.end()); Blob = Blob.substr(ASTFileSignature::size); - // Use BaseDirectoryAsWritten to ensure we use the same path in the - // ModuleCache as when writing. - StoredFile = ReadPathBlob(BaseDirectoryAsWritten, Record, Idx, Blob); + StoredFile = ReadStringBlob(Record, Idx, Blob); if (ImportedFile.empty()) { - ImportedFile = StoredFile; + if (ImportedKind == MK_ImplicitModule) { + assert(ModuleCacheLen != 0); + ImportedFile = + ModuleFileName::make_implicit(StoredFile, ModuleCacheLen); + } else { + assert(ModuleCacheLen == 0); + ImportedFile = ModuleFileName::make_explicit(StoredFile); + } } else if (!getDiags().isIgnored( diag::warn_module_file_mapping_mismatch, CurrentImportLoc)) { @@ -4911,7 +4918,8 @@ static bool SkipCursorToBlock(BitstreamCursor &Cursor, unsigned BlockID) { } } -ASTReader::ASTReadResult ASTReader::ReadAST(StringRef FileName, ModuleKind Type, +ASTReader::ASTReadResult ASTReader::ReadAST(ModuleFileName FileName, + ModuleKind Type, SourceLocation ImportLoc, unsigned ClientLoadCapabilities, ModuleFile **NewLoadedModuleFile) { @@ -5187,15 +5195,11 @@ static unsigned moduleKindForDiagnostic(ModuleKind Kind) { llvm_unreachable("unknown module kind"); } -ASTReader::ASTReadResult -ASTReader::ReadASTCore(StringRef FileName, - ModuleKind Type, - SourceLocation ImportLoc, - ModuleFile *ImportedBy, - SmallVectorImpl<ImportedModule> &Loaded, - off_t ExpectedSize, time_t ExpectedModTime, - ASTFileSignature ExpectedSignature, - unsigned ClientLoadCapabilities) { +ASTReader::ASTReadResult ASTReader::ReadASTCore( + ModuleFileName FileName, ModuleKind Type, SourceLocation ImportLoc, + ModuleFile *ImportedBy, SmallVectorImpl<ImportedModule> &Loaded, + off_t ExpectedSize, time_t ExpectedModTime, + ASTFileSignature ExpectedSignature, unsigned ClientLoadCapabilities) { ModuleFile *M; std::string ErrorStr; ModuleManager::AddModuleResult AddResult @@ -6141,14 +6145,13 @@ bool ASTReader::readASTFileControlBlock( continue; } - // Skip Size and ModTime. - Idx += 1 + 1; + // Skip Size, ModTime and ModuleCacheLen. + Idx += 1 + 1 + 1; // Skip signature. Blob = Blob.substr(ASTFileSignature::size); - StringRef FilenameStr = ReadStringBlob(Record, Idx, Blob); - auto Filename = ResolveImportedPath(PathBuf, FilenameStr, ModuleDir); - Listener.visitImport(ModuleName, *Filename); + StringRef Filename = ReadStringBlob(Record, Idx, Blob); + Listener.visitImport(ModuleName, Filename); break; } diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 2744d70b89aac..eb310f30078b0 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -1567,6 +1567,7 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, StringRef isysroot) { Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // Standard C++ mod Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // File size Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // File timestamp + Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Module cache len Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // File name len Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // Strings unsigned AbbrevCode = Stream.EmitAbbrev(std::move(Abbrev)); @@ -1593,6 +1594,7 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, StringRef isysroot) { Record.push_back(0); Record.push_back(0); Record.push_back(0); + Record.push_back(0); } else { // If we have calculated signature, there is no need to store // the size or timestamp. @@ -1601,7 +1603,15 @@ void ASTWriter::WriteControlBlock(Preprocessor &PP, StringRef isysroot) { llvm::append_range(Blob, M.Signature); - AddPathBlob(M.FileName, Record, Blob); + StringRef NormalizedModuleCache = + PP.getHeaderSearchInfo().getNormalizedModuleCachePath(); + unsigned ModuleCacheLen = 0; + if (M.Kind == MK_ImplicitModule && + StringRef(M.FileName).starts_with(NormalizedModuleCache)) + ModuleCacheLen = NormalizedModuleCache.size(); + Record.push_back(ModuleCacheLen); + + AddStringBlob(M.FileName, Record, Blob); } Stream.EmitRecordWithBlob(AbbrevCode, Record, Blob); diff --git a/clang/lib/Serialization/GlobalModuleIndex.cpp b/clang/lib/Serialization/GlobalModuleIndex.cpp index 2246a3ac0a57e..f173441c6e81b 100644 --- a/clang/lib/Serialization/GlobalModuleIndex.cpp +++ b/clang/lib/Serialization/GlobalModuleIndex.cpp @@ -638,6 +638,7 @@ llvm::Error GlobalModuleIndexBuilder::loadModuleFile(FileEntryRef File) { // Load stored size/modification time. off_t StoredSize = (off_t)Record[Idx++]; time_t StoredModTime = (time_t)Record[Idx++]; + (void)Record[Idx++]; // ModuleCacheLen // Skip the stored signature. // FIXME: we could read the signature out of the import and validate it. diff --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp index 16c3a1e04f649..bfb42a194a604 100644 --- a/clang/lib/Serialization/ModuleManager.cpp +++ b/clang/lib/Serialization/ModuleManager.cpp @@ -58,7 +58,16 @@ ModuleFile *ModuleManager::lookupByModuleName(StringRef Name) const { } ModuleFile *ModuleManager::lookup(const FileEntry *File) const { - return Modules.lookup(File); + return lookup(ModuleFileKey(File)); +} + +ModuleFile *ModuleManager::lookupByFileName(ModuleFileName Name) const { + std::optional<ModuleFileKey> Key = Name.makeKey(FileMgr); + return Key ? lookup(*Key) : nullptr; +} + +ModuleFile *ModuleManager::lookup(ModuleFileKey Key) const { + return Modules.lookup(Key); } std::unique_ptr<llvm::MemoryBuffer> @@ -70,6 +79,23 @@ ModuleManager::lookupBuffer(StringRef Name) { return std::move(InMemoryBuffers[*Entry]); } +static bool checkModuleFile(const FileEntry *File, off_t ExpectedSize, + time_t ExpectedModTime, std::string &ErrorStr) { + assert(File && "Checking expectations of a non-existent module file"); + + if (ExpectedSize && ExpectedSize != File->getSize()) { + ErrorStr = "module file has a different size than expected"; + return true; + } + + if (ExpectedModTime && ExpectedModTime != File->getModificationTime()) { + ErrorStr = "module file has a different modification time than expected"; + return true; + } + + return false; +} + static bool checkSignature(ASTFileSignature Signature, ASTFileSignature ExpectedSignature, std::string &ErrorStr) { @@ -94,24 +120,18 @@ static void updateModuleImports(ModuleFile &MF, ModuleFile *ImportedBy, } } -ModuleManager::AddModuleResult -ModuleManager::addModule(StringRef FileName, ModuleKind Type, - SourceLocation ImportLoc, ModuleFile *ImportedBy, - unsigned Generation, - off_t ExpectedSize, time_t ExpectedModTime, - ASTFileSignature ExpectedSignature, - ASTFileSignatureReader ReadSignature, - ModuleFile *&Module, - std::string &ErrorStr) { +ModuleManager::AddModuleResult ModuleManager::addModule( + ModuleFileName FileName, ModuleKind Type, SourceLocation ImportLoc, + ModuleFile *ImportedBy, unsigned Generation, off_t ExpectedSize, + time_t ExpectedModTime, ASTFileSignature ExpectedSignature, + ASTFileSignatureReader ReadSignature, ModuleFile *&Module, + std::string &ErrorStr) { Module = nullptr; uint64_t InputFilesValidationTimestamp = 0; if (Type == MK_ImplicitModule) InputFilesValidationTimestamp = ModCache.getModuleTimestamp(FileName); - // Look for the file entry. This only fails if the expected size or - // modification time differ. - OptionalFileEntryRef Entry; bool IgnoreModTime = Type == MK_ExplicitModule || Type == MK_PrebuiltModule; if (ImportedBy) IgnoreModTime &= ImportedBy->Kind == MK_ExplicitModule || @@ -123,91 +143,84 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type, // contents, but we can't check that.) ExpectedModTime = 0; } - // Note: ExpectedSize and ExpectedModTime will be 0 for MK_ImplicitModule - // when using an ASTFileSignature. - if (lookupModuleFile(FileName, ExpectedSize, ExpectedModTime, Entry)) { - ErrorStr = IgnoreModTime ? "module file has a different size than expected" - : "module file has a different size or " - "modification time than expected"; - return OutOfDate; - } - if (!Entry) { - ErrorStr = "module file not found"; + std::optional<ModuleFileKey> FileKey = FileName.makeKey(FileMgr); + if (!FileKey) return Missing; + + ModuleFile *ModuleEntry = lookup(*FileKey); + if (ModuleEntry) + ModuleEntry->Keys.insert(*FileKey); + + // TODO: Remove this. + if (!ModuleEntry) { + // Try looking up with the plain file name. + auto FileName2 = ModuleFileName::make_explicit(FileName); + std::optional<ModuleFileKey> FileKey2 = FileName2.makeKey(FileMgr); + if (!FileKey2) + return Missing; + ModuleEntry = lookup(*FileKey2); + if (ModuleEntry) + ModuleEntry->Keys.insert(*FileKey2); } - // The ModuleManager's use of FileEntry nodes as the keys for its map of - // loaded modules is less than ideal. Uniqueness for FileEntry nodes is - // maintained by FileManager, which in turn uses inode numbers on hosts - // that support that. When coupled with the module cache's proclivity for - // turning over and deleting stale PCMs, this means entries for different - // module files can wind up reusing the same underlying inode. When this - // happens, subsequent accesses to the Modules map will disagree on the - // ModuleFile associated with a given file. In general, it is not sufficient - // to resolve this conundrum with a type like FileEntryRef that stores the - // name of the FileEntry node on first access because of path canonicalization - // issues. However, the paths constructed for implicit module builds are - // fully under Clang's control. We *can*, therefore, rely on their structure - // being consistent across operating systems and across subsequent accesses - // to the Modules map. - auto implicitModuleNamesMatch = [](ModuleKind Kind, const ModuleFile *MF, - FileEntryRef Entry) -> bool { - if (Kind != MK_ImplicitModule) - return true; - return Entry.getName() == MF->FileName; - }; + if (ModuleEntry) { + // Check file properties. + if (checkModuleFile(ModuleEntry->File, ExpectedSize, ExpectedModTime, + ErrorStr)) + return OutOfDate; - // Check whether we already loaded this module, before - if (ModuleFile *ModuleEntry = Modules.lookup(*Entry)) { - if (implicitModuleNamesMatch(Type, ModuleEntry, *Entry)) { - // Check the stored signature. - if (checkSignature(ModuleEntry->Signature, ExpectedSignature, ErrorStr)) - return OutOfDate; + // Check the stored signature. + if (checkSignature(ModuleEntry->Signature, ExpectedSignature, ErrorStr)) + return OutOfDate; - Module = ModuleEntry; - updateModuleImports(*ModuleEntry, ImportedBy, ImportLoc); - return AlreadyLoaded; - } + Module = ModuleEntry; + updateModuleImports(*ModuleEntry, ImportedBy, ImportLoc); + return AlreadyLoaded; } - // Allocate a new module. - auto NewModule = std::make_unique<ModuleFile>(Type, *Entry, Generation); - NewModule->Index = Chain.size(); - NewModule->FileName = FileName.str(); - NewModule->ImportLoc = ImportLoc; - NewModule->InputFilesValidationTimestamp = InputFilesValidationTimestamp; - // Load the contents of the module + OptionalFileEntryRef Entry; + llvm::MemoryBuffer *ModuleBuffer = nullptr; std::unique_ptr<llvm::MemoryBuffer> NewFileBuffer = nullptr; if (std::unique_ptr<llvm::MemoryBuffer> Buffer = lookupBuffer(FileName)) { // The buffer was already provided for us. - NewModule->Buffer = &getModuleCache().getInMemoryModuleCache().addBuiltPCM( + ModuleBuffer = &getModuleCache().getInMemoryModuleCache().addBuiltPCM( FileName, std::move(Buffer)); - // Since the cached buffer is reused, it is safe to close the file - // descriptor that was opened while stat()ing the PCM in - // lookupModuleFile() above, it won't be needed any longer. - Entry->closeFile(); } else if (llvm::MemoryBuffer *Buffer = getModuleCache().getInMemoryModuleCache().lookupPCM( FileName)) { - NewModule->Buffer = Buffer; - // As above, the file descriptor is no longer needed. - Entry->closeFile(); + ModuleBuffer = Buffer; } else if (getModuleCache().getInMemoryModuleCache().shouldBuildPCM( FileName)) { // Report that the module is out of date, since we tried (and failed) to // import it earlier. - Entry->closeFile(); return OutOfDate; } else { + Entry = + expectedToOptional(FileName == StringRef("-") + ? FileMgr.getSTDIN() + : FileMgr.getFileRef(FileName, /*OpenFile=*/true, + /*CacheFailure=*/false)); + if (!Entry) { + ErrorStr = "module file not found"; + return Missing; + } + + // FIXME: Consider moving this after this else branch so that we check + // size/mtime expectations even when pulling the module file out of the + // in-memory module cache or the provided in-memory buffers. + // Check file properties. + if (checkModuleFile(*Entry, ExpectedSize, ExpectedModTime, ErrorStr)) + return OutOfDate; + // Get a buffer of the file and close the file descriptor when done. // The file is volatile because in a parallel build we expect multiple // compiler processes to use the same module file rebuilding it if needed. // // RequiresNullTerminator is false because module files don't need it, and // this allows the file to still be mmapped. - auto Buf = FileMgr.getBufferForFile(NewModule->File, + auto Buf = FileMgr.getBufferForFile(*Entry, /*IsVolatile=*/true, /*RequiresNullTerminator=*/false); @@ -217,11 +230,26 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type, } NewFileBuffer = std::move(*Buf); - NewModule->Buffer = NewFileBuffer.get(); + ModuleBuffer = NewFileBuffer.get(); } + if (!Entry) { + // Unless we loaded the buffer from a freshly open file (else branch above), + // we don't have any FileEntry for this ModuleFile. Make one up. + // FIXME: Make it so that ModuleFile is not tied to a FileEntry. + Entry = FileMgr.getVirtualFileRef(FileName, ExpectedSize, ExpectedModTime); + } + + // Allocate a new module. + auto NewModule = std::make_unique<ModuleFile>(Type, *Entry, Generation); + NewModule->Index = Chain.size(); + NewModule->FileName = FileName.str(); + NewModule->ImportLoc = ImportLoc; + NewModule->InputFilesValidationTimestamp = InputFilesValidationTimestamp; + NewModule->Buffer = ModuleBuffer; // Initialize the stream. NewModule->Data = PCHContainerRdr.ExtractPCH(*NewModule->Buffer); + NewModule->Keys.insert(*FileKey); // Read the signature eagerly now so that we can check it. Avoid calling // ReadSignature unless there's something to check though. @@ -233,8 +261,13 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type, getModuleCache().getInMemoryModuleCache().addPCM(FileName, std::move(NewFileBuffer)); - // We're keeping this module. Store it everywhere. - Module = Modules[*Entry] = NewModule.get(); + // We're keeping this module. Store it in the map. + Module = Modules[*FileKey] = NewModule.get(); + + // Support clients that still rely on being able to look up ModuleFile with + // normal FileEntry. + // TODO: Remove this. + Modules[ModuleFileKey(*Entry)] = Module; updateModuleImports(*NewModule, ImportedBy, ImportLoc); @@ -279,8 +312,11 @@ void ModuleManager::removeModules(ModuleIterator First) { } // Delete the modules. - for (ModuleIterator victim = First; victim != Last; ++victim) - Modules.erase(victim->File); + for (ModuleIterator victim = First; victim != Last; ++victim) { + Modules.erase(ModuleFileKey(victim->File)); + for (ModuleFileKey Key : victim->Keys) + Modules.erase(Key); + } Chain.erase(Chain.begin() + (First - begin()), Chain.end()); } @@ -439,29 +475,6 @@ void ModuleManager::visit(llvm::function_ref<bool(ModuleFile &M)> Visitor, returnVisitState(std::move(State)); } -bool ModuleManager::lookupModuleFile(StringRef FileName, off_t ExpectedSize, - time_t ExpectedModTime, - OptionalFileEntryRef &File) { - if (FileName == "-") { - File = expectedToOptional(FileMgr.getSTDIN()); - return false; - } - - // Open the file immediately to ensure there is no race between stat'ing and - // opening the file. - File = FileMgr.getOptionalFileRef(FileName, /*OpenFile=*/true, - /*CacheFailure=*/false); - - if (File && - ((ExpectedSize && ExpectedSize != File->getSize()) || - (ExpectedModTime && ExpectedModTime != File->getModificationTime()))) - // Do not destroy File, as it may be referenced. If we need to rebuild it, - // it will be destroyed by removeModules. - return true; - - return false; -} - #ifndef NDEBUG namespace llvm { diff --git a/clang/test/Modules/DebugInfoNamespace.cpp b/clang/test/Modules/DebugInfoNamespace.cpp index b2095adf68675..03523b2e69e59 100644 --- a/clang/test/Modules/DebugInfoNamespace.cpp +++ b/clang/test/Modules/DebugInfoNamespace.cpp @@ -3,7 +3,7 @@ // RUN: %clang_cc1 -x objective-c++ -std=c++11 -debug-info-kind=standalone \ // RUN: -dwarf-ext-refs -fmodules \ // RUN: -fmodule-format=obj -fimplicit-module-maps \ -// RUN: -triple %itanium_abi_triple -fmodules-cache-path=%t \ +// RUN: -triple %itanium_abi_triple -fmodules-cache-path=%t/cache \ // RUN: %s -I %S/Inputs/DebugInfoNamespace -I %t -emit-llvm -o - \ // RUN: | FileCheck %s diff --git a/clang/test/Modules/ExtDebugInfo.cpp b/clang/test/Modules/ExtDebugInfo.cpp index 184973bc1783c..0c49f8bad8668 100644 --- a/clang/test/Modules/ExtDebugInfo.cpp +++ b/clang/test/Modules/ExtDebugInfo.cpp @@ -7,7 +7,7 @@ // RUN: -dwarf-ext-refs -fmodules \ // RUN: -fmodule-format=obj -fimplicit-module-maps -DMODULES \ // RUN: -triple %itanium_abi_triple \ -// RUN: -fmodules-cache-path=%t %s -I %S/Inputs -I %t -emit-llvm -o %t-mod.ll +// RUN: -fmodules-cache-path=%t/cache %s -I %S/Inputs -I %t -emit-llvm -o %t-mod.ll // RUN: cat %t-mod.ll | FileCheck %s // PCH: diff --git a/clang/test/Modules/ExtDebugInfo.m b/clang/test/Modules/ExtDebugInfo.m index e2611ae530063..5b19be7bf370c 100644 --- a/clang/test/Modules/ExtDebugInfo.m +++ b/clang/test/Modules/ExtDebugInfo.m @@ -5,7 +5,7 @@ // Modules: // RUN: %clang_cc1 -x objective-c -debug-info-kind=limited -dwarf-ext-refs -fmodules \ // RUN: -fmodule-format=obj -fimplicit-module-maps -DMODULES \ -// RUN: -fmodules-cache-path=%t %s -I %S/Inputs -I %t -emit-llvm -o %t-mod.ll +// RUN: -fmodules-cache-path=%t/cache %s -I %S/Inputs -I %t -emit-llvm -o %t-mod.ll // RUN: cat %t-mod.ll | FileCheck %s // RUN: cat %t-mod.ll | FileCheck %s --check-prefix=DWOID diff --git a/clang/test/Modules/ModuleDebugInfo.cpp b/clang/test/Modules/ModuleDebugInfo.cpp index 4d78324867bcb..720444802ddd3 100644 --- a/clang/test/Modules/ModuleDebugInfo.cpp +++ b/clang/test/Modules/ModuleDebugInfo.cpp @@ -6,7 +6,7 @@ // Modules: // RUN: rm -rf %t -// RUN: %clang_cc1 -triple %itanium_abi_triple -x objective-c++ -std=c++11 -debugger-tuning=lldb -debug-info-kind=limited -dwarf-version=5 -fmodules -fmodule-format=obj -fimplicit-module-maps -DMODULES -fmodules-cache-path=%t %s -I %S/Inputs -I %t -emit-llvm -o %t.ll -mllvm -debug-only=pchcontainer &>%t-mod.ll +// RUN: %clang_cc1 -triple %itanium_abi_triple -x objective-c++ -std=c++11 -debugger-tuning=lldb -debug-info-kind=limited -dwarf-version=5 -fmodules -fmodule-format=obj -fimplicit-module-maps -DMODULES -fmodules-cache-path=%t/cache %s -I %S/Inputs -I %t -emit-llvm -o %t.ll -mllvm -debug-only=pchcontainer &>%t-mod.ll // RUN: cat %t-mod.ll | FileCheck %s // RUN: cat %t-mod.ll | FileCheck --check-prefix=CHECK-NEG %s // RUN: cat %t-mod.ll | FileCheck --check-prefix=CHECK-MOD %s diff --git a/clang/test/Modules/ModuleDebugInfo.m b/clang/test/Modules/ModuleDebugInfo.m index c527c43a0f4a2..0e92831ae5414 100644 --- a/clang/test/Modules/ModuleDebugInfo.m +++ b/clang/test/Modules/ModuleDebugInfo.m @@ -7,7 +7,7 @@ // Modules: // RUN: rm -rf %t // RUN: %clang_cc1 -x objective-c -fmodules -fmodule-format=obj \ -// RUN: -fimplicit-module-maps -DMODULES -fmodules-cache-path=%t %s \ +// RUN: -fimplicit-module-maps -DMODULES -fmodules-cache-path=%t/cache %s \ // RUN: -I %S/Inputs -I %t -emit-llvm -o %t.ll \ // RUN: -mllvm -debug-only=pchcontainer &>%t-mod.ll // RUN: cat %t-mod.ll | FileCheck %s diff --git a/clang/test/Modules/ModuleModuleDebugInfo.cpp b/clang/test/Modules/ModuleModuleDebugInfo.cpp index 61449643937a7..aa45fe9da0698 100644 --- a/clang/test/Modules/ModuleModuleDebugInfo.cpp +++ b/clang/test/Modules/ModuleModuleDebugInfo.cpp @@ -5,7 +5,7 @@ // RUN: -dwarf-ext-refs -fmodules \ // RUN: -fmodule-format=obj -fimplicit-module-maps -DMODULES \ // RUN: -triple %itanium_abi_triple \ -// RUN: -fmodules-cache-path=%t %s -I %S/Inputs -I %t -emit-llvm -o - \ +// RUN: -fmodules-cache-path=%t/cache %s -I %S/Inputs -I %t -emit-llvm -o - \ // RUN: | FileCheck %s #include "DebugNestedB.h" diff --git a/clang/test/Modules/cxx20-hu-04.cpp b/clang/test/Modules/cxx20-hu-04.cpp index 8546b33572dc8..075b44289f100 100644 --- a/clang/test/Modules/cxx20-hu-04.cpp +++ b/clang/test/Modules/cxx20-hu-04.cpp @@ -102,4 +102,4 @@ int success(int x) { } // CHECK-IMP-HU2: remark: importing module '.{{/|\\\\?}}hu-02.h' from 'hu-02.pcm' -// CHECK-IMP-HU2: remark: importing module '.{{/|\\\\?}}hu-01.h' into '.{{/|\\\\?}}hu-02.h' from '[[TDIR]]{{[/\\]}}hu-01.pcm' +// CHECK-IMP-HU2: remark: importing module '.{{/|\\\\?}}hu-01.h' into '.{{/|\\\\?}}hu-02.h' from 'hu-01.pcm' diff --git a/clang/test/Modules/debug-info-moduleimport.m b/clang/test/Modules/debug-info-moduleimport.m index acb7dbd48bdd0..bdeb05bc08a02 100644 --- a/clang/test/Modules/debug-info-moduleimport.m +++ b/clang/test/Modules/debug-info-moduleimport.m @@ -2,7 +2,7 @@ // RUN: rm -rf %t // RUN: %clang_cc1 -debug-info-kind=limited -fmodules \ // RUN: -DGREETING="Hello World" -UNDEBUG \ -// RUN: -fimplicit-module-maps -fmodules-cache-path=%t %s \ +// RUN: -fimplicit-module-maps -fmodules-cache-path=%t/cache %s \ // RUN: -I %S/Inputs -isysroot /tmp/.. -I %t -emit-llvm -o - \ // RUN: | FileCheck %s --check-prefix=NOIMPORT @@ -12,7 +12,7 @@ // RUN: rm -rf %t // RUN: %clang_cc1 -debug-info-kind=limited -fmodules \ // RUN: -DGREETING="Hello World" -UNDEBUG \ -// RUN: -fimplicit-module-maps -fmodules-cache-path=%t %s \ +// RUN: -fimplicit-module-maps -fmodules-cache-path=%t/cache %s \ // RUN: -I %S/Inputs -isysroot /tmp/.. -I %t -emit-llvm \ // RUN: -debugger-tuning=lldb -o - | FileCheck %s @@ -28,13 +28,13 @@ // CHECK: ![[F]] = !DIFile(filename: {{.*}}debug-info-moduleimport.m // RUN: %clang_cc1 -debug-info-kind=limited -fmodules -fimplicit-module-maps \ -// RUN: -fmodules-cache-path=%t %s -I %S/Inputs -isysroot /tmp/.. -I %t \ +// RUN: -fmodules-cache-path=%t/cache %s -I %S/Inputs -isysroot /tmp/.. -I %t \ // RUN: -emit-llvm -o - | FileCheck %s --check-prefix=NO-SKEL-CHECK // NO-SKEL-CHECK: distinct !DICompileUnit // NO-SKEL-CHECK-NOT: distinct !DICompileUnit // RUN: %clang_cc1 -debug-info-kind=limited -fmodules -fimplicit-module-maps \ -// RUN: -fmodules-cache-path=%t -fdebug-prefix-map=%t=/MODULE-CACHE \ +// RUN: -fmodules-cache-path=%t/cache -fdebug-prefix-map=%t/cache=/MODULE-CACHE \ // RUN: -fdebug-prefix-map=%S=/SRCDIR \ // RUN: -fmodule-format=obj -dwarf-ext-refs \ // RUN: %s -I %S/Inputs -isysroot /tmp/.. -I %t -emit-llvm -o - \ diff --git a/clang/test/Modules/global_index.m b/clang/test/Modules/global_index.m index 521914702d1b1..3857e10c2e922 100644 --- a/clang/test/Modules/global_index.m +++ b/clang/test/Modules/global_index.m @@ -1,12 +1,12 @@ // RUN: rm -rf %t // Run without global module index -// RUN: %clang_cc1 -Rmodule-include-translation -Wno-private-module -fmodules-cache-path=%t -fdisable-module-hash -fmodules -fimplicit-module-maps -fno-modules-global-index -F %S/Inputs %s -verify -// RUN: ls %t|not grep modules.idx +// RUN: %clang_cc1 -Rmodule-include-translation -Wno-private-module -fmodules-cache-path=%t/cache -fdisable-module-hash -fmodules -fimplicit-module-maps -fno-modules-global-index -F %S/Inputs %s -verify +// RUN: ls %t/cache | not grep modules.idx // Run and create the global module index -// RUN: %clang_cc1 -Rmodule-include-translation -Wno-private-module -fmodules-cache-path=%t -fdisable-module-hash -fmodules -fimplicit-module-maps -F %S/Inputs %s -verify -// RUN: ls %t|grep modules.idx +// RUN: %clang_cc1 -Rmodule-include-translation -Wno-private-module -fmodules-cache-path=%t/cache -fdisable-module-hash -fmodules -fimplicit-module-maps -F %S/Inputs %s -verify +// RUN: ls %t/cache | grep modules.idx // Run and use the global module index -// RUN: %clang_cc1 -Rmodule-include-translation -Wno-private-module -fmodules-cache-path=%t -fdisable-module-hash -fmodules -fimplicit-module-maps -F %S/Inputs %s -verify -print-stats 2>&1 | FileCheck %s +// RUN: %clang_cc1 -Rmodule-include-translation -Wno-private-module -fmodules-cache-path=%t/cache -fdisable-module-hash -fmodules -fimplicit-module-maps -F %S/Inputs %s -verify -print-stats 2>&1 | FileCheck %s // expected-no-diagnostics @import DependsOnModule; diff --git a/clang/test/Modules/load-after-failure.m b/clang/test/Modules/load-after-failure.m index 73ed0e79708b9..c0425365bb9d6 100644 --- a/clang/test/Modules/load-after-failure.m +++ b/clang/test/Modules/load-after-failure.m @@ -11,9 +11,9 @@ // RUN: echo 'module C { header "C.h" }' >> %t/module.modulemap // RUN: echo 'module D { header "D.h" }' >> %t/module.modulemap -// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %t %s -verify +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -I %t %s -verify // RUN: echo " " >> %t/D.h -// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %t %s -verify +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -I %t %s -verify // expected-no-diagnostics diff --git a/clang/test/Modules/module-debuginfo-compdir.m b/clang/test/Modules/module-debuginfo-compdir.m index fced242624f75..885a896d45918 100644 --- a/clang/test/Modules/module-debuginfo-compdir.m +++ b/clang/test/Modules/module-debuginfo-compdir.m @@ -5,7 +5,7 @@ // RUN: rm -rf %t // RUN: %clang_cc1 -x objective-c -fmodules -fmodule-format=obj \ // RUN: -fdebug-compilation-dir=/OVERRIDE \ -// RUN: -fimplicit-module-maps -DMODULES -fmodules-cache-path=%t %s \ +// RUN: -fimplicit-module-maps -DMODULES -fmodules-cache-path=%t/cache %s \ // RUN: -I %S/Inputs -I %t -emit-llvm -o %t.ll \ // RUN: -mllvm -debug-only=pchcontainer &>%t-mod.ll // RUN: cat %t-mod.ll | FileCheck %s diff --git a/clang/test/Modules/module-debuginfo-prefix.m b/clang/test/Modules/module-debuginfo-prefix.m index 0245ff1cabf7d..7270160417dd9 100644 --- a/clang/test/Modules/module-debuginfo-prefix.m +++ b/clang/test/Modules/module-debuginfo-prefix.m @@ -5,7 +5,7 @@ // RUN: rm -rf %t // RUN: %clang_cc1 -x objective-c -fmodules -fmodule-format=obj \ // RUN: -fdebug-prefix-map=%S/Inputs=/OVERRIDE \ -// RUN: -fimplicit-module-maps -DMODULES -fmodules-cache-path=%t %s \ +// RUN: -fimplicit-module-maps -DMODULES -fmodules-cache-path=%t/cache %s \ // RUN: -I %S/Inputs -I %t -emit-llvm -o %t.ll \ // RUN: -mllvm -debug-only=pchcontainer &>%t-mod.ll // RUN: cat %t-mod.ll | FileCheck %s diff --git a/clang/test/Modules/relocatable-modules.cpp b/clang/test/Modules/relocatable-modules.cpp index c8d1e6d455666..e047a79c17d3b 100644 --- a/clang/test/Modules/relocatable-modules.cpp +++ b/clang/test/Modules/relocatable-modules.cpp @@ -13,11 +13,9 @@ // RUN: -Wno-experimental-header-units -fmodule-file=hu-01.pcm -o hu-02-rel.pcm \ // RUN: -fmodule-file-home-is-cwd -// RUN: %clang -module-file-info hu-02-abs.pcm | FileCheck %s --check-prefix=IMPORT-ABS -DPREFIX=%t -// IMPORT-ABS: Imports module 'hu-01': [[PREFIX]]{{/|\\}}hu-01.pcm - -// RUN: %clang -module-file-info hu-02-rel.pcm | FileCheck %s --check-prefix=IMPORT-REL -// IMPORT-REL: Imports module 'hu-01': hu-01.pcm +// RUN: %clang -module-file-info hu-02-abs.pcm | FileCheck %s --check-prefix=IMPORT +// RUN: %clang -module-file-info hu-02-rel.pcm | FileCheck %s --check-prefix=IMPORT +// IMPORT: Imports module 'hu-01': hu-01.pcm // RUN: llvm-bcanalyzer --dump --disable-histogram %t/hu-02-abs.pcm \ // RUN: | FileCheck %s --check-prefix=INPUT-ABS -DPREFIX=%t diff --git a/clang/unittests/Serialization/ForceCheckFileInputTest.cpp b/clang/unittests/Serialization/ForceCheckFileInputTest.cpp index b76dcfec96063..81f5d8d3a81b6 100644 --- a/clang/unittests/Serialization/ForceCheckFileInputTest.cpp +++ b/clang/unittests/Serialization/ForceCheckFileInputTest.cpp @@ -137,9 +137,9 @@ export module a; export int aa = 44; )cpp"); - auto ReadResult = - Clang.getASTReader()->ReadAST(BMIPath, serialization::MK_MainFile, - SourceLocation(), ASTReader::ARR_None); + auto ReadResult = Clang.getASTReader()->ReadAST( + ModuleFileName::make_explicit(BMIPath), serialization::MK_MainFile, + SourceLocation(), ASTReader::ARR_None); // We shall be able to detect the content change here. EXPECT_NE(ReadResult, ASTReader::Success); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
