diff options
author | Duncan P. N. Exon Smith <dexonsmith@apple.com> | 2017-03-20 17:58:26 +0000 |
---|---|---|
committer | Duncan P. N. Exon Smith <dexonsmith@apple.com> | 2017-03-20 17:58:26 +0000 |
commit | 2bf5f686e2ea17e971fb6b5a1739282d70dc3536 (patch) | |
tree | c6a32b58da17cf19f7d061910f61131decb4b6b6 /unittests/Basic | |
parent | 22ceb362cdbd367ec680a2960dbc0904d7531868 (diff) |
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!
git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@298278 91177308-0d34-0410-b5e6-96231b3b80d8
Diffstat (limited to 'unittests/Basic')
-rw-r--r-- | unittests/Basic/CMakeLists.txt | 1 | ||||
-rw-r--r-- | unittests/Basic/MemoryBufferCacheTest.cpp | 95 | ||||
-rw-r--r-- | unittests/Basic/SourceManagerTest.cpp | 10 |
3 files changed, 103 insertions, 3 deletions
diff --git a/unittests/Basic/CMakeLists.txt b/unittests/Basic/CMakeLists.txt index 3cb3cb8d3c..3a9f34f3d2 100644 --- a/unittests/Basic/CMakeLists.txt +++ b/unittests/Basic/CMakeLists.txt @@ -6,6 +6,7 @@ add_clang_unittest(BasicTests CharInfoTest.cpp DiagnosticTest.cpp FileManagerTest.cpp + MemoryBufferCacheTest.cpp SourceManagerTest.cpp VirtualFileSystemTest.cpp ) diff --git a/unittests/Basic/MemoryBufferCacheTest.cpp b/unittests/Basic/MemoryBufferCacheTest.cpp new file mode 100644 index 0000000000..e7d61e3ac9 --- /dev/null +++ b/unittests/Basic/MemoryBufferCacheTest.cpp @@ -0,0 +1,95 @@ +//===- 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()), "", + /* RequiresNullTerminator = */ false); +} + +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 diff --git a/unittests/Basic/SourceManagerTest.cpp b/unittests/Basic/SourceManagerTest.cpp index a967b0ec7c..dddc3f9866 100644 --- a/unittests/Basic/SourceManagerTest.cpp +++ b/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); |