This is an automated email from the ASF dual-hosted git repository.
apitrou pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new bedc1e7cbb GH-36913: [C++] Skip empty buffer concatenation to fix
UBSan error (#36914)
bedc1e7cbb is described below
commit bedc1e7cbb5d07469dd976350763ab4e93ed64ad
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 135aa0c532..99dc29cfe5 100644
--- a/cpp/src/arrow/buffer.cc
+++ b/cpp/src/arrow/buffer.cc
@@ -213,8 +213,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 3dd95cb8af..13f6ea63b5 100644
--- a/cpp/src/arrow/buffer_test.cc
+++ b/cpp/src/arrow/buffer_test.cc
@@ -1014,4 +1014,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
