From d76a2a23fc789c988ca89eb812bb4907773e4d43 Mon Sep 17 00:00:00 2001 From: Lorenzo Feroleto Date: Fri, 25 Aug 2023 17:11:08 +0200 Subject: [PATCH] chore(shared_memory): tests, completed replaced old memory, some bug fixes --- crates/interpreter/src/instructions/host.rs | 1 - crates/interpreter/src/instructions/memory.rs | 4 +- crates/interpreter/src/interpreter.rs | 18 +- crates/primitives/src/shared_memory.rs | 290 +++++++++++++++--- crates/revm/src/evm_impl.rs | 3 +- crates/revm/src/inspector/customprinter.rs | 2 +- crates/revm/src/inspector/tracer_eip3155.rs | 2 +- 7 files changed, 257 insertions(+), 63 deletions(-) diff --git a/crates/interpreter/src/instructions/host.rs b/crates/interpreter/src/instructions/host.rs index 321c6e966e4..89fce027ab6 100644 --- a/crates/interpreter/src/instructions/host.rs +++ b/crates/interpreter/src/instructions/host.rs @@ -555,7 +555,6 @@ pub fn call_inner( let (reason, gas, return_data) = host.call(&mut call_input, &interpreter.shared_memory); interpreter.return_data_buffer = return_data; - let target_len = min(out_len, interpreter.return_data_buffer.len()); match reason { diff --git a/crates/interpreter/src/instructions/memory.rs b/crates/interpreter/src/instructions/memory.rs index 46a92a1639c..3791043f2d0 100644 --- a/crates/interpreter/src/instructions/memory.rs +++ b/crates/interpreter/src/instructions/memory.rs @@ -42,7 +42,7 @@ pub fn mstore8(interpreter: &mut Interpreter, _host: &mut dyn Host) { gas!(interpreter, gas::VERYLOW); pop!(interpreter, index, value); let index = as_usize_or_fail!(interpreter, index, InstructionResult::InvalidOperandOOG); - shared_memory_resize!(interpreter, index, 32); + shared_memory_resize!(interpreter, index, 1); let value = value.as_le_bytes()[0]; // Safety: we resized our memory two lines above. unsafe { @@ -57,7 +57,7 @@ pub fn msize(interpreter: &mut Interpreter, _host: &mut dyn Host) { gas!(interpreter, gas::BASE); push!( interpreter, - U256::from(interpreter.shared_memory.borrow().effective_len()) + U256::from(interpreter.shared_memory.borrow().len()) ); } diff --git a/crates/interpreter/src/interpreter.rs b/crates/interpreter/src/interpreter.rs index 9f86852501b..c023f2522f6 100644 --- a/crates/interpreter/src/interpreter.rs +++ b/crates/interpreter/src/interpreter.rs @@ -37,7 +37,7 @@ pub struct Interpreter { /// left gas. Memory gas can be found in Memory field. pub gas: Gas, /// Memory. - pub memory: Memory, + // pub memory: Memory, /// Stack. pub stack: Stack, /// After call returns, its return data is saved here. @@ -73,7 +73,7 @@ impl Interpreter { Self { instruction_pointer: contract.bytecode.as_ptr(), return_range: Range::default(), - memory: Memory::new(), + // memory: Memory::new(), stack: Stack::new(), return_data_buffer: Bytes::new(), contract, @@ -101,7 +101,7 @@ impl Interpreter { Self { instruction_pointer: contract.bytecode.as_ptr(), return_range: Range::default(), - memory: Memory::new(), + // memory: Memory::new(), stack: Stack::new(), return_data_buffer: Bytes::new(), contract, @@ -122,9 +122,9 @@ impl Interpreter { } /// Reference of interpreter memory. - pub fn memory(&self) -> &Memory { - &self.memory - } + // pub fn memory(&self) -> &Memory { + // &self.memory + // } /// Reference of interpreter stack. pub fn stack(&self) -> &Stack { @@ -182,13 +182,15 @@ impl Interpreter { /// Copy and get the return value of the interpreter, if any. pub fn return_value(&self) -> Bytes { // if start is usize max it means that our return len is zero and we need to return empty - if self.return_range.start == usize::MAX { + let bytes = if self.return_range.start == usize::MAX { Bytes::new() } else { Bytes::copy_from_slice(self.shared_memory.borrow().get_slice( self.return_range.start, self.return_range.end - self.return_range.start, )) - } + }; + self.shared_memory.borrow_mut().free_memory(); + bytes } } diff --git a/crates/primitives/src/shared_memory.rs b/crates/primitives/src/shared_memory.rs index d66af73790c..ea713982a1c 100644 --- a/crates/primitives/src/shared_memory.rs +++ b/crates/primitives/src/shared_memory.rs @@ -1,14 +1,12 @@ +use crate::alloc::vec; use crate::U256; -use core::{ - cmp::min, - ops::{BitAnd, Not}, -}; +use core::cmp::min; pub struct SharedMemory { - pub data: Vec, + data: Vec, pub limit: u64, /// Memory sizes checkpoint for each depth - pub msizes: Vec, + msizes: Vec, current_slice: *mut [u8], current_len: usize, } @@ -26,41 +24,60 @@ impl SharedMemory { let base_offset = self.msizes.last().unwrap_or(&0); let last_slice_offset = self.current_len; - self.msizes.push(base_offset + last_slice_offset); + let new_msize = base_offset + last_slice_offset; - let new_msize = self.msizes.last().unwrap(); + self.msizes.push(new_msize); - let range = if new_msize == &0 { - 0.. - } else { - new_msize + 1.. - }; + let range = new_msize..; self.current_slice = &mut self.data[range]; self.current_len = 0; } - pub fn new(gas_limit: u64, memory_limit: Option) -> Self { - let upper_bound = ((589_312 + 512 * gas_limit) as f64).sqrt() - 768_f64; - let limit = if let Some(mlimit) = memory_limit { - mlimit + pub fn free_memory(&mut self) { + if let Some(old_size) = self.msizes.pop() { + self.resize(old_size); + let last = *self.msizes.last().unwrap_or(&0); + self.current_slice = &mut self.data[last..]; + self.current_len = old_size - last; } else { - upper_bound as u64 - }; - // self.data = Vec::with_capacity(upper_bound as usize); - let mut data = vec![0; limit as usize]; - let msizes = vec![0_usize; 1024]; + panic!() + } + } + + pub fn new(_gas_limit: u64, _memory_limit: Option) -> Self { + // https://2π.com/22/eth-max-mem/ + // let mut upper_bound = + // 512 * 2_f32.sqrt() as isize * (gas_limit as f64 + 1151_f64).sqrt() as isize - 48 * 512; + + let mut data = vec![0; u32::MAX as usize]; + let msizes = Vec::with_capacity(1024); let current_slice: *mut [u8] = &mut data[..]; SharedMemory { data, - limit, + limit: u64::MAX, msizes, current_slice, current_len: 0, } } - /// Resize the memory. asume that we already checked if + /// Get the length of the current memory range. + pub fn len(&self) -> usize { + self.current_len + } + + /// Return true if current effective memory range is zero. + pub fn is_empty(&self) -> bool { + self.current_len == 0 + } + + /// Return the full memory. + pub fn data(&self) -> &[u8] { + self.get_current_slice() + } + + /// Resize the memory. assume that we already checked if /// we have enought gas to resize this vector and that we made new_size as multiply of 32 pub fn resize(&mut self, new_size: usize) { if new_size as u64 >= self.limit { @@ -69,10 +86,10 @@ impl SharedMemory { let range = if new_size > self.current_len { // extend with zeros - self.current_len + 1..=new_size + self.current_len..new_size } else { // truncate - new_size..=self.current_len + new_size..self.current_len }; self.get_current_slice_mut()[range] @@ -81,25 +98,6 @@ impl SharedMemory { self.current_len = new_size; } - pub fn effective_len(&self) -> usize { - self.current_len - } - - /// Get the length of the current memory range. - pub fn len(&self) -> usize { - self.current_len - } - - /// Return true if current effective memory range is zero. - pub fn is_empty(&self) -> bool { - self.current_len == 0 - } - - /// Return the full memory. - pub fn data(&self) -> &[u8] { - self.get_current_slice() - } - /// Get memory region at given offset. Dont check offset and size #[inline(always)] pub fn get_slice(&self, offset: usize, size: usize) -> &[u8] { @@ -163,8 +161,202 @@ impl SharedMemory { } } -#[inline] -pub(crate) fn next_multiple_of_32(x: usize) -> Option { - let r = x.bitand(31).not().wrapping_add(1).bitand(31); - x.checked_add(r) +// #[inline] +// pub(crate) fn next_multiple_of_32(x: usize) -> Option { +// let r = x.bitand(31).not().wrapping_add(1).bitand(31); +// x.checked_add(r) +// } + +#[cfg(test)] +mod tests { + use core::{cell::RefCell, u8}; + + use alloc::rc::Rc; + use ruint::aliases::U256; + + use super::SharedMemory; + + // #[test] + // fn new() { + // let gas_limit = 100_000; + // let upper_bound = (512 + // * 2_f32.sqrt() as isize + // * (gas_limit as f64 + 1151_f64).sqrt() as isize + // - 48 * 512) as usize; + // let mem = SharedMemory::new(gas_limit, None); + // + // assert_eq!(mem.data.len(), upper_bound); + // assert_eq!(mem.get_current_slice().len(), upper_bound); + // assert_eq!(mem.current_len, 0); + // } + + #[test] + fn use_new_memory_1() { + let mut mem = SharedMemory::new(100_000, None); + + mem.use_new_memory(); + assert_eq!(mem.len(), 0); + assert_eq!(mem.msizes, vec![0]); + assert_eq!(mem.len(), 0); + assert_eq!(mem.get_current_slice().len(), mem.data.len()); + } + + #[test] + fn set() { + let mut mem = SharedMemory::new(100_000, None); + mem.use_new_memory(); + + mem.set(32, &U256::MAX.to_le_bytes::<32>()); + assert_eq!( + mem.get_current_slice()[32..64].to_vec(), + U256::MAX.to_le_bytes::<32>().to_vec() + ); + } + + #[test] + fn set_data() { + let mut mem = SharedMemory::new(100_000, None); + mem.use_new_memory(); + + mem.set_data(32, 0, 8, &U256::MAX.to_le_bytes::<32>()); + assert_eq!( + mem.get_current_slice()[32..40].to_vec(), + [u8::MAX; 8].to_vec() + ); + } + + #[test] + fn set_byte() { + let mut mem = SharedMemory::new(100_000, None); + mem.use_new_memory(); + unsafe { + mem.set_byte(2, 8); + mem.set_byte(1, 7); + mem.set_byte(0, 6); + }; + assert_eq!(mem.get_current_slice()[0], 6); + assert_eq!(mem.get_current_slice()[1], 7); + assert_eq!(mem.get_current_slice()[2], 8); + } + + #[test] + fn set_u256() { + let mut mem = SharedMemory::new(100_000, None); + mem.use_new_memory(); + + mem.set_u256(32, U256::MAX); + assert_ne!( + mem.get_current_slice()[0..32], + U256::MAX.to_le_bytes::<32>() + ); + assert_eq!( + mem.get_current_slice()[32..64], + U256::MAX.to_le_bytes::<32>() + ); + } + + #[test] + fn resize_1() { + let mut mem = SharedMemory::new(100_000, None); + mem.use_new_memory(); + + mem.set_u256(0, U256::MAX); + mem.resize(32); + assert_eq!( + mem.get_current_slice()[..32], + U256::ZERO.to_le_bytes::<32>() + ); + assert_eq!(mem.len(), 32); + } + + #[test] + fn use_new_memory_2() { + let mut mem = SharedMemory::new(100_000, None); + mem.use_new_memory(); + + // mstore(0, U256::MAX) equivalent + mem.resize(32); + assert_eq!(mem.len(), 32); + mem.set_u256(0, U256::MAX); + + // new depth + mem.use_new_memory(); + assert_eq!(mem.msizes.len(), 2); + assert_eq!(mem.msizes[1], 32); + + mem.set_u256(0, U256::MAX); + assert_eq!( + mem.data.get(32..64).unwrap(), + mem.get_current_slice()[..32].to_vec(), + ); + assert_eq!( + mem.get_current_slice()[..32].to_vec(), + U256::MAX.to_le_bytes::<32>().to_vec() + ); + + mem.free_memory(); + assert_eq!( + mem.data.get(..32).unwrap(), + mem.get_current_slice()[..32].to_vec(), + ); + assert_eq!( + mem.data()[32..64].to_vec(), + U256::ZERO.to_le_bytes::<32>().to_vec() + ); + + assert_eq!(mem.len(), 32); + assert_eq!(mem.msizes.len(), 1); + assert_eq!(mem.msizes[0], 0); + assert_eq!( + mem.get_current_slice()[..32].to_vec(), + U256::MAX.to_le_bytes::<32>().to_vec() + ); + } + + #[test] + fn use_new_memory_3() { + let mem = Rc::new(RefCell::new(SharedMemory::new(100_000, None))); + let mem_1 = Rc::clone(&mem); + mem_1.borrow_mut().use_new_memory(); + + // mstore(0, U256::MAX) equivalent + mem_1.borrow_mut().resize(32); + assert_eq!(mem_1.borrow().len(), 32); + mem_1.borrow_mut().set_u256(0, U256::MAX); + + // new depth + let mem_2 = Rc::clone(&mem); + mem_2.borrow_mut().use_new_memory(); + assert_eq!(mem_2.borrow().msizes.len(), 2); + assert_eq!(mem_2.borrow().msizes[1], 32); + + mem_2.borrow_mut().set_u256(0, U256::MAX); + assert_eq!( + mem_2.borrow().data.get(32..64).unwrap(), + mem_2.borrow().get_current_slice()[..32].to_vec(), + ); + assert_eq!( + mem_2.borrow().get_current_slice()[..32].to_vec(), + U256::MAX.to_le_bytes::<32>().to_vec() + ); + + mem_2.borrow_mut().free_memory(); + drop(mem_2); + assert_eq!( + mem_1.borrow().data.get(..32).unwrap(), + mem_1.borrow().get_current_slice()[..32].to_vec(), + ); + assert_eq!( + mem_1.borrow().data()[32..64].to_vec(), + U256::ZERO.to_le_bytes::<32>().to_vec() + ); + + assert_eq!(mem_1.borrow().len(), 32); + assert_eq!(mem_1.borrow().msizes.len(), 1); + assert_eq!(mem_1.borrow().msizes[0], 0); + assert_eq!( + mem_1.borrow().get_current_slice()[..32].to_vec(), + U256::MAX.to_le_bytes::<32>().to_vec() + ); + } } diff --git a/crates/revm/src/evm_impl.rs b/crates/revm/src/evm_impl.rs index 687b950abe7..17315e7c7b6 100644 --- a/crates/revm/src/evm_impl.rs +++ b/crates/revm/src/evm_impl.rs @@ -746,10 +746,11 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, inputs.is_static, shared_memory, ); + let return_value = interpreter.return_value(); CallResult { result: exit_reason, gas: interpreter.gas, - return_value: interpreter.return_value(), + return_value, } } else { CallResult { diff --git a/crates/revm/src/inspector/customprinter.rs b/crates/revm/src/inspector/customprinter.rs index ba69bdf2494..72fd4d25da3 100644 --- a/crates/revm/src/inspector/customprinter.rs +++ b/crates/revm/src/inspector/customprinter.rs @@ -38,7 +38,7 @@ impl Inspector for CustomPrintTracer { interp.gas.refunded(), interp.gas.refunded(), interp.stack.data(), - interp.memory.data().len(), + interp.shared_memory.borrow().len(), ); self.gas_inspector.step(interp, data); diff --git a/crates/revm/src/inspector/tracer_eip3155.rs b/crates/revm/src/inspector/tracer_eip3155.rs index 463fd3ce4a3..7f8f9f8a22c 100644 --- a/crates/revm/src/inspector/tracer_eip3155.rs +++ b/crates/revm/src/inspector/tracer_eip3155.rs @@ -63,7 +63,7 @@ impl Inspector for TracerEip3155 { self.stack = interp.stack.clone(); self.pc = interp.program_counter(); self.opcode = interp.current_opcode(); - self.mem_size = interp.memory.len(); + self.mem_size = interp.shared_memory.borrow().len(); self.gas = self.gas_inspector.gas_remaining(); // InstructionResult::Continue