Reapply "Modules: Cache PCMs in memory and avoid a use-after-free"
This reverts commit r298185, effectively reapplying r298165, after fixing the
new unit tests (PR32338). The memory buffer generator doesn't null-terminate
the MemoryBuffer it creates; this version of the commit informs getMemBuffer
about that to avoid the assert.
Original commit message follows:
----
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: 298278
diff --git a/clang/lib/Frontend/ASTUnit.cpp b/clang/lib/Frontend/ASTUnit.cpp
index ac5c7ca..952992a 100644
--- a/clang/lib/Frontend/ASTUnit.cpp
+++ b/clang/lib/Frontend/ASTUnit.cpp
@@ -18,6 +18,7 @@
#include "clang/AST/StmtVisitor.h"
#include "clang/AST/TypeOrdering.h"
#include "clang/Basic/Diagnostic.h"
+#include "clang/Basic/MemoryBufferCache.h"
#include "clang/Basic/TargetInfo.h"
#include "clang/Basic/TargetOptions.h"
#include "clang/Basic/VirtualFileSystem.h"
@@ -185,7 +186,8 @@
llvm::BitstreamWriter Stream;
ASTWriter Writer;
- ASTWriterData() : Stream(Buffer), Writer(Stream, Buffer, {}) {}
+ ASTWriterData(MemoryBufferCache &PCMCache)
+ : Stream(Buffer), Writer(Stream, Buffer, PCMCache, {}) {}
};
void ASTUnit::clearFileLevelDecls() {
@@ -681,6 +683,7 @@
AST->SourceMgr = new SourceManager(AST->getDiagnostics(),
AST->getFileManager(),
UserFilesAreVolatile);
+ AST->PCMCache = new MemoryBufferCache;
AST->HSOpts = std::make_shared<HeaderSearchOptions>();
AST->HSOpts->ModuleFormat = PCHContainerRdr.getFormat();
AST->HeaderInfo.reset(new HeaderSearch(AST->HSOpts,
@@ -701,7 +704,7 @@
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 @@
AST->UserFilesAreVolatile = UserFilesAreVolatile;
AST->SourceMgr = new SourceManager(AST->getDiagnostics(), *AST->FileMgr,
UserFilesAreVolatile);
+ AST->PCMCache = new MemoryBufferCache;
return AST;
}
@@ -1997,6 +2001,7 @@
if (!VFS)
return nullptr;
AST->FileMgr = new FileManager(AST->FileSystemOpts, VFS);
+ AST->PCMCache = new MemoryBufferCache;
AST->OnlyLocalDecls = OnlyLocalDecls;
AST->CaptureDiagnostics = CaptureDiagnostics;
AST->TUKind = TUKind;
@@ -2008,7 +2013,7 @@
AST->StoredDiagnostics.swap(StoredDiagnostics);
AST->Invocation = CI;
if (ForSerialization)
- AST->WriterData.reset(new ASTWriterData());
+ AST->WriterData.reset(new ASTWriterData(*AST->PCMCache));
// Zero out now to ease cleanup during crash recovery.
CI = nullptr;
Diags = nullptr;
@@ -2523,7 +2528,8 @@
SmallString<128> Buffer;
llvm::BitstreamWriter Stream(Buffer);
- ASTWriter Writer(Stream, Buffer, {});
+ MemoryBufferCache PCMCache;
+ ASTWriter Writer(Stream, Buffer, PCMCache, {});
return serializeUnit(Writer, Buffer, getSema(), hasErrors, OS);
}
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index bf15460..f660429 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -13,6 +13,7 @@
#include "clang/AST/Decl.h"
#include "clang/Basic/Diagnostic.h"
#include "clang/Basic/FileManager.h"
+#include "clang/Basic/MemoryBufferCache.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Basic/TargetInfo.h"
#include "clang/Basic/Version.h"
@@ -55,9 +56,15 @@
CompilerInstance::CompilerInstance(
std::shared_ptr<PCHContainerOperations> PCHContainerOps,
- bool BuildingModule)
- : ModuleLoader(BuildingModule), Invocation(new CompilerInvocation()),
- ThePCHContainerOperations(std::move(PCHContainerOps)) {}
+ MemoryBufferCache *SharedPCMCache)
+ : ModuleLoader(/* BuildingModule = */ SharedPCMCache),
+ Invocation(new CompilerInvocation()),
+ PCMCache(SharedPCMCache ? SharedPCMCache : new MemoryBufferCache),
+ ThePCHContainerOperations(std::move(PCHContainerOps)) {
+ // Don't allow this to invalidate buffers in use by others.
+ if (SharedPCMCache)
+ getPCMCache().finalizeCurrentBuffers();
+}
CompilerInstance::~CompilerInstance() {
assert(OutputFiles.empty() && "Still output files in flight?");
@@ -128,6 +135,8 @@
return ModuleManager;
}
void CompilerInstance::setModuleManager(IntrusiveRefCntPtr<ASTReader> Reader) {
+ assert(PCMCache.get() == &Reader->getModuleManager().getPCMCache() &&
+ "Expected ASTReader to use the same PCM cache");
ModuleManager = std::move(Reader);
}
@@ -370,7 +379,7 @@
getDiagnostics(), getLangOpts(), &getTarget());
PP = std::make_shared<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 @@
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));