diff --git a/assembly/src/assembler/instruction/mem_ops.rs b/assembly/src/assembler/instruction/mem_ops.rs index c3eaa3633..90c610c89 100644 --- a/assembly/src/assembler/instruction/mem_ops.rs +++ b/assembly/src/assembler/instruction/mem_ops.rs @@ -1,9 +1,8 @@ use alloc::string::ToString; - use vm_core::{Felt, Operation::*}; -use super::{push_felt, push_u32_value, validate_param, BasicBlockBuilder}; -use crate::{assembler::ProcedureContext, diagnostics::Report, AssemblyError}; +use super::{push_felt, push_u32_value, BasicBlockBuilder}; +use crate::{assembler::ProcedureContext, diagnostics::Report, Spanned, AssemblyError}; // INSTRUCTION PARSERS // ================================================================================================ @@ -32,8 +31,7 @@ pub fn mem_read( // if the address was provided as an immediate value, put it onto the stack if let Some(addr) = addr { if is_local { - let num_locals = proc_ctx.num_locals(); - local_to_absolute_addr(block_builder, addr as u16, num_locals, is_single)?; + local_to_absolute_addr(block_builder, addr as u16, proc_ctx)?; } else { push_u32_value(block_builder, addr); } @@ -81,7 +79,7 @@ pub fn mem_write_imm( is_single: bool, ) -> Result<(), AssemblyError> { if is_local { - local_to_absolute_addr(block_builder, addr as u16, proc_ctx.num_locals(), is_single)?; + local_to_absolute_addr(block_builder, addr as u16, proc_ctx)?; } else { push_u32_value(block_builder, addr); } @@ -111,10 +109,10 @@ pub fn mem_write_imm( /// Returns an error if index is greater than the number of procedure locals. pub fn local_to_absolute_addr( block_builder: &mut BasicBlockBuilder, - index_of_local: u16, - num_proc_locals: u16, - is_single: bool, + index: u16, + proc_ctx: &ProcedureContext, ) -> Result<(), AssemblyError> { + let num_proc_locals = proc_ctx.num_locals(); if num_proc_locals == 0 { return Err(AssemblyError::Other( Report::msg( @@ -125,21 +123,17 @@ pub fn local_to_absolute_addr( )); } - // If a single local value is being accessed, then the index can take the full range - // [0, num_proc_locals - 1]. Otherwise, the index can take the range [0, num_proc_locals - 4] - // to account for the fact that a full word is being accessed. - let max = if is_single { - num_proc_locals - 1 - } else { - num_proc_locals - 4 - }; - validate_param(index_of_local, 0..=max)?; + let max_index = num_proc_locals - 1; + if index > max_index { + let span = proc_ctx.span(); + return Err(AssemblyError::InvalidLocalWordIndex { + span, + source_file: proc_ctx.source_manager().get(span.source_id()).ok(), + local_addr: index, + }); + } - // Local values are placed under the frame pointer, so we need to calculate the offset of the - // local value from the frame pointer. - // The offset is in the range [1, num_proc_locals], which is then subtracted from `fmp`. - let fmp_offset_of_local = num_proc_locals - index_of_local; - push_felt(block_builder, -Felt::from(fmp_offset_of_local)); + push_felt(block_builder, -Felt::from(max_index - index)); block_builder.push_op(FmpAdd); Ok(()) diff --git a/assembly/src/errors.rs b/assembly/src/errors.rs index 06ff9ab3b..aa36bd334 100644 --- a/assembly/src/errors.rs +++ b/assembly/src/errors.rs @@ -88,6 +88,17 @@ pub enum AssemblyError { #[source_code] source_file: Option>, }, + #[error("invalid local memory index: {index} is out of bounds")] + #[diagnostic(help("procedure has {num_locals} locals available, so the valid range for the index is 0..{max_index}"))] + InvalidLocalMemoryIndex { + #[label("invalid index used here")] + span: SourceSpan, + #[source_code] + source_file: Option>, + index: u16, + num_locals: u16, + max_index: u16, + }, #[error(transparent)] #[diagnostic(transparent)] Other(RelatedError),