Skip to content

Commit

Permalink
Fix issues discovered by fuzzer (#707)
Browse files Browse the repository at this point in the history
  • Loading branch information
robert3005 authored Sep 3, 2024
1 parent cdfb190 commit 9b8a2b4
Show file tree
Hide file tree
Showing 20 changed files with 128 additions and 119 deletions.
1 change: 1 addition & 0 deletions encodings/alp/src/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ struct ALPAccessor<F: ALPFloat> {
validity: Validity,
exponents: Exponents,
}

impl<F: ALPFloat> ALPAccessor<F> {
fn new(
encoded: AccessorRef<F::ALPInt>,
Expand Down
6 changes: 3 additions & 3 deletions encodings/alp/src/compute.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use vortex::compute::unary::{scalar_at_unchecked, ScalarAtFn};
use vortex::compute::{slice, take, ArrayCompute, SliceFn, TakeFn};
use vortex::{Array, IntoArray};
use vortex::{Array, ArrayDType, IntoArray};
use vortex_error::VortexResult;
use vortex_scalar::Scalar;

Expand Down Expand Up @@ -40,10 +40,10 @@ impl ScalarAtFn for ALPArray {

match_each_alp_float_ptype!(self.ptype(), |$T| {
let encoded_val: <$T as ALPFloat>::ALPInt = encoded_val.as_ref().try_into().unwrap();
Scalar::from(<$T as ALPFloat>::decode_single(
Scalar::primitive(<$T as ALPFloat>::decode_single(
encoded_val,
self.exponents(),
))
), self.dtype().nullability())
})
}
}
Expand Down
8 changes: 3 additions & 5 deletions encodings/byte-bool/src/compute/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use vortex::validity::{ArrayValidity, Validity};
use vortex::{Array, ArrayDType, IntoArray, IntoArrayVariant};
use vortex_dtype::{match_each_integer_ptype, Nullability};
use vortex_error::{vortex_err, VortexResult};
use vortex_scalar::{Scalar, ScalarValue};
use vortex_scalar::Scalar;

use super::ByteBoolArray;

Expand Down Expand Up @@ -40,10 +40,7 @@ impl ScalarAtFn for ByteBoolArray {
}

fn scalar_at_unchecked(&self, index: usize) -> Scalar {
Scalar::new(
self.dtype().clone(),
ScalarValue::Bool(self.buffer()[index] == 1),
)
Scalar::bool(self.buffer()[index] == 1, self.dtype().nullability())
}
}

Expand Down Expand Up @@ -179,6 +176,7 @@ mod tests {
use vortex::compute::unary::{scalar_at, scalar_at_unchecked};
use vortex::compute::{compare, slice};
use vortex::AsArray as _;
use vortex_scalar::ScalarValue;

use super::*;

Expand Down
2 changes: 1 addition & 1 deletion encodings/fastlanes/src/bitpacking/compute/scalar_at.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ impl ScalarAtFn for BitPackedArray {
fn scalar_at(&self, index: usize) -> VortexResult<Scalar> {
if let Some(patches) = self.patches() {
// NB: All non-null values are considered patches
if self.bit_width() == 0 || patches.with_dyn(|a| a.is_valid(index)) {
if patches.with_dyn(|a| a.is_valid(index)) {
return scalar_at_unchecked(&patches, index).cast(self.dtype());
}
}
Expand Down
13 changes: 1 addition & 12 deletions encodings/fastlanes/src/bitpacking/compute/take.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,13 @@ use std::cmp::min;

use fastlanes::BitPacking;
use itertools::Itertools;
use vortex::array::{ConstantArray, PrimitiveArray, SparseArray};
use vortex::array::{PrimitiveArray, SparseArray};
use vortex::compute::{slice, take, TakeFn};
use vortex::{Array, ArrayDType, IntoArray, IntoArrayVariant};
use vortex_dtype::{
match_each_integer_ptype, match_each_unsigned_integer_ptype, NativePType, PType,
};
use vortex_error::VortexResult;
use vortex_scalar::Scalar;

use crate::{unpack_single_primitive, BitPackedArray};

Expand All @@ -18,16 +17,6 @@ impl TakeFn for BitPackedArray {
let ptype: PType = self.dtype().try_into()?;
let validity = self.validity();
let taken_validity = validity.take(indices)?;
if self.bit_width() == 0 {
return if let Some(patches) = self.patches() {
take(&patches, indices)
} else {
Ok(
ConstantArray::new(Scalar::null(self.dtype().as_nullable()), indices.len())
.into_array(),
)
};
}

let indices = indices.clone().into_primitive()?;
let taken = match_each_unsigned_integer_ptype!(ptype, |$T| {
Expand Down
4 changes: 2 additions & 2 deletions encodings/fastlanes/src/for/compute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use vortex::compute::{
use vortex::{Array, ArrayDType, IntoArray};
use vortex_dtype::match_each_integer_ptype;
use vortex_error::VortexResult;
use vortex_scalar::{PrimitiveScalar, Scalar, ScalarValue};
use vortex_scalar::{PrimitiveScalar, Scalar};

use crate::FoRArray;

Expand Down Expand Up @@ -54,7 +54,7 @@ impl ScalarAtFn for FoRArray {
use num_traits::WrappingAdd;
encoded.typed_value::<$P>().map(|v| (v << self.shift()).wrapping_add(reference.typed_value::<$P>().unwrap()))
.map(|v| Scalar::primitive::<$P>(v, encoded.dtype().nullability()))
.unwrap_or_else(|| Scalar::new(encoded.dtype().clone(), ScalarValue::Null))
.unwrap_or_else(|| Scalar::null(encoded.dtype().clone()))
})
}
}
Expand Down
13 changes: 2 additions & 11 deletions encodings/fsst/src/compute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use vortex::compute::unary::{scalar_at_unchecked, ScalarAtFn};
use vortex::compute::{filter, slice, take, ArrayCompute, FilterFn, SliceFn, TakeFn};
use vortex::{Array, ArrayDType, IntoArray};
use vortex_buffer::Buffer;
use vortex_error::{vortex_bail, VortexResult};
use vortex_error::VortexResult;
use vortex_scalar::Scalar;

use crate::FSSTArray;
Expand Down Expand Up @@ -50,16 +50,7 @@ impl TakeFn for FSSTArray {

impl ScalarAtFn for FSSTArray {
fn scalar_at(&self, index: usize) -> VortexResult<Scalar> {
let compressed = scalar_at_unchecked(&self.codes(), index);
let binary_datum = match compressed.value().as_buffer()? {
Some(b) => b,
None => vortex_bail!("non-nullable scalar must unwrap"),
};

let decompressor = self.decompressor();
let decoded_buffer: Buffer = decompressor.decompress(binary_datum.as_slice()).into();

Ok(varbin_scalar(decoded_buffer, self.dtype()))
Ok(self.scalar_at_unchecked(index))
}

fn scalar_at_unchecked(&self, index: usize) -> Scalar {
Expand Down
12 changes: 6 additions & 6 deletions fuzz/fuzz_targets/fuzz_target_1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::collections::HashSet;

use libfuzzer_sys::{fuzz_target, Corpus};
use vortex::compute::slice;
use vortex::compute::unary::scalar_at_unchecked;
use vortex::compute::unary::scalar_at;
use vortex::encoding::EncodingId;
use vortex::Array;
use vortex_fuzz::{Action, FuzzArrayAction};
Expand All @@ -26,7 +26,7 @@ fuzz_target!(|fuzz_action: FuzzArrayAction| -> Corpus {
assert_array_eq(&array, &compressed_array);
Corpus::Keep
}
None => return Corpus::Reject,
None => Corpus::Reject,
},
Action::Slice(range) => {
let slice = slice(&array, range.start, range.end).unwrap();
Expand All @@ -48,8 +48,8 @@ fn fuzz_compress(array: &Array, compressor_ref: CompressorRef<'_>) -> Option<Arr

fn assert_slice(original: &Array, slice: &Array, start: usize) {
for idx in 0..slice.len() {
let o = scalar_at_unchecked(original, start + idx);
let s = scalar_at_unchecked(slice, idx);
let o = scalar_at(original, start + idx).unwrap();
let s = scalar_at(slice, idx).unwrap();

fuzzing_scalar_cmp(o, s, original.encoding().id(), slice.encoding().id(), idx);
}
Expand All @@ -58,8 +58,8 @@ fn assert_slice(original: &Array, slice: &Array, start: usize) {
fn assert_array_eq(lhs: &Array, rhs: &Array) {
assert_eq!(lhs.len(), rhs.len());
for idx in 0..lhs.len() {
let l = scalar_at_unchecked(lhs, idx);
let r = scalar_at_unchecked(rhs, idx);
let l = scalar_at(lhs, idx).unwrap();
let r = scalar_at(rhs, idx).unwrap();

fuzzing_scalar_cmp(l, r, lhs.encoding().id(), rhs.encoding().id(), idx);
}
Expand Down
10 changes: 6 additions & 4 deletions fuzz/src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
use std::fmt::Debug;
use std::ops::Range;

use libfuzzer_sys::arbitrary::{Arbitrary, Result, Unstructured};
use vortex::Array;
use vortex_sampling_compressor::compressors::alp::ALPCompressor;
use vortex_sampling_compressor::compressors::bitpacked::BitPackedCompressor;
use vortex_sampling_compressor::compressors::date_time_parts::DateTimePartsCompressor;
use vortex_sampling_compressor::compressors::dict::DictCompressor;
use vortex_sampling_compressor::compressors::r#for::FoRCompressor;
use vortex_sampling_compressor::compressors::roaring_bool::RoaringBoolCompressor;
Expand All @@ -18,7 +20,7 @@ pub struct FuzzArrayAction {
pub actions: Vec<Action>,
}

impl std::fmt::Debug for FuzzArrayAction {
impl Debug for FuzzArrayAction {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("FuzzArrayAction")
.field("action", &self.actions)
Expand All @@ -27,13 +29,12 @@ impl std::fmt::Debug for FuzzArrayAction {
}
}

#[derive()]
pub enum Action {
Compress(&'static dyn EncodingCompressor),
Slice(Range<usize>),
}

impl std::fmt::Debug for Action {
impl Debug for Action {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::Slice(arg0) => f.debug_tuple("Slice").field(arg0).finish(),
Expand All @@ -45,7 +46,7 @@ impl std::fmt::Debug for Action {
impl<'a> Arbitrary<'a> for FuzzArrayAction {
fn arbitrary(u: &mut Unstructured<'a>) -> Result<Self> {
let array = Array::arbitrary(u)?;
let action = match u.int_in_range(0..=9)? {
let action = match u.int_in_range(0..=10)? {
0 => {
let start = u.choose_index(array.len())?;
let stop = u.choose_index(array.len() - start)? + start;
Expand All @@ -60,6 +61,7 @@ impl<'a> Arbitrary<'a> for FuzzArrayAction {
7 => Action::Compress(&DEFAULT_RUN_END_COMPRESSOR),
8 => Action::Compress(&SparseCompressor),
9 => Action::Compress(&ZigZagCompressor),
10 => Action::Compress(&DateTimePartsCompressor),
_ => unreachable!(),
};

Expand Down
8 changes: 4 additions & 4 deletions vortex-array/src/array/arbitrary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ impl<'a> Arbitrary<'a> for Array {
}

fn random_array(u: &mut Unstructured) -> Result<Array> {
match u.int_in_range(0..=12).unwrap() {
match u.int_in_range(0..=12)? {
0 => random_primitive::<u8>(u),
1 => random_primitive::<u16>(u),
2 => random_primitive::<u32>(u),
Expand All @@ -33,7 +33,7 @@ fn random_array(u: &mut Unstructured) -> Result<Array> {

fn random_string(u: &mut Unstructured) -> Result<Array> {
let v = Vec::<Option<String>>::arbitrary(u)?;
let arr = match u.int_in_range(0..=1).unwrap() {
let arr = match u.int_in_range(0..=1)? {
0 => VarBinArray::from_iter(v, DType::Utf8(Nullability::Nullable)).into_array(),
1 => VarBinViewArray::from_iter_nullable_str(v).into_array(),
_ => unreachable!(),
Expand All @@ -44,8 +44,8 @@ fn random_string(u: &mut Unstructured) -> Result<Array> {

fn random_bytes(u: &mut Unstructured) -> Result<Array> {
let v = Vec::<Option<Vec<u8>>>::arbitrary(u)?;
let arr = match u.int_in_range(0..=1).unwrap() {
0 => VarBinArray::from_iter(v, DType::Utf8(Nullability::Nullable)).into_array(),
let arr = match u.int_in_range(0..=1)? {
0 => VarBinArray::from_iter(v, DType::Binary(Nullability::Nullable)).into_array(),
1 => VarBinViewArray::from_iter_nullable_bin(v).into_array(),
_ => unreachable!(),
};
Expand Down
8 changes: 7 additions & 1 deletion vortex-array/src/array/bool/compute/scalar_at.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use vortex_scalar::Scalar;

use crate::array::BoolArray;
use crate::compute::unary::ScalarAtFn;
use crate::ArrayDType;

impl ScalarAtFn for BoolArray {
fn scalar_at(&self, index: usize) -> VortexResult<Scalar> {
Expand All @@ -12,6 +13,11 @@ impl ScalarAtFn for BoolArray {
fn scalar_at_unchecked(&self, index: usize) -> Scalar {
// SAFETY:
// `scalar_at_unchecked` is fine with undefined behavior, so it should be acceptable here
unsafe { self.boolean_buffer().value_unchecked(index).into() }
unsafe {
Scalar::bool(
self.boolean_buffer().value_unchecked(index),
self.dtype().nullability(),
)
}
}
}
Loading

0 comments on commit 9b8a2b4

Please sign in to comment.