From c815108d08b0417c6f1104e7df70dc5278839406 Mon Sep 17 00:00:00 2001 From: Douglas Gregor Date: Tue, 16 Mar 2010 22:53:51 +0000 Subject: [PATCH] Teach SourceManager's content cache to keep track of whether its buffer was invalid when it was created, and use that bit to always set the "Invalid" flag according to whether the buffer is invalid. This ensures that all accesses to an invalid buffer are marked invalid, improving recovery. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@98690 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Basic/SourceManager.h | 17 +++++++---- lib/Basic/SourceManager.cpp | 45 +++++++++++++++-------------- test/PCH/changed-files.c | 1 + 3 files changed, 36 insertions(+), 27 deletions(-) diff --git a/include/clang/Basic/SourceManager.h b/include/clang/Basic/SourceManager.h index ef51a58883..d1239694fc 100644 --- a/include/clang/Basic/SourceManager.h +++ b/include/clang/Basic/SourceManager.h @@ -17,6 +17,7 @@ #include "clang/Basic/SourceLocation.h" #include "llvm/Support/Allocator.h" #include "llvm/System/DataTypes.h" +#include "llvm/ADT/PointerIntPair.h" #include "llvm/ADT/PointerUnion.h" #include "llvm/ADT/DenseMap.h" #include @@ -54,7 +55,8 @@ namespace SrcMgr { class ContentCache { /// Buffer - The actual buffer containing the characters from the input /// file. This is owned by the ContentCache object. - mutable const llvm::MemoryBuffer *Buffer; + /// The bit indicates whether the buffer is invalid. + mutable llvm::PointerIntPair Buffer; public: /// Reference to the file entry. This reference does not own @@ -92,8 +94,9 @@ namespace SrcMgr { unsigned getSizeBytesMapped() const; void setBuffer(const llvm::MemoryBuffer *B) { - assert(!Buffer && "MemoryBuffer already set."); - Buffer = B; + assert(!Buffer.getPointer() && "MemoryBuffer already set."); + Buffer.setPointer(B); + Buffer.setInt(false); } /// \brief Replace the existing buffer (which will be deleted) @@ -101,17 +104,19 @@ namespace SrcMgr { void replaceBuffer(const llvm::MemoryBuffer *B); ContentCache(const FileEntry *Ent = 0) - : Buffer(0), Entry(Ent), SourceLineCache(0), NumLines(0) {} + : Buffer(0, false), Entry(Ent), SourceLineCache(0), NumLines(0) {} ~ContentCache(); /// The copy ctor does not allow copies where source object has either /// a non-NULL Buffer or SourceLineCache. Ownership of allocated memory /// is not transfered, so this is a logical error. - ContentCache(const ContentCache &RHS) : Buffer(0), SourceLineCache(0) { + ContentCache(const ContentCache &RHS) + : Buffer(0, false), SourceLineCache(0) + { Entry = RHS.Entry; - assert (RHS.Buffer == 0 && RHS.SourceLineCache == 0 + assert (RHS.Buffer.getPointer() == 0 && RHS.SourceLineCache == 0 && "Passed ContentCache object cannot own a buffer."); NumLines = RHS.NumLines; diff --git a/lib/Basic/SourceManager.cpp b/lib/Basic/SourceManager.cpp index 254aec0226..be7c256b5b 100644 --- a/lib/Basic/SourceManager.cpp +++ b/lib/Basic/SourceManager.cpp @@ -32,14 +32,14 @@ using llvm::MemoryBuffer; //===----------------------------------------------------------------------===// ContentCache::~ContentCache() { - delete Buffer; + delete Buffer.getPointer(); } /// getSizeBytesMapped - Returns the number of bytes actually mapped for /// this ContentCache. This can be 0 if the MemBuffer was not actually /// instantiated. unsigned ContentCache::getSizeBytesMapped() const { - return Buffer ? Buffer->getBufferSize() : 0; + return Buffer.getPointer() ? Buffer.getPointer()->getBufferSize() : 0; } /// getSize - Returns the size of the content encapsulated by this ContentCache. @@ -47,15 +47,16 @@ unsigned ContentCache::getSizeBytesMapped() const { /// scratch buffer. If the ContentCache encapsulates a source file, that /// file is not lazily brought in from disk to satisfy this query. unsigned ContentCache::getSize() const { - return Buffer ? (unsigned) Buffer->getBufferSize() - : (unsigned) Entry->getSize(); + return Buffer.getPointer() ? (unsigned) Buffer.getPointer()->getBufferSize() + : (unsigned) Entry->getSize(); } void ContentCache::replaceBuffer(const llvm::MemoryBuffer *B) { - assert(B != Buffer); + assert(B != Buffer.getPointer()); - delete Buffer; - Buffer = B; + delete Buffer.getPointer(); + Buffer.setPointer(B); + Buffer.setInt(false); } const llvm::MemoryBuffer *ContentCache::getBuffer(Diagnostic &Diag, @@ -64,12 +65,13 @@ const llvm::MemoryBuffer *ContentCache::getBuffer(Diagnostic &Diag, *Invalid = false; // Lazily create the Buffer for ContentCaches that wrap files. - if (!Buffer && Entry) { + if (!Buffer.getPointer() && Entry) { std::string ErrorStr; struct stat FileInfo; - Buffer = MemoryBuffer::getFile(Entry->getName(), &ErrorStr, - Entry->getSize(), &FileInfo); - + Buffer.setPointer(MemoryBuffer::getFile(Entry->getName(), &ErrorStr, + Entry->getSize(), &FileInfo)); + Buffer.setInt(false); + // If we were unable to open the file, then we are in an inconsistent // situation where the content cache referenced a file which no longer // exists. Most likely, we were using a stat cache with an invalid entry but @@ -80,16 +82,16 @@ const llvm::MemoryBuffer *ContentCache::getBuffer(Diagnostic &Diag, // currently handle returning a null entry here. Ideally we should detect // that we are in an inconsistent situation and error out as quickly as // possible. - if (!Buffer) { + if (!Buffer.getPointer()) { const llvm::StringRef FillStr("<<>>\n"); - Buffer = MemoryBuffer::getNewMemBuffer(Entry->getSize(), ""); - char *Ptr = const_cast(Buffer->getBufferStart()); + Buffer.setPointer(MemoryBuffer::getNewMemBuffer(Entry->getSize(), + "")); + char *Ptr = const_cast(Buffer.getPointer()->getBufferStart()); for (unsigned i = 0, e = Entry->getSize(); i != e; ++i) Ptr[i] = FillStr[i % FillStr.size()]; Diag.Report(diag::err_cannot_open_file) << Entry->getName() << ErrorStr; - if (Invalid) - *Invalid = true; + Buffer.setInt(true); } else { // Check that the file's size and modification time is the same as // in the file entry (which may have come from a stat cache). @@ -97,17 +99,18 @@ const llvm::MemoryBuffer *ContentCache::getBuffer(Diagnostic &Diag, Diag.Report(diag::err_file_size_changed) << Entry->getName() << (unsigned)Entry->getSize() << (unsigned)FileInfo.st_size; - if (Invalid) - *Invalid = true; + Buffer.setInt(true); } else if (FileInfo.st_mtime != Entry->getModificationTime()) { Diag.Report(diag::err_file_modified) << Entry->getName(); - if (Invalid) - *Invalid = true; + Buffer.setInt(true); } } } - return Buffer; + if (Invalid) + *Invalid = Buffer.getInt(); + + return Buffer.getPointer(); } unsigned LineTableInfo::getLineTableFilenameID(const char *Ptr, unsigned Len) { diff --git a/test/PCH/changed-files.c b/test/PCH/changed-files.c index 4ef80ffb04..bca1caa0e8 100644 --- a/test/PCH/changed-files.c +++ b/test/PCH/changed-files.c @@ -1,5 +1,6 @@ const char *s0 = m0; int s1 = m1; +const char *s2 = m0; // RUN: echo '#define m0 ""' > %t.h // RUN: %clang_cc1 -emit-pch -o %t.h.pch %t.h -- 2.40.0