r312851 - Fix ownership of the MemoryBuffer in a FrontendInputFile.
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Wed Apr 24 17:20:08 PDT 2019
On Fri, 29 Mar 2019 at 05:41, Nico Weber via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
>
>
> (below)
>
> On Fri, Sep 8, 2017 at 9:15 PM Richard Smith via cfe-commits <cfe-commits at 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?
Oops, no. Comment fixed in r359158.
>> + /// 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 at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list