Index: clang/include/clang/Basic/DiagnosticSerializationKinds.td =================================================================== --- clang/include/clang/Basic/DiagnosticSerializationKinds.td +++ clang/include/clang/Basic/DiagnosticSerializationKinds.td @@ -176,6 +176,11 @@ "duplicate module file extension block name '%0'">, InGroup; +def warn_module_system_bit_conflict : Warning< + "module file '%0' was validated as a system module and is now being imported " + "as a non-system module; any difference in diagnostic options will be ignored">, + InGroup; + } // let CategoryName } // let Component Index: clang/include/clang/Basic/MemoryBufferCache.h =================================================================== --- /dev/null +++ clang/include/clang/Basic/MemoryBufferCache.h @@ -0,0 +1,80 @@ +//===- MemoryBufferCache.h - Cache for loaded memory buffers ----*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_BASIC_MEMORYBUFFERCACHE_H +#define LLVM_CLANG_BASIC_MEMORYBUFFERCACHE_H + +#include "llvm/ADT/IntrusiveRefCntPtr.h" +#include "llvm/ADT/StringMap.h" +#include + +namespace llvm { +class MemoryBuffer; +} // end namespace llvm + +namespace clang { + +/// Manage memory buffers across multiple users. +/// +/// Ensures that multiple users have a consistent view of each buffer. This is +/// used by \a CompilerInstance when building PCMs to ensure that each \a +/// ModuleManager sees the same files. +/// +/// \a finalizeCurrentBuffers() should be called before creating a new user. +/// This locks in the current buffers, ensuring that no buffer that has already +/// been accessed can be purged, preventing use-after-frees. +class MemoryBufferCache : public llvm::RefCountedBase { + struct BufferEntry { + std::unique_ptr Buffer; + + /// Track the timeline of when this was added to the cache. + unsigned Index; + }; + + /// Cache of buffers. + llvm::StringMap Buffers; + + /// Monotonically increasing index. + unsigned NextIndex = 0; + + /// Bumped to prevent "older" buffers from being removed. + unsigned FirstRemovableIndex = 0; + +public: + /// Store the Buffer under the Filename. + /// + /// \pre There is not already buffer is not already in the cache. + /// \return a reference to the buffer as a convenience. + llvm::MemoryBuffer &addBuffer(llvm::StringRef Filename, + std::unique_ptr Buffer); + + /// Try to remove a buffer from the cache. + /// + /// \return false on success, iff \c !isBufferFinal(). + bool tryToRemoveBuffer(llvm::StringRef Filename); + + /// Get a pointer to the buffer if it exists; else nullptr. + llvm::MemoryBuffer *lookupBuffer(llvm::StringRef Filename); + + /// Check whether the buffer is final. + /// + /// \return true iff \a finalizeCurrentBuffers() has been called since the + /// buffer was added. This prevents buffers from being removed. + bool isBufferFinal(llvm::StringRef Filename); + + /// Finalize the current buffers in the cache. + /// + /// Should be called when creating a new user to ensure previous uses aren't + /// invalidated. + void finalizeCurrentBuffers(); +}; + +} // end namespace clang + +#endif // LLVM_CLANG_BASIC_MEMORYBUFFERCACHE_H Index: clang/include/clang/Frontend/ASTUnit.h =================================================================== --- clang/include/clang/Frontend/ASTUnit.h +++ clang/include/clang/Frontend/ASTUnit.h @@ -51,6 +51,7 @@ class FileEntry; class FileManager; class HeaderSearch; +class MemoryBufferCache; class Preprocessor; class PCHContainerOperations; class PCHContainerReader; @@ -84,6 +85,7 @@ IntrusiveRefCntPtr Diagnostics; IntrusiveRefCntPtr FileMgr; IntrusiveRefCntPtr SourceMgr; + IntrusiveRefCntPtr PCMCache; std::unique_ptr HeaderInfo; IntrusiveRefCntPtr Target; std::shared_ptr PP; Index: clang/include/clang/Frontend/CompilerInstance.h =================================================================== --- clang/include/clang/Frontend/CompilerInstance.h +++ clang/include/clang/Frontend/CompilerInstance.h @@ -44,6 +44,7 @@ class FileEntry; class FileManager; class FrontendAction; +class MemoryBufferCache; class Module; class Preprocessor; class Sema; @@ -90,6 +91,9 @@ /// The source manager. IntrusiveRefCntPtr SourceMgr; + /// The cache of PCM files. + IntrusiveRefCntPtr PCMCache; + /// The preprocessor. std::shared_ptr PP; @@ -178,7 +182,7 @@ explicit CompilerInstance( std::shared_ptr PCHContainerOps = std::make_shared(), - bool BuildingModule = false); + MemoryBufferCache *SharedPCMCache = nullptr); ~CompilerInstance() override; /// @name High-Level Operations @@ -783,6 +787,8 @@ } void setExternalSemaSource(IntrusiveRefCntPtr ESS); + + MemoryBufferCache &getPCMCache() const { return *PCMCache; } }; } // end namespace clang Index: clang/include/clang/Lex/Preprocessor.h =================================================================== --- clang/include/clang/Lex/Preprocessor.h +++ clang/include/clang/Lex/Preprocessor.h @@ -47,6 +47,7 @@ class FileManager; class FileEntry; class HeaderSearch; +class MemoryBufferCache; class PragmaNamespace; class PragmaHandler; class CommentHandler; @@ -102,6 +103,7 @@ const TargetInfo *AuxTarget; FileManager &FileMgr; SourceManager &SourceMgr; + MemoryBufferCache &PCMCache; std::unique_ptr ScratchBuf; HeaderSearch &HeaderInfo; ModuleLoader &TheModuleLoader; @@ -652,6 +654,7 @@ public: Preprocessor(std::shared_ptr PPOpts, DiagnosticsEngine &diags, LangOptions &opts, SourceManager &SM, + MemoryBufferCache &PCMCache, HeaderSearch &Headers, ModuleLoader &TheModuleLoader, IdentifierInfoLookup *IILookup = nullptr, bool OwnsHeaderSearch = false, @@ -691,6 +694,7 @@ const TargetInfo *getAuxTargetInfo() const { return AuxTarget; } FileManager &getFileManager() const { return FileMgr; } SourceManager &getSourceManager() const { return SourceMgr; } + MemoryBufferCache &getPCMCache() const { return PCMCache; } HeaderSearch &getHeaderSearchInfo() const { return HeaderInfo; } IdentifierTable &getIdentifierTable() { return Identifiers; } Index: clang/include/clang/Serialization/ASTReader.h =================================================================== --- clang/include/clang/Serialization/ASTReader.h +++ clang/include/clang/Serialization/ASTReader.h @@ -408,6 +408,9 @@ /// \brief The module manager which manages modules and their dependencies ModuleManager ModuleMgr; + /// The cache that manages memory buffers for PCM files. + MemoryBufferCache &PCMCache; + /// \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: clang/include/clang/Serialization/ASTWriter.h =================================================================== --- clang/include/clang/Serialization/ASTWriter.h +++ clang/include/clang/Serialization/ASTWriter.h @@ -54,6 +54,7 @@ class OpaqueValueExpr; class OpenCLOptions; class ASTReader; +class MemoryBufferCache; class Module; class ModuleFileExtension; class ModuleFileExtensionWriter; @@ -109,6 +110,9 @@ /// The buffer associated with the bitstream. const SmallVectorImpl &Buffer; + /// \brief The PCM manager which manages memory buffers for pcm files. + MemoryBufferCache &PCMCache; + /// \brief The ASTContext we're writing. ASTContext *Context = nullptr; @@ -512,6 +516,7 @@ /// \brief Create a new precompiled header writer that outputs to /// the given bitstream. ASTWriter(llvm::BitstreamWriter &Stream, SmallVectorImpl &Buffer, + MemoryBufferCache &PCMCache, ArrayRef> Extensions, bool IncludeTimestamps = true); ~ASTWriter() override; Index: clang/include/clang/Serialization/Module.h =================================================================== --- clang/include/clang/Serialization/Module.h +++ clang/include/clang/Serialization/Module.h @@ -163,9 +163,9 @@ /// \brief The generation of which this module file is a part. unsigned Generation; - /// \brief The memory buffer that stores the data associated with - /// this AST file. - std::unique_ptr Buffer; + /// The memory buffer that stores the data associated with + /// this AST file, owned by the PCMCache in the ModuleManager. + llvm::MemoryBuffer *Buffer; /// \brief The size of this file, in bits. uint64_t SizeInBits = 0; Index: clang/include/clang/Serialization/ModuleManager.h =================================================================== --- clang/include/clang/Serialization/ModuleManager.h +++ clang/include/clang/Serialization/ModuleManager.h @@ -24,6 +24,7 @@ namespace clang { class GlobalModuleIndex; +class MemoryBufferCache; class ModuleMap; class PCHContainerReader; @@ -51,6 +52,9 @@ /// FileEntry *. FileManager &FileMgr; + /// Cache of PCM files. + IntrusiveRefCntPtr PCMCache; + /// \brief Knows how to unwrap module containers. const PCHContainerReader &PCHContainerRdr; @@ -123,7 +127,7 @@ ModuleReverseIterator; typedef std::pair ModuleOffset; - explicit ModuleManager(FileManager &FileMgr, + explicit ModuleManager(FileManager &FileMgr, MemoryBufferCache &PCMCache, const PCHContainerReader &PCHContainerRdr); ~ModuleManager(); @@ -290,6 +294,8 @@ /// \brief View the graphviz representation of the module graph. void viewGraph(); + + MemoryBufferCache &getPCMCache() const { return *PCMCache; } }; } } // end namespace clang::serialization Index: clang/lib/Basic/CMakeLists.txt =================================================================== --- clang/lib/Basic/CMakeLists.txt +++ clang/lib/Basic/CMakeLists.txt @@ -74,6 +74,7 @@ FileSystemStatCache.cpp IdentifierTable.cpp LangOptions.cpp + MemoryBufferCache.cpp Module.cpp ObjCRuntime.cpp OpenMPKinds.cpp Index: clang/lib/Basic/MemoryBufferCache.cpp =================================================================== --- /dev/null +++ clang/lib/Basic/MemoryBufferCache.cpp @@ -0,0 +1,48 @@ +//===- MemoryBufferCache.cpp - Cache for loaded memory buffers ------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "clang/Basic/MemoryBufferCache.h" +#include "llvm/Support/MemoryBuffer.h" + +using namespace clang; + +llvm::MemoryBuffer & +MemoryBufferCache::addBuffer(llvm::StringRef Filename, + std::unique_ptr Buffer) { + auto Insertion = Buffers.insert(std::make_pair( + Filename, BufferEntry{std::move(Buffer), NextIndex++})); + assert(Insertion.second && "Already has a buffer"); + return *Insertion.first->second.Buffer; +} + +llvm::MemoryBuffer *MemoryBufferCache::lookupBuffer(llvm::StringRef Filename) { + auto I = Buffers.find(Filename); + if (I == Buffers.end()) + return nullptr; + return I->second.Buffer.get(); +} + +bool MemoryBufferCache::isBufferFinal(llvm::StringRef Filename) { + auto I = Buffers.find(Filename); + if (I == Buffers.end()) + return false; + return I->second.Index < FirstRemovableIndex; +} + +bool MemoryBufferCache::tryToRemoveBuffer(llvm::StringRef Filename) { + auto I = Buffers.find(Filename); + assert(I != Buffers.end() && "No buffer to remove..."); + if (I->second.Index < FirstRemovableIndex) + return true; + + Buffers.erase(I); + return false; +} + +void MemoryBufferCache::finalizeCurrentBuffers() { FirstRemovableIndex = NextIndex; } Index: clang/lib/Frontend/ASTUnit.cpp =================================================================== --- clang/lib/Frontend/ASTUnit.cpp +++ clang/lib/Frontend/ASTUnit.cpp @@ -18,6 +18,7 @@ #include "clang/AST/StmtVisitor.h" #include "clang/AST/TypeOrdering.h" #include "clang/Basic/Diagnostic.h" +#include "clang/Basic/MemoryBufferCache.h" #include "clang/Basic/TargetInfo.h" #include "clang/Basic/TargetOptions.h" #include "clang/Basic/VirtualFileSystem.h" @@ -185,7 +186,8 @@ llvm::BitstreamWriter Stream; ASTWriter Writer; - ASTWriterData() : Stream(Buffer), Writer(Stream, Buffer, {}) {} + ASTWriterData(MemoryBufferCache &PCMCache) + : Stream(Buffer), Writer(Stream, Buffer, PCMCache, {}) {} }; void ASTUnit::clearFileLevelDecls() { @@ -681,6 +683,7 @@ AST->SourceMgr = new SourceManager(AST->getDiagnostics(), AST->getFileManager(), UserFilesAreVolatile); + AST->PCMCache = new MemoryBufferCache; AST->HSOpts = std::make_shared(); AST->HSOpts->ModuleFormat = PCHContainerRdr.getFormat(); AST->HeaderInfo.reset(new HeaderSearch(AST->HSOpts, @@ -701,7 +704,7 @@ AST->PP = std::make_shared( std::move(PPOpts), AST->getDiagnostics(), AST->ASTFileLangOpts, - AST->getSourceManager(), HeaderInfo, *AST, + AST->getSourceManager(), *AST->PCMCache, HeaderInfo, *AST, /*IILookup=*/nullptr, /*OwnsHeaderSearch=*/false); Preprocessor &PP = *AST->PP; @@ -1727,6 +1730,7 @@ AST->UserFilesAreVolatile = UserFilesAreVolatile; AST->SourceMgr = new SourceManager(AST->getDiagnostics(), *AST->FileMgr, UserFilesAreVolatile); + AST->PCMCache = new MemoryBufferCache; return AST; } @@ -1997,6 +2001,7 @@ if (!VFS) return nullptr; AST->FileMgr = new FileManager(AST->FileSystemOpts, VFS); + AST->PCMCache = new MemoryBufferCache; AST->OnlyLocalDecls = OnlyLocalDecls; AST->CaptureDiagnostics = CaptureDiagnostics; AST->TUKind = TUKind; @@ -2008,7 +2013,7 @@ AST->StoredDiagnostics.swap(StoredDiagnostics); AST->Invocation = CI; if (ForSerialization) - AST->WriterData.reset(new ASTWriterData()); + AST->WriterData.reset(new ASTWriterData(*AST->PCMCache)); // Zero out now to ease cleanup during crash recovery. CI = nullptr; Diags = nullptr; @@ -2523,7 +2528,8 @@ SmallString<128> Buffer; llvm::BitstreamWriter Stream(Buffer); - ASTWriter Writer(Stream, Buffer, {}); + MemoryBufferCache PCMCache; + ASTWriter Writer(Stream, Buffer, PCMCache, {}); return serializeUnit(Writer, Buffer, getSema(), hasErrors, OS); } Index: clang/lib/Frontend/CompilerInstance.cpp =================================================================== --- clang/lib/Frontend/CompilerInstance.cpp +++ clang/lib/Frontend/CompilerInstance.cpp @@ -13,6 +13,7 @@ #include "clang/AST/Decl.h" #include "clang/Basic/Diagnostic.h" #include "clang/Basic/FileManager.h" +#include "clang/Basic/MemoryBufferCache.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TargetInfo.h" #include "clang/Basic/Version.h" @@ -55,9 +56,15 @@ CompilerInstance::CompilerInstance( std::shared_ptr PCHContainerOps, - bool BuildingModule) - : ModuleLoader(BuildingModule), Invocation(new CompilerInvocation()), - ThePCHContainerOperations(std::move(PCHContainerOps)) {} + MemoryBufferCache *SharedPCMCache) + : ModuleLoader(/* BuildingModule = */ SharedPCMCache), + Invocation(new CompilerInvocation()), + PCMCache(SharedPCMCache ? SharedPCMCache : new MemoryBufferCache), + ThePCHContainerOperations(std::move(PCHContainerOps)) { + // Don't allow this to invalidate buffers in use by others. + if (SharedPCMCache) + getPCMCache().finalizeCurrentBuffers(); +} CompilerInstance::~CompilerInstance() { assert(OutputFiles.empty() && "Still output files in flight?"); @@ -128,6 +135,8 @@ return ModuleManager; } void CompilerInstance::setModuleManager(IntrusiveRefCntPtr Reader) { + assert(PCMCache.get() == &Reader->getModuleManager().getPCMCache() && + "Expected ASTReader to use the same PCM cache"); ModuleManager = std::move(Reader); } @@ -370,7 +379,7 @@ getDiagnostics(), getLangOpts(), &getTarget()); PP = std::make_shared( Invocation->getPreprocessorOptsPtr(), getDiagnostics(), getLangOpts(), - getSourceManager(), *HeaderInfo, *this, PTHMgr, + getSourceManager(), getPCMCache(), *HeaderInfo, *this, PTHMgr, /*OwnsHeaderSearch=*/true, TUKind); PP->Initialize(getTarget(), getAuxTarget()); @@ -1073,9 +1082,11 @@ Invocation->getModuleHash() && "Module hash mismatch!"); // Construct a compiler instance that will be used to actually create the - // module. + // module. Since we're sharing a PCMCache, + // CompilerInstance::CompilerInstance is responsible for finalizing the + // buffers to prevent use-after-frees. CompilerInstance Instance(ImportingInstance.getPCHContainerOperations(), - /*BuildingModule=*/true); + &ImportingInstance.getPreprocessor().getPCMCache()); auto &Inv = *Invocation; Instance.setInvocation(std::move(Invocation)); Index: clang/lib/Lex/Preprocessor.cpp =================================================================== --- clang/lib/Lex/Preprocessor.cpp +++ clang/lib/Lex/Preprocessor.cpp @@ -70,15 +70,15 @@ Preprocessor::Preprocessor(std::shared_ptr PPOpts, DiagnosticsEngine &diags, LangOptions &opts, - SourceManager &SM, HeaderSearch &Headers, - ModuleLoader &TheModuleLoader, + SourceManager &SM, MemoryBufferCache &PCMCache, + HeaderSearch &Headers, ModuleLoader &TheModuleLoader, IdentifierInfoLookup *IILookup, bool OwnsHeaders, TranslationUnitKind TUKind) : PPOpts(std::move(PPOpts)), Diags(&diags), LangOpts(opts), Target(nullptr), AuxTarget(nullptr), FileMgr(Headers.getFileMgr()), SourceMgr(SM), - ScratchBuf(new ScratchBuffer(SourceMgr)), HeaderInfo(Headers), - TheModuleLoader(TheModuleLoader), ExternalSource(nullptr), - Identifiers(opts, IILookup), + PCMCache(PCMCache), ScratchBuf(new ScratchBuffer(SourceMgr)), + HeaderInfo(Headers), TheModuleLoader(TheModuleLoader), + ExternalSource(nullptr), Identifiers(opts, IILookup), PragmaHandlers(new PragmaNamespace(StringRef())), IncrementalProcessing(false), TUKind(TUKind), CodeComplete(nullptr), CodeCompletionFile(nullptr), CodeCompletionOffset(0), Index: clang/lib/Serialization/ASTReader.cpp =================================================================== --- clang/lib/Serialization/ASTReader.cpp +++ clang/lib/Serialization/ASTReader.cpp @@ -37,6 +37,7 @@ #include "clang/Basic/FileManager.h" #include "clang/Basic/FileSystemOptions.h" #include "clang/Basic/LangOptions.h" +#include "clang/Basic/MemoryBufferCache.h" #include "clang/Basic/ObjCRuntime.h" #include "clang/Basic/OperatorKinds.h" #include "clang/Basic/Sanitizers.h" @@ -463,19 +464,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 @@ -487,17 +478,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 @@ -4064,12 +4075,41 @@ Listener.get(), WasImportedBy ? false : HSOpts.ModulesValidateDiagnosticOptions); + // If F was directly imported by another module, it's implicitly validated by + // the importing module. if (DisableValidation || WasImportedBy || (AllowConfigurationMismatch && Result == ConfigurationMismatch)) return Success; - if (Result == Failure) + if (Result == Failure) { Error("malformed block record in AST file"); + return Failure; + } + + if (Result == OutOfDate && F.Kind == MK_ImplicitModule) { + // If this module has already been finalized in the PCMCache, we're stuck + // with it; we can only load a single version of each module. + // + // This can happen when a module is imported in two contexts: in one, as a + // user module; in another, as a system module (due to an import from + // another module marked with the [system] flag). It usually indicates a + // bug in the module map: this module should also be marked with [system]. + // + // If -Wno-system-headers (the default), and the first import is as a + // system module, then validation will fail during the as-user import, + // since -Werror flags won't have been validated. However, it's reasonable + // to treat this consistently as a system module. + // + // If -Wsystem-headers, the PCM on disk was built with + // -Wno-system-headers, and the first import is as a user module, then + // validation will fail during the as-system import since the PCM on disk + // doesn't guarantee that -Werror was respected. However, the -Werror + // flags were checked during the initial as-user import. + if (PCMCache.isBufferFinal(F.FileName)) { + Diag(diag::warn_module_system_bit_conflict) << F.FileName; + return Success; + } + } return Result; } @@ -4122,7 +4162,7 @@ if (Listener && ValidateDiagnosticOptions && !AllowCompatibleConfigurationMismatch && ParseDiagnosticOptions(Record, Complain, *Listener)) - return OutOfDate; + Result = OutOfDate; // Don't return early. Read the signature. break; } case DIAG_PRAGMA_MAPPINGS: @@ -7301,7 +7341,7 @@ /// by heap-backed versus mmap'ed memory. void ASTReader::getMemoryBufferSizes(MemoryBufferSizes &sizes) const { for (ModuleFile &I : ModuleMgr) { - 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: @@ -9628,8 +9668,10 @@ : cast(new PCHValidator(PP, *this))), SourceMgr(PP.getSourceManager()), FileMgr(PP.getFileManager()), PCHContainerRdr(PCHContainerRdr), Diags(PP.getDiagnostics()), PP(PP), - Context(Context), ModuleMgr(PP.getFileManager(), PCHContainerRdr), - DummyIdResolver(PP), ReadTimer(std::move(ReadTimer)), isysroot(isysroot), + Context(Context), + ModuleMgr(PP.getFileManager(), PP.getPCMCache(), PCHContainerRdr), + PCMCache(PP.getPCMCache()), DummyIdResolver(PP), + ReadTimer(std::move(ReadTimer)), isysroot(isysroot), DisableValidation(DisableValidation), AllowASTWithCompilerErrors(AllowASTWithCompilerErrors), AllowConfigurationMismatch(AllowConfigurationMismatch), Index: clang/lib/Serialization/ASTWriter.cpp =================================================================== --- clang/lib/Serialization/ASTWriter.cpp +++ clang/lib/Serialization/ASTWriter.cpp @@ -35,6 +35,7 @@ #include "clang/Basic/FileSystemOptions.h" #include "clang/Basic/LLVM.h" #include "clang/Basic/LangOptions.h" +#include "clang/Basic/MemoryBufferCache.h" #include "clang/Basic/Module.h" #include "clang/Basic/ObjCRuntime.h" #include "clang/Basic/SourceManager.h" @@ -4304,10 +4305,11 @@ } ASTWriter::ASTWriter(llvm::BitstreamWriter &Stream, - SmallVectorImpl &Buffer, + SmallVectorImpl &Buffer, MemoryBufferCache &PCMCache, ArrayRef> Extensions, bool IncludeTimestamps) - : Stream(Stream), Buffer(Buffer), IncludeTimestamps(IncludeTimestamps) { + : Stream(Stream), Buffer(Buffer), PCMCache(PCMCache), + IncludeTimestamps(IncludeTimestamps) { for (const auto &Ext : Extensions) { if (auto Writer = Ext->createExtensionWriter(*this)) ModuleFileExtensionWriters.push_back(std::move(Writer)); @@ -4354,6 +4356,12 @@ this->BaseDirectory.clear(); WritingAST = false; + if (SemaRef.Context.getLangOpts().ImplicitModules && WritingModule) { + // Construct MemoryBuffer and update buffer manager. + PCMCache.addBuffer(OutputFile, + llvm::MemoryBuffer::getMemBufferCopy( + StringRef(Buffer.begin(), Buffer.size()))); + } return Signature; } Index: clang/lib/Serialization/GeneratePCH.cpp =================================================================== --- clang/lib/Serialization/GeneratePCH.cpp +++ clang/lib/Serialization/GeneratePCH.cpp @@ -28,7 +28,8 @@ 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, PP.getPCMCache(), Extensions, + IncludeTimestamps), AllowASTWithErrors(AllowASTWithErrors) { Buffer->IsComplete = false; } Index: clang/lib/Serialization/ModuleManager.cpp =================================================================== --- clang/lib/Serialization/ModuleManager.cpp +++ clang/lib/Serialization/ModuleManager.cpp @@ -12,6 +12,7 @@ // //===----------------------------------------------------------------------===// #include "clang/Serialization/ModuleManager.h" +#include "clang/Basic/MemoryBufferCache.h" #include "clang/Frontend/PCHContainerOperations.h" #include "clang/Lex/HeaderSearch.h" #include "clang/Lex/ModuleMap.h" @@ -137,7 +138,9 @@ // Load the contents of the module if (std::unique_ptr Buffer = lookupBuffer(FileName)) { // The buffer was already provided for us. - NewModule->Buffer = std::move(Buffer); + NewModule->Buffer = &PCMCache->addBuffer(FileName, std::move(Buffer)); + } else if (llvm::MemoryBuffer *Buffer = PCMCache->lookupBuffer(FileName)) { + NewModule->Buffer = Buffer; } else { // Open the AST file. llvm::ErrorOr> Buf((std::error_code())); @@ -158,7 +161,7 @@ return Missing; } - NewModule->Buffer = std::move(*Buf); + NewModule->Buffer = &PCMCache->addBuffer(FileName, std::move(*Buf)); } // Initialize the stream. @@ -167,8 +170,13 @@ // Read the signature eagerly now so that we can check it. Avoid calling // ReadSignature unless there's something to check though. if (ExpectedSignature && checkSignature(ReadSignature(NewModule->Data), - ExpectedSignature, ErrorStr)) + ExpectedSignature, ErrorStr)) { + // Try to remove the buffer. If it can't be removed, then it was already + // validated by this process. + if (!PCMCache->tryToRemoveBuffer(NewModule->FileName)) + FileMgr.invalidateCache(NewModule->File); return OutOfDate; + } // We're keeping this module. Store it everywhere. Module = Modules[Entry] = NewModule.get(); @@ -235,7 +243,12 @@ // 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) + // + // The PCMCache tracks whether the module was succesfully loaded in another + // thread/context; in that case, it won't need to be rebuilt (and we can't + // safely invalidate it anyway). + if (LoadedSuccessfully.count(&*victim) == 0 && + !PCMCache->tryToRemoveBuffer(victim->FileName)) FileMgr.invalidateCache(victim->File); } @@ -292,10 +305,10 @@ ModulesInCommonWithGlobalIndex.push_back(MF); } -ModuleManager::ModuleManager(FileManager &FileMgr, +ModuleManager::ModuleManager(FileManager &FileMgr, MemoryBufferCache &PCMCache, const PCHContainerReader &PCHContainerRdr) - : FileMgr(FileMgr), PCHContainerRdr(PCHContainerRdr), GlobalIndex(), - FirstVisitState(nullptr) {} + : FileMgr(FileMgr), PCMCache(&PCMCache), PCHContainerRdr(PCHContainerRdr), + GlobalIndex(), FirstVisitState(nullptr) {} ModuleManager::~ModuleManager() { delete FirstVisitState; } Index: clang/test/Modules/Inputs/system-out-of-date/X.h =================================================================== --- /dev/null +++ clang/test/Modules/Inputs/system-out-of-date/X.h @@ -0,0 +1 @@ +#import Index: clang/test/Modules/Inputs/system-out-of-date/Y.h =================================================================== --- /dev/null +++ clang/test/Modules/Inputs/system-out-of-date/Y.h @@ -0,0 +1 @@ +//empty Index: clang/test/Modules/Inputs/system-out-of-date/Z.h =================================================================== --- /dev/null +++ clang/test/Modules/Inputs/system-out-of-date/Z.h @@ -0,0 +1 @@ +#import Index: clang/test/Modules/Inputs/system-out-of-date/module.map =================================================================== --- /dev/null +++ clang/test/Modules/Inputs/system-out-of-date/module.map @@ -0,0 +1,12 @@ +module X [system] { + header "X.h" // imports Y + export * +} +module Y { + header "Y.h" + export * +} +module Z { + header "Z.h" // imports Y + export * +} Index: clang/test/Modules/Inputs/warning-mismatch/Mismatch.h =================================================================== --- /dev/null +++ clang/test/Modules/Inputs/warning-mismatch/Mismatch.h @@ -0,0 +1 @@ +struct Mismatch { int i; }; Index: clang/test/Modules/Inputs/warning-mismatch/System.h =================================================================== --- /dev/null +++ clang/test/Modules/Inputs/warning-mismatch/System.h @@ -0,0 +1,2 @@ +#import "Mismatch.h" +struct System { int i; }; Index: clang/test/Modules/Inputs/warning-mismatch/module.modulemap =================================================================== --- /dev/null +++ clang/test/Modules/Inputs/warning-mismatch/module.modulemap @@ -0,0 +1,7 @@ +module System [system] { + header "System.h" +} + +module Mismatch { + header "Mismatch.h" +} Index: clang/test/Modules/outofdate-rebuild.m =================================================================== --- /dev/null +++ clang/test/Modules/outofdate-rebuild.m @@ -0,0 +1,15 @@ +// RUN: rm -rf %t.cache +// RUN: echo "@import CoreText;" > %t.m +// RUN: %clang_cc1 -fdisable-module-hash -fmodules-cache-path=%t.cache \ +// RUN: -fmodules -fimplicit-module-maps -I%S/Inputs/outofdate-rebuild %s \ +// RUN: -fsyntax-only -Rmodule-build +// RUN: %clang_cc1 -DMISMATCH -Werror -fdisable-module-hash \ +// RUN: -fmodules-cache-path=%t.cache -fmodules -fimplicit-module-maps \ +// RUN: -I%S/Inputs/outofdate-rebuild %t.m -fsyntax-only -Rmodule-build +// RUN: %clang_cc1 -fdisable-module-hash -fmodules-cache-path=%t.cache \ +// RUN: -fmodules -fimplicit-module-maps -I%S/Inputs/outofdate-rebuild %s \ +// RUN: -fsyntax-only -Rmodule-build + +// This testcase reproduces a use-after-free in when ModuleManager removes an +// entry from the PCMCache without notifying its parent ASTReader. +@import Cocoa; Index: clang/test/Modules/system-out-of-date-test.m =================================================================== --- /dev/null +++ clang/test/Modules/system-out-of-date-test.m @@ -0,0 +1,17 @@ +// RUN: rm -rf %t.cache +// RUN: echo '@import X;' | \ +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps \ +// RUN: -fmodules-cache-path=%t.cache -I%S/Inputs/system-out-of-date \ +// RUN: -fsyntax-only -x objective-c - +// +// Build something with different diagnostic options. +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps \ +// RUN: -fmodules-cache-path=%t.cache -I%S/Inputs/system-out-of-date \ +// RUN: -fsyntax-only %s -Wnon-modular-include-in-framework-module \ +// RUN: -Werror=non-modular-include-in-framework-module 2>&1 \ +// RUN: | FileCheck %s +@import X; + +#import +// CHECK: While building module 'Z' imported from +// CHECK: {{.*}}Y-{{.*}}pcm' was validated as a system module and is now being imported as a non-system module Index: clang/test/Modules/warning-mismatch.m =================================================================== --- /dev/null +++ clang/test/Modules/warning-mismatch.m @@ -0,0 +1,13 @@ +// RUN: rm -rf %t.cache +// RUN: echo "@import Mismatch;" >%t.m +// RUN: %clang_cc1 -Wno-system-headers -fdisable-module-hash \ +// RUN: -fmodules-cache-path=%t.cache -fmodules -fimplicit-module-maps \ +// RUN: -I%S/Inputs/warning-mismatch %t.m -fsyntax-only -Rmodule-build +// RUN: %clang_cc1 -Wsystem-headers -fdisable-module-hash \ +// RUN: -fmodules-cache-path=%t.cache -fmodules -fimplicit-module-maps \ +// RUN: -I%S/Inputs/warning-mismatch %s -fsyntax-only -Rmodule-build + +// This testcase triggers a warning flag mismatch in an already validated +// header. +@import Mismatch; +@import System; Index: clang/unittests/Basic/CMakeLists.txt =================================================================== --- clang/unittests/Basic/CMakeLists.txt +++ clang/unittests/Basic/CMakeLists.txt @@ -6,6 +6,7 @@ CharInfoTest.cpp DiagnosticTest.cpp FileManagerTest.cpp + MemoryBufferCacheTest.cpp SourceManagerTest.cpp VirtualFileSystemTest.cpp ) Index: clang/unittests/Basic/MemoryBufferCacheTest.cpp =================================================================== --- /dev/null +++ clang/unittests/Basic/MemoryBufferCacheTest.cpp @@ -0,0 +1,94 @@ +//===- MemoryBufferCacheTest.cpp - MemoryBufferCache tests ----------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "clang/Basic/MemoryBufferCache.h" +#include "llvm/Support/MemoryBuffer.h" +#include "gtest/gtest.h" + +using namespace llvm; +using namespace clang; + +namespace { + +std::unique_ptr getBuffer(int I) { + SmallVector Bytes; + raw_svector_ostream(Bytes) << "data:" << I; + return MemoryBuffer::getMemBuffer(StringRef(Bytes.data(), Bytes.size())); +} + +TEST(MemoryBufferCacheTest, addBuffer) { + auto B1 = getBuffer(1); + auto B2 = getBuffer(2); + auto B3 = getBuffer(3); + auto *RawB1 = B1.get(); + auto *RawB2 = B2.get(); + auto *RawB3 = B3.get(); + + // Add a few buffers. + MemoryBufferCache Cache; + EXPECT_EQ(RawB1, &Cache.addBuffer("1", std::move(B1))); + EXPECT_EQ(RawB2, &Cache.addBuffer("2", std::move(B2))); + EXPECT_EQ(RawB3, &Cache.addBuffer("3", std::move(B3))); + EXPECT_EQ(RawB1, Cache.lookupBuffer("1")); + EXPECT_EQ(RawB2, Cache.lookupBuffer("2")); + EXPECT_EQ(RawB3, Cache.lookupBuffer("3")); + EXPECT_FALSE(Cache.isBufferFinal("1")); + EXPECT_FALSE(Cache.isBufferFinal("2")); + EXPECT_FALSE(Cache.isBufferFinal("3")); + + // Remove the middle buffer. + EXPECT_FALSE(Cache.tryToRemoveBuffer("2")); + EXPECT_EQ(nullptr, Cache.lookupBuffer("2")); + EXPECT_FALSE(Cache.isBufferFinal("2")); + + // Replace the middle buffer. + B2 = getBuffer(2); + ASSERT_NE(RawB2, B2.get()); + RawB2 = B2.get(); + EXPECT_EQ(RawB2, &Cache.addBuffer("2", std::move(B2))); + + // Check that nothing is final. + EXPECT_FALSE(Cache.isBufferFinal("1")); + EXPECT_FALSE(Cache.isBufferFinal("2")); + EXPECT_FALSE(Cache.isBufferFinal("3")); +} + +TEST(MemoryBufferCacheTest, finalizeCurrentBuffers) { + // Add a buffer. + MemoryBufferCache Cache; + auto B1 = getBuffer(1); + auto *RawB1 = B1.get(); + Cache.addBuffer("1", std::move(B1)); + ASSERT_FALSE(Cache.isBufferFinal("1")); + + // Finalize it. + Cache.finalizeCurrentBuffers(); + EXPECT_TRUE(Cache.isBufferFinal("1")); + EXPECT_TRUE(Cache.tryToRemoveBuffer("1")); + EXPECT_EQ(RawB1, Cache.lookupBuffer("1")); + EXPECT_TRUE(Cache.isBufferFinal("1")); + + // Repeat. + auto B2 = getBuffer(2); + auto *RawB2 = B2.get(); + Cache.addBuffer("2", std::move(B2)); + EXPECT_FALSE(Cache.isBufferFinal("2")); + + Cache.finalizeCurrentBuffers(); + EXPECT_TRUE(Cache.isBufferFinal("1")); + EXPECT_TRUE(Cache.isBufferFinal("2")); + EXPECT_TRUE(Cache.tryToRemoveBuffer("1")); + EXPECT_TRUE(Cache.tryToRemoveBuffer("2")); + EXPECT_EQ(RawB1, Cache.lookupBuffer("1")); + EXPECT_EQ(RawB2, Cache.lookupBuffer("2")); + EXPECT_TRUE(Cache.isBufferFinal("1")); + EXPECT_TRUE(Cache.isBufferFinal("2")); +} + +} // namespace Index: clang/unittests/Basic/SourceManagerTest.cpp =================================================================== --- clang/unittests/Basic/SourceManagerTest.cpp +++ clang/unittests/Basic/SourceManagerTest.cpp @@ -12,6 +12,7 @@ #include "clang/Basic/DiagnosticOptions.h" #include "clang/Basic/FileManager.h" #include "clang/Basic/LangOptions.h" +#include "clang/Basic/MemoryBufferCache.h" #include "clang/Basic/TargetInfo.h" #include "clang/Basic/TargetOptions.h" #include "clang/Lex/HeaderSearch.h" @@ -78,10 +79,11 @@ SourceMgr.setMainFileID(mainFileID); VoidModuleLoader ModLoader; + MemoryBufferCache PCMCache; HeaderSearch HeaderInfo(std::make_shared(), SourceMgr, Diags, LangOpts, &*Target); Preprocessor PP(std::make_shared(), Diags, LangOpts, - SourceMgr, HeaderInfo, ModLoader, + SourceMgr, PCMCache, HeaderInfo, ModLoader, /*IILookup =*/nullptr, /*OwnsHeaderSearch =*/false); PP.Initialize(*Target); @@ -198,10 +200,11 @@ SourceMgr.overrideFileContents(headerFile, std::move(HeaderBuf)); VoidModuleLoader ModLoader; + MemoryBufferCache PCMCache; HeaderSearch HeaderInfo(std::make_shared(), SourceMgr, Diags, LangOpts, &*Target); Preprocessor PP(std::make_shared(), Diags, LangOpts, - SourceMgr, HeaderInfo, ModLoader, + SourceMgr, PCMCache, HeaderInfo, ModLoader, /*IILookup =*/nullptr, /*OwnsHeaderSearch =*/false); PP.Initialize(*Target); @@ -298,10 +301,11 @@ SourceMgr.overrideFileContents(headerFile, std::move(HeaderBuf)); VoidModuleLoader ModLoader; + MemoryBufferCache PCMCache; HeaderSearch HeaderInfo(std::make_shared(), SourceMgr, Diags, LangOpts, &*Target); Preprocessor PP(std::make_shared(), Diags, LangOpts, - SourceMgr, HeaderInfo, ModLoader, + SourceMgr, PCMCache, HeaderInfo, ModLoader, /*IILookup =*/nullptr, /*OwnsHeaderSearch =*/false); PP.Initialize(*Target); Index: clang/unittests/Lex/LexerTest.cpp =================================================================== --- clang/unittests/Lex/LexerTest.cpp +++ clang/unittests/Lex/LexerTest.cpp @@ -12,6 +12,7 @@ #include "clang/Basic/DiagnosticOptions.h" #include "clang/Basic/FileManager.h" #include "clang/Basic/LangOptions.h" +#include "clang/Basic/MemoryBufferCache.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TargetInfo.h" #include "clang/Basic/TargetOptions.h" @@ -64,10 +65,12 @@ SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(Buf))); VoidModuleLoader ModLoader; + MemoryBufferCache PCMCache; HeaderSearch HeaderInfo(std::make_shared(), SourceMgr, Diags, LangOpts, Target.get()); Preprocessor PP(std::make_shared(), Diags, LangOpts, - SourceMgr, HeaderInfo, ModLoader, /*IILookup =*/nullptr, + SourceMgr, PCMCache, HeaderInfo, ModLoader, + /*IILookup =*/nullptr, /*OwnsHeaderSearch =*/false); PP.Initialize(*Target); PP.EnterMainSourceFile(); Index: clang/unittests/Lex/PPCallbacksTest.cpp =================================================================== --- clang/unittests/Lex/PPCallbacksTest.cpp +++ clang/unittests/Lex/PPCallbacksTest.cpp @@ -14,6 +14,7 @@ #include "clang/Basic/DiagnosticOptions.h" #include "clang/Basic/FileManager.h" #include "clang/Basic/LangOptions.h" +#include "clang/Basic/MemoryBufferCache.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TargetInfo.h" #include "clang/Basic/TargetOptions.h" @@ -161,13 +162,14 @@ SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(Buf))); VoidModuleLoader ModLoader; + MemoryBufferCache PCMCache; HeaderSearch HeaderInfo(std::make_shared(), SourceMgr, Diags, LangOpts, Target.get()); AddFakeHeader(HeaderInfo, HeaderPath, SystemHeader); Preprocessor PP(std::make_shared(), Diags, LangOpts, - SourceMgr, HeaderInfo, ModLoader, + SourceMgr, PCMCache, HeaderInfo, ModLoader, /*IILookup =*/nullptr, /*OwnsHeaderSearch =*/false); PP.Initialize(*Target); @@ -198,11 +200,12 @@ SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(SourceBuf))); VoidModuleLoader ModLoader; + MemoryBufferCache PCMCache; HeaderSearch HeaderInfo(std::make_shared(), SourceMgr, Diags, OpenCLLangOpts, Target.get()); Preprocessor PP(std::make_shared(), Diags, - OpenCLLangOpts, SourceMgr, HeaderInfo, ModLoader, + OpenCLLangOpts, SourceMgr, PCMCache, HeaderInfo, ModLoader, /*IILookup =*/nullptr, /*OwnsHeaderSearch =*/false); PP.Initialize(*Target); Index: clang/unittests/Lex/PPConditionalDirectiveRecordTest.cpp =================================================================== --- clang/unittests/Lex/PPConditionalDirectiveRecordTest.cpp +++ clang/unittests/Lex/PPConditionalDirectiveRecordTest.cpp @@ -12,6 +12,7 @@ #include "clang/Basic/DiagnosticOptions.h" #include "clang/Basic/FileManager.h" #include "clang/Basic/LangOptions.h" +#include "clang/Basic/MemoryBufferCache.h" #include "clang/Basic/SourceManager.h" #include "clang/Basic/TargetInfo.h" #include "clang/Basic/TargetOptions.h" @@ -93,10 +94,11 @@ SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(Buf))); VoidModuleLoader ModLoader; + MemoryBufferCache PCMCache; HeaderSearch HeaderInfo(std::make_shared(), SourceMgr, Diags, LangOpts, Target.get()); Preprocessor PP(std::make_shared(), Diags, LangOpts, - SourceMgr, HeaderInfo, ModLoader, + SourceMgr, PCMCache, HeaderInfo, ModLoader, /*IILookup =*/nullptr, /*OwnsHeaderSearch =*/false); PP.Initialize(*Target);