Index: include/clang/Basic/DiagnosticSerializationKinds.td =================================================================== --- include/clang/Basic/DiagnosticSerializationKinds.td +++ include/clang/Basic/DiagnosticSerializationKinds.td @@ -125,6 +125,15 @@ "duplicate module file extension block name '%0'">, InGroup; +def warn_module_system_bit_conflict : Warning< + "module file '%0' was validated as a system module, ignoring differences in " + "diagnostic options now it is a non-system module">, + InGroup; + +def err_module_ancestor_conflict : Error< + "module file '%0' was validated in an ancestor thread, now it needs to " + "be invalidated">; + } // let CategoryName } // let Component Index: include/clang/Basic/FileManager.h =================================================================== --- include/clang/Basic/FileManager.h +++ include/clang/Basic/FileManager.h @@ -39,6 +39,7 @@ namespace clang { class FileSystemStatCache; +class PCMCache; /// \brief Cached information about one directory (either on disk or in /// the virtual file system). @@ -171,6 +172,9 @@ /// or a directory) as virtual directories. void addAncestorsAsVirtualDirs(StringRef Path); + /// Manage memory buffers associated with pcm files. + std::unique_ptr BufferMgr; + public: FileManager(const FileSystemOptions &FileSystemOpts, IntrusiveRefCntPtr FS = nullptr); @@ -282,6 +286,50 @@ StringRef getCanonicalName(const DirectoryEntry *Dir); void PrintStats() const; + + /// Return the manager for memory buffers associated with pcm files. + PCMCache *getPCMCache() { + return BufferMgr.get(); + } +}; + +/// This class manages memory buffers associated with pcm files. It also keeps +/// track of the thread context when we start a thread to implicitly build +/// a module. +class PCMCache { + /// Map a pcm file to a MemoryBuffer and a boolean that tells us whether + // the pcm file is validated as a system module. + std::map, bool>> + ConsistentBuffers; + + typedef std::map IsSystemMap; + /// When we start a thread, we push in a ThreadContext; when we end + /// a thread, we pop a ThreadContext. + struct ThreadContext { + /// Keep track of all module files that have been validated in this thread + /// and whether the module file is validated as a system module. + IsSystemMap ModulesInParent; + }; + SmallVector NestedThreadContexts; + +public: + /// Add a memory buffer for a pcm file to the manager. + void addConsistentBuffer(std::string FileName, + std::unique_ptr Buffer); + /// Remove a pcm file from the map. + void removeFromConsistentBuffer(std::string FileName); + /// Return the memory buffer for a given pcm file. + llvm::MemoryBuffer *lookupConsistentBuffer(std::string Name); + /// Update the IsSystem boolean flag in ConsistentBuffers. + void setIsSystem(std::string FileName, bool IsSystem); + + /// Check if a pcm file was validated by an ancestor, if yes, return + /// whether it was validated as a system module. + bool isValidatedByAncestor(std::string FileName, bool &IsSystem); + + void StartThread(); + void EndThread(); + }; } // end namespace clang Index: include/clang/Serialization/ASTReader.h =================================================================== --- include/clang/Serialization/ASTReader.h +++ include/clang/Serialization/ASTReader.h @@ -405,6 +405,9 @@ /// \brief The module manager which manages modules and their dependencies ModuleManager ModuleMgr; + /// \brief The PCM manager which manages memory buffers for pcm files. + PCMCache *BufferMgr; + /// \brief A dummy identifier resolver used to merge TU-scope declarations in /// C, for the cases where we don't have a Sema object to provide a real /// identifier resolver. Index: include/clang/Serialization/ASTWriter.h =================================================================== --- include/clang/Serialization/ASTWriter.h +++ include/clang/Serialization/ASTWriter.h @@ -109,6 +109,9 @@ /// \brief The buffer associated with the bitstream. const SmallVectorImpl &Buffer; + /// \brief The PCM manager which manages memory buffers for pcm files. + PCMCache *BufferMgr; + /// \brief The ASTContext we're writing. ASTContext *Context; @@ -505,6 +508,7 @@ /// \brief Create a new precompiled header writer that outputs to /// the given bitstream. ASTWriter(llvm::BitstreamWriter &Stream, SmallVectorImpl &Buffer, + PCMCache *BufferMgr, ArrayRef> Extensions, bool IncludeTimestamps = true); ~ASTWriter() override; @@ -944,7 +948,7 @@ PCHGenerator( const Preprocessor &PP, StringRef OutputFile, StringRef isysroot, - std::shared_ptr Buffer, + std::shared_ptr Buffer, PCMCache *BufferMgr, ArrayRef> Extensions, bool AllowASTWithErrors = false, bool IncludeTimestamps = true); Index: include/clang/Serialization/Module.h =================================================================== --- include/clang/Serialization/Module.h +++ include/clang/Serialization/Module.h @@ -164,7 +164,8 @@ /// \brief The memory buffer that stores the data associated with /// this AST file. - std::unique_ptr Buffer; + /// The memory buffer is owned by PCMCache. + llvm::MemoryBuffer *Buffer; /// \brief The size of this file, in bits. uint64_t SizeInBits; Index: include/clang/Serialization/ModuleManager.h =================================================================== --- include/clang/Serialization/ModuleManager.h +++ include/clang/Serialization/ModuleManager.h @@ -159,7 +159,7 @@ /// \brief Returns the in-memory (virtual file) buffer with the given name std::unique_ptr lookupBuffer(StringRef Name); - + /// \brief Number of modules loaded unsigned size() const { return Chain.size(); } @@ -223,7 +223,8 @@ /// \brief Remove the given set of modules. void removeModules(ModuleIterator first, ModuleIterator last, llvm::SmallPtrSetImpl &LoadedSuccessfully, - ModuleMap *modMap); + ModuleMap *modMap, + llvm::SmallVectorImpl &ValidationConflicts); /// \brief Add an in-memory buffer the list of known buffers void addInMemoryBuffer(StringRef FileName, @@ -233,8 +234,9 @@ void setGlobalIndex(GlobalModuleIndex *Index); /// \brief Notification from the AST reader that the given module file - /// has been "accepted", and will not (can not) be unloaded. - void moduleFileAccepted(ModuleFile *MF); + /// has been "accepted", and will not (can not) be unloaded. The IsSystem + /// flag tells us whether the module file was validated as a system module. + void moduleFileAccepted(ModuleFile *MF, bool IsSystem); /// \brief Visit each of the modules. /// Index: lib/Basic/FileManager.cpp =================================================================== --- lib/Basic/FileManager.cpp +++ lib/Basic/FileManager.cpp @@ -59,6 +59,7 @@ // file system. if (!FS) this->FS = vfs::getRealFileSystem(); + BufferMgr = llvm::make_unique(); } FileManager::~FileManager() = default; Index: lib/Frontend/ASTUnit.cpp =================================================================== --- lib/Frontend/ASTUnit.cpp +++ lib/Frontend/ASTUnit.cpp @@ -185,7 +185,7 @@ llvm::BitstreamWriter Stream; ASTWriter Writer; - ASTWriterData() : Stream(Buffer), Writer(Stream, Buffer, { }) { } + ASTWriterData() : Stream(Buffer), Writer(Stream, Buffer, nullptr, { }) { } }; void ASTUnit::clearFileLevelDecls() { @@ -923,9 +923,10 @@ public: PrecompilePreambleConsumer(ASTUnit &Unit, PrecompilePreambleAction *Action, - const Preprocessor &PP, StringRef isysroot, + const Preprocessor &PP, PCMCache *BufferMgr, + StringRef isysroot, std::unique_ptr Out) - : PCHGenerator(PP, "", isysroot, std::make_shared(), + : PCHGenerator(PP, "", isysroot, std::make_shared(), BufferMgr, ArrayRef>(), /*AllowASTWithErrors=*/true), Unit(Unit), Hash(Unit.getCurrentTopLevelHashValue()), Action(Action), @@ -994,7 +995,8 @@ llvm::make_unique( Unit.getCurrentTopLevelHashValue())); return llvm::make_unique( - Unit, this, CI.getPreprocessor(), Sysroot, std::move(OS)); + Unit, this, CI.getPreprocessor(), CI.getFileManager().getPCMCache(), + Sysroot, std::move(OS)); } static bool isNonDriverDiag(const StoredDiagnostic &StoredDiag) { @@ -2519,7 +2521,7 @@ SmallString<128> Buffer; llvm::BitstreamWriter Stream(Buffer); - ASTWriter Writer(Stream, Buffer, { }); + ASTWriter Writer(Stream, Buffer, nullptr, { }); return serializeUnit(Writer, Buffer, getSema(), hasErrors, OS); } Index: lib/Frontend/ChainedIncludesSource.cpp =================================================================== --- lib/Frontend/ChainedIncludesSource.cpp +++ lib/Frontend/ChainedIncludesSource.cpp @@ -162,7 +162,8 @@ ArrayRef> Extensions; auto consumer = llvm::make_unique( Clang->getPreprocessor(), "-", /*isysroot=*/"", Buffer, - Extensions, /*AllowASTWithErrors=*/true); + Clang->getFileManager().getPCMCache(), Extensions, + /*AllowASTWithErrors=*/true); Clang->getASTContext().setASTMutationListener( consumer->GetASTMutationListener()); Clang->setASTConsumer(std::move(consumer)); Index: lib/Frontend/CompilerInstance.cpp =================================================================== --- lib/Frontend/CompilerInstance.cpp +++ lib/Frontend/CompilerInstance.cpp @@ -1063,7 +1063,9 @@ // Note that this module is part of the module build stack, so that we // can detect cycles in the module graph. + // PCMCache is part of the FileManager, so it is shared among threads. Instance.setFileManager(&ImportingInstance.getFileManager()); + ImportingInstance.getFileManager().getPCMCache()->StartThread(); Instance.createSourceManager(Instance.getFileManager()); SourceManager &SourceMgr = Instance.getSourceManager(); SourceMgr.setModuleBuildStack( @@ -1131,6 +1133,7 @@ ImportingInstance.setBuildGlobalModuleIndex(true); } + ImportingInstance.getFileManager().getPCMCache()->EndThread(); return !Instance.getDiagnostics().hasErrorOccurred(); } Index: lib/Frontend/FrontendActions.cpp =================================================================== --- lib/Frontend/FrontendActions.cpp +++ lib/Frontend/FrontendActions.cpp @@ -92,7 +92,8 @@ std::vector> Consumers; Consumers.push_back(llvm::make_unique( CI.getPreprocessor(), OutputFile, Sysroot, - Buffer, CI.getFrontendOpts().ModuleFileExtensions, + Buffer, CI.getFileManager().getPCMCache(), + CI.getFrontendOpts().ModuleFileExtensions, /*AllowASTWithErrors*/false, /*IncludeTimestamps*/ +CI.getFrontendOpts().IncludeTimestamps)); @@ -142,7 +143,8 @@ Consumers.push_back(llvm::make_unique( CI.getPreprocessor(), OutputFile, Sysroot, - Buffer, CI.getFrontendOpts().ModuleFileExtensions, + Buffer, CI.getFileManager().getPCMCache(), + CI.getFrontendOpts().ModuleFileExtensions, /*AllowASTWithErrors=*/false, /*IncludeTimestamps=*/ +CI.getFrontendOpts().BuildingImplicitModule)); Index: lib/Serialization/ASTReader.cpp =================================================================== --- lib/Serialization/ASTReader.cpp +++ lib/Serialization/ASTReader.cpp @@ -461,19 +461,9 @@ return checkDiagnosticGroupMappings(StoredDiags, Diags, Complain); } -bool PCHValidator::ReadDiagnosticOptions( - IntrusiveRefCntPtr DiagOpts, bool Complain) { - DiagnosticsEngine &ExistingDiags = PP.getDiagnostics(); - IntrusiveRefCntPtr DiagIDs(ExistingDiags.getDiagnosticIDs()); - IntrusiveRefCntPtr Diags( - new DiagnosticsEngine(DiagIDs, DiagOpts.get())); - // This should never fail, because we would have processed these options - // before writing them to an ASTFile. - ProcessWarningOptions(*Diags, *DiagOpts, /*Report*/false); - - ModuleManager &ModuleMgr = Reader.getModuleManager(); - assert(ModuleMgr.size() >= 1 && "what ASTFile is this then"); - +/// Return the top import module if it is implicit, nullptr otherwise. +static Module *getTopImportImplicitModule(ModuleManager &ModuleMgr, + Preprocessor &PP) { // If the original import came from a file explicitly generated by the user, // don't check the diagnostic mappings. // FIXME: currently this is approximated by checking whether this is not a @@ -485,17 +475,37 @@ while (!TopImport->ImportedBy.empty()) TopImport = TopImport->ImportedBy[0]; if (TopImport->Kind != MK_ImplicitModule) - return false; + return nullptr; StringRef ModuleName = TopImport->ModuleName; assert(!ModuleName.empty() && "diagnostic options read before module name"); Module *M = PP.getHeaderSearchInfo().lookupModule(ModuleName); assert(M && "missing module"); + return M; +} + +bool PCHValidator::ReadDiagnosticOptions( + IntrusiveRefCntPtr DiagOpts, bool Complain) { + DiagnosticsEngine &ExistingDiags = PP.getDiagnostics(); + IntrusiveRefCntPtr DiagIDs(ExistingDiags.getDiagnosticIDs()); + IntrusiveRefCntPtr Diags( + new DiagnosticsEngine(DiagIDs, DiagOpts.get())); + // This should never fail, because we would have processed these options + // before writing them to an ASTFile. + ProcessWarningOptions(*Diags, *DiagOpts, /*Report*/false); + + ModuleManager &ModuleMgr = Reader.getModuleManager(); + assert(ModuleMgr.size() >= 1 && "what ASTFile is this then"); + + Module *TopM = getTopImportImplicitModule(ModuleMgr, PP); + if (!TopM) + return false; // FIXME: if the diagnostics are incompatible, save a DiagnosticOptions that // contains the union of their flags. - return checkDiagnosticMappings(*Diags, ExistingDiags, M->IsSystem, Complain); + return checkDiagnosticMappings(*Diags, ExistingDiags, TopM->IsSystem, + Complain); } /// \brief Collect the macro definitions provided by the given preprocessor @@ -2177,7 +2187,9 @@ if (ValidateDiagnosticOptions && !AllowCompatibleConfigurationMismatch && ParseDiagnosticOptions(Record, Complain, Listener)) - return OutOfDate; + // Continue checking signature even if the diagnostic options + // are out of date. + Result = OutOfDate; break; } case SIGNATURE: { @@ -3640,11 +3652,15 @@ for (const ImportedModule &IM : Loaded) LoadedSet.insert(IM.Mod); + llvm::SmallVector ValidationConflicts; ModuleMgr.removeModules(ModuleMgr.begin() + NumModules, ModuleMgr.end(), LoadedSet, Context.getLangOpts().Modules ? &PP.getHeaderSearchInfo().getModuleMap() - : nullptr); + : nullptr, + ValidationConflicts); + for (auto N : ValidationConflicts) + Diag(diag::err_module_ancestor_conflict) << N; // If we find that any modules are unusable, the global index is going // to be out-of-date. Just remove it. @@ -3719,7 +3735,8 @@ M != MEnd; ++M) { ModuleFile &F = *M->Mod; - ModuleMgr.moduleFileAccepted(&F); + Module *Mod = getTopImportImplicitModule(ModuleMgr, PP); + ModuleMgr.moduleFileAccepted(&F, Mod ? Mod->IsSystem : false); // Set the import location. F.DirectImportLoc = ImportLoc; @@ -4020,6 +4037,22 @@ if (DisableValidation || (AllowConfigurationMismatch && Result == ConfigurationMismatch)) Result = Success; + else if (Result == OutOfDate && F.Kind == MK_ImplicitModule) { + // If this module was validated as a system module, and it is now a + // non-system module, emit a warning about ignoring differences in + // diagnostic options. + bool IsSystem; + if (BufferMgr->isValidatedByAncestor(F.FileName, IsSystem)) { + if (IsSystem) { + Module *M = getTopImportImplicitModule(ModuleMgr, PP); + assert(M && "missing module"); + if (!M->IsSystem) { + Diag(diag::warn_module_system_bit_conflict) << F.FileName; + Result = Success; + } + } + } + } } if (Result == Success) @@ -7155,7 +7188,7 @@ void ASTReader::getMemoryBufferSizes(MemoryBufferSizes &sizes) const { for (ModuleConstIterator I = ModuleMgr.begin(), E = ModuleMgr.end(); I != E; ++I) { - if (llvm::MemoryBuffer *buf = (*I)->Buffer.get()) { + if (llvm::MemoryBuffer *buf = (*I)->Buffer) { size_t bytes = buf->getBufferSize(); switch (buf->getBufferKind()) { case llvm::MemoryBuffer::MemoryBuffer_Malloc: @@ -8962,6 +8995,7 @@ FileMgr(PP.getFileManager()), PCHContainerRdr(PCHContainerRdr), Diags(PP.getDiagnostics()), SemaObj(nullptr), PP(PP), Context(Context), Consumer(nullptr), ModuleMgr(PP.getFileManager(), PCHContainerRdr), + BufferMgr(PP.getFileManager().getPCMCache()), DummyIdResolver(PP), ReadTimer(std::move(ReadTimer)), PragmaMSStructState(-1), Index: lib/Serialization/ASTWriter.cpp =================================================================== --- lib/Serialization/ASTWriter.cpp +++ lib/Serialization/ASTWriter.cpp @@ -4217,9 +4217,11 @@ ASTWriter::ASTWriter( llvm::BitstreamWriter &Stream, SmallVectorImpl &Buffer, + PCMCache *BufferMgr, ArrayRef> Extensions, bool IncludeTimestamps) - : Stream(Stream), Buffer(Buffer), Context(nullptr), PP(nullptr), Chain(nullptr), + : Stream(Stream), Buffer(Buffer), BufferMgr(BufferMgr), + Context(nullptr), PP(nullptr), Chain(nullptr), WritingModule(nullptr), IncludeTimestamps(IncludeTimestamps), WritingAST(false), DoneWritingDeclsAndTypes(false), ASTHasCompilerErrors(false), FirstDeclID(NUM_PREDEF_DECL_IDS), @@ -4283,6 +4285,16 @@ this->BaseDirectory.clear(); WritingAST = false; + if (BufferMgr && SemaRef.Context.getLangOpts().ImplicitModules && + WritingModule) { + // Construct MemoryBuffer and update buffer manager. + std::unique_ptr write_buffer = + llvm::MemoryBuffer::getMemBufferCopy( + StringRef(Buffer.begin(), Buffer.size())); + BufferMgr->addConsistentBuffer(OutputFile, std::move(write_buffer)); + BufferMgr->setIsSystem(OutputFile, + WritingModule ? WritingModule->IsSystem : false); + } return Signature; } Index: lib/Serialization/GeneratePCH.cpp =================================================================== --- lib/Serialization/GeneratePCH.cpp +++ lib/Serialization/GeneratePCH.cpp @@ -23,12 +23,12 @@ PCHGenerator::PCHGenerator( const Preprocessor &PP, StringRef OutputFile, StringRef isysroot, - std::shared_ptr Buffer, + std::shared_ptr Buffer, PCMCache *BufferMgr, ArrayRef> Extensions, bool AllowASTWithErrors, bool IncludeTimestamps) : PP(PP), OutputFile(OutputFile), isysroot(isysroot.str()), SemaPtr(nullptr), Buffer(Buffer), Stream(Buffer->Data), - Writer(Stream, Buffer->Data, Extensions, IncludeTimestamps), + Writer(Stream, Buffer->Data, BufferMgr, Extensions, IncludeTimestamps), AllowASTWithErrors(AllowASTWithErrors) { Buffer->IsComplete = false; } Index: lib/Serialization/ModuleManager.cpp =================================================================== --- lib/Serialization/ModuleManager.cpp +++ lib/Serialization/ModuleManager.cpp @@ -108,7 +108,11 @@ // Load the contents of the module if (std::unique_ptr Buffer = lookupBuffer(FileName)) { // The buffer was already provided for us. - ModuleEntry->Buffer = std::move(Buffer); + ModuleEntry->Buffer = Buffer.get(); + FileMgr.getPCMCache()->addConsistentBuffer(FileName, std::move(Buffer)); + } else if(llvm::MemoryBuffer *ConsistentB = + FileMgr.getPCMCache()->lookupConsistentBuffer(FileName)) { + ModuleEntry->Buffer = ConsistentB; } else { // Open the AST file. llvm::ErrorOr> Buf( @@ -131,7 +135,8 @@ return Missing; } - ModuleEntry->Buffer = std::move(*Buf); + ModuleEntry->Buffer = (*Buf).get(); + FileMgr.getPCMCache()->addConsistentBuffer(FileName, std::move(*Buf)); } // Initialize the stream. @@ -148,8 +153,11 @@ ErrorStr = ModuleEntry->Signature != ASTFileSignature({{0}}) ? "signature mismatch" : "could not read module signature"; - if (NewModule) + if (NewModule) { + FileMgr.getPCMCache()->removeFromConsistentBuffer( + ModuleEntry->FileName); delete ModuleEntry; + } return OutOfDate; } } @@ -184,7 +192,8 @@ void ModuleManager::removeModules( ModuleIterator first, ModuleIterator last, llvm::SmallPtrSetImpl &LoadedSuccessfully, - ModuleMap *modMap) { + ModuleMap *modMap, + llvm::SmallVectorImpl &ValidationConflicts) { if (first == last) return; @@ -227,9 +236,18 @@ // Files that didn't make it through ReadASTCore successfully will be // rebuilt (or there was an error). Invalidate them so that we can load the // new files that will be renamed over the old ones. - if (LoadedSuccessfully.count(*victim) == 0) - FileMgr.invalidateCache((*victim)->File); - + if (LoadedSuccessfully.count(*victim) == 0) { + // Before removing the module file, check if it was validated in an + // ancestor thread, if yes, throw a hard error instead of causing + // use-after-free in the ancestor thread. + bool IsSystem; + if (FileMgr.getPCMCache()->isValidatedByAncestor((*victim)->FileName, + IsSystem)) + ValidationConflicts.push_back((*victim)->FileName); + else + FileMgr.invalidateCache((*victim)->File); + FileMgr.getPCMCache()->removeFromConsistentBuffer((*victim)->FileName); + } delete *victim; } @@ -237,6 +255,57 @@ Chain.erase(first, last); } +llvm::MemoryBuffer* +PCMCache::lookupConsistentBuffer(std::string Name) { + if (!ConsistentBuffers.count(Name)) + return nullptr; + return ConsistentBuffers[Name].first.get(); +} + +void +PCMCache::addConsistentBuffer(std::string FileName, + std::unique_ptr Buffer) { + assert(!ConsistentBuffers.count(FileName) && "already has a buffer"); + ConsistentBuffers[FileName] = std::make_pair(std::move(Buffer), + /*IsSystem*/false); +} + +void PCMCache::removeFromConsistentBuffer(std::string FileName) { + assert(ConsistentBuffers.count(FileName) && "remove a non-existent buffer"); + + // This should release the memory. + ConsistentBuffers.erase(FileName); +} + +bool PCMCache::isValidatedByAncestor(std::string FileName, bool &IsSystem) { + IsSystem = false; + if (NestedThreadContexts.empty()) + return false; + if (NestedThreadContexts.back().ModulesInParent.count(FileName)) { + IsSystem = NestedThreadContexts.back().ModulesInParent[FileName]; + return true; + } + return false; +} + +void PCMCache::setIsSystem(std::string FileName, bool IsSystem) { + assert(ConsistentBuffers.count(FileName) && + "set IsSystem for a non-existent buffer"); + ConsistentBuffers[FileName].second = IsSystem; +} + +void PCMCache::StartThread() { + // Save all validated module files in thread context. + ThreadContext ThreadC; + for (auto &p : ConsistentBuffers) + ThreadC.ModulesInParent.insert(std::make_pair(p.first, p.second.second)); + NestedThreadContexts.push_back(ThreadC); +} + +void PCMCache::EndThread() { + NestedThreadContexts.pop_back(); +} + void ModuleManager::addInMemoryBuffer(StringRef FileName, std::unique_ptr Buffer) { @@ -281,7 +350,8 @@ } } -void ModuleManager::moduleFileAccepted(ModuleFile *MF) { +void ModuleManager::moduleFileAccepted(ModuleFile *MF, bool IsSystem) { + FileMgr.getPCMCache()->setIsSystem(MF->FileName, IsSystem); if (!GlobalIndex || GlobalIndex->loadedModuleFile(MF)) return;