r215388 - unique_ptr-ify FileSystemStatCache::setNextStatCache
David Blaikie
dblaikie at gmail.com
Mon Aug 11 14:29:24 PDT 2014
Author: dblaikie
Date: Mon Aug 11 16:29:24 2014
New Revision: 215388
URL: http://llvm.org/viewvc/llvm-project?rev=215388&view=rev
Log:
unique_ptr-ify FileSystemStatCache::setNextStatCache
And in the process, discover that FileManager::removeStatCache had a
double-delete when removing an element from the middle of the list (at
the beginning or the end of the list, there was no problem) and add a
unit test to exercise the code path (which successfully crashed when run
(with modifications to match the old API) without this patch applied)
Modified:
cfe/trunk/include/clang/Basic/FileManager.h
cfe/trunk/include/clang/Basic/FileSystemStatCache.h
cfe/trunk/include/clang/Lex/PTHManager.h
cfe/trunk/lib/Basic/FileManager.cpp
cfe/trunk/lib/Frontend/CacheTokens.cpp
cfe/trunk/lib/Lex/PTHLexer.cpp
cfe/trunk/lib/Lex/Preprocessor.cpp
cfe/trunk/unittests/Basic/FileManagerTest.cpp
Modified: cfe/trunk/include/clang/Basic/FileManager.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileManager.h?rev=215388&r1=215387&r2=215388&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/FileManager.h (original)
+++ cfe/trunk/include/clang/Basic/FileManager.h Mon Aug 11 16:29:24 2014
@@ -194,7 +194,8 @@ public:
/// \param AtBeginning whether this new stat cache must be installed at the
/// beginning of the chain of stat caches. Otherwise, it will be added to
/// the end of the chain.
- void addStatCache(FileSystemStatCache *statCache, bool AtBeginning = false);
+ void addStatCache(std::unique_ptr<FileSystemStatCache> statCache,
+ bool AtBeginning = false);
/// \brief Removes the specified FileSystemStatCache object from the manager.
void removeStatCache(FileSystemStatCache *statCache);
Modified: cfe/trunk/include/clang/Basic/FileSystemStatCache.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileSystemStatCache.h?rev=215388&r1=215387&r2=215388&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/FileSystemStatCache.h (original)
+++ cfe/trunk/include/clang/Basic/FileSystemStatCache.h Mon Aug 11 16:29:24 2014
@@ -74,8 +74,8 @@ public:
/// \brief Sets the next stat call cache in the chain of stat caches.
/// Takes ownership of the given stat cache.
- void setNextStatCache(FileSystemStatCache *Cache) {
- NextStatCache.reset(Cache);
+ void setNextStatCache(std::unique_ptr<FileSystemStatCache> Cache) {
+ NextStatCache = std::move(Cache);
}
/// \brief Retrieve the next stat call cache in the chain.
Modified: cfe/trunk/include/clang/Lex/PTHManager.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/PTHManager.h?rev=215388&r1=215387&r2=215388&view=diff
==============================================================================
--- cfe/trunk/include/clang/Lex/PTHManager.h (original)
+++ cfe/trunk/include/clang/Lex/PTHManager.h Mon Aug 11 16:29:24 2014
@@ -131,7 +131,7 @@ public:
/// FileManager objects. These objects use the PTH data to speed up
/// calls to stat by memoizing their results from when the PTH file
/// was generated.
- FileSystemStatCache *createStatCache();
+ std::unique_ptr<FileSystemStatCache> createStatCache();
};
} // end namespace clang
Modified: cfe/trunk/lib/Basic/FileManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=215388&r1=215387&r2=215388&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/FileManager.cpp (original)
+++ cfe/trunk/lib/Basic/FileManager.cpp Mon Aug 11 16:29:24 2014
@@ -64,20 +64,20 @@ FileManager::~FileManager() {
delete VirtualDirectoryEntries[i];
}
-void FileManager::addStatCache(FileSystemStatCache *statCache,
+void FileManager::addStatCache(std::unique_ptr<FileSystemStatCache> statCache,
bool AtBeginning) {
assert(statCache && "No stat cache provided?");
if (AtBeginning || !StatCache.get()) {
- statCache->setNextStatCache(StatCache.release());
- StatCache.reset(statCache);
+ statCache->setNextStatCache(std::move(StatCache));
+ StatCache = std::move(statCache);
return;
}
FileSystemStatCache *LastCache = StatCache.get();
while (LastCache->getNextStatCache())
LastCache = LastCache->getNextStatCache();
-
- LastCache->setNextStatCache(statCache);
+
+ LastCache->setNextStatCache(std::move(statCache));
}
void FileManager::removeStatCache(FileSystemStatCache *statCache) {
@@ -96,7 +96,7 @@ void FileManager::removeStatCache(FileSy
PrevCache = PrevCache->getNextStatCache();
assert(PrevCache && "Stat cache not found for removal");
- PrevCache->setNextStatCache(statCache->getNextStatCache());
+ PrevCache->setNextStatCache(statCache->takeNextStatCache());
}
void FileManager::clearStatCaches() {
Modified: cfe/trunk/lib/Frontend/CacheTokens.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CacheTokens.cpp?rev=215388&r1=215387&r2=215388&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/CacheTokens.cpp (original)
+++ cfe/trunk/lib/Frontend/CacheTokens.cpp Mon Aug 11 16:29:24 2014
@@ -572,8 +572,10 @@ void clang::CacheTokens(Preprocessor &PP
PTHWriter PW(*OS, PP);
// Install the 'stat' system call listener in the FileManager.
- StatListener *StatCache = new StatListener(PW.getPM());
- PP.getFileManager().addStatCache(StatCache, /*AtBeginning=*/true);
+ auto StatCacheOwner = llvm::make_unique<StatListener>(PW.getPM());
+ StatListener *StatCache = StatCacheOwner.get();
+ PP.getFileManager().addStatCache(std::move(StatCacheOwner),
+ /*AtBeginning=*/true);
// Lex through the entire file. This will populate SourceManager with
// all of the header information.
Modified: cfe/trunk/lib/Lex/PTHLexer.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PTHLexer.cpp?rev=215388&r1=215387&r2=215388&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/PTHLexer.cpp (original)
+++ cfe/trunk/lib/Lex/PTHLexer.cpp Mon Aug 11 16:29:24 2014
@@ -732,6 +732,6 @@ public:
};
} // end anonymous namespace
-FileSystemStatCache *PTHManager::createStatCache() {
- return new PTHStatCache(*((PTHFileLookup*) FileLookup));
+std::unique_ptr<FileSystemStatCache> PTHManager::createStatCache() {
+ return llvm::make_unique<PTHStatCache>(*((PTHFileLookup *)FileLookup));
}
Modified: cfe/trunk/lib/Lex/Preprocessor.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Preprocessor.cpp?rev=215388&r1=215387&r2=215388&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/Preprocessor.cpp (original)
+++ cfe/trunk/lib/Lex/Preprocessor.cpp Mon Aug 11 16:29:24 2014
@@ -27,6 +27,7 @@
#include "clang/Lex/Preprocessor.h"
#include "clang/Basic/FileManager.h"
+#include "clang/Basic/FileSystemStatCache.h"
#include "clang/Basic/SourceManager.h"
#include "clang/Basic/TargetInfo.h"
#include "clang/Lex/CodeCompletionHandler.h"
Modified: cfe/trunk/unittests/Basic/FileManagerTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/FileManagerTest.cpp?rev=215388&r1=215387&r2=215388&view=diff
==============================================================================
--- cfe/trunk/unittests/Basic/FileManagerTest.cpp (original)
+++ cfe/trunk/unittests/Basic/FileManagerTest.cpp Mon Aug 11 16:29:24 2014
@@ -97,7 +97,7 @@ TEST_F(FileManagerTest, NoVirtualDirecto
// FileManager to report "file/directory doesn't exist". This
// avoids the possibility of the result of this test being affected
// by what's in the real file system.
- manager.addStatCache(new FakeStatCache);
+ manager.addStatCache(llvm::make_unique<FakeStatCache>());
EXPECT_EQ(nullptr, manager.getDirectory("virtual/dir/foo"));
EXPECT_EQ(nullptr, manager.getDirectory("virtual/dir"));
@@ -107,7 +107,7 @@ TEST_F(FileManagerTest, NoVirtualDirecto
// When a virtual file is added, all of its ancestors should be created.
TEST_F(FileManagerTest, getVirtualFileCreatesDirectoryEntriesForAncestors) {
// Fake an empty real file system.
- manager.addStatCache(new FakeStatCache);
+ manager.addStatCache(llvm::make_unique<FakeStatCache>());
manager.getVirtualFile("virtual/dir/bar.h", 100, 0);
EXPECT_EQ(nullptr, manager.getDirectory("virtual/dir/foo"));
@@ -124,7 +124,7 @@ TEST_F(FileManagerTest, getVirtualFileCr
// getFile() returns non-NULL if a real file exists at the given path.
TEST_F(FileManagerTest, getFileReturnsValidFileEntryForExistingRealFile) {
// Inject fake files into the file system.
- FakeStatCache *statCache = new FakeStatCache;
+ auto statCache = llvm::make_unique<FakeStatCache>();
statCache->InjectDirectory("/tmp", 42);
statCache->InjectFile("/tmp/test", 43);
@@ -135,7 +135,7 @@ TEST_F(FileManagerTest, getFileReturnsVa
statCache->InjectFile(FileName, 45);
#endif
- manager.addStatCache(statCache);
+ manager.addStatCache(std::move(statCache));
const FileEntry *file = manager.getFile("/tmp/test");
ASSERT_TRUE(file != nullptr);
@@ -158,7 +158,7 @@ TEST_F(FileManagerTest, getFileReturnsVa
// getFile() returns non-NULL if a virtual file exists at the given path.
TEST_F(FileManagerTest, getFileReturnsValidFileEntryForExistingVirtualFile) {
// Fake an empty real file system.
- manager.addStatCache(new FakeStatCache);
+ manager.addStatCache(llvm::make_unique<FakeStatCache>());
manager.getVirtualFile("virtual/dir/bar.h", 100, 0);
const FileEntry *file = manager.getFile("virtual/dir/bar.h");
@@ -175,11 +175,11 @@ TEST_F(FileManagerTest, getFileReturnsVa
TEST_F(FileManagerTest, getFileReturnsDifferentFileEntriesForDifferentFiles) {
// Inject two fake files into the file system. Different inodes
// mean the files are not symlinked together.
- FakeStatCache *statCache = new FakeStatCache;
+ auto statCache = llvm::make_unique<FakeStatCache>();
statCache->InjectDirectory(".", 41);
statCache->InjectFile("foo.cpp", 42);
statCache->InjectFile("bar.cpp", 43);
- manager.addStatCache(statCache);
+ manager.addStatCache(std::move(statCache));
const FileEntry *fileFoo = manager.getFile("foo.cpp");
const FileEntry *fileBar = manager.getFile("bar.cpp");
@@ -192,10 +192,10 @@ TEST_F(FileManagerTest, getFileReturnsDi
// exists at the given path.
TEST_F(FileManagerTest, getFileReturnsNULLForNonexistentFile) {
// Inject a fake foo.cpp into the file system.
- FakeStatCache *statCache = new FakeStatCache;
+ auto statCache = llvm::make_unique<FakeStatCache>();
statCache->InjectDirectory(".", 41);
statCache->InjectFile("foo.cpp", 42);
- manager.addStatCache(statCache);
+ manager.addStatCache(std::move(statCache));
// Create a virtual bar.cpp file.
manager.getVirtualFile("bar.cpp", 200, 0);
@@ -211,11 +211,11 @@ TEST_F(FileManagerTest, getFileReturnsNU
// getFile() returns the same FileEntry for real files that are aliases.
TEST_F(FileManagerTest, getFileReturnsSameFileEntryForAliasedRealFiles) {
// Inject two real files with the same inode.
- FakeStatCache *statCache = new FakeStatCache;
+ auto statCache = llvm::make_unique<FakeStatCache>();
statCache->InjectDirectory("abc", 41);
statCache->InjectFile("abc/foo.cpp", 42);
statCache->InjectFile("abc/bar.cpp", 42);
- manager.addStatCache(statCache);
+ manager.addStatCache(std::move(statCache));
EXPECT_EQ(manager.getFile("abc/foo.cpp"), manager.getFile("abc/bar.cpp"));
}
@@ -224,11 +224,11 @@ TEST_F(FileManagerTest, getFileReturnsSa
// corresponding real files that are aliases.
TEST_F(FileManagerTest, getFileReturnsSameFileEntryForAliasedVirtualFiles) {
// Inject two real files with the same inode.
- FakeStatCache *statCache = new FakeStatCache;
+ auto statCache = llvm::make_unique<FakeStatCache>();
statCache->InjectDirectory("abc", 41);
statCache->InjectFile("abc/foo.cpp", 42);
statCache->InjectFile("abc/bar.cpp", 42);
- manager.addStatCache(statCache);
+ manager.addStatCache(std::move(statCache));
manager.getVirtualFile("abc/foo.cpp", 100, 0);
manager.getVirtualFile("abc/bar.cpp", 200, 0);
@@ -236,6 +236,15 @@ TEST_F(FileManagerTest, getFileReturnsSa
EXPECT_EQ(manager.getFile("abc/foo.cpp"), manager.getFile("abc/bar.cpp"));
}
+TEST_F(FileManagerTest, addRemoveStatCache) {
+ manager.addStatCache(llvm::make_unique<FakeStatCache>());
+ auto statCacheOwner = llvm::make_unique<FakeStatCache>();
+ auto *statCache = statCacheOwner.get();
+ manager.addStatCache(std::move(statCacheOwner));
+ manager.addStatCache(llvm::make_unique<FakeStatCache>());
+ manager.removeStatCache(statCache);
+}
+
#endif // !LLVM_ON_WIN32
} // anonymous namespace
More information about the cfe-commits
mailing list