You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@arrow.apache.org by ap...@apache.org on 2018/11/07 12:15:19 UTC
[arrow] branch master updated: ARROW-3477: [C++] fixes for 32 bit
architectures
This is an automated email from the ASF dual-hosted git repository.
apitrou pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/master by this push:
new 713d82a ARROW-3477: [C++] fixes for 32 bit architectures
713d82a is described below
commit 713d82a5148b7bc99c4ae245a33b15aee0d3e32e
Author: Dmitry Kalinkin <dm...@gmail.com>
AuthorDate: Wed Nov 7 13:15:11 2018 +0100
ARROW-3477: [C++] fixes for 32 bit architectures
Author: Dmitry Kalinkin <dm...@gmail.com>
Closes #2901 from veprbl/pr/ARROW-3477 and squashes the following commits:
1d22b345 <Dmitry Kalinkin> fix segfault in array-test on 32 bits
9b0131d4 <Dmitry Kalinkin> fix buffer-test and memory_pool-test on 32 bits
4d38c3e0 <Dmitry Kalinkin> detect some of the overflows in memory_pool.cc
fe7a482f <Dmitry Kalinkin> fix arrow-cpp and parquet-cpp on 32 bits
---
cpp/src/arrow/buffer-test.cc | 6 +++++-
cpp/src/arrow/builder.cc | 4 ++--
cpp/src/arrow/memory_pool-test.h | 6 +++++-
cpp/src/arrow/memory_pool.cc | 18 ++++++++++++++++--
cpp/src/arrow/util/bit-util.h | 2 +-
5 files changed, 29 insertions(+), 7 deletions(-)
diff --git a/cpp/src/arrow/buffer-test.cc b/cpp/src/arrow/buffer-test.cc
index 13ea8a3..4d16f7f 100644
--- a/cpp/src/arrow/buffer-test.cc
+++ b/cpp/src/arrow/buffer-test.cc
@@ -15,6 +15,7 @@
// specific language governing permissions and limitations
// under the License.
+#include <algorithm>
#include <cstdint>
#include <cstring>
#include <limits>
@@ -272,7 +273,10 @@ TYPED_TEST(TypedTestBuffer, ResizeOOM) {
TypeParam buf;
ASSERT_OK(AllocateResizableBuffer(0, &buf));
ASSERT_OK(buf->Resize(100));
- int64_t to_alloc = std::numeric_limits<int64_t>::max();
+ int64_t to_alloc = std::min<uint64_t>(std::numeric_limits<int64_t>::max(),
+ std::numeric_limits<size_t>::max());
+ // subtract 63 to prevent overflow after the size is aligned
+ to_alloc -= 63;
ASSERT_RAISES(OutOfMemory, buf->Resize(to_alloc));
#endif
}
diff --git a/cpp/src/arrow/builder.cc b/cpp/src/arrow/builder.cc
index 8b1b8b9..8c5cab7 100644
--- a/cpp/src/arrow/builder.cc
+++ b/cpp/src/arrow/builder.cc
@@ -736,7 +736,7 @@ Status BooleanBuilder::AppendValues(const std::vector<bool>& values,
int64_t i = 0;
internal::GenerateBitsUnrolled(raw_data_, length_, length,
- [values, &i]() -> bool { return values[i++]; });
+ [&values, &i]() -> bool { return values[i++]; });
// this updates length_
ArrayBuilder::UnsafeAppendToBitmap(is_valid);
@@ -749,7 +749,7 @@ Status BooleanBuilder::AppendValues(const std::vector<bool>& values) {
int64_t i = 0;
internal::GenerateBitsUnrolled(raw_data_, length_, length,
- [values, &i]() -> bool { return values[i++]; });
+ [&values, &i]() -> bool { return values[i++]; });
// this updates length_
ArrayBuilder::UnsafeSetNotNull(length);
diff --git a/cpp/src/arrow/memory_pool-test.h b/cpp/src/arrow/memory_pool-test.h
index 27ec718..34523a1 100644
--- a/cpp/src/arrow/memory_pool-test.h
+++ b/cpp/src/arrow/memory_pool-test.h
@@ -15,6 +15,7 @@
// specific language governing permissions and limitations
// under the License.
+#include <algorithm>
#include <cstdint>
#include <limits>
@@ -53,7 +54,10 @@ class TestMemoryPoolBase : public ::testing::Test {
auto pool = memory_pool();
uint8_t* data;
- int64_t to_alloc = std::numeric_limits<int64_t>::max();
+ int64_t to_alloc = std::min<uint64_t>(std::numeric_limits<int64_t>::max(),
+ std::numeric_limits<size_t>::max());
+ // subtract 63 to prevent overflow after the size is aligned
+ to_alloc -= 63;
ASSERT_RAISES(OutOfMemory, pool->Allocate(to_alloc, &data));
}
diff --git a/cpp/src/arrow/memory_pool.cc b/cpp/src/arrow/memory_pool.cc
index 4947f6a..eaa08a3 100644
--- a/cpp/src/arrow/memory_pool.cc
+++ b/cpp/src/arrow/memory_pool.cc
@@ -23,6 +23,7 @@
#include <cstdlib>
#include <cstring>
#include <iostream>
+#include <limits>
#include <memory>
#include <sstream> // IWYU pragma: keep
@@ -44,7 +45,13 @@ namespace {
// Allocate memory according to the alignment requirements for Arrow
// (as of May 2016 64 bytes)
Status AllocateAligned(int64_t size, uint8_t** out) {
-// TODO(emkornfield) find something compatible with windows
+ // TODO(emkornfield) find something compatible with windows
+ if (size < 0) {
+ return Status::Invalid("negative malloc size");
+ }
+ if (static_cast<uint64_t>(size) >= std::numeric_limits<size_t>::max()) {
+ return Status::CapacityError("malloc size overflows size_t");
+ }
#ifdef _WIN32
// Special code path for Windows
*out =
@@ -104,7 +111,14 @@ class DefaultMemoryPool : public MemoryPool {
Status Reallocate(int64_t old_size, int64_t new_size, uint8_t** ptr) override {
#ifdef ARROW_JEMALLOC
uint8_t* previous_ptr = *ptr;
- *ptr = reinterpret_cast<uint8_t*>(rallocx(*ptr, new_size, MALLOCX_ALIGN(kAlignment)));
+ if (new_size < 0) {
+ return Status::Invalid("negative realloc size");
+ }
+ if (static_cast<uint64_t>(new_size) >= std::numeric_limits<size_t>::max()) {
+ return Status::CapacityError("realloc overflows size_t");
+ }
+ *ptr = reinterpret_cast<uint8_t*>(
+ rallocx(*ptr, static_cast<size_t>(new_size), MALLOCX_ALIGN(kAlignment)));
if (*ptr == NULL) {
std::stringstream ss;
ss << "realloc of size " << new_size << " failed";
diff --git a/cpp/src/arrow/util/bit-util.h b/cpp/src/arrow/util/bit-util.h
index 4bb4411..0119c1f 100644
--- a/cpp/src/arrow/util/bit-util.h
+++ b/cpp/src/arrow/util/bit-util.h
@@ -170,7 +170,7 @@ static inline int CountLeadingZeros(uint32_t value) {
static inline int CountLeadingZeros(uint64_t value) {
#if defined(__clang__) || defined(__GNUC__)
if (value == 0) return 64;
- return static_cast<int>(__builtin_clzl(value));
+ return static_cast<int>(__builtin_clzll(value));
#elif defined(_MSC_VER)
unsigned long index; // NOLINT
if (_BitScanReverse64(&index, value)) { // NOLINT