r312851 - Fix ownership of the MemoryBuffer in a FrontendInputFile.

Richard Smith via cfe-commits cfe-commits at lists.llvm.org
Fri Sep 8 18:14:04 PDT 2017


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
+  /// 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);
     }
   }
 




More information about the cfe-commits mailing list