Avoid undefined behavior when nullptr is passed to memcpy with size 0

absl/strings/str_cat.cc

@@ -89,7 +89,9 @@ static char* Append(char* out, const AlphaNum& x) {
   // memcpy is allowed to overwrite arbitrary memory, so doing this after the
   // memcpy is allowed to overwrite arbitrary memory, so doing this after the
   // call would force an extra fetch of x.size().
   // call would force an extra fetch of x.size().
   char* after = out + x.size();
   char* after = out + x.size();
-  memcpy(out, x.data(), x.size());
+  if (x.size() != 0) {
+    memcpy(out, x.data(), x.size());
+  }
   return after;
   return after;
 }
 }
 
 
@@ -146,8 +148,10 @@ std::string CatPieces(std::initializer_list<absl::string_view> pieces) {
   char* out = begin;
   char* out = begin;
   for (const absl::string_view piece : pieces) {
   for (const absl::string_view piece : pieces) {
     const size_t this_size = piece.size();
     const size_t this_size = piece.size();
-    memcpy(out, piece.data(), this_size);
-    out += this_size;
+    if (this_size != 0) {
+      memcpy(out, piece.data(), this_size);
+      out += this_size;
+    }
   }
   }
   assert(out == begin + result.size());
   assert(out == begin + result.size());
   return result;
   return result;
@@ -176,8 +180,10 @@ void AppendPieces(std::string* dest,
   char* out = begin + old_size;
   char* out = begin + old_size;
   for (const absl::string_view piece : pieces) {
   for (const absl::string_view piece : pieces) {
     const size_t this_size = piece.size();
     const size_t this_size = piece.size();
-    memcpy(out, piece.data(), this_size);
-    out += this_size;
+    if (this_size != 0) {
+      memcpy(out, piece.data(), this_size);
+      out += this_size;
+    }
   }
   }
   assert(out == begin + dest->size());
   assert(out == begin + dest->size());
 }
 }

absl/strings/str_cat_test.cc

@@ -408,6 +408,20 @@ TEST(StrCat, VectorBoolReferenceTypes) {
   EXPECT_EQ(result, "1010");
   EXPECT_EQ(result, "1010");
 }
 }
 
 
+// Passing nullptr to memcpy is undefined behavior and this test
+// provides coverage of codepaths that handle empty strings with nullptrs.
+TEST(StrCat, AvoidsMemcpyWithNullptr) {
+  EXPECT_EQ(absl::StrCat(42, absl::string_view{}), "42");
+
+  // Cover CatPieces code.
+  EXPECT_EQ(absl::StrCat(1, 2, 3, 4, 5, absl::string_view{}), "12345");
+
+  // Cover AppendPieces.
+  std::string result;
+  absl::StrAppend(&result, 1, 2, 3, 4, 5, absl::string_view{});
+  EXPECT_EQ(result, "12345");
+}
+
 #ifdef GTEST_HAS_DEATH_TEST
 #ifdef GTEST_HAS_DEATH_TEST
 TEST(StrAppend, Death) {
 TEST(StrAppend, Death) {
   std::string s = "self";
   std::string s = "self";