From 824e2822e28b21854b35bb5f9d2049d5141a49cf Mon Sep 17 00:00:00 2001 From: Andrey Khmuro Date: Fri, 18 Aug 2023 21:44:05 +0200 Subject: [PATCH] refactor(processor): impose limit on the memory address --- CHANGELOG.md | 1 + processor/src/chiplets/memory/mod.rs | 35 ++++++++------- processor/src/chiplets/memory/segment.rs | 20 ++++----- processor/src/chiplets/memory/tests.rs | 57 ++++++++++++------------ processor/src/chiplets/mod.rs | 18 ++++---- processor/src/operations/io_ops.rs | 56 +++++++++++++++++------ 6 files changed, 110 insertions(+), 77 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3467a431ee..82fa6dca3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ - Improved handling of invalid/incomplete parameters in `StackOutputs` constructors (#1010). - Allowed the assembler to produce programs with "phantom" calls (#1019). - Added `TraceLenSummary` struct which holds information about traces lengths to the `ExecutionTrace` (#1029). +- Imposed the 2^32 limit for the memory addresses used in the memory chiplet (#1049). ## 0.6.1 (2023-06-29) diff --git a/processor/src/chiplets/memory/mod.rs b/processor/src/chiplets/memory/mod.rs index 2b87904fda..f126425d52 100644 --- a/processor/src/chiplets/memory/mod.rs +++ b/processor/src/chiplets/memory/mod.rs @@ -96,7 +96,7 @@ impl Memory { /// /// Unlike read() which modifies the memory access trace, this method returns the value at the /// specified address (if one exists) without altering the memory access trace. - pub fn get_value(&self, ctx: u32, addr: u64) -> Option { + pub fn get_value(&self, ctx: u32, addr: u32) -> Option { match self.trace.get(&ctx) { Some(segment) => segment.get_value(addr), None => None, @@ -105,7 +105,7 @@ impl Memory { /// Returns the word at the specified context/address which should be used as the "old value" for a /// write request. It will be the previously stored value, if one exists, or initialized memory. - pub fn get_old_value(&self, ctx: u32, addr: u64) -> Word { + pub fn get_old_value(&self, ctx: u32, addr: u32) -> Word { // get the stored word or return [0, 0, 0, 0], since the memory is initialized with zeros self.get_value(ctx, addr).unwrap_or(INIT_MEM_VALUE) } @@ -131,13 +131,13 @@ impl Memory { /// /// If the specified address hasn't been previously written to, four ZERO elements are /// returned. This effectively implies that memory is initialized to ZERO. - pub fn read(&mut self, ctx: u32, addr: Felt, clk: u32) -> Word { + pub fn read(&mut self, ctx: u32, addr: u32, clk: u32) -> Word { self.num_trace_rows += 1; self.trace.entry(ctx).or_default().read(addr, Felt::from(clk)) } /// Writes the provided word at the specified context/address. - pub fn write(&mut self, ctx: u32, addr: Felt, clk: u32, value: Word) { + pub fn write(&mut self, ctx: u32, addr: u32, clk: u32, value: Word) { self.num_trace_rows += 1; self.trace.entry(ctx).or_default().write(addr, Felt::from(clk), value); } @@ -170,7 +170,7 @@ impl Memory { let delta = if prev_ctx != ctx { (ctx - prev_ctx) as u64 } else if prev_addr != addr { - addr - prev_addr + (addr - prev_addr) as u64 } else { clk - prev_clk - 1 }; @@ -214,7 +214,7 @@ impl Memory { for (addr, addr_trace) in segment.into_inner() { // when we start a new address, we set the previous value to all zeros. the effect of // this is that memory is always initialized to zero. - let addr = Felt::new(addr); + let felt_addr = Felt::from(addr); for memory_access in addr_trace { let clk = memory_access.clk(); let value = memory_access.value(); @@ -223,7 +223,7 @@ impl Memory { trace.set(row, 0, selectors[0]); trace.set(row, 1, selectors[1]); trace.set(row, CTX_COL_IDX, ctx); - trace.set(row, ADDR_COL_IDX, addr); + trace.set(row, ADDR_COL_IDX, felt_addr); trace.set(row, CLK_COL_IDX, clk); for (idx, col) in V_COL_RANGE.enumerate() { trace.set(row, col, value[idx]); @@ -232,8 +232,8 @@ impl Memory { // compute delta as difference between context IDs, addresses, or clock cycles let delta = if prev_ctx != ctx { ctx - prev_ctx - } else if prev_addr != addr { - addr - prev_addr + } else if prev_addr != felt_addr { + felt_addr - prev_addr } else { clk - prev_clk - ONE }; @@ -245,14 +245,19 @@ impl Memory { trace.set(row, D_INV_COL_IDX, delta.inv()); // provide the memory access data to the chiplets bus. - let memory_lookup = - MemoryLookup::new(memory_access.op_label(), ctx, addr, clk, value); + let memory_lookup = MemoryLookup::new( + memory_access.op_label(), + ctx, + Felt::from(addr), + clk, + value, + ); chiplets_bus .provide_memory_operation(memory_lookup, (memory_start_row + row) as u32); // update values for the next iteration of the loop prev_ctx = ctx; - prev_addr = addr; + prev_addr = felt_addr; prev_clk = clk; row += 1; } @@ -265,7 +270,7 @@ impl Memory { /// Returns the context, address, and clock cycle of the first trace row, or None if the trace /// is empty. - fn get_first_row_info(&self) -> Option<(u32, u64, Felt)> { + fn get_first_row_info(&self) -> Option<(u32, u32, Felt)> { let (ctx, segment) = match self.trace.iter().next() { Some((&ctx, segment)) => (ctx, segment), None => return None, @@ -311,11 +316,11 @@ impl MemoryLookup { } } - pub fn from_ints(label: u8, ctx: u32, addr: Felt, clk: u32, word: Word) -> Self { + pub fn from_ints(label: u8, ctx: u32, addr: u32, clk: u32, word: Word) -> Self { Self { label, ctx: Felt::from(ctx), - addr, + addr: Felt::from(addr), clk: Felt::from(clk), word, } diff --git a/processor/src/chiplets/memory/segment.rs b/processor/src/chiplets/memory/segment.rs index 651db1dd9a..15ceb760ec 100644 --- a/processor/src/chiplets/memory/segment.rs +++ b/processor/src/chiplets/memory/segment.rs @@ -14,7 +14,7 @@ use super::{BTreeMap, Felt, StarkField, Vec, Word, INIT_MEM_VALUE}; /// Within each segment, the memory is word-addressable. That is, four field elements are located /// at each memory address, and we can read and write elements to/from memory in batches of four. #[derive(Default)] -pub struct MemorySegmentTrace(BTreeMap>); +pub struct MemorySegmentTrace(BTreeMap>); impl MemorySegmentTrace { // PUBLIC ACCESSORS @@ -25,7 +25,7 @@ impl MemorySegmentTrace { /// /// Unlike read() which modifies the memory access trace, this method returns the value at the /// specified address (if one exists) without altering the memory access trace. - pub fn get_value(&self, addr: u64) -> Option { + pub fn get_value(&self, addr: u32) -> Option { match self.0.get(&addr) { Some(addr_trace) => addr_trace.last().map(|access| access.value()), None => None, @@ -47,13 +47,13 @@ impl MemorySegmentTrace { for (&addr, addr_trace) in self.0.iter() { match addr_trace.binary_search_by(|access| access.clk().as_int().cmp(&search_clk)) { - Ok(i) => result.push((addr, addr_trace[i].value())), + Ok(i) => result.push((addr.into(), addr_trace[i].value())), Err(i) => { // Binary search finds the index of the data with the specified clock cycle. // Decrement the index to get the trace from the previously accessed clock // cycle to insert into the results. if i > 0 { - result.push((addr, addr_trace[i - 1].value())); + result.push((addr.into(), addr_trace[i - 1].value())); } } } @@ -70,12 +70,12 @@ impl MemorySegmentTrace { /// /// If the specified address hasn't been previously written to, four ZERO elements are /// returned. This effectively implies that memory is initialized to ZERO. - pub fn read(&mut self, addr: Felt, clk: Felt) -> Word { + pub fn read(&mut self, addr: u32, clk: Felt) -> Word { // look up the previous value in the appropriate address trace and add (clk, prev_value) // to it; if this is the first time we access this address, create address trace for it // with entry (clk, [ZERO, 4]). in both cases, return the last value in the address trace. self.0 - .entry(addr.as_int()) + .entry(addr) .and_modify(|addr_trace| { let last_value = addr_trace.last().expect("empty address trace").value(); let access = MemorySegmentAccess::new(clk, MemoryOperation::CopyRead, last_value); @@ -93,12 +93,12 @@ impl MemorySegmentTrace { /// Writes the provided word at the specified address. The memory access is assumed to happen /// at the provided clock cycle. - pub fn write(&mut self, addr: Felt, clk: Felt, value: Word) { + pub fn write(&mut self, addr: u32, clk: Felt, value: Word) { // add a memory access to the appropriate address trace; if this is the first time // we access this address, initialize address trace. let access = MemorySegmentAccess::new(clk, MemoryOperation::Write, value); self.0 - .entry(addr.as_int()) + .entry(addr) .and_modify(|addr_trace| addr_trace.push(access)) .or_insert_with(|| vec![access]); } @@ -107,12 +107,12 @@ impl MemorySegmentTrace { // -------------------------------------------------------------------------------------------- /// Returns a reference to the map underlying this memory segment trace. - pub(super) fn inner(&self) -> &BTreeMap> { + pub(super) fn inner(&self) -> &BTreeMap> { &self.0 } /// Returns a map underlying this memory segment trace while consuming self. - pub(super) fn into_inner(self) -> BTreeMap> { + pub(super) fn into_inner(self) -> BTreeMap> { self.0 } diff --git a/processor/src/chiplets/memory/tests.rs b/processor/src/chiplets/memory/tests.rs index 808cd18019..9bf890213e 100644 --- a/processor/src/chiplets/memory/tests.rs +++ b/processor/src/chiplets/memory/tests.rs @@ -1,8 +1,7 @@ use super::{ super::aux_trace::{ChipletLookup, ChipletsBusRow}, - ChipletsBus, Felt, FieldElement, Memory, MemoryLookup, StarkField, TraceFragment, Vec, - ADDR_COL_IDX, CLK_COL_IDX, CTX_COL_IDX, D0_COL_IDX, D1_COL_IDX, D_INV_COL_IDX, ONE, - V_COL_RANGE, ZERO, + ChipletsBus, Felt, FieldElement, Memory, MemoryLookup, TraceFragment, Vec, ADDR_COL_IDX, + CLK_COL_IDX, CTX_COL_IDX, D0_COL_IDX, D1_COL_IDX, D_INV_COL_IDX, ONE, V_COL_RANGE, ZERO, }; use miden_air::trace::chiplets::memory::{ Selectors, MEMORY_COPY_READ, MEMORY_INIT_READ, MEMORY_READ_LABEL, MEMORY_WRITE, @@ -21,14 +20,14 @@ fn mem_read() { let mut mem = Memory::default(); // read a value from address 0; clk = 1 - let addr0 = Felt::new(0); + let addr0 = 0; let value = mem.read(0, addr0, 1); assert_eq!([ZERO; 4], value); assert_eq!(1, mem.size()); assert_eq!(1, mem.trace_len()); // read a value from address 3; clk = 2 - let addr3 = Felt::new(3); + let addr3 = 3; let value = mem.read(0, addr3, 2); assert_eq!([ZERO; 4], value); assert_eq!(2, mem.size()); @@ -41,7 +40,7 @@ fn mem_read() { assert_eq!(3, mem.trace_len()); // read a value from address 2; clk = 4 - let addr2 = Felt::new(2); + let addr2 = 2; let value = mem.read(0, addr2, 4); assert_eq!([ZERO; 4], value); assert_eq!(3, mem.size()); @@ -76,33 +75,33 @@ fn mem_write() { let mut mem = Memory::default(); // write a value into address 0; clk = 1 - let addr0 = Felt::new(0); + let addr0 = 0; let value1 = [ONE, ZERO, ZERO, ZERO]; mem.write(0, addr0, 1, value1); - assert_eq!(value1, mem.get_value(0, addr0.as_int()).unwrap()); + assert_eq!(value1, mem.get_value(0, addr0).unwrap()); assert_eq!(1, mem.size()); assert_eq!(1, mem.trace_len()); // write a value into address 2; clk = 2 - let addr2 = Felt::new(2); + let addr2 = 2; let value5 = [Felt::new(5), ZERO, ZERO, ZERO]; mem.write(0, addr2, 2, value5); - assert_eq!(value5, mem.get_value(0, addr2.as_int()).unwrap()); + assert_eq!(value5, mem.get_value(0, addr2).unwrap()); assert_eq!(2, mem.size()); assert_eq!(2, mem.trace_len()); // write a value into address 1; clk = 3 - let addr1 = Felt::new(1); + let addr1 = 1; let value7 = [Felt::new(7), ZERO, ZERO, ZERO]; mem.write(0, addr1, 3, value7); - assert_eq!(value7, mem.get_value(0, addr1.as_int()).unwrap()); + assert_eq!(value7, mem.get_value(0, addr1).unwrap()); assert_eq!(3, mem.size()); assert_eq!(3, mem.trace_len()); // write a value into address 0; clk = 4 let value9 = [Felt::new(9), ZERO, ZERO, ZERO]; mem.write(0, addr0, 4, value9); - assert_eq!(value7, mem.get_value(0, addr1.as_int()).unwrap()); + assert_eq!(value7, mem.get_value(0, addr1).unwrap()); assert_eq!(3, mem.size()); assert_eq!(4, mem.trace_len()); @@ -135,12 +134,12 @@ fn mem_write_read() { let mut mem = Memory::default(); // write 1 into address 5; clk = 1 - let addr5 = Felt::new(5); + let addr5 = 5; let value1 = [ONE, ZERO, ZERO, ZERO]; mem.write(0, addr5, 1, value1); // write 4 into address 2; clk = 2 - let addr2 = Felt::new(2); + let addr2 = 2; let value4 = [Felt::new(4), ZERO, ZERO, ZERO]; mem.write(0, addr2, 2, value4); @@ -216,33 +215,33 @@ fn mem_multi_context() { // write a value into ctx = 0, addr = 0; clk = 1 let value1 = [ONE, ZERO, ZERO, ZERO]; - mem.write(0, ZERO, 1, value1); + mem.write(0, 0, 1, value1); assert_eq!(value1, mem.get_value(0, 0).unwrap()); assert_eq!(1, mem.size()); assert_eq!(1, mem.trace_len()); // write a value into ctx = 3, addr = 1; clk = 4 let value2 = [ZERO, ONE, ZERO, ZERO]; - mem.write(3, ONE, 4, value2); + mem.write(3, 1, 4, value2); assert_eq!(value2, mem.get_value(3, 1).unwrap()); assert_eq!(2, mem.size()); assert_eq!(2, mem.trace_len()); // read a value from ctx = 3, addr = 1; clk = 6 - let value = mem.read(3, ONE, 6); + let value = mem.read(3, 1, 6); assert_eq!(value2, value); assert_eq!(2, mem.size()); assert_eq!(3, mem.trace_len()); // write a value into ctx = 3, addr = 0; clk = 7 let value3 = [ZERO, ZERO, ONE, ZERO]; - mem.write(3, ZERO, 7, value3); + mem.write(3, 0, 7, value3); assert_eq!(value3, mem.get_value(3, 0).unwrap()); assert_eq!(3, mem.size()); assert_eq!(4, mem.trace_len()); // read a value from ctx = 0, addr = 0; clk = 9 - let value = mem.read(0, ZERO, 9); + let value = mem.read(0, 0, 9); assert_eq!(value1, value); assert_eq!(3, mem.size()); assert_eq!(5, mem.trace_len()); @@ -253,25 +252,25 @@ fn mem_multi_context() { // ctx = 0, addr = 0 let mut prev_row = [ZERO; MEMORY_TRACE_WIDTH]; - let memory_access = MemoryLookup::from_ints(MEMORY_WRITE_LABEL, 0, ZERO, 1, value1); + let memory_access = MemoryLookup::from_ints(MEMORY_WRITE_LABEL, 0, 0, 1, value1); prev_row = verify_memory_access(&trace, &chiplets_bus, 0, MEMORY_WRITE, &memory_access, prev_row); - let memory_access = MemoryLookup::from_ints(MEMORY_READ_LABEL, 0, ZERO, 9, value1); + let memory_access = MemoryLookup::from_ints(MEMORY_READ_LABEL, 0, 0, 9, value1); prev_row = verify_memory_access(&trace, &chiplets_bus, 1, MEMORY_COPY_READ, &memory_access, prev_row); // ctx = 3, addr = 0 - let memory_access = MemoryLookup::from_ints(MEMORY_WRITE_LABEL, 3, ZERO, 7, value3); + let memory_access = MemoryLookup::from_ints(MEMORY_WRITE_LABEL, 3, 0, 7, value3); prev_row = verify_memory_access(&trace, &chiplets_bus, 2, MEMORY_WRITE, &memory_access, prev_row); // ctx = 3, addr = 1 - let memory_access = MemoryLookup::from_ints(MEMORY_WRITE_LABEL, 3, ONE, 4, value2); + let memory_access = MemoryLookup::from_ints(MEMORY_WRITE_LABEL, 3, 1, 4, value2); prev_row = verify_memory_access(&trace, &chiplets_bus, 3, MEMORY_WRITE, &memory_access, prev_row); - let memory_access = MemoryLookup::from_ints(MEMORY_READ_LABEL, 3, ONE, 6, value2); + let memory_access = MemoryLookup::from_ints(MEMORY_READ_LABEL, 3, 1, 6, value2); verify_memory_access(&trace, &chiplets_bus, 4, MEMORY_COPY_READ, &memory_access, prev_row); } @@ -282,17 +281,17 @@ fn mem_get_state_at() { // Write 1 into (ctx = 0, addr = 5) at clk = 1. // This means that mem[5] = 1 at the beginning of clk = 2 let value1 = [ONE, ZERO, ZERO, ZERO]; - mem.write(0, Felt::new(5), 1, value1); + mem.write(0, 5, 1, value1); // Write 4 into (ctx = 0, addr = 2) at clk = 2. // This means that mem[2] = 4 at the beginning of clk = 3 let value4 = [Felt::new(4), ZERO, ZERO, ZERO]; - mem.write(0, Felt::new(2), 2, value4); + mem.write(0, 2, 2, value4); // write 7 into (ctx = 3, addr = 3) at clk = 4 // This means that mem[3] = 7 at the beginning of clk = 4 let value7 = [Felt::new(7), ZERO, ZERO, ZERO]; - mem.write(3, Felt::new(3), 4, value7); + mem.write(3, 3, 4, value7); // Check memory state at clk = 2 assert_eq!(mem.get_state_at(0, 2), vec![(5, value1)]); @@ -350,7 +349,7 @@ fn build_trace_row( row[0] = op_selectors[0]; row[1] = op_selectors[1]; row[CTX_COL_IDX] = ctx; - row[ADDR_COL_IDX] = addr; + row[ADDR_COL_IDX] = Felt::from(addr); row[CLK_COL_IDX] = clk; row[V_COL_RANGE.start] = new_val[0]; row[V_COL_RANGE.start + 1] = new_val[1]; diff --git a/processor/src/chiplets/mod.rs b/processor/src/chiplets/mod.rs index e81b04c329..df6231b728 100644 --- a/processor/src/chiplets/mod.rs +++ b/processor/src/chiplets/mod.rs @@ -348,7 +348,7 @@ impl Chiplets { /// /// If the specified address hasn't been previously written to, four ZERO elements are /// returned. This effectively implies that memory is initialized to ZERO. - pub fn read_mem(&mut self, ctx: u32, addr: Felt) -> Word { + pub fn read_mem(&mut self, ctx: u32, addr: u32) -> Word { // read the word from memory let value = self.memory.read(ctx, addr, self.clk); @@ -364,9 +364,9 @@ impl Chiplets { /// /// If either of the accessed addresses hasn't been previously written to, ZERO elements are /// returned. This effectively implies that memory is initialized to ZERO. - pub fn read_mem_double(&mut self, ctx: u32, addr: Felt) -> [Word; 2] { + pub fn read_mem_double(&mut self, ctx: u32, addr: u32) -> [Word; 2] { // read two words from memory: from addr and from addr + 1 - let addr2 = addr + ONE; + let addr2 = addr + 1; let words = [self.memory.read(ctx, addr, self.clk), self.memory.read(ctx, addr2, self.clk)]; // create lookups for both memory reads @@ -383,7 +383,7 @@ impl Chiplets { /// Writes the provided word at the specified context/address. /// /// This also modifies the memory access trace and sends a memory lookup request to the bus. - pub fn write_mem(&mut self, ctx: u32, addr: Felt, word: Word) { + pub fn write_mem(&mut self, ctx: u32, addr: u32, word: Word) { self.memory.write(ctx, addr, self.clk, word); // send the memory write request to the bus @@ -395,8 +395,8 @@ impl Chiplets { /// elements of the word previously stored at that address unchanged. /// /// This also modifies the memory access trace and sends a memory lookup request to the bus. - pub fn write_mem_element(&mut self, ctx: u32, addr: Felt, value: Felt) -> Word { - let old_word = self.memory.get_old_value(ctx, addr.as_int()); + pub fn write_mem_element(&mut self, ctx: u32, addr: u32, value: Felt) -> Word { + let old_word = self.memory.get_old_value(ctx, addr); let new_word = [value, old_word[1], old_word[2], old_word[3]]; self.memory.write(ctx, addr, self.clk, new_word); @@ -412,8 +412,8 @@ impl Chiplets { /// context, starting at the specified address. /// /// This also modifies the memory access trace and sends two memory lookup requests to the bus. - pub fn write_mem_double(&mut self, ctx: u32, addr: Felt, words: [Word; 2]) { - let addr2 = addr + ONE; + pub fn write_mem_double(&mut self, ctx: u32, addr: u32, words: [Word; 2]) { + let addr2 = addr + 1; // write two words to memory at addr and addr + 1 self.memory.write(ctx, addr, self.clk, words[0]); self.memory.write(ctx, addr2, self.clk, words[1]); @@ -434,7 +434,7 @@ impl Chiplets { /// Unlike mem_read() which modifies the memory access trace, this method returns the value at /// the specified address (if one exists) without altering the memory access trace. pub fn get_mem_value(&self, ctx: u32, addr: u32) -> Option { - self.memory.get_value(ctx, addr as u64) + self.memory.get_value(ctx, addr) } /// Returns the entire memory state for the specified execution context at the specified cycle. diff --git a/processor/src/operations/io_ops.rs b/processor/src/operations/io_ops.rs index b6db128569..c56ceb95c1 100644 --- a/processor/src/operations/io_ops.rs +++ b/processor/src/operations/io_ops.rs @@ -1,9 +1,6 @@ -use super::{AdviceProvider, ExecutionError, Felt, Operation, Process}; - -// CONSTANTS -// ================================================================================================ +use vm_core::StarkField; -const TWO: Felt = Felt::new(2); +use super::{AdviceProvider, ExecutionError, Felt, Operation, Process}; // INPUT / OUTPUT OPERATIONS // ================================================================================================ @@ -40,7 +37,7 @@ where pub(super) fn op_mloadw(&mut self) -> Result<(), ExecutionError> { // get the address from the stack and read the word from current memory context let ctx = self.system.ctx(); - let addr = self.stack.get(0); + let addr = Self::get_valid_address(self.stack.get(0))?; let word = self.chiplets.read_mem(ctx, addr); // reverse the order of the memory word & update the stack state @@ -67,7 +64,7 @@ where pub(super) fn op_mload(&mut self) -> Result<(), ExecutionError> { // get the address from the stack and read the word from memory let ctx = self.system.ctx(); - let addr = self.stack.get(0); + let addr = Self::get_valid_address(self.stack.get(0))?; let mut word = self.chiplets.read_mem(ctx, addr); // put the retrieved word into stack order word.reverse(); @@ -95,7 +92,7 @@ where pub(super) fn op_mstream(&mut self) -> Result<(), ExecutionError> { // get the address from position 12 on the stack let ctx = self.system.ctx(); - let addr = self.stack.get(12); + let addr = Self::get_valid_address(self.stack.get(12))?; // load two words from memory let words = self.chiplets.read_mem_double(ctx, addr); @@ -112,7 +109,7 @@ where } // increment the address by 2 - self.stack.set(12, addr + TWO); + self.stack.set(12, Felt::from(addr + 2)); // copy over the rest of the stack self.stack.copy_state(13); @@ -131,7 +128,7 @@ where pub(super) fn op_mstorew(&mut self) -> Result<(), ExecutionError> { // get the address from the stack and build the word to be saved from the stack values let ctx = self.system.ctx(); - let addr = self.stack.get(0); + let addr = Self::get_valid_address(self.stack.get(0))?; // build the word in memory order (reverse of stack order) let word = [self.stack.get(4), self.stack.get(3), self.stack.get(2), self.stack.get(1)]; @@ -164,7 +161,7 @@ where pub(super) fn op_mstore(&mut self) -> Result<(), ExecutionError> { // get the address and the value from the stack let ctx = self.system.ctx(); - let addr = self.stack.get(0); + let addr = Self::get_valid_address(self.stack.get(0))?; let value = self.stack.get(1); // write the value to the memory and get the previous word @@ -194,7 +191,7 @@ where pub(super) fn op_pipe(&mut self) -> Result<(), ExecutionError> { // get the address from position 12 on the stack let ctx = self.system.ctx(); - let addr = self.stack.get(12); + let addr = Self::get_valid_address(self.stack.get(12))?; // pop two words from the advice stack let words = self.advice_provider.pop_stack_dword()?; @@ -214,7 +211,7 @@ where } // increment the address by 2 - self.stack.set(12, addr + TWO); + self.stack.set(12, Felt::from(addr + 2)); // copy over the rest of the stack self.stack.copy_state(13); @@ -252,6 +249,21 @@ where Ok(()) } + + // HELPER FUNCTIONS + // -------------------------------------------------------------------------------------------- + + /// Checks that provided address is less than u32::MAX and returns it cast to u32. + /// + /// # Errors + /// Returns an error if the provided address is greater than u32::MAX. + fn get_valid_address(addr: Felt) -> Result { + let addr = addr.as_int(); + if addr > u32::MAX as u64 { + return Err(ExecutionError::MemoryAddressOutOfBounds(addr)); + } + Ok(addr as u32) + } } // TESTS @@ -322,7 +334,11 @@ mod tests { assert_eq!(1, process.chiplets.get_mem_size()); assert_eq!(word, process.chiplets.get_mem_value(0, 1).unwrap()); - // --- calling LOADW with a stack of minimum depth is ok ---------------- + // --- calling MLOADW with address greater than u32::MAX leads to an error ---------------- + process.execute_op(Operation::Push(Felt::from(u64::MAX / 2))).unwrap(); + assert!(process.execute_op(Operation::MLoadW).is_err()); + + // --- calling MLOADW with a stack of minimum depth is ok ---------------- let mut process = Process::new_dummy_with_decoder_helpers_and_empty_stack(); assert!(process.execute_op(Operation::MLoadW).is_ok()); } @@ -347,6 +363,10 @@ mod tests { assert_eq!(1, process.chiplets.get_mem_size()); assert_eq!(word, process.chiplets.get_mem_value(0, 2).unwrap()); + // --- calling MLOAD with address greater than u32::MAX leads to an error ----------------- + process.execute_op(Operation::Push(Felt::from(u64::MAX / 2))).unwrap(); + assert!(process.execute_op(Operation::MLoad).is_err()); + // --- calling MLOAD with a stack of minimum depth is ok ---------------- let mut process = Process::new_dummy_with_decoder_helpers_and_empty_stack(); assert!(process.execute_op(Operation::MLoad).is_ok()); @@ -428,6 +448,10 @@ mod tests { assert_eq!(word1, process.chiplets.get_mem_value(0, 0).unwrap()); assert_eq!(word2, process.chiplets.get_mem_value(0, 3).unwrap()); + // --- calling MSTOREW with address greater than u32::MAX leads to an error ---------------- + process.execute_op(Operation::Push(Felt::from(u64::MAX / 2))).unwrap(); + assert!(process.execute_op(Operation::MStoreW).is_err()); + // --- calling STOREW with a stack of minimum depth is ok ---------------- let mut process = Process::new_dummy_with_decoder_helpers_and_empty_stack(); assert!(process.execute_op(Operation::MStoreW).is_ok()); @@ -469,6 +493,10 @@ mod tests { assert_eq!(2, process.chiplets.get_mem_size()); assert_eq!(mem_2, process.chiplets.get_mem_value(0, 2).unwrap()); + // --- calling MSTORE with address greater than u32::MAX leads to an error ---------------- + process.execute_op(Operation::Push(Felt::from(u64::MAX / 2))).unwrap(); + assert!(process.execute_op(Operation::MStore).is_err()); + // --- calling MSTORE with a stack of minimum depth is ok ---------------- let mut process = Process::new_dummy_with_decoder_helpers_and_empty_stack(); assert!(process.execute_op(Operation::MStore).is_ok());