(below) On Fri, Sep 8, 2017 at 9:15 PM Richard Smith via cfe-commits < cfe-commits@lists.llvm.org> wrote:
> Author: rsmith > Date: Fri Sep 8 18:14:04 2017 > New Revision: 312851 > > URL: http://llvm.org/viewvc/llvm-project?rev=312851&view=rev > Log: > Fix ownership of the MemoryBuffer in a FrontendInputFile. > > This fixes a possible crash on certain kinds of corrupted AST file, but > checking in an AST file corrupted in just the right way will be a > maintenance > nightmare because the format changes frequently. > > Modified: > cfe/trunk/include/clang/Basic/SourceManager.h > cfe/trunk/include/clang/Frontend/FrontendOptions.h > cfe/trunk/lib/Basic/SourceManager.cpp > cfe/trunk/lib/Frontend/CompilerInstance.cpp > cfe/trunk/lib/Frontend/FrontendAction.cpp > > Modified: cfe/trunk/include/clang/Basic/SourceManager.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/SourceManager.h?rev=312851&r1=312850&r2=312851&view=diff > > ============================================================================== > --- cfe/trunk/include/clang/Basic/SourceManager.h (original) > +++ cfe/trunk/include/clang/Basic/SourceManager.h Fri Sep 8 18:14:04 2017 > @@ -212,12 +212,6 @@ namespace SrcMgr { > /// this content cache. This is used for performance analysis. > llvm::MemoryBuffer::BufferKind getMemoryBufferKind() const; > > - void setBuffer(std::unique_ptr<llvm::MemoryBuffer> B) { > - assert(!Buffer.getPointer() && "MemoryBuffer already set."); > - Buffer.setPointer(B.release()); > - Buffer.setInt(0); > - } > - > /// \brief Get the underlying buffer, returning NULL if the buffer is > not > /// yet available. > llvm::MemoryBuffer *getRawBuffer() const { return > Buffer.getPointer(); } > @@ -816,7 +810,22 @@ public: > SrcMgr::CharacteristicKind FileCharacter = > SrcMgr::C_User, > int LoadedID = 0, unsigned LoadedOffset = 0, > SourceLocation IncludeLoc = SourceLocation()) { > - return createFileID(createMemBufferContentCache(std::move(Buffer)), > + return createFileID( > + createMemBufferContentCache(Buffer.release(), /*DoNotFree*/ > false), > + IncludeLoc, FileCharacter, LoadedID, LoadedOffset); > + } > + > + enum UnownedTag { Unowned }; > + > + /// \brief Create a new FileID that represents the specified memory > buffer. > + /// > + /// This does no caching of the buffer and takes ownership of the > Is the "and takes ownership" part correct for this new overload? > + /// MemoryBuffer, so only pass a MemoryBuffer to this once. > + FileID createFileID(UnownedTag, llvm::MemoryBuffer *Buffer, > + SrcMgr::CharacteristicKind FileCharacter = > SrcMgr::C_User, > + int LoadedID = 0, unsigned LoadedOffset = 0, > + SourceLocation IncludeLoc = SourceLocation()) { > + return createFileID(createMemBufferContentCache(Buffer, > /*DoNotFree*/true), > IncludeLoc, FileCharacter, LoadedID, > LoadedOffset); > } > > @@ -1699,7 +1708,7 @@ private: > > /// \brief Create a new ContentCache for the specified memory buffer. > const SrcMgr::ContentCache * > - createMemBufferContentCache(std::unique_ptr<llvm::MemoryBuffer> Buf); > + createMemBufferContentCache(llvm::MemoryBuffer *Buf, bool DoNotFree); > > FileID getFileIDSlow(unsigned SLocOffset) const; > FileID getFileIDLocal(unsigned SLocOffset) const; > > Modified: cfe/trunk/include/clang/Frontend/FrontendOptions.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/FrontendOptions.h?rev=312851&r1=312850&r2=312851&view=diff > > ============================================================================== > --- cfe/trunk/include/clang/Frontend/FrontendOptions.h (original) > +++ cfe/trunk/include/clang/Frontend/FrontendOptions.h Fri Sep 8 18:14:04 > 2017 > @@ -128,21 +128,24 @@ class FrontendInputFile { > /// \brief The file name, or "-" to read from standard input. > std::string File; > > - llvm::MemoryBuffer *Buffer; > + /// The input, if it comes from a buffer rather than a file. This object > + /// does not own the buffer, and the caller is responsible for ensuring > + /// that it outlives any users. > + llvm::MemoryBuffer *Buffer = nullptr; > > /// \brief The kind of input, e.g., C source, AST file, LLVM IR. > InputKind Kind; > > /// \brief Whether we're dealing with a 'system' input (vs. a 'user' > input). > - bool IsSystem; > + bool IsSystem = false; > > public: > - FrontendInputFile() : Buffer(nullptr), Kind(), IsSystem(false) { } > + FrontendInputFile() { } > FrontendInputFile(StringRef File, InputKind Kind, bool IsSystem = false) > - : File(File.str()), Buffer(nullptr), Kind(Kind), IsSystem(IsSystem) { > } > - FrontendInputFile(llvm::MemoryBuffer *buffer, InputKind Kind, > + : File(File.str()), Kind(Kind), IsSystem(IsSystem) { } > + FrontendInputFile(llvm::MemoryBuffer *Buffer, InputKind Kind, > bool IsSystem = false) > - : Buffer(buffer), Kind(Kind), IsSystem(IsSystem) { } > + : Buffer(Buffer), Kind(Kind), IsSystem(IsSystem) { } > > InputKind getKind() const { return Kind; } > bool isSystem() const { return IsSystem; } > > Modified: cfe/trunk/lib/Basic/SourceManager.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/SourceManager.cpp?rev=312851&r1=312850&r2=312851&view=diff > > ============================================================================== > --- cfe/trunk/lib/Basic/SourceManager.cpp (original) > +++ cfe/trunk/lib/Basic/SourceManager.cpp Fri Sep 8 18:14:04 2017 > @@ -409,15 +409,16 @@ SourceManager::getOrCreateContentCache(c > } > > > -/// createMemBufferContentCache - Create a new ContentCache for the > specified > -/// memory buffer. This does no caching. > -const ContentCache *SourceManager::createMemBufferContentCache( > - std::unique_ptr<llvm::MemoryBuffer> Buffer) { > +/// Create a new ContentCache for the specified memory buffer. > +/// This does no caching. > +const ContentCache * > +SourceManager::createMemBufferContentCache(llvm::MemoryBuffer *Buffer, > + bool DoNotFree) { > // Add a new ContentCache to the MemBufferInfos list and return it. > ContentCache *Entry = ContentCacheAlloc.Allocate<ContentCache>(); > new (Entry) ContentCache(); > MemBufferInfos.push_back(Entry); > - Entry->setBuffer(std::move(Buffer)); > + Entry->replaceBuffer(Buffer, DoNotFree); > return Entry; > } > > > Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=312851&r1=312850&r2=312851&view=diff > > ============================================================================== > --- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original) > +++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Fri Sep 8 18:14:04 2017 > @@ -838,8 +838,8 @@ bool CompilerInstance::InitializeSourceM > : Input.isSystem() ? SrcMgr::C_System : SrcMgr::C_User; > > if (Input.isBuffer()) { > - SourceMgr.setMainFileID(SourceMgr.createFileID( > - std::unique_ptr<llvm::MemoryBuffer>(Input.getBuffer()), Kind)); > + SourceMgr.setMainFileID(SourceMgr.createFileID(SourceManager::Unowned, > + Input.getBuffer(), > Kind)); > assert(SourceMgr.getMainFileID().isValid() && > "Couldn't establish MainFileID!"); > return true; > > Modified: cfe/trunk/lib/Frontend/FrontendAction.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendAction.cpp?rev=312851&r1=312850&r2=312851&view=diff > > ============================================================================== > --- cfe/trunk/lib/Frontend/FrontendAction.cpp (original) > +++ cfe/trunk/lib/Frontend/FrontendAction.cpp Fri Sep 8 18:14:04 2017 > @@ -754,10 +754,11 @@ bool FrontendAction::BeginSourceFile(Com > goto failure; > > // Reinitialize the main file entry to refer to the new input. > - if (!CI.InitializeSourceManager(FrontendInputFile( > - Buffer.release(), > Input.getKind().withFormat(InputKind::Source), > - CurrentModule->IsSystem))) > - goto failure; > + auto Kind = CurrentModule->IsSystem ? SrcMgr::C_System : > SrcMgr::C_User; > + auto &SourceMgr = CI.getSourceManager(); > + auto BufferID = SourceMgr.createFileID(std::move(Buffer), Kind); > + assert(BufferID.isValid() && "couldn't creaate module buffer ID"); > + SourceMgr.setMainFileID(BufferID); > } > } > > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
