Repository navigation
GH-54570: [C++][Compute] Fix segfault casting null list with empty child to fixed_size_list - #54574
ShivanshhhG wants to merge 1 commit into
Conversation
…pty 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: apache#54570 Signed-off-by: SHIVANSH-ux-ys <singaser78@gmail.com>
|
|
|
|
| auto child = ArrayFromJSON(int64(), "[]"); | ||
| ASSERT_OK_AND_ASSIGN( | ||
| auto from, | ||
| ListArray::FromArrays(*ArrayFromJSON(int32(), "[0, 0]"), *child, |
There was a problem hiding this comment.
The C++ build has not run yet however compiling this locally reports an error because the third argument to FromArrays expects MemoryPool*, but receives an Array.
Did you compile/run this before submitting?
| } 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) { |
There was a problem hiding this comment.
The list-view inputs mentioned in the description still fail before reaching this branch. ValidateLengths reads offsets[length], but list views have only length offsets and a separate sizes buffer.
Please add regression tests for all-null ListView and LargeListView inputs with empty children. Either handle these layouts here or narrow the claimed scope and track the remaining bug separately.
Rationale for this change
When casting a
ListArray,LargeListArray, orListViewArraytoFixedSizeListArray, if the input array contains only null lists with an empty child array (which frequently occurs in IPC streams or deserialized batches from tools like Polarsto_arrow()),cast_values->lengthis 0.Previously,
BuildTakeIndicespopulated index0for every null slot (builder.UnsafeAppend(0)). WhenTakewas called withTakeOptions::NoBoundsCheck(), it attempted to access index 0 on an empty child array whose buffers may be null or unallocated, dereferencing a null pointer inFixedWidthTakeImpland causing a segmentation fault (GH-54570).What changes are included in this PR?
cast_values->length == 0, bypassTakeentirely and directly create the output child array usingMakeArrayOfNull(child_type, in_array.length * list_size, ctx->memory_pool()).BuildTakeIndices, emit null indices (builder.UnsafeAppendNull()) for null list slots instead of index0.cpp/src/arrow/compute/kernels/scalar_cast_test.ccverifying that casting null lists with empty children (including unallocated child buffers) toFixedSizeListsucceeds and produces the expected null list array.Are these changes tested?
Yes, unit tests added to
cpp/src/arrow/compute/kernels/scalar_cast_test.ccunderTEST(Cast, ListToFSL).Are there any user-facing changes?
No (fixes crash/segfault on valid spec-compliant input).