From b6cbd2319f9d156a19ef077d10e2647bab8d05b7 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 14:08:57 -0500 Subject: [PATCH] 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