Merge huonw#36

36: primal-sieve: Avoid UB use of `pointer::offset` r=cuviper a=cuviper

We must not use `pointer::offset` when the result may be out of bounds.

- In the `safe_assert!` checks, use `wrapping_offset`. We're comparing
  to both start and end pointers, so it can handle overflowing the end or even
  wrapping around less than start.
- For other updates, use `wrapping_offset` and assign `end` if it did wrap,
  so further code only has to check it against the end pointer.

Fixes huonw#35.

I've also included a few minor updates and a release bump:

- Use panic formatting in a test to avoid a deprecation
- Release primal-sieve 0.3.2
- Migrate from criterion deprecations


Co-authored-by: Josh Stone <cuviper@gmail.com>

Author: bors[bot]

Cargo.toml

@@ -26,7 +26,7 @@ primal-sieve = { path = "primal-sieve", version = "0.3" }
 
 [dev-dependencies]
 primal-slowsieve = { path = "primal-slowsieve", version = "0.3" }
-criterion = "0.3"
+criterion = { version = "0.3.4", features = ["html_reports"] }
 
 [[bench]]
 name = "bench"

benches/bench.rs

@@ -1,39 +1,36 @@
 #[macro_use]
 extern crate criterion;
-use criterion::{Criterion, Fun};
+use criterion::Criterion;
 
 const N: usize = 1_000_000;
 const STEP: usize = 101;
 
 fn is_prime(c: &mut Criterion) {
-    let miller_rabin = Fun::new(
-        "is_prime",
-        |b, _| {
-            b.iter(|| (1..N).step_by(STEP).filter(|&n| primal::is_prime(n as u64)).count())
-        });
-    let sieve = Fun::new(
-        "Sieve::is_prime",
-        |b, _| {
+    let mut group = c.benchmark_group("is_prime");
+
+    group.bench_function("is_prime", |b| {
+        b.iter(|| {
+            (1..N)
+                .step_by(STEP)
+                .filter(|&n| primal::is_prime(n as u64))
+                .count()
+        })
+    });
+
+    group.bench_function("Sieve::is_prime", |b| {
+        let sieve = primal::Sieve::new(N);
+        b.iter(|| (1..N).step_by(STEP).filter(|&n| sieve.is_prime(n)).count())
+    });
+
+    group.bench_function("Sieve::is_prime with init", |b| {
+        b.iter(|| {
             let sieve = primal::Sieve::new(N);
-            b.iter(|| {
-                (1..N).step_by(STEP)
-                    .filter(|&n| sieve.is_prime(n)).count()
-            })
-        });
-    let sieve_with_init = Fun::new(
-        "Sieve::is_prime with init",
-        |b, _| {
-            b.iter(|| {
-                let sieve = primal::Sieve::new(N);
-                (1..N).step_by(STEP)
-                    .filter(|&n| sieve.is_prime(n)).count()
-            })
-        });
+            (1..N).step_by(STEP).filter(|&n| sieve.is_prime(n)).count()
+        })
+    });
 
-    c.bench_functions(
-        "is_prime", vec![miller_rabin, sieve, sieve_with_init], ());
+    group.finish();
 }
 
 criterion_group!(benches, is_prime);
 criterion_main!(benches);
-

generators/src/bin/wheel-generator.rs

@@ -263,17 +263,21 @@ pub unsafe fn hardcoded_sieve(bytes: &mut [u8], si_: &mut usize, wi_: &mut usize
 
         for info in cur_infos {
             println!("\
-{indent}    safe_assert!(start <= p.offset(prime_ * {sl} + {off}) &&
-{indent}                 p.offset(prime_ * {sl} + {off}) < end);
+{indent}    safe_assert!(start <= p.wrapping_offset(prime_ * {sl} + {off}) &&
+{indent}                 p.wrapping_offset(prime_ * {sl} + {off}) < end);
 {indent}    *p.offset(prime_ * {sl} + {off}) &= {bit};",
                      // p starts at offset 1 * prime_, so strip off
                      // that factor.
                      sl = info.total_mult_factor - 1,
                      off = info.total_add_factor / wheel,
                      bit = info.unset_bit, indent = indent);
         }
+        // We can't use `p.offset(_)` here because this might go past `end`, which would be UB.
+        // That's fine with `wrapping_offset` as long as it's not dereferenced, but we also need to
+        // guard against the (unlikely) possibility of wrapping the entire address space.
         println!("
-{indent}    p = p.offset(prime_ * {} + {})
+{indent}    let p2 = p.wrapping_offset(prime_ * {} + {});
+{indent}    p = if p <= p2 {{ p2 }} else {{ end }};
 {indent}}}",
                  wheel, c,
                  indent = indent);
@@ -288,7 +292,9 @@ pub unsafe fn hardcoded_sieve(bytes: &mut [u8], si_: &mut usize, wi_: &mut usize
             println!("\
 {indent} if p >= end {{ wi = {val}; break 'outer; }}
 {indent} safe_assert!(start <= p && p < end);
-{indent} *p &= {}; p = p.offset(prime_ * {} + {});
+{indent} *p &= {};
+{indent} let p2 = p.wrapping_offset(prime_ * {} + {});
+{indent} p = if p <= p2 {{ p2 }} else {{ end }};
 {indent} {}
 {indent}}}",
 

primal-sieve/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "primal-sieve"
-version = "0.3.1"
+version = "0.3.2"
 authors = ["Huon Wilson <dbau.pp@gmail.com>"]
 edition = "2018"
 
@@ -22,7 +22,7 @@ smallvec = "1"
 [dev-dependencies]
 primal-slowsieve = { path = "../primal-slowsieve", version = "0.3" }
 primal = { path = "..", version = "0.3" }
-criterion = "0.3"
+criterion = { version = "0.3.4", features = ["html_reports"] }
 
 [[bench]]
 name = "bench"

primal-sieve/benches/bench.rs

@@ -1,26 +1,30 @@
 #[macro_use]
 extern crate criterion;
 
-use primal_sieve::{StreamingSieve, Sieve, Primes};
-use criterion::{Criterion, ParameterizedBenchmark};
+use criterion::{BenchmarkId, Criterion};
+use primal_sieve::{Primes, Sieve, StreamingSieve};
 
 const SIZES: [usize; 5] = [100, 10_000, 100_000, 1_000_000, 10_000_000];
 
 macro_rules! create_benchmarks {
     ($(
         fn $group_id: ident($input: expr) {
-            $first_name: expr => $first_func: expr,
-            $($rest_name: expr => $rest_func: expr,)*
+            $($name: expr => $func: expr,)*
         }
     )*) => {
         $(
             fn $group_id(c: &mut Criterion) {
                 let input = $input;
+                let mut group = c.benchmark_group(stringify!($group_id));
 
-                let bench = ParameterizedBenchmark::new(
-                    $first_name, $first_func, input.iter().copied())
-                    $( .with_function($rest_name, $rest_func) )*;
-                c.bench(stringify!($group_id), bench);
+                $(
+                    for i in &input {
+                        let id = BenchmarkId::new($name, i);
+                        group.bench_with_input(id, i, $func);
+                    }
+                )*
+
+                group.finish();
             }
         )*
     }

primal-sieve/src/streaming/primes.rs

@@ -220,7 +220,7 @@ mod tests {
         // will take too long, but we can cut it short by unwinding.
         Primes::all().fold((), |(), p| {
             if p > 123456789 {
-                panic!(format!("{}", p));
+                panic!("{}", p);
             }
         })
     }