From 5b3d32b37d06c6929369c71296c18414da216b3b Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 14:09:37 -0500 Subject: [PATCH] format: test the address overflow path on 64-bit hosts addr::to_usize's error branch only ran where usize is narrower than u64, and no such target runs tests in CI, so on x86_64 the test checked only that every u64 fits. to_usize and saturating_usize are now the usize instances of generic to_index/saturating_index; the test runs the same code with u32 standing in for a 32-bit usize: values past u32::MAX (including one an `as` cast would wrap to 0x1234) are Overflow, and the saturating form clamps. A mutant that truncates instead fails the test; the old addr.rs does not provide the helper the test needs. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-format/src/addr.rs | 55 +++++++++++++++++++++++------- 1 file changed, 43 insertions(+), 12 deletions(-) diff --git a/crates/clawhdf5-format/src/addr.rs b/crates/clawhdf5-format/src/addr.rs index 09c2cef..3198f1f 100644 --- a/crates/clawhdf5-format/src/addr.rs +++ b/crates/clawhdf5-format/src/addr.rs @@ -20,7 +20,16 @@ use crate::error::FormatError; /// platform's `usize` (only possible on targets narrower than 64 bits). #[inline] pub fn to_usize(value: u64) -> Result { - usize::try_from(value).map_err(|_| too_large(value)) + to_index::(value) +} + +/// [`to_usize`] for an index type of any width. `usize` is 64 bits wide on +/// the hosts CI tests on, where the error path cannot be reached through +/// `usize`; tests run the same code with `u32` in its place, as on a 32-bit +/// target. +#[inline] +fn to_index>(value: u64) -> Result { + T::try_from(value).map_err(|_| too_large(value)) } /// A count or offset into an in-memory buffer (a codec's progress counter, @@ -33,7 +42,14 @@ pub fn to_usize(value: u64) -> Result { /// read from the file uses [`to_usize`]. #[inline] pub fn saturating_usize(value: u64) -> usize { - usize::try_from(value).unwrap_or(usize::MAX) + saturating_index(value, usize::MAX) +} + +/// [`saturating_usize`] for an index type of any width, whose largest +/// value is `max` (see [`to_index`]). +#[inline] +fn saturating_index>(value: u64, max: T) -> T { + T::try_from(value).unwrap_or(max) } #[cold] @@ -66,16 +82,31 @@ mod tests { #[test] fn values_past_usize_max_are_an_error_not_truncated() { - // Only reachable where usize is narrower than u64; on a 64-bit host - // every u64 fits, which the first branch checks instead. - if let Some(past) = (usize::MAX as u64).checked_add(1) { - let err = to_usize(past).unwrap_err(); - assert!(matches!(err, FormatError::Overflow(_)), "{err:?}"); - // The value an `as usize` cast would have produced is not returned. - assert!(to_usize(u64::MAX).is_err()); - assert!(to_usize(past + 0x10).is_err()); - } else { - assert_eq!(to_usize(u64::MAX), Ok(u64::MAX as usize)); + // Reachable through `usize` only where it is narrower than u64 (no + // such target runs tests in CI), so the same conversion is run with + // u32 standing in for a 32-bit usize. + let max = u64::from(u32::MAX); + assert_eq!(to_index::(max), Ok(u32::MAX)); + for past in [max + 1, max + 0x10, 0x1_0000_1234, u64::MAX] { + let err = to_index::(past).unwrap_err(); + assert!( + matches!(err, FormatError::Overflow(_)), + "{past:#x}: {err:?}" + ); + } + // Where an `as` cast would have wrapped to a small, valid-looking + // index, it is not returned. + assert_eq!(0x1_0000_1234_u64 as u32, 0x1234); + assert!(to_index::(0x1_0000_1234).is_err()); + + assert_eq!(saturating_index(max + 1, u32::MAX), u32::MAX); + assert_eq!(saturating_index(0x1_0000_1234, u32::MAX), u32::MAX); + assert_eq!(saturating_index(0x1234, u32::MAX), 0x1234); + + // And through `usize` itself, whichever width it has here. + match (usize::MAX as u64).checked_add(1) { + Some(past) => assert!(matches!(to_usize(past), Err(FormatError::Overflow(_)))), + None => assert_eq!(to_usize(u64::MAX), Ok(usize::MAX)), } } }