From bd1d8f1a593d8d6e5842f9e8dffdfb981f45f245 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 08:58:49 -0500 Subject: [PATCH] fix(format): keep a dense index leaf within 65 535 records The link and attribute name indexes are one v2 B-tree leaf, sized to the next power of two. libhdf5 takes a leaf's capacity from that node size, but a leaf's record count is a 2-byte field. From about 47 700 links the node had room for more than 65 535 records, so adding a link in h5py overflowed the count: a group of 65 535 links crashed h5py, or could no longer be listed ("unknown link class"). The node is now capped at a full leaf of 65 535 records, so libhdf5 splits it instead. Dense attributes now go through the same index builder. Their record count was written modulo 65 536, without error; more than 65 535 attributes on one object are now refused, like links. Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/clawhdf5-format/src/file_writer.rs | 71 ++++++------------- .../clawhdf5/tests/writer_groups_interop.rs | 30 ++++++-- 2 files changed, 48 insertions(+), 53 deletions(-) diff --git a/crates/clawhdf5-format/src/file_writer.rs b/crates/clawhdf5-format/src/file_writer.rs index 65181a0..0b318a9 100644 --- a/crates/clawhdf5-format/src/file_writer.rs +++ b/crates/clawhdf5-format/src/file_writer.rs @@ -903,9 +903,6 @@ pub(crate) fn build_dense_attrs( .map(|a| crate::checksum::jenkins_lookup3(a.name.as_bytes())) .collect(); - let os = OFFSET_SIZE as usize; - let ls = LENGTH_SIZE as usize; - // Attribute heaps use max_heap_size 40 / heap ID length 8 (matching libhdf5). let heap = build_single_block_fractal_heap(&serialized, base_address, 40, 8)?; let frhp_addr = heap.frhp_addr; @@ -926,48 +923,16 @@ pub(crate) fn build_dense_attrs( } records.sort_by(|a, b| a.0.cmp(&b.0).then(a.1.cmp(&b.1))); - let bthd_size = 4 + 1 + 1 + 4 + 2 + 2 + 1 + 1 + os + 2 + ls + 4; - let num_records = attrs.len(); - let btlf_size = 4 + 1 + 1 + (num_records * record_size as usize) + 4; - let node_size = btlf_size.next_power_of_two().max(512) as u32; - + let records: Vec> = records.into_iter().map(|(_, _, rec)| rec).collect(); let bthd_addr = btree_addr; - let btlf_addr = bthd_addr + bthd_size as u64; - - let mut bthd = Vec::with_capacity(bthd_size); - bthd.extend_from_slice(b"BTHD"); - bthd.push(0); // version - bthd.push(8); // type = attribute name index - bthd.extend_from_slice(&node_size.to_le_bytes()); - bthd.extend_from_slice(&record_size.to_le_bytes()); - bthd.extend_from_slice(&0u16.to_le_bytes()); // depth = 0 - bthd.push(100); // split_percent - bthd.push(40); // merge_percent - write_offset(&mut bthd, btlf_addr, OFFSET_SIZE); - bthd.extend_from_slice(&(num_records as u16).to_le_bytes()); - write_length(&mut bthd, num_records as u64, LENGTH_SIZE); - let bthd_checksum = crate::checksum::jenkins_lookup3(&bthd); - bthd.extend_from_slice(&bthd_checksum.to_le_bytes()); - debug_assert_eq!(bthd.len(), bthd_size); - - let mut btlf = Vec::with_capacity(node_size as usize); - btlf.extend_from_slice(b"BTLF"); - btlf.push(0); // version - btlf.push(8); // type - for (_, _, rec) in &records { - btlf.extend_from_slice(rec); - } - // Checksum goes immediately after records (NOT at end of node). - // HDF5 C library computes checksum over sig+ver+type+records only. - let btlf_checksum = crate::checksum::jenkins_lookup3(&btlf); - btlf.extend_from_slice(&btlf_checksum.to_le_bytes()); - // Pad to node_size - btlf.resize(node_size as usize, 0); - - let mut blob = Vec::with_capacity(heap.blob.len() + bthd.len() + btlf.len()); - blob.extend_from_slice(&heap.blob); - blob.extend_from_slice(&bthd); - blob.extend_from_slice(&btlf); + let mut blob = heap.blob; + blob.extend_from_slice(&single_leaf_v2_btree( + 8, + record_size, + &records, + bthd_addr, + "attributes on one object", + )?); let attr_info = serialize_attribute_info(frhp_addr, bthd_addr); @@ -989,12 +954,14 @@ pub(crate) struct DenseLinkBlob { } /// A v2 B-tree of `btree_type` holding `records` (already in key order) in a -/// single leaf, laid out at `addr`: the header, then the leaf. +/// single leaf, laid out at `addr`: the header, then the leaf. `what` names +/// the records in the error for too many ("links in one group"). fn single_leaf_v2_btree( btree_type: u8, record_size: u16, records: &[Vec], addr: u64, + what: &str, ) -> Result, FormatError> { let os = OFFSET_SIZE as usize; let ls = LENGTH_SIZE as usize; @@ -1002,15 +969,21 @@ fn single_leaf_v2_btree( // internal nodes, which the writer does not build. let num_records = u16::try_from(records.len()).map_err(|_| { FormatError::SerializationError(format!( - "{} links in one group: a group holds at most {} links \ - (a deeper link index is not implemented)", + "{} {what}: at most {} can be written \ + (a deeper B-tree index is not implemented)", records.len(), u16::MAX )) })?; let bthd_size = 4 + 1 + 1 + 4 + 2 + 2 + 1 + 1 + os + 2 + ls + 4; let btlf_size = 4 + 1 + 1 + (records.len() * record_size as usize) + 4; - let node_size = btlf_size.next_power_of_two().max(512) as u32; + // libhdf5 sizes a leaf's capacity from the node size, and a leaf's + // record count is a 2-byte field: a node with room for more than + // 65 535 records makes it overflow that count when it adds one (the + // group can then no longer be listed). Cap the node at a full leaf. + let max_node = btlf_size - records.len() * record_size as usize + + usize::from(u16::MAX) * record_size as usize; + let node_size = btlf_size.next_power_of_two().max(512).min(max_node) as u32; let btlf_addr = addr + bthd_size as u64; let mut out = Vec::with_capacity(bthd_size + node_size as usize); @@ -1089,6 +1062,7 @@ pub(crate) fn build_dense_links( 4 + heap_id_length, &name_records, name_bt_addr, + "links in one group", )?); let link_info_message = if track_order { @@ -1113,6 +1087,7 @@ pub(crate) fn build_dense_links( 8 + heap_id_length, &order_records, order_bt_addr, + "links in one group", )?); let next_order = by_order.last().map_or(0, |&(o, _)| o + 1); serialize_link_info( diff --git a/crates/clawhdf5/tests/writer_groups_interop.rs b/crates/clawhdf5/tests/writer_groups_interop.rs index 770b835..27379bb 100644 --- a/crates/clawhdf5/tests/writer_groups_interop.rs +++ b/crates/clawhdf5/tests/writer_groups_interop.rs @@ -538,7 +538,23 @@ fn more_links_than_one_index_leaf_holds_is_an_error() { b.add_soft_link(&format!("s{i}"), "/x"); } let err = b.finish().unwrap_err().to_string(); - assert!(err.contains("at most 65535 links"), "{err}"); + assert!( + err.contains("70000 links in one group: at most 65535"), + "{err}" + ); + // Dense attributes have the same one-leaf index. Their count used to + // be written modulo 65 536. + let mut b = FileBuilder::new(); + let x = b.create_dataset("x"); + x.with_i32_data(&[1]); + for i in 0..70_000 { + x.set_attr(&format!("a{i}"), AttrValue::I64(i)); + } + let err = b.finish().unwrap_err().to_string(); + assert!( + err.contains("70000 attributes on one object: at most 65535"), + "{err}" + ); } #[test] @@ -712,17 +728,21 @@ fn dense_links_past_the_direct_blocks_of_the_root() { // libhdf5 can add to and delete from the heap. It could not when the // header's block allocation offset was 0: its next block overwrote the - // first ("bad version number for message"). + // first ("bad version number for message"). Adding to `deep` also + // needs its index's leaf node to have room for at most 65 535 records: + // a bigger node made libhdf5 overflow the leaf's 2-byte record count + // (a crash, or "unknown link class" when listing). let out = h5py( &path, "with h5py.File(path, 'r+') as f:\n\ \x20 f['g']['zz_new'] = np.arange(3)\n\ + \x20 f['deep']['zz_new'] = np.arange(4)\n\ \x20 del f['g/dataset_number_000005']\n\ with h5py.File(path, 'r') as f:\n\ - \x20 print(json.dumps([len(f['g']), int(f['g/zz_new'][2]),\n\ - \x20 int(f['g/dataset_number_019998'][0])]))", + \x20 print(json.dumps([len(f['g']), int(f['g/zz_new'][2]), len(f['deep']),\n\ + \x20 list(f['deep'])[-1], int(f['g/dataset_number_019998'][0])]))", ); - assert_eq!(out, r#"[20000, 2, 19998]"#); + assert_eq!(out, r#"[20000, 2, 65536, "zz_new", 19998]"#); h5dump_ok(&path); }