From 8196fab72a078b57118910b6a5ba42577d276d85 Mon Sep 17 00:00:00 2001 From: osobh Date: Fri, 25 Sep 2026 21:53:00 -0500 Subject: [PATCH] fix(format): read v2 B-tree internal nodes with libhdf5's pointer widths An internal node's child pointer is an address, the child's record count and (below the first internal level) the child subtree's total record count. libhdf5 (H5B2__hdr_init) encodes the record count in the width of a leaf's maximum and the subtree total in the width of cum_max_nrec for that depth, computed level by level from the node size. The reader guessed 2 * leaf_max and leaf_max^depth, which agree at depth 2 but not at depth 3: a 24 000-link group's name index has depth 3, its root's pointers were read 3 bytes wide instead of 2, and listing failed with a garbage heap offset. Regression tests: dense_group_with_a_three_level_name_index (h5py writes 24 000 links; listing compared with h5py) and subtree_capacity_matches_libhdf5. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 5 + crates/clawhdf5-format/src/btree_v2.rs | 104 +++++++++++------- .../clawhdf5/tests/dense_storage_interop.rs | 22 ++++ docs/known-issues.md | 5 +- 4 files changed, 94 insertions(+), 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a7272ba..9d94b66 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -305,6 +305,11 @@ with libhdf5's defaults: a few thousand long link names, or ~20 000 short ones) could not be listed: child indirect blocks were given the wrong number of rows, so every link stored in one was unreachable. + - v2 B-trees of depth 3 or more (a dense group of ~22 000+ links) were + misparsed: internal-node child pointers were read with widths from an + estimate instead of libhdf5's per-depth record capacities, and the + listing failed. The same B-tree code indexes dense attributes, shared + messages and chunks. - `clawhdf5-format` writer — **files libhdf5 rejects or reads wrong:** - Extensible Array (one unlimited dimension): chunks from index 244 on were written but never indexed and read as 0, by libhdf5 and by us. diff --git a/crates/clawhdf5-format/src/btree_v2.rs b/crates/clawhdf5-format/src/btree_v2.rs index 43fd7b0..0b70981 100644 --- a/crates/clawhdf5-format/src/btree_v2.rs +++ b/crates/clawhdf5-format/src/btree_v2.rs @@ -323,39 +323,21 @@ fn collect_internal_records( let records_start = pos; pos += records_total; - // Compute sizes for child pointers - // max_records at child depth - for variable-width nrec encoding + // Child pointer layout, as libhdf5 computes it (H5B2__hdr_init): the + // child's record count is always encoded in the width needed for a + // *leaf's* maximum, and — below the first internal level — the child + // subtree's total record count in the width needed for the most records + // a subtree of that depth can hold. let child_depth = depth - 1; - let max_nrec_child = if child_depth == 0 { - max_leaf_nrec - } else { - // For internal nodes at child_depth, the true max_nrec depends on the - // node size, record size, and the recursive width of child pointer - // entries (which themselves depend on max_nrec at deeper levels). - // Computing the exact value requires iterating from the leaf level - // upward, as described in the HDF5 spec (III.A.2 "Computing the Size - // of B-tree Nodes"). - // - // We use `max_leaf_nrec * 2` as a conservative upper bound. This - // over-estimates the nrec encoding width, which means we may read - // slightly more bytes per child pointer than strictly necessary, but - // never fewer. The over-read bytes are harmless because we only - // decode `num_records` entries (the actual count from the node header). - // - // Known limitation: for very deep trees (depth > 3) with small record - // sizes, the true max could exceed this estimate, causing us to - // under-allocate the nrec encoding width and misparse child pointers. - // In practice, HDF5 B-tree v2 depths rarely exceed 2-3. - max_leaf_nrec * 2 - }; - let nrec_width = bytes_for_max_records(max_nrec_child); - - // Total records in subtree width (only if depth > 1) + let nrec_width = bytes_for_max_records(max_leaf_nrec); let total_nrec_width = if depth > 1 { - // Width to hold total records in a subtree - // We compute max possible total records at this subtree depth - let max_total = header_max_total_records(max_leaf_nrec, depth - 1); - bytes_for_max_records(max_total) + bytes_for_max_records(cum_max_records( + node_size, + record_size, + offset_size, + max_leaf_nrec, + child_depth, + )) } else { 0 }; @@ -435,14 +417,36 @@ fn collect_internal_records( Ok(()) } -/// Estimate maximum total records at a given depth (for variable-width encoding). -fn header_max_total_records(max_leaf_nrec: u64, depth: u16) -> u64 { - // Conservative: branching factor * max_leaf at each level - let mut total = max_leaf_nrec; - for _ in 0..depth { - total = total.saturating_mul(max_leaf_nrec.max(2)); +/// Most records a subtree whose root is at `depth` can hold (libhdf5's +/// `cum_max_nrec`): a leaf holds `max_leaf_nrec`; an internal node at depth +/// `d` holds `max_nrec(d)` records and `max_nrec(d) + 1` subtrees of depth +/// `d - 1`, where `max_nrec(d)` is what fits in a node once each record is +/// paired with a child pointer of the width depth `d` needs. +fn cum_max_records( + node_size: u32, + record_size: u16, + offset_size: u8, + max_leaf_nrec: u64, + depth: u16, +) -> u64 { + // Internal node overhead: signature(4) + version(1) + type(1) + checksum(4). + const PREFIX: u64 = 10; + let nrec_width = bytes_for_max_records(max_leaf_nrec) as u64; + let mut cum = max_leaf_nrec; + let mut cum_width = 0u64; + for d in 1..=depth { + let ptr = u64::from(offset_size) + nrec_width + if d > 1 { cum_width } else { 0 }; + let max_nrec = u64::from(node_size) + .saturating_sub(PREFIX) + .saturating_sub(ptr) + / (u64::from(record_size) + ptr).max(1); + cum = max_nrec + .saturating_add(1) + .saturating_mul(cum) + .saturating_add(max_nrec); + cum_width = bytes_for_max_records(cum) as u64; } - total + cum } #[cfg(test)] @@ -512,9 +516,15 @@ mod tests { child_nrec: u64, ) -> Vec { let max_leaf = max_records_leaf(node_size, record_size); - let nrec_width = bytes_for_max_records(if depth == 1 { max_leaf } else { max_leaf * 2 }); + let nrec_width = bytes_for_max_records(max_leaf); let total_width = if depth > 1 { - bytes_for_max_records(header_max_total_records(max_leaf, depth - 1)) + bytes_for_max_records(cum_max_records( + node_size, + record_size, + 8, + max_leaf, + depth - 1, + )) } else { 0 }; @@ -673,4 +683,18 @@ mod tests { let records = collect_btree_v2_records(&header, &hdr, 8, 8).unwrap(); assert!(records.is_empty()); } + + #[test] + fn subtree_capacity_matches_libhdf5() { + // A link-name index (11-byte records, 512-byte nodes, 8-byte + // addresses): libhdf5's H5B2__hdr_init gives 45 records per leaf, + // then cum_max_nrec 1 149 at depth 1 and 26 449 at depth 2 — two + // bytes of subtree count in a depth-3 root's child pointers, where + // leaf_max^3 = 91 125 would need three. + let leaf = max_records_leaf(512, 11); + assert_eq!(leaf, 45); + assert_eq!(cum_max_records(512, 11, 8, leaf, 0), 45); + assert_eq!(cum_max_records(512, 11, 8, leaf, 1), 1_149); + assert_eq!(cum_max_records(512, 11, 8, leaf, 2), 26_449); + } } diff --git a/crates/clawhdf5/tests/dense_storage_interop.rs b/crates/clawhdf5/tests/dense_storage_interop.rs index e808d85..ecfff7a 100644 --- a/crates/clawhdf5/tests/dense_storage_interop.rs +++ b/crates/clawhdf5/tests/dense_storage_interop.rs @@ -141,3 +141,25 @@ fn dense_group_whose_heap_outgrows_the_root_direct_rows() { let last = format!("g/n02499_{}", "x".repeat(240)); assert_eq!(f.dataset(&last).unwrap().read_f64().unwrap(), vec![1.0]); } + +#[test] +fn dense_group_with_a_three_level_name_index() { + skip_if_no_python!(); + // 24 000 links give the link-name v2 B-tree a depth of 3. Internal-node + // child pointers carry the subtree's total record count in a width that + // depends on the most records a subtree can hold; the reader estimated + // that as leaf_max^depth, read the root's pointers 3 bytes wide instead + // of 2, and decoded garbage heap IDs. + let (_dir, path) = h5py_file( + "t = f.create_dataset('t', data=[1.0])\n\ + g = f.create_group('g')\n\ + for i in range(24000):\n\ + \x20 g['l%06d' % i] = t\n", + ); + assert_same_listing(&path, "g"); + let f = File::open(&path).unwrap(); + assert_eq!( + f.dataset("g/l023999").unwrap().read_f64().unwrap(), + vec![1.0] + ); +} diff --git a/docs/known-issues.md b/docs/known-issues.md index 59064cb..39cdcf2 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -77,8 +77,9 @@ the VDS item, which is marked. - **Groups and links:** - Groups with a user-defined link type (e.g. 187) cannot be listed. - Dense groups with more than about 22 000 links cannot be listed. - *Partly fixed 2026-09-25:* a link heap past its root block's direct rows - (child indirect blocks) is now read. + **Fixed 2026-09-25:** two bugs — fractal-heap child indirect blocks had + the wrong row count, and v2 B-tree internal nodes at depth 3+ were read + with the wrong pointer widths. - Soft links are left out of `datasets()`. - **Dense attributes:** a large attribute stored as a fractal-heap "huge" object makes every attribute on the object fail. This affects real NetCDF