From 6248b411f0530e8762fd2b4278ee918968e2e990 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 12:43:49 -0500 Subject: [PATCH 01/10] format: one checked conversion from file address to index addr::to_usize turns a 64-bit file address or length into a slice index, failing with FormatError::Overflow where it does not fit usize (32-bit targets such as wasm32) instead of truncating like an `as usize` cast. Callers are converted in the following commits. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-format/src/addr.rs | 59 ++++++++++++++++++++++++++++++ crates/clawhdf5-format/src/lib.rs | 1 + 2 files changed, 60 insertions(+) create mode 100644 crates/clawhdf5-format/src/addr.rs diff --git a/crates/clawhdf5-format/src/addr.rs b/crates/clawhdf5-format/src/addr.rs new file mode 100644 index 0000000..ec25e3d --- /dev/null +++ b/crates/clawhdf5-format/src/addr.rs @@ -0,0 +1,59 @@ +//! File address and length → in-memory index conversion. +//! +//! HDF5 addresses and lengths are 64-bit; the file is parsed through a +//! `&[u8]` indexed by `usize`. On a 64-bit target every `u64` fits, but on a +//! 32-bit one (`wasm32`, `i686`, `thumbv7em`) an address past `usize::MAX` +//! used to be truncated by an `as usize` cast — silently pointing at another +//! part of the file — or to panic. [`to_usize`] is the one conversion the +//! parsers use instead: such an address is a clean +//! [`FormatError::Overflow`]. It cannot be inside the data anyway: no slice +//! is longer than `isize::MAX` bytes. + +#[cfg(not(feature = "std"))] +use alloc::format; + +use crate::error::FormatError; + +/// A file address, offset or length from the file as a `usize` index. +/// +/// Fails with [`FormatError::Overflow`] when the value does not fit this +/// 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)) +} + +#[cold] +#[inline(never)] +fn too_large(value: u64) -> FormatError { + FormatError::Overflow(format!( + "file address or length {value:#x} exceeds this platform's address space" + )) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn values_that_fit_convert_exactly() { + assert_eq!(to_usize(0), Ok(0)); + assert_eq!(to_usize(0x1234), Ok(0x1234)); + assert_eq!(to_usize(usize::MAX as u64), Ok(usize::MAX)); + } + + #[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)); + } + } +} diff --git a/crates/clawhdf5-format/src/lib.rs b/crates/clawhdf5-format/src/lib.rs index a197c9f..2737c9b 100644 --- a/crates/clawhdf5-format/src/lib.rs +++ b/crates/clawhdf5-format/src/lib.rs @@ -57,6 +57,7 @@ #[cfg(not(feature = "std"))] extern crate alloc; +pub mod addr; pub mod attribute; pub mod attribute_info; pub mod btree_v1; From 02e89c1d2d6cf65f3eae207cfb23c25a1bd9ae87 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 13:33:19 -0500 Subject: [PATCH 02/10] read: look names up through the dense name indexes Finding one link or attribute by name read every entry: Group::dataset and Group::group (File, MmapFile, LazyFile) listed the whole group per call, and path resolution scanned each group's links. Opening every child of a 35 001-link group by name decoded ~1.2e9 links. Now a dense group's v2 B-tree name index (type 5, lookup3 hash of the name) is descended to the records with the name's hash (btree_v2::find_btree_v2_records reads only the nodes whose key interval overlaps), and only those links are read and compared; all hash-equal records are compared, so libhdf5's tie order does not matter. Dense attributes the same through their type 8 index (attribute::find_attribute_in_file, facade attr(name)); huge heap objects through their ID-ordered index. group_v2::resolve_child returns what the listing has under a name (soft links followed, dangling/external ones not found). Group::entries and File::group_at hand out a listing's addresses. The lookup-stats feature counts heap objects read. Tests: one lookup in an h5py-written 35 001-link group with colliding hashes reads at most two links (before: 35 001, failing), attribute lookups likewise (before: 3 000, failing), every child opens through all three readers and matches h5py, every link kind resolves as h5py resolves it in dense and compact groups, 300 huge attributes are found, and a range search matches a full scan at every tree depth. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-format/Cargo.toml | 3 + crates/clawhdf5-format/src/attribute.rs | 181 ++++++-- crates/clawhdf5-format/src/btree_v2.rs | 216 +++++++-- crates/clawhdf5-format/src/btree_v2_write.rs | 53 +++ crates/clawhdf5-format/src/fractal_heap.rs | 34 +- crates/clawhdf5-format/src/group_v2.rs | 217 +++++++-- crates/clawhdf5-format/src/lib.rs | 1 + crates/clawhdf5-format/src/lookup_stats.rs | 31 ++ crates/clawhdf5/Cargo.toml | 3 +- crates/clawhdf5/src/lazy.rs | 57 ++- crates/clawhdf5/src/mmap_file.rs | 57 ++- crates/clawhdf5/src/reader.rs | 88 +++- crates/clawhdf5/src/types.rs | 29 ++ .../clawhdf5/tests/dense_storage_interop.rs | 24 + .../clawhdf5/tests/indexed_lookup_interop.rs | 410 ++++++++++++++++++ 15 files changed, 1245 insertions(+), 159 deletions(-) create mode 100644 crates/clawhdf5-format/src/lookup_stats.rs create mode 100644 crates/clawhdf5/tests/indexed_lookup_interop.rs diff --git a/crates/clawhdf5-format/Cargo.toml b/crates/clawhdf5-format/Cargo.toml index c1ec6df..e6a1258 100644 --- a/crates/clawhdf5-format/Cargo.toml +++ b/crates/clawhdf5-format/Cargo.toml @@ -80,6 +80,9 @@ blosc = ["lz4_flex", "ruzstd", "snap", "deflate", "std"] blosc2 = ["blosc"] # Every plugin filter above. plugin-filters = ["lzf", "bitshuffle", "bzip2", "blosc", "blosc2"] +# Test instrumentation: per-thread counts of heap objects read (see +# `lookup_stats`), so tests can bound the cost of a name lookup. +lookup-stats = ["std"] [[bench]] name = "parallel_decompress_bench" diff --git a/crates/clawhdf5-format/src/attribute.rs b/crates/clawhdf5-format/src/attribute.rs index 6c4e9ae..d22a552 100644 --- a/crates/clawhdf5-format/src/attribute.rs +++ b/crates/clawhdf5-format/src/attribute.rs @@ -5,8 +5,10 @@ use alloc::{borrow::Cow, string::String, vec::Vec}; #[cfg(feature = "std")] use std::borrow::Cow; +use crate::addr::to_usize; use crate::attribute_info::AttributeInfoMessage; -use crate::btree_v2::{BTreeV2Header, collect_btree_v2_records}; +use crate::btree_v2::{BTreeV2Header, collect_btree_v2_records, find_btree_v2_records}; +use crate::checksum::jenkins_lookup3; use crate::data_read; use crate::dataspace::Dataspace; use crate::datatype::Datatype; @@ -341,7 +343,8 @@ fn compute_raw_data( dataspace: &Dataspace, datatype: &Datatype, ) -> Vec { - let num_elements = dataspace.num_elements() as usize; + // Saturating, like the product: the size is capped at what is there. + let num_elements = usize::try_from(dataspace.num_elements()).unwrap_or(usize::MAX); let elem_size = datatype.type_size() as usize; let expected_size = num_elements.saturating_mul(elem_size); let available = data.len().saturating_sub(pos); @@ -453,7 +456,145 @@ fn extract_attributes_with( // Each attribute's creation order, where the file records one. let mut orders: Vec = Vec::new(); - // Collect compact attributes (inline in OH) + extract_compact_attributes( + file_data, + header, + offset_size, + length_size, + &mut attrs, + &mut orders, + on_error, + )?; + + // Check for dense attributes via AttributeInfo message + let attr_info = find_attribute_info(header, offset_size)?; + if let Some(info) = &attr_info + && let Some(fh_addr) = info.fractal_heap_address + { + extract_dense_attributes( + file_data, + info, + fh_addr, + offset_size, + length_size, + &mut attrs, + &mut orders, + on_error, + )?; + } + + // An object that tracks attribute creation order lists its attributes + // in that order (h5py's `track_order=True`), as libhdf5 does; otherwise + // they come in storage order. + if attr_info.is_some_and(|i| i.max_creation_index.is_some()) { + let mut paired: Vec<(u32, AttributeMessage)> = orders.into_iter().zip(attrs).collect(); + paired.sort_by_key(|(o, _)| *o); + attrs = paired.into_iter().map(|(_, a)| a).collect(); + } + + Ok(attrs) +} + +/// B-tree v2 record type of dense attribute storage's name index. +const ATTRIBUTE_NAME_INDEX: u8 = 8; + +/// The attribute called `name` on the object with header `header`: the +/// first one [`extract_attributes_tolerant`] returns under that name, or +/// `None` if it returns none (an attribute that cannot be read is not +/// returned there either). +/// +/// Compact attributes are in the header and are scanned. Dense attributes +/// are found through the name index (a v2 B-tree of lookup3 name hashes, +/// record type 8): only the attributes whose names hash like `name` are read +/// from the heap, O(log n) instead of all of them. Errors in the structures +/// that index the attributes fail the call, as they fail a listing. +pub fn find_attribute_in_file( + file_data: &[u8], + header: &ObjectHeader, + name: &str, + offset_size: u8, + length_size: u8, +) -> Result, FormatError> { + let attr_info = find_attribute_info(header, offset_size)?; + let dense = attr_info + .as_ref() + .and_then(|i| Some((i.fractal_heap_address?, i.btree_name_index_address?))); + let Some((fh_addr, btree_addr)) = dense else { + // Compact only (or dense storage without a name index, which a + // listing reports): as a listing finds it. + return Ok( + extract_attributes_tolerant(file_data, header, offset_size, length_size)? + .0 + .into_iter() + .find(|a| a.name == name), + ); + }; + let btree_hdr = + BTreeV2Header::parse(file_data, to_usize(btree_addr)?, offset_size, length_size)?; + let fh = FractalHeapHeader::parse(file_data, to_usize(fh_addr)?, offset_size, length_size)?; + if btree_hdr.tree_type != ATTRIBUTE_NAME_INDEX || btree_hdr.record_size < 4 { + return Ok( + extract_attributes_tolerant(file_data, header, offset_size, length_size)? + .0 + .into_iter() + .find(|a| a.name == name), + ); + } + + // A listing has the compact attributes first. + let mut compact = Vec::new(); + extract_compact_attributes( + file_data, + header, + offset_size, + length_size, + &mut compact, + &mut Vec::new(), + &mut |_| Ok(()), + )?; + if let Some(a) = compact.into_iter().find(|a| a.name == name) { + return Ok(Some(a)); + } + + // Record: heap ID + message flags(1) + creation order(4) + hash(4); the + // hash is the last field. + let hash = jenkins_lookup3(name.as_bytes()); + let hash_at = usize::from(btree_hdr.record_size) - 4; + let records = find_btree_v2_records(file_data, &btree_hdr, offset_size, &mut |r| match r + .get(hash_at..hash_at + 4) + { + Some(h) => u32::from_le_bytes([h[0], h[1], h[2], h[3]]).cmp(&hash), + None => core::cmp::Ordering::Less, + })?; + let id_len = usize::from(fh.heap_id_length); + for record in &records { + let Some(id_bytes) = record.data.get(..id_len) else { + continue; + }; + let attr = fh + .read_managed_object(file_data, id_bytes, offset_size) + .and_then(|d| AttributeMessage::parse_in_file(&d, file_data, offset_size, length_size)); + // One that cannot be read is left out, as from a listing. + if let Ok(attr) = attr + && attr.name == name + { + return Ok(Some(attr)); + } + } + Ok(None) +} + +/// The attributes stored in the object header itself (compact storage), and +/// each one's creation order into `orders`. +fn extract_compact_attributes( + file_data: &[u8], + header: &ObjectHeader, + offset_size: u8, + length_size: u8, + attrs: &mut Vec, + orders: &mut Vec, + on_error: &mut dyn FnMut(FormatError) -> Result<(), FormatError>, +) -> Result<(), FormatError> { for msg in &header.messages { if msg.msg_type == MessageType::Attribute { let attr = if shared_message::is_shared(msg.flags) { @@ -489,34 +630,7 @@ fn extract_attributes_with( } } } - - // Check for dense attributes via AttributeInfo message - let attr_info = find_attribute_info(header, offset_size)?; - if let Some(info) = &attr_info - && let Some(fh_addr) = info.fractal_heap_address - { - extract_dense_attributes( - file_data, - info, - fh_addr, - offset_size, - length_size, - &mut attrs, - &mut orders, - on_error, - )?; - } - - // An object that tracks attribute creation order lists its attributes - // in that order (h5py's `track_order=True`), as libhdf5 does; otherwise - // they come in storage order. - if attr_info.is_some_and(|i| i.max_creation_index.is_some()) { - let mut paired: Vec<(u32, AttributeMessage)> = orders.into_iter().zip(attrs).collect(); - paired.sort_by_key(|(o, _)| *o); - attrs = paired.into_iter().map(|(_, a)| a).collect(); - } - - Ok(attrs) + Ok(()) } /// Find and parse the Attribute Info message from an object header. @@ -547,7 +661,7 @@ fn extract_dense_attributes( on_error: &mut dyn FnMut(FormatError) -> Result<(), FormatError>, ) -> Result<(), FormatError> { // Parse fractal heap - let fh = FractalHeapHeader::parse(file_data, fh_addr as usize, offset_size, length_size)?; + let fh = FractalHeapHeader::parse(file_data, to_usize(fh_addr)?, offset_size, length_size)?; // Parse B-tree v2 for name index (type 8) let btree_addr = attr_info @@ -556,7 +670,8 @@ fn extract_dense_attributes( expected: 1, available: 0, })?; - let btree_hdr = BTreeV2Header::parse(file_data, btree_addr as usize, offset_size, length_size)?; + let btree_hdr = + BTreeV2Header::parse(file_data, to_usize(btree_addr)?, offset_size, length_size)?; let records = collect_btree_v2_records(file_data, &btree_hdr, offset_size, length_size)?; for record in &records { diff --git a/crates/clawhdf5-format/src/btree_v2.rs b/crates/clawhdf5-format/src/btree_v2.rs index 9159253..7582c05 100644 --- a/crates/clawhdf5-format/src/btree_v2.rs +++ b/crates/clawhdf5-format/src/btree_v2.rs @@ -2,10 +2,12 @@ #[cfg(not(feature = "std"))] use alloc::vec::Vec; +use core::cmp::Ordering; #[cfg(feature = "checksum")] use byteorder::{ByteOrder, LittleEndian}; +use crate::addr::to_usize; use crate::error::FormatError; /// Parsed B-tree v2 header (signature "BTHD"). @@ -216,7 +218,7 @@ pub fn collect_btree_v2_records( // Root is a leaf parse_leaf_records( file_data, - header.root_node_address as usize, + to_usize(header.root_node_address)?, header.num_records_in_root, header.record_size, ) @@ -225,7 +227,7 @@ pub fn collect_btree_v2_records( let mut records = Vec::new(); collect_internal_records( file_data, - header.root_node_address as usize, + to_usize(header.root_node_address)?, header.num_records_in_root, header.depth, header.record_size, @@ -289,9 +291,10 @@ fn parse_leaf_records( Ok(records) } -/// Recursively collect records from an internal node. -#[allow(clippy::too_many_arguments, clippy::only_used_in_recursion)] -fn collect_internal_records( +/// An internal node's layout: where its records start, and its children as +/// `(address, record count)`. +#[allow(clippy::too_many_arguments)] +fn read_internal_node( file_data: &[u8], offset: usize, num_records: u16, @@ -299,11 +302,8 @@ fn collect_internal_records( record_size: u16, node_size: u32, offset_size: u8, - length_size: u8, max_leaf_nrec: u64, - budget: &mut usize, - out: &mut Vec, -) -> Result<(), FormatError> { +) -> Result<(usize, Vec<(u64, u16)>), FormatError> { // signature(4) + version(1) + type(1) = 6 ensure_len(file_data, offset, 6)?; if &file_data[offset..offset + 4] != b"BTIN" { @@ -314,7 +314,7 @@ fn collect_internal_records( let rs = record_size as usize; let mut pos = offset + 6; - // Read all records first + // Records first let records_total = nr.checked_mul(rs).ok_or(FormatError::UnexpectedEof { expected: usize::MAX, available: file_data.len(), @@ -346,7 +346,6 @@ fn collect_internal_records( let child_ptr_size = offset_size as usize + nrec_width + total_nrec_width; ensure_len(file_data, pos, num_children * child_ptr_size)?; - // Read child pointers let mut children = Vec::with_capacity(num_children); for _ in 0..num_children { let addr = read_offset(file_data, pos, offset_size)?; @@ -356,6 +355,61 @@ fn collect_internal_records( pos += total_nrec_width; // skip total records in subtree children.push((addr, child_nrec)); } + Ok((records_start, children)) +} + +/// Record `i` of an internal node whose records start at `records_start`. +fn internal_record( + file_data: &[u8], + records_start: usize, + i: usize, + rs: usize, +) -> Result<&[u8], FormatError> { + let overflow = || FormatError::UnexpectedEof { + expected: usize::MAX, + available: file_data.len(), + }; + let rec_start = i + .checked_mul(rs) + .and_then(|o| records_start.checked_add(o)) + .ok_or_else(overflow)?; + let rec_end = rec_start.checked_add(rs).ok_or_else(overflow)?; + file_data + .get(rec_start..rec_end) + .ok_or(FormatError::UnexpectedEof { + expected: rec_end, + available: file_data.len(), + }) +} + +/// Recursively collect records from an internal node. +#[allow(clippy::too_many_arguments, clippy::only_used_in_recursion)] +fn collect_internal_records( + file_data: &[u8], + offset: usize, + num_records: u16, + depth: u16, + record_size: u16, + node_size: u32, + offset_size: u8, + length_size: u8, + max_leaf_nrec: u64, + budget: &mut usize, + out: &mut Vec, +) -> Result<(), FormatError> { + let nr = num_records as usize; + let rs = record_size as usize; + let (records_start, children) = read_internal_node( + file_data, + offset, + num_records, + depth, + record_size, + node_size, + offset_size, + max_leaf_nrec, + )?; + let child_depth = depth - 1; // Interleave: child[0], record[0], child[1], record[1], ..., child[nr] // We collect child[0] records, then record[0], then child[1], etc. @@ -364,12 +418,12 @@ fn collect_internal_records( // Before parsing, so a refused tree is not also a large allocation. spend(budget, usize::from(child_nrec))?; let leaf_recs = - parse_leaf_records(file_data, child_addr as usize, child_nrec, record_size)?; + parse_leaf_records(file_data, to_usize(child_addr)?, child_nrec, record_size)?; out.extend(leaf_recs); } else { collect_internal_records( file_data, - child_addr as usize, + to_usize(child_addr)?, child_nrec, child_depth, record_size, @@ -384,32 +438,10 @@ fn collect_internal_records( // Add record[i] (except after the last child) if i < nr { - let rec_offset = i.checked_mul(rs).ok_or(FormatError::UnexpectedEof { - expected: usize::MAX, - available: file_data.len(), - })?; - let rec_start = - records_start - .checked_add(rec_offset) - .ok_or(FormatError::UnexpectedEof { - expected: usize::MAX, - available: file_data.len(), - })?; - let rec_end = rec_start - .checked_add(rs) - .ok_or(FormatError::UnexpectedEof { - expected: usize::MAX, - available: file_data.len(), - })?; - if rec_end > file_data.len() { - return Err(FormatError::UnexpectedEof { - expected: rec_end, - available: file_data.len(), - }); - } + let data = internal_record(file_data, records_start, i, rs)?; spend(budget, 1)?; out.push(BTreeV2Record { - data: file_data[rec_start..rec_end].to_vec(), + data: data.to_vec(), }); } } @@ -417,6 +449,116 @@ fn collect_internal_records( Ok(()) } +/// The records of a B-tree v2 that fall in one key range, found by +/// descending the tree instead of reading all of it. +/// +/// `cmp` places a record relative to the range: `Less` if the record sorts +/// before it, `Greater` if after, `Equal` if the record is in it. The tree +/// must be ordered consistently with `cmp`, as libhdf5 orders it (a link or +/// attribute name index by name hash, so all records with one hash form a +/// range whatever order their names are in). Only the nodes whose key +/// interval overlaps the range are read: O(depth) nodes plus those holding +/// the matches. Matches come in tree order. +pub fn find_btree_v2_records( + file_data: &[u8], + header: &BTreeV2Header, + offset_size: u8, + cmp: &mut dyn FnMut(&[u8]) -> Ordering, +) -> Result, FormatError> { + if header.total_records == 0 || header.num_records_in_root == 0 { + return Ok(Vec::new()); + } + if header.depth > MAX_DEPTH { + return Err(FormatError::NestingDepthExceeded); + } + // As in `collect_btree_v2_records`: a valid tree cannot hold more + // records than the file has room for, however its children are shared. + let mut budget = file_data.len() / usize::from(header.record_size.max(1)); + let max_leaf_nrec = max_records_leaf(header.node_size, header.record_size); + let mut out = Vec::new(); + find_in_node( + file_data, + header, + to_usize(header.root_node_address)?, + header.num_records_in_root, + header.depth, + offset_size, + max_leaf_nrec, + cmp, + &mut budget, + &mut out, + )?; + Ok(out) +} + +#[allow(clippy::too_many_arguments)] +fn find_in_node( + file_data: &[u8], + header: &BTreeV2Header, + offset: usize, + num_records: u16, + depth: u16, + offset_size: u8, + max_leaf_nrec: u64, + cmp: &mut dyn FnMut(&[u8]) -> Ordering, + budget: &mut usize, + out: &mut Vec, +) -> Result<(), FormatError> { + spend(budget, usize::from(num_records))?; + if depth == 0 { + let records = parse_leaf_records(file_data, offset, num_records, header.record_size)?; + out.extend( + records + .into_iter() + .filter(|r| cmp(&r.data) == Ordering::Equal), + ); + return Ok(()); + } + let rs = usize::from(header.record_size); + let (records_start, children) = read_internal_node( + file_data, + offset, + num_records, + depth, + header.record_size, + header.node_size, + offset_size, + max_leaf_nrec, + )?; + let nr = usize::from(num_records); + let mut order = Vec::with_capacity(nr); + for i in 0..nr { + order.push(cmp(internal_record(file_data, records_start, i, rs)?)); + } + // Child `i` holds the keys between record `i - 1` and record `i`: it can + // hold a match unless the record before it is already past the range or + // the record after it is still before it. + for (i, &(child_addr, child_nrec)) in children.iter().enumerate() { + let after_left = i == 0 || order[i - 1] != Ordering::Greater; + let before_right = i == nr || order[i] != Ordering::Less; + if after_left && before_right { + find_in_node( + file_data, + header, + to_usize(child_addr)?, + child_nrec, + depth - 1, + offset_size, + max_leaf_nrec, + cmp, + budget, + out, + )?; + } + if i < nr && order[i] == Ordering::Equal { + out.push(BTreeV2Record { + data: internal_record(file_data, records_start, i, rs)?.to_vec(), + }); + } + } + Ok(()) +} + /// Most records a subtree whose root is at `depth` can hold (libhdf5's /// `cum_max_nrec`). See [`node_info`]. fn cum_max_records( diff --git a/crates/clawhdf5-format/src/btree_v2_write.rs b/crates/clawhdf5-format/src/btree_v2_write.rs index 1a18984..795fb80 100644 --- a/crates/clawhdf5-format/src/btree_v2_write.rs +++ b/crates/clawhdf5-format/src/btree_v2_write.rs @@ -385,6 +385,59 @@ mod tests { assert!(nodes > 0); } + /// Descending to a key range finds exactly the records a full read + /// holds in it — runs of equal keys that straddle node boundaries + /// included — at every depth, and nothing for keys not in the tree. + #[test] + fn a_key_range_search_matches_a_full_scan() { + use crate::btree_v2::find_btree_v2_records; + use core::cmp::Ordering; + let rs = 11usize; + // Keys 0, 0, 0, 2, 2, 2, 4, ...: runs of three, odd keys missing. + for n in [1usize, 45, 46, 1150, 30_000] { + let mut recs = Vec::with_capacity(n * rs); + for i in 0..n { + let mut r = vec![0u8; rs]; + r[..8].copy_from_slice(&((i / 3 * 2) as u64).to_be_bytes()); + r[8..].copy_from_slice(&[(i % 3) as u8, 0, 0]); + recs.extend_from_slice(&r); + } + let base = 4096u64; + let tree = build_btree_v2(params(512, 11), &recs, base, 8, 8).unwrap(); + let mut file = vec![0u8; base as usize]; + file.extend_from_slice(&tree); + let hdr = BTreeV2Header::parse(&file, base as usize, 8, 8).unwrap(); + let all = collect_btree_v2_records(&file, &hdr, 8, 8).unwrap(); + let key = |r: &[u8]| u64::from_be_bytes(r[..8].try_into().unwrap()); + let last = key(&all[n - 1].data); + let probes = (0..=last + 1).step_by(if n > 1000 { 37 } else { 1 }); + for k in probes.chain([last, last + 1, u64::MAX]) { + let found = + find_btree_v2_records(&file, &hdr, 8, &mut |r: &[u8]| key(r).cmp(&k)).unwrap(); + let want: Vec<&[u8]> = all + .iter() + .map(|r| r.data.as_slice()) + .filter(|r| key(r) == k) + .collect(); + let got: Vec<&[u8]> = found.iter().map(|r| r.data.as_slice()).collect(); + assert_eq!(got, want, "n {n} key {k}"); + assert_eq!( + got.len(), + if k % 2 == 0 && k <= last { + want.len() + } else { + 0 + } + ); + } + // Every record, or none, when the whole tree is in or out of range. + let every = find_btree_v2_records(&file, &hdr, 8, &mut |_| Ordering::Equal).unwrap(); + assert_eq!(every.len(), n); + let none = find_btree_v2_records(&file, &hdr, 8, &mut |_| Ordering::Less).unwrap(); + assert!(none.is_empty()); + } + } + #[test] fn a_node_too_small_or_too_big_is_an_error() { assert!(build_btree_v2(params(16, 11), &records(1, 11), 0, 8, 8).is_err()); diff --git a/crates/clawhdf5-format/src/fractal_heap.rs b/crates/clawhdf5-format/src/fractal_heap.rs index 9bc4ba4..d896156 100644 --- a/crates/clawhdf5-format/src/fractal_heap.rs +++ b/crates/clawhdf5-format/src/fractal_heap.rs @@ -6,7 +6,8 @@ use alloc::{format, vec::Vec}; #[cfg(feature = "checksum")] use byteorder::{ByteOrder, LittleEndian}; -use crate::btree_v2::{BTreeV2Header, collect_btree_v2_records}; +use crate::addr::to_usize; +use crate::btree_v2::{BTreeV2Header, find_btree_v2_records}; use crate::error::FormatError; use crate::filter_pipeline::FilterPipeline; @@ -354,6 +355,7 @@ impl FractalHeapHeader { id_bytes: &[u8], offset_size: u8, ) -> Result, FormatError> { + crate::lookup_stats::heap_object_read(); let Some(&first) = id_bytes.first() else { return Err(FormatError::UnexpectedEof { expected: 1, @@ -448,7 +450,7 @@ impl FractalHeapHeader { } let hdr = BTreeV2Header::parse( file_data, - self.huge_btree_address as usize, + to_usize(self.huge_btree_address)?, self.offset_size, self.length_size, )?; @@ -463,8 +465,12 @@ impl FractalHeapHeader { if hdr.tree_type != expected_type || usize::from(hdr.record_size) < rec_len { return Err(heap_error("unexpected huge-object B-tree record type")); } - let records = - collect_btree_v2_records(file_data, &hdr, self.offset_size, self.length_size)?; + // Records are ordered by ID (the last field): descend to the ones + // equal to `key` instead of reading the whole index. + let id_at = rec_len - ls; + let records = find_btree_v2_records(file_data, &hdr, self.offset_size, &mut |r| { + le_uint(&r[id_at..id_at + ls]).cmp(&key) + })?; for rec in &records { let d = &rec.data; if d.len() < rec_len { @@ -533,24 +539,24 @@ impl FractalHeapHeader { self.read_from_direct_block( file_data, DirectBlock { - addr: self.root_block_address as usize, + addr: to_usize(self.root_block_address)?, size: self.starting_block_size, heap_offset: 0, filtered_size: self.root_direct_block_filtered_size, filter_mask: self.root_direct_block_filter_mask, }, heap_offset, - obj_len as usize, + to_usize(obj_len)?, ) } else { // Root is an indirect block — limit recursion to 64 levels self.read_from_indirect_block( file_data, - self.root_block_address as usize, + to_usize(self.root_block_address)?, self.current_rows_in_root_indirect_block, 0, // block offset heap_offset, - obj_len as usize, + to_usize(obj_len)?, offset_size, 64, // max recursion depth ) @@ -572,11 +578,11 @@ impl FractalHeapHeader { ) -> Result, FormatError> { if target_offset < block.heap_offset { return Err(FormatError::UnexpectedEof { - expected: block.heap_offset as usize, - available: target_offset as usize, + expected: to_usize(block.heap_offset)?, + available: to_usize(target_offset)?, }); } - let local_offset = (target_offset - block.heap_offset) as usize; + let local_offset = to_usize(target_offset - block.heap_offset)?; if let Some(pipeline) = &self.filter_pipeline { let stored_len = usize::try_from(block.filtered_size) .map_err(|_| heap_error("direct block size"))?; @@ -673,7 +679,7 @@ impl FractalHeapHeader { return self.read_from_direct_block( file_data, DirectBlock { - addr: child_addr as usize, + addr: to_usize(child_addr)?, size: block_size, heap_offset: current_heap_offset, filtered_size, @@ -705,7 +711,7 @@ impl FractalHeapHeader { { return self.read_from_indirect_block( file_data, - child_addr as usize, + to_usize(child_addr)?, child_nrows, current_heap_offset, target_offset, @@ -719,7 +725,7 @@ impl FractalHeapHeader { } Err(FormatError::UnexpectedEof { - expected: target_offset as usize + length, + expected: to_usize(target_offset)?.saturating_add(length), available: file_data.len(), }) } diff --git a/crates/clawhdf5-format/src/group_v2.rs b/crates/clawhdf5-format/src/group_v2.rs index b645bd6..e1bd9f5 100644 --- a/crates/clawhdf5-format/src/group_v2.rs +++ b/crates/clawhdf5-format/src/group_v2.rs @@ -6,7 +6,9 @@ #[cfg(not(feature = "std"))] use alloc::{string::String, vec::Vec}; -use crate::btree_v2::{BTreeV2Header, collect_btree_v2_records}; +use crate::addr::to_usize; +use crate::btree_v2::{BTreeV2Header, collect_btree_v2_records, find_btree_v2_records}; +use crate::checksum::jenkins_lookup3; use crate::error::FormatError; use crate::fractal_heap::FractalHeapHeader; use crate::group_v1::{self, GroupEntry}; @@ -93,13 +95,14 @@ fn for_each_dense_link( mut visit: impl FnMut(LinkMessage), ) -> Result<(), FormatError> { // Parse fractal heap - let fh = FractalHeapHeader::parse(file_data, fh_addr as usize, offset_size, length_size)?; + let fh = FractalHeapHeader::parse(file_data, to_usize(fh_addr)?, offset_size, length_size)?; // Parse B-tree v2 for name index let btree_addr = link_info .btree_name_index_address .ok_or_else(|| FormatError::PathNotFound(String::from("no B-tree v2 name index")))?; - let btree_hdr = BTreeV2Header::parse(file_data, btree_addr as usize, offset_size, length_size)?; + let btree_hdr = + BTreeV2Header::parse(file_data, to_usize(btree_addr)?, offset_size, length_size)?; let records = collect_btree_v2_records(file_data, &btree_hdr, offset_size, length_size)?; for record in &records { @@ -181,10 +184,55 @@ fn find_symbolic_link( if !is_v2_group(object_header) { return Ok(None); } - let is_symbolic = |t: &LinkTarget| !matches!(t, LinkTarget::Hard { .. }); + // As when all links were scanned: the last symbolic link of that name. + Ok( + links_named(file_data, object_header, name, offset_size, length_size)? + .into_iter() + .rev() + .map(|link| link.link_target) + .find(|t| !matches!(t, LinkTarget::Hard { .. })), + ) +} + +/// B-tree v2 record type of a dense group's link name index. +const LINK_NAME_INDEX: u8 = 5; + +/// The links called `name` in a v2 group (a valid group has at most one). +/// +/// In dense storage the link name index (a v2 B-tree of lookup3 name +/// hashes, record type 5) is descended to the records with the name's hash, +/// and only their links are read from the heap — O(log n) instead of every +/// link. libhdf5 orders records with equal hashes by name; all of them are +/// read and compared here, so that order does not matter. An index of +/// another type is scanned in full. +fn links_named( + file_data: &[u8], + object_header: &ObjectHeader, + name: &str, + offset_size: u8, + length_size: u8, +) -> Result, FormatError> { + let mut found = Vec::new(); let link_info = find_link_info(object_header, offset_size)?; - let mut found = None; - if let Some(fh_addr) = link_info.fractal_heap_address { + let Some(fh_addr) = link_info.fractal_heap_address else { + for msg in &object_header.messages { + if msg.msg_type == MessageType::Link + && let Some(link) = parse_link(&msg.data, offset_size)? + && link.name == name + { + found.push(link); + } + } + return Ok(found); + }; + + let fh = FractalHeapHeader::parse(file_data, to_usize(fh_addr)?, offset_size, length_size)?; + let btree_addr = link_info + .btree_name_index_address + .ok_or_else(|| FormatError::PathNotFound(String::from("no B-tree v2 name index")))?; + let btree_hdr = + BTreeV2Header::parse(file_data, to_usize(btree_addr)?, offset_size, length_size)?; + if btree_hdr.tree_type != LINK_NAME_INDEX { for_each_dense_link( file_data, &link_info, @@ -192,26 +240,134 @@ fn find_symbolic_link( offset_size, length_size, |link| { - if link.name == name && is_symbolic(&link.link_target) { - found = Some(link.link_target); + if link.name == name { + found.push(link); } }, )?; - } else { - for msg in &object_header.messages { - if msg.msg_type == MessageType::Link { - let Some(link) = parse_link(&msg.data, offset_size)? else { - continue; - }; - if link.name == name && is_symbolic(&link.link_target) { - found = Some(link.link_target); - } - } + return Ok(found); + } + + // Record: hash(4) + heap ID. + let hash = jenkins_lookup3(name.as_bytes()); + let records = find_btree_v2_records(file_data, &btree_hdr, offset_size, &mut |r| { + match r.get(..4) { + Some(h) => u32::from_le_bytes([h[0], h[1], h[2], h[3]]).cmp(&hash), + // Too short to hold a hash (a corrupt record size): never a match. + None => core::cmp::Ordering::Less, + } + })?; + let id_len = usize::from(fh.heap_id_length); + for record in &records { + let Some(id_bytes) = record.data.get(4..4 + id_len) else { + continue; + }; + let link_data = fh.read_managed_object(file_data, id_bytes, offset_size)?; + if let Some(link) = parse_link(&link_data, offset_size)? + && link.name == name + { + found.push(link); } } Ok(found) } +/// The link [`resolve_path_any`] follows for one path component `name` of +/// the group with header `object_header`: a hard link (as `Hard`), else a +/// soft or external link of that name, else `None`. Fails with +/// `PathNotFound` if the object is not a group. +fn lookup_link( + file_data: &[u8], + object_header: &ObjectHeader, + name: &str, + offset_size: u8, + length_size: u8, +) -> Result, FormatError> { + if is_v1_group(object_header) { + let entries = resolve_group_entries(file_data, object_header, offset_size, length_size)?; + if let Some(e) = entries + .iter() + .find(|e| e.name == name && e.object_header_address != u64::MAX) + { + return Ok(Some(LinkTarget::Hard { + object_header_address: e.object_header_address, + })); + } + return find_symbolic_link(file_data, object_header, name, offset_size, length_size); + } + if !is_v2_group(object_header) { + return Err(FormatError::PathNotFound(String::from( + "object header is not a group", + ))); + } + let links = links_named(file_data, object_header, name, offset_size, length_size)?; + if let Some(addr) = links.iter().find_map(|l| match l.link_target { + LinkTarget::Hard { + object_header_address, + } if object_header_address != u64::MAX => Some(object_header_address), + _ => None, + }) { + return Ok(Some(LinkTarget::Hard { + object_header_address: addr, + })); + } + Ok(links + .into_iter() + .rev() + .map(|link| link.link_target) + .find(|t| !matches!(t, LinkTarget::Hard { .. }))) +} + +/// The object header address of the child called `name` of the group at +/// `group_address`: the address [`resolve_group_children`] lists under that +/// name, or `PathNotFound` if it lists none. +/// +/// A dense group's child is found through its link name index (see +/// [`links_named`]) and only the named link is read and, if it is a soft +/// link, followed — not every link in the group. A v1 group is listed. +pub fn resolve_child( + file_data: &[u8], + superblock: &Superblock, + group_address: u64, + name: &str, +) -> Result { + let os = superblock.offset_size; + let ls = superblock.length_size; + let not_found = || FormatError::PathNotFound(String::from(name)); + let header = ObjectHeader::parse(file_data, to_usize(group_address)?, os, ls)?; + if !is_v2_group(&header) || is_v1_group(&header) { + return resolve_group_children(file_data, superblock, group_address)? + .into_iter() + .find(|e| e.name == name) + .map(|e| e.object_header_address) + .ok_or_else(not_found); + } + let links = links_named(file_data, &header, name, os, ls)?; + // The listing puts hard links before resolved soft links. + if let Some(addr) = links.iter().find_map(|l| match l.link_target { + LinkTarget::Hard { + object_header_address, + } => Some(object_header_address), + _ => None, + }) { + return Ok(addr); + } + for link in links { + if let LinkTarget::Soft { target_path } = link.link_target { + return match resolve_path_from(file_data, superblock, group_address, &target_path) { + // Left out of the listing: dangling, cyclic, or in another file. + Err( + FormatError::PathNotFound(_) + | FormatError::NestingDepthExceeded + | FormatError::ExternalLinkUnsupported { .. }, + ) => Err(not_found()), + other => other, + }; + } + } + Err(not_found()) +} + /// Find and parse the Link Info message from an object header. fn find_link_info( object_header: &ObjectHeader, @@ -298,7 +454,7 @@ pub fn resolve_group_children( ) -> Result, FormatError> { let os = superblock.offset_size; let ls = superblock.length_size; - let header = ObjectHeader::parse(file_data, group_address as usize, os, ls)?; + let header = ObjectHeader::parse(file_data, to_usize(group_address)?, os, ls)?; let mut entries = Vec::new(); let mut soft = Vec::new(); @@ -383,24 +539,21 @@ fn resolve_path_following_links( let ls = superblock.length_size; let mut current_addr = start; - let mut current_header = ObjectHeader::parse(file_data, start as usize, os, ls)?; + let mut current_header = ObjectHeader::parse(file_data, to_usize(start)?, os, ls)?; for (i, component) in components.iter().enumerate() { - let entries = resolve_group_entries(file_data, ¤t_header, os, ls)?; - - let found = entries - .iter() - .find(|e| e.name == *component && e.object_header_address != u64::MAX); - match found { - Some(entry) => { + match lookup_link(file_data, ¤t_header, component, os, ls)? { + Some(LinkTarget::Hard { + object_header_address, + }) => { if i == components.len() - 1 { - return Ok(entry.object_header_address); + return Ok(object_header_address); } - current_addr = entry.object_header_address; - current_header = ObjectHeader::parse(file_data, current_addr as usize, os, ls)?; + current_addr = object_header_address; + current_header = ObjectHeader::parse(file_data, to_usize(current_addr)?, os, ls)?; } - None => { - return match find_symbolic_link(file_data, ¤t_header, component, os, ls)? { + found => { + return match found { Some(LinkTarget::Soft { target_path }) => { if depth >= MAX_SOFT_LINK_DEPTH { return Err(FormatError::NestingDepthExceeded); diff --git a/crates/clawhdf5-format/src/lib.rs b/crates/clawhdf5-format/src/lib.rs index 2737c9b..345c923 100644 --- a/crates/clawhdf5-format/src/lib.rs +++ b/crates/clawhdf5-format/src/lib.rs @@ -108,6 +108,7 @@ pub mod lane_partition; pub mod link_info; pub mod link_message; pub mod local_heap; +pub mod lookup_stats; pub mod message_type; pub mod metadata_cache; pub mod metadata_index; diff --git a/crates/clawhdf5-format/src/lookup_stats.rs b/crates/clawhdf5-format/src/lookup_stats.rs new file mode 100644 index 0000000..0206729 --- /dev/null +++ b/crates/clawhdf5-format/src/lookup_stats.rs @@ -0,0 +1,31 @@ +//! Work counters for tests of lookup cost (feature `lookup-stats`). +//! +//! Counts fractal-heap objects read — each is one link or attribute message +//! decoded out of a dense group or dense attribute storage — so a test can +//! check that finding one name reads a handful of them, not the whole group. +//! Per thread, so tests running in parallel do not see each other's reads. +//! Without the feature the counting compiles to nothing. + +#[cfg(feature = "lookup-stats")] +std::thread_local! { + static HEAP_OBJECTS: core::cell::Cell = const { core::cell::Cell::new(0) }; +} + +/// Record one heap object read. +#[inline(always)] +pub(crate) fn heap_object_read() { + #[cfg(feature = "lookup-stats")] + HEAP_OBJECTS.with(|c| c.set(c.get() + 1)); +} + +/// Heap objects read on this thread since the last [`reset`]. +#[cfg(feature = "lookup-stats")] +pub fn heap_objects_read() -> u64 { + HEAP_OBJECTS.with(core::cell::Cell::get) +} + +/// Zero this thread's counters. +#[cfg(feature = "lookup-stats")] +pub fn reset() { + HEAP_OBJECTS.with(|c| c.set(0)); +} diff --git a/crates/clawhdf5/Cargo.toml b/crates/clawhdf5/Cargo.toml index 1eca373..4f68e58 100644 --- a/crates/clawhdf5/Cargo.toml +++ b/crates/clawhdf5/Cargo.toml @@ -19,7 +19,8 @@ rayon = { version = "1", optional = true } tempfile = { workspace = true } criterion = { workspace = true } clawhdf5-io = { path = "../clawhdf5-io", version = "2.7.0", features = ["mmap"] } -clawhdf5-format = { path = "../clawhdf5-format", version = "2.7.0", features = ["parallel", "fast-checksum"] } +clawhdf5-format = { path = "../clawhdf5-format", version = "2.7.0", features = ["parallel", "fast-checksum", "lookup-stats"] } +serde_json = "1" clawhdf5-filters = { path = "../clawhdf5-filters", version = "2.7.0" } [[bench]] diff --git a/crates/clawhdf5/src/lazy.rs b/crates/clawhdf5/src/lazy.rs index df41688..7ad7e78 100644 --- a/crates/clawhdf5/src/lazy.rs +++ b/crates/clawhdf5/src/lazy.rs @@ -28,7 +28,7 @@ use clawhdf5_format::superblock::Superblock; use clawhdf5_io::HDF5Read; use crate::error::Error; -use crate::types::{AttrValue, DType, classify_datatype, read_attrs}; +use crate::types::{AttrValue, DType, classify_datatype, read_attr, read_attrs}; /// A lazy HDF5 file handle that parses metadata on demand. /// @@ -304,12 +304,7 @@ impl<'f, R: HDF5Read> LazyGroup<'f, R> { /// Get a dataset within this group by name. pub fn dataset(&self, name: &str) -> Result, Error> { - let entries = self.children()?; - let entry = entries - .iter() - .find(|e| e.name == name) - .ok_or_else(|| Error::Format(FormatError::PathNotFound(name.to_string())))?; - let hdr = self.file.get_or_parse_header(entry.object_header_address)?; + let hdr = self.file.get_or_parse_header(self.child_address(name)?)?; if !has_message(&hdr, MessageType::DataLayout) { return Err(Error::NotADataset(name.to_string())); } @@ -322,17 +317,38 @@ impl<'f, R: HDF5Read> LazyGroup<'f, R> { /// Get a subgroup within this group by name. pub fn group(&self, name: &str) -> Result, Error> { - let entries = self.children()?; - let entry = entries - .iter() - .find(|e| e.name == name) - .ok_or_else(|| Error::Format(FormatError::PathNotFound(name.to_string())))?; Ok(LazyGroup { file: self.file, - address: entry.object_header_address, + address: self.child_address(name)?, }) } + /// The attribute called `name`, or `None` if it has none by that name + /// (or it cannot be read) — the value [`attrs`](Self::attrs) has under + /// that name, found without reading the other attributes when they are + /// stored densely. + pub fn attr(&self, name: &str) -> Result, Error> { + let hdr = self.file.get_or_parse_header(self.address)?; + let data = self.file.hdf5_bytes(); + read_attr( + data, + &hdr, + name, + self.file.offset_size(), + self.file.length_size(), + ) + } + + /// The object header address of the child called `name`: the entry of + /// the group's listing with that name, looked up through the group's + /// name index rather than by listing the group (see + /// [`group_v2::resolve_child`]). + fn child_address(&self, name: &str) -> Result { + let data = self.file.hdf5_bytes(); + group_v2::resolve_child(data, &self.file.superblock, self.address, name) + .map_err(Error::Format) + } + /// This group's links that can be opened: hard links, and soft links /// resolved to their targets (see /// [`group_v2::resolve_group_children`]); dangling, external and @@ -555,6 +571,21 @@ impl<'f, R: HDF5Read> LazyDataset<'f, R> { ) } + /// The attribute called `name`, or `None` if it has none by that name + /// (or it cannot be read) — the value [`attrs`](Self::attrs) has under + /// that name, found without reading the other attributes when they are + /// stored densely. + pub fn attr(&self, name: &str) -> Result, Error> { + let data = self.file.hdf5_bytes(); + read_attr( + data, + &self.header, + name, + self.file.offset_size(), + self.file.length_size(), + ) + } + /// A header message's payload, resolved through the shared-message /// indirection when needed (e.g. a committed datatype). See /// [`clawhdf5_format::shared_message::message_data`]. diff --git a/crates/clawhdf5/src/mmap_file.rs b/crates/clawhdf5/src/mmap_file.rs index b6df1d8..edb6dd4 100644 --- a/crates/clawhdf5/src/mmap_file.rs +++ b/crates/clawhdf5/src/mmap_file.rs @@ -23,7 +23,7 @@ use clawhdf5_format::superblock::Superblock; use clawhdf5_io::MmapReader; use crate::error::Error; -use crate::types::{AttrValue, DType, classify_datatype, read_attrs}; +use crate::types::{AttrValue, DType, classify_datatype, read_attr, read_attrs}; /// An HDF5 file opened via memory mapping. /// @@ -242,12 +242,7 @@ impl<'f> MmapGroup<'f> { /// Get a dataset within this group by name. pub fn dataset(&self, name: &str) -> Result, Error> { - let entries = self.children()?; - let entry = entries - .iter() - .find(|e| e.name == name) - .ok_or_else(|| Error::Format(FormatError::PathNotFound(name.to_string())))?; - let hdr = self.file.parse_header(entry.object_header_address)?; + let hdr = self.file.parse_header(self.child_address(name)?)?; if !has_message(&hdr, MessageType::DataLayout) { return Err(Error::NotADataset(name.to_string())); } @@ -260,17 +255,38 @@ impl<'f> MmapGroup<'f> { /// Get a subgroup within this group by name. pub fn group(&self, name: &str) -> Result, Error> { - let entries = self.children()?; - let entry = entries - .iter() - .find(|e| e.name == name) - .ok_or_else(|| Error::Format(FormatError::PathNotFound(name.to_string())))?; Ok(MmapGroup { file: self.file, - address: entry.object_header_address, + address: self.child_address(name)?, }) } + /// The attribute called `name`, or `None` if it has none by that name + /// (or it cannot be read) — the value [`attrs`](Self::attrs) has under + /// that name, found without reading the other attributes when they are + /// stored densely. + pub fn attr(&self, name: &str) -> Result, Error> { + let hdr = self.file.parse_header(self.address)?; + let data = self.file.hdf5_bytes(); + read_attr( + data, + &hdr, + name, + self.file.offset_size(), + self.file.length_size(), + ) + } + + /// The object header address of the child called `name`: the entry of + /// the group's listing with that name, looked up through the group's + /// name index rather than by listing the group (see + /// [`group_v2::resolve_child`]). + fn child_address(&self, name: &str) -> Result { + let data = self.file.meta()?; + group_v2::resolve_child(data, &self.file.superblock, self.address, name) + .map_err(Error::Format) + } + /// This group's links that can be opened: hard links, and soft links /// resolved to their targets (see /// [`group_v2::resolve_group_children`]); dangling, external and @@ -506,6 +522,21 @@ impl<'f> MmapDataset<'f> { ) } + /// The attribute called `name`, or `None` if it has none by that name + /// (or it cannot be read) — the value [`attrs`](Self::attrs) has under + /// that name, found without reading the other attributes when they are + /// stored densely. + pub fn attr(&self, name: &str) -> Result, Error> { + let data = self.file.hdf5_bytes(); + read_attr( + data, + &self.header, + name, + self.file.offset_size(), + self.file.length_size(), + ) + } + /// A header message's payload, resolved through the shared-message /// indirection when needed (e.g. a committed datatype). See /// [`clawhdf5_format::shared_message::message_data`]. diff --git a/crates/clawhdf5/src/reader.rs b/crates/clawhdf5/src/reader.rs index 1b20dd6..041a318 100644 --- a/crates/clawhdf5/src/reader.rs +++ b/crates/clawhdf5/src/reader.rs @@ -23,7 +23,7 @@ use clawhdf5_format::superblock::Superblock; use crate::cache_image::{self, ImageView}; use crate::error::Error; -use crate::types::{AttrValue, DType, classify_datatype, read_attrs}; +use crate::types::{AttrValue, DType, classify_datatype, read_attr, read_attrs}; // --------------------------------------------------------------------------- // FileData — internal storage for either owned bytes or an mmap @@ -230,9 +230,10 @@ impl File { /// A `Dataset` handle for the object header at `address` (an address /// from a group listing, or one kept from an earlier lookup), without - /// resolving a path. Resolving a path walks every group on it, which in - /// a large group costs a scan of its links; keep the address instead to - /// open the same dataset repeatedly. + /// resolving a path. Resolving a path looks each component up in its + /// group (through the name index of a dense group; a v1 group's entries + /// are scanned); keep the address instead to open the same dataset + /// repeatedly. pub fn dataset_at(&self, address: u64) -> Result, Error> { let hdr = self.parse_header(address)?; if !has_message(&hdr, MessageType::DataLayout) { @@ -245,6 +246,17 @@ impl File { .check_open() } + /// A `Group` handle for the object header at `address` (from + /// [`Group::entries`], or kept from an earlier lookup), without + /// resolving a path. Like [`group`](Self::group), the object is not + /// checked to be a group; a non-group has no children. + pub fn group_at(&self, address: u64) -> Group<'_> { + Group { + file: self, + address, + } + } + /// Resolve a path and return a `Group` handle. /// /// The path uses `/` separators (e.g., `"sensors"`). @@ -483,12 +495,7 @@ impl<'f> Group<'f> { /// Get a dataset within this group by name. pub fn dataset(&self, name: &str) -> Result, Error> { - let entries = self.children()?; - let entry = entries - .iter() - .find(|e| e.name == name) - .ok_or_else(|| Error::Format(FormatError::PathNotFound(name.to_string())))?; - let hdr = self.file.parse_header(entry.object_header_address)?; + let hdr = self.file.parse_header(self.child_address(name)?)?; if !has_message(&hdr, MessageType::DataLayout) { return Err(Error::NotADataset(name.to_string())); } @@ -501,17 +508,51 @@ impl<'f> Group<'f> { /// Get a subgroup within this group by name. pub fn group(&self, name: &str) -> Result, Error> { - let entries = self.children()?; - let entry = entries - .iter() - .find(|e| e.name == name) - .ok_or_else(|| Error::Format(FormatError::PathNotFound(name.to_string())))?; Ok(Group { file: self.file, - address: entry.object_header_address, + address: self.child_address(name)?, }) } + /// The attribute called `name`, or `None` if it has none by that name + /// (or it cannot be read) — the value [`attrs`](Self::attrs) has under + /// that name, found without reading the other attributes when they are + /// stored densely. + pub fn attr(&self, name: &str) -> Result, Error> { + let hdr = self.file.parse_header(self.address)?; + let data = self.file.data.as_bytes(); + read_attr( + data, + &hdr, + name, + self.file.offset_size(), + self.file.length_size(), + ) + } + + /// The object header address of the child called `name`: the entry of + /// the group's listing with that name, looked up through the group's + /// name index rather than by listing the group (see + /// [`group_v2::resolve_child`]). + fn child_address(&self, name: &str) -> Result { + let data = self.file.data.meta()?; + group_v2::resolve_child(data, &self.file.superblock, self.address, name) + .map_err(Error::Format) + } + + /// This group's children that can be opened, as `(name, object header + /// address)` in listing order — the entries [`datasets`](Self::datasets) + /// and [`groups`](Self::groups) are drawn from. Open one with + /// [`File::dataset_at`] or [`File::group_at`] to skip looking its name + /// up again, or keep the addresses to revisit the objects. + pub fn entries(&self) -> Result, Error> { + Ok(self + .children()? + .into_iter() + .map(|e| (e.name, e.object_header_address)) + .collect()) + } + /// This group's links that can be opened: hard links, and soft links /// resolved to their targets (see /// [`group_v2::resolve_group_children`]); dangling, external and @@ -1051,6 +1092,21 @@ impl<'f> Dataset<'f> { ) } + /// The attribute called `name`, or `None` if it has none by that name + /// (or it cannot be read) — the value [`attrs`](Self::attrs) has under + /// that name, found without reading the other attributes when they are + /// stored densely. + pub fn attr(&self, name: &str) -> Result, Error> { + let data = self.file.data.as_bytes(); + read_attr( + data, + &self.header, + name, + self.file.offset_size(), + self.file.length_size(), + ) + } + /// Verify this dataset's content against its stored provenance hash /// (`_provenance_sha256`, written automatically on save when a /// [`Provenance`](clawhdf5_format::provenance::Provenance) is set — see diff --git a/crates/clawhdf5/src/types.rs b/crates/clawhdf5/src/types.rs index 7023b14..acae280 100644 --- a/crates/clawhdf5/src/types.rs +++ b/crates/clawhdf5/src/types.rs @@ -182,6 +182,35 @@ pub(crate) fn read_attrs( )) } +/// The attribute called `name` on the object with header `header`, decoded +/// as [`read_attrs`] decodes it, or `None` (see +/// [`find_attribute_in_file`](clawhdf5_format::attribute::find_attribute_in_file)). +pub(crate) fn read_attr( + file_data: &[u8], + header: &clawhdf5_format::object_header::ObjectHeader, + name: &str, + offset_size: u8, + length_size: u8, +) -> Result, crate::Error> { + let Some(msg) = clawhdf5_format::attribute::find_attribute_in_file( + file_data, + header, + name, + offset_size, + length_size, + )? + else { + return Ok(None); + }; + Ok(attrs_to_map( + std::slice::from_ref(&msg), + file_data, + offset_size, + length_size, + ) + .remove(name)) +} + pub(crate) fn attrs_to_map( attrs: &[clawhdf5_format::attribute::AttributeMessage], file_data: &[u8], diff --git a/crates/clawhdf5/tests/dense_storage_interop.rs b/crates/clawhdf5/tests/dense_storage_interop.rs index b5aa8d1..3f741f6 100644 --- a/crates/clawhdf5/tests/dense_storage_interop.rs +++ b/crates/clawhdf5/tests/dense_storage_interop.rs @@ -211,6 +211,30 @@ fn dense_attribute_stored_as_a_huge_heap_object() { assert!(matches!(&attrs["bigger"], AttrValue::I64Array(v) if *v == bigger)); } +/// Enough huge attributes that the huge-object B-tree has internal nodes: +/// each one is found by descending it by heap ID (libhdf5 orders the +/// records of indirectly addressed huge objects by ID), and every value +/// matches what was written. +#[test] +fn many_huge_attributes_are_found_through_their_index() { + skip_if_no_python!(); + let (_dir, path) = h5py_file( + "d = f.create_dataset('d', data=[1.0])\n\ + for i in range(300):\n\ + \x20 d.attrs['h%03d' % i] = np.arange(600, dtype='i8') + i\n", + ); + let f = File::open(&path).unwrap(); + let d = f.dataset("d").unwrap(); + let attrs = d.attrs().unwrap(); + assert_eq!(attrs.len(), 300); + for i in 0..300i64 { + let name = format!("h{i:03}"); + let want: Vec = (0..600).map(|v| v + i).collect(); + assert!(matches!(&attrs[&name], AttrValue::I64Array(v) if *v == want), "{name}"); + assert!(matches!(d.attr(&name).unwrap(), Some(AttrValue::I64Array(v)) if v == want), "{name}"); + } +} + /// A group whose link heap has a deflate I/O filter (set on the group /// creation property list), with 3 000 links and one link whose message is /// larger than the heap's managed-object limit, so it is a huge object. diff --git a/crates/clawhdf5/tests/indexed_lookup_interop.rs b/crates/clawhdf5/tests/indexed_lookup_interop.rs new file mode 100644 index 0000000..518af1e --- /dev/null +++ b/crates/clawhdf5/tests/indexed_lookup_interop.rs @@ -0,0 +1,410 @@ +//! Looking one name up in a dense group (links in a fractal heap, indexed by +//! a v2 B-tree of name hashes) or in dense attribute storage reads the name +//! index, not every link: O(log n) index nodes and only the links whose +//! lookup3 hash equals the name's. Before, every lookup decoded all n links, +//! so opening each child of a 35 001-link group by name decoded ~1.2e9. +//! +//! The file is written by h5py (libhdf5 orders the index), with names whose +//! hashes collide, and every result is compared with what h5py reads. +//! +//! Skipped when python3 with h5py is unavailable, unless +//! `CLAWHDF5_REQUIRE_INTEROP=1`. + +use std::collections::{BTreeMap, HashMap}; +use std::process::Command; +use std::sync::OnceLock; +use std::time::{Duration, Instant}; + +use clawhdf5::{AttrValue, File, LazyFile, MmapFile}; +use clawhdf5_format::checksum::jenkins_lookup3; +use clawhdf5_format::error::FormatError; +use clawhdf5_format::lookup_stats; + +fn python() -> String { + std::env::var("CLAWHDF5_PYTHON").unwrap_or_else(|_| "python3".to_string()) +} + +fn interop_required() -> bool { + std::env::var("CLAWHDF5_REQUIRE_INTEROP").is_ok_and(|v| v == "1") +} + +fn python_available() -> bool { + Command::new(python()) + .args(["-c", "import h5py"]) + .output() + .map(|o| o.status.success()) + .unwrap_or(false) +} + +macro_rules! skip_if_no_python { + () => { + if !python_available() { + assert!( + !interop_required(), + "CLAWHDF5_REQUIRE_INTEROP=1 but python3 with h5py is not available" + ); + eprintln!("SKIP: python3 with h5py not available"); + return; + } + }; +} + +fn run_python(script: &str) -> String { + let output = Command::new(python()) + .args(["-c", script]) + .output() + .expect("failed to run python"); + assert!( + output.status.success(), + "Python script failed:\nSTDOUT: {}\nSTDERR: {}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8_lossy(&output.stdout).trim().to_string() +} + +/// Links in the big group, as libhdf5's `h5stat_newgrat.h5` has. +const LINKS: usize = 35_001; +/// Attributes on the dense-attribute dataset. +const ATTRS: usize = 3_000; + +/// Pairs of distinct names with equal lookup3 hashes, found by search (the +/// hash is fixed, so the pairs are too). +fn colliding_pairs(count: usize) -> Vec<(String, String)> { + let mut seen: HashMap = HashMap::new(); + let mut pairs = Vec::new(); + for i in 0.. { + let name = format!("c{i}"); + let h = jenkins_lookup3(name.as_bytes()); + if let Some(first) = seen.insert(h, name.clone()) { + pairs.push((first, name)); + if pairs.len() == count { + break; + } + } + } + pairs +} + +/// What h5py reads: link values, attribute values, and which of the +/// missing names it finds as links and as attributes (none). +type H5pyView = ( + BTreeMap, + BTreeMap, + Vec, + Vec, +); + +struct Fixture { + _dir: tempfile::TempDir, + path: String, + /// Names in the big group, with the value of the scalar dataset each + /// links to, as h5py reads them. + links: BTreeMap, + /// Names that are not links but hash like one that is. + missing_links: Vec, + /// Attributes of `/x`, as h5py reads them. + attrs: BTreeMap, + missing_attrs: Vec, +} + +fn fixture() -> &'static Fixture { + static FIXTURE: OnceLock = OnceLock::new(); + FIXTURE.get_or_init(|| { + let pairs = colliding_pairs(6); + for (a, b) in &pairs { + assert_ne!(a, b); + assert_eq!(jenkins_lookup3(a.as_bytes()), jenkins_lookup3(b.as_bytes())); + } + // Pairs 0-2 both present (either can be the one libhdf5 orders + // first), pairs 3-5 only the first: its partner must not be found. + // "k69209"/"k155448" is the pair the writer once misordered. + let mut present: Vec = vec!["k69209".into(), "k155448".into()]; + let mut missing: Vec = Vec::new(); + for (i, (a, b)) in pairs.into_iter().enumerate() { + present.push(a); + if i < 3 { + present.push(b); + } else { + missing.push(b); + } + } + missing.extend(["", "nope", "n35001x", "N1"].map(String::from)); + let mut links = present.clone(); + let mut i = 0; + while links.len() < LINKS { + links.push(format!("n{i}")); + i += 1; + } + let mut attrs = present.clone(); + attrs.extend((0..ATTRS - present.len()).map(|i| format!("a{i}"))); + + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("big.h5").display().to_string(); + // The names go through a file: 35 001 of them overflow an argument. + let names = dir.path().join("names.json"); + std::fs::write( + &names, + serde_json::to_string(&(&links, &attrs, &missing)).unwrap(), + ) + .unwrap(); + let names = names.display(); + let out = run_python(&format!( + "import h5py, json, numpy as np\n\ + links, attrs, missing = json.load(open(r'{names}'))\n\ + with h5py.File(r'{path}', 'w', libver='latest') as f:\n\ + \x20 g = f.create_group('g')\n\ + \x20 for i, n in enumerate(links):\n\ + \x20 g.create_dataset(n, data=np.int64(i))\n\ + \x20 x = f.create_dataset('x', data=np.int64(0))\n\ + \x20 for i, n in enumerate(attrs):\n\ + \x20 x.attrs[n] = np.int64(1000 + i)\n\ + with h5py.File(r'{path}', 'r') as f:\n\ + \x20 g, a = f['g'], f['x'].attrs\n\ + \x20 print(json.dumps([{{n: int(g[n][()]) for n in g}}, {{n: int(a[n]) for n in a}},\n\ + \x20 [n for n in missing if n and n in g], [n for n in missing if n and n in a]]))", + )); + let (links, attrs, found_links, found_attrs): H5pyView = + serde_json::from_str(&out).unwrap(); + assert_eq!(links.len(), LINKS); + assert_eq!(attrs.len(), ATTRS); + assert!(found_links.is_empty() && found_attrs.is_empty()); + Fixture { + _dir: dir, + path, + links, + missing_links: missing.clone(), + attrs, + missing_attrs: missing, + } + }) +} + +fn is_not_found(e: &clawhdf5::Error) -> bool { + matches!(e, clawhdf5::Error::Format(FormatError::PathNotFound(_))) +} + +#[test] +fn one_link_lookup_reads_the_index_not_every_link() { + skip_if_no_python!(); + let fx = fixture(); + let f = File::open(&fx.path).unwrap(); + let g = f.group("g").unwrap(); + for (name, value) in &fx.links { + lookup_stats::reset(); + let ds = g.dataset(name).unwrap(); + // One link decoded per lookup, two where hashes collide — not 35 001. + let read = lookup_stats::heap_objects_read(); + assert!(read <= 2, "looking up {name} read {read} heap objects"); + assert_eq!(ds.read_i64().unwrap(), vec![*value], "{name}"); + } + + for name in &fx.missing_links { + lookup_stats::reset(); + let err = g.dataset(name).unwrap_err(); + assert!(is_not_found(&err), "{name:?}: {err:?}"); + assert!(lookup_stats::heap_objects_read() <= 2, "{name:?}"); + } + + // A path resolves each component the same way. + for name in ["k155448", "n0", "n34000"] { + lookup_stats::reset(); + let ds = f.dataset(&format!("/g/{name}")).unwrap(); + assert!(lookup_stats::heap_objects_read() <= 2); + assert_eq!(ds.read_i64().unwrap(), vec![fx.links[name]]); + } +} + +#[test] +fn one_attribute_lookup_reads_the_index_not_every_attribute() { + skip_if_no_python!(); + let fx = fixture(); + let f = File::open(&fx.path).unwrap(); + let x = f.dataset("x").unwrap(); + let all = x.attrs().unwrap(); + assert_eq!(all.len(), ATTRS); + for (name, value) in &fx.attrs { + lookup_stats::reset(); + let got = x.attr(name).unwrap(); + assert!(lookup_stats::heap_objects_read() <= 2, "{name}"); + assert!( + matches!(got, Some(AttrValue::I64(v)) if v == *value), + "{name}: {got:?}" + ); + assert!(matches!(all.get(name), Some(AttrValue::I64(v)) if v == value)); + } + for name in &fx.missing_attrs { + lookup_stats::reset(); + assert!(x.attr(name).unwrap().is_none(), "{name:?}"); + assert!(lookup_stats::heap_objects_read() <= 2, "{name:?}"); + } + // Compact attributes (on the root group: none) and a group's attributes. + assert!(f.root().attr("k69209").unwrap().is_none()); +} + +/// Every child of the big group opened by name through each file type, +/// within `limit`: with a scan per lookup this is ~1.2e9 link decodes. +#[test] +fn opening_every_child_of_a_35001_link_group_by_name_is_quick() { + skip_if_no_python!(); + let fx = fixture(); + let limit = Duration::from_secs(120); + let started = Instant::now(); + let check_time = |n: usize| { + assert!( + started.elapsed() < limit, + "{n} lookups took {:?}", + started.elapsed() + ); + }; + + let f = File::open(&fx.path).unwrap(); + let g = f.group("g").unwrap(); + for (n, (name, value)) in fx.links.iter().enumerate() { + assert_eq!(g.dataset(name).unwrap().read_i64().unwrap(), vec![*value]); + check_time(n); + } + // The listing hands out entries: open each by address. + let entries = g.entries().unwrap(); + assert_eq!(entries.len(), LINKS); + for (name, address) in &entries { + let ds = f.dataset_at(*address).unwrap(); + assert_eq!(ds.read_i64().unwrap(), vec![fx.links[name]]); + } + assert!(f.group_at(g_address(&f)).dataset("n0").is_ok()); + + let m = MmapFile::open(&fx.path).unwrap(); + let mg = m.group("g").unwrap(); + for (n, (name, value)) in fx.links.iter().enumerate() { + assert_eq!(mg.dataset(name).unwrap().read_i64().unwrap(), vec![*value]); + check_time(n); + } + assert!(mg.group("nope").is_err_and(|e| is_not_found(&e))); + + let l = LazyFile::open_mmap(&fx.path).unwrap(); + let lg = l.group("g").unwrap(); + for (n, (name, value)) in fx.links.iter().enumerate() { + assert_eq!(lg.dataset(name).unwrap().read_i64().unwrap(), vec![*value]); + check_time(n); + } + let lx = l.dataset("x").unwrap(); + assert!( + matches!(lx.attr("k155448").unwrap(), Some(AttrValue::I64(v)) if v == fx.attrs["k155448"]) + ); + assert!( + lg.dataset(&fx.missing_links[0]) + .is_err_and(|e| is_not_found(&e)) + ); +} + +fn g_address(f: &File) -> u64 { + f.root() + .entries() + .unwrap() + .into_iter() + .find(|(n, _)| n == "g") + .unwrap() + .1 +} + +/// Every kind of link, looked up by name in a dense group (through the name +/// index) and in a compact one, opens what h5py opens and nothing it cannot: +/// hard links, soft links (absolute, relative, to a group), and not a +/// dangling soft link, an external link or a missing name. +#[test] +fn links_of_every_kind_resolve_by_name_as_in_h5py() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("links.h5").display().to_string(); + // For each group and name: "dataset ", "group", or "none" as + // h5py sees it. + let out = run_python(&format!( + "import h5py, json, numpy as np\n\ + with h5py.File(r'{path}', 'w', libver='latest') as f:\n\ + \x20 for gname, n in (('dense', 20), ('compact', 2)):\n\ + \x20 g = f.create_group(gname)\n\ + \x20 for i in range(n):\n\ + \x20 g.create_dataset(f'd{{i}}', data=np.int64(100 + i))\n\ + \x20 s = g.create_group('sub')\n\ + \x20 s.create_dataset('x', data=np.int64(7))\n\ + \x20 g['abs'] = h5py.SoftLink(f'/{{gname}}/d1')\n\ + \x20 g['rel'] = h5py.SoftLink('sub/x')\n\ + \x20 g['tosub'] = h5py.SoftLink('sub')\n\ + \x20 g['dangling'] = h5py.SoftLink('/nowhere')\n\ + \x20 g['ext'] = h5py.ExternalLink('other.h5', '/y')\n\ + names = ['d0', 'd1', 'sub', 'abs', 'rel', 'tosub', 'dangling', 'ext', 'nope', '']\n\ + seen = {{}}\n\ + with h5py.File(r'{path}', 'r') as f:\n\ + \x20 for gname in ('dense', 'compact'):\n\ + \x20 g = f[gname]\n\ + \x20 for n in names:\n\ + \x20 try:\n\ + \x20 o = g[n] if n else None\n\ + \x20 except (KeyError, OSError):\n\ + \x20 o = None\n\ + \x20 if isinstance(o, h5py.Dataset):\n\ + \x20 seen[f'{{gname}}/{{n}}'] = f'dataset {{int(o[()])}}'\n\ + \x20 elif isinstance(o, h5py.Group):\n\ + \x20 seen[f'{{gname}}/{{n}}'] = 'group'\n\ + \x20 else:\n\ + \x20 seen[f'{{gname}}/{{n}}'] = 'none'\n\ + print(json.dumps(seen))", + )); + let seen: BTreeMap = serde_json::from_str(&out).unwrap(); + assert_eq!(seen.len(), 20); + + let f = File::open(&path).unwrap(); + // The dense group's links are in a heap, the compact group's in its + // header. + for (gname, dense) in [("dense", true), ("compact", false)] { + let g = f.group(gname).unwrap(); + lookup_stats::reset(); + g.dataset("d0").unwrap(); + assert_eq!(lookup_stats::heap_objects_read() > 0, dense, "{gname}"); + } + let m = MmapFile::open(&path).unwrap(); + let l = LazyFile::open_mmap(&path).unwrap(); + for (key, want) in &seen { + let (gname, name) = key.split_once('/').unwrap(); + let got = { + let g = f.group(gname).unwrap(); + match (g.dataset(name), g.group(name)) { + (Ok(ds), _) => format!("dataset {}", ds.read_i64().unwrap()[0]), + (Err(clawhdf5::Error::NotADataset(_)), Ok(sub)) => { + // A group: it has the child `x` (checks the address). + assert!(sub.dataset("x").is_ok() || name == "sub" || name == "tosub"); + "group".to_string() + } + (Err(e), Err(e2)) => { + assert!(is_not_found(&e) && is_not_found(&e2), "{key}: {e:?} / {e2:?}"); + "none".to_string() + } + (Err(e), Ok(_)) => panic!("{key}: dataset {e:?} but group ok"), + } + }; + assert_eq!(&got, want, "{key}"); + // The other readers agree, and a path through the group resolves the + // same way. + let mg = m.group(gname).unwrap(); + let lg = l.group(gname).unwrap(); + match want.strip_prefix("dataset ") { + Some(v) => { + let v: i64 = v.parse().unwrap(); + assert_eq!(mg.dataset(name).unwrap().read_i64().unwrap(), vec![v]); + assert_eq!(lg.dataset(name).unwrap().read_i64().unwrap(), vec![v]); + let ds = f.dataset(&format!("/{gname}/{name}")).unwrap(); + assert_eq!(ds.read_i64().unwrap(), vec![v], "{key}"); + } + None if want == "group" => { + assert!(mg.group(name).unwrap().dataset("x").is_ok(), "{key}"); + assert!(lg.group(name).unwrap().dataset("x").is_ok(), "{key}"); + let ds = f.dataset(&format!("/{gname}/{name}/x")).unwrap(); + assert_eq!(ds.read_i64().unwrap(), vec![7]); + } + None => { + assert!(mg.dataset(name).is_err_and(|e| is_not_found(&e)), "{key}"); + assert!(lg.group(name).is_err_and(|e| is_not_found(&e)), "{key}"); + } + } + } +} From b41583113a883db41a14e5442c38bc29c1644d23 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 13:33:24 -0500 Subject: [PATCH 03/10] format: no truncating u64 -> usize casts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every `u64 as usize` cast in clawhdf5-format (115 on wasm32) now goes through addr::to_usize for values read from the file — addresses, lengths, counts, dimensions: FormatError::Overflow where the value does not fit instead of wrapping onto another part of the file on a 32-bit target — or addr::saturating_usize for counts bounded by something in memory (codec progress counters, writer sizes), which fail a bounds check or allocation rather than wrap. A chunk whose offset does not fit lies outside the dataset and is skipped; partial reads treat such an offset as out of the buffers. On 64-bit targets nothing changes. scripts/check-32bit-casts.sh (run by ci-test.sh) lints the wasm32 build with clippy's cast_possible_truncation and fails on any u64 -> usize finding; before this commit it listed 115. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-format/src/addr.rs | 22 +++++++++ crates/clawhdf5-format/src/btree_v1.rs | 8 +++- crates/clawhdf5-format/src/btree_v2_write.rs | 7 ++- crates/clawhdf5-format/src/chunk_index.rs | 11 ++++- crates/clawhdf5-format/src/chunked_read.rs | 46 +++++++++++++------ crates/clawhdf5-format/src/chunked_write.rs | 23 +++++----- crates/clawhdf5-format/src/data_layout.rs | 5 +- crates/clawhdf5-format/src/data_read.rs | 17 +++---- crates/clawhdf5-format/src/ea_writer.rs | 3 +- .../clawhdf5-format/src/extensible_array.rs | 11 +++-- crates/clawhdf5-format/src/file_writer.rs | 20 ++++---- crates/clawhdf5-format/src/fill_value.rs | 9 +++- crates/clawhdf5-format/src/filters.rs | 16 +++++-- crates/clawhdf5-format/src/filters_blosc2.rs | 3 +- crates/clawhdf5-format/src/filters_bzip2.rs | 7 +-- crates/clawhdf5-format/src/fixed_array.rs | 5 +- crates/clawhdf5-format/src/group_v1.rs | 11 +++-- crates/clawhdf5-format/src/link_message.rs | 3 +- crates/clawhdf5-format/src/local_heap.rs | 7 +-- crates/clawhdf5-format/src/object_header.rs | 11 +++-- crates/clawhdf5-format/src/partial_read.rs | 15 ++++-- crates/clawhdf5-format/src/selection.rs | 9 ++-- crates/clawhdf5-format/src/shared_message.rs | 9 ++-- crates/clawhdf5-format/src/vds.rs | 13 +++--- crates/clawhdf5-format/src/vl_data.rs | 7 +-- scripts/check-32bit-casts.sh | 35 ++++++++++++++ scripts/ci-test.sh | 2 + 27 files changed, 236 insertions(+), 99 deletions(-) create mode 100755 scripts/check-32bit-casts.sh diff --git a/crates/clawhdf5-format/src/addr.rs b/crates/clawhdf5-format/src/addr.rs index ec25e3d..09c2cef 100644 --- a/crates/clawhdf5-format/src/addr.rs +++ b/crates/clawhdf5-format/src/addr.rs @@ -23,6 +23,19 @@ pub fn to_usize(value: u64) -> Result { usize::try_from(value).map_err(|_| too_large(value)) } +/// A count or offset into an in-memory buffer (a codec's progress counter, +/// a size the writer computed from data it holds) as a `usize`, saturating +/// at `usize::MAX` instead of truncating. +/// +/// For values that are bounded by the length of something in memory, so +/// always fit; if one ever did not, a saturated index fails its bounds check +/// or allocation instead of silently addressing the wrong bytes. A value +/// read from the file uses [`to_usize`]. +#[inline] +pub fn saturating_usize(value: u64) -> usize { + usize::try_from(value).unwrap_or(usize::MAX) +} + #[cold] #[inline(never)] fn too_large(value: u64) -> FormatError { @@ -42,6 +55,15 @@ mod tests { assert_eq!(to_usize(usize::MAX as u64), Ok(usize::MAX)); } + #[test] + fn saturating_conversion_never_wraps() { + assert_eq!(saturating_usize(0), 0); + assert_eq!(saturating_usize(0x1234), 0x1234); + assert_eq!(saturating_usize(usize::MAX as u64), usize::MAX); + // Past usize::MAX (32-bit targets) or at u64::MAX: saturates. + assert_eq!(saturating_usize(u64::MAX), usize::MAX); + } + #[test] fn values_past_usize_max_are_an_error_not_truncated() { // Only reachable where usize is narrower than u64; on a 64-bit host diff --git a/crates/clawhdf5-format/src/btree_v1.rs b/crates/clawhdf5-format/src/btree_v1.rs index b652dcc..0e9b7fe 100644 --- a/crates/clawhdf5-format/src/btree_v1.rs +++ b/crates/clawhdf5-format/src/btree_v1.rs @@ -3,6 +3,7 @@ #[cfg(not(feature = "std"))] use alloc::vec::Vec; +use crate::addr::to_usize; use crate::error::FormatError; /// A parsed B-tree v1 node. @@ -164,7 +165,12 @@ fn collect_symbol_table_nodes_inner( return Err(FormatError::NestingDepthExceeded); } - let node = BTreeV1Node::parse(file_data, btree_address as usize, offset_size, length_size)?; + let node = BTreeV1Node::parse( + file_data, + to_usize(btree_address)?, + offset_size, + length_size, + )?; if node.node_type != 0 { return Err(FormatError::InvalidBTreeNodeType(node.node_type)); diff --git a/crates/clawhdf5-format/src/btree_v2_write.rs b/crates/clawhdf5-format/src/btree_v2_write.rs index 795fb80..38fa7c1 100644 --- a/crates/clawhdf5-format/src/btree_v2_write.rs +++ b/crates/clawhdf5-format/src/btree_v2_write.rs @@ -6,6 +6,7 @@ //! libhdf5 uses (`H5B2__hdr_init`) and the reader decodes pointers with, so //! the pointer widths the writer encodes are the ones every reader expects. +use crate::addr::saturating_usize; #[cfg(not(feature = "std"))] use alloc::{format, vec, vec::Vec}; @@ -105,7 +106,9 @@ pub(crate) fn build_btree_v2( first_node: addr + hdr_len as u64, nodes: Vec::new(), }; - let root = (n > 0).then(|| w.node(depth, 0, n as usize)).transpose()?; + let root = (n > 0) + .then(|| w.node(depth, 0, saturating_usize(n))) + .transpose()?; let mut out = Vec::with_capacity(hdr_len + w.nodes.len() * p.node_size as usize); out.extend_from_slice(b"BTHD"); @@ -201,7 +204,7 @@ impl TreeWriter<'_> { "cannot spread {n} B-tree v2 records over {k} children at depth {depth}" ))); } - let k = k as usize; + let k = saturating_usize(k); let in_children = n - (k - 1); let (base, extra) = (in_children / k, in_children % k); diff --git a/crates/clawhdf5-format/src/chunk_index.rs b/crates/clawhdf5-format/src/chunk_index.rs index c307572..706bad0 100644 --- a/crates/clawhdf5-format/src/chunk_index.rs +++ b/crates/clawhdf5-format/src/chunk_index.rs @@ -18,6 +18,7 @@ use alloc::collections::BTreeMap; #[cfg(feature = "std")] use std::collections::HashMap; +use crate::addr::to_usize; use crate::chunk_cache::ChunkCoord; use crate::chunked_read::ChunkInfo; @@ -167,7 +168,15 @@ impl ChunkLayout { for (_coord, ci) in index.iter() { let coord: ChunkCoord = ci.offsets.iter().take(rank).copied().collect(); - let chunk_offsets: Vec = coord.iter().map(|&o| o as usize).collect(); + // `ds_dims` are `usize`: a chunk at an offset past `usize::MAX` + // (only on a 32-bit target) lies outside the dataset. + let Ok(chunk_offsets) = coord + .iter() + .map(|&o| to_usize(o)) + .collect::, _>>() + else { + continue; + }; let copies = if rank == 0 { // Scalar dataset — single copy diff --git a/crates/clawhdf5-format/src/chunked_read.rs b/crates/clawhdf5-format/src/chunked_read.rs index 6dda3cb..e249168 100644 --- a/crates/clawhdf5-format/src/chunked_read.rs +++ b/crates/clawhdf5-format/src/chunked_read.rs @@ -6,6 +6,7 @@ extern crate alloc; #[cfg(not(feature = "std"))] use alloc::{format, vec, vec::Vec}; +use crate::addr::to_usize; #[cfg(feature = "std")] use crate::chunk_cache::{CacheAlignedBuffer, ChunkCache}; use crate::data_layout::DataLayout; @@ -265,7 +266,7 @@ fn fill_from_chunks( ))); } let offsets = &c.offsets[..rank]; - let c_addr = c.address as usize; + let c_addr = to_usize(c.address)?; let size = c.chunk_size as usize; ensure_len(file_data, c_addr, size)?; let raw = &file_data[c_addr..c_addr + size]; @@ -825,7 +826,7 @@ fn parse_chunk_node( return Err(FormatError::NestingDepthExceeded); } - let offset = btree_address as usize; + let offset = to_usize(btree_address)?; let os = offset_size as usize; // Parse B-tree v1 header @@ -945,7 +946,8 @@ pub fn generate_implicit_chunks( } let total_chunks: u64 = num_chunks_per_dim.iter().product(); - let mut chunks = Vec::with_capacity(total_chunks as usize); + // A capacity hint only (a count past `usize::MAX` could not be pushed). + let mut chunks = Vec::with_capacity(usize::try_from(total_chunks).unwrap_or(0)); for linear_idx in 0..total_chunks { let mut offsets = vec![0u64; rank]; let mut remaining = linear_idx; @@ -993,7 +995,7 @@ fn read_btree_v2_chunks( use crate::btree_v2::{BTreeV2Header, collect_btree_v2_records}; let bad = |what: &str| FormatError::ChunkedReadError(format!("B-tree v2 chunk index: {what}")); - let header = BTreeV2Header::parse(file_data, addr as usize, offset_size, length_size)?; + let header = BTreeV2Header::parse(file_data, to_usize(addr)?, offset_size, length_size)?; let rank = chunk_dims.len(); let os = offset_size as usize; let record_size = header.record_size as usize; @@ -1122,7 +1124,11 @@ pub fn list_chunks( // Both v3 and v4 include element size as last dim (rank+1) let (rank, chunk_dims) = chunk_geometry(chunk_dimensions, version, dataspace, elem_size)?; - let ds_dims: Vec = dataspace.dimensions.iter().map(|&d| d as usize).collect(); + let ds_dims: Vec = dataspace + .dimensions + .iter() + .map(|&d| to_usize(d)) + .collect::>()?; // Collect chunks based on version and index type let mut chunks = match (version, chunk_index_type) { @@ -1158,7 +1164,7 @@ pub fn list_chunks( // Fixed Array — use spatial chunk dims only let spatial_chunk_dims: &[u32] = &chunk_dimensions[..rank]; let header = - FixedArrayHeader::parse(file_data, addr as usize, offset_size, length_size)?; + FixedArrayHeader::parse(file_data, to_usize(addr)?, offset_size, length_size)?; read_fixed_array_chunks( file_data, &header, @@ -1174,7 +1180,7 @@ pub fn list_chunks( // Extensible Array — use spatial chunk dims only let spatial_chunk_dims: &[u32] = &chunk_dimensions[..rank]; let header = - ExtensibleArrayHeader::parse(file_data, addr as usize, offset_size, length_size)?; + ExtensibleArrayHeader::parse(file_data, to_usize(addr)?, offset_size, length_size)?; read_extensible_array_chunks( file_data, &header, @@ -1349,7 +1355,11 @@ pub(crate) fn read_chunked_full( // dimension the total is 0 even if other dimensions are huge. return Ok(output); } - let ds_dims: Vec = dataspace.dimensions.iter().map(|&d| d as usize).collect(); + let ds_dims: Vec = dataspace + .dimensions + .iter() + .map(|&d| to_usize(d)) + .collect::>()?; let placer = ChunkPlacer::new(&chunk_dims, &ds_dims, elem_size); let chunk_total_bytes = checked_chunk_byte_len(&chunk_dims, elem_size)?; // Chunks are cached only when the whole dataset fits: pushing a larger @@ -1603,7 +1613,11 @@ pub fn read_chunked_data_sweep( check_chunk_element_size(layout, datatype, offset_size)?; let elem_size = datatype.type_size() as usize; let (rank, chunk_dims) = chunk_geometry(chunk_dimensions, version, dataspace, elem_size)?; - let ds_dims: Vec = dataspace.dimensions.iter().map(|&d| d as usize).collect(); + let ds_dims: Vec = dataspace + .dimensions + .iter() + .map(|&d| to_usize(d)) + .collect::>()?; // The per-file cache is shared across datasets (and threads); every // lookup is keyed by this dataset's chunk-index address, so another @@ -1659,7 +1673,7 @@ pub fn read_chunked_data_sweep( cached } else { // Decompress from file - let c_addr = chunk_info.address as usize; + let c_addr = to_usize(chunk_info.address)?; let size = chunk_info.chunk_size as usize; ensure_len(file_data, c_addr, size)?; let raw_chunk = &file_data[c_addr..c_addr + size]; @@ -1682,8 +1696,8 @@ pub fn read_chunked_data_sweep( .offsets .iter() .take(rank) - .map(|&o| o as usize) - .collect(); + .map(|&o| to_usize(o)) + .collect::>()?; if rank == 0 { let copy_len = decompressed.len().min(output.len()); @@ -1743,7 +1757,11 @@ pub fn read_chunked_data_indexed( check_chunk_element_size(layout, datatype, offset_size)?; let elem_size = datatype.type_size() as usize; let (rank, chunk_dims) = chunk_geometry(chunk_dimensions, version, dataspace, elem_size)?; - let ds_dims: Vec = dataspace.dimensions.iter().map(|&d| d as usize).collect(); + let ds_dims: Vec = dataspace + .dimensions + .iter() + .map(|&d| to_usize(d)) + .collect::>()?; // Chunk index and assembly plan for this dataset, built on first access // and kept per dataset (keyed by chunk-index address) in the shared cache. @@ -1776,7 +1794,7 @@ pub fn read_chunked_data_indexed( if let Some(cached) = cache.get_decompressed_in(addr, coord) { chunk_buffers.push(cached); } else { - let c_addr = *file_offset as usize; + let c_addr = to_usize(*file_offset)?; let size = *file_size as usize; ensure_len(file_data, c_addr, size)?; let raw_chunk = &file_data[c_addr..c_addr + size]; diff --git a/crates/clawhdf5-format/src/chunked_write.rs b/crates/clawhdf5-format/src/chunked_write.rs index 2a8bd09..cf5c958 100644 --- a/crates/clawhdf5-format/src/chunked_write.rs +++ b/crates/clawhdf5-format/src/chunked_write.rs @@ -3,6 +3,7 @@ #[cfg(not(feature = "std"))] extern crate alloc; +use crate::addr::saturating_usize; #[cfg(not(feature = "std"))] use alloc::{format, vec, vec::Vec}; @@ -414,18 +415,18 @@ pub fn split_into_chunks( // Dataset strides (row-major) let mut ds_strides = vec![1usize; rank]; for i in (0..rank.saturating_sub(1)).rev() { - ds_strides[i] = ds_strides[i + 1] * shape[i + 1] as usize; + ds_strides[i] = ds_strides[i + 1] * saturating_usize(shape[i + 1]); } // Chunk strides let mut chunk_strides = vec![1usize; rank]; for i in (0..rank.saturating_sub(1)).rev() { - chunk_strides[i] = chunk_strides[i + 1] * chunk_dims[i + 1] as usize; + chunk_strides[i] = chunk_strides[i + 1] * saturating_usize(chunk_dims[i + 1]); } - let chunk_total_elements: usize = chunk_dims.iter().map(|&d| d as usize).product(); + let chunk_total_elements: usize = chunk_dims.iter().map(|&d| saturating_usize(d)).product(); - let mut result = Vec::with_capacity(total_chunks as usize); + let mut result = Vec::with_capacity(saturating_usize(total_chunks)); for linear_idx in 0..total_chunks { // Convert linear index to chunk grid coordinates @@ -453,8 +454,8 @@ pub fn split_into_chunks( let coord_in_chunk = remaining_idx / chunk_strides[d]; remaining_idx %= chunk_strides[d]; - let global_coord = offsets[d] as usize + coord_in_chunk; - if global_coord >= shape[d] as usize { + let global_coord = saturating_usize(offsets[d]) + coord_in_chunk; + if global_coord >= saturating_usize(shape[d]) { out_of_bounds = true; break; } @@ -1036,7 +1037,7 @@ impl ChunkIndexPlan { Ok(Self::SingleChunk) } else { let grid = ChunkGrid::fixed_array(shape, Some(max), chunk_dims)?; - Ok(Self::FixedArray(grid, nslots as usize)) + Ok(Self::FixedArray(grid, saturating_usize(nslots))) } } 1 => Ok(Self::ExtensibleArray(ChunkGrid::extensible_array( @@ -1251,7 +1252,7 @@ pub fn write_selection_to_buffer( let rank = dims.len(); let mut ds_strides = vec![1usize; rank]; for i in (0..rank.saturating_sub(1)).rev() { - ds_strides[i] = ds_strides[i + 1] * dims[i + 1] as usize; + ds_strides[i] = ds_strides[i + 1] * saturating_usize(dims[i + 1]); } let mut src_offset = 0usize; @@ -1301,7 +1302,7 @@ pub fn write_selection_to_buffer( buffer, new_data, src_offset, - current_ds_offset + coord as usize * ds_strides[d], + current_ds_offset + saturating_usize(coord) * ds_strides[d], ); } } @@ -1328,14 +1329,14 @@ pub fn write_selection_to_buffer( let rank = dims.len(); let mut ds_strides = vec![1usize; rank]; for i in (0..rank.saturating_sub(1)).rev() { - ds_strides[i] = ds_strides[i + 1] * dims[i + 1] as usize; + ds_strides[i] = ds_strides[i + 1] * saturating_usize(dims[i + 1]); } for (pi, pt) in pts.iter().enumerate() { let flat: usize = pt .iter() .zip(ds_strides.iter()) - .map(|(&p, &s)| p as usize * s) + .map(|(&p, &s)| saturating_usize(p) * s) .sum(); let dst = flat * elem_size; let src = pi * elem_size; diff --git a/crates/clawhdf5-format/src/data_layout.rs b/crates/clawhdf5-format/src/data_layout.rs index 59a9065..c9b93c7 100644 --- a/crates/clawhdf5-format/src/data_layout.rs +++ b/crates/clawhdf5-format/src/data_layout.rs @@ -6,6 +6,7 @@ use alloc::{format, string::String, vec::Vec}; #[cfg(feature = "std")] use std::string::String; +use crate::addr::to_usize; use crate::error::FormatError; /// A single VDS (Virtual Dataset) source mapping. @@ -207,7 +208,7 @@ pub fn parse_vds_mappings( "VDS mapping shares a name with a later entry".into(), )); } - Ok(idx as usize) + to_usize(idx) }; let source_file = if flags & VDS_SOURCE_SAME_FILE != 0 { @@ -320,7 +321,7 @@ impl DataLayout { { let coll = crate::global_heap::GlobalHeapCollection::parse( file_data, - addr as usize, + to_usize(addr)?, length_size, )?; let obj = coll.get_object(*global_heap_index as u16).ok_or( diff --git a/crates/clawhdf5-format/src/data_read.rs b/crates/clawhdf5-format/src/data_read.rs index ef73a04..bcf1a18 100644 --- a/crates/clawhdf5-format/src/data_read.rs +++ b/crates/clawhdf5-format/src/data_read.rs @@ -6,6 +6,7 @@ use alloc::{collections::BTreeMap, format, string::String, vec, vec::Vec}; #[cfg(feature = "std")] use std::collections::BTreeMap; +use crate::addr::to_usize; #[cfg(feature = "std")] use crate::chunk_cache::ChunkCache; use crate::chunked_read::read_chunked_data; @@ -117,7 +118,7 @@ pub fn read_raw_data_zerocopy<'a>( dataspace: &Dataspace, datatype: &Datatype, ) -> Result, FormatError> { - let num_elements = dataspace.num_elements() as usize; + let num_elements = to_usize(dataspace.num_elements())?; let elem_size = datatype.type_size() as usize; let expected_size = num_elements.checked_mul(elem_size).ok_or_else(|| { FormatError::Overflow(format!( @@ -128,7 +129,7 @@ pub fn read_raw_data_zerocopy<'a>( match layout { DataLayout::Contiguous { address, size } => { let addr = address.ok_or(FormatError::NoDataAllocated)?; - let addr = addr as usize; + let addr = to_usize(addr)?; let sz = contiguous_read_len(*size, expected_size)?; ensure_len(file_data, addr, sz)?; Ok(Some(&file_data[addr..addr + sz])) @@ -219,7 +220,7 @@ fn read_raw_data_full_impl( length_size: u8, resolver: Option<&VdsSourceResolver>, ) -> Result, FormatError> { - let num_elements = dataspace.num_elements() as usize; + let num_elements = to_usize(dataspace.num_elements())?; let elem_size = datatype.type_size() as usize; let expected_size = num_elements.checked_mul(elem_size).ok_or_else(|| { FormatError::Overflow(format!( @@ -239,7 +240,7 @@ fn read_raw_data_full_impl( } DataLayout::Contiguous { address, size } => { let addr = address.ok_or(FormatError::NoDataAllocated)?; - let addr = addr as usize; + let addr = to_usize(addr)?; let sz = contiguous_read_len(*size, expected_size)?; ensure_len(file_data, addr, sz)?; let mut out = crate::bulk_alloc::vec_for_bulk(sz); @@ -582,7 +583,7 @@ pub fn extract_selection_from_buffer( let rank = dims.len(); let mut ds_strides = vec![1usize; rank]; for i in (0..rank.saturating_sub(1)).rev() { - ds_strides[i] = ds_strides[i + 1] * dims[i + 1] as usize; + ds_strides[i] = ds_strides[i + 1] * to_usize(dims[i + 1])?; } let mut output = Vec::with_capacity(pts.len() * elem_size); @@ -590,8 +591,8 @@ pub fn extract_selection_from_buffer( let flat: usize = pt .iter() .zip(ds_strides.iter()) - .map(|(&p, &s)| p as usize * s) - .sum(); + .map(|(&p, &s)| Ok(to_usize(p)? * s)) + .sum::>()?; let src = flat * elem_size; if src + elem_size <= full_data.len() { output.extend_from_slice(&full_data[src..src + elem_size]); @@ -1341,7 +1342,7 @@ pub fn read_compound_fields( let mut fields = Vec::with_capacity(members.len()); for m in members { let field_size = m.datatype.type_size() as usize; - let offset = m.byte_offset as usize; + let offset = to_usize(m.byte_offset)?; if offset .checked_add(field_size) .is_none_or(|end| end > elem_size) diff --git a/crates/clawhdf5-format/src/ea_writer.rs b/crates/clawhdf5-format/src/ea_writer.rs index f669534..de7859c 100644 --- a/crates/clawhdf5-format/src/ea_writer.rs +++ b/crates/clawhdf5-format/src/ea_writer.rs @@ -3,6 +3,7 @@ #[cfg(not(feature = "std"))] extern crate alloc; +use crate::addr::saturating_usize; #[cfg(not(feature = "std"))] use alloc::{vec, vec::Vec}; @@ -247,7 +248,7 @@ pub fn build_extensible_array_at( // Header (EAHD). The six statistics are, in order: super blocks, their // bytes, data blocks, their bytes, max index set, elements realised. - let mut out = Vec::with_capacity((cursor - ea_base_address) as usize); + let mut out = Vec::with_capacity(saturating_usize(cursor - ea_base_address)); out.extend_from_slice(b"EAHD"); out.push(0); // version out.push(client_id); diff --git a/crates/clawhdf5-format/src/extensible_array.rs b/crates/clawhdf5-format/src/extensible_array.rs index ba1c822..9256326 100644 --- a/crates/clawhdf5-format/src/extensible_array.rs +++ b/crates/clawhdf5-format/src/extensible_array.rs @@ -9,6 +9,7 @@ extern crate alloc; #[cfg(not(feature = "std"))] use alloc::{format, vec, vec::Vec}; +use crate::addr::to_usize; use crate::chunk_grid::ChunkGrid; use crate::chunked_read::ChunkInfo; use crate::error::FormatError; @@ -451,7 +452,7 @@ pub fn read_extensible_array_chunks( // Parse index block (EAIB): signature(4) + version(1) + client_id(1) // + header address(offset_size), then the inline elements, then the // direct data block addresses, then the super block addresses. - let ib_offset = header.index_block_address as usize; + let ib_offset = to_usize(header.index_block_address)?; let ib_header_size = 4 + 1 + 1 + os; ensure_len(file_data, ib_offset, ib_header_size)?; @@ -463,7 +464,7 @@ pub fn read_extensible_array_chunks( let mut pos = ib_offset + ib_header_size; let mut chunks = Vec::new(); - let total_elements = header.num_elements as usize; + let total_elements = to_usize(header.num_elements)?; let dmin = header.min_dblk_nelmts as usize; if dmin == 0 || !dmin.is_power_of_two() { @@ -563,7 +564,7 @@ pub fn read_extensible_array_chunks( } chunks.extend(read_data_block_elements( file_data, - addr as usize, + to_usize(addr)?, dblk_nelmts, header, offset_size, @@ -592,7 +593,7 @@ pub fn read_extensible_array_chunks( if !is_undefined_addr(sb_addr, offset_size) { chunks.extend(read_super_block( file_data, - sb_addr as usize, + to_usize(sb_addr)?, ndblks, dblk_nelmts, header, @@ -676,7 +677,7 @@ fn read_super_block( if !is_undefined_addr(addr, offset_size) { chunks.extend(read_data_block_elements( file_data, - addr as usize, + to_usize(addr)?, dblk_nelmts, header, offset_size, diff --git a/crates/clawhdf5-format/src/file_writer.rs b/crates/clawhdf5-format/src/file_writer.rs index 2568582..1a1af83 100644 --- a/crates/clawhdf5-format/src/file_writer.rs +++ b/crates/clawhdf5-format/src/file_writer.rs @@ -3,6 +3,7 @@ //! Produces valid HDF5 files with v3 superblock, v2 object headers, //! link messages, contiguous datasets, inline and dense attributes. +use crate::addr::saturating_usize; #[cfg(not(feature = "std"))] use alloc::{format, vec, vec::Vec}; @@ -336,7 +337,7 @@ pub(crate) fn build_single_block_fractal_heap( // An object must fit one direct block: the writer has no huge-object // path, and libhdf5 cannot read an object that overruns its block. - let max_managed = max_direct_block_size as usize - dblock_header_size; + let max_managed = saturating_usize(max_direct_block_size) - dblock_header_size; if let Some(big) = serialized.iter().find(|s| s.len() > max_managed) { return Err(FormatError::SerializationError(format!( "a {}-byte message cannot go in dense storage: a fractal heap \ @@ -392,7 +393,7 @@ pub(crate) fn build_single_block_fractal_heap( let dblock_addr = frhp_addr + frhp_size as u64; let btree_addr = dblock_addr + starting_block_size; - let data_space = starting_block_size as usize - dblock_header_size; + let data_space = saturating_usize(starting_block_size) - dblock_header_size; let free_space = data_space - total_data_size; // Build fractal heap header @@ -428,7 +429,7 @@ pub(crate) fn build_single_block_fractal_heap( debug_assert_eq!(frhp.len(), frhp_size); // Build direct block: header (with checksum) + data + padding - let mut dblock = Vec::with_capacity(starting_block_size as usize); + let mut dblock = Vec::with_capacity(saturating_usize(starting_block_size)); dblock.extend_from_slice(b"FHDB"); dblock.push(0); // version write_offset(&mut dblock, frhp_addr, OFFSET_SIZE); @@ -446,12 +447,12 @@ pub(crate) fn build_single_block_fractal_heap( } // Pad to full block size - dblock.resize(starting_block_size as usize, 0); + dblock.resize(saturating_usize(starting_block_size), 0); // Checksum: computed over entire block with checksum field zeroed let dblock_checksum = crate::checksum::jenkins_lookup3(&dblock); dblock[cksum_pos..cksum_pos + 4].copy_from_slice(&dblock_checksum.to_le_bytes()); - debug_assert_eq!(dblock.len(), starting_block_size as usize); + debug_assert_eq!(dblock.len(), saturating_usize(starting_block_size)); // Build heap IDs let heap_ids: Vec> = obj_offsets @@ -706,7 +707,7 @@ impl HeapIndirectBlock { let cksum_pos = out.len(); out.extend_from_slice(&[0u8; 4]); // checksum placeholder out.extend_from_slice(&b.data); - out.resize(d + b.size as usize, 0); + out.resize(d + saturating_usize(b.size), 0); let cksum = crate::checksum::jenkins_lookup3(&out[d..]); out[cksum_pos..cksum_pos + 4].copy_from_slice(&cksum.to_le_bytes()); child += b.size; @@ -740,7 +741,7 @@ impl HeapPacker<'_> { nrows: Option, ) -> Result { let geom = self.geom; - let width = geom.width as usize; + let width = saturating_usize(geom.width); let mut slots = Vec::new(); let mut off = heap_offset; let mut row = 0usize; @@ -763,7 +764,8 @@ impl HeapPacker<'_> { // A child whose biggest direct block cannot hold the // next object is skipped whole, not walked. let biggest = geom.row_size(child_rows.min(geom.max_direct_rows()) - 1); - if self.objects[self.next].len() > (biggest as usize - geom.dblock_header_size) + if self.objects[self.next].len() + > (saturating_usize(biggest) - geom.dblock_header_size) { slots.push(HeapSlot::Empty); off += size; @@ -794,7 +796,7 @@ impl HeapPacker<'_> { /// objects as fit; leave it unallocated if not even the next one does. fn fill_direct(&mut self, heap_offset: u64, size: u64) -> HeapSlot { let header = self.geom.dblock_header_size; - let capacity = size as usize - header; + let capacity = saturating_usize(size) - header; let mut data = Vec::new(); while let Some(obj) = self.objects.get(self.next) { if data.len() + obj.len() > capacity { diff --git a/crates/clawhdf5-format/src/fill_value.rs b/crates/clawhdf5-format/src/fill_value.rs index 3b18790..d8c34f3 100644 --- a/crates/clawhdf5-format/src/fill_value.rs +++ b/crates/clawhdf5-format/src/fill_value.rs @@ -12,6 +12,7 @@ #[cfg(not(feature = "std"))] use alloc::{format, vec, vec::Vec}; +use crate::addr::to_usize; use crate::chunked_read::{alloc_output, checked_byte_len, list_chunks}; use crate::data_layout::DataLayout; use crate::dataspace::Dataspace; @@ -256,7 +257,11 @@ pub fn apply_to_unallocated_chunks( length_size, )?; let rank = chunk_dims.len(); - let ds_dims: Vec = dataspace.dimensions.iter().map(|&d| d as usize).collect(); + let ds_dims: Vec = dataspace + .dimensions + .iter() + .map(|&d| to_usize(d)) + .collect::>()?; if rank == 0 || ds_dims.len() != rank || chunk_dims.contains(&0) { return Ok(()); } @@ -288,7 +293,7 @@ pub fn apply_to_unallocated_chunks( let mut cell = 0usize; let mut in_range = true; for d in 0..rank { - let coord = chunk.offsets[d] as usize / chunk_dims[d]; + let coord = to_usize(chunk.offsets[d])? / chunk_dims[d]; if coord >= grid[d] { in_range = false; break; diff --git a/crates/clawhdf5-format/src/filters.rs b/crates/clawhdf5-format/src/filters.rs index 40d6cd9..672649b 100644 --- a/crates/clawhdf5-format/src/filters.rs +++ b/crates/clawhdf5-format/src/filters.rs @@ -3,6 +3,8 @@ #[cfg(not(feature = "std"))] extern crate alloc; +#[cfg(feature = "deflate")] +use crate::addr::saturating_usize; #[cfg(not(feature = "std"))] use alloc::{boxed::Box, format, vec, vec::Vec}; @@ -1114,7 +1116,11 @@ fn inflate_bounded_into( loop { let (in_before, out_before) = (inflater.total_in(), inflater.total_out()); let status = inflater - .decompress_vec(&data[in_before as usize..], out, FlushDecompress::Finish) + .decompress_vec( + &data[saturating_usize(in_before)..], + out, + FlushDecompress::Finish, + ) .map_err(|e| format!("deflate: {e}"))?; if out.len() > limit { return Err("deflate: output exceeds size limit".into()); @@ -1132,7 +1138,7 @@ fn inflate_bounded_into( } Status::Ok | Status::BufError => { // Room left, so the decoder stopped for want of input. - if inflater.total_in() as usize >= data.len() + if saturating_usize(inflater.total_in()) >= data.len() || (inflater.total_in(), inflater.total_out()) == (in_before, out_before) { return Err("deflate: truncated stream".into()); @@ -1232,7 +1238,11 @@ pub(crate) fn deflate_bounded(data: &[u8], level: u32) -> Result, String loop { let (in_before, out_before) = (deflater.total_in(), deflater.total_out()); let status = deflater - .compress_vec(&data[in_before as usize..], &mut out, FlushCompress::Finish) + .compress_vec( + &data[saturating_usize(in_before)..], + &mut out, + FlushCompress::Finish, + ) .map_err(|e| format!("deflate: {e}"))?; match status { Status::StreamEnd => return Ok(out), diff --git a/crates/clawhdf5-format/src/filters_blosc2.rs b/crates/clawhdf5-format/src/filters_blosc2.rs index f4d1690..a609a26 100644 --- a/crates/clawhdf5-format/src/filters_blosc2.rs +++ b/crates/clawhdf5-format/src/filters_blosc2.rs @@ -46,6 +46,7 @@ //! variable-length blocks, dictionaries, lazy chunks, user-defined codecs //! and registered filters (e.g. bytedelta), sparse frames. +use crate::addr::saturating_usize; use crate::error::FormatError; use crate::filter_registry::FilterContext; use crate::filters_bitshuffle::bitunshuffle_block; @@ -546,7 +547,7 @@ fn parse_frame(buf: &[u8], limit: usize) -> Result, FormatError> { return Err(err("negative size in frame header")); } let header_len = header_len as usize; - let buf = &buf[..frame_len as usize]; + let buf = &buf[..saturating_usize(frame_len)]; let cbytes = usize::try_from(cbytes).map_err(|_| err("bad compressed size"))?; let data_end = header_len .checked_add(cbytes) diff --git a/crates/clawhdf5-format/src/filters_bzip2.rs b/crates/clawhdf5-format/src/filters_bzip2.rs index 93e33a3..6989c5b 100644 --- a/crates/clawhdf5-format/src/filters_bzip2.rs +++ b/crates/clawhdf5-format/src/filters_bzip2.rs @@ -4,6 +4,7 @@ //! the compression level). Decoded with the `bzip2` crate's default backend, //! `libbz2-rs-sys`, a pure-Rust port of libbzip2. +use crate::addr::saturating_usize; use crate::error::FormatError; use crate::filter_registry::FilterContext; @@ -28,7 +29,7 @@ pub(crate) fn bzip2_decode(input: &[u8], ctx: &FilterContext<'_>) -> Result limit { return Err(err("output exceeds the chunk size")); @@ -43,7 +44,7 @@ pub(crate) fn bzip2_decode(input: &[u8], ctx: &FilterContext<'_>) -> Result= input.len() + } else if saturating_usize(dec.total_in()) >= input.len() || (dec.total_in(), dec.total_out()) == (in_before, out_before) { return Err(err("truncated stream")); @@ -61,7 +62,7 @@ pub(crate) fn bzip2_encode(input: &[u8], ctx: &FilterContext<'_>) -> Result Result, FormatError> { - let db_offset = header.data_block_address as usize; + let db_offset = to_usize(header.data_block_address)?; // Parse data block header: FADB(4) + version(1) + client_id(1) + header_address(offset_size) let db_header_size = 4 + 1 + 1 + offset_size as usize; @@ -174,7 +175,7 @@ pub fn read_fixed_array_chunks( // Elements start immediately after the data block prefix. let elements_start = db_offset + db_header_size; - let num_elements = header.num_elements as usize; + let num_elements = to_usize(header.num_elements)?; // A chunk index cannot describe more elements than the file has bytes (each // element occupies at least `offset_size` bytes). Reject a corrupt count // before it can drive a huge loop or overflow an offset computation. diff --git a/crates/clawhdf5-format/src/group_v1.rs b/crates/clawhdf5-format/src/group_v1.rs index a4f97c8..3dff9cd 100644 --- a/crates/clawhdf5-format/src/group_v1.rs +++ b/crates/clawhdf5-format/src/group_v1.rs @@ -3,6 +3,7 @@ #[cfg(not(feature = "std"))] use alloc::{string::String, vec::Vec}; +use crate::addr::to_usize; use crate::btree_v1::collect_symbol_table_nodes; use crate::error::FormatError; use crate::local_heap::LocalHeap; @@ -54,7 +55,7 @@ pub(crate) fn v1_group_entries( // Parse local heap let heap = LocalHeap::parse( file_data, - sym_table_msg.local_heap_address as usize, + to_usize(sym_table_msg.local_heap_address)?, offset_size, length_size, )?; @@ -70,7 +71,7 @@ pub(crate) fn v1_group_entries( let mut entries = Vec::new(); let mut heap_checked = false; for snod_addr in snod_addrs { - let snod = SymbolTableNode::parse(file_data, snod_addr as usize, offset_size)?; + let snod = SymbolTableNode::parse(file_data, to_usize(snod_addr)?, offset_size)?; for entry in &snod.entries { // Like libhdf5, look at the heap's free list only once a name is // needed: an empty group with a damaged heap still lists. @@ -152,7 +153,7 @@ fn for_each_v1_soft_link( ) -> Result<(), FormatError> { let heap = LocalHeap::parse( file_data, - sym_table_msg.local_heap_address as usize, + to_usize(sym_table_msg.local_heap_address)?, offset_size, length_size, )?; @@ -164,7 +165,7 @@ fn for_each_v1_soft_link( )?; let mut heap_checked = false; for snod_addr in snod_addrs { - let snod = SymbolTableNode::parse(file_data, snod_addr as usize, offset_size)?; + let snod = SymbolTableNode::parse(file_data, to_usize(snod_addr)?, offset_size)?; for entry in &snod.entries { if entry.cache_type != CACHE_TYPE_SOFT_LINK { continue; @@ -242,7 +243,7 @@ pub fn resolve_path( // Not last — must be a group, parse its object header to get symbol table let obj_header = ObjectHeader::parse( file_data, - entry.object_header_address as usize, + to_usize(entry.object_header_address)?, offset_size, length_size, )?; diff --git a/crates/clawhdf5-format/src/link_message.rs b/crates/clawhdf5-format/src/link_message.rs index 27044cc..55cb269 100644 --- a/crates/clawhdf5-format/src/link_message.rs +++ b/crates/clawhdf5-format/src/link_message.rs @@ -3,6 +3,7 @@ #[cfg(not(feature = "std"))] use alloc::{string::String, vec::Vec}; +use crate::addr::to_usize; use crate::datatype::CharacterSet; use crate::error::FormatError; @@ -247,7 +248,7 @@ impl LinkMessage { }; // Link name length - let name_len = read_offset(data, pos, name_size_field_width)? as usize; + let name_len = to_usize(read_offset(data, pos, name_size_field_width)?)?; pos += name_size_field_width as usize; // Link name diff --git a/crates/clawhdf5-format/src/local_heap.rs b/crates/clawhdf5-format/src/local_heap.rs index 39e9b26..b2ef923 100644 --- a/crates/clawhdf5-format/src/local_heap.rs +++ b/crates/clawhdf5-format/src/local_heap.rs @@ -3,6 +3,7 @@ #[cfg(not(feature = "std"))] use alloc::string::String; +use crate::addr::to_usize; use crate::error::FormatError; /// Parsed HDF5 Local Heap header. @@ -140,15 +141,15 @@ impl LocalHeap { /// Read a null-terminated string from the heap's data segment at the given byte offset. pub fn read_string(&self, file_data: &[u8], string_offset: u64) -> Result { - let seg_addr = self.data_segment_address as usize; + let seg_addr = to_usize(self.data_segment_address)?; let str_start = seg_addr - .checked_add(string_offset as usize) + .checked_add(to_usize(string_offset)?) .ok_or(FormatError::Overflow( "local heap seg_addr + string_offset overflow".into(), ))?; let seg_end = seg_addr - .checked_add(self.data_segment_size as usize) + .checked_add(to_usize(self.data_segment_size)?) .ok_or(FormatError::Overflow( "local heap seg_addr + data_segment_size overflow".into(), ))?; diff --git a/crates/clawhdf5-format/src/object_header.rs b/crates/clawhdf5-format/src/object_header.rs index 1fc8696..3d7b7f7 100644 --- a/crates/clawhdf5-format/src/object_header.rs +++ b/crates/clawhdf5-format/src/object_header.rs @@ -5,6 +5,7 @@ use alloc::vec::Vec; use byteorder::{ByteOrder, LittleEndian}; +use crate::addr::to_usize; use crate::error::FormatError; use crate::message_type::MessageType; @@ -264,8 +265,8 @@ impl ObjectHeader { // Follow continuations (v1 continuation chunks are just raw // messages, no signature); check_message has checked the body. if msg_type == MessageType::ObjectHeaderContinuation { - let cont_offset = read_offset(body, 0, offset_size)? as usize; - let cont_length = read_offset(body, offset_size as usize, length_size)? as usize; + let cont_offset = to_usize(read_offset(body, 0, offset_size)?)?; + let cont_length = to_usize(read_offset(body, offset_size as usize, length_size)?)?; Self::parse_v1_chunk( data, cont_offset, @@ -339,7 +340,7 @@ impl ObjectHeader { _ => unreachable!(), }; ensure_len(data, pos, chunk_size_width as usize)?; - let chunk0_size = read_offset(data, pos, chunk_size_width)? as usize; + let chunk0_size = to_usize(read_offset(data, pos, chunk_size_width)?)?; pos += chunk_size_width as usize; // Bit 2: attribute creation order tracked → messages include creation order field let has_creation_order = flags & 0x04 != 0; @@ -472,8 +473,8 @@ impl ObjectHeader { let msg_type = MessageType::from_u16(msg_type_raw); if msg_type == MessageType::ObjectHeaderContinuation { // check_message has checked the body holds both fields. - let cont_off = read_offset(body, 0, offset_size)? as usize; - let cont_len = read_offset(body, offset_size as usize, length_size)? as usize; + let cont_off = to_usize(read_offset(body, 0, offset_size)?)?; + let cont_len = to_usize(read_offset(body, offset_size as usize, length_size)?)?; continuations.push((cont_off, cont_len)); } else if msg_type == MessageType::Nil { null_count += 1; diff --git a/crates/clawhdf5-format/src/partial_read.rs b/crates/clawhdf5-format/src/partial_read.rs index 4c5a01e..ad5dc8e 100644 --- a/crates/clawhdf5-format/src/partial_read.rs +++ b/crates/clawhdf5-format/src/partial_read.rs @@ -203,7 +203,12 @@ fn copy_overlap( }; let (src_strides, out_strides) = (strides(src_shape), strides(box_extent)); let last = rank - 1; - let run = ((hi[last] - lo[last]) as usize) * elem_size; + // Byte offsets into the in-memory buffers; one that does not fit `usize` + // (a 32-bit target) is out of both buffers, like one past their ends. + let bytes = |elements: u64| usize::try_from(elements).ok()?.checked_mul(elem_size); + let Some(run) = bytes(hi[last] - lo[last]) else { + return; + }; let mut idx = lo.clone(); loop { @@ -213,8 +218,12 @@ fn copy_overlap( let out_at: u64 = (0..rank) .map(|d| (idx[d] - box_start[d]) * out_strides[d]) .sum(); - let (s, o) = (src_at as usize * elem_size, out_at as usize * elem_size); - if let (Some(from), Some(to)) = (src.get(s..s + run), out.get_mut(o..o + run)) { + if let (Some(s), Some(o)) = (bytes(src_at), bytes(out_at)) + && let (Some(from), Some(to)) = ( + src.get(s..s.saturating_add(run)), + out.get_mut(o..o.saturating_add(run)), + ) + { to.copy_from_slice(from); } // Advance over every dimension but the last. diff --git a/crates/clawhdf5-format/src/selection.rs b/crates/clawhdf5-format/src/selection.rs index de31e46..3dbc6ee 100644 --- a/crates/clawhdf5-format/src/selection.rs +++ b/crates/clawhdf5-format/src/selection.rs @@ -19,6 +19,7 @@ use alloc::{vec, vec::Vec}; use core::ops::Range; +use crate::addr::to_usize; use crate::error::FormatError; /// A selection describing which elements of a dataset to access. @@ -562,7 +563,7 @@ fn decode_hyperslab(r: &mut SelReader, version: u64) -> Result 32 { @@ -625,11 +626,11 @@ fn decode_hyperslab(r: &mut SelReader, version: u64) -> Result { let target_header = - ObjectHeader::parse(file_data, addr as usize, offset_size, length_size)?; + ObjectHeader::parse(file_data, to_usize(addr)?, offset_size, length_size)?; for msg in &target_header.messages { if msg.msg_type == target_msg_type && !is_shared(msg.flags) { return Ok(msg.data.clone()); diff --git a/crates/clawhdf5-format/src/vds.rs b/crates/clawhdf5-format/src/vds.rs index d67b832..df1b068 100644 --- a/crates/clawhdf5-format/src/vds.rs +++ b/crates/clawhdf5-format/src/vds.rs @@ -15,6 +15,7 @@ #[cfg(not(feature = "std"))] use alloc::{format, string::String, vec, vec::Vec}; +use crate::addr::to_usize; use crate::data_layout::{DataLayout, VdsMapping, parse_vds_mappings}; use crate::dataspace::Dataspace; use crate::datatype::Datatype; @@ -208,7 +209,7 @@ fn load_mappings( return Ok(Vec::new()); }; let coll = - crate::global_heap::GlobalHeapCollection::parse(file_data, addr as usize, length_size)?; + crate::global_heap::GlobalHeapCollection::parse(file_data, to_usize(addr)?, length_size)?; let index = u16::try_from(*global_heap_index) .map_err(|_| vds_err("VDS mapping heap index out of range"))?; let obj = coll @@ -611,12 +612,12 @@ fn scatter( return Err(vds_err("virtual/source selection element counts differ")); } for (&v, &s) in vidx.iter().zip(sidx) { - let (vo, so) = (v as usize * elem_size, s as usize * elem_size); + let (vo, so) = (to_usize(v)? * elem_size, to_usize(s)? * elem_size); if vo + elem_size > out.len() || so + elem_size > src.len() { return Err(vds_err("virtual dataset selection out of bounds")); } out[vo..vo + elem_size].copy_from_slice(&src[so..so + elem_size]); - mapped[v as usize] = true; + mapped[to_usize(v)?] = true; } Ok(()) } @@ -747,7 +748,7 @@ fn selection_indices( return Err(vds_err("VDS selection blocks overlap")); } } - let mut out = Vec::with_capacity(volume as usize); + let mut out = Vec::with_capacity(to_usize(volume)?); for (s, e) in starts.chunks_exact(rank).zip(ends.chunks_exact(rank)) { let mut cur = s.to_vec(); 'block: loop { @@ -868,7 +869,7 @@ fn load_source_file(whole: &mut [u8]) -> Result<(), FormatError> { // read as before, up to its length. let end = sb .data_end(base as u64, whole.len() as u64) - .map_or(whole.len(), |e| base + e as usize); + .map_or(Ok(whole.len()), |e| to_usize(e).map(|e| base + e))?; crate::superblock_ext::apply_cache_image_in_place(&mut whole[base..end], &sb) } @@ -921,7 +922,7 @@ fn open_source(file_data: &[u8], path: &str) -> Result, Forma Err(FormatError::PathNotFound(_)) => return Ok(None), Err(e) => return Err(e), }; - let header = crate::object_header::ObjectHeader::parse(file_data, addr as usize, os, ls)?; + let header = crate::object_header::ObjectHeader::parse(file_data, to_usize(addr)?, os, ls)?; let mut src = OpenSource { offset_size: os, length_size: ls, diff --git a/crates/clawhdf5-format/src/vl_data.rs b/crates/clawhdf5-format/src/vl_data.rs index 3a54c1a..a998a62 100644 --- a/crates/clawhdf5-format/src/vl_data.rs +++ b/crates/clawhdf5-format/src/vl_data.rs @@ -9,6 +9,7 @@ use alloc::{collections::BTreeMap, format, string::String, vec, vec::Vec}; #[cfg(feature = "std")] use std::collections::BTreeMap; +use crate::addr::to_usize; use crate::error::FormatError; use crate::global_heap::{GlobalHeapCollection, GlobalHeapIndex}; @@ -55,7 +56,7 @@ pub fn parse_vl_references( ) -> Result, FormatError> { let elem_size = 4 + offset_size as usize + 4; // length + address + index let total = - (num_elements as usize) + to_usize(num_elements)? .checked_mul(elem_size) .ok_or(FormatError::UnexpectedEof { expected: usize::MAX, @@ -68,7 +69,7 @@ pub fn parse_vl_references( }); } - let mut elements = Vec::with_capacity(num_elements as usize); + let mut elements = Vec::with_capacity(to_usize(num_elements)?); let mut pos = 0; for _ in 0..num_elements { @@ -406,7 +407,7 @@ impl<'a> VlResolver<'a> { let index = GlobalHeapCollection::parse_index(self.file_data, offset, self.length_size)?; // parse_index checked that the collection lies in the file. - let end = offset + index.collection_size as usize; + let end = offset + to_usize(index.collection_size)?; self.check_overlap(offset, end)?; let coll = CachedCollection::new(index); if self.cached_bytes.saturating_add(coll.cost()) > self.budget { diff --git a/scripts/check-32bit-casts.sh b/scripts/check-32bit-casts.sh new file mode 100755 index 0000000..cfed6f5 --- /dev/null +++ b/scripts/check-32bit-casts.sh @@ -0,0 +1,35 @@ +#!/usr/bin/env bash +# CI check: clawhdf5-format has no truncating `u64 as usize` cast on a 32-bit +# target. HDF5 addresses and lengths are 64-bit; on wasm32 (or any 32-bit +# target) such a cast silently wraps an address past 4 GiB onto another part +# of the file. File values go through `addr::to_usize` (a clean error) and +# in-memory counts through `addr::saturating_usize`. +# +# Lints wasm32-unknown-unknown with clippy's cast_possible_truncation and +# fails on any u64 -> usize finding (other truncations are not checked here). +# +# Usage: +# ./scripts/check-32bit-casts.sh +# +# Prerequisites: +# rustup target add wasm32-unknown-unknown + +set -euo pipefail + +TARGET="wasm32-unknown-unknown" +echo "==> Checking for truncating u64 -> usize casts in clawhdf5-format ($TARGET)" + +out=$(cargo clippy -p clawhdf5-format --target "$TARGET" \ + --features plugin-filters --message-format short \ + -- -A clippy::all -W clippy::cast_possible_truncation 2>&1) || { + echo "$out" + echo "==> clippy failed" >&2 + exit 1 +} +found=$(grep -F 'casting `u64` to `usize`' <<<"$out" || true) +if [ -n "$found" ]; then + echo "$found" + echo "==> use addr::to_usize (file values) or addr::saturating_usize (in-memory counts)" >&2 + exit 1 +fi +echo "==> no truncating u64 -> usize casts" diff --git a/scripts/ci-test.sh b/scripts/ci-test.sh index 3012cba..c7c77dd 100755 --- a/scripts/ci-test.sh +++ b/scripts/ci-test.sh @@ -147,6 +147,8 @@ run_step "wasm32 clippy (clawhdf5-wasm)" cargo clippy \ --target wasm32-unknown-unknown \ --all-targets \ -- -D warnings +# A 64-bit file address must not wrap on a 32-bit target. +run_step "check-32bit-casts.sh" "$SCRIPT_DIR/check-32bit-casts.sh" # The built wasm package, run under Node against h5py/netCDF4-written files, # and the viewer page in headless Chromium when one is found. From 1b4a93f65a555b414ab82d56dbf7156c1aefed3a Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 13:33:24 -0500 Subject: [PATCH 04/10] docs: indexed name lookups and checked address conversion (M0) Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 35 +++++++++++++++++++++++++++++++++++ docs/design/range-reads.md | 9 +++++++++ 2 files changed, 44 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76f4316..f3b0b6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,41 @@ ## Unreleased +### Name lookups through the name index (2026-09-26) +- **Finding one link or attribute by name reads the name index, not every + entry.** In a dense group (links in a fractal heap) the v2 B-tree name + index (record type 5, lookup3 hash of the name) is descended to the + records with the name's hash and only their links are read — O(log n) + instead of all n. Path resolution (`File::dataset`, `resolve_path_any`, + soft-link targets) and `Group::dataset`/`Group::group` (on `File`, + `MmapFile` and `LazyFile`, which listed the whole group per call) use it; + names whose hashes collide are all compared, so the order libhdf5 gives + them does not matter. New `clawhdf5_format::group_v2::resolve_child`, + `btree_v2::find_btree_v2_records` (records in one key range), and a + `lookup-stats` feature counting heap objects read, for tests. Huge heap + objects are found through their index the same way. +- **`attr(name)`** on the facade's groups and datasets (all three file + types): one attribute, found in dense storage through its name index + (record type 8) instead of reading every attribute + (`clawhdf5_format::attribute::find_attribute_in_file`). +- **`Group::entries()` and `File::group_at(address)`**: a listing's + `(name, address)` pairs, to open children without looking names up again. +- Test: `crates/clawhdf5/tests/indexed_lookup_interop.rs` — every child of + an h5py-written 35 001-link group (with colliding hashes) opened by name + reads at most two links per lookup (before: 35 001), matches h5py, and + every link kind (soft, relative, dangling, external) resolves as h5py + resolves it in dense and compact groups. + +### Checked address conversion (2026-09-26) +- **No 64-bit file value is truncated on a 32-bit target.** Every + `u64 as usize` cast in `clawhdf5-format` (115) is gone: file addresses, + lengths and counts go through `addr::to_usize`, which fails with + `FormatError::Overflow` where the value does not fit (wasm32 and other + 32-bit targets; it used to wrap onto another part of the file), and + in-memory counts through `addr::saturating_usize`. On 64-bit targets + nothing changes. `scripts/check-32bit-casts.sh` (run by `ci-test.sh`) + lints the wasm32 build and fails on any new truncating cast. + ### Chunked full reads (2026-09-26) - **Chunks are decoded straight into the output, into reused buffers.** A full read of a chunked dataset faulted in about three times its size in diff --git a/docs/design/range-reads.md b/docs/design/range-reads.md index 2e82a6c..1edee1a 100644 --- a/docs/design/range-reads.md +++ b/docs/design/range-reads.md @@ -379,6 +379,15 @@ fast path within benchmark noise. n children decodes its links O(n) times. Look names up through the index (above) and let a listing hand out its entries, so the cache has less to absorb. +- *Status 2026-09-26:* done on branch `perf/p3-indexed-lookups` — link and + attribute names through the name indexes (`group_v2::resolve_child`, + `attribute::find_attribute_in_file`; creation-order lookups by name do + not exist in the API, so the creation-order index is still only listed), + `addr::to_usize`/`saturating_usize` for all 115 `u64 as usize` casts in + `clawhdf5-format` (the 133 above counted any `*addr*/*offset* as usize`, + mostly widening `u8`/`u32` casts; `scripts/check-32bit-casts.sh` lints + wasm32 for the truncating ones), and `Group::entries`/`File::group_at`. + The facade, io and ann casts are not converted. **M1 — metadata over the trait, in-memory impl identical to today (2–3 weeks).** - Add `Storage` (above) to `clawhdf5-format`, `no_std`-compatible, with From 85efde0b4af74dcbd413e3827cda36f405a0fa13 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 13:36:02 -0500 Subject: [PATCH 05/10] test: rustfmt the lookup tests Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5/tests/dense_storage_interop.rs | 10 ++++++++-- crates/clawhdf5/tests/indexed_lookup_interop.rs | 5 ++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/crates/clawhdf5/tests/dense_storage_interop.rs b/crates/clawhdf5/tests/dense_storage_interop.rs index 3f741f6..f4c8a5b 100644 --- a/crates/clawhdf5/tests/dense_storage_interop.rs +++ b/crates/clawhdf5/tests/dense_storage_interop.rs @@ -230,8 +230,14 @@ fn many_huge_attributes_are_found_through_their_index() { for i in 0..300i64 { let name = format!("h{i:03}"); let want: Vec = (0..600).map(|v| v + i).collect(); - assert!(matches!(&attrs[&name], AttrValue::I64Array(v) if *v == want), "{name}"); - assert!(matches!(d.attr(&name).unwrap(), Some(AttrValue::I64Array(v)) if v == want), "{name}"); + assert!( + matches!(&attrs[&name], AttrValue::I64Array(v) if *v == want), + "{name}" + ); + assert!( + matches!(d.attr(&name).unwrap(), Some(AttrValue::I64Array(v)) if v == want), + "{name}" + ); } } diff --git a/crates/clawhdf5/tests/indexed_lookup_interop.rs b/crates/clawhdf5/tests/indexed_lookup_interop.rs index 518af1e..30adb0c 100644 --- a/crates/clawhdf5/tests/indexed_lookup_interop.rs +++ b/crates/clawhdf5/tests/indexed_lookup_interop.rs @@ -376,7 +376,10 @@ fn links_of_every_kind_resolve_by_name_as_in_h5py() { "group".to_string() } (Err(e), Err(e2)) => { - assert!(is_not_found(&e) && is_not_found(&e2), "{key}: {e:?} / {e2:?}"); + assert!( + is_not_found(&e) && is_not_found(&e2), + "{key}: {e:?} / {e2:?}" + ); "none".to_string() } (Err(e), Ok(_)) => panic!("{key}: dataset {e:?} but group ok"), From 92c8285549c5c556cabe4d9506274b3a593495e7 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 14:07:01 -0500 Subject: [PATCH 06/10] format: verify B-tree v2 internal node checksums Only leaves and the header were checked. Harmless while every lookup read the whole tree, but the indexed lookup prunes children by the keys stored in internal nodes, so one corrupted byte there could route a name to the wrong child and report it missing with no error. A BTIN whose lookup3 checksum does not match is now ChecksumMismatch on every read (lookups and full traversals), as in libhdf5. Test: one byte of the root BTIN of the 35 001-link h5py group's name index changed -> lookups, paths and listings through File, MmapFile and LazyFile all fail with ChecksumMismatch, and h5py refuses both. Before, lookups returned Ok. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-format/src/btree_v2.rs | 19 +++++ .../clawhdf5/tests/indexed_lookup_interop.rs | 84 +++++++++++++++++++ 2 files changed, 103 insertions(+) diff --git a/crates/clawhdf5-format/src/btree_v2.rs b/crates/clawhdf5-format/src/btree_v2.rs index 7582c05..d3f6250 100644 --- a/crates/clawhdf5-format/src/btree_v2.rs +++ b/crates/clawhdf5-format/src/btree_v2.rs @@ -355,6 +355,23 @@ fn read_internal_node( pos += total_nrec_width; // skip total records in subtree children.push((addr, child_nrec)); } + + // The checksum follows the child pointers and covers the node up to it. + // Lookups prune children by the keys in this node, so an unverified + // internal node could hide a record without any error: libhdf5 refuses + // a mismatch here, and so does this. + #[cfg(feature = "checksum")] + { + ensure_len(file_data, pos, 4)?; + let stored = LittleEndian::read_u32(&file_data[pos..pos + 4]); + let computed = crate::checksum::jenkins_lookup3(&file_data[offset..pos]); + if computed != stored { + return Err(FormatError::ChecksumMismatch { + expected: stored, + computed, + }); + } + } Ok((records_start, children)) } @@ -733,6 +750,8 @@ mod tests { buf.extend_from_slice(&child_nrec.to_le_bytes()[..nrec_width]); buf.resize(buf.len() + total_width, 0); } + let sum = crate::checksum::jenkins_lookup3(&buf); + buf.extend_from_slice(&sum.to_le_bytes()); buf } diff --git a/crates/clawhdf5/tests/indexed_lookup_interop.rs b/crates/clawhdf5/tests/indexed_lookup_interop.rs index 30adb0c..71f48f0 100644 --- a/crates/clawhdf5/tests/indexed_lookup_interop.rs +++ b/crates/clawhdf5/tests/indexed_lookup_interop.rs @@ -411,3 +411,87 @@ fn links_of_every_kind_resolve_by_name_as_in_h5py() { } } } + +/// The link name index (v2 B-tree, record type 5) of the big group: its +/// depth and root node address, read from the one type-5 `BTHD` in the file. +fn name_index_root(bytes: &[u8]) -> (u16, usize) { + let headers: Vec = bytes + .windows(4) + .enumerate() + .filter(|(i, w)| *w == b"BTHD" && bytes.get(i + 5) == Some(&5)) + .map(|(i, _)| i) + .collect(); + assert_eq!(headers.len(), 1, "type-5 B-tree headers at {headers:?}"); + let h = headers[0]; + // signature, version, type, node size (4), record size (2), depth (2), + // split and merge percent, root address (8). + let depth = u16::from_le_bytes([bytes[h + 12], bytes[h + 13]]); + let root = u64::from_le_bytes(bytes[h + 16..h + 24].try_into().unwrap()); + (depth, usize::try_from(root).unwrap()) +} + +/// One byte changed in a key of the name index's root (an internal node) +/// must be an error, not a name quietly routed to the wrong child and +/// reported missing: lookups prune children by those keys. libhdf5 checks +/// the internal node's checksum and refuses the group; so must we, for a +/// lookup and for a listing. +#[test] +fn a_corrupt_internal_index_node_is_an_error_not_a_missing_name() { + skip_if_no_python!(); + let fx = fixture(); + let mut bytes = std::fs::read(&fx.path).unwrap(); + let (depth, root) = name_index_root(&bytes); + assert!(depth >= 2, "want a deep index, got depth {depth}"); + assert_eq!(&bytes[root..root + 4], b"BTIN"); + // Signature, version, type, then record 0: its name hash comes first. + bytes[root + 6] ^= 0x5a; + let dir = tempfile::tempdir().unwrap(); + let bad = dir.path().join("bad.h5"); + std::fs::write(&bad, &bytes).unwrap(); + let bad = bad.display().to_string(); + + let is_checksum = |e: &clawhdf5::Error| { + matches!( + e, + clawhdf5::Error::Format(FormatError::ChecksumMismatch { .. }) + ) + }; + let f = File::open(&bad).unwrap(); + let g = f.group("g").unwrap(); + // Every name, present or not, goes through the root. + for name in fx.links.keys().step_by(97).chain(&fx.missing_links) { + let err = g.dataset(name).map(|_| ()).unwrap_err(); + assert!(is_checksum(&err), "dataset({name:?}): {err:?}"); + } + let err = f.dataset("/g/n0").map(|_| ()).unwrap_err(); + assert!(is_checksum(&err), "path: {err:?}"); + let err = g.datasets().unwrap_err(); + assert!(is_checksum(&err), "listing: {err:?}"); + let err = g.entries().unwrap_err(); + assert!(is_checksum(&err), "entries: {err:?}"); + + let m = MmapFile::open(&bad).unwrap(); + let mg = m.group("g").unwrap(); + assert!(mg.dataset("n0").is_err_and(|e| is_checksum(&e))); + assert!(mg.datasets().is_err_and(|e| is_checksum(&e))); + let l = LazyFile::open_mmap(&bad).unwrap(); + let lg = l.group("g").unwrap(); + assert!(lg.dataset("n0").is_err_and(|e| is_checksum(&e))); + assert!(lg.datasets().is_err_and(|e| is_checksum(&e))); + + // libhdf5 refuses both too. + let out = run_python(&format!( + "import h5py\n\ + r = []\n\ + with h5py.File(r'{bad}', 'r') as f:\n\ + \x20 g = f['g']\n\ + \x20 for op in (lambda: g['n0'], lambda: list(g)):\n\ + \x20 try:\n\ + \x20 op()\n\ + \x20 r.append('ok')\n\ + \x20 except Exception as e:\n\ + \x20 r.append('checksum' if 'checksum' in str(e) else repr(e))\n\ + print(' '.join(r))", + )); + assert_eq!(out, "checksum checksum"); +} From b6cbd2319f9d156a19ef077d10e2647bab8d05b7 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 14:08:57 -0500 Subject: [PATCH 07/10] format: checked chunk addresses on the parallel read path Three `chunk_info.address as usize` casts behind the `parallel` feature survived the conversion, because check-32bit-casts.sh linted only default features plus plugin-filters. On a 32-bit target with rayon a chunk address past 4 GiB still wrapped onto another part of the file. They go through addr::to_usize now, and the lane index (h % n, always < n) through saturating_usize. The script now lints no default features, default features, and every optional feature but szip (wasm32; the set with zstd, which does not build for wasm32, on the host, where the lint reports the same casts). With the old parallel_read.rs/lane_partition.rs it fails listing the four casts; the old script passed them. CHANGELOG and the design note give the exact count (119) and what is not covered. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 13 ++++- crates/clawhdf5-format/src/lane_partition.rs | 3 +- crates/clawhdf5-format/src/parallel_read.rs | 7 ++- docs/design/range-reads.md | 9 +-- scripts/check-32bit-casts.sh | 61 +++++++++++++++----- 5 files changed, 68 insertions(+), 25 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f3b0b6c..14f8053 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,14 +28,21 @@ resolves it in dense and compact groups. ### Checked address conversion (2026-09-26) -- **No 64-bit file value is truncated on a 32-bit target.** Every - `u64 as usize` cast in `clawhdf5-format` (115) is gone: file addresses, +- **No 64-bit file value is truncated on a 32-bit target.** All 119 + truncating `u64 as usize` casts in `clawhdf5-format` that clippy's + `cast_possible_truncation` reports, under every feature the crate is + built with in CI except `szip` (115 with default features and + `plugin-filters`, 4 more behind `parallel`), are gone: file addresses, lengths and counts go through `addr::to_usize`, which fails with `FormatError::Overflow` where the value does not fit (wasm32 and other 32-bit targets; it used to wrap onto another part of the file), and in-memory counts through `addr::saturating_usize`. On 64-bit targets nothing changes. `scripts/check-32bit-casts.sh` (run by `ci-test.sh`) - lints the wasm32 build and fails on any new truncating cast. + lints the crate with no default features, with default features, and + with every optional feature but `szip` (for wasm32; the set with `zstd`, + which does not build for wasm32, for the host), and fails on any new + truncating cast. The facade, `clawhdf5-io` and `clawhdf5-ann` are not + covered. ### Chunked full reads (2026-09-26) - **Chunks are decoded straight into the output, into reused buffers.** A diff --git a/crates/clawhdf5-format/src/lane_partition.rs b/crates/clawhdf5-format/src/lane_partition.rs index 2b8b8da..0b86e6b 100644 --- a/crates/clawhdf5-format/src/lane_partition.rs +++ b/crates/clawhdf5-format/src/lane_partition.rs @@ -112,7 +112,8 @@ pub fn partition( for idx in 0..num_items { let h = fxhash_combine(seed, idx as u64); - let lane = (h % num_lanes as u64) as usize; + // Below `num_lanes`, so it fits. + let lane = crate::addr::saturating_usize(h % num_lanes as u64); lanes[lane].push(idx); } diff --git a/crates/clawhdf5-format/src/parallel_read.rs b/crates/clawhdf5-format/src/parallel_read.rs index d9927c5..92fb1fa 100644 --- a/crates/clawhdf5-format/src/parallel_read.rs +++ b/crates/clawhdf5-format/src/parallel_read.rs @@ -7,6 +7,7 @@ //! The lane assignment is seeded by dataset metadata so repeated reads of //! the same region produce identical partitions (cache-friendly, reproducible). +use crate::addr::to_usize; use crate::chunked_read::ChunkInfo; use crate::error::FormatError; use crate::filter_pipeline::FilterPipeline; @@ -210,7 +211,7 @@ pub fn decompress_chunks_lane_partitioned( for &index in &indices { let chunk_info = &chunks[index]; - let c_addr = chunk_info.address as usize; + let c_addr = to_usize(chunk_info.address)?; let size = chunk_info.chunk_size as usize; if c_addr @@ -288,7 +289,7 @@ pub fn decompress_chunks_parallel( .par_iter() .enumerate() .map(|(index, chunk_info)| { - let c_addr = chunk_info.address as usize; + let c_addr = to_usize(chunk_info.address)?; let size = chunk_info.chunk_size as usize; if c_addr .checked_add(size) @@ -332,7 +333,7 @@ pub fn decompress_chunks_sequential( ) -> Result>, FormatError> { let mut result = Vec::with_capacity(chunks.len()); for chunk_info in chunks { - let c_addr = chunk_info.address as usize; + let c_addr = to_usize(chunk_info.address)?; let size = chunk_info.chunk_size as usize; if c_addr .checked_add(size) diff --git a/docs/design/range-reads.md b/docs/design/range-reads.md index 1edee1a..9629a21 100644 --- a/docs/design/range-reads.md +++ b/docs/design/range-reads.md @@ -383,10 +383,11 @@ fast path within benchmark noise. attribute names through the name indexes (`group_v2::resolve_child`, `attribute::find_attribute_in_file`; creation-order lookups by name do not exist in the API, so the creation-order index is still only listed), - `addr::to_usize`/`saturating_usize` for all 115 `u64 as usize` casts in - `clawhdf5-format` (the 133 above counted any `*addr*/*offset* as usize`, - mostly widening `u8`/`u32` casts; `scripts/check-32bit-casts.sh` lints - wasm32 for the truncating ones), and `Group::entries`/`File::group_at`. + `addr::to_usize`/`saturating_usize` for all 119 truncating `u64 as usize` + casts clippy finds in `clawhdf5-format` under any CI-built feature set but + `szip` (the 133 above counted any `*addr*/*offset* as usize`, mostly + widening `u8`/`u32` casts; `scripts/check-32bit-casts.sh` lints those + feature sets for new ones), and `Group::entries`/`File::group_at`. The facade, io and ann casts are not converted. **M1 — metadata over the trait, in-memory impl identical to today (2–3 weeks).** diff --git a/scripts/check-32bit-casts.sh b/scripts/check-32bit-casts.sh index cfed6f5..104bdcd 100755 --- a/scripts/check-32bit-casts.sh +++ b/scripts/check-32bit-casts.sh @@ -5,8 +5,18 @@ # of the file. File values go through `addr::to_usize` (a clean error) and # in-memory counts through `addr::saturating_usize`. # -# Lints wasm32-unknown-unknown with clippy's cast_possible_truncation and -# fails on any u64 -> usize finding (other truncations are not checked here). +# Lints with clippy's cast_possible_truncation and fails on any u64 -> usize +# finding (other truncations are not checked here), once per feature set +# below. Together the sets compile every feature-gated line of the crate that +# ci-test.sh builds: features only add code, except `not(feature = ...)` +# paths for std/checksum/fast-checksum/szip, which the no-default-features +# and default sets cover. szip is left out (it needs libaec), as in +# ci-test.sh. +# +# The sets are linted for wasm32 where they build there. zstd links a C +# library that does not build for wasm32, so the set with it is linted for +# the host: the lint reports u64 -> usize casts whatever the target's +# pointer width, and the crate has no pointer-width-dependent code. # # Usage: # ./scripts/check-32bit-casts.sh @@ -16,19 +26,42 @@ set -euo pipefail -TARGET="wasm32-unknown-unknown" -echo "==> Checking for truncating u64 -> usize casts in clawhdf5-format ($TARGET)" +WASM="wasm32-unknown-unknown" +ALL_BUT_ZSTD="parallel,lz4,pcodec,fast-checksum,blake3_hash,plugin-filters,lookup-stats" -out=$(cargo clippy -p clawhdf5-format --target "$TARGET" \ - --features plugin-filters --message-format short \ - -- -A clippy::all -W clippy::cast_possible_truncation 2>&1) || { - echo "$out" - echo "==> clippy failed" >&2 - exit 1 -} -found=$(grep -F 'casting `u64` to `usize`' <<<"$out" || true) -if [ -n "$found" ]; then - echo "$found" +# target|cargo feature arguments +SETS=( + "$WASM|--no-default-features" + "$WASM|--no-default-features --features std,checksum" + "$WASM|" + "$WASM|--features $ALL_BUT_ZSTD" + "host|--features $ALL_BUT_ZSTD,zstd" +) + +status=0 +for set in "${SETS[@]}"; do + target=${set%%|*} + args=${set#*|} + target_args=() + if [ "$target" != host ]; then + target_args=(--target "$target") + fi + echo "==> Checking for truncating u64 -> usize casts in clawhdf5-format ($target: ${args:-default features})" + # shellcheck disable=SC2086 # $args is a list of arguments + out=$(cargo clippy -p clawhdf5-format "${target_args[@]}" $args \ + --message-format short \ + -- -A clippy::all -W clippy::cast_possible_truncation 2>&1) || { + echo "$out" + echo "==> clippy failed" >&2 + exit 1 + } + found=$(grep -F 'casting `u64` to `usize`' <<<"$out" || true) + if [ -n "$found" ]; then + echo "$found" + status=1 + fi +done +if [ "$status" -ne 0 ]; then echo "==> use addr::to_usize (file values) or addr::saturating_usize (in-memory counts)" >&2 exit 1 fi From 5b3d32b37d06c6929369c71296c18414da216b3b Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 14:09:37 -0500 Subject: [PATCH 08/10] 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)), } } } From 04a7f6f6c7bc40b2ecfbaffc3211c2cda680c10d Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 14:11:47 -0500 Subject: [PATCH 09/10] read: of two links with one name, the first wins everywhere A valid group has one link per name, but a damaged or hand-made one can have two. resolve_child followed the first soft link of the name, the listing skipped a dangling one and listed the name via a later link, and path resolution followed the last symbolic link: three answers. All now take the first link of the name (header message order in a compact group, name index order in a dense one) and ignore the rest, even if the first dangles. That is libhdf5's rule for compact groups (H5G__compact_lookup stops at the first Link message); h5py opens nothing for a dangling first link although a later one resolves. For a dense group libhdf5 binary-searches the index and may land on another of several exact duplicates; documented on first_link_named. find_symbolic_link's v2 branch was dead (only v1 groups reach it) and is now v1-only. Test: an h5py compact group with soft links dup_A (dangling, or to /d) and dup_B (the other), dup_B renamed to dup_A in the header and re-checksummed. Lookup, path and listing through all three readers match h5py for both orders. With the old group_v2.rs the path lookup returned 42 where h5py opens nothing. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 10 ++ crates/clawhdf5-format/src/group_v2.rs | 154 ++++++++++-------- .../clawhdf5/tests/indexed_lookup_interop.rs | 115 +++++++++++++ 3 files changed, 211 insertions(+), 68 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 14f8053..70cd926 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,16 @@ types): one attribute, found in dense storage through its name index (record type 8) instead of reading every attribute (`clawhdf5_format::attribute::find_attribute_in_file`). +- **Two links of one name: the first wins everywhere.** A group cannot + validly hold two links of one name, but a damaged or hand-made one can. + The listing, `resolve_child` (`Group::dataset`/`group`) and path + resolution now all use only the first link of a name (header message + order in a compact group, name index order in a dense one) and ignore + the rest, even if the first dangles — libhdf5's rule for compact groups + (h5py fails to open a dangling first link although a later one + resolves). Before, the listing skipped a dangling first link and listed + the name via a later one that lookup did not follow, and path resolution + followed the last. - **`Group::entries()` and `File::group_at(address)`**: a listing's `(name, address)` pairs, to open children without looking names up again. - Test: `crates/clawhdf5/tests/indexed_lookup_interop.rs` — every child of diff --git a/crates/clawhdf5-format/src/group_v2.rs b/crates/clawhdf5-format/src/group_v2.rs index e1bd9f5..0edf972 100644 --- a/crates/clawhdf5-format/src/group_v2.rs +++ b/crates/clawhdf5-format/src/group_v2.rs @@ -6,6 +6,11 @@ #[cfg(not(feature = "std"))] use alloc::{string::String, vec::Vec}; +#[cfg(not(feature = "std"))] +use alloc::collections::BTreeSet; +#[cfg(feature = "std")] +use std::collections::BTreeSet; + use crate::addr::to_usize; use crate::btree_v2::{BTreeV2Header, collect_btree_v2_records, find_btree_v2_records}; use crate::checksum::jenkins_lookup3; @@ -159,45 +164,34 @@ fn resolve_dense_entries( Ok(entries) } -/// The soft or external link called `name` in this group, if there is one. -/// Hard links are what `resolve_group_entries` returns; this is consulted only -/// when a path component isn't among them. -fn find_symbolic_link( +/// The soft link called `name` in a v1 (symbol table) group, if there is +/// one. Hard links are what `resolve_group_entries` returns; this is +/// consulted only when a path component isn't among them. +fn find_v1_symbolic_link( file_data: &[u8], object_header: &ObjectHeader, name: &str, offset_size: u8, length_size: u8, ) -> Result, FormatError> { - if is_v1_group(object_header) { - let Some(sym_msg) = object_header - .messages - .iter() - .find(|m| m.msg_type == MessageType::SymbolTable) - else { - return Ok(None); - }; - let stm = SymbolTableMessage::parse(&sym_msg.data, offset_size)?; - return group_v1::find_v1_soft_link(file_data, &stm, name, offset_size, length_size) - .map(|target| target.map(|target_path| LinkTarget::Soft { target_path })); - } - if !is_v2_group(object_header) { + let Some(sym_msg) = object_header + .messages + .iter() + .find(|m| m.msg_type == MessageType::SymbolTable) + else { return Ok(None); - } - // As when all links were scanned: the last symbolic link of that name. - Ok( - links_named(file_data, object_header, name, offset_size, length_size)? - .into_iter() - .rev() - .map(|link| link.link_target) - .find(|t| !matches!(t, LinkTarget::Hard { .. })), - ) + }; + let stm = SymbolTableMessage::parse(&sym_msg.data, offset_size)?; + group_v1::find_v1_soft_link(file_data, &stm, name, offset_size, length_size) + .map(|target| target.map(|target_path| LinkTarget::Soft { target_path })) } /// B-tree v2 record type of a dense group's link name index. const LINK_NAME_INDEX: u8 = 5; -/// The links called `name` in a v2 group (a valid group has at most one). +/// The links called `name` in a v2 group (a valid group has at most one), +/// in storage order: header message order for a compact group, name index +/// order for a dense one. /// /// In dense storage the link name index (a v2 B-tree of lookup3 name /// hashes, record type 5) is descended to the records with the name's hash, @@ -272,6 +266,32 @@ fn links_named( Ok(found) } +/// The link called `name` in a v2 group, if any. +/// +/// A valid group has at most one; libhdf5 cannot create two. If a damaged +/// or hand-made group has several, the first wins and the rest are +/// ignored, whatever their kind and even if the first cannot be followed. +/// That is libhdf5's rule for a compact group (`H5G__compact_lookup` stops +/// at the first Link message of that name; h5py then fails to open a +/// dangling first link although a later one resolves). For a dense group +/// "first" is first in name index order; libhdf5 binary-searches the index +/// and may land on another of several exact duplicates. The listing +/// ([`resolve_group_children`]), [`resolve_child`] and path resolution all +/// apply this rule, so they agree. +fn first_link_named( + file_data: &[u8], + object_header: &ObjectHeader, + name: &str, + offset_size: u8, + length_size: u8, +) -> Result, FormatError> { + Ok( + links_named(file_data, object_header, name, offset_size, length_size)? + .into_iter() + .next(), + ) +} + /// The link [`resolve_path_any`] follows for one path component `name` of /// the group with header `object_header`: a hard link (as `Hard`), else a /// soft or external link of that name, else `None`. Fails with @@ -293,29 +313,25 @@ fn lookup_link( object_header_address: e.object_header_address, })); } - return find_symbolic_link(file_data, object_header, name, offset_size, length_size); + return find_v1_symbolic_link(file_data, object_header, name, offset_size, length_size); } if !is_v2_group(object_header) { return Err(FormatError::PathNotFound(String::from( "object header is not a group", ))); } - let links = links_named(file_data, object_header, name, offset_size, length_size)?; - if let Some(addr) = links.iter().find_map(|l| match l.link_target { - LinkTarget::Hard { - object_header_address, - } if object_header_address != u64::MAX => Some(object_header_address), - _ => None, - }) { - return Ok(Some(LinkTarget::Hard { - object_header_address: addr, - })); - } - Ok(links - .into_iter() - .rev() - .map(|link| link.link_target) - .find(|t| !matches!(t, LinkTarget::Hard { .. }))) + Ok( + first_link_named(file_data, object_header, name, offset_size, length_size)? + .map(|link| link.link_target) + .filter(|t| { + !matches!( + t, + LinkTarget::Hard { + object_header_address: u64::MAX + } + ) + }), + ) } /// The object header address of the child called `name` of the group at @@ -342,19 +358,14 @@ pub fn resolve_child( .map(|e| e.object_header_address) .ok_or_else(not_found); } - let links = links_named(file_data, &header, name, os, ls)?; - // The listing puts hard links before resolved soft links. - if let Some(addr) = links.iter().find_map(|l| match l.link_target { - LinkTarget::Hard { + // The first link of that name only, as the listing (see + // `first_link_named`). + match first_link_named(file_data, &header, name, os, ls)?.map(|l| l.link_target) { + Some(LinkTarget::Hard { object_header_address, - } => Some(object_header_address), - _ => None, - }) { - return Ok(addr); - } - for link in links { - if let LinkTarget::Soft { target_path } = link.link_target { - return match resolve_path_from(file_data, superblock, group_address, &target_path) { + }) => Ok(object_header_address), + Some(LinkTarget::Soft { target_path }) => { + match resolve_path_from(file_data, superblock, group_address, &target_path) { // Left out of the listing: dangling, cyclic, or in another file. Err( FormatError::PathNotFound(_) @@ -362,10 +373,10 @@ pub fn resolve_child( | FormatError::ExternalLinkUnsupported { .. }, ) => Err(not_found()), other => other, - }; + } } + Some(LinkTarget::External { .. }) | None => Err(not_found()), } - Err(not_found()) } /// Find and parse the Link Info message from an object header. @@ -471,16 +482,23 @@ pub fn resolve_group_children( } entries.extend(all.into_iter().filter(|e| !group_v1::is_v1_soft_link(e))); } else if is_v2_group(&header) { - let mut visit = |link: LinkMessage| match link.link_target { - LinkTarget::Hard { - object_header_address, - } => entries.push(GroupEntry { - name: link.name, - object_header_address, - cache_type: 0, - }), - LinkTarget::Soft { target_path } => soft.push((link.name, target_path)), - LinkTarget::External { .. } => {} + // Only the first link of each name counts (see `first_link_named`). + let mut seen = BTreeSet::new(); + let mut visit = |link: LinkMessage| { + if !seen.insert(link.name.clone()) { + return; + } + match link.link_target { + LinkTarget::Hard { + object_header_address, + } => entries.push(GroupEntry { + name: link.name, + object_header_address, + cache_type: 0, + }), + LinkTarget::Soft { target_path } => soft.push((link.name, target_path)), + LinkTarget::External { .. } => {} + } }; let link_info = find_link_info(&header, os)?; if let Some(fh_addr) = link_info.fractal_heap_address { diff --git a/crates/clawhdf5/tests/indexed_lookup_interop.rs b/crates/clawhdf5/tests/indexed_lookup_interop.rs index 71f48f0..98b04f0 100644 --- a/crates/clawhdf5/tests/indexed_lookup_interop.rs +++ b/crates/clawhdf5/tests/indexed_lookup_interop.rs @@ -495,3 +495,118 @@ fn a_corrupt_internal_index_node_is_an_error_not_a_missing_name() { )); assert_eq!(out, "checksum checksum"); } + +/// Rename the one link called `from` to `to` (same length) in `bytes`, and +/// re-checksum the object header chunk holding it: two links of one name, +/// which libhdf5 cannot write. +fn rename_link_in_header(bytes: &mut [u8], from: &[u8], to: &[u8]) { + assert_eq!(from.len(), to.len()); + let find = |hay: &[u8], needle: &[u8]| hay.windows(needle.len()).position(|w| w == needle); + let at = find(bytes, from).expect("link name"); + assert!(find(&bytes[at + 1..], from).is_none(), "name not unique"); + bytes[at..at + to.len()].copy_from_slice(to); + // The v2 object header (chunk 0) holding it. + let ohdr = bytes[..at] + .windows(4) + .rposition(|w| w == b"OHDR") + .expect("OHDR"); + let flags = bytes[ohdr + 5]; + let mut pos = ohdr + 6; + if flags & 0x20 != 0 { + pos += 16; // times + } + if flags & 0x10 != 0 { + pos += 4; // attribute phase change + } + let width = 1usize << (flags & 3); + let mut size = [0u8; 8]; + size[..width].copy_from_slice(&bytes[pos..pos + width]); + let end = pos + width + usize::try_from(u64::from_le_bytes(size)).unwrap(); + assert!(at < end, "name outside chunk 0"); + let sum = jenkins_lookup3(&bytes[ohdr..end]); + bytes[end..end + 4].copy_from_slice(&sum.to_le_bytes()); +} + +/// Two soft links of one name (a damaged or hand-made group; libhdf5 +/// cannot create one), one dangling: only the first counts, as in libhdf5, +/// which opens the first Link message of a name and fails if it dangles. +/// Lookup, path and listing agree — before, the listing skipped a dangling +/// first link and listed the name via the second, which lookup did not +/// follow, and path resolution followed the last. +#[test] +fn of_two_links_with_one_name_the_first_wins_everywhere() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + for dangling_first in [true, false] { + let path = dir + .path() + .join(format!("dup_{dangling_first}.h5")) + .display() + .to_string(); + let (first, second) = if dangling_first { + ("/nowhere_xyz", "/d") + } else { + ("/d", "/nowhere_xyz") + }; + run_python(&format!( + "import h5py, numpy as np\n\ + with h5py.File(r'{path}', 'w', libver='latest') as f:\n\ + \x20 f.create_dataset('d', data=np.int64(42))\n\ + \x20 s = f.create_group('s')\n\ + \x20 s['dup_A'] = h5py.SoftLink('{first}')\n\ + \x20 s['dup_B'] = h5py.SoftLink('{second}')", + )); + let mut bytes = std::fs::read(&path).unwrap(); + rename_link_in_header(&mut bytes, b"dup_B", b"dup_A"); + std::fs::write(&path, &bytes).unwrap(); + + // What libhdf5 opens under that name: both names listed, first link + // followed. + let out = run_python(&format!( + "import h5py\n\ + with h5py.File(r'{path}', 'r') as f:\n\ + \x20 s = f['s']\n\ + \x20 assert list(s) == ['dup_A', 'dup_A'], list(s)\n\ + \x20 try:\n\ + \x20 print(int(s['dup_A'][()]))\n\ + \x20 except KeyError:\n\ + \x20 print('none')", + )); + let want = if dangling_first { "none" } else { "42" }; + assert_eq!(out, want, "h5py, dangling first: {dangling_first}"); + let want = (!dangling_first).then_some(42i64); + + let f = File::open(&path).unwrap(); + let s = f.group("s").unwrap(); + let got = |r: Result, clawhdf5::Error>| match r { + Ok(ds) => Some(ds.read_i64().unwrap()[0]), + Err(e) => { + assert!(is_not_found(&e), "{e:?}"); + None + } + }; + assert_eq!(got(s.dataset("dup_A")), want, "lookup, {dangling_first}"); + assert_eq!(got(f.dataset("/s/dup_A")), want, "path, {dangling_first}"); + let listed = s.datasets().unwrap(); + let listed_n = listed.iter().filter(|n| *n == "dup_A").count(); + assert_eq!(listed_n, usize::from(want.is_some()), "{listed:?}"); + let entries = s.entries().unwrap(); + assert_eq!(entries.len(), listed_n, "{entries:?}"); + + let m = MmapFile::open(&path).unwrap(); + let l = LazyFile::open_mmap(&path).unwrap(); + let (mg, lg) = (m.group("s").unwrap(), l.group("s").unwrap()); + match want { + Some(v) => { + assert_eq!(mg.dataset("dup_A").unwrap().read_i64().unwrap(), vec![v]); + assert_eq!(lg.dataset("dup_A").unwrap().read_i64().unwrap(), vec![v]); + } + None => { + assert!(mg.dataset("dup_A").is_err_and(|e| is_not_found(&e))); + assert!(lg.dataset("dup_A").is_err_and(|e| is_not_found(&e))); + } + } + assert_eq!(mg.datasets().unwrap(), listed); + assert_eq!(lg.datasets().unwrap(), listed); + } +} From 1ea9132e10135e20c55536497bffc520814d0fdf Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 14:11:47 -0500 Subject: [PATCH 10/10] docs: changelog note for B-tree v2 internal node checksums Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 70cd926..33e6bb6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,10 @@ resolves). Before, the listing skipped a dangling first link and listed the name via a later one that lookup did not follow, and path resolution followed the last. +- **B-tree v2 internal nodes are checksum-verified.** Lookups prune + children by internal-node keys, so a corrupted internal node could hide + a name with no error; a mismatch is now `ChecksumMismatch`, as in + libhdf5, for lookups and listings alike. - **`Group::entries()` and `File::group_at(address)`**: a listing's `(name, address)` pairs, to open children without looking names up again. - Test: `crates/clawhdf5/tests/indexed_lookup_interop.rs` — every child of