diff --git a/CHANGELOG.md b/CHANGELOG.md index 6715b67331..a22c67514c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ - [BREAKING] Removed `Process`, `VmStateIterator` and `miden_processor::execute_iter()` ([#2483](https://github.com/0xMiden/miden-vm/pull/2483)). - [BREAKING] Removed `miden debug`, `miden analyze` and `miden repl` ([#2483](https://github.com/0xMiden/miden-vm/pull/2483)). - [BREAKING] Change backend from winterfell to Plonky3 ([#2472](https://github.com/0xMiden/miden-vm/pull/2472)). +- Fix issue where calling `strip_decorators()` would panic on further decorator access ([#2524](https://github.com/0xMiden/miden-vm/pull/2524)). ## 0.20.1 (2025-12-14) diff --git a/core/src/mast/debuginfo/decorator_storage.rs b/core/src/mast/debuginfo/decorator_storage.rs index 681c541d61..119c152337 100644 --- a/core/src/mast/debuginfo/decorator_storage.rs +++ b/core/src/mast/debuginfo/decorator_storage.rs @@ -140,12 +140,38 @@ impl OpToDecoratorIds { /// - Last value in `node_indptr_for_op_idx` must be <= `op_indptr_for_dec_ids.len() - 1` /// - Both `op_indptr_for_dec_ids` and `node_indptr_for_op_idx` must be strictly monotonic (each /// successive value must be >= the previous one) - #[cfg(test)] - pub fn from_components( + pub(crate) fn from_components( decorator_ids: Vec, op_indptr_for_dec_ids: Vec, node_indptr_for_op_idx: IndexVec, ) -> Result { + // Completely empty structures are valid (no nodes, no decorators) + if decorator_ids.is_empty() + && op_indptr_for_dec_ids.is_empty() + && node_indptr_for_op_idx.is_empty() + { + return Ok(Self { + decorator_ids, + op_indptr_for_dec_ids, + node_indptr_for_op_idx, + }); + } + + // Structures with nodes but no decorators are also valid + // (empty decorator_ids and op_indptr, but non-empty node_indptr with all zeros) + if decorator_ids.is_empty() && op_indptr_for_dec_ids.is_empty() { + // Verify all node pointers are 0 (pointing to empty range) + if node_indptr_for_op_idx.iter().all(|&ptr| ptr == 0) { + return Ok(Self { + decorator_ids, + op_indptr_for_dec_ids, + node_indptr_for_op_idx, + }); + } else { + return Err(DecoratorIndexError::InternalStructure); + } + } + // Validate the structure if op_indptr_for_dec_ids.is_empty() { return Err(DecoratorIndexError::InternalStructure); @@ -729,9 +755,9 @@ mod tests { #[test] fn test_from_components_invalid_structure() { - // Test with empty operation pointers + // Completely empty structures are now valid (no nodes, no decorators) let result = OpToDecoratorIds::from_components(vec![], vec![], IndexVec::new()); - assert_eq!(result, Err(DecoratorIndexError::InternalStructure)); + assert!(result.is_ok(), "Empty structures should be valid"); // Test with operation pointer exceeding decorator indices let result = OpToDecoratorIds::from_components( diff --git a/core/src/mast/debuginfo/mod.rs b/core/src/mast/debuginfo/mod.rs index 7690904ddf..30e579c2f9 100644 --- a/core/src/mast/debuginfo/mod.rs +++ b/core/src/mast/debuginfo/mod.rs @@ -109,6 +109,28 @@ impl DebugInfo { } } + /// Creates an empty [DebugInfo] with valid CSR structures for N nodes. + /// + /// Used when stripping decorators while keeping CSR structure valid. + pub fn empty_for_nodes(num_nodes: usize) -> Self { + // For N nodes, CSR needs N+1 zero entries + let mut node_indptr_for_op_idx = IndexVec::new(); + for _ in 0..=num_nodes { + let _ = node_indptr_for_op_idx.push(0); + } + + let op_decorator_storage = + OpToDecoratorIds::from_components(Vec::new(), Vec::new(), node_indptr_for_op_idx) + .expect("Empty CSR structure should be valid"); + + Self { + decorators: IndexVec::new(), + op_decorator_storage, + node_decorator_storage: NodeToDecoratorIds::new(), + error_codes: BTreeMap::new(), + } + } + // PUBLIC ACCESSORS // -------------------------------------------------------------------------------------------- diff --git a/core/src/mast/mod.rs b/core/src/mast/mod.rs index 00894d7b46..699848ea5f 100644 --- a/core/src/mast/mod.rs +++ b/core/src/mast/mod.rs @@ -163,24 +163,11 @@ impl MastForest { id_remappings } - /// Removes all decorators from this MAST forest. + /// Removes all decorators and error codes while keeping CSR structure valid. /// - /// This method modifies the forest in-place, removing all decorator information - /// including operation-indexed decorators, before-enter decorators, after-exit - /// decorators, and error codes. - /// - /// # Examples - /// - /// ```rust - /// use miden_core::mast::MastForest; - /// - /// let mut forest = MastForest::new(); - /// // Add decorators and nodes to the forest - /// forest.strip_decorators(); // forest is now stripped - /// ``` + /// Useful for performance benchmarking without decorator overhead. pub fn strip_decorators(&mut self) { - // Clear all debug info (decorators and error codes) - self.debug_info.clear(); + self.debug_info = DebugInfo::empty_for_nodes(self.nodes.len()); } /// Compacts the forest by merging duplicate nodes. diff --git a/core/src/mast/tests.rs b/core/src/mast/tests.rs index 19a07ce395..77a6207592 100644 --- a/core/src/mast/tests.rs +++ b/core/src/mast/tests.rs @@ -241,40 +241,62 @@ fn test_decorator_storage_consistency_with_multiple_blocks() { fn test_decorator_storage_after_strip_decorators() { let mut forest = MastForest::new(); - // Create decorators let deco1 = forest.add_decorator(Decorator::Trace(1)).unwrap(); let deco2 = forest.add_decorator(Decorator::Trace(2)).unwrap(); - - // Create operations and decorators let operations = vec![Operation::Push(Felt::new(1)), Operation::Add]; - let decorators = vec![(0, deco1), (1, deco2)]; - - // Add block to forest - let block_id = BasicBlockNodeBuilder::new(operations, decorators) + let block_id = BasicBlockNodeBuilder::new(operations, vec![(0, deco1), (1, deco2)]) .add_to_forest(&mut forest) .unwrap(); - // Verify decorators exist initially - assert!(!forest.debug_info.op_decorator_storage().is_empty()); - assert_eq!(forest.debug_info.op_decorator_storage().num_nodes(), 1); + assert_eq!(forest.debug_info.num_decorators(), 2); assert_eq!(forest.debug_info.op_decorator_storage().num_decorator_ids(), 2); - // Strip decorators forest.strip_decorators(); - // Verify decorators are cleared from storage - assert!(forest.debug_info.op_decorator_storage().is_empty()); + assert_eq!(forest.debug_info.num_decorators(), 0); + assert_eq!(forest.debug_info.op_decorator_storage().num_nodes(), 1); + assert!(forest.decorator_links_for_node(block_id).unwrap().into_iter().next().is_none()); + assert!(forest.get_assembly_op(block_id, Some(0)).is_none()); +} + +#[test] +fn test_strip_decorators_edge_cases() { + // Empty forest + let mut forest = MastForest::new(); + forest.strip_decorators(); + assert_eq!(forest.debug_info.num_decorators(), 0); assert_eq!(forest.debug_info.op_decorator_storage().num_nodes(), 0); - assert_eq!(forest.debug_info.op_decorator_storage().num_decorator_ids(), 0); - // Verify block also has no decorators after stripping - let block = if let crate::mast::MastNode::Block(block) = &forest[block_id] { - block - } else { - panic!("Expected a block node"); - }; - let block_decorators: Vec<_> = block.indexed_decorator_iter(&forest).collect(); - assert_eq!(block_decorators, []); + // Idempotent: stripping twice should be safe + let operations = vec![Operation::Push(Felt::new(1)), Operation::Add]; + let block_id = BasicBlockNodeBuilder::new(operations, vec![]) + .add_to_forest(&mut forest) + .unwrap(); + forest.strip_decorators(); + forest.strip_decorators(); + assert_eq!(forest.debug_info.num_decorators(), 0); + assert_eq!(forest.debug_info.op_decorator_storage().num_nodes(), 1); + assert!(forest.decorator_links_for_node(block_id).unwrap().into_iter().next().is_none()); +} + +#[test] +fn test_strip_decorators_multiple_node_types() { + let mut forest = MastForest::new(); + let deco = forest.add_decorator(Decorator::Trace(1)).unwrap(); + let block_id = BasicBlockNodeBuilder::new( + vec![Operation::Push(Felt::new(1)), Operation::Add], + vec![(0, deco)], + ) + .add_to_forest(&mut forest) + .unwrap(); + + JoinNodeBuilder::new([block_id, block_id]).add_to_forest(&mut forest).unwrap(); + SplitNodeBuilder::new([block_id, block_id]).add_to_forest(&mut forest).unwrap(); + + forest.strip_decorators(); + + assert_eq!(forest.debug_info.op_decorator_storage().num_nodes(), 3); + assert!(forest.decorator_links_for_node(block_id).unwrap().into_iter().next().is_none()); } #[test]