From 53dbddb07b167639f8a6f3d5fdd596516fea6b4d Mon Sep 17 00:00:00 2001 From: osobh Date: Fri, 25 Sep 2026 21:03:50 -0500 Subject: [PATCH] fix(format): saturate out-of-range integer reads instead of truncating Reading wider or differently-signed integers kept the low bits: i64 2^40+5 read as i32 was 5, u64::MAX read as i64 was -1, and -1 read as u64 was 4294967295. u32 data read as i32 also took the bulk-copy fast path meant for i32. Saturate at the target range like libhdf5's hard conversions (a negative value read as unsigned is 0), and keep the i32 fast path to signed data. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-format/src/data_read.rs | 38 +++++++----- .../tests/numeric_conversion_interop.rs | 61 +++++++++++++++++++ 2 files changed, 85 insertions(+), 14 deletions(-) diff --git a/crates/clawhdf5-format/src/data_read.rs b/crates/clawhdf5-format/src/data_read.rs index c998fad..01060a6 100644 --- a/crates/clawhdf5-format/src/data_read.rs +++ b/crates/clawhdf5-format/src/data_read.rs @@ -991,21 +991,24 @@ enum Scalar { } impl Scalar { - /// Float to integer conversions follow libhdf5's hard conversions: - /// truncate toward zero, and saturate a value outside the target range to - /// its minimum or maximum. NaN converts to 0 (libhdf5 leaves that case to - /// the C cast, whose result is platform-dependent). + // Every conversion follows libhdf5's default (hard) conversions: a value + // outside the target type's range saturates to its minimum or maximum — + // including a negative value read as unsigned, which reads as 0 — rather + // than being truncated to its low bits. Floats truncate toward zero; NaN + // converts to 0 (libhdf5 leaves that case to the C cast, whose result is + // platform-dependent). + fn to_i64(self) -> i64 { match self { Scalar::Signed(v) => v, - Scalar::Unsigned(v) => v as i64, + Scalar::Unsigned(v) => i64::try_from(v).unwrap_or(i64::MAX), Scalar::Float(v) => v as i64, } } fn to_u64(self) -> u64 { match self { - Scalar::Signed(v) => v as u64, + Scalar::Signed(v) => u64::try_from(v).unwrap_or(0), Scalar::Unsigned(v) => v, Scalar::Float(v) => v as u64, } @@ -1013,8 +1016,8 @@ impl Scalar { fn to_i32(self) -> i32 { match self { - Scalar::Signed(v) => v as i32, - Scalar::Unsigned(v) => v as i32, + Scalar::Signed(v) => v.clamp(i32::MIN.into(), i32::MAX.into()) as i32, + Scalar::Unsigned(v) => i32::try_from(v).unwrap_or(i32::MAX), Scalar::Float(v) => v as i32, } } @@ -1048,8 +1051,10 @@ fn decode_scalar( /// Convert raw bytes to `i64` values. /// -/// Floating-point data is converted the way libhdf5 converts it: truncated -/// toward zero, saturating at the target type's range, with NaN read as 0. +/// Values are converted the way libhdf5 converts them: integers outside the +/// target range saturate at its minimum or maximum (a negative value read as +/// unsigned is 0), and floating-point data is truncated toward zero and +/// saturated, with NaN read as 0. pub fn read_as_i64(raw: &[u8], datatype: &Datatype) -> Result, FormatError> { if let Datatype::Array { base_type, .. } = datatype { return read_as_i64(raw, base_type); @@ -1091,8 +1096,10 @@ pub fn read_as_i64(raw: &[u8], datatype: &Datatype) -> Result, FormatEr /// Convert raw bytes to `u64` values. /// -/// Floating-point data is converted the way libhdf5 converts it: truncated -/// toward zero, saturating at the target type's range, with NaN read as 0. +/// Values are converted the way libhdf5 converts them: integers outside the +/// target range saturate at its minimum or maximum (a negative value read as +/// unsigned is 0), and floating-point data is truncated toward zero and +/// saturated, with NaN read as 0. pub fn read_as_u64(raw: &[u8], datatype: &Datatype) -> Result, FormatError> { if let Datatype::Array { base_type, .. } = datatype { return read_as_u64(raw, base_type); @@ -1207,8 +1214,10 @@ pub fn read_as_f32(raw: &[u8], datatype: &Datatype) -> Result, FormatEr /// Convert raw bytes to `i32` values. /// -/// Floating-point data is converted the way libhdf5 converts it: truncated -/// toward zero, saturating at the target type's range, with NaN read as 0. +/// Values are converted the way libhdf5 converts them: integers outside the +/// target range saturate at its minimum or maximum (a negative value read as +/// unsigned is 0), and floating-point data is truncated toward zero and +/// saturated, with NaN read as 0. pub fn read_as_i32(raw: &[u8], datatype: &Datatype) -> Result, FormatError> { if let Datatype::Array { base_type, .. } = datatype { return read_as_i32(raw, base_type); @@ -1231,6 +1240,7 @@ pub fn read_as_i32(raw: &[u8], datatype: &Datatype) -> Result, FormatEr datatype, Datatype::FixedPoint { byte_order: DatatypeByteOrder::LittleEndian, + signed: true, .. } ) diff --git a/crates/clawhdf5/tests/numeric_conversion_interop.rs b/crates/clawhdf5/tests/numeric_conversion_interop.rs index 5c0fc3c..e75700a 100644 --- a/crates/clawhdf5/tests/numeric_conversion_interop.rs +++ b/crates/clawhdf5/tests/numeric_conversion_interop.rs @@ -127,3 +127,64 @@ with h5py.File("{path}", "r") as f: let f2 = file.dataset("f2").unwrap(); assert_eq!(f2.read_u64().unwrap(), vec![1, 0, 0, 0, 7, 65504, 0]); } + +#[test] +fn integer_reads_saturate_out_of_range_values_like_libhdf5() { + // Narrowing reads used to keep the low bits (i64 2^40+5 read as i32 was + // 5, u64::MAX read as i64 was -1) and signed-to-unsigned reads wrapped + // (-1 read as u64 was 4294967295). + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("int_narrowing.h5"); + let script = format!( + r#"{PRELUDE} +data = {{ + "i8": np.array([2**40 + 5, -(2**35), 7, -1, 2**63 - 1, -(2**63)], "i8"), + "u8": np.array([2**64 - 1, 2**63, 5, 0], "i2"), + "u1": np.array([255, 0, 128], "u1"), +}} +with h5py.File("{path}", "w") as f: + for name, arr in data.items(): + f.create_dataset(name, data=arr) +with h5py.File("{path}", "r") as f: + for name in data: + d = f[name] + emit(name + ":i32", d.astype("(&expected[&format!("{name}:i32")]), + "{name} as i32" + ); + assert_eq!( + ds.read_i64().unwrap(), + parse::(&expected[&format!("{name}:i64")]), + "{name} as i64" + ); + // libhdf5 wraps a negative big-endian i64 read as little-endian u64 + // (it only byte-swaps when the sizes match and the order differs); + // every other signed-to-unsigned read saturates at 0, so do that. + if name != "i8be" { + assert_eq!( + ds.read_u64().unwrap(), + parse::(&expected[&format!("{name}:u64")]), + "{name} as u64" + ); + } + } + let i8be = file.dataset("i8be").unwrap(); + assert_eq!(i8be.read_u64().unwrap(), vec![(1 << 40) + 5, 0, 7, 0]); +}