Skip to content

Commit

Permalink
Fix crash in ArrayVectorBase::copyRangesImpl in case target offsets o…
Browse files Browse the repository at this point in the history
…r sizes is nullptr (facebookincubator#9725)

Summary:

`offset_` or `sizes_` can be `nullptr` if it is multi-referenced and cleared during `prepareForReuse`, in that case the old code results in crash.  Reallocate them using `mutableOffsets` or `mutableSizes` in `copyRangesImpl`.

Differential Revision: D57012135
  • Loading branch information
Yuhta authored and facebook-github-bot committed May 6, 2024
1 parent 5bf1e27 commit c874f5d
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 2 deletions.
5 changes: 3 additions & 2 deletions velox/vector/ComplexVector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -541,8 +541,9 @@ void ArrayVectorBase::copyRangesImpl(

auto sourceArray = leafSource->asUnchecked<ArrayVectorBase>();
auto setNotNulls = mayHaveNulls() || source->mayHaveNulls();
auto* mutableOffsets = offsets_->asMutable<vector_size_t>();
auto* mutableSizes = sizes_->asMutable<vector_size_t>();
auto* mutableOffsets =
this->mutableOffsets(length_)->asMutable<vector_size_t>();
auto* mutableSizes = this->mutableSizes(length_)->asMutable<vector_size_t>();
vector_size_t childSize = targetValues->get()->size();
if (ranges.size() == 1 && ranges.back().count == 1) {
auto& range = ranges.back();
Expand Down
12 changes: 12 additions & 0 deletions velox/vector/tests/VectorTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3773,5 +3773,17 @@ TEST_F(VectorTest, mapUpdateMultipleUpdates) {
}
}

TEST_F(VectorTest, arrayCopyTargetNullOffsets) {
auto target = BaseVector::create(ARRAY(BIGINT()), 11, pool());
auto offsetsRef = target->asUnchecked<ArrayVector>()->offsets();
ASSERT_TRUE(offsetsRef);
BaseVector::prepareForReuse(target, target->size());
ASSERT_FALSE(target->asUnchecked<ArrayVector>()->offsets());
auto source = makeArrayVector<int64_t>(
11, [](auto) { return 1; }, [](auto i, auto) { return i; });
target->copy(source.get(), 0, 0, source->size());
test::assertEqualVectors(source, target);
}

} // namespace
} // namespace facebook::velox

0 comments on commit c874f5d

Please sign in to comment.