Module: use PCMCache to manage memory buffers for pcm files.

[dexonsmith]

Summary

We create a PCMCache class to manage memory buffers associated with pcm files. With implicit modules, we currently use lock files to make
sure we are making progress when a single clang invocation builds a module file and then loads the module file immediately after. This
implementation requires lock file for correctness and causes all kinds of locking issues.

This patch tries to use PCMCache to share the content between writes and reads within a single clang invocation, so we can remove the
correctness-side of the lock file, and only use lock file for performance i.e to reduce chance of building the same pcm file from multiple
clang invocations.

We also add ThreadContext in PCMCache to diagnose cases where an ancestor validates a module and later on, a child thread tries
to invalidate it. Without the patch, this scenario will cause use-after-free of the FileEntry held by the ancestor.

[aprantl]

include/clang/Basic/DiagnosticSerializationKinds.td

131

Is this better?

"module file '%0' was validated as a system module and is now being imported as a non-system module; any differences in diagnostics options will be ignored"

135

I wonder if there is a way to rephrase this message such that an end-user could understan how to interpret this error?

[benlangmuir]

Can we test the already-validated diagnostics?

include/clang/Basic/FileManager.h

176

Why is this inside the FileManager? It isn't used by the FileManager.

308

Can we call this something like "ModuleLoadContext" or maybe "ModuleCompilationContext"? The word thread is misleading, since the threads are not run concurrently, and are only an implementation detail of our crash recovery.

Also, can you explain why it is only necessary to keep information about the the current stack of contexts? In a situation like

A imports B imports C
A imports D imports C

We would no longer have information about C being validated, right? What happens if there's a mismatch there? Can this never happen?

[manmanren]

I forgot to upload the testing case, I will add it now.

Manman

include/clang/Basic/FileManager.h

176

I initially had another patch that puts PCMCache object at the top level (i.e whenever we create a FileManager, we create a PCMCache object). The initial patch is much bigger than this patch. PCMCache is cacheing the content of a PCM File, and is shared across threads that we spawned to build modules implicitly, just like FileManager. I had some discussions with Bruno and Adrian, we felt FileManager is a better place. But if you have other suggestions, please let me know!

308

Sure, we can call it ModuleCompilationContext because we are spawning threads to build modules implicitly.

The issue we are trying to detect is use-after-free, i.e we don't want a thread to hold on to an invalid FileEntry. We error out before trying to delete the FileEntry that is live in another thread.

In a situation like
A imports B imports C
A imports D imports C

A will start a thread to build B, and B will start a thread to build C, C will be validated in B's compilation thread and A's compilation thread. Later on, if D tries to invalidate C, D knows that an ancestor thread A has validated C, so the compiler will error out. And it is okay for A to invalidate C in A's compilation thread, because we can clean up the references to C's FileEntry.

[manmanren]

Add testing case.

[manmanren]

include/clang/Basic/DiagnosticSerializationKinds.td

131

Thanks Adrian. Your wording sounds better.

135

I am not sure if we will emit this error diagnostic in some case, that is why I don't have a testing case. Maybe it is better to throw a hard error instead of a diagnostic that user can't understand?

[benlangmuir]

include/clang/Basic/FileManager.h

176

I see. It's not ideal, but I see why it would be convenient to use the FileManager this way, since you want to share it in all the same places the FileManager is shared. Since I don't have any better answer, I guess I'm okay with this...

308

Okay

[manmanren]

Addressing review comments.

[manmanren]

Ping :]
Hoping to wrap this up this week.
Cheers,
Manman

[benlangmuir]

Sorry for the delay, I though you were still iterating with @aprantl for some reason. LGTM

[bruno]

lib/Serialization/ASTReader.cpp

3662

This should honor ARR_OutOfDate, so that the module can be rebuilt in case a user module mismatch via a different diagnostics (lib/Serialization/ASTReader.cpp:4076) and returns OutOfDate instead of Success:

if ((ClientLoadCapabilities & ARR_OutOfDate) == 0)
  for (auto N : ValidationConflicts)
    Diag(diag::err_module_ancestor_conflict) << N;

[dexonsmith]

Taking this over to rebase it.

[dexonsmith]

Rebased on top of the current https://reviews.llvm.org/D27689. This also has some substantive changes:

• Renamed the data structure PCMCache to MemoryBufferCache, and split it out into its own files with tests.

• Simplified logic around whether a MemoryBuffer has already been used. Tracking of what has already been validated now takes O(1) memory instead of O(n^2).

• Moved FileManager::BufferMgr (the instance of PCMCache) to CompilerInstance::PCMCache and ModuleManager::PCMCache (instances of IntrusiveRefCntPtr<MemoryBufferCache>). This took some work to thread through, but since the PCMCache isn't related to the FileManager, this seemed like a cleaner result.

• Added some testcases for bugs we've found in the meantime (thanks to Adrian Prantl). Primarily, we no longer error or cause a use-after-free if a MemoryBuffer was validated by an ancestor.

[dexonsmith]

This has changed enough it needs another review. Richard or Ben, could you take a(nother) look?

[dexonsmith]

Rebased again, and cleaned up tests, and a ping... now that dependencies are resolved.

Richard and/or Ben, can you take a look?

[dexonsmith]

! In D28299#701194, @dexonsmith wrote:
Rebased again, and cleaned up tests, and a ping... now that dependencies are resolved.

Richard and/or Ben, can you take a look?

(Or anyone else that feels comfortable confirming that the ownership semantics of MemoryBufferCache are correct.)

[aprantl]

Drive-by comment.

clang/lib/Basic/MemoryBufferCache.cpp

18 ↗

(On Diff #91799)

we should be able to replace the std::make_pair() with {}.

[dexonsmith]

Thanks for the reviews! Committed in r298165.

include/clang/Basic/DiagnosticSerializationKinds.td

 def warn_module_uses_date_time : Warning<
   "%select{precompiled header|module}0 uses __DATE__ or __TIME__">,
   InGroup<DiagGroup<"pch-date-time">>;
 
 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, ignoring differences in "
+  "diagnostic options now it is a non-system module">,
+  InGroup<ModuleConflict>;
+
+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
 

include/clang/Basic/FileManager.h

 
 class MemoryBuffer;
 
 } // end namespace llvm
 
 namespace clang {
 
 class FileSystemStatCache;
+class PCMCache;
 
 /// \brief Cached information about one directory (either on disk or in
 /// the virtual file system).
 class DirectoryEntry {
   friend class FileManager;
 
   StringRef Name; // Name of the directory.
 
class FileManager : public RefCountedBase<FileManager> {
 
   bool getStatValue(StringRef Path, FileData &Data, bool isFile,
                     std::unique_ptr<vfs::File> *F);
 
   /// Add all ancestors of the given path (pointing to either a file
   /// or a directory) as virtual directories.
   void addAncestorsAsVirtualDirs(StringRef Path);
 
+  /// Manage memory buffers associated with pcm files.
+  std::unique_ptr<PCMCache> BufferMgr;
+
 public:
   FileManager(const FileSystemOptions &FileSystemOpts,
               IntrusiveRefCntPtr<vfs::FileSystem> FS = nullptr);
   ~FileManager();
 
   /// \brief Installs the provided FileSystemStatCache object within
   /// the FileManager.
   ///
public:
   /// \brief Retrieve the canonical name for a given directory.
   ///
   /// This is a very expensive operation, despite its results being cached,
   /// and should only be used when the physical layout of the file system is
   /// required, which is (almost) never.
   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<std::string, std::pair<std::unique_ptr<llvm::MemoryBuffer>, bool>>
+      ConsistentBuffers;
+
+  typedef std::map<std::string, bool> 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<ThreadContext, 8> NestedThreadContexts;
+
+public:
+  /// Add a memory buffer for a pcm file to the manager.
+  void addConsistentBuffer(std::string FileName,
+                           std::unique_ptr<llvm::MemoryBuffer> 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
 
 #endif // LLVM_CLANG_BASIC_FILEMANAGER_H

include/clang/Serialization/ASTReader.h

private:
   ASTContext &Context;
 
   /// \brief The AST consumer.
   ASTConsumer *Consumer;
 
   /// \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.
   IdentifierResolver DummyIdResolver;
 
   /// A mapping from extension block names to module file extensions.
   llvm::StringMap<IntrusiveRefCntPtr<ModuleFileExtension>> ModuleFileExtensions;
 

include/clang/Serialization/ASTWriter.h

typedef llvm::DenseMap<QualType, serialization::TypeIdx,
     TypeIdxMap;
 
   /// \brief The bitstream writer used to emit this precompiled header.
   llvm::BitstreamWriter &Stream;
 
   /// \brief The buffer associated with the bitstream.
   const SmallVectorImpl<char> &Buffer;
 
+  /// \brief The PCM manager which manages memory buffers for pcm files.
+  PCMCache *BufferMgr;
+
   /// \brief The ASTContext we're writing.
   ASTContext *Context;
 
   /// \brief The preprocessor we're writing.
   Preprocessor *PP;
 
   /// \brief The reader of existing AST files, if we're chaining.
   ASTReader *Chain;
private:
   ASTFileSignature WriteASTCore(Sema &SemaRef, StringRef isysroot,
                                 const std::string &OutputFile,
                                 Module *WritingModule);
 
 public:
   /// \brief Create a new precompiled header writer that outputs to
   /// the given bitstream.
   ASTWriter(llvm::BitstreamWriter &Stream, SmallVectorImpl<char> &Buffer,
+            PCMCache *BufferMgr,
             ArrayRef<llvm::IntrusiveRefCntPtr<ModuleFileExtension>> Extensions,
             bool IncludeTimestamps = true);
   ~ASTWriter() override;
 
   const LangOptions &getLangOpts() const;
 
   /// \brief Get a timestamp for output into the AST file. The actual timestamp
   /// of the specified file may be ignored if we have been instructed to not
protected:
   ASTWriter &getWriter() { return Writer; }
   const ASTWriter &getWriter() const { return Writer; }
   SmallVectorImpl<char> &getPCH() const { return Buffer->Data; }
 
 public:
   PCHGenerator(
     const Preprocessor &PP, StringRef OutputFile,
     StringRef isysroot,
-    std::shared_ptr<PCHBuffer> Buffer,
+    std::shared_ptr<PCHBuffer> Buffer, PCMCache *BufferMgr,
     ArrayRef<llvm::IntrusiveRefCntPtr<ModuleFileExtension>> Extensions,
     bool AllowASTWithErrors = false,
     bool IncludeTimestamps = true);
   ~PCHGenerator() override;
   void InitializeSema(Sema &S) override { SemaPtr = &S; }
   void HandleTranslationUnit(ASTContext &Ctx) override;
   ASTMutationListener *GetASTMutationListener() override;
   ASTDeserializationListener *GetASTDeserializationListener() override;
   bool hasEmittedPCH() const { return Buffer->IsComplete; }
 };
 
 } // end namespace clang
 
 #endif

include/clang/Serialization/Module.h

public:
   /// user.
   bool DirectlyImported;
 
   /// \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<llvm::MemoryBuffer> Buffer;
+  /// The memory buffer is owned by PCMCache.
+  llvm::MemoryBuffer *Buffer;
 
   /// \brief The size of this file, in bits.
   uint64_t SizeInBits;
 
   /// \brief The global bit offset (or base) of this module
   uint64_t GlobalBitOffset;
 
   /// \brief The serialized bitstream data for this file.

include/clang/Serialization/ModuleManager.h

public:
   /// \brief Returns the module associated with the given name
   ModuleFile *lookup(StringRef Name);
 
   /// \brief Returns the module associated with the given module file.
   ModuleFile *lookup(const FileEntry *File);
 
   /// \brief Returns the in-memory (virtual file) buffer with the given name
   std::unique_ptr<llvm::MemoryBuffer> lookupBuffer(StringRef Name);
-  
   /// \brief Number of modules loaded
   unsigned size() const { return Chain.size(); }
 
   /// \brief The result of attempting to add a new module.
   enum AddModuleResult {
     /// \brief The module file had already been loaded.
     AlreadyLoaded,
     /// \brief The module file was just loaded in response to this call.
AddModuleResult addModule(StringRef FileName, ModuleKind Type,
                             ASTFileSignature ExpectedSignature,
                             ASTFileSignatureReader ReadSignature,
                             ModuleFile *&Module,
                             std::string &ErrorStr);
 
   /// \brief Remove the given set of modules.
   void removeModules(ModuleIterator first, ModuleIterator last,
                      llvm::SmallPtrSetImpl<ModuleFile *> &LoadedSuccessfully,
-                     ModuleMap *modMap);
+                     ModuleMap *modMap,
+                     llvm::SmallVectorImpl<std::string> &ValidationConflicts);
 
   /// \brief Add an in-memory buffer the list of known buffers
   void addInMemoryBuffer(StringRef FileName,
                          std::unique_ptr<llvm::MemoryBuffer> Buffer);
 
   /// \brief Set the global module index.
   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.
   ///
   /// This routine visits each of the modules, starting with the
   /// "root" modules that no other loaded modules depend on, and
   /// proceeding to the leaf modules, visiting each module only once
   /// during the traversal.
   ///

lib/Basic/FileManager.cpp

: FS(FS), FileSystemOpts(FSO),
     SeenDirEntries(64), SeenFileEntries(64), NextFileUID(0) {
   NumDirLookups = NumFileLookups = 0;
   NumDirCacheMisses = NumFileCacheMisses = 0;
 
   // If the caller doesn't provide a virtual file system, just grab the real
   // file system.
   if (!FS)
     this->FS = vfs::getRealFileSystem();
+  BufferMgr = llvm::make_unique<PCMCache>();
 }
 
 FileManager::~FileManager() = default;
 
 void FileManager::addStatCache(std::unique_ptr<FileSystemStatCache> statCache,
                                bool AtBeginning) {
   assert(statCache && "No stat cache provided?");
   if (AtBeginning || !StatCache.get()) {

lib/Frontend/ASTUnit.cpp

void OnDiskData::Cleanup() {
   CleanPreambleFile();
 }
 
 struct ASTUnit::ASTWriterData {
   SmallString<128> Buffer;
   llvm::BitstreamWriter Stream;
   ASTWriter Writer;
 
-  ASTWriterData() : Stream(Buffer), Writer(Stream, Buffer, { }) { }
+  ASTWriterData() : Stream(Buffer), Writer(Stream, Buffer, nullptr, { }) { }
 };
 
 void ASTUnit::clearFileLevelDecls() {
   llvm::DeleteContainerSeconds(FileDecls);
 }
 
 void ASTUnit::CleanTemporaryFiles() {
   getOnDiskData(this).CleanTemporaryFiles();
class PrecompilePreambleConsumer : public PCHGenerator {
   ASTUnit &Unit;
   unsigned &Hash;
   std::vector<Decl *> TopLevelDecls;
   PrecompilePreambleAction *Action;
   std::unique_ptr<raw_ostream> Out;
 
 public:
   PrecompilePreambleConsumer(ASTUnit &Unit, PrecompilePreambleAction *Action,
-                             const Preprocessor &PP, StringRef isysroot,
+                             const Preprocessor &PP, PCMCache *BufferMgr,
+                             StringRef isysroot,
                              std::unique_ptr<raw_ostream> Out)
-      : PCHGenerator(PP, "", isysroot, std::make_shared<PCHBuffer>(),
+      : PCHGenerator(PP, "", isysroot, std::make_shared<PCHBuffer>(), BufferMgr,
                      ArrayRef<llvm::IntrusiveRefCntPtr<ModuleFileExtension>>(),
                      /*AllowASTWithErrors=*/true),
         Unit(Unit), Hash(Unit.getCurrentTopLevelHashValue()), Action(Action),
         Out(std::move(Out)) {
     Hash = 0;
   }
 
   bool HandleTopLevelDecl(DeclGroupRef DG) override {
PrecompilePreambleAction::CreateASTConsumer(CompilerInstance &CI,
 
   if (!CI.getFrontendOpts().RelocatablePCH)
     Sysroot.clear();
 
   CI.getPreprocessor().addPPCallbacks(
       llvm::make_unique<MacroDefinitionTrackerPPCallbacks>(
                                            Unit.getCurrentTopLevelHashValue()));
   return llvm::make_unique<PrecompilePreambleConsumer>(
-      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) {
   return StoredDiag.getLocation().isValid();
 }
 
 static void
 checkAndRemoveNonDriverDiags(SmallVectorImpl<StoredDiagnostic> &StoredDiags) {
bool ASTUnit::serialize(raw_ostream &OS) {
   bool hasErrors = getDiagnostics().hasUncompilableErrorOccurred();
 
   if (WriterData)
     return serializeUnit(WriterData->Writer, WriterData->Buffer,
                          getSema(), hasErrors, OS);
 
   SmallString<128> Buffer;
   llvm::BitstreamWriter Stream(Buffer);
-  ASTWriter Writer(Stream, Buffer, { });
+  ASTWriter Writer(Stream, Buffer, nullptr, { });
   return serializeUnit(Writer, Buffer, getSema(), hasErrors, OS);
 }
 
 typedef ContinuousRangeMap<unsigned, int, 2> SLocRemap;
 
 void ASTUnit::TranslateStoredDiagnostics(
                           FileManager &FileMgr,
                           SourceManager &SrcMgr,

lib/Frontend/ChainedIncludesSource.cpp

for (unsigned i = 0, e = includes.size(); i != e; ++i) {
     Clang->getDiagnosticClient().BeginSourceFile(Clang->getLangOpts(),
                                                  &Clang->getPreprocessor());
     Clang->createASTContext();
 
     auto Buffer = std::make_shared<PCHBuffer>();
     ArrayRef<llvm::IntrusiveRefCntPtr<ModuleFileExtension>> Extensions;
     auto consumer = llvm::make_unique<PCHGenerator>(
         Clang->getPreprocessor(), "-", /*isysroot=*/"", Buffer,
-        Extensions, /*AllowASTWithErrors=*/true);
+        Clang->getFileManager().getPCMCache(), Extensions,
+        /*AllowASTWithErrors=*/true);
     Clang->getASTContext().setASTMutationListener(
                                             consumer->GetASTMutationListener());
     Clang->setASTConsumer(std::move(consumer));
     Clang->createSema(TU_Prefix, nullptr);
 
     if (firstInclude) {
       Preprocessor &PP = Clang->getPreprocessor();
       PP.getBuiltinInfo().initializeBuiltins(PP.getIdentifierTable(),

lib/Frontend/CompilerInstance.cpp

static bool compileModuleImpl(CompilerInstance &ImportingInstance,
   Instance.createDiagnostics(new ForwardingDiagnosticConsumer(
                                    ImportingInstance.getDiagnosticClient()),
                              /*ShouldOwnClient=*/true);
 
   Instance.setVirtualFileSystem(&ImportingInstance.getVirtualFileSystem());
 
   // 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(
     ImportingInstance.getSourceManager().getModuleBuildStack());
   SourceMgr.pushModuleBuildStack(Module->getTopLevelModuleName(),
     FullSourceLoc(ImportLoc, ImportingInstance.getSourceManager()));
 
   // If we're collecting module dependencies, we need to share a collector
static bool compileModuleImpl(CompilerInstance &ImportingInstance,
   Instance.clearOutputFiles(/*EraseFiles=*/true);
 
   // We've rebuilt a module. If we're allowed to generate or update the global
   // module index, record that fact in the importing compiler instance.
   if (ImportingInstance.getFrontendOpts().GenerateGlobalModuleIndex) {
     ImportingInstance.setBuildGlobalModuleIndex(true);
   }
 
+  ImportingInstance.getFileManager().getPCMCache()->EndThread();
   return !Instance.getDiagnostics().hasErrorOccurred();
 }
 
 static bool compileAndLoadModule(CompilerInstance &ImportingInstance,
                                  SourceLocation ImportLoc,
                                  SourceLocation ModuleNameLoc, Module *Module,
                                  StringRef ModuleFileName) {
   DiagnosticsEngine &Diags = ImportingInstance.getDiagnostics();

lib/Frontend/FrontendActions.cpp

GeneratePCHAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) {
 
   if (!CI.getFrontendOpts().RelocatablePCH)
     Sysroot.clear();
 
   auto Buffer = std::make_shared<PCHBuffer>();
   std::vector<std::unique_ptr<ASTConsumer>> Consumers;
   Consumers.push_back(llvm::make_unique<PCHGenerator>(
                         CI.getPreprocessor(), OutputFile, Sysroot,
-                        Buffer, CI.getFrontendOpts().ModuleFileExtensions,
+                        Buffer, CI.getFileManager().getPCMCache(),
+                        CI.getFrontendOpts().ModuleFileExtensions,
                         /*AllowASTWithErrors*/false,
                         /*IncludeTimestamps*/
                           +CI.getFrontendOpts().IncludeTimestamps));
   Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(
       CI, InFile, OutputFile, std::move(OS), Buffer));
 
   return llvm::make_unique<MultiplexConsumer>(std::move(Consumers));
 }
GenerateModuleAction::CreateASTConsumer(CompilerInstance &CI,
   std::string OutputFile = CI.getFrontendOpts().OutputFile;
   std::string Sysroot;
 
   auto Buffer = std::make_shared<PCHBuffer>();
   std::vector<std::unique_ptr<ASTConsumer>> Consumers;
 
   Consumers.push_back(llvm::make_unique<PCHGenerator>(
                         CI.getPreprocessor(), OutputFile, Sysroot,
-                        Buffer, CI.getFrontendOpts().ModuleFileExtensions,
+                        Buffer, CI.getFileManager().getPCMCache(),
+                        CI.getFrontendOpts().ModuleFileExtensions,
                         /*AllowASTWithErrors=*/false,
                         /*IncludeTimestamps=*/
                           +CI.getFrontendOpts().BuildingImplicitModule));
   Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(
       CI, InFile, OutputFile, std::move(OS), Buffer));
   return llvm::make_unique<MultiplexConsumer>(std::move(Consumers));
 }
 

lib/Serialization/ASTReader.cpp

if (isExtHandlingFromDiagsError(Diags) &&
     if (Complain)
       Diags.Report(diag::err_pch_diagopt_mismatch) << "-pedantic-errors";
     return true;
   }
 
   return checkDiagnosticGroupMappings(StoredDiags, Diags, Complain);
 }
 
-bool PCHValidator::ReadDiagnosticOptions(
-    IntrusiveRefCntPtr<DiagnosticOptions> DiagOpts, bool Complain) {
-  DiagnosticsEngine &ExistingDiags = PP.getDiagnostics();
-  IntrusiveRefCntPtr<DiagnosticIDs> DiagIDs(ExistingDiags.getDiagnosticIDs());
-  IntrusiveRefCntPtr<DiagnosticsEngine> 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
   // 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;
+    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();
+  IntrusiveRefCntPtr<DiagnosticIDs> DiagIDs(ExistingDiags.getDiagnosticIDs());
+  IntrusiveRefCntPtr<DiagnosticsEngine> 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
 /// options.
 static void
 collectMacroDefinitions(const PreprocessorOptions &PPOpts,
                         MacroDefinitionsMap &Macros,
                         SmallVectorImpl<StringRef> *MacroNames = nullptr) {
while (1) {
     // Read and process a record.
     Record.clear();
     switch ((UnhashedControlBlockRecordTypes)Stream.readRecord(Entry.ID, Record)) {
     case DIAGNOSTIC_OPTIONS: {
       bool Complain = (ClientLoadCapabilities & ARR_OutOfDate) == 0;
       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: {
       if (!F)
         break;
       int Pos = 0;
       F->Signature[Pos++] = Record[0];
       F->Signature[Pos++] = Record[1];
ASTReader::ASTReadResult ASTReader::ReadAST(StringRef FileName,
   case OutOfDate:
   case VersionMismatch:
   case ConfigurationMismatch:
   case HadErrors: {
     llvm::SmallPtrSet<ModuleFile *, 4> LoadedSet;
     for (const ImportedModule &IM : Loaded)
       LoadedSet.insert(IM.Mod);
 
+    llvm::SmallVector<std::string, 4> 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.
     GlobalIndex.reset();
     ModuleMgr.setGlobalIndex(nullptr);
     return ReadResult;
   }
   case Success:
ASTReader::ASTReadResult ASTReader::ReadAST(StringRef FileName,
 
   // Setup the import locations and notify the module manager that we've
   // committed to these module files.
   for (SmallVectorImpl<ImportedModule>::iterator M = Loaded.begin(),
                                               MEnd = Loaded.end();
        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;
     // FIXME: We assume that locations from PCH / preamble do not need
     // any translation.
     if (!M->ImportedBy)
       F.ImportLoc = M->ImportLoc;
     else
case UNHASHED_CONTROL_BLOCK_ID: {
         if (Result == Failure) {
           Error("malformed block record in AST file");
           return Failure;
         }
 
         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)
         // Record that we've loaded this module.
         Loaded.push_back(ImportedModule(M, ImportedBy, ImportLoc));
 
       // Move Stream backward to point to ASTCursor.
       Stream = ASTCursor;
for (ModuleManager::ModuleConstIterator M = ModuleMgr.begin(),
     (*M)->dump();
 }
 
 /// Return the amount of memory used by memory buffers, breaking down
 /// by heap-backed versus mmap'ed memory.
 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:
           sizes.malloc_bytes += bytes;
           break;
         case llvm::MemoryBuffer::MemoryBuffer_MMap:
           sizes.mmap_bytes += bytes;
           break;
std::unique_ptr<llvm::Timer> ReadTimer)
     : Listener(DisableValidation ?
         cast<ASTReaderListener>(new SimpleASTReaderListener(PP)) :
         cast<ASTReaderListener>(new PCHValidator(PP, *this))),
       DeserializationListener(nullptr),
       OwnsDeserializationListener(false), SourceMgr(PP.getSourceManager()),
       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),
       PragmaMSPointersToMembersState(-1),
       isysroot(isysroot), DisableValidation(DisableValidation),
       AllowASTWithCompilerErrors(AllowASTWithCompilerErrors),
       AllowConfigurationMismatch(AllowConfigurationMismatch),
       ValidateSystemInputs(ValidateSystemInputs),

lib/Serialization/ASTWriter.cpp

void ASTWriter::SetSelectorOffset(Selector Sel, uint32_t Offset) {
   // file.
   if (ID < FirstSelectorID)
     return;
   SelectorOffsets[ID - FirstSelectorID] = Offset;
 }
 
 ASTWriter::ASTWriter(
   llvm::BitstreamWriter &Stream, SmallVectorImpl<char> &Buffer,
+  PCMCache *BufferMgr,
   ArrayRef<llvm::IntrusiveRefCntPtr<ModuleFileExtension>> 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),
       NextDeclID(FirstDeclID), FirstTypeID(NUM_PREDEF_TYPE_IDS),
       NextTypeID(FirstTypeID), FirstIdentID(NUM_PREDEF_IDENT_IDS),
       NextIdentID(FirstIdentID), FirstMacroID(NUM_PREDEF_MACRO_IDS),
       NextMacroID(FirstMacroID), FirstSubmoduleID(NUM_PREDEF_SUBMODULE_IDS),
       NextSubmoduleID(FirstSubmoduleID),
ASTFileSignature ASTWriter::WriteAST(Sema &SemaRef, const std::string &OutputFile,
   ASTFileSignature Signature =
       WriteASTCore(SemaRef, isysroot, OutputFile, WritingModule);
   Context = nullptr;
   PP = nullptr;
   this->WritingModule = nullptr;
   this->BaseDirectory.clear();
 
   WritingAST = false;
+  if (BufferMgr && SemaRef.Context.getLangOpts().ImplicitModules &&
+      WritingModule) {
+    // Construct MemoryBuffer and update buffer manager.
+    std::unique_ptr<llvm::MemoryBuffer> 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;
 }
 
 template<typename Vector>
 static void AddLazyVectorDecls(ASTWriter &Writer, Vector &Vec,
                                ASTWriter::RecordData &Record) {
   for (typename Vector::iterator I = Vec.begin(nullptr, true), E = Vec.end();
        I != E; ++I) {

lib/Serialization/GeneratePCH.cpp

 #include "clang/Sema/SemaConsumer.h"
 #include "clang/Serialization/ASTWriter.h"
 #include "llvm/Bitcode/BitstreamWriter.h"
 
 using namespace clang;
 
 PCHGenerator::PCHGenerator(
     const Preprocessor &PP, StringRef OutputFile, StringRef isysroot,
-    std::shared_ptr<PCHBuffer> Buffer,
+    std::shared_ptr<PCHBuffer> Buffer, PCMCache *BufferMgr,
     ArrayRef<llvm::IntrusiveRefCntPtr<ModuleFileExtension>> 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;
 }
 
 PCHGenerator::~PCHGenerator() {
 }
 
 void PCHGenerator::HandleTranslationUnit(ASTContext &Ctx) {

lib/Serialization/ModuleManager.cpp

if (ModuleEntry->Kind == MK_ImplicitModule) {
       if (!FileMgr.getNoncachedStatValue(TimestampFilename, Status))
         ModuleEntry->InputFilesValidationTimestamp =
             llvm::sys::toTimeT(Status.getLastModificationTime());
     }
 
     // Load the contents of the module
     if (std::unique_ptr<llvm::MemoryBuffer> 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<std::unique_ptr<llvm::MemoryBuffer>> Buf(
           (std::error_code()));
       if (FileName == "-") {
         Buf = llvm::MemoryBuffer::getSTDIN();
       } else {
         // Leave the FileEntry open so if it gets read again by another
         // ModuleManager it must be the same underlying file.
         // FIXME: Because FileManager::getFile() doesn't guarantee that it will
         // give us an open file, this may not be 100% reliable.
         Buf = FileMgr.getBufferForFile(ModuleEntry->File,
                                        /*IsVolatile=*/false,
                                        /*ShouldClose=*/false);
       }
 
       if (!Buf) {
         ErrorStr = Buf.getError().message();
         delete ModuleEntry;
         return Missing;
       }
 
-      ModuleEntry->Buffer = std::move(*Buf);
+      ModuleEntry->Buffer = (*Buf).get();
+      FileMgr.getPCMCache()->addConsistentBuffer(FileName, std::move(*Buf));
     }
 
     // Initialize the stream.
     ModuleEntry->Data = PCHContainerRdr.ExtractPCH(*ModuleEntry->Buffer);
   }
 
   if (ExpectedSignature != ASTFileSignature({{0}})) {
     // If we've not read the control block yet, read the signature eagerly now
     // so that we can check it.
     if (ModuleEntry->Signature == ASTFileSignature({{0}}))
       ModuleEntry->Signature = ReadSignature(ModuleEntry->Data);
 
     if (ModuleEntry->Signature != ExpectedSignature) {
       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;
     }
   }
 
   if (ImportedBy) {
     ModuleEntry->ImportedBy.insert(ImportedBy);
     ImportedBy->Imports.insert(ModuleEntry);
   } else {
if (!ImportedBy)
     Roots.push_back(ModuleEntry);
 
   return NewlyLoaded;
 }
 
 void ModuleManager::removeModules(
     ModuleIterator first, ModuleIterator last,
     llvm::SmallPtrSetImpl<ModuleFile *> &LoadedSuccessfully,
-    ModuleMap *modMap) {
+    ModuleMap *modMap,
+    llvm::SmallVectorImpl<std::string> &ValidationConflicts) {
   if (first == last)
     return;
 
   // Explicitly clear VisitOrder since we might not notice it is stale.
   VisitOrder.clear();
 
   // Collect the set of module file pointers that we'll be removing.
   llvm::SmallPtrSet<ModuleFile *, 4> victimSet(first, last);
if (modMap) {
       if (Module *mod = modMap->findModule(ModuleName)) {
         mod->setASTFile(nullptr);
       }
     }
 
     // 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)
+    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;
   }
 
   // Remove the modules from the chain.
   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<llvm::MemoryBuffer> 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<llvm::MemoryBuffer> Buffer) {
 
   const FileEntry *Entry =
       FileMgr.getVirtualFile(FileName, Buffer->getBufferSize(), 0);
   InMemoryBuffers[Entry] = std::move(Buffer);
 }
void ModuleManager::setGlobalIndex(GlobalModuleIndex *Index) {
   // loaded.
   for (unsigned I = 0, N = Chain.size(); I != N; ++I) {
     if (!GlobalIndex->loadedModuleFile(Chain[I])) {
       ModulesInCommonWithGlobalIndex.push_back(Chain[I]);
     }
   }
 }
 
-void ModuleManager::moduleFileAccepted(ModuleFile *MF) {
+void ModuleManager::moduleFileAccepted(ModuleFile *MF, bool IsSystem) {
+  FileMgr.getPCMCache()->setIsSystem(MF->FileName, IsSystem);
   if (!GlobalIndex || GlobalIndex->loadedModuleFile(MF))
     return;
 
   ModulesInCommonWithGlobalIndex.push_back(MF);
 }
 
 ModuleManager::ModuleManager(FileManager &FileMgr,
                              const PCHContainerReader &PCHContainerRdr)