From 8d043b2e1e88cd7524a7e6cf1c482e788d1fd306 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Mon, 4 Dec 2023 23:22:40 +0000 Subject: [PATCH 1/2] Use scratch allocations when fetching memory --- optimism/src/mips/interpreter.rs | 46 ++++++++++++++++++++++++++++---- optimism/src/mips/witness.rs | 10 +++++-- 2 files changed, 49 insertions(+), 7 deletions(-) diff --git a/optimism/src/mips/interpreter.rs b/optimism/src/mips/interpreter.rs index 56ebb2bf1d..287ab080cf 100644 --- a/optimism/src/mips/interpreter.rs +++ b/optimism/src/mips/interpreter.rs @@ -207,7 +207,14 @@ pub trait InterpreterEnv { fn fetch_register_checked(&self, register_idx: &Self::Variable) -> Self::Variable; - fn fetch_memory(&mut self, addr: &Self::Variable) -> Self::Variable; + /// Fetch the memory value at address `addr` and store it in local position `output`. + /// + /// This is unsafe: no lookups or other constraints are added as part of this operation. + unsafe fn fetch_memory( + &mut self, + addr: &Self::Variable, + output: Self::Position, + ) -> Self::Variable; fn set_instruction_pointer(&mut self, ip: Self::Variable); @@ -367,10 +374,39 @@ pub fn interpret_itype(env: &mut Env, instr: ITypeInstructi addr.clone() ); // We load 4 bytes, i.e. one word. - let v0 = env.fetch_memory(&addr_with_offset); - let v1 = env.fetch_memory(&(addr_with_offset.clone() + Env::constant(1))); - let v2 = env.fetch_memory(&(addr_with_offset.clone() + Env::constant(2))); - let v3 = env.fetch_memory(&(addr_with_offset.clone() + Env::constant(3))); + let v0 = unsafe { + // FIXME: A safe wrapper should be exposed in the trait, and it must add the + // constraints that are missing here. + let output_location = env.alloc_scratch(); + env.fetch_memory(&addr_with_offset, output_location) + }; + let v1 = unsafe { + // FIXME: A safe wrapper should be exposed in the trait, and it must add the + // constraints that are missing here. + let output_location = env.alloc_scratch(); + env.fetch_memory( + &(addr_with_offset.clone() + Env::constant(1)), + output_location, + ) + }; + let v2 = unsafe { + // FIXME: A safe wrapper should be exposed in the trait, and it must add the + // constraints that are missing here. + let output_location = env.alloc_scratch(); + env.fetch_memory( + &(addr_with_offset.clone() + Env::constant(2)), + output_location, + ) + }; + let v3 = unsafe { + // FIXME: A safe wrapper should be exposed in the trait, and it must add the + // constraints that are missing here. + let output_location = env.alloc_scratch(); + env.fetch_memory( + &(addr_with_offset.clone() + Env::constant(3)), + output_location, + ) + }; let value = (v0 * Env::constant(1 << 24)) + (v1 * Env::constant(1 << 16)) + (v2 * Env::constant(1 << 8)) diff --git a/optimism/src/mips/witness.rs b/optimism/src/mips/witness.rs index f25504c8f9..3234a94e80 100644 --- a/optimism/src/mips/witness.rs +++ b/optimism/src/mips/witness.rs @@ -133,12 +133,18 @@ impl InterpreterEnv for Env { self.instruction_parts[part] } - fn fetch_memory(&mut self, addr: &Self::Variable) -> Self::Variable { + unsafe fn fetch_memory( + &mut self, + addr: &Self::Variable, + output: Self::Position, + ) -> Self::Variable { let page = addr >> PAGE_ADDRESS_SIZE; let page_address = (addr & PAGE_ADDRESS_MASK) as usize; for (page_index, memory) in self.memory.iter() { if *page_index == page { - return memory[page_address].into(); + let value = memory[page_address]; + self.write_column(output, value.into()); + return value.into(); } } panic!("Could not access address") From ed4d469ea4277b3d8e26fd47c8b097c683271ede Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Tue, 5 Dec 2023 02:23:06 +0000 Subject: [PATCH 2/2] `#safety` --- optimism/src/mips/interpreter.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/optimism/src/mips/interpreter.rs b/optimism/src/mips/interpreter.rs index 287ab080cf..b768d3a692 100644 --- a/optimism/src/mips/interpreter.rs +++ b/optimism/src/mips/interpreter.rs @@ -209,7 +209,10 @@ pub trait InterpreterEnv { /// Fetch the memory value at address `addr` and store it in local position `output`. /// - /// This is unsafe: no lookups or other constraints are added as part of this operation. + /// # Safety + /// + /// No lookups or other constraints are added as part of this operation. The caller must + /// manually add the lookups for this memory operation. unsafe fn fetch_memory( &mut self, addr: &Self::Variable,