From e7b83631dbc38d2e5b003d05318b564ad389062e Mon Sep 17 00:00:00 2001 From: stringhandler Date: Thu, 26 Feb 2026 15:38:59 +0200 Subject: [PATCH 1/2] add replicating tests --- src/value.rs | 111 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) diff --git a/src/value.rs b/src/value.rs index 4b9bf285..86de4eed 100644 --- a/src/value.rs +++ b/src/value.rs @@ -1262,6 +1262,117 @@ mod tests { let new_v = Value::from_padded_bits(&mut iter, &v.ty).unwrap(); assert_eq!(v, new_v); } + + // --- Bug regression tests for right_shift_1 when new_bit=false --- + // + // right_shift_1(inner, bit_offset > 0, new_bit=false) is supposed to insert + // a 0 (Left tag) at position bit_offset-1 by decrementing bit_offset. The bug + // is that it clones the Arc without clearing that position, so any pre-existing + // 1-bit at bit_offset-1 corrupts the Left tag, causing the value to read as + // Right instead of Left. + // + // The dirty bit originates from product sub-value extraction: as_product() + // returns ValueRef children that share the parent buffer. The right child has + // bit_offset = parent.bit_offset + left_bit_width, so every 1-bit encoded by + // the left component sits "before" the right child's logical start. When that + // right child is later wrapped in Left, right_shift_1 may see a stale 1-bit. + + /// Simplest direct construction: extract the right sub-value of a product + /// whose left component has 1-bits, then wrap the sub-value in Left. + /// + /// `product(Right(u1(1)), Right(u1(0)))` encodes as [0xE0] at bit_offset=0: + /// bits 0..=1 = Right(u1(1)) = 1, 1 ← left component + /// bits 2..=3 = Right(u1(0)) = 1, 0 ← right component (bit_offset=2) + /// + /// Wrapping the right sub-value (bit_offset=2 in [0xE0]) in Left calls + /// right_shift_1(false) which should clear bit 1. The bug leaves bit 1 = 1, + /// so the result reads as Right instead of Left. + #[test] + fn left_tag_not_corrupted_by_dirty_buffer() { + let product_val = Value::product( + Value::right(Final::u1(), Value::u1(1)), // encodes bits 0,1 as 1,1 + Value::right(Final::u1(), Value::u1(0)), // starts at bit_offset=2 + ); + + let (_, right_sub) = product_val.as_product().unwrap(); + // right_sub shares [0xE0]; bit at position 1 (before its start) is 1. + let right_sub_owned = right_sub.to_value(); + + let left_val = Value::left(right_sub_owned, Final::unit()); + + assert!( + left_val.as_left().is_some(), + "BUG: Left tag corrupted — right_shift_1(false) did not clear dirty bit; \ + value reads as Right instead of Left" + ); + assert!( + left_val.as_right().is_none(), + "BUG: Left tag corrupted — as_right() should return None for a Left value" + ); + } + + /// Verify the dirty-buffer bug at the bit level: the first padded bit of a + /// Left value must always be 0 (the Left discriminant). + #[test] + fn left_tag_first_padded_bit_is_zero() { + let product_val = Value::product( + Value::right(Final::u1(), Value::u1(1)), + Value::right(Final::u1(), Value::u1(0)), + ); + + let (_, right_sub) = product_val.as_product().unwrap(); + let left_val = Value::left(right_sub.to_value(), Final::unit()); + + let first_bit = left_val.iter_padded().next(); + assert_eq!( + first_bit, + Some(false), + "BUG: first padded bit of Left value is {:?}, expected Some(false)", + first_bit + ); + } + + /// End-to-end via prune: pruning can extract a sub-value via to_value() that + /// shares the parent buffer and has 1-bits before its logical start. When + /// prune then calls Value::left() on it (Task::MakeLeft), the bug corrupts + /// the Left tag. + /// + /// Original: Left(product(Right(u1(1)), Right(u1(0)))) + /// type: ((u1+u1) * (u1+u1)) + unit + /// + /// Pruned to: (unit * (u1+u1)) + unit + /// + /// The right component of the inner product keeps its type (u1+u1) unchanged, + /// so prune returns it via to_value() — sharing the same dirty buffer. + /// MakeLeft then wraps it, triggering the bug. + #[test] + fn prune_does_not_corrupt_left_tag_via_shared_buffer() { + let original = Value::left( + Value::product( + Value::right(Final::u1(), Value::u1(1)), + Value::right(Final::u1(), Value::u1(0)), + ), + Final::unit(), + ); + + // Shrink the left component: (u1+u1)*(u1+u1) → unit*(u1+u1). + // The right sub-value of the product keeps type u1+u1 (matches exactly), + // so prune returns it via to_value() with the dirty shared buffer intact. + let pruned_ty = Final::sum( + Final::product(Final::unit(), Final::sum(Final::u1(), Final::u1())), + Final::unit(), + ); + + let pruned = original + .prune(&pruned_ty) + .expect("prune returned None; the target type is compatible"); + + assert!( + pruned.as_left().is_some(), + "BUG: prune corrupted the Left tag — right_shift_1(false) did not clear \ + a dirty bit from the shared product buffer; value reads as Right" + ); + } } #[cfg(bench)] From c70e3ed7337a2fb50ea719049f8002bcea324eee Mon Sep 17 00:00:00 2001 From: stringhandler Date: Thu, 26 Feb 2026 16:40:54 +0200 Subject: [PATCH 2/2] fix value bit corruption New benchmarks test node::redeem::benches::decode_fixed_program ... bench: 47,575.83 ns/iter (+/- 1,888.92) test value::benches::bench_value_create_512_256 ... bench: 62,040.00 ns/iter (+/- 684.18) test value::benches::bench_value_create_512_256_compact ... bench: 3,091.83 ns/iter (+/- 19.26) test value::benches::bench_value_create_512_256_padded ... bench: 3,034.38 ns/iter (+/- 17.38) test value::benches::bench_value_create_64k ... bench: 671,748.75 ns/iter (+/- 4,494.75) test value::benches::bench_value_create_64k_compact ... bench: 32,111.70 ns/iter (+/- 5,517.22) test value::benches::bench_value_create_64k_padded ... bench: 32,177.62 ns/iter (+/- 7,199.71) test value::benches::bench_value_create_deep_some ... bench: 401,901.88 ns/iter (+/- 4,945.75) test value::benches::bench_value_create_deep_some_compact ... bench: 145,748.00 ns/iter (+/- 833.20) test value::benches::bench_value_create_deep_some_padded ... bench: 1,205.44 ns/iter (+/- 5.89) test value::benches::bench_value_create_u2048 ... bench: 482,537.50 ns/iter (+/- 8,726.38) test value::benches::bench_value_create_u2048_compact ... bench: 2,169.61 ns/iter (+/- 7.86) test value::benches::bench_value_create_u2048_padded ... bench: 2,075.53 ns/iter (+/- 16.34) test value::benches::bench_value_create_u64 ... bench: 32.03 ns/iter (+/- 0.91) test value::benches::bench_value_create_u64_compact ... bench: 162.57 ns/iter (+/- 1.75) test value::benches::bench_value_create_u64_padded ... bench: 113.74 ns/iter (+/- 0.67) test value::benches::bench_value_display_512_256 ... bench: 74,847.14 ns/iter (+/- 1,099.57) test value::benches::bench_value_display_64k ... bench: 709,160.00 ns/iter (+/- 12,038.00) test value::benches::bench_value_display_deep_some ... bench: 43,401.18 ns/iter (+/- 559.59) test value::benches::bench_value_display_u2024 ... bench: 44,913.00 ns/iter (+/- 477.15) test value::benches::bench_value_display_u64 ... bench: 357.24 ns/iter (+/- 7.69) --- src/value.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/value.rs b/src/value.rs index 86de4eed..d378f216 100644 --- a/src/value.rs +++ b/src/value.rs @@ -276,14 +276,19 @@ fn right_shift_1(inner: &Arc<[u8]>, bit_offset: usize, new_bit: bool) -> (Arc<[u // If the current bit offset is nonzero this is super easy: we just // lower the bit offset and call that a fix. if bit_offset > 0 { - if new_bit { - let new_bit_offset = bit_offset - 1; + let new_bit_offset = bit_offset - 1; + let mask = 1u8 << (7 - new_bit_offset % 8); + let current_bit = inner[new_bit_offset / 8] & mask != 0; + if current_bit == new_bit { + (Arc::clone(inner), new_bit_offset) + } else if new_bit { let mut bx: Box<[u8]> = inner.as_ref().into(); - bx[new_bit_offset / 8] |= 1 << (7 - new_bit_offset % 8); + bx[new_bit_offset / 8] |= mask; (bx.into(), new_bit_offset) } else { - // ...and if we are inserting a 0 we don't even need to allocate a new [u8] - (Arc::clone(inner), bit_offset - 1) + let mut bx: Box<[u8]> = inner.as_ref().into(); + bx[new_bit_offset / 8] &= !mask; + (bx.into(), new_bit_offset) } } else { // If the current bit offset is 0, we just shift everything right by 8