diff --git a/Cargo.lock b/Cargo.lock index d6b2c93..fbcef9b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1350,7 +1350,6 @@ dependencies = [ "miden-assembly", "miden-assembly-syntax", "miden-core", - "miden-crypto", "miden-debug-types", "miden-mast-package", "miden-processor", diff --git a/Cargo.toml b/Cargo.toml index 5ba28a4..b84a56c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -35,12 +35,12 @@ crossterm = { version = "0.28.1", optional = true, features = ["event-stream"] } env_logger = { version = "0.11", optional = true } log = "0.4" glob = { version = "0.3.1", optional = true } -miden-assembly = { version = "=0.21.1", default-features = false } -miden-assembly-syntax = { version = "=0.21.1", default-features = false } -miden-core = { version = "=0.21.1", default-features = false } -miden-debug-types = { version = "=0.21.1", default-features = false } -miden-mast-package = { version = "=0.21.1", default-features = false } -miden-processor = { version = "=0.21.1", default-features = false } +miden-assembly = { version = "0.21", default-features = false } +miden-assembly-syntax = { version = "0.21", default-features = false } +miden-core = { version = "0.21", default-features = false } +miden-debug-types = { version = "0.21", default-features = false } +miden-mast-package = { version = "0.21", default-features = false } +miden-processor = { version = "0.21", default-features = false } num-traits = "0.2" ratatui = { version = "0.29.0", optional = true } rustc-demangle = { version = "0.1", features = ["std"] } @@ -72,5 +72,3 @@ tokio-util = "0.7.11" futures = "0.3.30" proptest = { version = "1.4", optional = true } -# Pin miden-crypto to match what miden-vm v0.21.1 requires -miden-crypto = { version = "=0.22.3", default-features = false } diff --git a/src/debug/memory.rs b/src/debug/memory.rs index 6618451..2bb4915 100644 --- a/src/debug/memory.rs +++ b/src/debug/memory.rs @@ -263,3 +263,28 @@ impl clap::builder::TypedValueParser for FormatTypeParser { value.parse().map_err(|err| Error::raw(ErrorKind::InvalidValue, err)) } } + +#[cfg(test)] +mod tests { + use super::FormatType; + use crate::test_utils::write_scalar_bytes; + + #[test] + fn write_scalar_bytes_reads_little_endian_u64() { + let mut output = String::new(); + + write_scalar_bytes(&mut output, "u64", FormatType::Decimal, &[1, 2, 3, 4, 5, 6, 7, 8]) + .unwrap(); + + assert_eq!(output, u64::from_le_bytes([1, 2, 3, 4, 5, 6, 7, 8]).to_string()); + } + + #[test] + fn write_scalar_bytes_reads_little_endian_u16_hex() { + let mut output = String::new(); + + write_scalar_bytes(&mut output, "u16", FormatType::Hex, &[0x34, 0x12]).unwrap(); + + assert_eq!(output, "1234"); + } +} diff --git a/src/exec/config.rs b/src/exec/config.rs index 5d692d7..f4c0eac 100644 --- a/src/exec/config.rs +++ b/src/exec/config.rs @@ -46,7 +46,7 @@ impl ExecutionConfig { let inputs = StackInputs::new(&felts).map_err(|err| format!("invalid value for 'stack': {err}"))?; let advice_inputs = AdviceInputs::default() - .with_stack(file.inputs.advice.stack.into_iter().rev().map(|felt| felt.0)) + .with_stack(file.inputs.advice.stack.into_iter().map(|felt| felt.0)) .with_map(file.inputs.advice.map.into_iter().map(|entry| { (entry.digest.0, entry.values.into_iter().map(|felt| felt.0).collect::>()) })); @@ -182,7 +182,7 @@ mod tests { use miden_processor::Felt as RawFelt; use toml::toml; - use super::*; + use super::{ExecutionConfig, *}; #[test] fn execution_config_empty() { @@ -270,7 +270,7 @@ mod tests { assert_eq!(file.inputs.as_ref(), expected_inputs.as_ref()); assert_eq!( file.advice_inputs.stack, - &[RawFelt::new(4), RawFelt::new(3), RawFelt::new(2), RawFelt::new(1)] + &[RawFelt::new(1), RawFelt::new(2), RawFelt::new(3), RawFelt::new(4)] ); assert_eq!( file.advice_inputs.map.get(&digest).map(|value| value.as_ref()), diff --git a/src/exec/executor.rs b/src/exec/executor.rs index a1e4b2c..15bc612 100644 --- a/src/exec/executor.rs +++ b/src/exec/executor.rs @@ -15,7 +15,9 @@ use miden_mast_package::{ MemDependencyResolverByDigest, ResolvedDependency, }; use miden_processor::{ - ContextId, ExecutionError, ExecutionOptions, FastProcessor, Felt, advice::AdviceInputs, + ContextId, ExecutionError, ExecutionOptions, FastProcessor, Felt, + advice::AdviceInputs, + event::{EventHandler, EventName}, trace::RowIndex, }; @@ -33,6 +35,7 @@ pub struct Executor { advice: AdviceInputs, options: ExecutionOptions, libraries: Vec>, + event_handlers: Vec<(EventName, Arc)>, dependency_resolver: MemDependencyResolverByDigest, } impl Executor { @@ -63,6 +66,7 @@ impl Executor { advice: advice_inputs, options, libraries: Default::default(), + event_handlers: Default::default(), dependency_resolver, } } @@ -132,6 +136,16 @@ impl Executor { self } + /// Register a VM event handler to be available during execution. + pub fn register_event_handler( + &mut self, + event: EventName, + handler: Arc, + ) -> Result<&mut Self, ExecutionError> { + self.event_handlers.push((event, handler)); + Ok(self) + } + /// Convert this [Executor] into a [DebugExecutor], which captures much more information /// about the program being executed, and must be stepped manually. pub fn into_debug( @@ -145,6 +159,10 @@ impl Executor { for lib in core::mem::take(&mut self.libraries) { host.load_mast_forest(lib.mast_forest().clone()); } + for (event, handler) in core::mem::take(&mut self.event_handlers) { + host.register_event_handler(event, handler) + .expect("failed to register debug executor event handler"); + } let trace_events: Rc>> = Rc::new(Default::default()); let frame_start_events = Rc::clone(&trace_events); diff --git a/src/exec/host.rs b/src/exec/host.rs index 6a556f9..c32e84f 100644 --- a/src/exec/host.rs +++ b/src/exec/host.rs @@ -1,11 +1,18 @@ use std::{collections::BTreeMap, num::NonZeroU32, sync::Arc}; use miden_assembly::SourceManager; -use miden_core::Word; +use miden_core::{ + Word, + events::{EventId, EventName}, +}; use miden_debug_types::{Location, SourceFile, SourceSpan}; use miden_processor::{ - FutureMaybeSend, Host, MastForestStore, MemMastForestStore, ProcessorState, TraceError, - advice::AdviceMutation, event::EventError, mast::MastForest, trace::RowIndex, + ExecutionError, FutureMaybeSend, Host, MastForestStore, MemMastForestStore, ProcessorState, + TraceError, + advice::AdviceMutation, + event::{EventError, EventHandler, EventHandlerRegistry}, + mast::MastForest, + trace::RowIndex, }; use super::{TraceEvent, TraceHandler}; @@ -15,6 +22,7 @@ use super::{TraceEvent, TraceHandler}; /// events that record the entry or exit of a procedure call frame. pub struct DebuggerHost { store: MemMastForestStore, + event_handlers: EventHandlerRegistry, tracing_callbacks: BTreeMap>>, on_assert_failed: Option>, source_manager: Arc, @@ -27,6 +35,7 @@ where pub fn new(source_manager: Arc) -> Self { Self { store: Default::default(), + event_handlers: EventHandlerRegistry::default(), tracing_callbacks: Default::default(), on_assert_failed: None, source_manager, @@ -67,6 +76,15 @@ where pub fn load_mast_forest(&mut self, forest: Arc) { self.store.insert(forest); } + + /// Registers an event handler for use during program execution. + pub fn register_event_handler( + &mut self, + event: EventName, + handler: Arc, + ) -> Result<(), ExecutionError> { + self.event_handlers.register(event, handler) + } } impl Host for DebuggerHost @@ -88,9 +106,21 @@ where fn on_event( &mut self, - _process: &ProcessorState<'_>, + process: &ProcessorState<'_>, ) -> impl FutureMaybeSend, EventError>> { - std::future::ready(Ok(Vec::new())) + let event_id = EventId::from_felt(process.get_stack_item(0)); + let result = match self.event_handlers.handle_event(event_id, process) { + Ok(Some(mutations)) => Ok(mutations), + Ok(None) => { + #[derive(Debug, thiserror::Error)] + #[error("no event handler registered")] + struct UnhandledEvent; + + Err(UnhandledEvent.into()) + } + Err(err) => Err(err), + }; + std::future::ready(result) } fn on_trace(&mut self, process: &ProcessorState<'_>, trace_id: u32) -> Result<(), TraceError> { @@ -103,4 +133,8 @@ where } Ok(()) } + + fn resolve_event(&self, event_id: EventId) -> Option<&EventName> { + self.event_handlers.resolve_event(event_id) + } } diff --git a/src/exec/trace.rs b/src/exec/trace.rs index 678ef99..25d4f2b 100644 --- a/src/exec/trace.rs +++ b/src/exec/trace.rs @@ -124,7 +124,7 @@ impl ExecutionTrace { let mut needed = size - buf.len(); for elem in elems { - let bytes = ((elem.as_canonical_u64() & U32_MASK) as u32).to_be_bytes(); + let bytes = ((elem.as_canonical_u64() & U32_MASK) as u32).to_le_bytes(); let take = core::cmp::min(needed, 4); buf.extend(&bytes[0..take]); needed -= take; @@ -154,43 +154,88 @@ impl ExecutionTrace { where T: core::any::Any + FromMidenRepr, { - use core::any::TypeId; - let ptr = NativePtr::from_ptr(addr); - if TypeId::of::() == TypeId::of::() { - assert_eq!(ptr.offset, 0, "cannot read values of type Felt from unaligned addresses"); - } assert_eq!(ptr.offset, 0, "support for unaligned reads is not yet implemented"); - match ::size_in_felts() { - 1 => { - let felt = self.read_memory_element_in_context(ptr.addr, ctx, clk)?; - Some(T::from_felts(&[felt])) - } - 2 => { - let lo = self.read_memory_element_in_context(ptr.addr, ctx, clk)?; - let hi = self.read_memory_element_in_context(ptr.addr + 1, ctx, clk)?; - Some(T::from_felts(&[lo, hi])) - } - 3 => { - let lo_l = self.read_memory_element_in_context(ptr.addr, ctx, clk)?; - let lo_h = self.read_memory_element_in_context(ptr.addr + 1, ctx, clk)?; - let hi_l = self.read_memory_element_in_context(ptr.addr + 2, ctx, clk)?; - Some(T::from_felts(&[lo_l, lo_h, hi_l])) - } - n => { - assert_ne!(n, 0); - let num_words = n.next_multiple_of(4) / 4; - let mut words = SmallVec::<[_; 2]>::with_capacity(num_words); - for word_index in 0..(num_words as u32) { - let addr = ptr.addr + (word_index * 4); - let mut word = self.read_memory_word(addr)?; - word.reverse(); - dbg!(word_index, word); - words.push(word); - } - words.resize(num_words, Word::new([Felt::ZERO; 4])); - Some(T::from_words(&words)) - } + let size = ::size_in_felts(); + let mut felts = SmallVec::<[_; 4]>::with_capacity(size); + for index in 0..(size as u32) { + felts.push(self.read_memory_element_in_context(ptr.addr + index, ctx, clk)?); } + Some(T::from_felts(&felts)) + } +} + +#[cfg(test)] +mod tests { + use std::sync::Arc; + + use miden_assembly::DefaultSourceManager; + use miden_assembly_syntax::ast::types::Type; + use miden_processor::{ContextId, trace::RowIndex}; + + use super::ExecutionTrace; + use crate::{Executor, debug::NativePtr, felt::ToMidenRepr}; + + fn empty_trace() -> ExecutionTrace { + ExecutionTrace { + root_context: ContextId::root(), + last_cycle: RowIndex::from(0_u32), + processor: miden_processor::FastProcessor::new(miden_processor::StackInputs::default()), + outputs: miden_processor::StackOutputs::default(), + } + } + + fn execute_trace(source: &str) -> ExecutionTrace { + let source_manager = Arc::new(DefaultSourceManager::default()); + let program = miden_assembly::Assembler::new(source_manager.clone()) + .assemble_program(source) + .unwrap(); + + Executor::new(vec![]).capture_trace(&program, source_manager) + } + + #[test] + fn parse_result_reads_multi_felt_outputs_in_stack_order() { + let outputs = 0x0807_0605_0403_0201_u64.to_felts(); + let trace = ExecutionTrace { + outputs: miden_processor::StackOutputs::new(&outputs).unwrap(), + ..empty_trace() + }; + + let result = trace.parse_result::().unwrap(); + + assert_eq!(result, 0x0807_0605_0403_0201_u64); + } + + #[test] + fn read_bytes_for_type_preserves_little_endian_bytes() { + let trace = execute_trace( + r#" +begin + push.4660 + push.8 + mem_store + + push.67305985 + push.12 + mem_store + + push.134678021 + push.13 + mem_store +end +"#, + ); + let ctx = ContextId::root(); + + let u16_bytes = trace + .read_bytes_for_type(NativePtr::new(8, 0), &Type::U16, ctx, RowIndex::from(0_u32)) + .unwrap(); + let u64_bytes = trace + .read_bytes_for_type(NativePtr::new(12, 0), &Type::U64, ctx, RowIndex::from(0_u32)) + .unwrap(); + + assert_eq!(u16_bytes, vec![0x34, 0x12]); + assert_eq!(u64_bytes, vec![1, 2, 3, 4, 5, 6, 7, 8]); } } diff --git a/src/felt.rs b/src/felt.rs index 71fdfa5..2a902f2 100644 --- a/src/felt.rs +++ b/src/felt.rs @@ -1,5 +1,4 @@ -use miden_core::Word; -use miden_core::field::PrimeField64; +use miden_core::{Word, field::PrimeField64}; use miden_processor::Felt as RawFelt; #[cfg(feature = "proptest")] use proptest::{ @@ -66,10 +65,7 @@ pub trait ToMidenRepr { /// If pushing arguments for functions compiled from Wasm, consider using /// [`push_wasm_ty_to_operand_stack`] instead. fn push_to_operand_stack(&self, stack: &mut Vec) { - let felts = self.to_felts(); - for felt in felts.into_iter().rev() { - stack.push(felt); - } + stack.extend(self.to_felts()); } /// Push this value in its [Self::to_words] representation, on the given stack. @@ -921,6 +917,9 @@ mod tests { let mut stack = Vec::default(); true.push_to_operand_stack(&mut stack); + assert_eq!(stack.as_slice(), true.to_felts().as_slice()); + + stack.reverse(); let popped = ::pop_from_stack(&mut stack); assert!(popped); } @@ -941,6 +940,9 @@ mod tests { let mut stack = Vec::default(); u8::MAX.push_to_operand_stack(&mut stack); + assert_eq!(stack.as_slice(), u8::MAX.to_felts().as_slice()); + + stack.reverse(); let popped = ::pop_from_stack(&mut stack); assert_eq!(popped, u8::MAX); } @@ -961,6 +963,9 @@ mod tests { let mut stack = Vec::default(); u16::MAX.push_to_operand_stack(&mut stack); + assert_eq!(stack.as_slice(), u16::MAX.to_felts().as_slice()); + + stack.reverse(); let popped = ::pop_from_stack(&mut stack); assert_eq!(popped, u16::MAX); } @@ -981,6 +986,9 @@ mod tests { let mut stack = Vec::default(); u32::MAX.push_to_operand_stack(&mut stack); + assert_eq!(stack.as_slice(), u32::MAX.to_felts().as_slice()); + + stack.reverse(); let popped = ::pop_from_stack(&mut stack); assert_eq!(popped, u32::MAX); } @@ -1001,6 +1009,9 @@ mod tests { let mut stack = Vec::default(); u64::MAX.push_to_operand_stack(&mut stack); + assert_eq!(stack.as_slice(), u64::MAX.to_felts().as_slice()); + + stack.reverse(); let popped = ::pop_from_stack(&mut stack); assert_eq!(popped, u64::MAX); } @@ -1021,6 +1032,9 @@ mod tests { let mut stack = Vec::default(); u128::MAX.push_to_operand_stack(&mut stack); + assert_eq!(stack.as_slice(), u128::MAX.to_felts().as_slice()); + + stack.reverse(); let popped = ::pop_from_stack(&mut stack); assert_eq!(popped, u128::MAX); } @@ -1039,6 +1053,9 @@ mod tests { let mut stack = Vec::default(); bytes.push_to_operand_stack(&mut stack); + assert_eq!(stack.as_slice(), bytes.to_felts().as_slice()); + + stack.reverse(); let popped = <[u8; 8] as FromMidenRepr>::pop_from_stack(&mut stack); assert_eq!(popped, bytes); } diff --git a/src/lib.rs b/src/lib.rs index 79f23a2..486f83e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -4,6 +4,8 @@ mod exec; mod felt; mod input; mod linker; +#[cfg(test)] +mod test_utils; pub use self::{ debug::*, diff --git a/src/test_utils.rs b/src/test_utils.rs new file mode 100644 index 0000000..c27d60b --- /dev/null +++ b/src/test_utils.rs @@ -0,0 +1,61 @@ +//! Test-only utilities shared across unit tests. + +use miden_assembly_syntax::ast::types::Type; + +use crate::debug::FormatType; + +/// Formats a scalar value that has already been read from memory as little-endian bytes. +pub(crate) fn write_scalar_bytes( + output: &mut String, + ty: &'static str, + format: FormatType, + bytes: &[u8], +) -> Result<(), String> { + use core::fmt::Write; + + let ty = match ty { + "i1" => Type::I1, + "i8" => Type::I8, + "u8" => Type::U8, + "i16" => Type::I16, + "u16" => Type::U16, + "i32" => Type::I32, + "u32" => Type::U32, + "i64" => Type::I64, + "u64" => Type::U64, + other => return Err(format!("unsupported scalar test type '{other}'")), + }; + + macro_rules! write_with_format_type { + ($value:expr) => { + match format { + FormatType::Decimal => write!(output, "{}", $value).unwrap(), + FormatType::Hex => write!(output, "{:0x}", $value).unwrap(), + FormatType::Binary => write!(output, "{:0b}", $value).unwrap(), + } + }; + } + + match ty { + Type::I1 => match format { + FormatType::Decimal => write!(output, "{}", bytes[0] != 0).unwrap(), + FormatType::Hex => write!(output, "{:#0x}", (bytes[0] != 0) as u8).unwrap(), + FormatType::Binary => write!(output, "{:#0b}", (bytes[0] != 0) as u8).unwrap(), + }, + Type::I8 => write_with_format_type!(bytes[0] as i8), + Type::U8 => write_with_format_type!(bytes[0]), + Type::I16 => write_with_format_type!(i16::from_le_bytes([bytes[0], bytes[1]])), + Type::U16 => write_with_format_type!(u16::from_le_bytes([bytes[0], bytes[1]])), + Type::I32 => { + write_with_format_type!(i32::from_le_bytes([bytes[0], bytes[1], bytes[2], bytes[3]])) + } + Type::U32 => { + write_with_format_type!(u32::from_le_bytes([bytes[0], bytes[1], bytes[2], bytes[3]])) + } + Type::I64 => write_with_format_type!(i64::from_le_bytes(bytes[..8].try_into().unwrap())), + Type::U64 => write_with_format_type!(u64::from_le_bytes(bytes[..8].try_into().unwrap())), + _ => return Err(format!("support for reads of type '{ty}' are not implemented yet")), + } + + Ok(()) +} diff --git a/src/ui/state.rs b/src/ui/state.rs index b0f2d37..0340344 100644 --- a/src/ui/state.rs +++ b/src/ui/state.rs @@ -2,8 +2,10 @@ use std::sync::Arc; use miden_assembly::{DefaultSourceManager, SourceManager}; use miden_assembly_syntax::diagnostics::{IntoDiagnostic, Report}; -use miden_core::field::{PrimeCharacteristicRing, PrimeField64}; -use miden_core::serde::Deserializable; +use miden_core::{ + field::{PrimeCharacteristicRing, PrimeField64}, + serde::Deserializable, +}; use miden_processor::{Felt, StackInputs}; use crate::{ @@ -41,10 +43,11 @@ impl State { let source_manager = Arc::new(DefaultSourceManager::default()); let mut inputs = config.inputs.clone().unwrap_or_default(); if !config.args.is_empty() { - inputs.inputs = StackInputs::new(&config.args.iter().map(|n| n.0).collect::>()) - .into_diagnostic()?; + // CLI args model sequential pushes, but StackInputs expects the top element first. + let args = config.args.iter().rev().map(|felt| felt.0).collect::>(); + inputs.inputs = StackInputs::new(&args).into_diagnostic()?; } - let args = inputs.inputs.iter().copied().rev().collect::>(); + let args = inputs.inputs.iter().copied().collect::>(); let package = load_package(&config)?; // Load libraries from link_libraries and sysroot BEFORE resolving dependencies @@ -108,12 +111,11 @@ impl State { let mut inputs = self.config.inputs.clone().unwrap_or_default(); if !self.config.args.is_empty() { - inputs.inputs = StackInputs::new( - &self.config.args.iter().copied().map(|n| n.0).collect::>(), - ) - .into_diagnostic()?; + // CLI args model sequential pushes, but StackInputs expects the top element first. + let args = self.config.args.iter().rev().map(|felt| felt.0).collect::>(); + inputs.inputs = StackInputs::new(&args).into_diagnostic()?; } - let args = inputs.inputs.iter().copied().rev().collect::>(); + let args = inputs.inputs.iter().copied().collect::>(); // Load libraries from link_libraries and sysroot BEFORE resolving dependencies let mut libs = Vec::with_capacity(self.config.link_libraries.len()); @@ -274,25 +276,23 @@ impl State { Type::I8 => write_with_format_type!(output, expr, bytes[0] as i8), Type::U8 => write_with_format_type!(output, expr, bytes[0]), Type::I16 => { - write_with_format_type!(output, expr, i16::from_be_bytes([bytes[0], bytes[1]])) + write_with_format_type!(output, expr, i16::from_le_bytes([bytes[0], bytes[1]])) } Type::U16 => { - write_with_format_type!(output, expr, u16::from_be_bytes([bytes[0], bytes[1]])) + write_with_format_type!(output, expr, u16::from_le_bytes([bytes[0], bytes[1]])) } Type::I32 => write_with_format_type!( output, expr, - i32::from_be_bytes([bytes[0], bytes[1], bytes[2], bytes[3]]) + i32::from_le_bytes([bytes[0], bytes[1], bytes[2], bytes[3]]) ), Type::U32 => write_with_format_type!( output, expr, - u32::from_be_bytes([bytes[0], bytes[1], bytes[2], bytes[3]]) + u32::from_le_bytes([bytes[0], bytes[1], bytes[2], bytes[3]]) ), ty @ (Type::I64 | Type::U64) => { - let hi = u32::from_be_bytes([bytes[0], bytes[1], bytes[2], bytes[3]]) as u64; - let lo = u32::from_be_bytes([bytes[4], bytes[5], bytes[6], bytes[7]]) as u64; - let val = (hi * 2u64.pow(32)) + lo; + let val = u64::from_le_bytes(bytes[..8].try_into().unwrap()); if matches!(ty, Type::I64) { write_with_format_type!(output, expr, val as i64) } else {