From 59d3c5cd053e10f18fc052dbbe7f7bbc52905c0b Mon Sep 17 00:00:00 2001 From: SHIVANSH-ux-ys Date: Mon, 19 Oct 2026 14:01:24 +0530 Subject: [PATCH] GH-54570: [C++][Compute] Fix segfault casting null list with empty child to fixed_size_list When casting a ListArray or LargeListArray to FixedSizeListArray, if an array contains only null lists with an empty child array, cast_values->length is 0. Previously, BuildTakeIndices populated index 0 for null slots, and Take was called with TakeOptions::NoBoundsCheck(), dereferencing an unallocated child buffer and causing a segmentation fault. This commit resolves the issue by: 1. Handling empty child values (cast_values->length == 0) directly using MakeArrayOfNull rather than attempting Take. 2. Emitting null indices (builder.UnsafeAppendNull()) for null list slots in BuildTakeIndices instead of index 0. 3. Adding regression tests for casting null lists with empty children and unallocated buffers to FixedSizeList. Fixes: #54570 Signed-off-by: SHIVANSH-ux-ys --- .../compute/kernels/scalar_cast_nested.cc | 91 +++++++---- .../arrow/compute/kernels/scalar_cast_test.cc | 143 ++++++++++++++++++ 2 files changed, 204 insertions(+), 30 deletions(-) diff --git a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc index 392fd9fbb705..ac6b6cfcc619 100644 --- a/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc +++ b/cpp/src/arrow/compute/kernels/scalar_cast_nested.cc @@ -191,38 +191,62 @@ struct CastVarToFixedList { // Additionally, checks if the null slots are all the list size. Returns true // if so, which can be used to optimize the cast. static Result ValidateLengths(const ArraySpan& in_array, int32_t list_size) { - const auto* offsets = in_array.GetValues(1); - src_offset_type expected_offset = offsets[0] + list_size; - // If there are nulls, it's possible that the null slots are the correct - // size. That enables an optimization later in this function. - bool nulls_have_correct_length = true; - if (in_array.GetNullCount() > 0) { - for (int64_t i = 1; i <= in_array.length; ++i) { - if (in_array.IsNull(i - 1)) { - if (offsets[i] != expected_offset) { - nulls_have_correct_length = false; + if constexpr (is_list_view_type::value) { + const auto* offsets = in_array.GetValues(1); + const auto* sizes = in_array.GetValues(2); + bool can_use_slice = true; + src_offset_type expected_offset = in_array.length > 0 ? offsets[0] : 0; + for (int64_t i = 0; i < in_array.length; ++i) { + if (in_array.IsNull(i)) { + if (sizes[i] != list_size || offsets[i] != expected_offset) { + can_use_slice = false; } - // If element is null, it can be any size, so the next offset is valid. - expected_offset = offsets[i] + list_size; } else { - if (offsets[i] != expected_offset) { + if (sizes[i] != list_size) { return Status::Invalid("ListType can only be casted to FixedSizeListType ", "if the lists are all the expected size."); } - expected_offset += list_size; + if (offsets[i] != expected_offset) { + can_use_slice = false; + } } + expected_offset += list_size; } + return can_use_slice; } else { - // Don't need to check null slots if there are no nulls - for (int64_t i = 1; i <= in_array.length; ++i) { - if (offsets[i] != expected_offset) { - return Status::Invalid("ListType can only be casted to FixedSizeListType ", - "if the lists are all the expected size."); + const auto* offsets = in_array.GetValues(1); + src_offset_type expected_offset = in_array.length > 0 ? offsets[0] + list_size : 0; + // If there are nulls, it's possible that the null slots are the correct + // size. That enables an optimization later in this function. + bool can_use_slice = true; + if (in_array.GetNullCount() > 0) { + for (int64_t i = 1; i <= in_array.length; ++i) { + if (in_array.IsNull(i - 1)) { + if (offsets[i] != expected_offset) { + can_use_slice = false; + } + // If element is null, it can be any size, so the next offset is valid. + expected_offset = offsets[i] + list_size; + } else { + if (offsets[i] != expected_offset) { + return Status::Invalid("ListType can only be casted to FixedSizeListType ", + "if the lists are all the expected size."); + } + expected_offset += list_size; + } + } + } else { + // Don't need to check null slots if there are no nulls + for (int64_t i = 1; i <= in_array.length; ++i) { + if (offsets[i] != expected_offset) { + return Status::Invalid("ListType can only be casted to FixedSizeListType ", + "if the lists are all the expected size."); + } + expected_offset += list_size; } - expected_offset += list_size; } + return can_use_slice; } - return nulls_have_correct_length; } // Build take indices for the values. This is used to fill in the null slots @@ -236,9 +260,9 @@ struct CastVarToFixedList { RETURN_NOT_OK(builder.Reserve(in_array.length * list_size)); for (int64_t offset_i = 0; offset_i < in_array.length; ++offset_i) { if (in_array.IsNull(offset_i)) { - // If element is null, just fill in the null slots with first value. + // If element is null, fill in the null slots with null values. for (int64_t j = 0; j < list_size; ++j) { - builder.UnsafeAppend(0); + builder.UnsafeAppendNull(); } } else { int64_t value_i = offsets[offset_i]; @@ -260,7 +284,7 @@ struct CastVarToFixedList { const auto& out_type = checked_cast(*out->type()); const int32_t list_size = out_type.list_size(); - ARROW_ASSIGN_OR_RAISE(bool nulls_have_correct_length, + ARROW_ASSIGN_OR_RAISE(bool can_use_slice, ValidateLengths(in_array, list_size)); ArrayData* out_array = out->array_data().get(); @@ -275,18 +299,25 @@ struct CastVarToFixedList { DCHECK(cast_values_datum.is_array()); std::shared_ptr cast_values = cast_values_datum.array(); - const auto* offsets = in_array.GetValues(1); - if (in_array.GetNullCount() > 0 && !nulls_have_correct_length) { - // We need to fill in the null slots, so we'll use Take on the values. + if (cast_values->length == 0) { + ARROW_ASSIGN_OR_RAISE( + auto null_values, + MakeArrayOfNull(child_type, in_array.length * list_size, ctx->memory_pool())); + cast_values = null_values->data(); + } else if (!can_use_slice) { + // We need to reorder, handle gaps, or fill in null slots: use Take on the values. ARROW_ASSIGN_OR_RAISE(auto indices, BuildTakeIndices(in_array, list_size, ctx)); ARROW_ASSIGN_OR_RAISE(auto take_result, Take(cast_values, Datum(indices), TakeOptions::NoBoundsCheck(), ctx->exec_context())); DCHECK(take_result.is_array()); cast_values = take_result.array(); - } else if (offsets[0] != 0) { - // No nulls, but we need to slice the values - cast_values = cast_values->Slice(offsets[0], in_array.length * list_size); + } else { + const auto* offsets = in_array.GetValues(1); + const src_offset_type first_offset = in_array.length > 0 ? offsets[0] : 0; + if (first_offset != 0 || cast_values->length > in_array.length * list_size) { + cast_values = cast_values->Slice(first_offset, in_array.length * list_size); + } } out_array->child_data.emplace_back(std::move(cast_values)); diff --git a/cpp/src/arrow/compute/kernels/scalar_cast_test.cc b/cpp/src/arrow/compute/kernels/scalar_cast_test.cc index 864ec4afce0c..ca2f82e4be94 100644 --- a/cpp/src/arrow/compute/kernels/scalar_cast_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_cast_test.cc @@ -3787,6 +3787,149 @@ TEST(Cast, ListToFSL) { // Invalid fixed_size_list cast if inconsistent size ASSERT_RAISES(Invalid, Cast(ArrayFromJSON(list(int32()), "[[0, 1, 2], null, [3, 4]]"), CastOptions::Safe(fixed_size_list(int32(), 3)))); + + // GH-54570: Null list array with empty child array cast to FixedSizeList + { + auto child = ArrayFromJSON(int64(), "[]"); + auto null_bitmap = ArrayFromJSON(boolean(), "[false]")->data()->buffers[1]; + ASSERT_OK_AND_ASSIGN( + auto from, + ListArray::FromArrays(*ArrayFromJSON(int32(), "[0, 0]"), *child, + default_memory_pool(), null_bitmap)); + auto to = ArrayFromJSON(fixed_size_list(int64(), 3), "[null]"); + CheckCast(from, to); + } + + // GH-54570: LargeList with empty child cast to FixedSizeList + { + auto child = ArrayFromJSON(int64(), "[]"); + auto null_bitmap = ArrayFromJSON(boolean(), "[false]")->data()->buffers[1]; + ASSERT_OK_AND_ASSIGN( + auto from, + LargeListArray::FromArrays(*ArrayFromJSON(int64(), "[0, 0]"), *child, + default_memory_pool(), null_bitmap)); + auto to = ArrayFromJSON(fixed_size_list(int64(), 3), "[null]"); + CheckCast(from, to); + } + + // GH-54570: All-null ListView with empty child cast to FixedSizeList + { + auto child = ArrayFromJSON(int64(), "[]"); + auto null_bitmap = ArrayFromJSON(boolean(), "[false]")->data()->buffers[1]; + ASSERT_OK_AND_ASSIGN( + auto from, + ListViewArray::FromArrays(*ArrayFromJSON(int32(), "[0]"), + *ArrayFromJSON(int32(), "[0]"), *child, + default_memory_pool(), null_bitmap)); + auto to = ArrayFromJSON(fixed_size_list(int64(), 3), "[null]"); + CheckCast(from, to); + } + + // GH-54570: All-null LargeListView with empty child cast to FixedSizeList + { + auto child = ArrayFromJSON(int64(), "[]"); + auto null_bitmap = ArrayFromJSON(boolean(), "[false]")->data()->buffers[1]; + ASSERT_OK_AND_ASSIGN( + auto from, + LargeListViewArray::FromArrays(*ArrayFromJSON(int64(), "[0]"), + *ArrayFromJSON(int64(), "[0]"), *child, + default_memory_pool(), null_bitmap)); + auto to = ArrayFromJSON(fixed_size_list(int64(), 3), "[null]"); + CheckCast(from, to); + } + + // GH-54570: Multiple null lists with empty child + { + auto child = ArrayFromJSON(int64(), "[]"); + auto null_bitmap = ArrayFromJSON(boolean(), "[false, false]")->data()->buffers[1]; + ASSERT_OK_AND_ASSIGN( + auto from, + ListArray::FromArrays(*ArrayFromJSON(int32(), "[0, 0, 0]"), *child, + default_memory_pool(), null_bitmap)); + auto to = ArrayFromJSON(fixed_size_list(int64(), 2), "[null, null]"); + CheckCast(from, to); + } + + // GH-54570: Null list with unallocated/null child data buffers + { + auto child_data = ArrayData::Make(int64(), 0, {nullptr, nullptr}); + auto list_bitmap = ArrayFromJSON(boolean(), "[false]")->data()->buffers[1]; + auto list_offsets = ArrayFromJSON(int32(), "[0, 0]")->data()->buffers[1]; + auto list_data = ArrayData::Make(list(int64()), 1, {list_bitmap, list_offsets}, + {child_data}, 1); + auto from = MakeArray(list_data); + auto to = ArrayFromJSON(fixed_size_list(int64(), 3), "[null]"); + CheckCast(from, to); + } + + // GH-54570: Null slots with length 0 alongside valid slots + { + auto child = ArrayFromJSON(int32(), "[10, 20]"); + auto null_bitmap = ArrayFromJSON(boolean(), "[false, true]")->data()->buffers[1]; + ASSERT_OK_AND_ASSIGN( + auto from, + ListArray::FromArrays(*ArrayFromJSON(int32(), "[0, 0, 2]"), *child, + default_memory_pool(), null_bitmap)); + auto to = ArrayFromJSON(fixed_size_list(int32(), 2), "[null, [10, 20]]"); + CheckCast(from, to); + } + + { + auto child = ArrayFromJSON(int32(), "[10, 20]"); + auto null_bitmap = ArrayFromJSON(boolean(), "[true, false]")->data()->buffers[1]; + ASSERT_OK_AND_ASSIGN( + auto from, + ListArray::FromArrays(*ArrayFromJSON(int32(), "[0, 2, 2]"), *child, + default_memory_pool(), null_bitmap)); + auto to = ArrayFromJSON(fixed_size_list(int32(), 2), "[[10, 20], null]"); + CheckCast(from, to); + } + + // GH-54570: Non-contiguous ListView with gaps and no nulls + { + auto child = ArrayFromJSON(int32(), "[1, 2, 3, 4, 5]"); + ASSERT_OK_AND_ASSIGN( + auto from, + ListViewArray::FromArrays(*ArrayFromJSON(int32(), "[0, 3]"), + *ArrayFromJSON(int32(), "[2, 2]"), *child)); + auto to = ArrayFromJSON(fixed_size_list(int32(), 2), "[[1, 2], [4, 5]]"); + CheckCast(from, to); + } + + // GH-54570: Reordered ListView with no nulls + { + auto child = ArrayFromJSON(int32(), "[1, 2, 3, 4, 5]"); + ASSERT_OK_AND_ASSIGN( + auto from, + ListViewArray::FromArrays(*ArrayFromJSON(int32(), "[2, 0]"), + *ArrayFromJSON(int32(), "[2, 2]"), *child)); + auto to = ArrayFromJSON(fixed_size_list(int32(), 2), "[[3, 4], [1, 2]]"); + CheckCast(from, to); + } + + // GH-54570: LargeListView with gaps and reordering with no nulls + { + auto child = ArrayFromJSON(int64(), "[10, 20, 30, 40, 50]"); + ASSERT_OK_AND_ASSIGN( + auto from, + LargeListViewArray::FromArrays(*ArrayFromJSON(int64(), "[3, 0]"), + *ArrayFromJSON(int64(), "[2, 2]"), *child)); + auto to = ArrayFromJSON(fixed_size_list(int64(), 2), "[[40, 50], [10, 20]]"); + CheckCast(from, to); + } + + // GH-54570: ListView with gaps and nulls + { + auto child = ArrayFromJSON(int32(), "[1, 2, 99, 99, 5, 6]"); + auto null_bitmap = ArrayFromJSON(boolean(), "[true, false, true]")->data()->buffers[1]; + ASSERT_OK_AND_ASSIGN( + auto from, + ListViewArray::FromArrays(*ArrayFromJSON(int32(), "[0, 2, 4]"), + *ArrayFromJSON(int32(), "[2, 0, 2]"), *child, + default_memory_pool(), null_bitmap)); + auto to = ArrayFromJSON(fixed_size_list(int32(), 2), "[[1, 2], null, [5, 6]]"); + CheckCast(from, to); + } } TEST(Cast, CastMap) {