diff --git a/CHANGELOG.md b/CHANGELOG.md index 826433283c..94359a76f9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ #### Bug Fixes +- Fixed host event and advice-mutation diagnostics to point to the triggering `emit.event(...)` instruction ([#3042](https://github.com/0xMiden/miden-vm/pull/3042)). - Fixed debug-only underflow in memory range-check trace generation when the first memory access is at `clk = 0` ([#2976](https://github.com/0xMiden/miden-vm/pull/2976)). - Replaced unsound `ptr::read` with safe unbox in panic recovery, removing UB from potential double-drop ([#2934](https://github.com/0xMiden/miden-vm/pull/2934)). - Reverted `InvokeKind::ProcRef` back to `InvokeKind::Exec` in `visit_mut_procref` and added an explanatory comment (#2893). diff --git a/processor/src/errors.rs b/processor/src/errors.rs index f280143b68..c97c563eb8 100644 --- a/processor/src/errors.rs +++ b/processor/src/errors.rs @@ -413,8 +413,9 @@ pub fn advice_error_with_context( mast_forest: &MastForest, node_id: MastNodeId, host: &impl BaseHost, + op_idx: Option, ) -> ExecutionError { - let (label, source_file) = get_label_and_source_file(None, mast_forest, node_id, host); + let (label, source_file) = get_label_and_source_file(op_idx, mast_forest, node_id, host); ExecutionError::AdviceError { label, source_file, err } } @@ -427,10 +428,11 @@ pub fn event_error_with_context( mast_forest: &MastForest, node_id: MastNodeId, host: &impl BaseHost, + op_idx: Option, event_id: EventId, event_name: Option, ) -> ExecutionError { - let (label, source_file) = get_label_and_source_file(None, mast_forest, node_id, host); + let (label, source_file) = get_label_and_source_file(op_idx, mast_forest, node_id, host); ExecutionError::EventError { label, source_file, @@ -559,7 +561,7 @@ impl MapExecErr for Result { ) -> Result { match self { Ok(v) => Ok(v), - Err(err) => Err(advice_error_with_context(err, mast_forest, node_id, host)), + Err(err) => Err(advice_error_with_context(err, mast_forest, node_id, host, None)), } } } diff --git a/processor/src/fast/basic_block/mod.rs b/processor/src/fast/basic_block/mod.rs index ea4467d9db..1e8c50f601 100644 --- a/processor/src/fast/basic_block/mod.rs +++ b/processor/src/fast/basic_block/mod.rs @@ -45,6 +45,7 @@ impl FastProcessor { current_forest: &MastForest, node_id: MastNodeId, host: &impl BaseHost, + op_idx: usize, event_id: EventId, mutations: Result, EventError>, ) -> ControlFlow { @@ -57,6 +58,7 @@ impl FastProcessor { current_forest, node_id, host, + Some(op_idx), event_id, event_name, ))); @@ -70,6 +72,7 @@ impl FastProcessor { current_forest, node_id, host, + Some(op_idx), ))), } } @@ -115,7 +118,7 @@ impl FastProcessor { let processor_state = self.state(); let mutations = host.on_event(&processor_state); - self.apply_host_event_mutations(current_forest, node_id, host, event_id, mutations) + self.apply_host_event_mutations(current_forest, node_id, host, op_idx, event_id, mutations) } #[inline(always)] @@ -134,6 +137,6 @@ impl FastProcessor { let processor_state = self.state(); let mutations = host.on_event(&processor_state).await; - self.apply_host_event_mutations(current_forest, node_id, host, event_id, mutations) + self.apply_host_event_mutations(current_forest, node_id, host, op_idx, event_id, mutations) } } diff --git a/processor/src/tests/mod.rs b/processor/src/tests/mod.rs index c6a0221f15..c76ac8ac5a 100644 --- a/processor/src/tests/mod.rs +++ b/processor/src/tests/mod.rs @@ -17,14 +17,38 @@ use miden_utils_testing::{ /// Tests in this file make sure that diagnostics presented to the user are as expected. use crate::{ - DefaultHost, FastProcessor, Kernel, ONE, Program, StackInputs, Word, ZERO, - advice::{AdviceInputs, AdviceMap}, + DefaultHost, FastProcessor, Kernel, ONE, ProcessorState, Program, StackInputs, Word, ZERO, + advice::{AdviceInputs, AdviceMap, AdviceMutation}, + event::{EventError, EventHandler, EventName}, operation::Operation, }; mod debug; mod debug_mode_decorator_tests; +#[derive(Debug, thiserror::Error)] +#[error("dummy host event failure")] +struct DummyHostEventError; + +struct AlwaysFailEventHandler; + +impl EventHandler for AlwaysFailEventHandler { + fn on_event(&self, _process: &ProcessorState) -> Result, EventError> { + Err(DummyHostEventError.into()) + } +} + +struct DuplicateMapMutationHandler; + +impl EventHandler for DuplicateMapMutationHandler { + fn on_event(&self, _process: &ProcessorState) -> Result, EventError> { + Ok(vec![AdviceMutation::extend_map(AdviceMap::from_iter([( + Word::default(), + vec![ONE], + )]))]) + } +} + // AdviceMap inlined in the script // ------------------------------------------------------------------------------------------------ @@ -140,6 +164,74 @@ fn test_diagnostic_advice_map_key_not_found_2() { ); } +// Host event diagnostics +// ------------------------------------------------------------------------------------------------ + +#[test] +fn test_diagnostic_host_event_error_uses_emit_location() { + let event = EventName::new("test::host_event_error"); + let source_manager = Arc::new(DefaultSourceManager::default()); + let source = format!( + " + begin + push.1 emit.event(\"{event}\") + end" + ); + let program = Assembler::new(source_manager.clone()).assemble_program(source).unwrap(); + let mut host = DefaultHost::default().with_source_manager(source_manager); + host.register_handler(event.clone(), Arc::new(AlwaysFailEventHandler)).unwrap(); + + let processor = FastProcessor::new(StackInputs::default()) + .with_advice(AdviceInputs::default()) + .with_debugging(true) + .with_tracing(true); + let err = processor.execute_sync(&program, &mut host).expect_err("expected error"); + assert_diagnostic_lines!( + err, + format!(" x error during processing of event '{event}' (ID: {})", event.to_event_id()), + " `-> dummy host event failure", + regex!(r#",-\[::\$exec:3:20\]"#), + " 2 | begin", + format!(" 3 | push.1 emit.event(\"{event}\")"), + " : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^", + " 4 | end", + " `----" + ); +} + +#[test] +fn test_diagnostic_host_event_advice_error_uses_emit_location() { + let event = EventName::new("test::host_event_advice_error"); + let source_manager = Arc::new(DefaultSourceManager::default()); + let source = format!( + " + begin + push.1 emit.event(\"{event}\") + end" + ); + let program = Assembler::new(source_manager.clone()).assemble_program(source).unwrap(); + let mut host = DefaultHost::default().with_source_manager(source_manager); + host.register_handler(event.clone(), Arc::new(DuplicateMapMutationHandler)) + .unwrap(); + + let processor = FastProcessor::new(StackInputs::default()) + .with_advice(AdviceInputs::default().with_map([(Word::default(), vec![ZERO])])) + .with_debugging(true) + .with_tracing(true); + let err = processor.execute_sync(&program, &mut host).expect_err("expected error"); + assert_diagnostic_lines!( + err, + " x value for key 0x0000000000000000000000000000000000000000000000000000000000000000 already present in the advice map", + regex!(r#",-\[::\$exec:3:20\]"#), + " 2 | begin", + format!(" 3 | push.1 emit.event(\"{event}\")"), + " : ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^", + " 4 | end", + " `----", + "help: previous values at key were '[0]'. Operation would have replaced them with '[1]'" + ); +} + // AdviceStackReadFailed // ------------------------------------------------------------------------------------------------