This is an automated email from the ASF dual-hosted git repository.
paleolimbot pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow-nanoarrow.git
The following commit(s) were added to refs/heads/main by this push:
new f29659b fix: Ensure that test for increasing offsets is not affected
by overflow (#291)
f29659b is described below
commit f29659b7d832f4ec7915557c9d06c52b280019cd
Author: Dewey Dunnington <[email protected]>
AuthorDate: Mon Aug 28 14:36:28 2023 -0300
fix: Ensure that test for increasing offsets is not affected by overflow
(#291)
Closes #290.
This PR fixes the check for increasing offsets: previously it calculated
a diff between pairs of elements and ensured that the value was > 0.
This failed for expressions like `int32_t diff = (int32_t)-2147483648 -
(int32_t)2080374784`, where because of overflow, `diff` evalutes to `>
0`.
---
src/nanoarrow/array.c | 12 ++++--------
src/nanoarrow/array_test.cc | 31 +++++++++++++++++++++----------
2 files changed, 25 insertions(+), 18 deletions(-)
diff --git a/src/nanoarrow/array.c b/src/nanoarrow/array.c
index 0f40583..1e59777 100644
--- a/src/nanoarrow/array.c
+++ b/src/nanoarrow/array.c
@@ -1041,10 +1041,8 @@ static int ArrowAssertIncreasingInt32(struct
ArrowBufferView view,
}
for (int64_t i = 1; i < view.size_bytes / (int64_t)sizeof(int32_t); i++) {
- int32_t diff = view.data.as_int32[i] - view.data.as_int32[i - 1];
- if (diff < 0) {
- ArrowErrorSet(error, "[%ld] Expected element size >= 0 but found element
size %ld",
- (long)i, (long)diff);
+ if (view.data.as_int32[i] < view.data.as_int32[i - 1]) {
+ ArrowErrorSet(error, "[%ld] Expected element size >= 0", (long)i);
return EINVAL;
}
}
@@ -1059,10 +1057,8 @@ static int ArrowAssertIncreasingInt64(struct
ArrowBufferView view,
}
for (int64_t i = 1; i < view.size_bytes / (int64_t)sizeof(int64_t); i++) {
- int64_t diff = view.data.as_int64[i] - view.data.as_int64[i - 1];
- if (diff < 0) {
- ArrowErrorSet(error, "[%ld] Expected element size >= 0 but found element
size %ld",
- (long)i, (long)diff);
+ if (view.data.as_int64[i] < view.data.as_int64[i - 1]) {
+ ArrowErrorSet(error, "[%ld] Expected element size >= 0", (long)i);
return EINVAL;
}
}
diff --git a/src/nanoarrow/array_test.cc b/src/nanoarrow/array_test.cc
index d6702f2..ac99ecf 100644
--- a/src/nanoarrow/array_test.cc
+++ b/src/nanoarrow/array_test.cc
@@ -265,7 +265,7 @@ TEST(ArrayTest, ArrayTestExplicitValidationLevel) {
NANOARROW_OK);
EXPECT_EQ(ArrowArrayFinishBuilding(&array, NANOARROW_VALIDATION_LEVEL_FULL,
&error),
EINVAL);
- EXPECT_STREQ(error.message, "[1] Expected element size >= 0 but found
element size -1");
+ EXPECT_STREQ(error.message, "[1] Expected element size >= 0");
offsets[1] = 4;
// Valid at validation_level < NANOARROW_VALIDATION_LEVEL_DEFAULT
@@ -1630,20 +1630,23 @@ TEST(ArrayTest, ArrayViewTestString) {
EXPECT_EQ(array_view.buffer_views[1].size_bytes, 0);
EXPECT_EQ(array_view.buffer_views[2].size_bytes, 0);
- // Build non-zero length (the array ["abcd"])
+ // Build non-zero length (the array ["abcd", "efg"])
ASSERT_EQ(ArrowBufferAppendInt32(ArrowArrayBuffer(&array, 1), 0),
NANOARROW_OK);
ASSERT_EQ(ArrowBufferAppendInt32(ArrowArrayBuffer(&array, 1), 4),
NANOARROW_OK);
- ASSERT_EQ(ArrowBufferReserve(ArrowArrayBuffer(&array, 2), 4), NANOARROW_OK);
+ ASSERT_EQ(ArrowBufferAppendInt32(ArrowArrayBuffer(&array, 1), 7),
NANOARROW_OK);
+
+ ASSERT_EQ(ArrowBufferReserve(ArrowArrayBuffer(&array, 2), 7), NANOARROW_OK);
ArrowBufferAppendUnsafe(ArrowArrayBuffer(&array, 2), "abcd", 4);
- array.length = 1;
+ ArrowBufferAppendUnsafe(ArrowArrayBuffer(&array, 2), "efg", 3);
+ array.length = 2;
ASSERT_EQ(ArrowArrayFinishBuildingDefault(&array, nullptr), NANOARROW_OK);
EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), NANOARROW_OK);
EXPECT_EQ(ArrowArrayViewValidate(&array_view,
NANOARROW_VALIDATION_LEVEL_FULL, &error),
NANOARROW_OK);
EXPECT_EQ(array_view.buffer_views[0].size_bytes, 0);
- EXPECT_EQ(array_view.buffer_views[1].size_bytes, (1 + 1) * sizeof(int32_t));
- EXPECT_EQ(array_view.buffer_views[2].size_bytes, 4);
+ EXPECT_EQ(array_view.buffer_views[1].size_bytes, (1 + array.length) *
sizeof(int32_t));
+ EXPECT_EQ(array_view.buffer_views[2].size_bytes, 7);
// Expect error for offsets that will cause bad access
int32_t* offsets =
@@ -1658,7 +1661,15 @@ TEST(ArrayTest, ArrayViewTestString) {
EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), NANOARROW_OK);
EXPECT_EQ(ArrowArrayViewValidate(&array_view,
NANOARROW_VALIDATION_LEVEL_FULL, &error),
EINVAL);
- EXPECT_STREQ(error.message, "[1] Expected element size >= 0 but found
element size -1");
+ EXPECT_STREQ(error.message, "[1] Expected element size >= 0");
+
+ // Check sequential offsets whose diff causes overflow
+ offsets[1] = 2080374784;
+ offsets[2] = -2147483648;
+ EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), NANOARROW_OK);
+ EXPECT_EQ(ArrowArrayViewValidate(&array_view,
NANOARROW_VALIDATION_LEVEL_FULL, &error),
+ EINVAL);
+ EXPECT_STREQ(error.message, "[2] Expected element size >= 0");
array.release(&array);
ArrowArrayViewReset(&array_view);
@@ -1729,7 +1740,7 @@ TEST(ArrayTest, ArrayViewTestLargeString) {
EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), NANOARROW_OK);
EXPECT_EQ(ArrowArrayViewValidate(&array_view,
NANOARROW_VALIDATION_LEVEL_FULL, &error),
EINVAL);
- EXPECT_STREQ(error.message, "[1] Expected element size >= 0 but found
element size -1");
+ EXPECT_STREQ(error.message, "[1] Expected element size >= 0");
array.release(&array);
ArrowArrayViewReset(&array_view);
@@ -1815,7 +1826,7 @@ TEST(ArrayTest, ArrayViewTestList) {
EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), NANOARROW_OK);
EXPECT_EQ(ArrowArrayViewValidate(&array_view,
NANOARROW_VALIDATION_LEVEL_FULL, &error),
EINVAL);
- EXPECT_STREQ(error.message, "[1] Expected element size >= 0 but found
element size -1");
+ EXPECT_STREQ(error.message, "[1] Expected element size >= 0");
array.release(&array);
ArrowArrayViewReset(&array_view);
@@ -1936,7 +1947,7 @@ TEST(ArrayTest, ArrayViewTestLargeList) {
EXPECT_EQ(ArrowArrayViewSetArray(&array_view, &array, &error), NANOARROW_OK);
EXPECT_EQ(ArrowArrayViewValidate(&array_view,
NANOARROW_VALIDATION_LEVEL_FULL, &error),
EINVAL);
- EXPECT_STREQ(error.message, "[1] Expected element size >= 0 but found
element size -1");
+ EXPECT_STREQ(error.message, "[1] Expected element size >= 0");
array.release(&array);
ArrowArrayViewReset(&array_view);
