Modules: Cache PCMs in memory and avoid a use-after-free

Clang's internal build system for implicit modules uses lock files to
ensure that after a process writes a PCM it will read the same one back
in (without contention from other -cc1 commands).  Since PCMs are read
from disk repeatedly while invalidating, building, and importing, the
lock is not released quickly.  Furthermore, the LockFileManager is not
robust in every environment.  Other -cc1 commands can stall until
timeout (after about eight minutes).

This commit changes the lock file from being necessary for correctness
to a (possibly dubious) performance hack.  The remaining benefit is to
reduce duplicate work in competing -cc1 commands which depend on the
same module.  Follow-up commits will change the internal build system to
continue after a timeout, and reduce the timeout.  Perhaps we should
reconsider blocking at all.

This also fixes a use-after-free, when one part of a compilation
validates a PCM and starts using it, and another tries to swap out the
PCM for something new.

The PCMCache is a new type called MemoryBufferCache, which saves memory
buffers based on their filename.  Its ownership is shared by the
CompilerInstance and ModuleManager.

  - The ModuleManager stores PCMs there that it loads from disk, never
    touching the disk if the cache is hot.

  - When modules fail to validate, they're removed from the cache.

  - When a CompilerInstance is spawned to build a new module, each
    already-loaded PCM is assumed to be valid, and is frozen to avoid
    the use-after-free.

  - Any newly-built module is written directly to the cache to avoid the
    round-trip to the filesystem, making lock files unnecessary for
    correctness.

Original patch by Manman Ren; most testcases by Adrian Prantl!

llvm-svn: 298165

clang/include/clang/Basic/DiagnosticSerializationKinds.td

@@ -176,6 +176,11 @@ def warn_duplicate_module_file_extension : Warning<
   "duplicate module file extension block name '%0'">,
   InGroup<ModuleFileExtension>;
 
+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<ModuleConflict>;
+
 } // let CategoryName
 } // let Component
 

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 <memory>
+
+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<MemoryBufferCache> {
+  struct BufferEntry {
+    std::unique_ptr<llvm::MemoryBuffer> Buffer;
+
+    /// Track the timeline of when this was added to the cache.
+    unsigned Index;
+  };
+
+  /// Cache of buffers.
+  llvm::StringMap<BufferEntry> 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<llvm::MemoryBuffer> 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

clang/include/clang/Frontend/ASTUnit.h

@@ -51,6 +51,7 @@ class DiagnosticsEngine;
 class FileEntry;
 class FileManager;
 class HeaderSearch;
+class MemoryBufferCache;
 class Preprocessor;
 class PCHContainerOperations;
 class PCHContainerReader;
@@ -84,6 +85,7 @@ private:
   IntrusiveRefCntPtr<DiagnosticsEngine>   Diagnostics;
   IntrusiveRefCntPtr<FileManager>         FileMgr;
   IntrusiveRefCntPtr<SourceManager>       SourceMgr;
+  IntrusiveRefCntPtr<MemoryBufferCache>   PCMCache;
   std::unique_ptr<HeaderSearch>           HeaderInfo;
   IntrusiveRefCntPtr<TargetInfo>          Target;
   std::shared_ptr<Preprocessor>           PP;

clang/include/clang/Frontend/CompilerInstance.h

@@ -44,6 +44,7 @@ class ExternalASTSource;
 class FileEntry;
 class FileManager;
 class FrontendAction;
+class MemoryBufferCache;
 class Module;
 class Preprocessor;
 class Sema;
@@ -90,6 +91,9 @@ class CompilerInstance : public ModuleLoader {
   /// The source manager.
   IntrusiveRefCntPtr<SourceManager> SourceMgr;
 
+  /// The cache of PCM files.
+  IntrusiveRefCntPtr<MemoryBufferCache> PCMCache;
+
   /// The preprocessor.
   std::shared_ptr<Preprocessor> PP;
 
@@ -178,7 +182,7 @@ public:
   explicit CompilerInstance(
       std::shared_ptr<PCHContainerOperations> PCHContainerOps =
           std::make_shared<PCHContainerOperations>(),
-      bool BuildingModule = false);
+      MemoryBufferCache *SharedPCMCache = nullptr);
   ~CompilerInstance() override;
 
   /// @name High-Level Operations
@@ -783,6 +787,8 @@ public:
   }
 
   void setExternalSemaSource(IntrusiveRefCntPtr<ExternalSemaSource> ESS);
+
+  MemoryBufferCache &getPCMCache() const { return *PCMCache; }
 };
 
 } // end namespace clang

clang/include/clang/Lex/Preprocessor.h

@@ -47,6 +47,7 @@ class ExternalPreprocessorSource;
 class FileManager;
 class FileEntry;
 class HeaderSearch;
+class MemoryBufferCache;
 class PragmaNamespace;
 class PragmaHandler;
 class CommentHandler;
@@ -102,6 +103,7 @@ class Preprocessor {
   const TargetInfo  *AuxTarget;
   FileManager       &FileMgr;
   SourceManager     &SourceMgr;
+  MemoryBufferCache &PCMCache;
   std::unique_ptr<ScratchBuffer> ScratchBuf;
   HeaderSearch      &HeaderInfo;
   ModuleLoader      &TheModuleLoader;
@@ -652,6 +654,7 @@ class Preprocessor {
 public:
   Preprocessor(std::shared_ptr<PreprocessorOptions> PPOpts,
                DiagnosticsEngine &diags, LangOptions &opts, SourceManager &SM,
+               MemoryBufferCache &PCMCache,
                HeaderSearch &Headers, ModuleLoader &TheModuleLoader,
                IdentifierInfoLookup *IILookup = nullptr,
                bool OwnsHeaderSearch = false,
@@ -691,6 +694,7 @@ public:
   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; }

clang/include/clang/Serialization/ASTReader.h

@@ -408,6 +408,9 @@ private:
   /// \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.

clang/include/clang/Serialization/ASTWriter.h

@@ -54,6 +54,7 @@ class MacroInfo;
 class OpaqueValueExpr;
 class OpenCLOptions;
 class ASTReader;
+class MemoryBufferCache;
 class Module;
 class ModuleFileExtension;
 class ModuleFileExtensionWriter;
@@ -109,6 +110,9 @@ private:
   /// The buffer associated with the bitstream.
   const SmallVectorImpl<char> &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 @@ public:
   /// \brief Create a new precompiled header writer that outputs to
   /// the given bitstream.
   ASTWriter(llvm::BitstreamWriter &Stream, SmallVectorImpl<char> &Buffer,
+            MemoryBufferCache &PCMCache,
             ArrayRef<std::shared_ptr<ModuleFileExtension>> Extensions,
             bool IncludeTimestamps = true);
   ~ASTWriter() override;

clang/include/clang/Serialization/Module.h

@@ -163,9 +163,9 @@ public:
   /// \brief The generation of which this module file is a part.
   unsigned Generation;
   
-  /// \brief The memory buffer that stores the data associated with
+  /// The memory buffer that stores the data associated with
-  /// this AST file.
+  /// this AST file, owned by the PCMCache in the ModuleManager.
-  std::unique_ptr<llvm::MemoryBuffer> Buffer;
+  llvm::MemoryBuffer *Buffer;
 
   /// \brief The size of this file, in bits.
   uint64_t SizeInBits = 0;

clang/include/clang/Serialization/ModuleManager.h

@@ -24,6 +24,7 @@
 namespace clang { 
 
 class GlobalModuleIndex;
+class MemoryBufferCache;
 class ModuleMap;
 class PCHContainerReader;
 
@@ -51,6 +52,9 @@ class ModuleManager {
   /// FileEntry *.
   FileManager &FileMgr;
 
+  /// Cache of PCM files.
+  IntrusiveRefCntPtr<MemoryBufferCache> PCMCache;
+
   /// \brief Knows how to unwrap module containers.
   const PCHContainerReader &PCHContainerRdr;
 
@@ -123,7 +127,7 @@ public:
       ModuleReverseIterator;
   typedef std::pair<uint32_t, StringRef> ModuleOffset;
 
-  explicit ModuleManager(FileManager &FileMgr,
+  explicit ModuleManager(FileManager &FileMgr, MemoryBufferCache &PCMCache,
                          const PCHContainerReader &PCHContainerRdr);
   ~ModuleManager();
 
@@ -290,6 +294,8 @@ public:
 
   /// \brief View the graphviz representation of the module graph.
   void viewGraph();
+
+  MemoryBufferCache &getPCMCache() const { return *PCMCache; }
 };
 
 } } // end namespace clang::serialization

clang/lib/Basic/CMakeLists.txt

@@ -74,6 +74,7 @@ add_clang_library(clangBasic
   FileSystemStatCache.cpp
   IdentifierTable.cpp
   LangOptions.cpp
+  MemoryBufferCache.cpp
   Module.cpp
   ObjCRuntime.cpp
   OpenMPKinds.cpp

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<llvm::MemoryBuffer> Buffer) {
+  auto Insertion =
+      Buffers.insert({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; }

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 @@ struct ASTUnit::ASTWriterData {
   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 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromASTFile(
   AST->SourceMgr = new SourceManager(AST->getDiagnostics(),
                                      AST->getFileManager(),
                                      UserFilesAreVolatile);
+  AST->PCMCache = new MemoryBufferCache;
   AST->HSOpts = std::make_shared<HeaderSearchOptions>();
   AST->HSOpts->ModuleFormat = PCHContainerRdr.getFormat();
   AST->HeaderInfo.reset(new HeaderSearch(AST->HSOpts,
@@ -701,7 +704,7 @@ std::unique_ptr<ASTUnit> ASTUnit::LoadFromASTFile(
 
   AST->PP = std::make_shared<Preprocessor>(
       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 @@ ASTUnit::create(std::shared_ptr<CompilerInvocation> CI,
   AST->UserFilesAreVolatile = UserFilesAreVolatile;
   AST->SourceMgr = new SourceManager(AST->getDiagnostics(), *AST->FileMgr,
                                      UserFilesAreVolatile);
+  AST->PCMCache = new MemoryBufferCache;
 
   return AST;
 }
@@ -1997,6 +2001,7 @@ ASTUnit *ASTUnit::LoadFromCommandLine(
   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 @@ ASTUnit *ASTUnit::LoadFromCommandLine(
   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 @@ bool ASTUnit::serialize(raw_ostream &OS) {
 
   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);
 }
 

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 @@ using namespace clang;
 
 CompilerInstance::CompilerInstance(
     std::shared_ptr<PCHContainerOperations> PCHContainerOps,
-    bool BuildingModule)
+    MemoryBufferCache *SharedPCMCache)
-    : ModuleLoader(BuildingModule), Invocation(new CompilerInvocation()),
+    : ModuleLoader(/* BuildingModule = */ SharedPCMCache),
-      ThePCHContainerOperations(std::move(PCHContainerOps)) {}
+      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 @@ IntrusiveRefCntPtr<ASTReader> CompilerInstance::getModuleManager() const {
   return ModuleManager;
 }
 void CompilerInstance::setModuleManager(IntrusiveRefCntPtr<ASTReader> Reader) {
+  assert(PCMCache.get() == &Reader->getModuleManager().getPCMCache() &&
+         "Expected ASTReader to use the same PCM cache");
   ModuleManager = std::move(Reader);
 }
 
@@ -370,7 +379,7 @@ void CompilerInstance::createPreprocessor(TranslationUnitKind TUKind) {
                        getDiagnostics(), getLangOpts(), &getTarget());
   PP = std::make_shared<Preprocessor>(
       Invocation->getPreprocessorOptsPtr(), getDiagnostics(), getLangOpts(),
-      getSourceManager(), *HeaderInfo, *this, PTHMgr,
+      getSourceManager(), getPCMCache(), *HeaderInfo, *this, PTHMgr,
       /*OwnsHeaderSearch=*/true, TUKind);
   PP->Initialize(getTarget(), getAuxTarget());
 
@@ -1073,9 +1082,11 @@ static bool compileModuleImpl(CompilerInstance &ImportingInstance,
          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));
 

clang/lib/Lex/Preprocessor.cpp

@@ -70,15 +70,15 @@ ExternalPreprocessorSource::~ExternalPreprocessorSource() { }
 
 Preprocessor::Preprocessor(std::shared_ptr<PreprocessorOptions> PPOpts,
                            DiagnosticsEngine &diags, LangOptions &opts,
-                           SourceManager &SM, HeaderSearch &Headers,
+                           SourceManager &SM, MemoryBufferCache &PCMCache,
-                           ModuleLoader &TheModuleLoader,
+                           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),
+      PCMCache(PCMCache), ScratchBuf(new ScratchBuffer(SourceMgr)),
-      TheModuleLoader(TheModuleLoader), ExternalSource(nullptr),
+      HeaderInfo(Headers), TheModuleLoader(TheModuleLoader),
-      Identifiers(opts, IILookup),
+      ExternalSource(nullptr), Identifiers(opts, IILookup),
       PragmaHandlers(new PragmaNamespace(StringRef())),
       IncrementalProcessing(false), TUKind(TUKind), CodeComplete(nullptr),
       CodeCompletionFile(nullptr), CodeCompletionOffset(0),

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,6 +464,30 @@ static bool checkDiagnosticMappings(DiagnosticsEngine &StoredDiags,
   return checkDiagnosticGroupMappings(StoredDiags, Diags, Complain);
 }
 
+/// 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
+  // module import of an implicitly-loaded module file.
+  // Note: ModuleMgr.rbegin() may not be the current module, but it must be in
+  // the transitive closure of its imports, since unrelated modules cannot be
+  // imported until after this module finishes validation.
+  ModuleFile *TopImport = &*ModuleMgr.rbegin();
+  while (!TopImport->ImportedBy.empty())
+    TopImport = TopImport->ImportedBy[0];
+  if (TopImport->Kind != MK_ImplicitModule)
+    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<DiagnosticOptions> DiagOpts, bool Complain) {
   DiagnosticsEngine &ExistingDiags = PP.getDiagnostics();
@@ -476,28 +501,14 @@ bool PCHValidator::ReadDiagnosticOptions(
   ModuleManager &ModuleMgr = Reader.getModuleManager();
   assert(ModuleMgr.size() >= 1 && "what ASTFile is this then");
 
-  // If the original import came from a file explicitly generated by the user,
+  Module *TopM = getTopImportImplicitModule(ModuleMgr, PP);
-  // don't check the diagnostic mappings.
+  if (!TopM)
-  // FIXME: currently this is approximated by checking whether this is not a
-  // module import of an implicitly-loaded module file.
-  // Note: ModuleMgr.rbegin() may not be the current module, but it must be in
-  // the transitive closure of its imports, since unrelated modules cannot be
-  // imported until after this module finishes validation.
-  ModuleFile *TopImport = &*ModuleMgr.rbegin();
-  while (!TopImport->ImportedBy.empty())
-    TopImport = TopImport->ImportedBy[0];
-  if (TopImport->Kind != MK_ImplicitModule)
     return false;
 
-  StringRef ModuleName = TopImport->ModuleName;
-  assert(!ModuleName.empty() && "diagnostic options read before module name");
-
-  Module *M = PP.getHeaderSearchInfo().lookupModule(ModuleName);
-  assert(M && "missing module");
-
   // 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 @@ ASTReader::readUnhashedControlBlock(ModuleFile &F, bool WasImportedBy,
       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 @@ ASTReader::ASTReadResult ASTReader::readUnhashedControlBlockImpl(
       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 @@ LLVM_DUMP_METHOD void ASTReader::dump() {
 /// 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 @@ ASTReader::ASTReader(Preprocessor &PP, ASTContext &Context,
                    : cast<ASTReaderListener>(new PCHValidator(PP, *this))),
       SourceMgr(PP.getSourceManager()), FileMgr(PP.getFileManager()),
       PCHContainerRdr(PCHContainerRdr), Diags(PP.getDiagnostics()), PP(PP),
-      Context(Context), ModuleMgr(PP.getFileManager(), PCHContainerRdr),
+      Context(Context),
-      DummyIdResolver(PP), ReadTimer(std::move(ReadTimer)), isysroot(isysroot),
+      ModuleMgr(PP.getFileManager(), PP.getPCMCache(), PCHContainerRdr),
+      PCMCache(PP.getPCMCache()), DummyIdResolver(PP),
+      ReadTimer(std::move(ReadTimer)), isysroot(isysroot),
       DisableValidation(DisableValidation),
       AllowASTWithCompilerErrors(AllowASTWithCompilerErrors),
       AllowConfigurationMismatch(AllowConfigurationMismatch),

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 @@ void ASTWriter::SetSelectorOffset(Selector Sel, uint32_t Offset) {
 }
 
 ASTWriter::ASTWriter(llvm::BitstreamWriter &Stream,
-                     SmallVectorImpl<char> &Buffer,
+                     SmallVectorImpl<char> &Buffer, MemoryBufferCache &PCMCache,
                      ArrayRef<std::shared_ptr<ModuleFileExtension>> 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 @@ ASTFileSignature ASTWriter::WriteAST(Sema &SemaRef,
   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;
 }
 

clang/lib/Serialization/GeneratePCH.cpp

@@ -28,7 +28,8 @@ PCHGenerator::PCHGenerator(
     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;
 }

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 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type,
   // Load the contents of the module
   if (std::unique_ptr<llvm::MemoryBuffer> 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<std::unique_ptr<llvm::MemoryBuffer>> Buf((std::error_code()));
@@ -158,7 +161,7 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type,
       return Missing;
     }
 
-    NewModule->Buffer = std::move(*Buf);
+    NewModule->Buffer = &PCMCache->addBuffer(FileName, std::move(*Buf));
   }
 
   // Initialize the stream.
@@ -167,8 +170,13 @@ ModuleManager::addModule(StringRef FileName, ModuleKind Type,
   // 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 @@ void ModuleManager::removeModules(
     // 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 @@ void ModuleManager::moduleFileAccepted(ModuleFile *MF) {
   ModulesInCommonWithGlobalIndex.push_back(MF);
 }
 
-ModuleManager::ModuleManager(FileManager &FileMgr,
+ModuleManager::ModuleManager(FileManager &FileMgr, MemoryBufferCache &PCMCache,
                              const PCHContainerReader &PCHContainerRdr)
-    : FileMgr(FileMgr), PCHContainerRdr(PCHContainerRdr), GlobalIndex(),
+    : FileMgr(FileMgr), PCMCache(&PCMCache), PCHContainerRdr(PCHContainerRdr),
-      FirstVisitState(nullptr) {}
+      GlobalIndex(), FirstVisitState(nullptr) {}
 
 ModuleManager::~ModuleManager() { delete FirstVisitState; }
 

clang/test/Modules/Inputs/system-out-of-date/X.h

@@ -0,0 +1 @@
+#import <Y.h>

clang/test/Modules/Inputs/system-out-of-date/Y.h

@@ -0,0 +1 @@
+//empty

clang/test/Modules/Inputs/system-out-of-date/Z.h

@@ -0,0 +1 @@
+#import <Y.h>

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 *
+}

clang/test/Modules/Inputs/warning-mismatch/Mismatch.h

@@ -0,0 +1 @@
+struct Mismatch { int i; };

clang/test/Modules/Inputs/warning-mismatch/System.h

@@ -0,0 +1,2 @@
+#import "Mismatch.h"
+struct System { int i; };

clang/test/Modules/Inputs/warning-mismatch/module.modulemap

@@ -0,0 +1,7 @@
+module System [system] {
+  header "System.h"
+}
+
+module Mismatch {
+  header "Mismatch.h"
+}

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
+// 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
+// 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
+
+// This testcase reproduces a use-after-free in when ModuleManager removes an
+// entry from the PCMCache without notifying its parent ASTReader.
+@import Cocoa;

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 <Z.h>
+// 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

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
+// 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
+
+// This testcase triggers a warning flag mismatch in an already validated
+// header.
+@import Mismatch;
+@import System;

clang/unittests/Basic/CMakeLists.txt

@@ -6,6 +6,7 @@ add_clang_unittest(BasicTests
   CharInfoTest.cpp
   DiagnosticTest.cpp
   FileManagerTest.cpp
+  MemoryBufferCacheTest.cpp
   SourceManagerTest.cpp
   VirtualFileSystemTest.cpp
   )

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<MemoryBuffer> getBuffer(int I) {
+  SmallVector<char, 8> 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

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 @@ TEST_F(SourceManagerTest, isBeforeInTranslationUnit) {
   SourceMgr.setMainFileID(mainFileID);
 
   VoidModuleLoader ModLoader;
+  MemoryBufferCache PCMCache;
   HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr,
                           Diags, LangOpts, &*Target);
   Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,
-                  SourceMgr, HeaderInfo, ModLoader,
+                  SourceMgr, PCMCache, HeaderInfo, ModLoader,
                   /*IILookup =*/nullptr,
                   /*OwnsHeaderSearch =*/false);
   PP.Initialize(*Target);
@@ -198,10 +200,11 @@ TEST_F(SourceManagerTest, getMacroArgExpandedLocation) {
   SourceMgr.overrideFileContents(headerFile, std::move(HeaderBuf));
 
   VoidModuleLoader ModLoader;
+  MemoryBufferCache PCMCache;
   HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr,
                           Diags, LangOpts, &*Target);
   Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,
-                  SourceMgr, HeaderInfo, ModLoader,
+                  SourceMgr, PCMCache, HeaderInfo, ModLoader,
                   /*IILookup =*/nullptr,
                   /*OwnsHeaderSearch =*/false);
   PP.Initialize(*Target);
@@ -298,10 +301,11 @@ TEST_F(SourceManagerTest, isBeforeInTranslationUnitWithMacroInInclude) {
   SourceMgr.overrideFileContents(headerFile, std::move(HeaderBuf));
 
   VoidModuleLoader ModLoader;
+  MemoryBufferCache PCMCache;
   HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr,
                           Diags, LangOpts, &*Target);
   Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,
-                  SourceMgr, HeaderInfo, ModLoader,
+                  SourceMgr, PCMCache, HeaderInfo, ModLoader,
                   /*IILookup =*/nullptr,
                   /*OwnsHeaderSearch =*/false);
   PP.Initialize(*Target);

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 @@ protected:
     SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(Buf)));
 
     VoidModuleLoader ModLoader;
+    MemoryBufferCache PCMCache;
     HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr,
                             Diags, LangOpts, Target.get());
     Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,
-                    SourceMgr, HeaderInfo, ModLoader, /*IILookup =*/nullptr,
+                    SourceMgr, PCMCache, HeaderInfo, ModLoader,
+                    /*IILookup =*/nullptr,
                     /*OwnsHeaderSearch =*/false);
     PP.Initialize(*Target);
     PP.EnterMainSourceFile();

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 @@ protected:
     SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(Buf)));
 
     VoidModuleLoader ModLoader;
+    MemoryBufferCache PCMCache;
 
     HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr,
                             Diags, LangOpts, Target.get());
     AddFakeHeader(HeaderInfo, HeaderPath, SystemHeader);
 
     Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,
-                    SourceMgr, HeaderInfo, ModLoader,
+                    SourceMgr, PCMCache, HeaderInfo, ModLoader,
                     /*IILookup =*/nullptr,
                     /*OwnsHeaderSearch =*/false);
     PP.Initialize(*Target);
@@ -198,11 +200,12 @@ protected:
     SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(SourceBuf)));
 
     VoidModuleLoader ModLoader;
+    MemoryBufferCache PCMCache;
     HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr,
                             Diags, OpenCLLangOpts, Target.get());
 
     Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags,
-                    OpenCLLangOpts, SourceMgr, HeaderInfo, ModLoader,
+                    OpenCLLangOpts, SourceMgr, PCMCache, HeaderInfo, ModLoader,
                     /*IILookup =*/nullptr,
                     /*OwnsHeaderSearch =*/false);
     PP.Initialize(*Target);

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 @@ TEST_F(PPConditionalDirectiveRecordTest, PPRecAPI) {
   SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(Buf)));
 
   VoidModuleLoader ModLoader;
+  MemoryBufferCache PCMCache;
   HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr,
                           Diags, LangOpts, Target.get());
   Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,
-                  SourceMgr, HeaderInfo, ModLoader,
+                  SourceMgr, PCMCache, HeaderInfo, ModLoader,
                   /*IILookup =*/nullptr,
                   /*OwnsHeaderSearch =*/false);
   PP.Initialize(*Target);