From 2683d7c0de090e8546a1097521bee0519697f386 Mon Sep 17 00:00:00 2001 From: MesTTo Date: Sun, 21 Jun 2026 17:20:31 +1000 Subject: [PATCH 1/5] Fix composite meet/join operand selection in DenseByteNode combine_algebraic_results copied the left operand unconditionally in the two mixed value-only / rec-only branches; node_get_payloads treated an exact value-only request as exhaustive even when the CoFree held an unrequested onward link. Both could assemble a result from the wrong child or let a left-only chain survive an intersection. Select the operand by the SELF_IDENT mask, and mark the value-only enumeration non-exhaustive when an onward link is present. A 512-seed dual-distributivity differential vs a BTreeSet oracle (+ the seed-44 meet-associativity regression) failed 83 seeds before, 0 after. MORK's atom_intersection and DNF meet hit this on prefix-nested unit-keyed data. (cherry picked from commit 2619d9fba5b51e77c5a37d715f3b9faedac834ba) --- src/dense_byte_node.rs | 21 ++- tests/pathmap_algebra_differential.rs | 180 ++++++++++++++++++++++++++ 2 files changed, 199 insertions(+), 2 deletions(-) create mode 100644 tests/pathmap_algebra_differential.rs diff --git a/src/dense_byte_node.rs b/src/dense_byte_node.rs index ca253c3..6b97690 100644 --- a/src/dense_byte_node.rs +++ b/src/dense_byte_node.rs @@ -698,6 +698,13 @@ impl> TrieNode requested_mask.clear_bit(byte); match self.get(byte) { Some(cf) => { + // An exact value-only request does not enumerate an onward link stored in the + // same CoFree. The preceding stashed-value fast path means this branch is + // reached only when that link was not requested separately. + if key.len() == 1 && *expect_val && cf.has_rec() { + unrequested_cofree_half = true; + } + //A key longer than 1 byte or an explicit request for a rec link can be answered with a Child if key.len() > 1 || !*expect_val { match cf.rec() { @@ -1914,7 +1921,12 @@ impl, OtherCf: CoFree if new_mask > 0 { AlgebraicResult::Identity(new_mask) } else { - AlgebraicResult::Element(Self::new(None, self.val().cloned())) + let val = if val_mask & SELF_IDENT > 0 { + self.val().cloned() + } else { + other.val().cloned() + }; + AlgebraicResult::Element(Self::new(None, val)) } }, (AlgebraicResult::Identity(rec_mask), AlgebraicResult::None) => { @@ -1928,7 +1940,12 @@ impl, OtherCf: CoFree if new_mask > 0 { AlgebraicResult::Identity(new_mask) } else { - AlgebraicResult::Element(Self::new(self.rec().cloned(), None)) + let rec = if rec_mask & SELF_IDENT > 0 { + self.rec().cloned() + } else { + other.rec().cloned() + }; + AlgebraicResult::Element(Self::new(rec, None)) } }, (rec_el, val_el) => { diff --git a/tests/pathmap_algebra_differential.rs b/tests/pathmap_algebra_differential.rs new file mode 100644 index 0000000..d42066b --- /dev/null +++ b/tests/pathmap_algebra_differential.rs @@ -0,0 +1,180 @@ +use std::collections::BTreeSet; + +use pathmap::PathMap; + +type KeySet = BTreeSet>; + +fn next_u64(state: &mut u64) -> u64 { + *state = state + .wrapping_mul(6_364_136_223_846_793_005) + .wrapping_add(1_442_695_040_888_963_407); + *state +} + +fn fixed_width_set(seed: u64, salt: u64) -> KeySet { + let mut state = seed ^ salt; + let mut keys = KeySet::new(); + for ordinal in 0..72_u64 { + let mut key = vec![0_u8; 8]; + for byte in &mut key { + *byte = (next_u64(&mut state) >> 32) as u8; + } + key[0] ^= ordinal as u8; + keys.insert(key); + } + keys +} + +fn prefix_heavy_set(seed: u64, salt: u64) -> KeySet { + let mut state = seed ^ salt; + let mut keys = KeySet::new(); + if next_u64(&mut state) & 7 == 0 { + keys.insert(Vec::new()); + } + for index in 0..48_u8 { + let length = (next_u64(&mut state) % 9) as usize; + let mut key = Vec::with_capacity(length); + for position in 0..length { + let selector = next_u64(&mut state); + key.push(match selector % 5 { + 0 => index, + 1 => position as u8, + 2 => (selector >> 32) as u8, + 3 => b'a' + (selector % 7) as u8, + _ => 0xff_u8.wrapping_sub(index), + }); + } + keys.insert(key.clone()); + if key.len() > 1 && index % 3 == 0 { + keys.insert(key[..key.len() - 1].to_vec()); + } + if index % 7 == 0 { + key.extend_from_slice(&[0, index]); + keys.insert(key); + } + } + keys +} + +fn map_from_set(keys: &KeySet) -> PathMap<()> { + let mut map = PathMap::new(); + for key in keys { + map.insert(key, ()); + } + map +} + +fn set_from_map(map: &PathMap<()>) -> KeySet { + map.iter().map(|(key, ())| key).collect() +} + +#[test] +fn seeded_prefix_free_algebra_matches_btreeset_oracle() { + for seed in 0_u64..256 { + let a = fixed_width_set(seed, 0x243f_6a88_85a3_08d3); + let b = fixed_width_set(seed, 0x1319_8a2e_0370_7344); + let c = fixed_width_set(seed, 0xa409_3822_299f_31d0); + let ma = map_from_set(&a); + let mb = map_from_set(&b); + let mc = map_from_set(&c); + + let union = a.union(&b).cloned().collect::(); + let intersection = a.intersection(&b).cloned().collect::(); + let difference = a.difference(&b).cloned().collect::(); + assert_eq!(set_from_map(&ma.join(&mb)), union, "join seed {seed}"); + assert_eq!( + set_from_map(&ma.meet(&mb)), + intersection, + "meet seed {seed}" + ); + assert_eq!( + set_from_map(&ma.subtract(&mb)), + difference, + "subtract seed {seed}" + ); + + assert_eq!(set_from_map(&ma.join(&mb)), set_from_map(&mb.join(&ma))); + assert_eq!(set_from_map(&ma.meet(&mb)), set_from_map(&mb.meet(&ma))); + assert_eq!(set_from_map(&ma.join(&ma)), a); + assert_eq!(set_from_map(&ma.meet(&ma)), a); + assert!(set_from_map(&ma.subtract(&ma)).is_empty()); + assert_eq!( + set_from_map(&ma.join(&mb).join(&mc)), + set_from_map(&ma.join(&mb.join(&mc))) + ); + assert_eq!( + set_from_map(&ma.meet(&mb).meet(&mc)), + set_from_map(&ma.meet(&mb.meet(&mc))) + ); + assert_eq!( + set_from_map(&ma.meet(&mb.join(&mc))), + set_from_map(&ma.meet(&mb).join(&ma.meet(&mc))) + ); + } +} + +#[test] +fn cloned_prefix_heavy_maps_are_logically_isolated_under_mutation() { + for seed in 0_u64..128 { + let original_set = prefix_heavy_set(seed, 0x082e_fa98_ec4e_6c89); + let original = map_from_set(&original_set); + let mut changed = original.clone(); + let removed = original_set.iter().next().cloned(); + if let Some(key) = &removed { + assert!(changed.remove(key).is_some()); + } + let inserted = vec![0xfe, (seed >> 8) as u8, seed as u8, 0x01]; + changed.insert(&inserted, ()); + assert_eq!( + set_from_map(&original), + original_set, + "original seed {seed}" + ); + let mut expected = original_set; + if let Some(key) = removed { + expected.remove(&key); + } + expected.insert(inserted); + assert_eq!(set_from_map(&changed), expected, "clone seed {seed}"); + } +} + +#[test] +fn prefix_valued_meet_is_associative_seed_44() { + let seed = 44; + let a = map_from_set(&prefix_heavy_set(seed, 0x243f_6a88_85a3_08d3)); + let b = map_from_set(&prefix_heavy_set(seed, 0x1319_8a2e_0370_7344)); + let c = map_from_set(&prefix_heavy_set(seed, 0xa409_3822_299f_31d0)); + assert_eq!( + set_from_map(&a.meet(&b).meet(&c)), + set_from_map(&a.meet(&b.meet(&c))) + ); +} + +#[test] +fn seeded_prefix_heavy_dual_distributivity_matches_btreeset_oracle() { + // Seeds 10, 77, and 287 are focused regressions for CoFree identity + // operand selection and mixed value/onward-link exhaustiveness. + for seed in 0_u64..512 { + let a = prefix_heavy_set(seed, 0x243f_6a88_85a3_08d3); + let b = prefix_heavy_set(seed, 0x1319_8a2e_0370_7344); + let c = prefix_heavy_set(seed, 0xa409_3822_299f_31d0); + let ma = map_from_set(&a); + let mb = map_from_set(&b); + let mc = map_from_set(&c); + + let b_meet_c = b.intersection(&c).cloned().collect::(); + let expected = a.union(&b_meet_c).cloned().collect::(); + + assert_eq!( + set_from_map(&ma.join(&mb.meet(&mc))), + expected, + "left dual-distributive form seed {seed}" + ); + assert_eq!( + set_from_map(&ma.join(&mb).meet(&ma.join(&mc))), + expected, + "right dual-distributive form seed {seed}" + ); + } +} From 15e219848f2549c1d49feee5b29658a984b80018 Mon Sep 17 00:00:00 2001 From: MesTTo Date: Sun, 5 Jul 2026 04:03:31 +1000 Subject: [PATCH 2/5] Fix stacked-borrows UB in zipper_merge_n_mono via safe get_disjoint_mut with_k built K raw pointers with xs.as_mut_ptr().add(idx) inside a loop; each as_mut_ptr() re-borrowed the slice and invalidated the pointers from earlier iterations, so the later &mut *p retag hit a tag no longer in the borrow stack (Miri UB, test_almost_identical_paths_n). Replace with slice::get_disjoint_mut over the distinct bitmask indices: safe, no unsafe, same result, negligible cost for small K. Removes an unsafe block; Miri-clean. Verified: cargo +nightly miri test on this exact tree aborts at the claimed line (with_k's ptrs.map(|p| &mut *p)) before this fix, and is clean (120/120 zipper_algebra tests) after it. --- src/experimental/zipper_algebra.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/experimental/zipper_algebra.rs b/src/experimental/zipper_algebra.rs index 51f2bf7..e7b8378 100644 --- a/src/experimental/zipper_algebra.rs +++ b/src/experimental/zipper_algebra.rs @@ -1423,23 +1423,23 @@ fn with_k( ) -> R { debug_assert!(bits.count_ones() as usize >= K); - // collect raw pointers first (safe) - let mut ptrs: [*mut T; K] = [std::ptr::null_mut(); K]; - + // Extract the K distinct active indices from the bitmask, then take K + // disjoint &mut into the slice with the safe checked API. The previous + // version built raw pointers via `xs.as_mut_ptr().add(idx)` inside the + // loop, but each `as_mut_ptr()` re-borrowed `xs` and invalidated the + // pointers from earlier iterations under Stacked Borrows (a real UB Miri + // flags). `get_disjoint_mut` proves distinctness + in-bounds and hands + // back the disjoint references; for small K its check is negligible. + let mut indices = [0usize; K]; let mut i = 0; while i < K { - let idx = bits.trailing_zeros() as usize; + indices[i] = bits.trailing_zeros() as usize; bits &= bits - 1; - ptrs[i] = unsafe { xs.as_mut_ptr().add(idx) }; i += 1; } - - // SAFETY: - // - indices are distinct (bitmask) - // - derived from same slice - - // should be zero-cost after inlining - let refs = unsafe { ptrs.map(|p| &mut *p) }; + let refs = xs + .get_disjoint_mut(indices) + .expect("active bitmask indices are distinct and in bounds"); f(refs) } From a8dce58fd61ecc1483e05dfde53a725e7ad57333 Mon Sep 17 00:00:00 2001 From: MesTTo Date: Tue, 23 Jun 2026 16:06:52 +1000 Subject: [PATCH 3/5] Reject malformed serialized offsets deserialize_file indexed into the deserialized-records vec by offsets read straight from the serialized file, so a malformed forward or out-of-range offset panicked (index out of bounds) instead of returning an error. Bound every offset lookup with .get().ok_or_else(...) so malformed input is rejected, not a panic. Verified: on this exact tree, a crafted forward-offset fixture ("P x...1x...2" with an empty records vec) panics at serialization.rs:820 without this fix; with it, deserialize_file returns Err("...offset out of bounds") and both new regression tests pass. --- src/experimental/serialization.rs | 48 +++++++++++++++++++++++++------ 1 file changed, 40 insertions(+), 8 deletions(-) diff --git a/src/experimental/serialization.rs b/src/experimental/serialization.rs index d900625..0cabd82 100644 --- a/src/experimental/serialization.rs +++ b/src/experimental/serialization.rs @@ -817,8 +817,8 @@ pub fn deserialize_file(file_path : impl AsRef, d let [path_idx, node_idx] = node_buf.map(|x| x as usize); - let Deserialized::Path(path) = &deserialized[path_idx] else { return Err(std::io::Error::other("Malformed serialized ByteTrie, expected path")); }; - let Deserialized::Node(node) = &deserialized[node_idx] else { return Err(std::io::Error::other("Malformed serialized ByteTrie, expected node")); }; + let Deserialized::Path(path) = deserialized.get(path_idx).ok_or_else(|| std::io::Error::other("Malformed serialized ByteTrie, path offset out of bounds"))? else { return Err(std::io::Error::other("Malformed serialized ByteTrie, expected path")); }; + let Deserialized::Node(node) = deserialized.get(node_idx).ok_or_else(|| std::io::Error::other("Malformed serialized ByteTrie, node offset out of bounds"))? else { return Err(std::io::Error::other("Malformed serialized ByteTrie, expected node")); }; let mut path_node = PathMap::new(); @@ -838,8 +838,8 @@ pub fn deserialize_file(file_path : impl AsRef, d let [val_idx, node_idx] = node_buf.map(|x| x as usize); - let Deserialized::Value(value) = &deserialized[val_idx] else { return Err(std::io::Error::other("Malformed serialized ByteTrie, expected value")); }; - let Deserialized::Node(node) = &deserialized[node_idx] else { return Err(std::io::Error::other("Malformed serialized ByteTrie, expected node")); }; + let Deserialized::Value(value) = deserialized.get(val_idx).ok_or_else(|| std::io::Error::other("Malformed serialized ByteTrie, value offset out of bounds"))? else { return Err(std::io::Error::other("Malformed serialized ByteTrie, expected value")); }; + let Deserialized::Node(node) = deserialized.get(node_idx).ok_or_else(|| std::io::Error::other("Malformed serialized ByteTrie, node offset out of bounds"))? else { return Err(std::io::Error::other("Malformed serialized ByteTrie, expected node")); }; let mut value_node = node.clone(); value_node.set_val_at(&[], value.clone()); @@ -852,10 +852,10 @@ pub fn deserialize_file(file_path : impl AsRef, d let [mask_idx, branches_idx] = node_buf.map(|x| x as usize); - let Deserialized::ChildMask(mask) = &deserialized[mask_idx] else { return Err(std::io::Error::other("Malformed serialized ByteTrie, expected childmask as `(/?)*`")); }; + let Deserialized::ChildMask(mask) = deserialized.get(mask_idx).ok_or_else(|| std::io::Error::other("Malformed serialized ByteTrie, childmask offset out of bounds"))? else { return Err(std::io::Error::other("Malformed serialized ByteTrie, expected childmask as `(/?)*`")); }; let iter: crate::utils::ByteMaskIter = (*mask).into(); - let Deserialized::Branches(r) = &deserialized[branches_idx] else { return Err(std::io::Error::other("Malformed serialized ByteTrie, expected branches")); }; + let Deserialized::Branches(r) = deserialized.get(branches_idx).ok_or_else(|| std::io::Error::other("Malformed serialized ByteTrie, branches offset out of bounds"))? else { return Err(std::io::Error::other("Malformed serialized ByteTrie, expected branches")); }; let branches = &branches_buffer[r.start..r.end]; core::debug_assert_eq!(mask.into_iter().copied().map(u64::count_ones).sum::() as usize, branches.len()); @@ -864,7 +864,7 @@ pub fn deserialize_file(file_path : impl AsRef, d let mut wz = branch_node.write_zipper(); for (byte, &idx) in iter.into_iter().zip(branches) { - let Deserialized::Node(node) = &deserialized[idx as usize] else { return Err(std::io::Error::other("Malformed serialized ByteTrie, expected node")); }; + let Deserialized::Node(node) = deserialized.get(idx as usize).ok_or_else(|| std::io::Error::other("Malformed serialized ByteTrie, child node offset out of bounds"))? else { return Err(std::io::Error::other("Malformed serialized ByteTrie, expected node")); }; core::debug_assert!(!node.is_empty()); @@ -986,6 +986,38 @@ mod test { use super::*; use std::sync::Arc; + fn write_serialized_fixture(dir : &tempfile::TempDir, name : &str, data : &[u8])->PathBuf { + let path = dir.path().join(name); + std::fs::write(&path, data).unwrap(); + path + } + + #[test] + fn deserialize_rejects_malformed_records() { + let temp_dir = tempfile::tempdir().unwrap(); + + let bad_tag = write_serialized_fixture(&temp_dir, "bad_tag.data", b"header\n? bad\n"); + let err = deserialize_file::>(&bad_tag, |b| Arc::<[u8]>::from(b)).unwrap_err(); + assert!(err.to_string().contains("expected ``")); + + let odd_path_hex = write_serialized_fixture(&temp_dir, "odd_path_hex.data", b"header\np A\n"); + let err = deserialize_file::>(&odd_path_hex, |b| Arc::<[u8]>::from(b)).unwrap_err(); + assert!(err.to_string().contains("expected path")); + } + + #[test] + fn deserialize_rejects_forward_offsets_without_panic() { + let temp_dir = tempfile::tempdir().unwrap(); + let path = write_serialized_fixture( + &temp_dir, + "forward_offset.data", + b"header\nP x0000000000000001x0000000000000002\n" + ); + + let err = deserialize_file::>(&path, |b| Arc::<[u8]>::from(b)).unwrap_err(); + assert!(err.to_string().contains("offset out of bounds")); + } + #[test] fn serialization_trivial_test() { const LEN : usize = 0x_80; @@ -1598,4 +1630,4 @@ pub fn dbg_hex_line_numbers(f : &std::fs::File, path : impl AsRef Date: Tue, 23 Jun 2026 16:07:21 +1000 Subject: [PATCH 4/5] Guard line-list node layout Pins LineListNode<[u8;1024]> to 64 bytes under slim_ptrs on x86_64 (excluding miri, which pads for its own bookkeeping) with a compile-time size assert, so a layout change that grows the node is caught at build time instead of surfacing as a memory regression later. Only the slim_ptrs arm is asserted, for a concrete reason: the not(slim_ptrs) TrieNodeODRc has no empty-sentinel representation, so the shared call sites' new_empty/is_empty/make_unique/== do not exist there and that configuration does not compile (26 errors, all one root cause). Giving the Arc-backed variant a sentinel is the open design flagged in the TaggedNodeRefMut EmptyNode note (the allocator prevents a free empty sentinel); once that lands, the second size assert follows. --- src/line_list_node.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/line_list_node.rs b/src/line_list_node.rs index 92c3133..55787bc 100644 --- a/src/line_list_node.rs +++ b/src/line_list_node.rs @@ -39,6 +39,16 @@ pub(crate) const KEY_BYTES_CNT: usize = 42; #[cfg(not(feature = "slim_ptrs"))] pub(crate) const KEY_BYTES_CNT: usize = 14; +// Only the slim_ptrs layout is asserted. The not(slim_ptrs) TrieNodeODRc has no +// empty-sentinel representation yet (`new_empty`/`is_empty`/`make_unique`/`==` +// exist only on the slim variant; the allocator prevents a free sentinel for the +// Arc form, the open design in the note at trie_node.rs on TaggedNodeRefMut's +// EmptyNode arm), so that configuration does not compile and its size cannot be +// asserted until the sentinel design lands. +#[cfg(all(feature = "slim_ptrs", target_arch = "x86_64", not(miri)))] +const _: [(); core::mem::size_of::>()] = + [(); 64]; + const SLOT_0_USED_MASK: u16 = 1 << 15; const SLOT_1_USED_MASK: u16 = 1 << 14; const BOTH_SLOTS_USED_MASK: u16 = SLOT_0_USED_MASK | SLOT_1_USED_MASK; From 064b3207ce063b4e0afc6c0ad2659963367a185e Mon Sep 17 00:00:00 2001 From: MesTTo Date: Sun, 5 Jul 2026 04:12:31 +1000 Subject: [PATCH 5/5] Re-enable the tree_serde_2 test The test passes on this tree (verified via -- --ignored before removing the attribute), so the ignore is stale: the round-trip it covers works, and keeping it ignored just forfeits the regression coverage. If tree serialization still has known gaps elsewhere, this pins the part that does work. --- src/experimental/tree_serialization.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/experimental/tree_serialization.rs b/src/experimental/tree_serialization.rs index ed406e3..5386f95 100644 --- a/src/experimental/tree_serialization.rs +++ b/src/experimental/tree_serialization.rs @@ -60,7 +60,6 @@ mod tests { use crate::morphisms::Catamorphism; use crate::PathMap; - #[ignore] //GOAT, re-enable if/when this code is ready. #[test] fn tree_serde_2() { let keys = [vec![12, 13, 14], vec![12, 13, 14, 100, 101]];