From 7375a64999d8845c2cf0c2e48b343a4f1ff0e64b Mon Sep 17 00:00:00 2001 From: Jonathan Paprocki Date: Tue, 12 Nov 2024 01:19:15 -0500 Subject: [PATCH 01/15] add slab copying test --- crown/src/noun/slab.rs | 87 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/crown/src/noun/slab.rs b/crown/src/noun/slab.rs index 868e82f..2e149b8 100644 --- a/crown/src/noun/slab.rs +++ b/crown/src/noun/slab.rs @@ -10,9 +10,15 @@ use std::ptr::copy_nonoverlapping; use sword::mem::NockStack; use sword::mug::{calc_atom_mug_u32, calc_cell_mug_u32, get_mug, set_mug}; use sword::noun::{Atom, Cell, CellMemory, DirectAtom, IndirectAtom, Noun, NounAllocator, D}; +use sword::jets::hot::URBIT_HOT_STATE; use sword::persist::pma_contains; use sword::serialization::{met0_u64_to_usize, met0_usize}; use thiserror::Error; +use sword::interpreter::{Context, interpret}; +use sword::jets::{cold::Cold, warm::Warm, hot::Hot}; +use sword::hamt::Hamt; +use std::pin::Pin; +use crate::utils::slogger::CrownSlogger; const CELL_MEM_WORD_SIZE: usize = (size_of::() + 7) >> 3; @@ -857,4 +863,85 @@ mod tests { ); } } + + #[test] + fn test_nockstack_slab_equality() { + // Create initial NockStack with the test noun + let mut stack = NockStack::new(1000, 0); + let a_values = [ + 0xea31bc2f1dcd1ff1u64, + 0x0dd3c7e3b75f3abbu64, + 0x8f9ff1e4b2ca417fu64, + 0x3bc6aedea88fed36u64, + 0x9d68355b4a0a18d0u64, + ]; + // DIRECT_MAX is u64::MAX >> 1 = 0x7FFF_FFFF_FFFF_FFFF + // Compare each value against DIRECT_MAX to determine if it needs to be indirect + println!("DIRECT_MAX = 0x7FFF_FFFF_FFFF_FFFF"); + for (i, &val) in a_values.iter().enumerate() { + println!("a_values[{}] = 0x{:016x} {}", i, val, + if val <= 0x7FFF_FFFF_FFFF_FFFF { + "(direct)" + } else { + "(indirect)" + } + ); + } + // Convert each value to an Atom using from_bytes + let atoms: Vec<_> = a_values.iter() + .map(|&x| { + let bytes = Bytes::copy_from_slice(&x.to_le_bytes()); + Atom::from_bytes(&mut stack, &bytes).as_noun() + }) + .collect(); + + // Create 'a' as a cell containing these atoms + let a = T(&mut stack, &atoms); + + // Create cell X = [a a] + let x = T(&mut stack, &[a, a]); + + println!("Original X in NockStack:"); + println!("{:?}", x); + + // Copy to NounSlab + let mut slab = NounSlab::new(); + slab.copy_into(x); + let slab_x = unsafe { slab.root() }; + + println!("X in NounSlab:"); + println!("{:?}", slab_x); + + // Copy back to new NockStack + let mut new_stack = NockStack::new(1000, 0); + let copied_x = slab.copy_to_stack(&mut new_stack); + + println!("Copied X back to NockStack:"); + println!("{:?}", copied_x); + + // Create interpreter context + let mut cold = Cold::new(&mut new_stack); + let hot = Hot::init(&mut new_stack, &URBIT_HOT_STATE); + let warm = Warm::init(&mut new_stack, &mut cold, &hot); + let cache = Hamt::new(&mut new_stack); + let mut context = Context { + stack: new_stack, + slogger: std::boxed::Box::pin(CrownSlogger {}), + cold, + warm, + hot, + cache, + scry_stack: D(0), + trace_info: None, + }; + + // Test equality using interpret + let tail = T(&mut context.stack, &[D(0), D(2)]); + let formula = T(&mut context.stack, &[D(5), tail, D(0), D(3)]); + let result = interpret(&mut context, copied_x, formula); + + println!("Interpret result: {:?}", result); + assert!(unsafe { result.unwrap().raw_equals(D(0)) }, + "Result should be 0 (true) since head and tail are equal"); + } } From bb6139636be5433718eb4c424482af8f48c69890 Mon Sep 17 00:00:00 2001 From: Jonathan Paprocki Date: Tue, 12 Nov 2024 16:32:42 -0500 Subject: [PATCH 02/15] blah --- crown/src/noun/slab.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/crown/src/noun/slab.rs b/crown/src/noun/slab.rs index 2e149b8..6a9a0cc 100644 --- a/crown/src/noun/slab.rs +++ b/crown/src/noun/slab.rs @@ -923,10 +923,11 @@ mod tests { let mut cold = Cold::new(&mut new_stack); let hot = Hot::init(&mut new_stack, &URBIT_HOT_STATE); let warm = Warm::init(&mut new_stack, &mut cold, &hot); - let cache = Hamt::new(&mut new_stack); + let cache = Hamt::::new(&mut new_stack); + let slogger = std::boxed::Box::pin(CrownSlogger {}); let mut context = Context { stack: new_stack, - slogger: std::boxed::Box::pin(CrownSlogger {}), + slogger, cold, warm, hot, @@ -935,9 +936,11 @@ mod tests { trace_info: None, }; - // Test equality using interpret - let tail = T(&mut context.stack, &[D(0), D(2)]); - let formula = T(&mut context.stack, &[D(5), tail, D(0), D(3)]); + // Get the head and tail of X to compare them + let head = T(&mut context.stack, &[D(0), D(2)]); // [0 2] gets head + let tail = T(&mut context.stack, &[D(0), D(3)]); // [0 3] gets tail + let formula = T(&mut context.stack, &[D(5), head, tail]); // [5 head tail] + let result = interpret(&mut context, copied_x, formula); println!("Interpret result: {:?}", result); From f3f9f831fb2d030cae996cc14a4cc56b892743b8 Mon Sep 17 00:00:00 2001 From: Jonathan Paprocki Date: Wed, 13 Nov 2024 10:06:52 -0500 Subject: [PATCH 03/15] bigger nockstack --- crown/src/noun/slab.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crown/src/noun/slab.rs b/crown/src/noun/slab.rs index 6a9a0cc..bc7f1df 100644 --- a/crown/src/noun/slab.rs +++ b/crown/src/noun/slab.rs @@ -867,7 +867,7 @@ mod tests { #[test] fn test_nockstack_slab_equality() { // Create initial NockStack with the test noun - let mut stack = NockStack::new(1000, 0); + let mut stack = NockStack::new(10000, 0); let a_values = [ 0xea31bc2f1dcd1ff1u64, 0x0dd3c7e3b75f3abbu64, @@ -913,7 +913,7 @@ mod tests { println!("{:?}", slab_x); // Copy back to new NockStack - let mut new_stack = NockStack::new(1000, 0); + let mut new_stack = NockStack::new(10000, 0); let copied_x = slab.copy_to_stack(&mut new_stack); println!("Copied X back to NockStack:"); From 87d70d72721de527abd757ac53b1f09f71976d25 Mon Sep 17 00:00:00 2001 From: Jonathan Paprocki Date: Wed, 13 Nov 2024 11:57:19 -0500 Subject: [PATCH 04/15] validate_root --- crown/src/noun/slab.rs | 85 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/crown/src/noun/slab.rs b/crown/src/noun/slab.rs index bc7f1df..6d09a92 100644 --- a/crown/src/noun/slab.rs +++ b/crown/src/noun/slab.rs @@ -405,6 +405,61 @@ impl NounSlab { pub unsafe fn root(&self) -> Noun { self.root } + + /// Validates that: + /// 1. All allocated nouns in the tree are contained within this slab or the PMA + /// 2. All indirect atoms are normalized (no leading zeros) + /// + /// Returns Ok(()) if valid, or Err with a description of the first validation failure found + pub fn validate_root(&self) -> Result<(), String> { + let mut stack = vec![self.root]; + let mut visited = IntMap::new(); + + while let Some(noun) = stack.pop() { + if let Ok(allocated) = noun.as_allocated() { + let ptr = unsafe { allocated.to_raw_pointer() }; + + // Skip if we've seen this pointer before + if visited.contains_key(ptr as u64) { + continue; + } + visited.insert(ptr as u64, ()); + + // Check if pointer is in slab or PMA + let is_in_slab = !self.allocation_start.is_null() && unsafe { + ptr >= self.allocation_start && ptr < self.allocation_stop + }; + let is_in_pma = unsafe { pma_contains(ptr, 1) }; + + if !is_in_slab && !is_in_pma { + return Err(format!( + "Found noun allocated outside of slab and PMA at {:p}", + ptr + )); + } + + match allocated.as_either() { + Either::Left(indirect) => { + // Check normalization of indirect atoms + let slice = indirect.as_slice(); + if !slice.is_empty() && slice.last().unwrap() == &0 { + return Err(format!( + "Found non-normalized indirect atom at {:p} with value {:?}", + ptr, + slice + )); + } + } + Either::Right(cell) => { + // Add cell's head and tail to stack for processing + stack.push(cell.head()); + stack.push(cell.tail()); + } + } + } + } + Ok(()) + } } impl Drop for NounSlab { @@ -912,6 +967,15 @@ mod tests { println!("X in NounSlab:"); println!("{:?}", slab_x); + // Validate that slab_x is properly allocated and normalized + match slab.validate_root() { + Ok(_) => println!("Slab validation passed"), + Err(e) => { + println!("Slab validation failed: {}", e); + panic!("Validation failed"); + } + } + // Copy back to new NockStack let mut new_stack = NockStack::new(10000, 0); let copied_x = slab.copy_to_stack(&mut new_stack); @@ -947,4 +1011,25 @@ mod tests { assert!(unsafe { result.unwrap().raw_equals(D(0)) }, "Result should be 0 (true) since head and tail are equal"); } + + #[test] + fn test_validate_root() { + let mut slab = NounSlab::new(); + + // Valid case - simple cell + let noun = T(&mut slab, &[D(1), D(2)]); + slab.set_root(noun); + assert!(slab.validate_root().is_ok(), "Valid noun should pass validation"); + + // Valid case - indirect atom + let large_number = u64::MAX as u128 + 1; + let large_number_bytes = Bytes::from(large_number.to_le_bytes().to_vec()); + let indirect_atom = Atom::from_bytes(&mut slab, &large_number_bytes); + slab.set_root(indirect_atom.as_noun()); + assert!(slab.validate_root().is_ok(), "Valid indirect atom should pass validation"); + + // Invalid case - non-normalized indirect atom + // This would require creating a non-normalized atom directly, which isn't + // possible through normal APIs. You might need unsafe code to test this. + } } From 9683f2ea144f1d2c812606b15e40a311974c9591 Mon Sep 17 00:00:00 2001 From: Jonathan Paprocki Date: Wed, 13 Nov 2024 12:07:29 -0500 Subject: [PATCH 05/15] validate_root_all_slabs --- crown/src/noun/slab.rs | 75 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 74 insertions(+), 1 deletion(-) diff --git a/crown/src/noun/slab.rs b/crown/src/noun/slab.rs index 6d09a92..e704a0c 100644 --- a/crown/src/noun/slab.rs +++ b/crown/src/noun/slab.rs @@ -460,6 +460,69 @@ impl NounSlab { } Ok(()) } + + /// Validates that all allocated nouns in the tree are contained within one of this slab's + /// allocated memory regions or the PMA + pub fn validate_root_all_slabs(&self) -> Result<(), String> { + let mut stack = vec![self.root]; + let mut visited = IntMap::new(); + + while let Some(noun) = stack.pop() { + if let Ok(allocated) = noun.as_allocated() { + let ptr = unsafe { allocated.to_raw_pointer() }; + + // Skip if we've seen this pointer before + if visited.contains_key(ptr as u64) { + continue; + } + visited.insert(ptr as u64, ()); + + // Check if pointer is in any of the slabs + let mut found_in_slab = false; + for (slab_ptr, layout) in &self.slabs { + if !slab_ptr.is_null() { + let slab_start = *slab_ptr as *mut u64; + let slab_size = layout.size() / std::mem::size_of::(); + let slab_end = unsafe { slab_start.add(slab_size) }; + + if (ptr as *mut u64) >= slab_start && (ptr as *mut u64) < slab_end { + found_in_slab = true; + break; + } + } + } + + let is_in_pma = unsafe { pma_contains(ptr, 1) }; + + if !found_in_slab && !is_in_pma { + return Err(format!( + "Found noun allocated outside of all slabs and PMA at {:p}", + ptr + )); + } + + match allocated.as_either() { + Either::Left(indirect) => { + // Check normalization of indirect atoms + let slice = indirect.as_slice(); + if !slice.is_empty() && slice.last().unwrap() == &0 { + return Err(format!( + "Found non-normalized indirect atom at {:p} with value {:?}", + ptr, + slice + )); + } + } + Either::Right(cell) => { + // Add cell's head and tail to stack for processing + stack.push(cell.head()); + stack.push(cell.tail()); + } + } + } + } + Ok(()) + } } impl Drop for NounSlab { @@ -967,7 +1030,16 @@ mod tests { println!("X in NounSlab:"); println!("{:?}", slab_x); - // Validate that slab_x is properly allocated and normalized + // Validate across all slabs in the slab + match slab.validate_root_all_slabs() { + Ok(_) => println!("All slabs validation passed"), + Err(e) => { + println!("All slabs validation failed: {}", e); + panic!("All slabs validation failed"); + } + } + + // Validate that nouns are in the allocation range match slab.validate_root() { Ok(_) => println!("Slab validation passed"), Err(e) => { @@ -976,6 +1048,7 @@ mod tests { } } + // Copy back to new NockStack let mut new_stack = NockStack::new(10000, 0); let copied_x = slab.copy_to_stack(&mut new_stack); From 39100a81747abb5a402c3bbb4de42197f7b08443 Mon Sep 17 00:00:00 2001 From: Jonathan Paprocki Date: Wed, 13 Nov 2024 13:03:29 -0500 Subject: [PATCH 06/15] remove incorrect validate_root --- crown/src/noun/slab.rs | 71 ++---------------------------------------- 1 file changed, 3 insertions(+), 68 deletions(-) diff --git a/crown/src/noun/slab.rs b/crown/src/noun/slab.rs index e704a0c..0965548 100644 --- a/crown/src/noun/slab.rs +++ b/crown/src/noun/slab.rs @@ -406,64 +406,9 @@ impl NounSlab { self.root } - /// Validates that: - /// 1. All allocated nouns in the tree are contained within this slab or the PMA - /// 2. All indirect atoms are normalized (no leading zeros) - /// - /// Returns Ok(()) if valid, or Err with a description of the first validation failure found - pub fn validate_root(&self) -> Result<(), String> { - let mut stack = vec![self.root]; - let mut visited = IntMap::new(); - - while let Some(noun) = stack.pop() { - if let Ok(allocated) = noun.as_allocated() { - let ptr = unsafe { allocated.to_raw_pointer() }; - - // Skip if we've seen this pointer before - if visited.contains_key(ptr as u64) { - continue; - } - visited.insert(ptr as u64, ()); - - // Check if pointer is in slab or PMA - let is_in_slab = !self.allocation_start.is_null() && unsafe { - ptr >= self.allocation_start && ptr < self.allocation_stop - }; - let is_in_pma = unsafe { pma_contains(ptr, 1) }; - - if !is_in_slab && !is_in_pma { - return Err(format!( - "Found noun allocated outside of slab and PMA at {:p}", - ptr - )); - } - - match allocated.as_either() { - Either::Left(indirect) => { - // Check normalization of indirect atoms - let slice = indirect.as_slice(); - if !slice.is_empty() && slice.last().unwrap() == &0 { - return Err(format!( - "Found non-normalized indirect atom at {:p} with value {:?}", - ptr, - slice - )); - } - } - Either::Right(cell) => { - // Add cell's head and tail to stack for processing - stack.push(cell.head()); - stack.push(cell.tail()); - } - } - } - } - Ok(()) - } - /// Validates that all allocated nouns in the tree are contained within one of this slab's /// allocated memory regions or the PMA - pub fn validate_root_all_slabs(&self) -> Result<(), String> { + pub fn validate_root(&self) -> Result<(), String> { let mut stack = vec![self.root]; let mut visited = IntMap::new(); @@ -985,7 +930,7 @@ mod tests { #[test] fn test_nockstack_slab_equality() { // Create initial NockStack with the test noun - let mut stack = NockStack::new(10000, 0); + let mut stack = NockStack::new(100000, 0); let a_values = [ 0xea31bc2f1dcd1ff1u64, 0x0dd3c7e3b75f3abbu64, @@ -1030,15 +975,6 @@ mod tests { println!("X in NounSlab:"); println!("{:?}", slab_x); - // Validate across all slabs in the slab - match slab.validate_root_all_slabs() { - Ok(_) => println!("All slabs validation passed"), - Err(e) => { - println!("All slabs validation failed: {}", e); - panic!("All slabs validation failed"); - } - } - // Validate that nouns are in the allocation range match slab.validate_root() { Ok(_) => println!("Slab validation passed"), @@ -1048,9 +984,8 @@ mod tests { } } - // Copy back to new NockStack - let mut new_stack = NockStack::new(10000, 0); + let mut new_stack = NockStack::new(100000, 0); let copied_x = slab.copy_to_stack(&mut new_stack); println!("Copied X back to NockStack:"); From 737698b4c4dc94ed25a53d62fcbdfcd6a98f7890 Mon Sep 17 00:00:00 2001 From: Jonathan Paprocki Date: Wed, 13 Nov 2024 13:29:07 -0500 Subject: [PATCH 07/15] indirect atom size check --- crown/src/noun/slab.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/crown/src/noun/slab.rs b/crown/src/noun/slab.rs index 0965548..5ac209b 100644 --- a/crown/src/noun/slab.rs +++ b/crown/src/noun/slab.rs @@ -11,6 +11,7 @@ use sword::mem::NockStack; use sword::mug::{calc_atom_mug_u32, calc_cell_mug_u32, get_mug, set_mug}; use sword::noun::{Atom, Cell, CellMemory, DirectAtom, IndirectAtom, Noun, NounAllocator, D}; use sword::jets::hot::URBIT_HOT_STATE; +use sword::jets::bits::util::met; use sword::persist::pma_contains; use sword::serialization::{met0_u64_to_usize, met0_usize}; use thiserror::Error; @@ -457,6 +458,29 @@ impl NounSlab { slice )); } + + // Check that indirect atom size is correct + let atom = indirect.as_atom(); + let actual_size = slice.len(); + let expected_size = met(6, atom); + if actual_size != expected_size { + return Err(format!( + "Found indirect atom with incorrect size at {:p} - expected {} words but got {}", + ptr, + expected_size, + actual_size + )); + } + + // Check that it shouldn't be a direct atom + let bit_size = met(0, atom); + if bit_size <= 63 { + return Err(format!( + "Found indirect atom that should be direct at {:p} - only {} bits", + ptr, + bit_size + )); + } } Either::Right(cell) => { // Add cell's head and tail to stack for processing From 858e00ff273e5e9a9891755abe26501c759b35ca Mon Sep 17 00:00:00 2001 From: Jonathan Paprocki Date: Wed, 13 Nov 2024 13:41:21 -0500 Subject: [PATCH 08/15] use HashSet instead of IntMap --- crown/src/noun/slab.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/crown/src/noun/slab.rs b/crown/src/noun/slab.rs index 5ac209b..8dea8fb 100644 --- a/crown/src/noun/slab.rs +++ b/crown/src/noun/slab.rs @@ -411,17 +411,16 @@ impl NounSlab { /// allocated memory regions or the PMA pub fn validate_root(&self) -> Result<(), String> { let mut stack = vec![self.root]; - let mut visited = IntMap::new(); + let mut visited = std::collections::HashSet::new(); while let Some(noun) = stack.pop() { if let Ok(allocated) = noun.as_allocated() { let ptr = unsafe { allocated.to_raw_pointer() }; // Skip if we've seen this pointer before - if visited.contains_key(ptr as u64) { + if !visited.insert(ptr as u64) { continue; } - visited.insert(ptr as u64, ()); // Check if pointer is in any of the slabs let mut found_in_slab = false; From 66914a403e8980b160be2c199a197c411a42aa8d Mon Sep 17 00:00:00 2001 From: Jonathan Paprocki Date: Wed, 13 Nov 2024 15:00:00 -0500 Subject: [PATCH 09/15] check that noun copied into stack is in the stack --- crown/src/noun/slab.rs | 57 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/crown/src/noun/slab.rs b/crown/src/noun/slab.rs index 8dea8fb..95dc24a 100644 --- a/crown/src/noun/slab.rs +++ b/crown/src/noun/slab.rs @@ -262,9 +262,66 @@ impl NounSlab { break; } } + + // Verify that the copied noun is fully in the stack or PMA + self.verify_copied_noun(res).expect("Noun was not fully copied to stack"); + res } + /// Verifies that a noun does not contain any references to memory in this slab + fn verify_copied_noun(&self, noun: Noun) -> Result<(), String> { + let mut stack = vec![noun]; + let mut visited = std::collections::HashSet::new(); + + while let Some(noun) = stack.pop() { + if let Ok(allocated) = noun.as_allocated() { + let ptr = unsafe { allocated.to_raw_pointer() }; + + // Skip if we've seen this pointer before + if !visited.insert(ptr as u64) { + continue; + } + + // Check if pointer is in PMA + if unsafe { pma_contains(ptr, 1) } { + return Err(format!( + "Found noun allocated in PMA at {:p}", + ptr + )); + } + + // Check if pointer is in any of the slabs (this would be bad) + for (slab_ptr, layout) in &self.slabs { + if !slab_ptr.is_null() { + let slab_start = *slab_ptr as *mut u64; + let slab_size = layout.size() / std::mem::size_of::(); + let slab_end = unsafe { slab_start.add(slab_size) }; + + if (ptr as *mut u64) >= slab_start && (ptr as *mut u64) < slab_end { + return Err(format!( + "Found noun still allocated in slab at {:p}", + ptr + )); + } + } + } + + // If we get here, the pointer is neither in PMA nor in slabs + // This is expected as it should be in the NockStack + + match allocated.as_either() { + Either::Left(_) => (), // No need to traverse indirect atoms + Either::Right(cell) => { + stack.push(cell.head()); + stack.push(cell.tail()); + } + } + } + } + Ok(()) + } + /// Set the root of the noun slab. /// /// Panics if the given root is not in the noun slab or PMA. From 621d9f4604630560dc71b3ef605fb4afdb794913 Mon Sep 17 00:00:00 2001 From: Jonathan Paprocki Date: Wed, 13 Nov 2024 15:04:17 -0500 Subject: [PATCH 10/15] check normalization after copying --- crown/src/noun/slab.rs | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/crown/src/noun/slab.rs b/crown/src/noun/slab.rs index 95dc24a..b4f58b1 100644 --- a/crown/src/noun/slab.rs +++ b/crown/src/noun/slab.rs @@ -311,7 +311,40 @@ impl NounSlab { // This is expected as it should be in the NockStack match allocated.as_either() { - Either::Left(_) => (), // No need to traverse indirect atoms + Either::Left(indirect) => { + // Check normalization of indirect atoms + let slice = indirect.as_slice(); + if !slice.is_empty() && slice.last().unwrap() == &0 { + return Err(format!( + "Found non-normalized indirect atom at {:p} with value {:?}", + ptr, + slice + )); + } + + // Check that indirect atom size is correct + let atom = indirect.as_atom(); + let actual_size = slice.len(); + let expected_size = met(6, atom); + if actual_size != expected_size { + return Err(format!( + "Found indirect atom with incorrect size at {:p} - expected {} words but got {}", + ptr, + expected_size, + actual_size + )); + } + + // Check that it shouldn't be a direct atom + let bit_size = met(0, atom); + if bit_size <= 63 { + return Err(format!( + "Found indirect atom that should be direct at {:p} - only {} bits", + ptr, + bit_size + )); + } + }, Either::Right(cell) => { stack.push(cell.head()); stack.push(cell.tail()); From e8e7305c7afb5d29bf239949306b3b60b00df429 Mon Sep 17 00:00:00 2001 From: Jonathan Paprocki Date: Wed, 20 Nov 2024 13:13:58 -0500 Subject: [PATCH 11/15] validate mugs --- crown/src/noun/slab.rs | 210 +++++++++++++++-------------------------- 1 file changed, 77 insertions(+), 133 deletions(-) diff --git a/crown/src/noun/slab.rs b/crown/src/noun/slab.rs index b4f58b1..e8a82dc 100644 --- a/crown/src/noun/slab.rs +++ b/crown/src/noun/slab.rs @@ -7,19 +7,13 @@ use intmap::IntMap; use std::alloc::Layout; use std::mem::size_of; use std::ptr::copy_nonoverlapping; +use sword::jets::bits::util::met; use sword::mem::NockStack; -use sword::mug::{calc_atom_mug_u32, calc_cell_mug_u32, get_mug, set_mug}; +use sword::mug::{calc_atom_mug_u32, calc_cell_mug_u32, get_mug, mug_u32, set_mug}; use sword::noun::{Atom, Cell, CellMemory, DirectAtom, IndirectAtom, Noun, NounAllocator, D}; -use sword::jets::hot::URBIT_HOT_STATE; -use sword::jets::bits::util::met; use sword::persist::pma_contains; use sword::serialization::{met0_u64_to_usize, met0_usize}; use thiserror::Error; -use sword::interpreter::{Context, interpret}; -use sword::jets::{cold::Cold, warm::Warm, hot::Hot}; -use sword::hamt::Hamt; -use std::pin::Pin; -use crate::utils::slogger::CrownSlogger; const CELL_MEM_WORD_SIZE: usize = (size_of::() + 7) >> 3; @@ -200,6 +194,8 @@ impl NounSlab { break; } } + self.validate_root() + .expect("Noun not properly copied into slab"); } /// Copy the root noun from this slab into the given NockStack, only leaving references into the PMA @@ -264,20 +260,21 @@ impl NounSlab { } // Verify that the copied noun is fully in the stack or PMA - self.verify_copied_noun(res).expect("Noun was not fully copied to stack"); - + self.verify_copied_noun(res, stack) + .expect("Noun was not properly copied to stack"); + res } /// Verifies that a noun does not contain any references to memory in this slab - fn verify_copied_noun(&self, noun: Noun) -> Result<(), String> { - let mut stack = vec![noun]; + fn verify_copied_noun(&self, noun: Noun, nockstack: &mut NockStack) -> Result<(), String> { + let mut stack = vec![noun]; // traversal stack let mut visited = std::collections::HashSet::new(); while let Some(noun) = stack.pop() { if let Ok(allocated) = noun.as_allocated() { let ptr = unsafe { allocated.to_raw_pointer() }; - + // Skip if we've seen this pointer before if !visited.insert(ptr as u64) { continue; @@ -285,10 +282,18 @@ impl NounSlab { // Check if pointer is in PMA if unsafe { pma_contains(ptr, 1) } { - return Err(format!( - "Found noun allocated in PMA at {:p}", - ptr - )); + return Err(format!("Found noun allocated in PMA at {:p}", ptr)); + } + + // Verify cached mug if present + if let Some(cached_mug) = allocated.get_cached_mug() { + let computed_mug = mug_u32(nockstack, noun); + if cached_mug != computed_mug { + return Err(format!( + "Found noun with incorrect mug at {:p} - cached: {}, computed: {}", + ptr, cached_mug, computed_mug + )); + } } // Check if pointer is in any of the slabs (this would be bad) @@ -297,12 +302,9 @@ impl NounSlab { let slab_start = *slab_ptr as *mut u64; let slab_size = layout.size() / std::mem::size_of::(); let slab_end = unsafe { slab_start.add(slab_size) }; - + if (ptr as *mut u64) >= slab_start && (ptr as *mut u64) < slab_end { - return Err(format!( - "Found noun still allocated in slab at {:p}", - ptr - )); + return Err(format!("Found noun still allocated in slab at {:p}", ptr)); } } } @@ -317,8 +319,7 @@ impl NounSlab { if !slice.is_empty() && slice.last().unwrap() == &0 { return Err(format!( "Found non-normalized indirect atom at {:p} with value {:?}", - ptr, - slice + ptr, slice )); } @@ -340,11 +341,10 @@ impl NounSlab { if bit_size <= 63 { return Err(format!( "Found indirect atom that should be direct at {:p} - only {} bits", - ptr, - bit_size + ptr, bit_size )); } - }, + } Either::Right(cell) => { stack.push(cell.head()); stack.push(cell.tail()); @@ -394,6 +394,7 @@ impl NounSlab { } } } + // we do not call validate_root here since this already performs the same check self.root = root; } @@ -487,6 +488,8 @@ impl NounSlab { } } } + self.validate_root() + .expect("Noun was not fully cued into slab"); Ok(res) } @@ -506,12 +509,23 @@ impl NounSlab { while let Some(noun) = stack.pop() { if let Ok(allocated) = noun.as_allocated() { let ptr = unsafe { allocated.to_raw_pointer() }; - + // Skip if we've seen this pointer before if !visited.insert(ptr as u64) { continue; } + // Verify cached mug if present + if let Some(cached_mug) = allocated.get_cached_mug() { + let computed_mug = slab_mug(noun); + if cached_mug != computed_mug { + return Err(format!( + "Found noun with incorrect mug at {:p} - cached: {}, computed: {}", + ptr, cached_mug, computed_mug + )); + } + } + // Check if pointer is in any of the slabs let mut found_in_slab = false; for (slab_ptr, layout) in &self.slabs { @@ -519,7 +533,7 @@ impl NounSlab { let slab_start = *slab_ptr as *mut u64; let slab_size = layout.size() / std::mem::size_of::(); let slab_end = unsafe { slab_start.add(slab_size) }; - + if (ptr as *mut u64) >= slab_start && (ptr as *mut u64) < slab_end { found_in_slab = true; break; @@ -528,10 +542,10 @@ impl NounSlab { } let is_in_pma = unsafe { pma_contains(ptr, 1) }; - + if !found_in_slab && !is_in_pma { return Err(format!( - "Found noun allocated outside of all slabs and PMA at {:p}", + "Found noun allocated outside of all slabs and PMA at {:p}", ptr )); } @@ -543,8 +557,7 @@ impl NounSlab { if !slice.is_empty() && slice.last().unwrap() == &0 { return Err(format!( "Found non-normalized indirect atom at {:p} with value {:?}", - ptr, - slice + ptr, slice )); } @@ -566,8 +579,7 @@ impl NounSlab { if bit_size <= 63 { return Err(format!( "Found indirect atom that should be direct at {:p} - only {} bits", - ptr, - bit_size + ptr, bit_size )); } } @@ -1040,117 +1052,49 @@ mod tests { } } - #[test] - fn test_nockstack_slab_equality() { - // Create initial NockStack with the test noun - let mut stack = NockStack::new(100000, 0); - let a_values = [ - 0xea31bc2f1dcd1ff1u64, - 0x0dd3c7e3b75f3abbu64, - 0x8f9ff1e4b2ca417fu64, - 0x3bc6aedea88fed36u64, - 0x9d68355b4a0a18d0u64, - ]; - // DIRECT_MAX is u64::MAX >> 1 = 0x7FFF_FFFF_FFFF_FFFF - // Compare each value against DIRECT_MAX to determine if it needs to be indirect - println!("DIRECT_MAX = 0x7FFF_FFFF_FFFF_FFFF"); - for (i, &val) in a_values.iter().enumerate() { - println!("a_values[{}] = 0x{:016x} {}", i, val, - if val <= 0x7FFF_FFFF_FFFF_FFFF { - "(direct)" - } else { - "(indirect)" - } - ); - } - // Convert each value to an Atom using from_bytes - let atoms: Vec<_> = a_values.iter() - .map(|&x| { - let bytes = Bytes::copy_from_slice(&x.to_le_bytes()); - Atom::from_bytes(&mut stack, &bytes).as_noun() - }) - .collect(); - - // Create 'a' as a cell containing these atoms - let a = T(&mut stack, &atoms); - - // Create cell X = [a a] - let x = T(&mut stack, &[a, a]); - - println!("Original X in NockStack:"); - println!("{:?}", x); - - // Copy to NounSlab - let mut slab = NounSlab::new(); - slab.copy_into(x); - let slab_x = unsafe { slab.root() }; - - println!("X in NounSlab:"); - println!("{:?}", slab_x); - - // Validate that nouns are in the allocation range - match slab.validate_root() { - Ok(_) => println!("Slab validation passed"), - Err(e) => { - println!("Slab validation failed: {}", e); - panic!("Validation failed"); - } - } - - // Copy back to new NockStack - let mut new_stack = NockStack::new(100000, 0); - let copied_x = slab.copy_to_stack(&mut new_stack); - - println!("Copied X back to NockStack:"); - println!("{:?}", copied_x); - - // Create interpreter context - let mut cold = Cold::new(&mut new_stack); - let hot = Hot::init(&mut new_stack, &URBIT_HOT_STATE); - let warm = Warm::init(&mut new_stack, &mut cold, &hot); - let cache = Hamt::::new(&mut new_stack); - let slogger = std::boxed::Box::pin(CrownSlogger {}); - let mut context = Context { - stack: new_stack, - slogger, - cold, - warm, - hot, - cache, - scry_stack: D(0), - trace_info: None, - }; - - // Get the head and tail of X to compare them - let head = T(&mut context.stack, &[D(0), D(2)]); // [0 2] gets head - let tail = T(&mut context.stack, &[D(0), D(3)]); // [0 3] gets tail - let formula = T(&mut context.stack, &[D(5), head, tail]); // [5 head tail] - - let result = interpret(&mut context, copied_x, formula); - - println!("Interpret result: {:?}", result); - assert!(unsafe { result.unwrap().raw_equals(D(0)) }, - "Result should be 0 (true) since head and tail are equal"); - } - #[test] fn test_validate_root() { let mut slab = NounSlab::new(); - + // Valid case - simple cell let noun = T(&mut slab, &[D(1), D(2)]); slab.set_root(noun); - assert!(slab.validate_root().is_ok(), "Valid noun should pass validation"); + assert!( + slab.validate_root().is_ok(), + "Valid noun should pass validation" + ); // Valid case - indirect atom let large_number = u64::MAX as u128 + 1; let large_number_bytes = Bytes::from(large_number.to_le_bytes().to_vec()); let indirect_atom = Atom::from_bytes(&mut slab, &large_number_bytes); slab.set_root(indirect_atom.as_noun()); - assert!(slab.validate_root().is_ok(), "Valid indirect atom should pass validation"); + assert!( + slab.validate_root().is_ok(), + "Valid indirect atom should pass validation" + ); // Invalid case - non-normalized indirect atom - // This would require creating a non-normalized atom directly, which isn't - // possible through normal APIs. You might need unsafe code to test this. + unsafe { + // Allocate space for a 2-word indirect atom + let ptr = slab.alloc_indirect(2); + // Write a value ending in zero (non-normalized) + *ptr.add(1) = 2; // size 2 words + *ptr.add(2) = 123; // first word + *ptr.add(3) = 0; // second word is zero - not normalized + // Create an indirect atom from this memory + let non_normalized = IndirectAtom::from_raw_pointer(ptr).as_atom().as_noun(); + slab.set_root(non_normalized); + + let result = slab.validate_root(); + assert!( + result.is_err(), + "Non-normalized indirect atom should fail validation" + ); + assert!( + result.unwrap_err().contains("non-normalized indirect atom"), + "Error message should mention non-normalized atom" + ); + } } } From 8fb9e8effa622795fc1cbe445fe2d8f3f257b414 Mon Sep 17 00:00:00 2001 From: Jonathan Paprocki Date: Wed, 20 Nov 2024 13:44:26 -0500 Subject: [PATCH 12/15] actually compute the mug when needed --- crown/src/noun/slab.rs | 75 ++++++++++++++++++++++++++++++++++++++---- 1 file changed, 69 insertions(+), 6 deletions(-) diff --git a/crown/src/noun/slab.rs b/crown/src/noun/slab.rs index e8a82dc..044e3ca 100644 --- a/crown/src/noun/slab.rs +++ b/crown/src/noun/slab.rs @@ -260,14 +260,13 @@ impl NounSlab { } // Verify that the copied noun is fully in the stack or PMA - self.verify_copied_noun(res, stack) + self.verify_copied_noun(res) .expect("Noun was not properly copied to stack"); - res } /// Verifies that a noun does not contain any references to memory in this slab - fn verify_copied_noun(&self, noun: Noun, nockstack: &mut NockStack) -> Result<(), String> { + fn verify_copied_noun(&self, noun: Noun) -> Result<(), String> { let mut stack = vec![noun]; // traversal stack let mut visited = std::collections::HashSet::new(); @@ -287,7 +286,8 @@ impl NounSlab { // Verify cached mug if present if let Some(cached_mug) = allocated.get_cached_mug() { - let computed_mug = mug_u32(nockstack, noun); + let computed_mug = slab_mug_no_cache(noun); + println!("cached_mug: {}, computed_mug: {}", cached_mug, computed_mug); if cached_mug != computed_mug { return Err(format!( "Found noun with incorrect mug at {:p} - cached: {}, computed: {}", @@ -394,7 +394,8 @@ impl NounSlab { } } } - // we do not call validate_root here since this already performs the same check + self.validate_root() + .expect("Noun was not properly copied into slab"); self.root = root; } @@ -517,7 +518,7 @@ impl NounSlab { // Verify cached mug if present if let Some(cached_mug) = allocated.get_cached_mug() { - let computed_mug = slab_mug(noun); + let computed_mug = slab_mug_no_cache(noun); if cached_mug != computed_mug { return Err(format!( "Found noun with incorrect mug at {:p} - cached: {}, computed: {}", @@ -856,6 +857,28 @@ fn slab_mug(a: Noun) -> u32 { get_mug(a).expect("Noun should have a mug once mugged.") } +/// Calculate the mug of a noun without using the cache. +fn slab_mug_no_cache(a: Noun) -> u32 { + match a.as_either_direct_allocated() { + Either::Left(direct) => { + calc_atom_mug_u32(direct.as_atom()) + } + Either::Right(allocated) => { + match allocated.as_either() { + Either::Left(indirect) => { + calc_atom_mug_u32(indirect.as_atom()) + } + Either::Right(cell) => { + // Recursively calculate mugs for head and tail + let head_mug = slab_mug_no_cache(cell.head()); + let tail_mug = slab_mug_no_cache(cell.tail()); + unsafe { calc_cell_mug_u32(head_mug, tail_mug) } + } + } + } + } +} + enum CueStackEntry { DestinationPointer(*mut Noun), BackRef(u64, *const Noun), @@ -1097,4 +1120,44 @@ mod tests { ); } } + + #[test] + fn test_verify_copied_noun() { + let mut stack = NockStack::new(10000, 0); + let slab = NounSlab::new(); + + // Create a valid noun in the stack + let valid_noun = T(&mut stack, &[D(1), D(2)]); + + // This should pass verification since it's properly allocated in the stack + let result = slab.verify_copied_noun(valid_noun); + assert!( + result.is_ok(), + "Valid noun in stack should pass verification" + ); + + // Create a noun with an incorrect mug + unsafe { + // Allocate a cell in the stack + let cell_mem = stack.alloc_cell(); + (*cell_mem).head = D(1); + (*cell_mem).tail = D(2); + // Set an incorrect mug value + let cell = Cell::from_raw_pointer(cell_mem); + set_mug(cell.as_allocated(), 12345); // Wrong mug value + + let invalid_noun = cell.as_noun(); + + // This should fail verification due to incorrect mug + let result = slab.verify_copied_noun(invalid_noun); + assert!( + result.is_err(), + "Noun with incorrect mug should fail verification" + ); + assert!( + result.unwrap_err().contains("incorrect mug"), + "Error message should mention incorrect mug" + ); + } + } } From 13ea6137b6fa2869190da39f81c732d4d1e4817c Mon Sep 17 00:00:00 2001 From: Jonathan Paprocki Date: Wed, 20 Nov 2024 13:52:33 -0500 Subject: [PATCH 13/15] add validate-nouns feature --- README.md | 5 +++++ crown/Cargo.toml | 1 + crown/src/noun/slab.rs | 27 +++++++++++++-------------- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 8e21255..9487ae2 100644 --- a/README.md +++ b/README.md @@ -28,3 +28,8 @@ The `crown` library is the primary framework for building NockApps. It provides For compiling Hoon to Nock, we're also including a pre-release of `choo`: a NockApp for the Hoon compiler. `choo` can compile Hoon to Nock as a batch-mode command-line process, without the need to spin up an interactive Urbit ship. It is intended both for developer workflows and for CI. `choo` is also our first example NockApp. More are coming! +## Features + +- `validate-nouns`: Enable runtime validation of noun operations. This performs additional checks to ensure noun integrity but may impact performance. Disable this feature in production for better performance. + +In order to enable this feature, run `cargo build -p crown --features validate-nouns`. \ No newline at end of file diff --git a/crown/Cargo.toml b/crown/Cargo.toml index a02d421..b234116 100644 --- a/crown/Cargo.toml +++ b/crown/Cargo.toml @@ -5,6 +5,7 @@ edition.workspace = true [features] trait-alias = [] +validate-nouns = [] [dependencies] anyhow = { workspace = true } diff --git a/crown/src/noun/slab.rs b/crown/src/noun/slab.rs index 044e3ca..db7d85a 100644 --- a/crown/src/noun/slab.rs +++ b/crown/src/noun/slab.rs @@ -9,7 +9,7 @@ use std::mem::size_of; use std::ptr::copy_nonoverlapping; use sword::jets::bits::util::met; use sword::mem::NockStack; -use sword::mug::{calc_atom_mug_u32, calc_cell_mug_u32, get_mug, mug_u32, set_mug}; +use sword::mug::{calc_atom_mug_u32, calc_cell_mug_u32, get_mug, set_mug}; use sword::noun::{Atom, Cell, CellMemory, DirectAtom, IndirectAtom, Noun, NounAllocator, D}; use sword::persist::pma_contains; use sword::serialization::{met0_u64_to_usize, met0_usize}; @@ -194,6 +194,7 @@ impl NounSlab { break; } } + #[cfg(feature = "validate-nouns")] self.validate_root() .expect("Noun not properly copied into slab"); } @@ -260,6 +261,7 @@ impl NounSlab { } // Verify that the copied noun is fully in the stack or PMA + #[cfg(feature = "validate-nouns")] self.verify_copied_noun(res) .expect("Noun was not properly copied to stack"); res @@ -489,6 +491,7 @@ impl NounSlab { } } } + #[cfg(feature = "validate-nouns")] self.validate_root() .expect("Noun was not fully cued into slab"); Ok(res) @@ -860,14 +863,10 @@ fn slab_mug(a: Noun) -> u32 { /// Calculate the mug of a noun without using the cache. fn slab_mug_no_cache(a: Noun) -> u32 { match a.as_either_direct_allocated() { - Either::Left(direct) => { - calc_atom_mug_u32(direct.as_atom()) - } + Either::Left(direct) => calc_atom_mug_u32(direct.as_atom()), Either::Right(allocated) => { match allocated.as_either() { - Either::Left(indirect) => { - calc_atom_mug_u32(indirect.as_atom()) - } + Either::Left(indirect) => calc_atom_mug_u32(indirect.as_atom()), Either::Right(cell) => { // Recursively calculate mugs for head and tail let head_mug = slab_mug_no_cache(cell.head()); @@ -1102,13 +1101,13 @@ mod tests { // Allocate space for a 2-word indirect atom let ptr = slab.alloc_indirect(2); // Write a value ending in zero (non-normalized) - *ptr.add(1) = 2; // size 2 words - *ptr.add(2) = 123; // first word - *ptr.add(3) = 0; // second word is zero - not normalized - // Create an indirect atom from this memory + *ptr.add(1) = 2; // size 2 words + *ptr.add(2) = 123; // first word + *ptr.add(3) = 0; // second word is zero - not normalized + // Create an indirect atom from this memory let non_normalized = IndirectAtom::from_raw_pointer(ptr).as_atom().as_noun(); slab.set_root(non_normalized); - + let result = slab.validate_root(); assert!( result.is_err(), @@ -1128,7 +1127,7 @@ mod tests { // Create a valid noun in the stack let valid_noun = T(&mut stack, &[D(1), D(2)]); - + // This should pass verification since it's properly allocated in the stack let result = slab.verify_copied_noun(valid_noun); assert!( @@ -1147,7 +1146,7 @@ mod tests { set_mug(cell.as_allocated(), 12345); // Wrong mug value let invalid_noun = cell.as_noun(); - + // This should fail verification due to incorrect mug let result = slab.verify_copied_noun(invalid_noun); assert!( From 1cdb16541738e87b35b62349ce3a56080b5a75a2 Mon Sep 17 00:00:00 2001 From: Jonathan Paprocki Date: Wed, 20 Nov 2024 13:54:01 -0500 Subject: [PATCH 14/15] comment --- crown/src/noun/slab.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crown/src/noun/slab.rs b/crown/src/noun/slab.rs index db7d85a..6690924 100644 --- a/crown/src/noun/slab.rs +++ b/crown/src/noun/slab.rs @@ -493,7 +493,7 @@ impl NounSlab { } #[cfg(feature = "validate-nouns")] self.validate_root() - .expect("Noun was not fully cued into slab"); + .expect("Noun was not properly cued into slab"); Ok(res) } From 07f2de6296659833014fb9f081bc50a2f923e74f Mon Sep 17 00:00:00 2001 From: Jonathan Paprocki Date: Thu, 21 Nov 2024 11:53:18 -0500 Subject: [PATCH 15/15] CI --- crown/src/noun/slab.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crown/src/noun/slab.rs b/crown/src/noun/slab.rs index 6690924..b1268cf 100644 --- a/crown/src/noun/slab.rs +++ b/crown/src/noun/slab.rs @@ -268,6 +268,7 @@ impl NounSlab { } /// Verifies that a noun does not contain any references to memory in this slab + #[allow(dead_code)] fn verify_copied_noun(&self, noun: Noun) -> Result<(), String> { let mut stack = vec![noun]; // traversal stack let mut visited = std::collections::HashSet::new(); @@ -396,6 +397,7 @@ impl NounSlab { } } } + #[cfg(feature = "validate-nouns")] self.validate_root() .expect("Noun was not properly copied into slab"); self.root = root; @@ -506,6 +508,7 @@ impl NounSlab { /// Validates that all allocated nouns in the tree are contained within one of this slab's /// allocated memory regions or the PMA + #[allow(dead_code)] pub fn validate_root(&self) -> Result<(), String> { let mut stack = vec![self.root]; let mut visited = std::collections::HashSet::new();