This is an automated email from the ASF dual-hosted git repository. raulcd pushed a commit to branch maint-13.0.0 in repository https://gitbox.apache.org/repos/asf/arrow.git
commit 4d228512a9a3986c60d93475964218306f5ca240 Author: Elliott Brossard <[email protected]> AuthorDate: Thu Jul 27 10:48:26 2023 -0700 GH-36913: [C++] Skip empty buffer concatenation to fix UBSan error (#36914) ### Rationale for this change This is a trivial fix for a UBSan error in calls to `ConcatenateBuffers` with an empty buffer that has a null data pointer. ### What changes are included in this PR? Conditional call to `std::memcpy` based on whether the buffer's length is 0. ### Are these changes tested? Test added in buffer_test.cc. ### Are there any user-facing changes? No * Closes: #36913 Lead-authored-by: Elliott Brossard <[email protected]> Co-authored-by: Elliott Brossard <[email protected]> Co-authored-by: Antoine Pitrou <[email protected]> Signed-off-by: Antoine Pitrou <[email protected]> --- cpp/src/arrow/buffer.cc | 7 +++++-- cpp/src/arrow/buffer_test.cc | 9 +++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/cpp/src/arrow/buffer.cc b/cpp/src/arrow/buffer.cc index afe3d77359..3b0c31309b 100644 --- a/cpp/src/arrow/buffer.cc +++ b/cpp/src/arrow/buffer.cc @@ -209,8 +209,11 @@ Result<std::shared_ptr<Buffer>> ConcatenateBuffers( ARROW_ASSIGN_OR_RAISE(auto out, AllocateBuffer(out_length, pool)); auto out_data = out->mutable_data(); for (const auto& buffer : buffers) { - std::memcpy(out_data, buffer->data(), buffer->size()); - out_data += buffer->size(); + // Passing nullptr to std::memcpy is undefined behavior, so skip empty buffers + if (buffer->size() != 0) { + std::memcpy(out_data, buffer->data(), buffer->size()); + out_data += buffer->size(); + } } return std::move(out); } diff --git a/cpp/src/arrow/buffer_test.cc b/cpp/src/arrow/buffer_test.cc index ce8bab846d..43c28c691f 100644 --- a/cpp/src/arrow/buffer_test.cc +++ b/cpp/src/arrow/buffer_test.cc @@ -1000,4 +1000,13 @@ TYPED_TEST(TypedTestBuffer, ResizeOOM) { #endif } +TEST(TestBufferConcatenation, EmptyBuffer) { + // GH-36913: UB shouldn't be triggered by copying from a null pointer + const std::string contents = "hello, world"; + auto buffer = std::make_shared<Buffer>(contents); + auto empty_buffer = std::make_shared<Buffer>(/*data=*/nullptr, /*size=*/0); + ASSERT_OK_AND_ASSIGN(auto result, ConcatenateBuffers({buffer, empty_buffer})); + AssertMyBufferEqual(*result, contents); +} + } // namespace arrow
