Skip to content

Range sampling: signed lengths overflow end - start, and the full-width early return can return end or panic #3

Description

@leonardocustodio

Hey @wolfmcnally ,

Another one here; those are the only two things it reported. Please evaluate with caution, as it is AI-reported, and I'm not a Rust expert.

rng_next_in_range / rng_next_in_closed_range: upper_bound - lower_bound overflows for signed ranges longer than MAX; the full-width early return can return end (half-open) or panic (narrow closed)

Where: src/random_number_generator.rs:118-127 and :146-155; src/magnitude.rs:56-78 (to_magnitude = wrapping_abs() as u*)

Call Default --release overflow-checks = true
i8 -128..=127 ×10 [-128, -127, -128, -127, -127, -128, -127, -128, -128, -127] (a 2-value range) panic attempt to subtract with overflow
i8 -128..127 ×10 all -128 same panic
i16 -32768..=32767 ×6 all -32768 same panic
i8 -100..100 ×5 [-73, -61, -81, -60, -59] (length 200 > 127) same panic
i64 -7..=i64::MAX 552341500324479799 same panic
i64 i64::MIN..0 -8671030536530296001 same panic

Two defects in one code path:

  1. (upper_bound - lower_bound).to_magnitude() subtracts in the signed type.
    For any signed range longer than the width's MAX (-128..=127,
    -100..100, -7..=i64::MAX, i64::MIN..0) that overflows: a panic with
    overflow checks (debug builds), and with Cargo's default release profile a
    silent wrap into a shorter, wrong range. The outcome therefore depends on
    the build profile.
  2. The early return if delta == T::Magnitude::max_value() { return T::from_u64(rng.next_u64()).unwrap(); } returns a raw 64-bit draw. For the
    half-open full-width range (0..u8::MAX, 0..u64::MAX) it can return
    end, which is outside the range. For every width narrower than 64 bits
    (0..=255, 0..=65535, 0..=u32::MAX, closed) from_u64(..).unwrap()
    panics unless the raw draw happens to fit the width, so a closed full-width
    range on u8/u16/u32 panics almost always (from the fixture seed the
    first draw is 1104683000648959614). Executed with constant generators:
    u8 0..255 drawing 255 returns 255; u8 0..=255 drawing 256 panics
    called `Option::unwrap()` on a `None` value.

Proposal:

  • Add fn to_bits(&self) -> Self::Magnitude to HasMagnitude (a bit cast:
    *self as u8 for i8, identity for the unsigned types).
  • delta = upper_bound.to_bits().wrapping_sub(&lower_bound.to_bits()): the
    two's-complement distance, exact for every ordered pair with no overflow.
  • Half-open: always rng_next_with_upper_bound(rng, delta) (no raw early
    return, so end is never returned).
  • Closed with delta == Magnitude::MAX (the whole width): return the raw draw
    truncated to the width, NumCast::from(rng.next_u64() & MAX as u64), instead
    of from_u64(..).unwrap().
  • Otherwise rng_next_with_upper_bound(rng, delta + 1).
  • The result is T::from_magnitude(lower_bound.to_bits().overflowing_add(&random).0).
-    let delta = (upper_bound - lower_bound).to_magnitude();
-
-    if delta == T::Magnitude::max_value() {
-        return T::from_u64(rng.next_u64()).unwrap();
-    }
-
+    let delta = upper_bound.to_bits().wrapping_sub(&lower_bound.to_bits());
     let random = rng_next_with_upper_bound(rng, delta);
-    lower_bound + T::from_magnitude(random)
+    T::from_magnitude(lower_bound.to_bits().overflowing_add(&random).0)

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions