WritableMemoryBuffer

[zturner]

Summary

I'm explicitly not including llvm-commits on here because this patch isn't done. Unfortunately I have to switch gears because something high priority came up. You're welcome to finish this up if you want, otherwise I won't hold up the patch on the LLDB side. The quirk that I ran into is that llvm::sys::fs::openFileForWrite() does not have correct semantics when the file already exists. Currently it always overwrites the file. Fixing this is more work than I have time for at the moment :(

I might come back to this later if you don't feel up to it, but you're welcome to move forward with the LLDB patch to unblock yourself.

[labath]

You don't need to (and in fact, shouldn't) open the file read-write for this to
work. This is version that does that, but I don't feel like writing a
motivational CL description right now. I'll do it on monday if you don't beat
me to it.

include/llvm/Support/MemoryBuffer.h

 //  This file defines the MemoryBuffer interface.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_SUPPORT_MEMORYBUFFER_H
 #define LLVM_SUPPORT_MEMORYBUFFER_H
 
 #include "llvm-c/Types.h"
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/CBindingWrapping.h"
 #include "llvm/Support/ErrorOr.h"
+#include "llvm/Support/FileSystem.h"
 #include <cstddef>
 #include <cstdint>
 #include <memory>
 
 namespace llvm {
 
 class MemoryBufferRef;
 
public:
   const char *getBufferStart() const { return Buffer.begin(); }
   const char *getBufferEnd() const { return Buffer.end(); }
   size_t getBufferSize() const { return Buffer.size(); }
 };
 
 // Create wrappers for C Binding types (see CBindingWrapping.h).
 DEFINE_SIMPLE_CONVERSION_FUNCTIONS(MemoryBuffer, LLVMMemoryBufferRef)
 
+/// This interface is similar to MemoryBuffer, but allows writing to the underlying
+/// contents.  It only supports creation methods that are guaranteed to produce a
+/// writable buffer.  For example, mapping a file read-only is not supported.
+class WritableMemoryBuffer {
+  std::unique_ptr<MemoryBuffer> Buffer;
+
+  explicit WritableMemoryBuffer(std::unique_ptr<MemoryBuffer> Buffer)
+    : Buffer(std::move(Buffer)) {}
+
+public:
+  // const_cast is well-defined here, because the underlying buffer is guaranteed to
+  // have been initialized with a mutable buffer.
+  uint8_t *getBufferStart() { return getBuffer().begin(); }
+  uint8_t *getBufferEnd() { return getBuffer().end(); }
+
+  const uint8_t *getBufferStart() const { return getBuffer().begin(); }
+  const uint8_t *getBufferEnd() const { return getBuffer().end(); }
+
+  size_t getBufferSize() const { return Buffer->getBufferSize(); }
+
+  ArrayRef<uint8_t> getBuffer() const {
+    const uint8_t *Start = reinterpret_cast<const uint8_t*>(Buffer->getBufferStart());
+    const uint8_t *End = reinterpret_cast<const uint8_t*>(Buffer->getBufferEnd());
+    return makeArrayRef(Start, End);
+  }
+  MutableArrayRef<uint8_t> getBuffer() {
+    auto B = static_cast<const WritableMemoryBuffer*>(this)->getBuffer();
+    return MutableArrayRef<uint8_t>(
+      const_cast<uint8_t*>(B.begin()), const_cast<uint8_t*>(B.end()));
+  }
+
+  StringRef getBufferIdentifier() const { return Buffer->getBufferIdentifier(); }
+  MemoryBuffer::BufferKind getBufferKind() const { return Buffer->getBufferKind(); }
+
+  static ErrorOr<std::unique_ptr<WritableMemoryBuffer>>
+    getFile(const Twine &Filename, int64_t FileSize = -1,
+      bool RequiresNullTerminator = true, bool IsVolatile = false);
+
+  /// Map a subrange of the specified file as a WritableMemoryBuffer.
+  static ErrorOr<std::unique_ptr<WritableMemoryBuffer>>
+    getFileSlice(const Twine &Filename, uint64_t MapSize, uint64_t Offset,
+      bool IsVolatile = false);
+};
+
 } // end namespace llvm
 
 #endif // LLVM_SUPPORT_MEMORYBUFFER_H

lib/Support/MemoryBuffer.cpp

public:
   BufferKind getBufferKind() const override {
     return MemoryBuffer_Malloc;
   }
 };
 }
 
 static ErrorOr<std::unique_ptr<MemoryBuffer>>
 getFileAux(const Twine &Filename, int64_t FileSize, uint64_t MapSize, 
-           uint64_t Offset, bool RequiresNullTerminator, bool IsVolatile);
+           uint64_t Offset, bool RequiresNullTerminator, bool IsVolatile, bool WritableBuffer);
 
 std::unique_ptr<MemoryBuffer>
 MemoryBuffer::getMemBuffer(StringRef InputData, StringRef BufferName,
                            bool RequiresNullTerminator) {
   auto *Ret = new (NamedBufferAlloc(BufferName))
       MemoryBufferMem(InputData, RequiresNullTerminator);
   return std::unique_ptr<MemoryBuffer>(Ret);
 }
MemoryBuffer::getFileOrSTDIN(const Twine &Filename, int64_t FileSize,
   if (NameRef == "-")
     return getSTDIN();
   return getFile(Filename, FileSize, RequiresNullTerminator);
 }
 
 ErrorOr<std::unique_ptr<MemoryBuffer>>
 MemoryBuffer::getFileSlice(const Twine &FilePath, uint64_t MapSize, 
                            uint64_t Offset, bool IsVolatile) {
-  return getFileAux(FilePath, -1, MapSize, Offset, false, IsVolatile);
+  return getFileAux(FilePath, -1, MapSize, Offset, false, IsVolatile, /*WritableBuffer*/ false);
 }
 
-
 //===----------------------------------------------------------------------===//
 // MemoryBuffer::getFile implementation.
 //===----------------------------------------------------------------------===//
 
 namespace {
 /// \brief Memory maps a file descriptor using sys::fs::mapped_file_region.
 ///
 /// This handles converting the offset into a legal offset on the platform.
class MemoryBufferMMapFile : public MemoryBuffer {
   }
 
   const char *getStart(uint64_t Len, uint64_t Offset) {
     return MFR.const_data() + (Offset - getLegalMapOffset(Offset));
   }
 
 public:
   MemoryBufferMMapFile(bool RequiresNullTerminator, int FD, uint64_t Len,
-                       uint64_t Offset, std::error_code &EC)
-      : MFR(FD, sys::fs::mapped_file_region::readonly,
+                       uint64_t Offset, bool Writable, std::error_code &EC)
+      : MFR(FD, Writable ? sys::fs::mapped_file_region::priv : sys::fs::mapped_file_region::readonly,
             getLegalMapSize(Len, Offset), getLegalMapOffset(Offset), EC) {
     if (!EC) {
       const char *Start = getStart(Len, Offset);
       init(Start, Start + Len, RequiresNullTerminator);
     }
   }
 
   /// Disable sized deallocation for MemoryBufferMMapFile, because it has
getMemoryBufferForStream(int FD, const Twine &BufferName) {
   return MemoryBuffer::getMemBufferCopy(Buffer, BufferName);
 }
 
 
 ErrorOr<std::unique_ptr<MemoryBuffer>>
 MemoryBuffer::getFile(const Twine &Filename, int64_t FileSize,
                       bool RequiresNullTerminator, bool IsVolatile) {
   return getFileAux(Filename, FileSize, FileSize, 0,
-                    RequiresNullTerminator, IsVolatile);
+                    RequiresNullTerminator, IsVolatile, /*WritableBuffer*/ false);
 }
 
 static ErrorOr<std::unique_ptr<MemoryBuffer>>
 getOpenFileImpl(int FD, const Twine &Filename, uint64_t FileSize,
                 uint64_t MapSize, int64_t Offset, bool RequiresNullTerminator,
-                bool IsVolatile);
+                bool IsVolatile, bool WritableBuffer);
 
 static ErrorOr<std::unique_ptr<MemoryBuffer>>
 getFileAux(const Twine &Filename, int64_t FileSize, uint64_t MapSize,
-           uint64_t Offset, bool RequiresNullTerminator, bool IsVolatile) {
+           uint64_t Offset, bool RequiresNullTerminator, bool IsVolatile, bool WritableBuffer) {
   int FD;
   std::error_code EC = sys::fs::openFileForRead(Filename, FD);
+
   if (EC)
     return EC;
 
   ErrorOr<std::unique_ptr<MemoryBuffer>> Ret =
       getOpenFileImpl(FD, Filename, FileSize, MapSize, Offset,
-                      RequiresNullTerminator, IsVolatile);
+                      RequiresNullTerminator, IsVolatile, WritableBuffer);
   close(FD);
   return Ret;
 }
 
+ErrorOr<std::unique_ptr<WritableMemoryBuffer>>
+WritableMemoryBuffer::getFile(const Twine &Filename, int64_t FileSize,
+  bool RequiresNullTerminator, bool IsVolatile) {
+  auto Result = getFileAux(Filename, FileSize, FileSize, 0, RequiresNullTerminator, IsVolatile, /*WritableBuffer*/ true);
+  if (!Result)
+    return Result.getError();
+  return std::unique_ptr<WritableMemoryBuffer>(new WritableMemoryBuffer(std::move(*Result)));
+}
+
+ErrorOr<std::unique_ptr<WritableMemoryBuffer>>
+WritableMemoryBuffer::getFileSlice(const Twine &Filename, uint64_t MapSize, uint64_t Offset,
+  bool IsVolatile) {
+  auto Result = getFileAux(Filename, -1, MapSize, Offset, false, IsVolatile, sys::fs::F_RW);
+  if (!Result)
+    return Result.getError();
+
+  return std::unique_ptr<WritableMemoryBuffer>(new WritableMemoryBuffer(std::move(*Result)));
+}
+
+
+
 static bool shouldUseMmap(int FD,
                           size_t FileSize,
                           size_t MapSize,
                           off_t Offset,
                           bool RequiresNullTerminator,
                           int PageSize,
                           bool IsVolatile) {
   // mmap may leave the buffer without null terminator if the file size changed

 #endif
 
   return true;
 }
 
 static ErrorOr<std::unique_ptr<MemoryBuffer>>
 getOpenFileImpl(int FD, const Twine &Filename, uint64_t FileSize,
                 uint64_t MapSize, int64_t Offset, bool RequiresNullTerminator,
-                bool IsVolatile) {
+                bool IsVolatile, bool WritableBuffer) {
   static int PageSize = sys::Process::getPageSize();
 
   // Default is to map the full file.
   if (MapSize == uint64_t(-1)) {
     // If we don't know the file size, use fstat to find out.  fstat on an open
     // file descriptor is cheaper than stat on a random path.
     if (FileSize == uint64_t(-1)) {
       sys::fs::file_status Status;
if (MapSize == uint64_t(-1)) {
     MapSize = FileSize;
   }
 
   if (shouldUseMmap(FD, FileSize, MapSize, Offset, RequiresNullTerminator,
                     PageSize, IsVolatile)) {
     std::error_code EC;
     std::unique_ptr<MemoryBuffer> Result(
         new (NamedBufferAlloc(Filename))
-        MemoryBufferMMapFile(RequiresNullTerminator, FD, MapSize, Offset, EC));
+        MemoryBufferMMapFile(RequiresNullTerminator, FD, MapSize, Offset, WritableBuffer, EC));
     if (!EC)
       return std::move(Result);
   }
 
   std::unique_ptr<MemoryBuffer> Buf =
       MemoryBuffer::getNewUninitMemBuffer(MapSize, Filename);
   if (!Buf) {
     // Failed to create a buffer. The only way it can fail is if
#endif
 
   return std::move(Buf);
 }
 
 ErrorOr<std::unique_ptr<MemoryBuffer>>
 MemoryBuffer::getOpenFile(int FD, const Twine &Filename, uint64_t FileSize,
                           bool RequiresNullTerminator, bool IsVolatile) {
   return getOpenFileImpl(FD, Filename, FileSize, FileSize, 0,
-                         RequiresNullTerminator, IsVolatile);
+                         RequiresNullTerminator, IsVolatile, false);
 }
 
 ErrorOr<std::unique_ptr<MemoryBuffer>>
 MemoryBuffer::getOpenFileSlice(int FD, const Twine &Filename, uint64_t MapSize,
                                int64_t Offset, bool IsVolatile) {
   assert(MapSize != uint64_t(-1));
-  return getOpenFileImpl(FD, Filename, -1, MapSize, Offset, false, IsVolatile);
+  return getOpenFileImpl(FD, Filename, -1, MapSize, Offset, false, IsVolatile, /*WritableBuffer*/ false);
 }
 
 ErrorOr<std::unique_ptr<MemoryBuffer>> MemoryBuffer::getSTDIN() {
   // Read in all of the data from stdin, we cannot mmap stdin.
   //
   // FIXME: That isn't necessarily true, we should try to mmap stdin and
   // fallback if it fails.
   sys::ChangeStdinToBinary();

unittests/Support/MemoryBufferTest.cpp

 // This file implements unit tests for the MemoryBuffer support class.
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/raw_ostream.h"
+#include "llvm/Testing/Support/Error.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
 
 namespace {
 
 class MemoryBufferTest : public testing::Test {
 protected:
TEST_F(MemoryBufferTest, slice) {
   EXPECT_EQ(0x3000UL, MB2.get()->getBufferSize());
   
   StringRef BufData2 = MB2.get()->getBuffer();
   EXPECT_TRUE(BufData2.substr(0x0000,8).equals("12345678"));
   EXPECT_TRUE(BufData2.substr(0x17F8,8).equals("12345678"));
   EXPECT_TRUE(BufData2.substr(0x1800,8).equals("abcdefgh"));
   EXPECT_TRUE(BufData2.substr(0x2FF8,8).equals("abcdefgh"));
 }
+
+TEST_F(MemoryBufferTest, writableSlice) {
+  // Create a file initialized with a bunch of zeros.
+  int FD;
+  SmallString<64> TestPath;
+  sys::fs::createTemporaryFile("MemoryBufferTest_WritableSlice", "temp", FD, TestPath);
+  FileRemover Cleanup(TestPath);
+  raw_fd_ostream OF(FD, true, /*unbuffered=*/true);
+  for (unsigned i = 0; i < 0x4000; ++i) {
+    OF << '0';
+  }
+  OF.close();
+
+  auto MBOrError = WritableMemoryBuffer::getFileSlice(TestPath.str(), 0x1000, 0x2000);
+  std::error_code EC = MBOrError.getError();
+  ASSERT_FALSE(EC);
+
+  {
+    // Write some data.  It should be mapped private, so that upon completion
+    // the original file contents are not modified.
+    WritableMemoryBuffer &MB = **MBOrError;
+    ASSERT_EQ(0x1000u, MB.getBufferSize());
+    uint8_t *Start = MB.getBufferStart();
+    ASSERT_EQ(MB.getBufferEnd(), MB.getBufferStart() + MB.getBufferSize());
+    ::memset(Start, '1', MB.getBufferSize());
+  }
+
+  auto MB2OrError = MemoryBuffer::getFile(TestPath);
+  ASSERT_FALSE(MB2OrError.getError());
+  auto &MB2 = **MB2OrError;
+  ASSERT_EQ(0x4000u, MB2.getBufferSize());
+  for(size_t i = 0; i < MB2.getBufferSize(); ++i)
+    EXPECT_EQ('0', MB2.getBufferStart()[i]) << "i: " << i;
+}
 }