diff --git a/conformance/probe/src/main.rs b/conformance/probe/src/main.rs index 9fa9929..fe619a0 100644 --- a/conformance/probe/src/main.rs +++ b/conformance/probe/src/main.rs @@ -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), diff --git a/crates/clawhdf5-format/src/superblock_ext.rs b/crates/clawhdf5-format/src/superblock_ext.rs index d7ec3cc..2db3d8b 100644 --- a/crates/clawhdf5-format/src/superblock_ext.rs +++ b/crates/clawhdf5-format/src/superblock_ext.rs @@ -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, 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>, 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 { + 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)]) -> Vec { 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| { + 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()); + } }