[llvm] 710983a - [Support][MemBuffer] Prevent UB on empty StringRefs

[Kadir Cetinkaya via llvm-commits]

Author: Kadir Cetinkaya
Date: 2023-03-14T12:58:37+01:00
New Revision: 710983ab540fa372e846d47e9fa80b7d8b96afb1

URL: https://github.com/llvm/llvm-project/commit/710983ab540fa372e846d47e9fa80b7d8b96afb1
DIFF: https://github.com/llvm/llvm-project/commit/710983ab540fa372e846d47e9fa80b7d8b96afb1.diff

LOG: [Support][MemBuffer] Prevent UB on empty StringRefs

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.

Differential Revision: https://reviews.llvm.org/D144706

Added: 
    

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

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Support/MemoryBuffer.cpp b/llvm/lib/Support/MemoryBuffer.cpp
index 0bb11725d2fcb..4cc4fe019b75b 100644
--- a/llvm/lib/Support/MemoryBuffer.cpp
+++ b/llvm/lib/Support/MemoryBuffer.cpp
@@ -11,6 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/Config/config.h"
 #include "llvm/Support/Alignment.h"
@@ -22,6 +23,7 @@
 #include "llvm/Support/Process.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/SmallVectorMemoryBuffer.h"
+#include <algorithm>
 #include <cassert>
 #include <cstring>
 #include <new>
@@ -132,10 +134,13 @@ MemoryBuffer::getMemBuffer(MemoryBufferRef Ref, bool RequiresNullTerminator) {
 
 static ErrorOr<std::unique_ptr<WritableMemoryBuffer>>
 getMemBufferCopyImpl(StringRef InputData, const Twine &BufferName) {
-  auto Buf = WritableMemoryBuffer::getNewUninitMemBuffer(InputData.size(), BufferName);
+  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}.
+  llvm::copy(InputData, Buf->getBufferStart());
   return std::move(Buf);
 }
 

diff  --git a/llvm/unittests/Support/MemoryBufferTest.cpp b/llvm/unittests/Support/MemoryBufferTest.cpp
index d3969fb106d2b..bfabd734f02ef 100644
--- a/llvm/unittests/Support/MemoryBufferTest.cpp
+++ b/llvm/unittests/Support/MemoryBufferTest.cpp
@@ -161,6 +161,10 @@ TEST_F(MemoryBufferTest, copy) {
 
   // 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