Instead of removing the FileEntry 'just' invalidate its cache.

When there is an error introduced by #include-ing a file we should be able to
hit the disk (in cling's context) the next time. The user could fix it in the
meanwhile.

Newer clang supports a flag which can be used to mark the user files as volatile.
This means that the file stat will be invalidated, causing clang to hit the
disk and fetch the new content of the FileEntry.

The complication comes from the fact that when the file size of the file entry
is invalidated the cache in the SourceManager is not syncronized, thus clang
issued an error. The patch in clang checks if the file size == 0 and the
modification time == 0 this means that we are in cling's context and have to sync
the cache and continue with no errors.

This fixes an issue exposed by Jerome's implementation. Before we removed the
entire FileEntry to achieve the same behavior, however the SourceManager kept
the reference to it. This leads to seg faults when iterating over the included
files (eg. .files or .storeState)
This commit is contained in:
Vassil Vassilev 2013-09-17 16:25:52 +02:00 committed by sftnight
parent 42c0385deb
commit 71910784b3
3 changed files with 40 additions and 8 deletions

View File

@ -30,7 +30,7 @@ namespace cling {
///
class DeclReverter : public DeclVisitor<DeclReverter, bool> {
private:
typedef llvm::DenseSet<const FileEntry*> FileEntries;
typedef llvm::DenseSet<FileID> FileIDs;
///\brief The Sema object being reverted (contains the AST as well).
///
@ -52,7 +52,7 @@ namespace cling {
/// This ADT keeps track of the files from which the reverted declarations
/// came from so that in the end they could be removed from clang's cache.
///
FileEntries m_FilesToUncache;
FileIDs m_FilesToUncache;
public:
DeclReverter(Sema* S, const Transaction* T): m_Sema(S), m_CurTransaction(T) {
@ -171,9 +171,19 @@ namespace cling {
DeclReverter::~DeclReverter() {
FileManager& FM = m_Sema->getSourceManager().getFileManager();
for (FileEntries::iterator I = m_FilesToUncache.begin(),
SourceManager& SM = m_Sema->getSourceManager();
for (FileIDs::iterator I = m_FilesToUncache.begin(),
E = m_FilesToUncache.end(); I != E; ++I) {
FM.invalidateCache(*I);
const SrcMgr::FileInfo& fInfo = SM.getSLocEntry(*I).getFile();
// We need to reset the cache
SrcMgr::ContentCache* cache
= const_cast<SrcMgr::ContentCache*>(fInfo.getContentCache());
cache->replaceBuffer(0,/*free*/true);
FileEntry* entry = const_cast<FileEntry*>(cache->ContentsEntry);
// We have to reset the file entry size to keep the cache and the file
// entry in sync.
if (entry)
FileManager::modifyFileEntry(entry, /*size*/0, 0);
}
// Clean up the pending instantiations
@ -184,9 +194,9 @@ namespace cling {
void DeclReverter::PreVisitDecl(Decl *D) {
const SourceLocation Loc = D->getLocStart();
const SourceManager& SM = m_Sema->getSourceManager();
const FileEntry* OldEntry = SM.getFileEntryForID(SM.getFileID(Loc));
if (OldEntry && !m_FilesToUncache.count(OldEntry))
m_FilesToUncache.insert(OldEntry);
FileID FID = SM.getFileID(Loc);
if (!FID.isInvalid() && !m_FilesToUncache.count(FID))
m_FilesToUncache.insert(FID);
}
// Gives us access to the protected members that we need.

View File

@ -234,7 +234,10 @@ namespace cling {
// Set up source and file managers
CI->createFileManager();
CI->createSourceManager(CI->getFileManager());
SourceManager* SM = new SourceManager(CI->getDiagnostics(),
CI->getFileManager(),
/*UserFilesAreVolatile*/ true);
CI->setSourceManager(SM); // FIXME: SM leaks.
// Set up the memory buffer
if (buffer)

View File

@ -0,0 +1,19 @@
diff --git a/interpreter/llvm/src/tools/clang/lib/Basic/SourceManager.cpp b/interpreter/llvm/src/tools/clang/lib/Basic/SourceManager.cpp
index 44649c3..8caa8f0 100644
--- tools/clang/lib/Basic/SourceManager.cpp
+++ tools/clang/lib/Basic/SourceManager.cpp
@@ -132,6 +132,14 @@ const llvm::MemoryBuffer *ContentCache::getBuffer(DiagnosticsEngine &Diag,
if (Invalid) *Invalid = true;
return Buffer.getPointer();
}
+
+ // If there is no information about the size and the modification time,
+ // this means that we are in cling-like context and we should sync the
+ // FileEntry and the invalidated cache.
+ if (!ContentsEntry->getSize() && !ContentsEntry->getModificationTime()) {
+ FileManager::modifyFileEntry(const_cast<FileEntry*>(ContentsEntry),
+ getRawBuffer()->getBufferSize(), time(0));
+ }
// Check that the file's size is the same as in the file entry (which may
// have come from a stat cache).