fix(format): resolve cache-image flush-dependency parents as libhdf5 does

The review suggested libhdf5 loads every image entry and resolves
flush-dependency parents afterwards. It does not:
H5C__reconstruct_cache_contents (HDF5 1.14.6 and 2.0.0, and develop)
inserts each entry and then searches the cache index for its parents in
the same loop, failing with "fd parent not in cache?!?" when one is
missing. So a parent must be an earlier image entry, as before, or
metadata cached before the image loads: the superblock (address 0) and
the superblock extension's object header, which libhdf5 reads to find
the image. Those two were refused as parents; they are now accepted.
A parent listed after its child is still refused, as libhdf5 refuses
it, and so is an entry that is its own parent ("Child entry flush
dependency parent can't be itself").

apply_cache_image takes the superblock to know the extension address.

Test: superblock_ext::tests::flush_dependency_parents_must_already_be_cached
(parent-first loads, child-first refused, extension header accepted,
self-parent refused); the extension-header case fails without the fix.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
osobh
2026-09-26 11:36:10 -05:00
co-authored by Claude Opus 5.5
parent 742ed4dfb8
commit a6ed3a5c7d
2 changed files with 81 additions and 12 deletions
+1 -1
View File
@@ -731,7 +731,7 @@ fn main() {
let view = match ext.and_then(|x| x.cache_image) {
None => None,
Some(loc) => match guarded(|| {
superblock_ext::apply_cache_image(hdf5, loc, sb.offset_size, sb.length_size)
superblock_ext::apply_cache_image(hdf5, loc, &sb)
.map_err(e)
}) {
Ok(v) => Some(v),
+80 -11
View File
@@ -297,16 +297,16 @@ struct ImageEntry {
/// (`H5C__decode_cache_image_header`, `H5C__reconstruct_cache_entry`):
/// signature and version, the image length it records, entry types, rings
/// and ages in range, entry addresses inside the file and not repeated,
/// flush-dependency parents that are earlier entries.
/// flush-dependency parents already in the cache.
///
/// libhdf5 does not verify the block's trailing checksum when it loads an
/// image, so neither does this.
pub fn apply_cache_image(
data: &[u8],
location: CacheImageLocation,
offset_size: u8,
length_size: u8,
sb: &Superblock,
) -> Result<Vec<u8>, FormatError> {
let (offset_size, length_size) = (sb.offset_size, sb.length_size);
let bad = FormatError::InvalidCacheImage;
let start = usize::try_from(location.address).map_err(|_| bad("address out of range"))?;
let len = usize::try_from(location.length).map_err(|_| bad("length out of range"))?;
@@ -336,6 +336,17 @@ pub fn apply_cache_image(
}
let mut entries = Vec::new();
// What is in libhdf5's cache when it loads the image: the superblock
// and the superblock extension's object header (read to find the image).
// Each entry's flush-dependency parents are looked up in the cache as
// the entry is inserted (`H5C__reconstruct_cache_contents` searches the
// index inside the loop that inserts the entries, in HDF5 1.14.6 and
// 2.0.0 alike), so a parent must be one of those or an earlier entry.
let mut cached = BTreeSet::new();
cached.insert(0);
if let Some(ext) = sb.superblock_extension_address {
cached.insert(ext);
}
let mut seen = BTreeSet::new();
for _ in 0..n_entries {
let type_id = c.u8()?;
@@ -374,8 +385,8 @@ pub fn apply_cache_image(
let parent = c
.addr(offset_size)?
.ok_or(bad("invalid flush dependency parent offset"))?;
if !seen.contains(&parent) {
return Err(bad("flush dependency parent not in the image"));
if !seen.contains(&parent) && !cached.contains(&parent) {
return Err(bad("fd parent not in cache"));
}
}
let len = usize::try_from(size).map_err(|_| bad(RAN_OFF))?;
@@ -415,7 +426,7 @@ pub fn metadata_view(data: &[u8], sb: &Superblock) -> Result<Option<Vec<u8>>, Fo
Some(SuperblockExtension {
cache_image: Some(location),
..
}) => apply_cache_image(data, location, sb.offset_size, sb.length_size).map(Some),
}) => apply_cache_image(data, location, sb).map(Some),
_ => Ok(None),
}
}
@@ -562,18 +573,37 @@ mod tests {
/// A cache image block with `entries` of (address, bytes).
fn image(entries: &[(u64, &[u8])]) -> Vec<u8> {
let with_deps: Vec<_> = entries.iter().map(|&(a, b)| (a, b, 0, None)).collect();
image_with_deps(&with_deps)
}
/// A cache image block with `entries` of (address, bytes, flush
/// dependency children, flush dependency parent).
fn image_with_deps(entries: &[(u64, &[u8], u16, Option<u64>)]) -> Vec<u8> {
let mut b = Vec::new();
b.extend_from_slice(MDCI_SIGNATURE);
b.push(0);
b.push(0);
b.extend_from_slice(&0u64.to_le_bytes()); // length, patched below
b.extend_from_slice(&(entries.len() as u32).to_le_bytes());
for (addr, bytes) in entries {
b.extend_from_slice(&[5, 0x02, 1, 0]); // type, flags (in LRU), ring, age
b.extend_from_slice(&[0; 6]); // children, dirty children, parents
for &(addr, bytes, children, parent) in entries {
let mut flags = 0x02; // in LRU
if children > 0 {
flags |= MDCI_ENTRY_IS_FD_PARENT;
}
if parent.is_some() {
flags |= MDCI_ENTRY_IS_FD_CHILD;
}
b.extend_from_slice(&[5, flags, 1, 0]); // type, flags, ring, age
b.extend_from_slice(&children.to_le_bytes());
b.extend_from_slice(&0u16.to_le_bytes()); // dirty children
b.extend_from_slice(&u16::from(parent.is_some()).to_le_bytes());
b.extend_from_slice(&0i32.to_le_bytes());
b.extend_from_slice(&addr.to_le_bytes());
b.extend_from_slice(&(bytes.len() as u64).to_le_bytes());
if let Some(p) = parent {
b.extend_from_slice(&p.to_le_bytes());
}
b.extend_from_slice(bytes);
}
b.extend_from_slice(&[0; 4]); // checksum (not verified, as in libhdf5)
@@ -592,7 +622,7 @@ mod tests {
address: at,
length: img.len() as u64,
};
let out = apply_cache_image(&f, loc, 8, 8).unwrap();
let out = apply_cache_image(&f, loc, &sb_v2(u64::MAX)).unwrap();
assert_eq!(out.len(), f.len());
assert_eq!(&out[16..22], b"HEADER");
assert_eq!(&out[40..44], b"NODE");
@@ -605,7 +635,7 @@ mod tests {
address: 64,
length: img.len() as u64,
};
apply_cache_image(&f, loc, 8, 8).unwrap_err()
apply_cache_image(&f, loc, &sb_v2(u64::MAX)).unwrap_err()
};
let mut sig = image(&[(16, b"x")]);
sig[0] = b'X';
@@ -627,4 +657,43 @@ mod tests {
cut[6..14].copy_from_slice(&n.to_le_bytes());
assert!(matches!(bad(cut), FormatError::InvalidCacheImage(_)));
}
/// libhdf5 resolves an entry's flush-dependency parents as it inserts
/// the entry (`H5C__reconstruct_cache_contents`): a parent must be an
/// earlier entry, or the superblock or its extension's object header,
/// which are cached before the image loads. A parent listed after its
/// child fails ("fd parent not in cache?!?").
#[test]
fn flush_dependency_parents_must_already_be_cached() {
let load = |img: Vec<u8>| {
let mut f = vec![0u8; 64];
f.extend_from_slice(&img);
let loc = CacheImageLocation {
address: 64,
length: img.len() as u64,
};
apply_cache_image(&f, loc, &sb_v2(48))
};
// Parent first, as libhdf5 writes images.
assert!(
load(image_with_deps(&[
(16, b"P", 1, None),
(40, b"C", 0, Some(16))
]))
.is_ok()
);
// Child first: libhdf5 does not find the parent.
assert_eq!(
load(image_with_deps(&[
(40, b"C", 0, Some(16)),
(16, b"P", 1, None)
]))
.unwrap_err(),
FormatError::InvalidCacheImage("fd parent not in cache")
);
// The superblock extension's header (at 48 here) is in the cache.
assert!(load(image_with_deps(&[(40, b"C", 0, Some(48))])).is_ok());
// An entry cannot be its own parent.
assert!(load(image_with_deps(&[(40, b"C", 1, Some(40))])).is_err());
}
}