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!

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@298165 91177308-0d34-0410-b5e6-96231b3b80d8

Author: dexonsmith

Diff for: 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
 

Diff for: 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

Diff for: 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 @@ class ASTUnit : public ModuleLoader {
   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;

Diff for: 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 @@ class CompilerInstance : public ModuleLoader {
   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 @@ class CompilerInstance : public ModuleLoader {
   }
 
   void setExternalSemaSource(IntrusiveRefCntPtr<ExternalSemaSource> ESS);
+
+  MemoryBufferCache &getPCMCache() const { return *PCMCache; }
 };
 
 } // end namespace clang

Diff for: 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 @@ class Preprocessor {
   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; }

Diff for: include/clang/Serialization/ASTReader.h

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

Diff for: 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 @@ class ASTWriter : public ASTDeserializationListener,
   /// 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 @@ class ASTWriter : public ASTDeserializationListener,
   /// \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;

Diff for: include/clang/Serialization/Module.h

@@ -163,9 +163,9 @@ class ModuleFile {
   /// \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 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;

Diff for: 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 @@ class ModuleManager {
       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 @@ class ModuleManager {
 
   /// \brief View the graphviz representation of the module graph.
   void viewGraph();
+
+  MemoryBufferCache &getPCMCache() const { return *PCMCache; }
 };
 
 } } // end namespace clang::serialization

Diff for: 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

Diff for: 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; }

Diff for: 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);
 }