fix(format): measure compound members by their stored size

The compound overlap check measured each earlier member with
Datatype::type_size, which is a fixed 16 for a variable-length type. On
disk a VL member takes 4 + offset size + 4 bytes, 12 in a file with
4-byte offsets, so a member right after one was refused as "member
overlaps with previous member" (and with the type, every attribute of
the object). libhdf5 measures members by their decoded, stored size
(times a v1 member's array dimensions); so does this now.

Reading VL values in such files is a separate, older gap, now recorded
in known-issues.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
osobh
2026-09-26 01:24:17 -05:00
co-authored by Claude Opus 5.5
parent 17f09375ad
commit 9238605661
4 changed files with 78 additions and 10 deletions
+3
View File
@@ -321,6 +321,9 @@
half its bits unused (`Datatype::parse_in_header`, half its bits unused (`Datatype::parse_in_header`,
`Datatype::check_unused_bits`). A v1/v2 float's class bit 6 was read as `Datatype::check_unused_bits`). A v1/v2 float's class bit 6 was read as
VAX byte order; libhdf5 ignores it before version 3, and so does this. VAX byte order; libhdf5 ignores it before version 3, and so does this.
The overlap check measures each earlier member by its stored size, as
libhdf5 does, so a variable-length member (4 + offset size + 4 bytes)
in a file with 4-byte offsets does not overlap the member after it.
- chunked layouts (`FormatError::InvalidChunkDimensions`): a zero chunk - chunked layouts (`FormatError::InvalidChunkDimensions`): a zero chunk
dimension, a chunk rank that does not match the dataspace, a chunk of dimension, a chunk rank that does not match the dataspace, a chunk of
4 GiB or more indexed by a v1 B-tree (layout version 3 or earlier; 4 GiB or more indexed by a v1 B-tree (layout version 3 or earlier;
+18 -10
View File
@@ -215,11 +215,6 @@ fn stored_type_size(data: &[u8], pos: usize) -> Result<u32, FormatError> {
Ok(LittleEndian::read_u32(&data[pos + 4..pos + 8])) Ok(LittleEndian::read_u32(&data[pos + 4..pos + 8]))
} }
/// A compound member's size in the compound, as libhdf5 counts it.
fn member_size(dt: &Datatype) -> u64 {
u64::from(dt.type_size())
}
/// libhdf5 refuses an array type of more than `H5S_MAX_RANK` (32) /// libhdf5 refuses an array type of more than `H5S_MAX_RANK` (32)
/// dimensions. /// dimensions.
fn check_array_rank(ndims: usize) -> Result<(), FormatError> { fn check_array_rank(ndims: usize) -> Result<(), FormatError> {
@@ -543,11 +538,17 @@ impl Datatype {
return Err(invalid("invalid number of members: 0")); return Err(invalid("invalid number of members: 0"));
} }
let mut members: Vec<CompoundMember> = Vec::with_capacity(num_members as usize); let mut members: Vec<CompoundMember> = Vec::with_capacity(num_members as usize);
// Each member's size in the compound as libhdf5 decodes it:
// its stored size, times a v1 member's array dimensions. A
// variable-length member takes 4 + offset size + 4 bytes on
// disk, not the 16 of `Datatype::type_size`.
let mut member_sizes: Vec<u64> = Vec::with_capacity(num_members as usize);
// libhdf5 checks each member as it is decoded: it must fit in // libhdf5 checks each member as it is decoded: it must fit in
// the compound (by its own stored size, before a v1 member's // the compound (by its own stored size, before a v1 member's
// array dimensions are applied), and must not repeat a name // array dimensions are applied), and must not repeat a name
// or overlap an earlier member (by its final size). // or overlap an earlier member (by its final size).
let check_member = |members: &[CompoundMember], let check_member = |members: &[CompoundMember],
member_sizes: &[u64],
name: &str, name: &str,
byte_offset: u64, byte_offset: u64,
stored_size: u32, stored_size: u32,
@@ -565,9 +566,8 @@ impl Datatype {
))); )));
} }
let end = byte_offset + final_size; let end = byte_offset + final_size;
if members.iter().any(|m| { if members.iter().zip(member_sizes).any(|(m, &m_size)| {
let m_end = m.byte_offset + member_size(&m.datatype); byte_offset < m.byte_offset + m_size && m.byte_offset < end
byte_offset < m_end && m.byte_offset < end
}) { }) {
return Err(invalid("member overlaps with previous member")); return Err(invalid("member overlaps with previous member"));
} }
@@ -588,13 +588,16 @@ impl Datatype {
let (member_dt, consumed) = let (member_dt, consumed) =
Self::parse_with_depth(&data[pos..], depth + 1)?; Self::parse_with_depth(&data[pos..], depth + 1)?;
pos += consumed; pos += consumed;
let final_size = u64::from(stored_size);
check_member( check_member(
&members, &members,
&member_sizes,
&name, &name,
byte_offset, byte_offset,
stored_size, stored_size,
member_size(&member_dt), final_size,
)?; )?;
member_sizes.push(final_size);
members.push(CompoundMember { members.push(CompoundMember {
name, name,
byte_offset, byte_offset,
@@ -652,6 +655,9 @@ impl Datatype {
let (mut member_dt, consumed) = let (mut member_dt, consumed) =
Self::parse_with_depth(&data[pos..], depth + 1)?; Self::parse_with_depth(&data[pos..], depth + 1)?;
pos += consumed; pos += consumed;
let final_size = array_dims.iter().fold(u64::from(stored_size), |a, &d| {
a.saturating_mul(u64::from(d))
});
if !array_dims.is_empty() { if !array_dims.is_empty() {
member_dt = Datatype::Array { member_dt = Datatype::Array {
base_type: Box::new(member_dt), base_type: Box::new(member_dt),
@@ -660,11 +666,13 @@ impl Datatype {
} }
check_member( check_member(
&members, &members,
&member_sizes,
&name, &name,
byte_offset, byte_offset,
stored_size, stored_size,
member_size(&member_dt), final_size,
)?; )?;
member_sizes.push(final_size);
members.push(CompoundMember { members.push(CompoundMember {
name, name,
byte_offset, byte_offset,
@@ -313,3 +313,54 @@ with h5py.File(os.path.join(d, "big.h5"), "r") as f:
let values = file.dataset("d").unwrap().read_f64().unwrap(); let values = file.dataset("d").unwrap().read_f64().unwrap();
assert_eq!(values, (0..10).map(f64::from).collect::<Vec<_>>()); assert_eq!(values, (0..10).map(f64::from).collect::<Vec<_>>());
} }
/// A variable-length compound member takes 4 + offset size + 4 bytes, which
/// is 12 in a file with 4-byte offsets, and libhdf5 checks for overlapping
/// members with that stored size. A member right after one was refused as
/// "member overlaps with previous member" (the check took the 16 bytes of
/// an 8-byte-offset file), and with it every attribute of the object.
#[test]
fn variable_length_compound_members_in_files_with_4_byte_offsets() {
skip_if_no_python!();
let dir = tempfile::tempdir().unwrap();
run_python(
dir.path(),
r#"
from h5py import h5f, h5p
dt = np.dtype([("s", h5py.string_dtype()), ("i", "<i4")])
a = np.array([("x", 1), ("yy", 2)], dtype=dt)
for libver in ("earliest", "latest"):
fcpl = h5p.create(h5p.FILE_CREATE)
fcpl.set_sizes(4, 4)
fapl = h5p.create(h5p.FILE_ACCESS)
low = h5f.LIBVER_EARLIEST if libver == "earliest" else h5f.LIBVER_LATEST
fapl.set_libver_bounds(low, h5f.LIBVER_LATEST)
path = os.path.join(d, f"{libver}.h5")
with h5py.File(h5f.create(path.encode(), h5f.ACC_TRUNC, fcpl=fcpl, fapl=fapl)) as f:
f.create_dataset("c", data=a)
f.attrs["c"] = a
f.attrs["note"] = "kept"
with h5py.File(path, "r") as f:
assert f["c"][1]["i"] == 2
assert f.attrs["note"] == "kept"
"#,
);
for libver in ["earliest", "latest"] {
let file = File::open(dir.path().join(format!("{libver}.h5"))).unwrap();
let dtype = file.dataset("c").unwrap().dtype();
assert!(
matches!(&dtype, Ok(clawhdf5::DType::Compound(fields))
if fields.iter().map(|f| f.0.as_str()).eq(["s", "i"])),
"{libver}: {dtype:?}"
);
// (Variable-length values in files with 4-byte offsets are not
// decoded yet; see docs/known-issues.md. The attributes must at
// least be listed without errors.)
let (attrs, errors) = file.root().attrs_with_errors().unwrap();
assert!(errors.is_empty(), "{libver}: {errors:?}");
assert!(
attrs.contains_key("c") && attrs.contains_key("note"),
"{libver}: {attrs:?}"
);
}
}
+6
View File
@@ -129,6 +129,12 @@ fill-value item that did is fixed).
failing the others. failing the others.
- **Other readers:** - **Other readers:**
- VL-string datasets are not readable through `File`. - VL-string datasets are not readable through `File`.
- Variable-length values inside a compound (and VL-string attributes) in
a file with 4-byte offsets (`sizeof_addr = 4`) fail with
`GlobalHeapObjectNotFound` or come back as `Raw`: these paths assume
the 16-byte element of an 8-byte-offset file. The datatype itself reads
(it was refused as "member overlaps with previous member" until
2026-09-26).
- Metadata cache images are not supported. - Metadata cache images are not supported.
- x87 long double and binary128 are refused. - x87 long double and binary128 are refused.
- N-Bit on 64-bit scale-offset data and some N-Bit parameter layouts fail. - N-Bit on 64-bit scale-offset data and some N-Bit parameter layouts fail.