diff options
author | Roman Gershman <romange@gmail.com> | 2019-05-23T03·03+0300 |
---|---|---|
committer | Derek Mauro <761129+derekmauro@users.noreply.github.com> | 2019-05-23T03·03-0400 |
commit | 27c30ec671cb7b5ba84c4e79feff7fd0b0ac6338 (patch) | |
tree | 2a66b5c72434251348d3cac81314e638848043c1 | |
parent | ce65f5ac3cbf897bb5e3de1a51d80fd00866abaa (diff) |
Avoid undefined behavior when nullptr is passed to memcpy with size 0
-rw-r--r-- | absl/strings/str_cat.cc | 16 | ||||
-rw-r--r-- | absl/strings/str_cat_test.cc | 14 |
2 files changed, 25 insertions, 5 deletions
diff --git a/absl/strings/str_cat.cc b/absl/strings/str_cat.cc index 2667976d99a5..ffe99db81713 100644 --- a/absl/strings/str_cat.cc +++ b/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 // call would force an extra fetch of 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; } @@ -146,8 +148,10 @@ std::string CatPieces(std::initializer_list<absl::string_view> pieces) { char* out = begin; for (const absl::string_view piece : pieces) { 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()); return result; @@ -176,8 +180,10 @@ void AppendPieces(std::string* dest, char* out = begin + old_size; for (const absl::string_view piece : pieces) { 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()); } diff --git a/absl/strings/str_cat_test.cc b/absl/strings/str_cat_test.cc index 1f1051d41f1e..29db9c023781 100644 --- a/absl/strings/str_cat_test.cc +++ b/absl/strings/str_cat_test.cc @@ -408,6 +408,20 @@ TEST(StrCat, VectorBoolReferenceTypes) { 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 TEST(StrAppend, Death) { std::string s = "self"; |