Merge pull request #328 from Dr-Emann/internal_validate

Add a function for internal validation, and use it in the fuzzer

Author: Kerollmops

.github/workflows/test.yml

@@ -162,5 +162,8 @@ jobs:
             fuzz/artifacts
             fuzz/corpus
 
-      - name: Run Fuzzer vs croaring for 30 minutes
-        run: cargo fuzz run against_croaring -s none -- -timeout=5 -max_total_time=1800
+      - name: Run Fuzzer vs croaring for 15 minutes
+        run: cargo fuzz run against_croaring -s none -- -timeout=5 -max_total_time=900
+
+      - name: Run Fuzzer (with simd) vs croaring for 15 minutes
+        run: cargo fuzz run --features=simd against_croaring -- -timeout=5 -max_total_time=900

fuzz/Cargo.lock

@@ -12,6 +12,9 @@ libfuzzer-sys = { version = "0.4.9", features = ["arbitrary-derive"] }
 roaring = { path = "../roaring" }
 croaring = "2.0"
 
+[features]
+simd = ["roaring/simd"]
+
 [[bin]]
 name = "against_croaring"
 path = "fuzz_targets/against_croaring.rs"

fuzz/Cargo.toml

@@ -33,6 +33,8 @@ fuzz_target!(|input: FuzzInput| {
     for op in input.ops {
         op.apply(&mut lhs_c, &mut rhs_c, &mut lhs_r, &mut rhs_r);
     }
+    lhs_r.internal_validate().unwrap();
+    rhs_r.internal_validate().unwrap();
     check_equal(&lhs_c, &lhs_r);
     check_equal(&rhs_c, &rhs_r);
 });

fuzz/fuzz_targets/against_croaring.rs

@@ -899,6 +899,28 @@ impl RoaringBitmap {
         }
         changed
     }
+
+    /// Ensure the bitmap is internally valid
+    ///
+    /// This is useful for development, but is not needed for normal use:
+    /// bitmaps should _always_ be internally valid.
+    ///
+    /// # Errors
+    ///
+    /// Returns an error if the bitmap is not valid, with a description of the problem.
+    #[doc(hidden)]
+    pub fn internal_validate(&self) -> Result<(), &'static str> {
+        for window in self.containers.windows(2) {
+            let [first, second] = window else { unreachable!() };
+            if second.key <= first.key {
+                return Err("keys are not strictly increasing");
+            }
+        }
+        for container in &self.containers {
+            container.store.internal_validate()?;
+        }
+        Ok(())
+    }
 }
 
 impl Default for RoaringBitmap {

roaring/src/bitmap/inherent.rs

@@ -307,6 +307,23 @@ impl ArrayStore {
         }
         self.vec.truncate(pos);
     }
+
+    pub(crate) fn internal_validate(&self) -> Result<(), &'static str> {
+        if self.vec.is_empty() {
+            return Err("zero cardinality in array container");
+        }
+        if self.vec.len() > super::ARRAY_LIMIT as usize {
+            return Err("array cardinality exceeds ARRAY_LIMIT");
+        }
+        for window in self.vec.windows(2) {
+            let &[first, second] = window else { unreachable!() };
+            if first >= second {
+                return Err("array elements not strictly increasing");
+            }
+        }
+
+        Ok(())
+    }
 }
 
 impl Default for ArrayStore {

roaring/src/bitmap/store/array_store/mod.rs

@@ -426,6 +426,17 @@ impl BitmapStore {
             }
         }
     }
+
+    pub(crate) fn internal_validate(&self) -> Result<(), &'static str> {
+        let expected_len: u64 = self.bits.iter().map(|bits| u64::from(bits.count_ones())).sum();
+        if self.len != expected_len {
+            return Err("bitmap cardinality is incorrect");
+        }
+        if self.len <= super::ARRAY_LIMIT {
+            return Err("cardinality is too small for a bitmap container");
+        }
+        Ok(())
+    }
 }
 
 // this can be done in 3 instructions on x86-64 with bmi2 with: tzcnt(pdep(1 << rank, value))

roaring/src/bitmap/store/bitmap_store.rs

@@ -482,6 +482,28 @@ impl IntervalStore {
     pub(crate) fn iter_intervals(&'_ self) -> core::slice::Iter<'_, Interval> {
         self.0.iter()
     }
+
+    pub(crate) fn internal_validate(&self) -> Result<(), &'static str> {
+        if self.0.is_empty() {
+            return Err("run container with zero runs");
+        }
+        let mut last_end: Option<u16> = None;
+        for run in &self.0 {
+            if run.start > run.end {
+                return Err("empty run container");
+            }
+            if let Some(last_end) = last_end.replace(run.end) {
+                if last_end >= run.start {
+                    return Err("overlapping or unordered runs");
+                }
+                if last_end.saturating_add(1) >= run.start {
+                    return Err("contiguous runs");
+                }
+            }
+        }
+
+        Ok(())
+    }
 }
 
 impl From<IntervalStore> for BitmapStore {

roaring/src/bitmap/store/interval_store.rs

@@ -400,6 +400,14 @@ impl Store {
             Run(intervals) => Run(intervals.clone()),
         }
     }
+
+    pub(crate) fn internal_validate(&self) -> Result<(), &'static str> {
+        match self {
+            Array(vec) => vec.internal_validate(),
+            Bitmap(bits) => bits.internal_validate(),
+            Run(runs) => runs.internal_validate(),
+        }
+    }
 }
 
 impl Default for Store {