From 1efe0d1292334739a1a6873ea35d7637e81498ee 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 | 101 ++++++++++++------ .../arrow/compute/kernels/scalar_cast_test.cc | 97 +++++++++++++++++ 2 files changed, 163 insertions(+), 35 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..1bf0191bc12a 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 = 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 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 nulls_have_correct_length; } - 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]; @@ -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. - 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); + 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 { + 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. + 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 || cast_values->length > in_array.length * list_size) { + // No nulls or nulls have correct length, but we need to slice the values + cast_values = cast_values->Slice(offsets[0], 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..3c09ef8048a5 100644 --- a/cpp/src/arrow/compute/kernels/scalar_cast_test.cc +++ b/cpp/src/arrow/compute/kernels/scalar_cast_test.cc @@ -3787,6 +3787,103 @@ 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); + } } TEST(Cast, CastMap) {