[Support] Add WritableMemoryBuffer::getNewMemBuffer

[labath]

Summary

The idea is that it would replace
(non-Writable)MemoryBuffer::getNewMemBuffer, which is quite useless
unless you const_cast its contents to write to it (which all (both)
callers of this function were doing). This patch also fixes one of the usages in
COFFWriter. After fixing the other usage in clang, I plan to delete the old
function.

[dblaikie]

Seems good to me

include/llvm/Support/MemoryBuffer.h

public:
   static std::unique_ptr<MemoryBuffer>
   getMemBuffer(MemoryBufferRef Ref, bool RequiresNullTerminator = true);
 
   /// Open the specified memory range as a MemoryBuffer, copying the contents
   /// and taking ownership of it. InputData does not have to be null terminated.
   static std::unique_ptr<MemoryBuffer>
   getMemBufferCopy(StringRef InputData, const Twine &BufferName = "");
 
-  /// Allocate a new zero-initialized MemoryBuffer of the specified size. Note
-  /// that the caller need not initialize the memory allocated by this method.
-  /// The memory is owned by the MemoryBuffer object.
+  // TODO: Remove after all callers are switched to
+  // WritableMemoryBuffer::getNewMemBuffer
   static std::unique_ptr<MemoryBuffer>
   getNewMemBuffer(size_t Size, StringRef BufferName = "");
 
   /// Read all of stdin into a file buffer, and return it.
   static ErrorOr<std::unique_ptr<MemoryBuffer>> getSTDIN();
 
   /// Open the specified file as a MemoryBuffer, or open stdin if the Filename
   /// is "-".
getFileSlice(const Twine &Filename, uint64_t MapSize, uint64_t Offset,
                bool IsVolatile = false);
 
   /// Allocate a new MemoryBuffer of the specified size that is not initialized.
   /// Note that the caller should initialize the memory allocated by this
   /// method. The memory is owned by the MemoryBuffer object.
   static std::unique_ptr<WritableMemoryBuffer>
   getNewUninitMemBuffer(size_t Size, const Twine &BufferName = "");
 
+  /// Allocate a new zero-initialized MemoryBuffer of the specified size. Note
+  /// that the caller need not initialize the memory allocated by this method.
+  /// The memory is owned by the MemoryBuffer object.
+  static std::unique_ptr<WritableMemoryBuffer>
+  getNewMemBuffer(size_t Size, const Twine &BufferName = "");
+
 private:
   // Hide these base class factory function so one can't write
   //   WritableMemoryBuffer::getXXX()
   // and be surprised that he got a read-only Buffer.
   using MemoryBuffer::getFileAsStream;
   using MemoryBuffer::getFileOrSTDIN;
   using MemoryBuffer::getMemBuffer;
   using MemoryBuffer::getMemBufferCopy;
-  using MemoryBuffer::getNewMemBuffer;
   using MemoryBuffer::getOpenFile;
   using MemoryBuffer::getOpenFileSlice;
   using MemoryBuffer::getSTDIN;
 };
 
 class MemoryBufferRef {
   StringRef Buffer;
   StringRef Identifier;

lib/Object/WindowsResource.cpp

private:
   void writeSecondSectionHeader();
   void writeFirstSection();
   void writeSecondSection();
   void writeSymbolTable();
   void writeStringTable();
   void writeDirectoryTree();
   void writeDirectoryStringTable();
   void writeFirstSectionRelocations();
-  std::unique_ptr<MemoryBuffer> OutputBuffer;
+  std::unique_ptr<WritableMemoryBuffer> OutputBuffer;
   char *BufferStart;
   uint64_t CurrentOffset = 0;
   COFF::MachineTypes MachineType;
   const WindowsResourceParser::TreeNode &Resources;
   const ArrayRef<std::vector<uint8_t>> Data;
   uint64_t FileSize;
   uint32_t SymbolTableOffset;
   uint32_t SectionOneSize;

 
 WindowsResourceCOFFWriter::WindowsResourceCOFFWriter(
     COFF::MachineTypes MachineType, const WindowsResourceParser &Parser,
     Error &E)
     : MachineType(MachineType), Resources(Parser.getTree()),
       Data(Parser.getData()), StringTable(Parser.getStringTable()) {
   performFileLayout();
 
-  OutputBuffer = MemoryBuffer::getNewMemBuffer(FileSize);
+  OutputBuffer = WritableMemoryBuffer::getNewMemBuffer(FileSize);
 }
 
 void WindowsResourceCOFFWriter::performFileLayout() {
   // Add size of COFF header.
   FileSize = COFF::Header16Size;
 
   // one .rsrc section header for directory tree, another for resource data.
   FileSize += 2 * COFF::SectionSize;

 static std::time_t getTime() {
   std::time_t Now = time(nullptr);
   if (Now < 0 || !isUInt<32>(Now))
     return UINT32_MAX;
   return Now;
 }
 
 std::unique_ptr<MemoryBuffer> WindowsResourceCOFFWriter::write() {
-  BufferStart = const_cast<char *>(OutputBuffer->getBufferStart());
+  BufferStart = OutputBuffer->getBufferStart();
 
   writeCOFFHeader();
   writeFirstSectionHeader();
   writeSecondSectionHeader();
   writeFirstSection();
   writeSecondSection();
   writeSymbolTable();
   writeStringTable();

lib/Support/MemoryBuffer.cpp

MemoryBuffer::getMemBufferCopy(StringRef InputData, const Twine &BufferName) {
   auto Buf = getMemBufferCopyImpl(InputData, BufferName);
   if (Buf)
     return std::move(*Buf);
   return nullptr;
 }
 
 std::unique_ptr<MemoryBuffer>
 MemoryBuffer::getNewMemBuffer(size_t Size, StringRef BufferName) {
-  auto SB = WritableMemoryBuffer::getNewUninitMemBuffer(Size, BufferName);
-  if (!SB)
-    return nullptr;
-  memset(SB->getBufferStart(), 0, Size);
-  return std::move(SB);
+  return WritableMemoryBuffer::getNewMemBuffer(Size, BufferName);
 }
 
 ErrorOr<std::unique_ptr<MemoryBuffer>>
 MemoryBuffer::getFileOrSTDIN(const Twine &Filename, int64_t FileSize,
                              bool RequiresNullTerminator) {
   SmallString<256> NameBuf;
   StringRef NameRef = Filename.toStringRef(NameBuf);
 
WritableMemoryBuffer::getNewUninitMemBuffer(size_t Size, const Twine &BufferName) {
   // The buffer begins after the name and must be aligned.
   char *Buf = Mem + AlignedStringLen;
   Buf[Size] = 0; // Null terminate buffer.
 
   auto *Ret = new (Mem) MemBuffer(StringRef(Buf, Size), true);
   return std::unique_ptr<WritableMemoryBuffer>(Ret);
 }
 
+std::unique_ptr<WritableMemoryBuffer>
+WritableMemoryBuffer::getNewMemBuffer(size_t Size, const Twine &BufferName) {
+  auto SB = WritableMemoryBuffer::getNewUninitMemBuffer(Size, BufferName);
+  if (!SB)
+    return nullptr;
+  memset(SB->getBufferStart(), 0, Size);
+  return SB;
+}
+
 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

unittests/Support/MemoryBufferTest.cpp

TEST_F(MemoryBufferTest, make_new) {
   OwningBuffer One(WritableMemoryBuffer::getNewUninitMemBuffer(321));
   EXPECT_TRUE(nullptr != One.get());
 
   // uninitialized buffer with name
   OwningBuffer Two(WritableMemoryBuffer::getNewUninitMemBuffer(123, "bla"));
   EXPECT_TRUE(nullptr != Two.get());
 
   // 0-initialized buffer with no name
-  OwningBuffer Three(MemoryBuffer::getNewMemBuffer(321, data));
+  OwningBuffer Three(WritableMemoryBuffer::getNewMemBuffer(321, data));
   EXPECT_TRUE(nullptr != Three.get());
   for (size_t i = 0; i < 321; ++i)
     EXPECT_EQ(0, Three->getBufferStart()[0]);
 
   // 0-initialized buffer with name
-  OwningBuffer Four(MemoryBuffer::getNewMemBuffer(123, "zeros"));
+  OwningBuffer Four(WritableMemoryBuffer::getNewMemBuffer(123, "zeros"));
   EXPECT_TRUE(nullptr != Four.get());
   for (size_t i = 0; i < 123; ++i)
     EXPECT_EQ(0, Four->getBufferStart()[0]);
 }
 
 void MemoryBufferTest::testGetOpenFileSlice(bool Reopen) {
   // Test that MemoryBuffer::getOpenFile works properly when no null
   // terminator is requested and the size is large enough to trigger