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

1 files changed, 20 insertions, 7 deletions

diff --git a/clang/lib/Serialization/ModuleManager.cpp b/clang/lib/Serialization/ModuleManager.cpp
index f0654fc..d9c9fb7 100644
--- a/clang/lib/Serialization/ModuleManager.cpp
+++ b/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(),
-      FirstVisitState(nullptr) {}
+    : FileMgr(FileMgr), PCMCache(&PCMCache), PCHContainerRdr(PCHContainerRdr),
+      GlobalIndex(), FirstVisitState(nullptr) {}
 
 ModuleManager::~ModuleManager() { delete FirstVisitState; }