r312851 - Fix ownership of the MemoryBuffer in a FrontendInputFile.

Nico Weber via cfe-commits cfe-commits at lists.llvm.org
Fri Mar 29 05:41:16 PDT 2019


(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?


> +  /// 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20190329/be4b2f95/attachment.html>


More information about the cfe-commits mailing list