[PATCH] D144706: [Support][MemBuffer] Prevent UB on empty StringRefs

[Kadir Cetinkaya via Phabricator via llvm-commits]

kadircet created this revision.
Herald added a subscriber: hiraditya.
Herald added a project: All.
kadircet requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Empty StringRefs are usually identified by their length being zero, and
sometimes they'll have Data==nullptr (e.g. default constructed, or derived from
an operation like split/copy and result turned out to be empty).

If such StringRef objects are passed to llvm::MemoryBuffer::getMemBufferCopy,
it'll result in UB as neither src nor dst can be null, even if size is zero.

This patch prevents that UB by not issuing a copy whenever StringRef is empty.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D144706

Files:
  llvm/lib/Support/MemoryBuffer.cpp
  llvm/unittests/Support/MemoryBufferTest.cpp


Index: llvm/unittests/Support/MemoryBufferTest.cpp
===================================================================
--- llvm/unittests/Support/MemoryBufferTest.cpp
+++ llvm/unittests/Support/MemoryBufferTest.cpp
@@ -161,6 +161,10 @@
 
   // verify the two copies do not point to the same place
   EXPECT_NE(MBC1->getBufferStart(), MBC2->getBufferStart());
+
+  // check that copies from defaulted stringrefs don't trigger UB.
+  OwningBuffer MBC3(MemoryBuffer::getMemBufferCopy(StringRef{}));
+  EXPECT_NE(nullptr, MBC3.get());
 }
 
 #if LLVM_ENABLE_THREADS
Index: llvm/lib/Support/MemoryBuffer.cpp
===================================================================
--- llvm/lib/Support/MemoryBuffer.cpp
+++ llvm/lib/Support/MemoryBuffer.cpp
@@ -135,7 +135,11 @@
   auto Buf = WritableMemoryBuffer::getNewUninitMemBuffer(InputData.size(), BufferName);
   if (!Buf)
     return make_error_code(errc::not_enough_memory);
-  memcpy(Buf->getBufferStart(), InputData.data(), InputData.size());
+  // Calling memcpy with null src/dst is UB, and an empty StringRef is
+  // represented with {nullptr, 0}. Make sure we don't copy anything in that
+  // case.
+  if (!InputData.empty())
+    memcpy(Buf->getBufferStart(), InputData.data(), InputData.size());
   return std::move(Buf);
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D144706.500100.patch
Type: text/x-patch
Size: 1285 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230224/61d2192f/attachment.bin>