diff --git a/vm/src/hint_processor/builtin_hint_processor/segments.rs b/vm/src/hint_processor/builtin_hint_processor/segments.rs index c996c2f9b1..bc95c6c1ba 100644 --- a/vm/src/hint_processor/builtin_hint_processor/segments.rs +++ b/vm/src/hint_processor/builtin_hint_processor/segments.rs @@ -19,7 +19,10 @@ pub fn relocate_segment( ap_tracking: &ApTracking, ) -> Result<(), HintError> { let src_ptr = get_ptr_from_var_name("src_ptr", vm, ids_data, ap_tracking)?; + #[cfg(not(feature = "extensive_hints"))] let dest_ptr = get_ptr_from_var_name("dest_ptr", vm, ids_data, ap_tracking)?; + #[cfg(feature = "extensive_hints")] + let dest_ptr = crate::hint_processor::builtin_hint_processor::hint_utils::get_maybe_relocatable_from_var_name("dest_ptr", vm, ids_data, ap_tracking)?; vm.add_relocation_rule(src_ptr, dest_ptr) .map_err(HintError::Memory)?; diff --git a/vm/src/hint_processor/cairo_1_hint_processor/dict_manager.rs b/vm/src/hint_processor/cairo_1_hint_processor/dict_manager.rs index a82a4fa2db..f3fe2fb9a0 100644 --- a/vm/src/hint_processor/cairo_1_hint_processor/dict_manager.rs +++ b/vm/src/hint_processor/cairo_1_hint_processor/dict_manager.rs @@ -126,7 +126,7 @@ impl DictManagerExecScope { if self.use_temporary_segments { let mut prev_end = vm.add_memory_segment(); for tracker in &self.trackers { - vm.add_relocation_rule(tracker.start, prev_end)?; + vm.add_relocation_rule(tracker.start, prev_end.into())?; prev_end += (tracker.end.unwrap_or_default() - tracker.start)?; prev_end += 1; } diff --git a/vm/src/vm/vm_core.rs b/vm/src/vm/vm_core.rs index bd257a416d..6069f3d09c 100644 --- a/vm/src/vm/vm_core.rs +++ b/vm/src/vm/vm_core.rs @@ -1000,6 +1000,10 @@ impl VirtualMachine { /// Add a new relocation rule. /// + /// When using feature "extensive_hints" the destination is allowed to be an Integer (via + /// MaybeRelocatable). Relocating memory to anything other than a `Relocatable` is generally + /// not useful, but it does make the implementation consistent with the pythonic version. + /// /// Will return an error if any of the following conditions are not met: /// - Source address's segment must be negative (temporary). /// - Source address's offset must be zero. @@ -1007,29 +1011,14 @@ impl VirtualMachine { pub fn add_relocation_rule( &mut self, src_ptr: Relocatable, + #[cfg(not(feature = "extensive_hints"))] dst_ptr: Relocatable, + #[cfg(feature = "extensive_hints")] + dst_ptr: MaybeRelocatable, ) -> Result<(), MemoryError> { self.segments.memory.add_relocation_rule(src_ptr, dst_ptr) } - /// Add a new relocation rule, allowing for dst to be a `MaybeRelocatable`. - /// - /// Relocating memory to anything other than a `Relocatable` is generally not useful, but it - /// does make the implementation consistent with the pythonic version. In most cases it is - /// advisable to use `add_relocation_rule()`, which only accepts a Relocatable. - /// - /// Will return an error if any of the following conditions are not met: - /// - Source address's segment must be negative (temporary). - /// - Source address's offset must be zero. - /// - There shouldn't already be relocation at the source segment. - pub fn add_relocation_rule_maybe_relocatable( - &mut self, - src_ptr: Relocatable, - dst: MaybeRelocatable, - ) -> Result<(), MemoryError> { - self.segments.memory.add_relocation_rule_maybe_relocatable(src_ptr, dst) - } - pub fn gen_arg(&mut self, arg: &dyn Any) -> Result { self.segments.gen_arg(arg) } diff --git a/vm/src/vm/vm_memory/memory.rs b/vm/src/vm/vm_memory/memory.rs index 9b5bd55ed2..1f98ee62f4 100644 --- a/vm/src/vm/vm_memory/memory.rs +++ b/vm/src/vm/vm_memory/memory.rs @@ -161,6 +161,9 @@ pub struct Memory { pub(crate) temp_data: Vec>, // relocation_rules's keys map to temp_data's indices and therefore begin at // zero; that is, segment_index = -1 maps to key 0, -2 to key 1... + #[cfg(not(feature = "extensive_hints"))] + pub(crate) relocation_rules: HashMap, + #[cfg(feature = "extensive_hints")] pub(crate) relocation_rules: HashMap, pub validated_addresses: AddressSet, validation_rules: Vec>, @@ -247,6 +250,21 @@ impl Memory { } // Version of Memory.relocate_value() that doesn't require a self reference + #[cfg(not(feature = "extensive_hints"))] + fn relocate_address( + addr: Relocatable, + relocation_rules: &HashMap, + ) -> Result { + if addr.segment_index < 0 { + // Adjust the segment index to begin at zero, as per the struct field's + // comment. + if let Some(x) = relocation_rules.get(&(-(addr.segment_index + 1) as usize)) { + return Ok((*x + addr.offset)?.into()); + } + } + Ok(addr.into()) + } + #[cfg(feature = "extensive_hints")] fn relocate_address( addr: Relocatable, relocation_rules: &HashMap, @@ -293,6 +311,7 @@ impl Memory { if let Some(base_addr) = self.relocation_rules.get(&index) { let data_segment = self.temp_data.remove(index); + #[cfg(feature = "extensive_hints")] let base_addr = match base_addr { MaybeRelocatable::RelocatableValue(addr) => addr, MaybeRelocatable::Int(_) => { @@ -323,29 +342,41 @@ impl Memory { } /// Add a new relocation rule. /// + /// When using feature "extensive_hints" the destination is allowed to be an Integer (via + /// MaybeRelocatable). Relocating memory to anything other than a `Relocatable` is generally + /// not useful, but it does make the implementation consistent with the pythonic version. + /// /// Will return an error if any of the following conditions are not met: /// - Source address's segment must be negative (temporary). /// - Source address's offset must be zero. /// - There shouldn't already be relocation at the source segment. + #[cfg(not(feature = "extensive_hints"))] pub(crate) fn add_relocation_rule( &mut self, src_ptr: Relocatable, dst_ptr: Relocatable, ) -> Result<(), MemoryError> { - self.add_relocation_rule_maybe_relocatable(src_ptr, MaybeRelocatable::RelocatableValue(dst_ptr)) - } + if src_ptr.segment_index >= 0 { + return Err(MemoryError::AddressNotInTemporarySegment( + src_ptr.segment_index, + )); + } + if src_ptr.offset != 0 { + return Err(MemoryError::NonZeroOffset(src_ptr.offset)); + } - /// Add a new relocation rule. - /// - /// Relocating memory to anything other than a `Relocatable` is generally not useful, but it - /// does make the implementation consistent with the pythonic version. In most cases it is - /// advisable to use `add_relocation_rule()`, which only accepts a Relocatable. - /// - /// Will return an error if any of the following conditions are not met: - /// - Source address's segment must be negative (temporary). - /// - Source address's offset must be zero. - /// - There shouldn't already be relocation at the source segment. - pub(crate) fn add_relocation_rule_maybe_relocatable( + // Adjust the segment index to begin at zero, as per the struct field's + // comment. + let segment_index = -(src_ptr.segment_index + 1) as usize; + if self.relocation_rules.contains_key(&segment_index) { + return Err(MemoryError::DuplicatedRelocation(src_ptr.segment_index)); + } + + self.relocation_rules.insert(segment_index, dst_ptr); + Ok(()) + } + #[cfg(feature = "extensive_hints")] + pub(crate) fn add_relocation_rule( &mut self, src_ptr: Relocatable, dst: MaybeRelocatable, @@ -674,6 +705,23 @@ pub(crate) trait RelocateValue<'a, Input: 'a, Output: 'a> { fn relocate_value(&self, value: Input) -> Result; } +#[cfg(not(feature = "extensive_hints"))] +impl RelocateValue<'_, Relocatable, Relocatable> for Memory { + fn relocate_value(&self, addr: Relocatable) -> Result { + if addr.segment_index < 0 { + // Adjust the segment index to begin at zero, as per the struct field's + // comment. + if let Some(x) = self + .relocation_rules + .get(&(-(addr.segment_index + 1) as usize)) + { + return (*x + addr.offset).map_err(MemoryError::Math); + } + } + Ok(addr) + } +} +#[cfg(feature = "extensive_hints")] impl RelocateValue<'_, Relocatable, MaybeRelocatable> for Memory { fn relocate_value(&self, addr: Relocatable) -> Result { if addr.segment_index < 0 {