From 3d09aa54645b77f0e0c34015bfe7bc8f85737c85 Mon Sep 17 00:00:00 2001 From: Lorenzo Feroleto Date: Mon, 14 Aug 2023 16:52:46 +0200 Subject: [PATCH 01/19] feat: shared memory primitive --- crates/primitives/src/lib.rs | 1 + crates/primitives/src/shared_memory.rs | 161 +++++++++++++++++++++++++ 2 files changed, 162 insertions(+) create mode 100644 crates/primitives/src/shared_memory.rs diff --git a/crates/primitives/src/lib.rs b/crates/primitives/src/lib.rs index 6fe75693ef..b57958fc69 100644 --- a/crates/primitives/src/lib.rs +++ b/crates/primitives/src/lib.rs @@ -10,6 +10,7 @@ pub mod env; pub mod log; pub mod precompile; pub mod result; +pub mod shared_memory; pub mod specification; pub mod state; pub mod utilities; diff --git a/crates/primitives/src/shared_memory.rs b/crates/primitives/src/shared_memory.rs new file mode 100644 index 0000000000..2cbc81a9e6 --- /dev/null +++ b/crates/primitives/src/shared_memory.rs @@ -0,0 +1,161 @@ +use crate::U256; +use core::{ + cmp::min, + ops::{BitAnd, Not}, +}; + +pub struct SharedMemory { + pub data: Vec, + pub limit: u64, + /// Memory sizes checkpoint for each depth + pub msizes: Vec, + current_slice: *mut [u8], + current_len: usize, +} + +impl SharedMemory { + fn get_current_slice(&self) -> &[u8] { + unsafe { &*self.current_slice } + } + + fn get_current_slice_mut(&mut self) -> &mut [u8] { + unsafe { &mut *self.current_slice } + } + + pub fn use_new_memory(&mut self) { + 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 = self.msizes.last().unwrap(); + + let range = if new_msize == &0 { + 0.. + } else { + new_msize + 1.. + }; + + 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 + } 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]; + let current_slice: *mut [u8] = &mut data[..]; + SharedMemory { + data, + limit, + msizes, + current_slice, + current_len: 0, + } + } + + /// Resize the memory. asume 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 { + panic!("Max limit reached") + } + + let range = if new_size > self.current_len { + // extend with zeros + self.current_len + 1..=new_size + } else { + // truncate + new_size..=self.current_len + }; + + self.get_current_slice_mut()[range] + .iter_mut() + .for_each(|byte| *byte = 0); + 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] { + &self.get_current_slice()[offset..offset + size] + } + + #[inline(always)] + pub fn set_u256(&mut self, index: usize, value: U256) { + self.get_current_slice_mut()[index..index + 32] + .copy_from_slice(&value.to_be_bytes::<{ U256::BYTES }>()); + } + + /// Set memory region at given offset. The offset and value are already checked + #[inline(always)] + pub fn set(&mut self, offset: usize, value: &[u8]) { + if !value.is_empty() { + self.get_current_slice_mut()[offset..(value.len() + offset)].copy_from_slice(value); + } + } + + /// Set memory from data. Our memory offset+len is expected to be correct but we + /// are doing bound checks on data/data_offeset/len and zeroing parts that is not copied. + #[inline(always)] + pub fn set_data(&mut self, memory_offset: usize, data_offset: usize, len: usize, data: &[u8]) { + if data_offset >= data.len() { + // nulify all memory slots + for i in &mut self.get_current_slice_mut()[memory_offset..memory_offset + len] { + *i = 0; + } + return; + } + let data_end = min(data_offset + len, data.len()); + let memory_data_end = memory_offset + (data_end - data_offset); + self.get_current_slice_mut()[memory_offset..memory_data_end] + .copy_from_slice(&data[data_offset..data_end]); + + // nulify rest of memory slots + // Safety: Memory is assumed to be valid. And it is commented where that assumption is made + for i in &mut self.get_current_slice_mut()[memory_data_end..memory_offset + len] { + *i = 0; + } + } + + /// In memory copy given a src, dst, and length + /// + /// # Safety + /// The caller is responsible to check that we resized memory properly. + #[inline(always)] + pub fn copy(&mut self, dst: usize, src: usize, length: usize) { + self.get_current_slice_mut() + .copy_within(src..src + length, dst); + } +} + +#[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) +} From 9ad9da86b7344603f50ef87e39002ef1f94072f6 Mon Sep 17 00:00:00 2001 From: Lorenzo Feroleto Date: Wed, 23 Aug 2023 16:55:45 +0200 Subject: [PATCH 02/19] feat(shared_memory): available inside interpreter and ready to use --- crates/interpreter/src/host.rs | 11 ++- crates/interpreter/src/host/dummy.rs | 11 ++- crates/interpreter/src/instructions/host.rs | 5 +- crates/interpreter/src/interpreter.rs | 16 +++- crates/revm/benches/bench.rs | 7 +- crates/revm/src/evm_impl.rs | 95 ++++++++++++++------- 6 files changed, 106 insertions(+), 39 deletions(-) diff --git a/crates/interpreter/src/host.rs b/crates/interpreter/src/host.rs index 777a84957e..1dab851428 100644 --- a/crates/interpreter/src/host.rs +++ b/crates/interpreter/src/host.rs @@ -1,3 +1,5 @@ +use core::cell::RefCell; + use crate::primitives::Bytecode; use crate::{ primitives::{Bytes, Env, B160, B256, U256}, @@ -7,6 +9,8 @@ use alloc::vec::Vec; pub use dummy::DummyHost; mod dummy; +use alloc::rc::Rc; +use revm_primitives::shared_memory::SharedMemory; /// EVM context host. pub trait Host { @@ -51,7 +55,12 @@ pub trait Host { fn create( &mut self, inputs: &mut CreateInputs, + shared_memory: &Rc>, ) -> (InstructionResult, Option, Gas, Bytes); /// Invoke a call operation. - fn call(&mut self, input: &mut CallInputs) -> (InstructionResult, Gas, Bytes); + fn call( + &mut self, + input: &mut CallInputs, + shared_memory: &Rc>, + ) -> (InstructionResult, Gas, Bytes); } diff --git a/crates/interpreter/src/host/dummy.rs b/crates/interpreter/src/host/dummy.rs index c2df6b2f65..722b415911 100644 --- a/crates/interpreter/src/host/dummy.rs +++ b/crates/interpreter/src/host/dummy.rs @@ -1,9 +1,13 @@ +use core::cell::RefCell; + use crate::primitives::{hash_map::Entry, Bytecode, Bytes, HashMap, U256}; use crate::{ primitives::{Env, Log, B160, B256, KECCAK_EMPTY}, CallInputs, CreateInputs, Gas, Host, InstructionResult, Interpreter, SelfDestructResult, }; +use alloc::rc::Rc; use alloc::vec::Vec; +use revm_primitives::shared_memory::SharedMemory; pub struct DummyHost { pub env: Env, @@ -137,12 +141,17 @@ impl Host for DummyHost { fn create( &mut self, _inputs: &mut CreateInputs, + _shared_memory: &Rc>, ) -> (InstructionResult, Option, Gas, Bytes) { panic!("Create is not supported for this host") } #[inline] - fn call(&mut self, _input: &mut CallInputs) -> (InstructionResult, Gas, Bytes) { + fn call( + &mut self, + _input: &mut CallInputs, + _shared_memory: &Rc>, + ) -> (InstructionResult, Gas, Bytes) { panic!("Call is not supported for this host") } } diff --git a/crates/interpreter/src/instructions/host.rs b/crates/interpreter/src/instructions/host.rs index 95d549d633..7e0d44642f 100644 --- a/crates/interpreter/src/instructions/host.rs +++ b/crates/interpreter/src/instructions/host.rs @@ -320,7 +320,8 @@ pub fn create( return; }; - let (return_reason, address, gas, return_data) = host.create(&mut create_input); + let (return_reason, address, gas, return_data) = + host.create(&mut create_input, &mut interpreter.shared_memory); interpreter.return_data_buffer = match return_reason { // Save data to return data buffer if the create reverted @@ -540,7 +541,7 @@ pub fn call_inner( }; // Call host to interact with target contract - let (reason, gas, return_data) = host.call(&mut call_input); + let (reason, gas, return_data) = host.call(&mut call_input, &mut interpreter.shared_memory); interpreter.return_data_buffer = return_data; diff --git a/crates/interpreter/src/interpreter.rs b/crates/interpreter/src/interpreter.rs index 9f21ad5202..bf7a08e556 100644 --- a/crates/interpreter/src/interpreter.rs +++ b/crates/interpreter/src/interpreter.rs @@ -14,7 +14,10 @@ use crate::{ instructions::{eval, InstructionResult}, Gas, Host, }; +use alloc::rc::Rc; +use core::cell::RefCell; use core::ops::Range; +use revm_primitives::shared_memory::SharedMemory; pub const CALL_STACK_LIMIT: u64 = 1024; @@ -48,6 +51,8 @@ pub struct Interpreter { /// Memory limit. See [`crate::CfgEnv`]. #[cfg(feature = "memory_limit")] pub memory_limit: u64, + + pub shared_memory: Rc>, } impl Interpreter { @@ -57,7 +62,12 @@ impl Interpreter { } /// Create new interpreter - pub fn new(contract: Box, gas_limit: u64, is_static: bool) -> Self { + pub fn new( + contract: Box, + gas_limit: u64, + is_static: bool, + shared_memory: &Rc>, + ) -> Self { #[cfg(not(feature = "memory_limit"))] { Self { @@ -75,7 +85,7 @@ impl Interpreter { #[cfg(feature = "memory_limit")] { - Self::new_with_memory_limit(contract, gas_limit, is_static, u64::MAX) + Self::new_with_memory_limit(contract, gas_limit, is_static, u64::MAX, shared_memory) } } @@ -85,6 +95,7 @@ impl Interpreter { gas_limit: u64, is_static: bool, memory_limit: u64, + shared_memory: &Rc>, ) -> Self { Self { instruction_pointer: contract.bytecode.as_ptr(), @@ -97,6 +108,7 @@ impl Interpreter { is_static, gas: Gas::new(gas_limit), memory_limit, + shared_memory: Rc::clone(shared_memory), } } diff --git a/crates/revm/benches/bench.rs b/crates/revm/benches/bench.rs index d51e174d0c..f7ff90c87b 100644 --- a/crates/revm/benches/bench.rs +++ b/crates/revm/benches/bench.rs @@ -7,7 +7,8 @@ use revm::{ interpreter::{analysis::to_analysed, BytecodeLocked, Contract, DummyHost, Interpreter}, primitives::{BerlinSpec, Bytecode, BytecodeState, TransactTo, U256}, }; -use std::time::Duration; +use revm_interpreter::primitives::shared_memory::SharedMemory; +use std::{cell::RefCell, rc::Rc, time::Duration}; type Evm = revm::EVM; @@ -110,8 +111,10 @@ fn bench_eval(g: &mut BenchmarkGroup<'_, WallTime>, evm: &Evm) { ..Default::default() }; let mut host = DummyHost::new(evm.env.clone()); + let shared_memory = Rc::new(RefCell::new(SharedMemory::new(evm.env.tx.gas_limit, None))); b.iter(|| { - let mut interpreter = Interpreter::new(Box::new(contract.clone()), u64::MAX, false); + let mut interpreter = + Interpreter::new(Box::new(contract.clone()), u64::MAX, false, &shared_memory); let res = interpreter.run::<_, BerlinSpec>(&mut host); host.clear(); res diff --git a/crates/revm/src/evm_impl.rs b/crates/revm/src/evm_impl.rs index c3da184cf0..d4c68e9983 100644 --- a/crates/revm/src/evm_impl.rs +++ b/crates/revm/src/evm_impl.rs @@ -13,9 +13,12 @@ use crate::primitives::{ }; use crate::{db::Database, journaled_state::JournaledState, precompile, Inspector}; use alloc::boxed::Box; +use alloc::rc::Rc; use alloc::vec::Vec; +use core::cell::RefCell; use core::{cmp::min, marker::PhantomData}; use revm_interpreter::gas::initial_tx_gas; +use revm_interpreter::primitives::shared_memory::SharedMemory; use revm_interpreter::MAX_CODE_SIZE; use revm_precompile::{Precompile, Precompiles}; @@ -160,6 +163,8 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> Transact let transact_gas_limit = tx_gas_limit - initial_gas_spend; + let shared_memory = Rc::new(RefCell::new(SharedMemory::new(transact_gas_limit, None))); + // call inner handling of call/create let (exit_reason, ret_gas, output) = match self.data.env.tx.transact_to { TransactTo::Call(address) => { @@ -167,34 +172,40 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> Transact caller_account.info.nonce = caller_account.info.nonce.checked_add(1).unwrap_or(u64::MAX); - let (exit, gas, bytes) = self.call(&mut CallInputs { - contract: address, - transfer: Transfer { - source: tx_caller, - target: address, - value: tx_value, - }, - input: tx_data, - gas_limit: transact_gas_limit, - context: CallContext { - caller: tx_caller, - address, - code_address: address, - apparent_value: tx_value, - scheme: CallScheme::Call, + let (exit, gas, bytes) = self.call( + &mut CallInputs { + contract: address, + transfer: Transfer { + source: tx_caller, + target: address, + value: tx_value, + }, + input: tx_data, + gas_limit: transact_gas_limit, + context: CallContext { + caller: tx_caller, + address, + code_address: address, + apparent_value: tx_value, + scheme: CallScheme::Call, + }, + is_static: false, }, - is_static: false, - }); + &shared_memory, + ); (exit, gas, Output::Call(bytes)) } TransactTo::Create(scheme) => { - let (exit, address, ret_gas, bytes) = self.create(&mut CreateInputs { - caller: tx_caller, - scheme, - value: tx_value, - init_code: tx_data, - gas_limit: transact_gas_limit, - }); + let (exit, address, ret_gas, bytes) = self.create( + &mut CreateInputs { + caller: tx_caller, + scheme, + value: tx_value, + init_code: tx_data, + gas_limit: transact_gas_limit, + }, + &shared_memory, + ); (exit, ret_gas, Output::Create(bytes, address)) } }; @@ -446,7 +457,11 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, } /// EVM create opcode for both initial crate and CREATE and CREATE2 opcodes. - fn create_inner(&mut self, inputs: &CreateInputs) -> CreateResult { + fn create_inner( + &mut self, + inputs: &CreateInputs, + shared_memory: &Rc>, + ) -> CreateResult { // Prepare crate. let prepared_create = match self.prepare_create(inputs) { Ok(o) => o, @@ -454,8 +469,12 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, }; // Create new interpreter and execute initcode - let (exit_reason, mut interpreter) = - self.run_interpreter(prepared_create.contract, prepared_create.gas.limit(), false); + let (exit_reason, mut interpreter) = self.run_interpreter( + prepared_create.contract, + prepared_create.gas.limit(), + false, + shared_memory, + ); // Host error if present on execution match exit_reason { @@ -558,6 +577,7 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, contract: Box, gas_limit: u64, is_static: bool, + shared_memory: &Rc>, ) -> (InstructionResult, Box) { // Create inspector #[cfg(feature = "memory_limit")] @@ -566,8 +586,11 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, gas_limit, is_static, self.data.env.cfg.memory_limit, + shared_memory, )); + interpreter.shared_memory.borrow_mut().use_new_memory(); + #[cfg(not(feature = "memory_limit"))] let mut interpreter = Box::new(Interpreter::new(contract, gas_limit, is_static)); @@ -697,7 +720,11 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, } /// Main contract call of the EVM. - fn call_inner(&mut self, inputs: &CallInputs) -> CallResult { + fn call_inner( + &mut self, + inputs: &CallInputs, + shared_memory: &Rc>, + ) -> CallResult { // Prepare call let prepared_call = match self.prepare_call(inputs) { Ok(o) => o, @@ -712,6 +739,7 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, prepared_call.contract, prepared_call.gas.limit(), inputs.is_static, + shared_memory, ); CallResult { result: exit_reason, @@ -868,6 +896,7 @@ impl<'a, GSPEC: Spec, DB: Database + 'a, const INSPECT: bool> Host fn create( &mut self, inputs: &mut CreateInputs, + shared_memory: &Rc>, ) -> (InstructionResult, Option, Gas, Bytes) { // Call inspector if INSPECT { @@ -878,7 +907,7 @@ impl<'a, GSPEC: Spec, DB: Database + 'a, const INSPECT: bool> Host .create_end(&mut self.data, inputs, ret, address, gas, out); } } - let ret = self.create_inner(inputs); + let ret = self.create_inner(inputs, shared_memory); if INSPECT { self.inspector.create_end( &mut self.data, @@ -893,7 +922,11 @@ impl<'a, GSPEC: Spec, DB: Database + 'a, const INSPECT: bool> Host } } - fn call(&mut self, inputs: &mut CallInputs) -> (InstructionResult, Gas, Bytes) { + fn call( + &mut self, + inputs: &mut CallInputs, + shared_memory: &Rc>, + ) -> (InstructionResult, Gas, Bytes) { if INSPECT { let (ret, gas, out) = self.inspector.call(&mut self.data, inputs); if ret != InstructionResult::Continue { @@ -902,7 +935,7 @@ impl<'a, GSPEC: Spec, DB: Database + 'a, const INSPECT: bool> Host .call_end(&mut self.data, inputs, gas, ret, out); } } - let ret = self.call_inner(inputs); + let ret = self.call_inner(inputs, shared_memory); if INSPECT { self.inspector.call_end( &mut self.data, From 1e699078bb588d12b981aa3d5d668985491075c1 Mon Sep 17 00:00:00 2001 From: Lorenzo Feroleto Date: Thu, 24 Aug 2023 10:35:59 +0200 Subject: [PATCH 03/19] chore(memory): replaced memory with shared memory --- .../interpreter/src/instructions/control.rs | 4 +- crates/interpreter/src/instructions/host.rs | 39 ++++++++++----- crates/interpreter/src/instructions/macros.rs | 47 ++++++++++++++++--- crates/interpreter/src/instructions/memory.rs | 34 ++++++++++---- crates/interpreter/src/instructions/system.rs | 23 +++++---- crates/interpreter/src/interpreter.rs | 3 +- crates/primitives/src/shared_memory.rs | 9 ++++ crates/revm/src/evm_impl.rs | 11 +++-- 8 files changed, 125 insertions(+), 45 deletions(-) diff --git a/crates/interpreter/src/instructions/control.rs b/crates/interpreter/src/instructions/control.rs index ccb532f6c5..aafaa4508d 100644 --- a/crates/interpreter/src/instructions/control.rs +++ b/crates/interpreter/src/instructions/control.rs @@ -50,7 +50,7 @@ pub fn ret(interpreter: &mut Interpreter, _host: &mut dyn Host) { interpreter.return_range = usize::MAX..usize::MAX; } else { let offset = as_usize_or_fail!(interpreter, start, InstructionResult::InvalidOperandOOG); - memory_resize!(interpreter, offset, len); + shared_memory_resize!(interpreter, offset, len); interpreter.return_range = offset..(offset + len); } interpreter.instruction_result = InstructionResult::Return; @@ -66,7 +66,7 @@ pub fn revert(interpreter: &mut Interpreter, _host: &mut dyn Host) { interpreter.return_range = usize::MAX..usize::MAX; } else { let offset = as_usize_or_fail!(interpreter, start, InstructionResult::InvalidOperandOOG); - memory_resize!(interpreter, offset, len); + shared_memory_resize!(interpreter, offset, len); interpreter.return_range = offset..(offset + len); } interpreter.instruction_result = InstructionResult::Revert; diff --git a/crates/interpreter/src/instructions/host.rs b/crates/interpreter/src/instructions/host.rs index 7e0d44642f..321c6e966e 100644 --- a/crates/interpreter/src/instructions/host.rs +++ b/crates/interpreter/src/instructions/host.rs @@ -113,11 +113,12 @@ pub fn extcodecopy(interpreter: &mut Interpreter, host: &mut dyn Hos InstructionResult::InvalidOperandOOG ); let code_offset = min(as_usize_saturated!(code_offset), code.len()); - memory_resize!(interpreter, memory_offset, len); + shared_memory_resize!(interpreter, memory_offset, len); // Safety: set_data is unsafe function and memory_resize ensures us that it is safe to call it interpreter - .memory + .shared_memory + .borrow_mut() .set_data(memory_offset, code_offset, len, code.bytes()); } @@ -201,8 +202,8 @@ pub fn log(interpreter: &mut Interpreter, host: &mut dyn Host) { Bytes::new() } else { let offset = as_usize_or_fail!(interpreter, offset, InstructionResult::InvalidOperandOOG); - memory_resize!(interpreter, offset, len); - Bytes::copy_from_slice(interpreter.memory.get_slice(offset, len)) + shared_memory_resize!(interpreter, offset, len); + Bytes::copy_from_slice(interpreter.shared_memory.borrow().get_slice(offset, len)) }; let n = N as usize; if interpreter.stack.len() < n { @@ -278,8 +279,13 @@ pub fn prepare_create_inputs( } gas!(interpreter, gas::initcode_cost(len as u64)); } - memory_resize!(interpreter, code_offset, len); - Bytes::copy_from_slice(interpreter.memory.get_slice(code_offset, len)) + shared_memory_resize!(interpreter, code_offset, len); + Bytes::copy_from_slice( + interpreter + .shared_memory + .borrow() + .get_slice(code_offset, len), + ) }; let scheme = if IS_CREATE2 { @@ -321,7 +327,7 @@ pub fn create( }; let (return_reason, address, gas, return_data) = - host.create(&mut create_input, &mut interpreter.shared_memory); + host.create(&mut create_input, &interpreter.shared_memory); interpreter.return_data_buffer = match return_reason { // Save data to return data buffer if the create reverted @@ -403,8 +409,13 @@ fn prepare_call_inputs( let input = if in_len != 0 { let in_offset = as_usize_or_fail!(interpreter, in_offset, InstructionResult::InvalidOperandOOG); - memory_resize!(interpreter, in_offset, in_len); - Bytes::copy_from_slice(interpreter.memory.get_slice(in_offset, in_len)) + shared_memory_resize!(interpreter, in_offset, in_len); + Bytes::copy_from_slice( + interpreter + .shared_memory + .borrow() + .get_slice(in_offset, in_len), + ) } else { Bytes::new() }; @@ -416,7 +427,7 @@ fn prepare_call_inputs( out_offset, InstructionResult::InvalidOperandOOG ); - memory_resize!(interpreter, out_offset, *result_len); + shared_memory_resize!(interpreter, out_offset, *result_len); out_offset } else { usize::MAX //unrealistic value so we are sure it is not used @@ -541,7 +552,7 @@ pub fn call_inner( }; // Call host to interact with target contract - let (reason, gas, return_data) = host.call(&mut call_input, &mut interpreter.shared_memory); + let (reason, gas, return_data) = host.call(&mut call_input, &interpreter.shared_memory); interpreter.return_data_buffer = return_data; @@ -555,7 +566,8 @@ pub fn call_inner( interpreter.gas.record_refund(gas.refunded()); } interpreter - .memory + .shared_memory + .borrow_mut() .set(out_offset, &interpreter.return_data_buffer[..target_len]); push!(interpreter, U256::from(1)); } @@ -564,7 +576,8 @@ pub fn call_inner( interpreter.gas.erase_cost(gas.remaining()); } interpreter - .memory + .shared_memory + .borrow_mut() .set(out_offset, &interpreter.return_data_buffer[..target_len]); push!(interpreter, U256::ZERO); } diff --git a/crates/interpreter/src/instructions/macros.rs b/crates/interpreter/src/instructions/macros.rs index 618abbf9c9..639cb57898 100644 --- a/crates/interpreter/src/instructions/macros.rs +++ b/crates/interpreter/src/instructions/macros.rs @@ -49,18 +49,51 @@ macro_rules! gas_or_fail { }; } -macro_rules! memory_resize { - ($interp:expr, $offset:expr, $len:expr) => { +// macro_rules! memory_resize { +// ($interp:expr, $offset:expr, $len:expr) => {{ +// let len: usize = $len; +// let offset: usize = $offset; +// if let Some(new_size) = +// crate::interpreter::memory::next_multiple_of_32(offset.saturating_add(len)) +// { +// #[cfg(feature = "memory_limit")] +// if new_size > ($interp.memory_limit as usize) { +// $interp.instruction_result = InstructionResult::MemoryLimitOOG; +// return; +// } +// +// if new_size > $interp.memory.len() { +// if crate::USE_GAS { +// let num_bytes = new_size / 32; +// if !$interp.gas.record_memory(crate::gas::memory_gas(num_bytes)) { +// $interp.instruction_result = InstructionResult::MemoryLimitOOG; +// return; +// } +// } +// $interp.memory.resize(new_size); +// } +// } else { +// $interp.instruction_result = InstructionResult::MemoryOOG; +// return; +// } +// }}; +// } + +macro_rules! shared_memory_resize { + ($interp:expr, $offset:expr, $len:expr) => {{ + let len: usize = $len; + let offset: usize = $offset; + let mut mem = $interp.shared_memory.borrow_mut(); if let Some(new_size) = - crate::interpreter::memory::next_multiple_of_32($offset.saturating_add($len)) + crate::interpreter::memory::next_multiple_of_32(offset.saturating_add(len)) { #[cfg(feature = "memory_limit")] - if new_size > ($interp.memory_limit as usize) { + if new_size > (mem.limit as usize) { $interp.instruction_result = InstructionResult::MemoryLimitOOG; return; } - if new_size > $interp.memory.len() { + if new_size > mem.len() { if crate::USE_GAS { let num_bytes = new_size / 32; if !$interp.gas.record_memory(crate::gas::memory_gas(num_bytes)) { @@ -68,13 +101,13 @@ macro_rules! memory_resize { return; } } - $interp.memory.resize(new_size); + mem.resize(new_size); } } else { $interp.instruction_result = InstructionResult::MemoryOOG; return; } - }; + };}; } macro_rules! pop_address { diff --git a/crates/interpreter/src/instructions/memory.rs b/crates/interpreter/src/instructions/memory.rs index a33040c25f..46a92a1639 100644 --- a/crates/interpreter/src/instructions/memory.rs +++ b/crates/interpreter/src/instructions/memory.rs @@ -13,11 +13,16 @@ pub fn mload(interpreter: &mut Interpreter, _host: &mut dyn Host) { gas!(interpreter, gas::VERYLOW); pop!(interpreter, index); let index = as_usize_or_fail!(interpreter, index, InstructionResult::InvalidOperandOOG); - memory_resize!(interpreter, index, 32); + shared_memory_resize!(interpreter, index, 32); push!( interpreter, U256::from_be_bytes::<{ U256::BYTES }>( - interpreter.memory.get_slice(index, 32).try_into().unwrap() + interpreter + .shared_memory + .borrow() + .get_slice(index, 32) + .try_into() + .unwrap() ) ); } @@ -26,23 +31,34 @@ pub fn mstore(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); - memory_resize!(interpreter, index, 32); - interpreter.memory.set_u256(index, value); + shared_memory_resize!(interpreter, index, 32); + interpreter + .shared_memory + .borrow_mut() + .set_u256(index, value); } 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); - memory_resize!(interpreter, index, 1); + shared_memory_resize!(interpreter, index, 32); let value = value.as_le_bytes()[0]; // Safety: we resized our memory two lines above. - unsafe { interpreter.memory.set_byte(index, value) } + unsafe { + interpreter + .shared_memory + .borrow_mut() + .set_byte(index, value) + } } pub fn msize(interpreter: &mut Interpreter, _host: &mut dyn Host) { gas!(interpreter, gas::BASE); - push!(interpreter, U256::from(interpreter.memory.effective_len())); + push!( + interpreter, + U256::from(interpreter.shared_memory.borrow().effective_len()) + ); } // From EIP-5656 MCOPY @@ -64,7 +80,7 @@ pub fn mcopy(interpreter: &mut Interpreter, _host: &mut dyn Host) { let dest = as_usize_or_fail!(interpreter, dest, InstructionResult::InvalidOperandOOG); let src = as_usize_or_fail!(interpreter, src, InstructionResult::InvalidOperandOOG); // resize memory - memory_resize!(interpreter, max(dest, src), len); + shared_memory_resize!(interpreter, max(dest, src), len); // copy memory in place - interpreter.memory.copy(dest, src, len); + interpreter.shared_memory.borrow_mut().copy(dest, src, len); } diff --git a/crates/interpreter/src/instructions/system.rs b/crates/interpreter/src/instructions/system.rs index a40a6c6cf5..0be0df5d8a 100644 --- a/crates/interpreter/src/instructions/system.rs +++ b/crates/interpreter/src/instructions/system.rs @@ -14,8 +14,8 @@ pub fn calculate_keccak256(interpreter: &mut Interpreter, _host: &mut dyn Host) KECCAK_EMPTY } else { let from = as_usize_or_fail!(interpreter, from, InstructionResult::InvalidOperandOOG); - memory_resize!(interpreter, from, len); - keccak256(interpreter.memory.get_slice(from, len)) + shared_memory_resize!(interpreter, from, len); + keccak256(interpreter.shared_memory.borrow().get_slice(from, len)) }; push_b256!(interpreter, hash); @@ -49,10 +49,10 @@ pub fn codecopy(interpreter: &mut Interpreter, _host: &mut dyn Host) { InstructionResult::InvalidOperandOOG ); let code_offset = as_usize_saturated!(code_offset); - memory_resize!(interpreter, memory_offset, len); + shared_memory_resize!(interpreter, memory_offset, len); // Safety: set_data is unsafe function and memory_resize ensures us that it is safe to call it - interpreter.memory.set_data( + interpreter.shared_memory.borrow_mut().set_data( memory_offset, code_offset, len, @@ -100,12 +100,15 @@ pub fn calldatacopy(interpreter: &mut Interpreter, _host: &mut dyn Host) { InstructionResult::InvalidOperandOOG ); let data_offset = as_usize_saturated!(data_offset); - memory_resize!(interpreter, memory_offset, len); + shared_memory_resize!(interpreter, memory_offset, len); // Safety: set_data is unsafe function and memory_resize ensures us that it is safe to call it - interpreter - .memory - .set_data(memory_offset, data_offset, len, &interpreter.contract.input); + interpreter.shared_memory.borrow_mut().set_data( + memory_offset, + data_offset, + len, + &interpreter.contract.input, + ); } pub fn returndatasize(interpreter: &mut Interpreter, _host: &mut dyn Host) { @@ -136,8 +139,8 @@ pub fn returndatacopy(interpreter: &mut Interpreter, _host: &mut dyn memory_offset, InstructionResult::InvalidOperandOOG ); - memory_resize!(interpreter, memory_offset, len); - interpreter.memory.set( + shared_memory_resize!(interpreter, memory_offset, len); + interpreter.shared_memory.borrow_mut().set( memory_offset, &interpreter.return_data_buffer[data_offset..data_end], ); diff --git a/crates/interpreter/src/interpreter.rs b/crates/interpreter/src/interpreter.rs index bf7a08e556..9f86852501 100644 --- a/crates/interpreter/src/interpreter.rs +++ b/crates/interpreter/src/interpreter.rs @@ -80,6 +80,7 @@ impl Interpreter { instruction_result: InstructionResult::Continue, is_static, gas: Gas::new(gas_limit), + shared_memory: Rc::clone(shared_memory), } } @@ -184,7 +185,7 @@ impl Interpreter { if self.return_range.start == usize::MAX { Bytes::new() } else { - Bytes::copy_from_slice(self.memory.get_slice( + Bytes::copy_from_slice(self.shared_memory.borrow().get_slice( self.return_range.start, self.return_range.end - self.return_range.start, )) diff --git a/crates/primitives/src/shared_memory.rs b/crates/primitives/src/shared_memory.rs index 2cbc81a9e6..d66af73790 100644 --- a/crates/primitives/src/shared_memory.rs +++ b/crates/primitives/src/shared_memory.rs @@ -106,6 +106,15 @@ impl SharedMemory { &self.get_current_slice()[offset..offset + size] } + /// Set memory region at given offset + /// + /// # Safety + /// The caller is responsible for checking the offset and value + #[inline(always)] + pub unsafe fn set_byte(&mut self, index: usize, byte: u8) { + self.get_current_slice_mut()[index] = byte; + } + #[inline(always)] pub fn set_u256(&mut self, index: usize, value: U256) { self.get_current_slice_mut()[index..index + 32] diff --git a/crates/revm/src/evm_impl.rs b/crates/revm/src/evm_impl.rs index d4c68e9983..687b950abe 100644 --- a/crates/revm/src/evm_impl.rs +++ b/crates/revm/src/evm_impl.rs @@ -589,10 +589,15 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, shared_memory, )); - interpreter.shared_memory.borrow_mut().use_new_memory(); - #[cfg(not(feature = "memory_limit"))] - let mut interpreter = Box::new(Interpreter::new(contract, gas_limit, is_static)); + let mut interpreter = Box::new(Interpreter::new( + contract, + gas_limit, + is_static, + shared_memory, + )); + + interpreter.shared_memory.borrow_mut().use_new_memory(); if INSPECT { self.inspector From b415024ddf5b2b78efc95de05d27e7c4c6e0a05f Mon Sep 17 00:00:00 2001 From: Lorenzo Feroleto Date: Fri, 25 Aug 2023 17:11:08 +0200 Subject: [PATCH 04/19] 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 | 291 +++++++++++++++--- 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, 258 insertions(+), 63 deletions(-) diff --git a/crates/interpreter/src/instructions/host.rs b/crates/interpreter/src/instructions/host.rs index 321c6e966e..89fce027ab 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 46a92a1639..3791043f2d 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 9f86852501..c023f2522f 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 d66af73790..715e941929 100644 --- a/crates/primitives/src/shared_memory.rs +++ b/crates/primitives/src/shared_memory.rs @@ -1,14 +1,13 @@ +use crate::alloc::vec; +use crate::alloc::vec::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 +25,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 +87,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 +99,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 +162,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 687b950abe..17315e7c7b 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 ba69bdf249..72fd4d25da 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 463fd3ce4a..7f8f9f8a22 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 From caad052e4e47eba66f8979d999a37ea2e19f3b27 Mon Sep 17 00:00:00 2001 From: Lorenzo Feroleto Date: Tue, 29 Aug 2023 14:17:34 +0200 Subject: [PATCH 05/19] chore(shared_memory): moved under interpreter crate; small refactoring; doc comments --- crates/interpreter/src/host.rs | 2 +- crates/interpreter/src/host/dummy.rs | 2 +- crates/interpreter/src/instructions/macros.rs | 35 +- crates/interpreter/src/interpreter.rs | 18 +- crates/interpreter/src/interpreter/memory.rs | 174 --------- .../src/interpreter/shared_memory.rs | 189 +++++++++ crates/interpreter/src/lib.rs | 2 +- crates/primitives/src/lib.rs | 1 - crates/primitives/src/shared_memory.rs | 363 ------------------ crates/revm/benches/bench.rs | 4 +- crates/revm/src/evm_impl.rs | 5 +- crates/revm/src/inspector/tracer_eip3155.rs | 4 +- 12 files changed, 205 insertions(+), 594 deletions(-) delete mode 100644 crates/interpreter/src/interpreter/memory.rs create mode 100644 crates/interpreter/src/interpreter/shared_memory.rs delete mode 100644 crates/primitives/src/shared_memory.rs diff --git a/crates/interpreter/src/host.rs b/crates/interpreter/src/host.rs index 1dab851428..4e1a75e0e5 100644 --- a/crates/interpreter/src/host.rs +++ b/crates/interpreter/src/host.rs @@ -4,13 +4,13 @@ use crate::primitives::Bytecode; use crate::{ primitives::{Bytes, Env, B160, B256, U256}, CallInputs, CreateInputs, Gas, InstructionResult, Interpreter, SelfDestructResult, + SharedMemory, }; use alloc::vec::Vec; pub use dummy::DummyHost; mod dummy; use alloc::rc::Rc; -use revm_primitives::shared_memory::SharedMemory; /// EVM context host. pub trait Host { diff --git a/crates/interpreter/src/host/dummy.rs b/crates/interpreter/src/host/dummy.rs index 722b415911..7c0dab6a1a 100644 --- a/crates/interpreter/src/host/dummy.rs +++ b/crates/interpreter/src/host/dummy.rs @@ -4,10 +4,10 @@ use crate::primitives::{hash_map::Entry, Bytecode, Bytes, HashMap, U256}; use crate::{ primitives::{Env, Log, B160, B256, KECCAK_EMPTY}, CallInputs, CreateInputs, Gas, Host, InstructionResult, Interpreter, SelfDestructResult, + SharedMemory, }; use alloc::rc::Rc; use alloc::vec::Vec; -use revm_primitives::shared_memory::SharedMemory; pub struct DummyHost { pub env: Env, diff --git a/crates/interpreter/src/instructions/macros.rs b/crates/interpreter/src/instructions/macros.rs index 639cb57898..a3d988c5b9 100644 --- a/crates/interpreter/src/instructions/macros.rs +++ b/crates/interpreter/src/instructions/macros.rs @@ -49,46 +49,15 @@ macro_rules! gas_or_fail { }; } -// macro_rules! memory_resize { -// ($interp:expr, $offset:expr, $len:expr) => {{ -// let len: usize = $len; -// let offset: usize = $offset; -// if let Some(new_size) = -// crate::interpreter::memory::next_multiple_of_32(offset.saturating_add(len)) -// { -// #[cfg(feature = "memory_limit")] -// if new_size > ($interp.memory_limit as usize) { -// $interp.instruction_result = InstructionResult::MemoryLimitOOG; -// return; -// } -// -// if new_size > $interp.memory.len() { -// if crate::USE_GAS { -// let num_bytes = new_size / 32; -// if !$interp.gas.record_memory(crate::gas::memory_gas(num_bytes)) { -// $interp.instruction_result = InstructionResult::MemoryLimitOOG; -// return; -// } -// } -// $interp.memory.resize(new_size); -// } -// } else { -// $interp.instruction_result = InstructionResult::MemoryOOG; -// return; -// } -// }}; -// } - macro_rules! shared_memory_resize { ($interp:expr, $offset:expr, $len:expr) => {{ let len: usize = $len; let offset: usize = $offset; let mut mem = $interp.shared_memory.borrow_mut(); if let Some(new_size) = - crate::interpreter::memory::next_multiple_of_32(offset.saturating_add(len)) + crate::interpreter::shared_memory::next_multiple_of_32(offset.saturating_add(len)) { - #[cfg(feature = "memory_limit")] - if new_size > (mem.limit as usize) { + if new_size as u64 > mem.limit { $interp.instruction_result = InstructionResult::MemoryLimitOOG; return; } diff --git a/crates/interpreter/src/interpreter.rs b/crates/interpreter/src/interpreter.rs index c023f2522f..ef28fc61a7 100644 --- a/crates/interpreter/src/interpreter.rs +++ b/crates/interpreter/src/interpreter.rs @@ -1,11 +1,11 @@ pub mod analysis; mod contract; -pub(crate) mod memory; +pub(crate) mod shared_memory; mod stack; pub use analysis::BytecodeLocked; pub use contract::Contract; -pub use memory::Memory; +pub use shared_memory::SharedMemory; pub use stack::{Stack, STACK_LIMIT}; use crate::primitives::{Bytes, Spec}; @@ -17,7 +17,6 @@ use crate::{ use alloc::rc::Rc; use core::cell::RefCell; use core::ops::Range; -use revm_primitives::shared_memory::SharedMemory; pub const CALL_STACK_LIMIT: u64 = 1024; @@ -36,8 +35,8 @@ pub struct Interpreter { pub instruction_result: InstructionResult, /// left gas. Memory gas can be found in Memory field. pub gas: Gas, - /// Memory. - // pub memory: Memory, + /// Shared memory. + pub shared_memory: Rc>, /// Stack. pub stack: Stack, /// After call returns, its return data is saved here. @@ -51,8 +50,6 @@ pub struct Interpreter { /// Memory limit. See [`crate::CfgEnv`]. #[cfg(feature = "memory_limit")] pub memory_limit: u64, - - pub shared_memory: Rc>, } impl Interpreter { @@ -121,11 +118,6 @@ impl Interpreter { &self.gas } - /// Reference of interpreter memory. - // pub fn memory(&self) -> &Memory { - // &self.memory - // } - /// Reference of interpreter stack. pub fn stack(&self) -> &Stack { &self.stack @@ -190,7 +182,7 @@ impl Interpreter { self.return_range.end - self.return_range.start, )) }; - self.shared_memory.borrow_mut().free_memory(); + self.shared_memory.borrow_mut().free_context_memory(); bytes } } diff --git a/crates/interpreter/src/interpreter/memory.rs b/crates/interpreter/src/interpreter/memory.rs deleted file mode 100644 index 764fb3118e..0000000000 --- a/crates/interpreter/src/interpreter/memory.rs +++ /dev/null @@ -1,174 +0,0 @@ -use crate::{alloc::vec::Vec, primitives::U256}; -use core::{ - cmp::min, - ops::{BitAnd, Not}, -}; - -/// A sequential memory. It uses Rust's `Vec` for internal -/// representation. -#[derive(Clone, Debug, Eq, PartialEq)] -#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] -pub struct Memory { - data: Vec, -} - -impl Default for Memory { - fn default() -> Self { - Self { - data: Vec::with_capacity(4 * 1024), // took it from evmone - } - } -} - -impl Memory { - /// Create a new memory with the given limit. - pub fn new() -> Self { - Self { - data: Vec::with_capacity(4 * 1024), // took it from evmone - } - } - - pub fn effective_len(&self) -> usize { - self.data.len() - } - - /// Get the length of the current memory range. - pub fn len(&self) -> usize { - self.data.len() - } - - /// Return true if current effective memory range is zero. - pub fn is_empty(&self) -> bool { - self.len() == 0 - } - - /// Return the full memory. - pub fn data(&self) -> &Vec { - &self.data - } - - /// Consumes the type and returns the full memory. - pub fn into_data(self) -> Vec { - self.data - } - - /// Shrinks the capacity of the data buffer as much as possible. - pub fn shrink_to_fit(&mut self) { - self.data.shrink_to_fit() - } - - /// 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) { - self.data.resize(new_size, 0); - } - - /// Get memory region at given offset. Don't check offset and size - #[inline(always)] - pub fn get_slice(&self, offset: usize, size: usize) -> &[u8] { - &self.data[offset..offset + size] - } - - /// Set memory region at given offset - /// - /// # Safety - /// The caller is responsible for checking the offset and value - #[inline(always)] - pub unsafe fn set_byte(&mut self, index: usize, byte: u8) { - *self.data.get_mut(index).unwrap() = byte; - } - - #[inline(always)] - pub fn set_u256(&mut self, index: usize, value: U256) { - self.data[index..index + 32].copy_from_slice(&value.to_be_bytes::<{ U256::BYTES }>()); - } - - /// Set memory region at given offset. The offset and value are already checked - #[inline(always)] - pub fn set(&mut self, offset: usize, value: &[u8]) { - if !value.is_empty() { - self.data[offset..(value.len() + offset)].copy_from_slice(value); - } - } - - /// Set memory from data. Our memory offset+len is expected to be correct but we - /// are doing bound checks on data/data_offset/len and zeroing parts that is not copied. - #[inline(always)] - pub fn set_data(&mut self, memory_offset: usize, data_offset: usize, len: usize, data: &[u8]) { - if data_offset >= data.len() { - // nullify all memory slots - for i in &mut self.data[memory_offset..memory_offset + len] { - *i = 0; - } - return; - } - let data_end = min(data_offset + len, data.len()); - let memory_data_end = memory_offset + (data_end - data_offset); - self.data[memory_offset..memory_data_end].copy_from_slice(&data[data_offset..data_end]); - - // nullify rest of memory slots - // Safety: Memory is assumed to be valid. And it is commented where that assumption is made - for i in &mut self.data[memory_data_end..memory_offset + len] { - *i = 0; - } - } - - /// In memory copy given a src, dst, and length - /// - /// # Safety - /// The caller is responsible to check that we resized memory properly. - #[inline(always)] - pub fn copy(&mut self, dst: usize, src: usize, length: usize) { - self.data.copy_within(src..src + length, dst); - } -} - -/// Rounds up `x` to the closest multiple of 32. If `x % 32 == 0` then `x` is returned. -#[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 crate::Memory; - - use super::next_multiple_of_32; - - #[test] - fn test_copy() { - // Create a sample memory instance - let mut memory = Memory::new(); - - // Set up initial memory data - let data: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8, 9, 10]; - memory.resize(data.len()); - memory.set_data(0, 0, data.len(), &data); - - // Perform a copy operation - memory.copy(5, 0, 4); - - // Verify the copied data - let copied_data = memory.get_slice(5, 4); - assert_eq!(copied_data, &[1, 2, 3, 4]); - } - - #[test] - fn test_next_multiple_of_32() { - // next_multiple_of_32 returns x when it is a multiple of 32 - for i in 0..32 { - let x = i * 32; - assert_eq!(Some(x), next_multiple_of_32(x)); - } - - // next_multiple_of_32 rounds up to the nearest multiple of 32 when `x % 32 != 0` - for x in 0..1024 { - if x % 32 == 0 { - continue; - } - let next_multiple = x + 32 - (x % 32); - assert_eq!(Some(next_multiple), next_multiple_of_32(x)); - } - } -} diff --git a/crates/interpreter/src/interpreter/shared_memory.rs b/crates/interpreter/src/interpreter/shared_memory.rs new file mode 100644 index 0000000000..ad87ddf06c --- /dev/null +++ b/crates/interpreter/src/interpreter/shared_memory.rs @@ -0,0 +1,189 @@ +use revm_primitives::U256; + +use crate::alloc::vec; +use crate::alloc::vec::Vec; +use core::{ + cmp::min, + ops::{BitAnd, Not}, +}; + +/// A sequential memory shared between calls, which uses +/// a `Vec` for internal representation. +/// A [SharedMemory] instance should always be obtained using +/// the `new` static method to ensure memory safety. +pub struct SharedMemory { + /// Shared buffer + data: Vec, + /// Memory checkpoints for each depth + checkpoints: Vec, + /// Amount of memory left for assignment + pub limit: u64, + /// Raw pointer used for the portion of memory used + /// by the current context + current_slice: *mut [u8], + /// How much memory has been used in the current context + current_len: usize, +} + +impl SharedMemory { + /// Allocate memory to be shared between calls. + /// Memory size is estimated using https://2π.com/22/eth-max-mem + /// using transaction [gas_limit]; + pub fn new(gas_limit: u64, _memory_limit: Option) -> Self { + let upper_bound = 4096 * (2_f64 * gas_limit as f64).sqrt() as usize; + // let max_alloc_size = isize::MAX as usize; + let max_alloc_size = u32::MAX as usize; + let size = min(upper_bound, max_alloc_size); + + let mut data = vec![0; size]; + let checkpoints = Vec::with_capacity(1024); + let current_slice: *mut [u8] = &mut data[..]; + SharedMemory { + data, + limit: u64::MAX, + checkpoints, + current_slice, + current_len: 0, + } + } + /// Prepares the shared memory for a new context + pub fn new_context_memory(&mut self) { + let base_offset = self.checkpoints.last().unwrap_or(&0); + let new_checkpoint = base_offset + self.current_len; + + self.checkpoints.push(new_checkpoint); + + self.current_slice = &mut self.data[new_checkpoint..]; + self.current_len = 0; + } + + /// Prepares the shared memory for returning to the previous context + pub fn free_context_memory(&mut self) { + if let Some(old_checkpoint) = self.checkpoints.pop() { + let last = *self.checkpoints.last().unwrap_or(&0); + self.current_slice = &mut self.data[last..]; + self.current_len = old_checkpoint - last; + self.update_limit(); + } + } + + /// 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 + /// - we made new_size as multiply of 32 + /// - [new_size] is greater than `self.len()` + pub fn resize(&mut self, new_size: usize) { + // extend with zeros + let range = self.current_len..new_size; + + self.get_current_slice_mut()[range] + .iter_mut() + .for_each(|byte| *byte = 0); + self.current_len = new_size; + self.update_limit(); + } + + /// Get memory region at given offset. Dont check offset and size + #[inline(always)] + pub fn get_slice(&self, offset: usize, size: usize) -> &[u8] { + &self.get_current_slice()[offset..offset + size] + } + + /// Set memory region at given offset + /// + /// # Safety + /// The caller is responsible for checking the offset and value + #[inline(always)] + pub unsafe fn set_byte(&mut self, index: usize, byte: u8) { + self.get_current_slice_mut()[index] = byte; + } + + #[inline(always)] + pub fn set_u256(&mut self, index: usize, value: U256) { + self.get_current_slice_mut()[index..index + 32] + .copy_from_slice(&value.to_be_bytes::<{ U256::BYTES }>()); + } + + /// Set memory region at given offset. The offset and value are already checked + #[inline(always)] + pub fn set(&mut self, offset: usize, value: &[u8]) { + if !value.is_empty() { + self.get_current_slice_mut()[offset..(value.len() + offset)].copy_from_slice(value); + } + } + + /// Set memory from data. Our memory offset+len is expected to be correct but we + /// are doing bound checks on data/data_offeset/len and zeroing parts that is not copied. + #[inline(always)] + pub fn set_data(&mut self, memory_offset: usize, data_offset: usize, len: usize, data: &[u8]) { + if data_offset >= data.len() { + // nulify all memory slots + for i in &mut self.get_current_slice_mut()[memory_offset..memory_offset + len] { + *i = 0; + } + return; + } + let data_end = min(data_offset + len, data.len()); + let memory_data_end = memory_offset + (data_end - data_offset); + self.get_current_slice_mut()[memory_offset..memory_data_end] + .copy_from_slice(&data[data_offset..data_end]); + + // nulify rest of memory slots + // Safety: Memory is assumed to be valid. And it is commented where that assumption is made + for i in &mut self.get_current_slice_mut()[memory_data_end..memory_offset + len] { + *i = 0; + } + } + + /// In memory copy given a src, dst, and length + /// + /// # Safety + /// The caller is responsible to check that we resized memory properly. + #[inline(always)] + pub fn copy(&mut self, dst: usize, src: usize, length: usize) { + self.get_current_slice_mut() + .copy_within(src..src + length, dst); + } + + /// Get a refernce to the memory of the current context + #[inline(always)] + fn get_current_slice(&self) -> &[u8] { + // Safety: if it is a valid pointer to a slice of `self.data` + unsafe { &*self.current_slice } + } + + /// Get a mutable refernce to the memory of the current context + #[inline(always)] + fn get_current_slice_mut(&mut self) -> &mut [u8] { + // Safety: it is a valid pointer to a slice of `self.data` + unsafe { &mut *self.current_slice } + } + + /// Update the amount of memory left for usage + #[inline(always)] + fn update_limit(&mut self) { + self.limit = + (self.data.len() - *self.checkpoints.last().unwrap_or(&0) - self.current_len) as u64; + } +} + +/// Rounds up `x` to the closest multiple of 32. If `x % 32 == 0` then `x` is returned. +#[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) +} diff --git a/crates/interpreter/src/lib.rs b/crates/interpreter/src/lib.rs index a82107503d..e2e6e682c7 100644 --- a/crates/interpreter/src/lib.rs +++ b/crates/interpreter/src/lib.rs @@ -19,7 +19,7 @@ pub use inner_models::*; pub use instruction_result::InstructionResult; pub use instructions::opcode::{self, OpCode, OPCODE_JUMPMAP}; pub use interpreter::*; -pub use interpreter::{BytecodeLocked, Contract, Interpreter, Memory, Stack}; +pub use interpreter::{BytecodeLocked, Contract, Interpreter, Stack}; #[doc(inline)] pub use revm_primitives as primitives; diff --git a/crates/primitives/src/lib.rs b/crates/primitives/src/lib.rs index b57958fc69..6fe75693ef 100644 --- a/crates/primitives/src/lib.rs +++ b/crates/primitives/src/lib.rs @@ -10,7 +10,6 @@ pub mod env; pub mod log; pub mod precompile; pub mod result; -pub mod shared_memory; pub mod specification; pub mod state; pub mod utilities; diff --git a/crates/primitives/src/shared_memory.rs b/crates/primitives/src/shared_memory.rs deleted file mode 100644 index 715e941929..0000000000 --- a/crates/primitives/src/shared_memory.rs +++ /dev/null @@ -1,363 +0,0 @@ -use crate::alloc::vec; -use crate::alloc::vec::Vec; -use crate::U256; -use core::cmp::min; - -pub struct SharedMemory { - data: Vec, - pub limit: u64, - /// Memory sizes checkpoint for each depth - msizes: Vec, - current_slice: *mut [u8], - current_len: usize, -} - -impl SharedMemory { - fn get_current_slice(&self) -> &[u8] { - unsafe { &*self.current_slice } - } - - fn get_current_slice_mut(&mut self) -> &mut [u8] { - unsafe { &mut *self.current_slice } - } - - pub fn use_new_memory(&mut self) { - let base_offset = self.msizes.last().unwrap_or(&0); - let last_slice_offset = self.current_len; - - let new_msize = base_offset + last_slice_offset; - - self.msizes.push(new_msize); - - let range = new_msize..; - - self.current_slice = &mut self.data[range]; - self.current_len = 0; - } - - 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 { - 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: u64::MAX, - msizes, - current_slice, - current_len: 0, - } - } - - /// 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 { - panic!("Max limit reached") - } - - let range = if new_size > self.current_len { - // extend with zeros - self.current_len..new_size - } else { - // truncate - new_size..self.current_len - }; - - self.get_current_slice_mut()[range] - .iter_mut() - .for_each(|byte| *byte = 0); - self.current_len = new_size; - } - - /// Get memory region at given offset. Dont check offset and size - #[inline(always)] - pub fn get_slice(&self, offset: usize, size: usize) -> &[u8] { - &self.get_current_slice()[offset..offset + size] - } - - /// Set memory region at given offset - /// - /// # Safety - /// The caller is responsible for checking the offset and value - #[inline(always)] - pub unsafe fn set_byte(&mut self, index: usize, byte: u8) { - self.get_current_slice_mut()[index] = byte; - } - - #[inline(always)] - pub fn set_u256(&mut self, index: usize, value: U256) { - self.get_current_slice_mut()[index..index + 32] - .copy_from_slice(&value.to_be_bytes::<{ U256::BYTES }>()); - } - - /// Set memory region at given offset. The offset and value are already checked - #[inline(always)] - pub fn set(&mut self, offset: usize, value: &[u8]) { - if !value.is_empty() { - self.get_current_slice_mut()[offset..(value.len() + offset)].copy_from_slice(value); - } - } - - /// Set memory from data. Our memory offset+len is expected to be correct but we - /// are doing bound checks on data/data_offeset/len and zeroing parts that is not copied. - #[inline(always)] - pub fn set_data(&mut self, memory_offset: usize, data_offset: usize, len: usize, data: &[u8]) { - if data_offset >= data.len() { - // nulify all memory slots - for i in &mut self.get_current_slice_mut()[memory_offset..memory_offset + len] { - *i = 0; - } - return; - } - let data_end = min(data_offset + len, data.len()); - let memory_data_end = memory_offset + (data_end - data_offset); - self.get_current_slice_mut()[memory_offset..memory_data_end] - .copy_from_slice(&data[data_offset..data_end]); - - // nulify rest of memory slots - // Safety: Memory is assumed to be valid. And it is commented where that assumption is made - for i in &mut self.get_current_slice_mut()[memory_data_end..memory_offset + len] { - *i = 0; - } - } - - /// In memory copy given a src, dst, and length - /// - /// # Safety - /// The caller is responsible to check that we resized memory properly. - #[inline(always)] - pub fn copy(&mut self, dst: usize, src: usize, length: usize) { - self.get_current_slice_mut() - .copy_within(src..src + length, dst); - } -} - -// #[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/benches/bench.rs b/crates/revm/benches/bench.rs index f7ff90c87b..de1905f1de 100644 --- a/crates/revm/benches/bench.rs +++ b/crates/revm/benches/bench.rs @@ -7,7 +7,7 @@ use revm::{ interpreter::{analysis::to_analysed, BytecodeLocked, Contract, DummyHost, Interpreter}, primitives::{BerlinSpec, Bytecode, BytecodeState, TransactTo, U256}, }; -use revm_interpreter::primitives::shared_memory::SharedMemory; +use revm_interpreter::SharedMemory; use std::{cell::RefCell, rc::Rc, time::Duration}; type Evm = revm::EVM; @@ -104,6 +104,7 @@ fn bench_transact(g: &mut BenchmarkGroup<'_, WallTime>, evm: &mut Evm) { } fn bench_eval(g: &mut BenchmarkGroup<'_, WallTime>, evm: &Evm) { + let shared_memory = Rc::new(RefCell::new(SharedMemory::new(evm.env.tx.gas_limit, None))); g.bench_function("eval", |b| { let contract = Contract { input: evm.env.tx.data.clone(), @@ -111,7 +112,6 @@ fn bench_eval(g: &mut BenchmarkGroup<'_, WallTime>, evm: &Evm) { ..Default::default() }; let mut host = DummyHost::new(evm.env.clone()); - let shared_memory = Rc::new(RefCell::new(SharedMemory::new(evm.env.tx.gas_limit, None))); b.iter(|| { let mut interpreter = Interpreter::new(Box::new(contract.clone()), u64::MAX, false, &shared_memory); diff --git a/crates/revm/src/evm_impl.rs b/crates/revm/src/evm_impl.rs index 17315e7c7b..59a887b680 100644 --- a/crates/revm/src/evm_impl.rs +++ b/crates/revm/src/evm_impl.rs @@ -18,8 +18,7 @@ use alloc::vec::Vec; use core::cell::RefCell; use core::{cmp::min, marker::PhantomData}; use revm_interpreter::gas::initial_tx_gas; -use revm_interpreter::primitives::shared_memory::SharedMemory; -use revm_interpreter::MAX_CODE_SIZE; +use revm_interpreter::{SharedMemory, MAX_CODE_SIZE}; use revm_precompile::{Precompile, Precompiles}; pub struct EVMData<'a, DB: Database> { @@ -597,7 +596,7 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, shared_memory, )); - interpreter.shared_memory.borrow_mut().use_new_memory(); + interpreter.shared_memory.borrow_mut().new_context_memory(); if INSPECT { self.inspector diff --git a/crates/revm/src/inspector/tracer_eip3155.rs b/crates/revm/src/inspector/tracer_eip3155.rs index 7f8f9f8a22..216f7447aa 100644 --- a/crates/revm/src/inspector/tracer_eip3155.rs +++ b/crates/revm/src/inspector/tracer_eip3155.rs @@ -5,7 +5,7 @@ use crate::interpreter::{CallInputs, CreateInputs, Gas, InstructionResult}; use crate::primitives::{db::Database, hex, Bytes, B160}; use crate::{evm_impl::EVMData, Inspector}; use revm_interpreter::primitives::U256; -use revm_interpreter::{opcode, Interpreter, Memory, Stack}; +use revm_interpreter::{opcode, Interpreter, SharedMemory, Stack}; use serde_json::json; use std::io::Write; @@ -24,7 +24,7 @@ pub struct TracerEip3155 { gas: u64, mem_size: usize, #[allow(dead_code)] - memory: Option, + memory: Option, skip: bool, } From 2bcddef9a24b94da3bf4d05933b10c4fae6e68a9 Mon Sep 17 00:00:00 2001 From: Lorenzo Feroleto Date: Wed, 30 Aug 2023 19:45:35 +0200 Subject: [PATCH 06/19] chore(shared_memory): memory limit feature management --- crates/interpreter/src/interpreter.rs | 37 +------------- .../src/interpreter/shared_memory.rs | 51 +++++++++++++++---- crates/revm/src/evm_impl.rs | 22 ++++---- 3 files changed, 51 insertions(+), 59 deletions(-) diff --git a/crates/interpreter/src/interpreter.rs b/crates/interpreter/src/interpreter.rs index ef28fc61a7..5e28475087 100644 --- a/crates/interpreter/src/interpreter.rs +++ b/crates/interpreter/src/interpreter.rs @@ -47,9 +47,6 @@ pub struct Interpreter { pub is_static: bool, /// Contract information and invoking data pub contract: Box, - /// Memory limit. See [`crate::CfgEnv`]. - #[cfg(feature = "memory_limit")] - pub memory_limit: u64, } impl Interpreter { @@ -64,49 +61,17 @@ impl Interpreter { gas_limit: u64, is_static: bool, shared_memory: &Rc>, - ) -> Self { - #[cfg(not(feature = "memory_limit"))] - { - Self { - instruction_pointer: contract.bytecode.as_ptr(), - return_range: Range::default(), - // memory: Memory::new(), - stack: Stack::new(), - return_data_buffer: Bytes::new(), - contract, - instruction_result: InstructionResult::Continue, - is_static, - gas: Gas::new(gas_limit), - shared_memory: Rc::clone(shared_memory), - } - } - - #[cfg(feature = "memory_limit")] - { - Self::new_with_memory_limit(contract, gas_limit, is_static, u64::MAX, shared_memory) - } - } - - #[cfg(feature = "memory_limit")] - pub fn new_with_memory_limit( - contract: Box, - gas_limit: u64, - is_static: bool, - memory_limit: u64, - shared_memory: &Rc>, ) -> Self { Self { instruction_pointer: contract.bytecode.as_ptr(), return_range: Range::default(), - // memory: Memory::new(), stack: Stack::new(), + shared_memory: Rc::clone(shared_memory), return_data_buffer: Bytes::new(), contract, instruction_result: InstructionResult::Continue, is_static, gas: Gas::new(gas_limit), - memory_limit, - shared_memory: Rc::clone(shared_memory), } } diff --git a/crates/interpreter/src/interpreter/shared_memory.rs b/crates/interpreter/src/interpreter/shared_memory.rs index ad87ddf06c..c2e7bd67e1 100644 --- a/crates/interpreter/src/interpreter/shared_memory.rs +++ b/crates/interpreter/src/interpreter/shared_memory.rs @@ -16,36 +16,58 @@ pub struct SharedMemory { data: Vec, /// Memory checkpoints for each depth checkpoints: Vec, - /// Amount of memory left for assignment - pub limit: u64, /// Raw pointer used for the portion of memory used /// by the current context current_slice: *mut [u8], /// How much memory has been used in the current context current_len: usize, + /// Amount of memory left for assignment + pub limit: u64, } impl SharedMemory { /// Allocate memory to be shared between calls. /// Memory size is estimated using https://2π.com/22/eth-max-mem - /// using transaction [gas_limit]; - pub fn new(gas_limit: u64, _memory_limit: Option) -> Self { - let upper_bound = 4096 * (2_f64 * gas_limit as f64).sqrt() as usize; - // let max_alloc_size = isize::MAX as usize; - let max_alloc_size = u32::MAX as usize; - let size = min(upper_bound, max_alloc_size); + /// which depends on transaction [gas_limit]. + /// Maximum allocation size is 2^32 - 1 bytes; + pub fn new(gas_limit: u64) -> Self { + let size = min( + SharedMemory::calculate_upper_bound(gas_limit) as usize, + 2usize.pow(32) - 1, + ); + + let mut data = vec![0; size]; + let current_slice: *mut [u8] = &mut data[..]; + SharedMemory { + data, + checkpoints: Vec::with_capacity(1024), + current_slice, + current_len: 0, + limit: size as u64, + } + } + + /// Allocate memory to be shared between calls. + /// Memory size is estimated using https://2π.com/22/eth-max-mem + /// which depends on transaction [gas_limit]. + /// Uses [memory_limit] as maximum allocation size + pub fn new_with_memory_limit(gas_limit: u64, memory_limit: u64) -> Self { + let size = min( + SharedMemory::calculate_upper_bound(gas_limit) as usize, + memory_limit as usize, + ); let mut data = vec![0; size]; - let checkpoints = Vec::with_capacity(1024); let current_slice: *mut [u8] = &mut data[..]; SharedMemory { data, - limit: u64::MAX, - checkpoints, + checkpoints: Vec::with_capacity(1024), current_slice, current_len: 0, + limit: size as u64, } } + /// Prepares the shared memory for a new context pub fn new_context_memory(&mut self) { let base_offset = self.checkpoints.last().unwrap_or(&0); @@ -173,6 +195,13 @@ impl SharedMemory { unsafe { &mut *self.current_slice } } + /// Calculates memory allocation upper bound using + /// https://2π.com/22/eth-max-mem + #[inline(always)] + fn calculate_upper_bound(gas_limit: u64) -> u64 { + 4096 * ((2 * gas_limit) as f64).sqrt() as u64 + } + /// Update the amount of memory left for usage #[inline(always)] fn update_limit(&mut self) { diff --git a/crates/revm/src/evm_impl.rs b/crates/revm/src/evm_impl.rs index 59a887b680..60e82890ac 100644 --- a/crates/revm/src/evm_impl.rs +++ b/crates/revm/src/evm_impl.rs @@ -123,6 +123,8 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> Transact fn transact_preverified(&mut self) -> EVMResult { let env = &self.data.env; + #[cfg(feature = "memory_limit")] + let memory_limit = env.cfg.memory_limit; let tx_caller = env.tx.caller; let tx_value = env.tx.value; let tx_data = env.tx.data.clone(); @@ -162,7 +164,14 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> Transact let transact_gas_limit = tx_gas_limit - initial_gas_spend; - let shared_memory = Rc::new(RefCell::new(SharedMemory::new(transact_gas_limit, None))); + #[cfg(feature = "memory_limit")] + let shared_memory = Rc::new(RefCell::new(SharedMemory::new_with_memory_limit( + transact_gas_limit, + memory_limit, + ))); + + #[cfg(not(feature = "memory_limit"))] + let shared_memory = Rc::new(RefCell::new(SharedMemory::new(transact_gas_limit))); // call inner handling of call/create let (exit_reason, ret_gas, output) = match self.data.env.tx.transact_to { @@ -578,17 +587,6 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, is_static: bool, shared_memory: &Rc>, ) -> (InstructionResult, Box) { - // Create inspector - #[cfg(feature = "memory_limit")] - let mut interpreter = Box::new(Interpreter::new_with_memory_limit( - contract, - gas_limit, - is_static, - self.data.env.cfg.memory_limit, - shared_memory, - )); - - #[cfg(not(feature = "memory_limit"))] let mut interpreter = Box::new(Interpreter::new( contract, gas_limit, From bdd1cccf4f564933319be9eec252301b0823f87d Mon Sep 17 00:00:00 2001 From: Lorenzo Feroleto Date: Thu, 31 Aug 2023 09:17:32 +0200 Subject: [PATCH 07/19] chore(shared_memory): make benchmarks use right amount of gas for proper memory allocation --- crates/revm/benches/bench.rs | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/crates/revm/benches/bench.rs b/crates/revm/benches/bench.rs index de1905f1de..df1f4ece0e 100644 --- a/crates/revm/benches/bench.rs +++ b/crates/revm/benches/bench.rs @@ -23,6 +23,8 @@ fn analysis(c: &mut Criterion) { .parse() .unwrap(), ); + // Empirical good value for gas_limit for low shared_memory usage + evm.env.tx.gas_limit = 22_000; // evm.env.tx.data = Bytes::from(hex::decode("30627b7c").unwrap()); evm.env.tx.data = Bytes::from(hex::decode("8035F0CE").unwrap()); @@ -69,7 +71,7 @@ fn snailtracer(c: &mut Criterion) { .measurement_time(Duration::from_secs(10)) .sample_size(10); bench_transact(&mut g, &mut evm); - bench_eval(&mut g, &evm); + bench_eval(&mut g, &mut evm); g.finish(); } @@ -100,11 +102,20 @@ fn bench_transact(g: &mut BenchmarkGroup<'_, WallTime>, evm: &mut Evm) { BytecodeState::Analysed { .. } => "analysed", }; let id = format!("transact/{state}"); - g.bench_function(id, |b| b.iter(|| evm.transact().unwrap())); + g.bench_function(id, |b| { + b.iter(|| { + // reset gas limit to the right amount before tx + evm.env.tx.gas_limit = 22_000; + evm.transact().unwrap() + }) + }); } -fn bench_eval(g: &mut BenchmarkGroup<'_, WallTime>, evm: &Evm) { - let shared_memory = Rc::new(RefCell::new(SharedMemory::new(evm.env.tx.gas_limit, None))); +fn bench_eval(g: &mut BenchmarkGroup<'_, WallTime>, evm: &mut Evm) { + let gas_limit = 22_000; + evm.env.tx.gas_limit = gas_limit; + let shared_memory = Rc::new(RefCell::new(SharedMemory::new(evm.env.tx.gas_limit))); + g.bench_function("eval", |b| { let contract = Contract { input: evm.env.tx.data.clone(), @@ -113,6 +124,8 @@ fn bench_eval(g: &mut BenchmarkGroup<'_, WallTime>, evm: &Evm) { }; let mut host = DummyHost::new(evm.env.clone()); b.iter(|| { + // reset gas limit to the right amount before tx + evm.env.tx.gas_limit = gas_limit; let mut interpreter = Interpreter::new(Box::new(contract.clone()), u64::MAX, false, &shared_memory); let res = interpreter.run::<_, BerlinSpec>(&mut host); From e91224e0bfe8969f516717980e364d2f880c9637 Mon Sep 17 00:00:00 2001 From: Lorenzo Feroleto Date: Thu, 31 Aug 2023 18:32:29 +0200 Subject: [PATCH 08/19] chore(shared_memory): formatting --- crates/revm/src/evm_impl.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/crates/revm/src/evm_impl.rs b/crates/revm/src/evm_impl.rs index 60e82890ac..0515ba1bdf 100644 --- a/crates/revm/src/evm_impl.rs +++ b/crates/revm/src/evm_impl.rs @@ -743,11 +743,10 @@ 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, + return_value: interpreter.return_value(), } } else { CallResult { From ed4079f725e7fdacb3c26849a8baaf5ce525e498 Mon Sep 17 00:00:00 2001 From: Lorenzo Feroleto Date: Fri, 1 Sep 2023 09:12:51 +0200 Subject: [PATCH 09/19] chore(shared_memory): improved shared_memory! macro --- crates/interpreter/src/instructions/host.rs | 29 +++++------------ crates/interpreter/src/instructions/macros.rs | 15 +++++---- crates/interpreter/src/instructions/memory.rs | 31 +++++-------------- crates/interpreter/src/instructions/system.rs | 21 +++++-------- 4 files changed, 33 insertions(+), 63 deletions(-) diff --git a/crates/interpreter/src/instructions/host.rs b/crates/interpreter/src/instructions/host.rs index 89fce027ab..8cdd7ba2e8 100644 --- a/crates/interpreter/src/instructions/host.rs +++ b/crates/interpreter/src/instructions/host.rs @@ -113,13 +113,10 @@ pub fn extcodecopy(interpreter: &mut Interpreter, host: &mut dyn Hos InstructionResult::InvalidOperandOOG ); let code_offset = min(as_usize_saturated!(code_offset), code.len()); - shared_memory_resize!(interpreter, memory_offset, len); + shared_memory_resize!(interpreter, memory_offset, len, memory); // Safety: set_data is unsafe function and memory_resize ensures us that it is safe to call it - interpreter - .shared_memory - .borrow_mut() - .set_data(memory_offset, code_offset, len, code.bytes()); + memory.set_data(memory_offset, code_offset, len, code.bytes()); } pub fn blockhash(interpreter: &mut Interpreter, host: &mut dyn Host) { @@ -202,8 +199,8 @@ pub fn log(interpreter: &mut Interpreter, host: &mut dyn Host) { Bytes::new() } else { let offset = as_usize_or_fail!(interpreter, offset, InstructionResult::InvalidOperandOOG); - shared_memory_resize!(interpreter, offset, len); - Bytes::copy_from_slice(interpreter.shared_memory.borrow().get_slice(offset, len)) + shared_memory_resize!(interpreter, offset, len, memory); + Bytes::copy_from_slice(memory.get_slice(offset, len)) }; let n = N as usize; if interpreter.stack.len() < n { @@ -279,13 +276,8 @@ pub fn prepare_create_inputs( } gas!(interpreter, gas::initcode_cost(len as u64)); } - shared_memory_resize!(interpreter, code_offset, len); - Bytes::copy_from_slice( - interpreter - .shared_memory - .borrow() - .get_slice(code_offset, len), - ) + shared_memory_resize!(interpreter, code_offset, len, memory); + Bytes::copy_from_slice(memory.get_slice(code_offset, len)) }; let scheme = if IS_CREATE2 { @@ -409,13 +401,8 @@ fn prepare_call_inputs( let input = if in_len != 0 { let in_offset = as_usize_or_fail!(interpreter, in_offset, InstructionResult::InvalidOperandOOG); - shared_memory_resize!(interpreter, in_offset, in_len); - Bytes::copy_from_slice( - interpreter - .shared_memory - .borrow() - .get_slice(in_offset, in_len), - ) + shared_memory_resize!(interpreter, in_offset, in_len, memory); + Bytes::copy_from_slice(memory.get_slice(in_offset, in_len)) } else { Bytes::new() }; diff --git a/crates/interpreter/src/instructions/macros.rs b/crates/interpreter/src/instructions/macros.rs index a3d988c5b9..16866f7b78 100644 --- a/crates/interpreter/src/instructions/macros.rs +++ b/crates/interpreter/src/instructions/macros.rs @@ -50,19 +50,22 @@ macro_rules! gas_or_fail { } macro_rules! shared_memory_resize { - ($interp:expr, $offset:expr, $len:expr) => {{ + ($interp:expr, $offset:expr, $len:expr) => { + shared_memory_resize!($interp, $offset, $len, memory); + }; + ($interp:expr, $offset:expr, $len:expr, $memory: ident) => { let len: usize = $len; let offset: usize = $offset; - let mut mem = $interp.shared_memory.borrow_mut(); + let mut $memory = $interp.shared_memory.borrow_mut(); if let Some(new_size) = crate::interpreter::shared_memory::next_multiple_of_32(offset.saturating_add(len)) { - if new_size as u64 > mem.limit { + if new_size as u64 > $memory.limit { $interp.instruction_result = InstructionResult::MemoryLimitOOG; return; } - if new_size > mem.len() { + if new_size > $memory.len() { if crate::USE_GAS { let num_bytes = new_size / 32; if !$interp.gas.record_memory(crate::gas::memory_gas(num_bytes)) { @@ -70,13 +73,13 @@ macro_rules! shared_memory_resize { return; } } - mem.resize(new_size); + $memory.resize(new_size); } } else { $interp.instruction_result = InstructionResult::MemoryOOG; return; } - };}; + }; } macro_rules! pop_address { diff --git a/crates/interpreter/src/instructions/memory.rs b/crates/interpreter/src/instructions/memory.rs index 3791043f2d..d9164be483 100644 --- a/crates/interpreter/src/instructions/memory.rs +++ b/crates/interpreter/src/instructions/memory.rs @@ -13,17 +13,10 @@ pub fn mload(interpreter: &mut Interpreter, _host: &mut dyn Host) { gas!(interpreter, gas::VERYLOW); pop!(interpreter, index); let index = as_usize_or_fail!(interpreter, index, InstructionResult::InvalidOperandOOG); - shared_memory_resize!(interpreter, index, 32); + shared_memory_resize!(interpreter, index, 32, memory); push!( interpreter, - U256::from_be_bytes::<{ U256::BYTES }>( - interpreter - .shared_memory - .borrow() - .get_slice(index, 32) - .try_into() - .unwrap() - ) + U256::from_be_bytes::<{ U256::BYTES }>(memory.get_slice(index, 32).try_into().unwrap()) ); } @@ -31,26 +24,18 @@ pub fn mstore(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); - interpreter - .shared_memory - .borrow_mut() - .set_u256(index, value); + shared_memory_resize!(interpreter, index, 32, memory); + memory.set_u256(index, value); } 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, 1); + shared_memory_resize!(interpreter, index, 1, memory); let value = value.as_le_bytes()[0]; // Safety: we resized our memory two lines above. - unsafe { - interpreter - .shared_memory - .borrow_mut() - .set_byte(index, value) - } + unsafe { memory.set_byte(index, value) } } pub fn msize(interpreter: &mut Interpreter, _host: &mut dyn Host) { @@ -80,7 +65,7 @@ pub fn mcopy(interpreter: &mut Interpreter, _host: &mut dyn Host) { let dest = as_usize_or_fail!(interpreter, dest, InstructionResult::InvalidOperandOOG); let src = as_usize_or_fail!(interpreter, src, InstructionResult::InvalidOperandOOG); // resize memory - shared_memory_resize!(interpreter, max(dest, src), len); + shared_memory_resize!(interpreter, max(dest, src), len, memory); // copy memory in place - interpreter.shared_memory.borrow_mut().copy(dest, src, len); + memory.copy(dest, src, len); } diff --git a/crates/interpreter/src/instructions/system.rs b/crates/interpreter/src/instructions/system.rs index 0be0df5d8a..3f8efdaf94 100644 --- a/crates/interpreter/src/instructions/system.rs +++ b/crates/interpreter/src/instructions/system.rs @@ -14,8 +14,8 @@ pub fn calculate_keccak256(interpreter: &mut Interpreter, _host: &mut dyn Host) KECCAK_EMPTY } else { let from = as_usize_or_fail!(interpreter, from, InstructionResult::InvalidOperandOOG); - shared_memory_resize!(interpreter, from, len); - keccak256(interpreter.shared_memory.borrow().get_slice(from, len)) + shared_memory_resize!(interpreter, from, len, memory); + keccak256(memory.get_slice(from, len)) }; push_b256!(interpreter, hash); @@ -49,10 +49,10 @@ pub fn codecopy(interpreter: &mut Interpreter, _host: &mut dyn Host) { InstructionResult::InvalidOperandOOG ); let code_offset = as_usize_saturated!(code_offset); - shared_memory_resize!(interpreter, memory_offset, len); + shared_memory_resize!(interpreter, memory_offset, len, memory); // Safety: set_data is unsafe function and memory_resize ensures us that it is safe to call it - interpreter.shared_memory.borrow_mut().set_data( + memory.set_data( memory_offset, code_offset, len, @@ -100,15 +100,10 @@ pub fn calldatacopy(interpreter: &mut Interpreter, _host: &mut dyn Host) { InstructionResult::InvalidOperandOOG ); let data_offset = as_usize_saturated!(data_offset); - shared_memory_resize!(interpreter, memory_offset, len); + shared_memory_resize!(interpreter, memory_offset, len, memory); // Safety: set_data is unsafe function and memory_resize ensures us that it is safe to call it - interpreter.shared_memory.borrow_mut().set_data( - memory_offset, - data_offset, - len, - &interpreter.contract.input, - ); + memory.set_data(memory_offset, data_offset, len, &interpreter.contract.input); } pub fn returndatasize(interpreter: &mut Interpreter, _host: &mut dyn Host) { @@ -139,8 +134,8 @@ pub fn returndatacopy(interpreter: &mut Interpreter, _host: &mut dyn memory_offset, InstructionResult::InvalidOperandOOG ); - shared_memory_resize!(interpreter, memory_offset, len); - interpreter.shared_memory.borrow_mut().set( + shared_memory_resize!(interpreter, memory_offset, len, memory); + memory.set( memory_offset, &interpreter.return_data_buffer[data_offset..data_end], ); From 3c5946ce1ded5d2db2c07ea6a0985a77fe99cb50 Mon Sep 17 00:00:00 2001 From: Lorenzo Feroleto Date: Tue, 5 Sep 2023 12:07:09 +0200 Subject: [PATCH 10/19] chore(shared_memory): replace Rc>, + shared_memory: &mut SharedMemory, ) -> (InstructionResult, Option, Gas, Bytes); /// Invoke a call operation. fn call( &mut self, input: &mut CallInputs, - shared_memory: &Rc>, + shared_memory: &mut SharedMemory, ) -> (InstructionResult, Gas, Bytes); } diff --git a/crates/interpreter/src/host/dummy.rs b/crates/interpreter/src/host/dummy.rs index 7c0dab6a1a..a65aa85cd8 100644 --- a/crates/interpreter/src/host/dummy.rs +++ b/crates/interpreter/src/host/dummy.rs @@ -1,12 +1,9 @@ -use core::cell::RefCell; - use crate::primitives::{hash_map::Entry, Bytecode, Bytes, HashMap, U256}; use crate::{ primitives::{Env, Log, B160, B256, KECCAK_EMPTY}, CallInputs, CreateInputs, Gas, Host, InstructionResult, Interpreter, SelfDestructResult, SharedMemory, }; -use alloc::rc::Rc; use alloc::vec::Vec; pub struct DummyHost { @@ -141,7 +138,7 @@ impl Host for DummyHost { fn create( &mut self, _inputs: &mut CreateInputs, - _shared_memory: &Rc>, + _shared_memory: &mut SharedMemory, ) -> (InstructionResult, Option, Gas, Bytes) { panic!("Create is not supported for this host") } @@ -150,7 +147,7 @@ impl Host for DummyHost { fn call( &mut self, _input: &mut CallInputs, - _shared_memory: &Rc>, + _shared_memory: &mut SharedMemory, ) -> (InstructionResult, Gas, Bytes) { panic!("Call is not supported for this host") } diff --git a/crates/interpreter/src/instructions/host.rs b/crates/interpreter/src/instructions/host.rs index 8cdd7ba2e8..7ecc6e3613 100644 --- a/crates/interpreter/src/instructions/host.rs +++ b/crates/interpreter/src/instructions/host.rs @@ -113,10 +113,12 @@ pub fn extcodecopy(interpreter: &mut Interpreter, host: &mut dyn Hos InstructionResult::InvalidOperandOOG ); let code_offset = min(as_usize_saturated!(code_offset), code.len()); - shared_memory_resize!(interpreter, memory_offset, len, memory); + shared_memory_resize!(interpreter, memory_offset, len); // Safety: set_data is unsafe function and memory_resize ensures us that it is safe to call it - memory.set_data(memory_offset, code_offset, len, code.bytes()); + interpreter + .shared_memory + .set_data(memory_offset, code_offset, len, code.bytes()); } pub fn blockhash(interpreter: &mut Interpreter, host: &mut dyn Host) { @@ -199,8 +201,8 @@ pub fn log(interpreter: &mut Interpreter, host: &mut dyn Host) { Bytes::new() } else { let offset = as_usize_or_fail!(interpreter, offset, InstructionResult::InvalidOperandOOG); - shared_memory_resize!(interpreter, offset, len, memory); - Bytes::copy_from_slice(memory.get_slice(offset, len)) + shared_memory_resize!(interpreter, offset, len); + Bytes::copy_from_slice(interpreter.shared_memory.get_slice(offset, len)) }; let n = N as usize; if interpreter.stack.len() < n { @@ -276,8 +278,8 @@ pub fn prepare_create_inputs( } gas!(interpreter, gas::initcode_cost(len as u64)); } - shared_memory_resize!(interpreter, code_offset, len, memory); - Bytes::copy_from_slice(memory.get_slice(code_offset, len)) + shared_memory_resize!(interpreter, code_offset, len); + Bytes::copy_from_slice(interpreter.shared_memory.get_slice(code_offset, len)) }; let scheme = if IS_CREATE2 { @@ -319,7 +321,7 @@ pub fn create( }; let (return_reason, address, gas, return_data) = - host.create(&mut create_input, &interpreter.shared_memory); + host.create(&mut create_input, interpreter.shared_memory); interpreter.return_data_buffer = match return_reason { // Save data to return data buffer if the create reverted @@ -401,8 +403,8 @@ fn prepare_call_inputs( let input = if in_len != 0 { let in_offset = as_usize_or_fail!(interpreter, in_offset, InstructionResult::InvalidOperandOOG); - shared_memory_resize!(interpreter, in_offset, in_len, memory); - Bytes::copy_from_slice(memory.get_slice(in_offset, in_len)) + shared_memory_resize!(interpreter, in_offset, in_len); + Bytes::copy_from_slice(interpreter.shared_memory.get_slice(in_offset, in_len)) } else { Bytes::new() }; @@ -539,7 +541,7 @@ pub fn call_inner( }; // Call host to interact with target contract - let (reason, gas, return_data) = host.call(&mut call_input, &interpreter.shared_memory); + 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()); @@ -553,7 +555,6 @@ pub fn call_inner( } interpreter .shared_memory - .borrow_mut() .set(out_offset, &interpreter.return_data_buffer[..target_len]); push!(interpreter, U256::from(1)); } @@ -563,7 +564,6 @@ pub fn call_inner( } interpreter .shared_memory - .borrow_mut() .set(out_offset, &interpreter.return_data_buffer[..target_len]); push!(interpreter, U256::ZERO); } diff --git a/crates/interpreter/src/instructions/macros.rs b/crates/interpreter/src/instructions/macros.rs index 16866f7b78..180ca21c72 100644 --- a/crates/interpreter/src/instructions/macros.rs +++ b/crates/interpreter/src/instructions/macros.rs @@ -51,21 +51,17 @@ macro_rules! gas_or_fail { macro_rules! shared_memory_resize { ($interp:expr, $offset:expr, $len:expr) => { - shared_memory_resize!($interp, $offset, $len, memory); - }; - ($interp:expr, $offset:expr, $len:expr, $memory: ident) => { let len: usize = $len; let offset: usize = $offset; - let mut $memory = $interp.shared_memory.borrow_mut(); if let Some(new_size) = crate::interpreter::shared_memory::next_multiple_of_32(offset.saturating_add(len)) { - if new_size as u64 > $memory.limit { + if new_size as u64 > $interp.shared_memory.limit { $interp.instruction_result = InstructionResult::MemoryLimitOOG; return; } - if new_size > $memory.len() { + if new_size > $interp.shared_memory.len() { if crate::USE_GAS { let num_bytes = new_size / 32; if !$interp.gas.record_memory(crate::gas::memory_gas(num_bytes)) { @@ -73,7 +69,7 @@ macro_rules! shared_memory_resize { return; } } - $memory.resize(new_size); + $interp.shared_memory.resize(new_size); } } else { $interp.instruction_result = InstructionResult::MemoryOOG; diff --git a/crates/interpreter/src/instructions/memory.rs b/crates/interpreter/src/instructions/memory.rs index d9164be483..4dfbe430ce 100644 --- a/crates/interpreter/src/instructions/memory.rs +++ b/crates/interpreter/src/instructions/memory.rs @@ -13,10 +13,16 @@ pub fn mload(interpreter: &mut Interpreter, _host: &mut dyn Host) { gas!(interpreter, gas::VERYLOW); pop!(interpreter, index); let index = as_usize_or_fail!(interpreter, index, InstructionResult::InvalidOperandOOG); - shared_memory_resize!(interpreter, index, 32, memory); + shared_memory_resize!(interpreter, index, 32); push!( interpreter, - U256::from_be_bytes::<{ U256::BYTES }>(memory.get_slice(index, 32).try_into().unwrap()) + U256::from_be_bytes::<{ U256::BYTES }>( + interpreter + .shared_memory + .get_slice(index, 32) + .try_into() + .unwrap() + ) ); } @@ -24,26 +30,23 @@ pub fn mstore(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, memory); - memory.set_u256(index, value); + shared_memory_resize!(interpreter, index, 32); + interpreter.shared_memory.set_u256(index, value); } 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, 1, memory); + shared_memory_resize!(interpreter, index, 1); let value = value.as_le_bytes()[0]; // Safety: we resized our memory two lines above. - unsafe { memory.set_byte(index, value) } + unsafe { interpreter.shared_memory.set_byte(index, value) } } pub fn msize(interpreter: &mut Interpreter, _host: &mut dyn Host) { gas!(interpreter, gas::BASE); - push!( - interpreter, - U256::from(interpreter.shared_memory.borrow().len()) - ); + push!(interpreter, U256::from(interpreter.shared_memory.len())); } // From EIP-5656 MCOPY @@ -65,7 +68,7 @@ pub fn mcopy(interpreter: &mut Interpreter, _host: &mut dyn Host) { let dest = as_usize_or_fail!(interpreter, dest, InstructionResult::InvalidOperandOOG); let src = as_usize_or_fail!(interpreter, src, InstructionResult::InvalidOperandOOG); // resize memory - shared_memory_resize!(interpreter, max(dest, src), len, memory); + shared_memory_resize!(interpreter, max(dest, src), len); // copy memory in place - memory.copy(dest, src, len); + interpreter.shared_memory.copy(dest, src, len); } diff --git a/crates/interpreter/src/instructions/system.rs b/crates/interpreter/src/instructions/system.rs index 3f8efdaf94..4e28d8415f 100644 --- a/crates/interpreter/src/instructions/system.rs +++ b/crates/interpreter/src/instructions/system.rs @@ -14,8 +14,8 @@ pub fn calculate_keccak256(interpreter: &mut Interpreter, _host: &mut dyn Host) KECCAK_EMPTY } else { let from = as_usize_or_fail!(interpreter, from, InstructionResult::InvalidOperandOOG); - shared_memory_resize!(interpreter, from, len, memory); - keccak256(memory.get_slice(from, len)) + shared_memory_resize!(interpreter, from, len); + keccak256(interpreter.shared_memory.get_slice(from, len)) }; push_b256!(interpreter, hash); @@ -49,10 +49,10 @@ pub fn codecopy(interpreter: &mut Interpreter, _host: &mut dyn Host) { InstructionResult::InvalidOperandOOG ); let code_offset = as_usize_saturated!(code_offset); - shared_memory_resize!(interpreter, memory_offset, len, memory); + shared_memory_resize!(interpreter, memory_offset, len); // Safety: set_data is unsafe function and memory_resize ensures us that it is safe to call it - memory.set_data( + interpreter.shared_memory.set_data( memory_offset, code_offset, len, @@ -100,10 +100,15 @@ pub fn calldatacopy(interpreter: &mut Interpreter, _host: &mut dyn Host) { InstructionResult::InvalidOperandOOG ); let data_offset = as_usize_saturated!(data_offset); - shared_memory_resize!(interpreter, memory_offset, len, memory); + shared_memory_resize!(interpreter, memory_offset, len); // Safety: set_data is unsafe function and memory_resize ensures us that it is safe to call it - memory.set_data(memory_offset, data_offset, len, &interpreter.contract.input); + interpreter.shared_memory.set_data( + memory_offset, + data_offset, + len, + &interpreter.contract.input, + ); } pub fn returndatasize(interpreter: &mut Interpreter, _host: &mut dyn Host) { @@ -134,8 +139,8 @@ pub fn returndatacopy(interpreter: &mut Interpreter, _host: &mut dyn memory_offset, InstructionResult::InvalidOperandOOG ); - shared_memory_resize!(interpreter, memory_offset, len, memory); - memory.set( + shared_memory_resize!(interpreter, memory_offset, len); + interpreter.shared_memory.set( memory_offset, &interpreter.return_data_buffer[data_offset..data_end], ); diff --git a/crates/interpreter/src/interpreter.rs b/crates/interpreter/src/interpreter.rs index 5e28475087..ebb98ad013 100644 --- a/crates/interpreter/src/interpreter.rs +++ b/crates/interpreter/src/interpreter.rs @@ -14,8 +14,6 @@ use crate::{ instructions::{eval, InstructionResult}, Gas, Host, }; -use alloc::rc::Rc; -use core::cell::RefCell; use core::ops::Range; pub const CALL_STACK_LIMIT: u64 = 1024; @@ -28,7 +26,7 @@ pub const MAX_CODE_SIZE: usize = 0x6000; /// EIP-3860: Limit and meter initcode pub const MAX_INITCODE_SIZE: usize = 2 * MAX_CODE_SIZE; -pub struct Interpreter { +pub struct Interpreter<'a> { /// Instruction pointer. pub instruction_pointer: *const u8, /// Return is main control flag, it tell us if we should continue interpreter or break from it @@ -36,7 +34,7 @@ pub struct Interpreter { /// left gas. Memory gas can be found in Memory field. pub gas: Gas, /// Shared memory. - pub shared_memory: Rc>, + pub shared_memory: &'a mut SharedMemory, /// Stack. pub stack: Stack, /// After call returns, its return data is saved here. @@ -49,7 +47,7 @@ pub struct Interpreter { pub contract: Box, } -impl Interpreter { +impl<'a> Interpreter<'a> { /// Current opcode pub fn current_opcode(&self) -> u8 { unsafe { *self.instruction_pointer } @@ -60,13 +58,13 @@ impl Interpreter { contract: Box, gas_limit: u64, is_static: bool, - shared_memory: &Rc>, + shared_memory: &'a mut SharedMemory, ) -> Self { Self { instruction_pointer: contract.bytecode.as_ptr(), return_range: Range::default(), stack: Stack::new(), - shared_memory: Rc::clone(shared_memory), + shared_memory, return_data_buffer: Bytes::new(), contract, instruction_result: InstructionResult::Continue, @@ -137,17 +135,17 @@ impl Interpreter { } /// Copy and get the return value of the interpreter, if any. - pub fn return_value(&self) -> Bytes { + pub fn return_value(&mut self) -> Bytes { // if start is usize max it means that our return len is zero and we need to return empty let bytes = if self.return_range.start == usize::MAX { Bytes::new() } else { - Bytes::copy_from_slice(self.shared_memory.borrow().get_slice( + Bytes::copy_from_slice(self.shared_memory.get_slice( self.return_range.start, self.return_range.end - self.return_range.start, )) }; - self.shared_memory.borrow_mut().free_context_memory(); + self.shared_memory.free_context_memory(); bytes } } diff --git a/crates/revm/benches/bench.rs b/crates/revm/benches/bench.rs index df1f4ece0e..736e6f4a1c 100644 --- a/crates/revm/benches/bench.rs +++ b/crates/revm/benches/bench.rs @@ -8,7 +8,7 @@ use revm::{ primitives::{BerlinSpec, Bytecode, BytecodeState, TransactTo, U256}, }; use revm_interpreter::SharedMemory; -use std::{cell::RefCell, rc::Rc, time::Duration}; +use std::time::Duration; type Evm = revm::EVM; @@ -114,7 +114,7 @@ fn bench_transact(g: &mut BenchmarkGroup<'_, WallTime>, evm: &mut Evm) { fn bench_eval(g: &mut BenchmarkGroup<'_, WallTime>, evm: &mut Evm) { let gas_limit = 22_000; evm.env.tx.gas_limit = gas_limit; - let shared_memory = Rc::new(RefCell::new(SharedMemory::new(evm.env.tx.gas_limit))); + let mut shared_memory = SharedMemory::new(evm.env.tx.gas_limit); g.bench_function("eval", |b| { let contract = Contract { @@ -126,8 +126,12 @@ fn bench_eval(g: &mut BenchmarkGroup<'_, WallTime>, evm: &mut Evm) { b.iter(|| { // reset gas limit to the right amount before tx evm.env.tx.gas_limit = gas_limit; - let mut interpreter = - Interpreter::new(Box::new(contract.clone()), u64::MAX, false, &shared_memory); + let mut interpreter = Interpreter::new( + Box::new(contract.clone()), + u64::MAX, + false, + &mut shared_memory, + ); let res = interpreter.run::<_, BerlinSpec>(&mut host); host.clear(); res diff --git a/crates/revm/src/evm_impl.rs b/crates/revm/src/evm_impl.rs index 0515ba1bdf..71535c1fb8 100644 --- a/crates/revm/src/evm_impl.rs +++ b/crates/revm/src/evm_impl.rs @@ -13,9 +13,7 @@ use crate::primitives::{ }; use crate::{db::Database, journaled_state::JournaledState, precompile, Inspector}; use alloc::boxed::Box; -use alloc::rc::Rc; use alloc::vec::Vec; -use core::cell::RefCell; use core::{cmp::min, marker::PhantomData}; use revm_interpreter::gas::initial_tx_gas; use revm_interpreter::{SharedMemory, MAX_CODE_SIZE}; @@ -165,13 +163,11 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> Transact let transact_gas_limit = tx_gas_limit - initial_gas_spend; #[cfg(feature = "memory_limit")] - let shared_memory = Rc::new(RefCell::new(SharedMemory::new_with_memory_limit( - transact_gas_limit, - memory_limit, - ))); + let mut shared_memory = + SharedMemory::new_with_memory_limit(transact_gas_limit, memory_limit); #[cfg(not(feature = "memory_limit"))] - let shared_memory = Rc::new(RefCell::new(SharedMemory::new(transact_gas_limit))); + let mut shared_memory = SharedMemory::new(transact_gas_limit); // call inner handling of call/create let (exit_reason, ret_gas, output) = match self.data.env.tx.transact_to { @@ -199,7 +195,7 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> Transact }, is_static: false, }, - &shared_memory, + &mut shared_memory, ); (exit, gas, Output::Call(bytes)) } @@ -212,7 +208,7 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> Transact init_code: tx_data, gas_limit: transact_gas_limit, }, - &shared_memory, + &mut shared_memory, ); (exit, ret_gas, Output::Create(bytes, address)) } @@ -468,7 +464,7 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, fn create_inner( &mut self, inputs: &CreateInputs, - shared_memory: &Rc>, + shared_memory: &mut SharedMemory, ) -> CreateResult { // Prepare crate. let prepared_create = match self.prepare_create(inputs) { @@ -477,7 +473,7 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, }; // Create new interpreter and execute initcode - let (exit_reason, mut interpreter) = self.run_interpreter( + let (exit_reason, mut bytes, mut gas) = self.run_interpreter( prepared_create.contract, prepared_create.gas.limit(), false, @@ -488,8 +484,7 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, match exit_reason { return_ok!() => { // if ok, check contract creation limit and calculate gas deduction on output len. - let mut bytes = interpreter.return_value(); - + // // EIP-3541: Reject new contract code starting with the 0xEF byte if GSPEC::enabled(LONDON) && !bytes.is_empty() && bytes.first() == Some(&0xEF) { self.data @@ -498,7 +493,7 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, return CreateResult { result: InstructionResult::CreateContractStartingWithEF, created_address: Some(prepared_create.created_address), - gas: interpreter.gas, + gas, return_value: bytes, }; } @@ -520,13 +515,13 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, return CreateResult { result: InstructionResult::CreateContractSizeLimit, created_address: Some(prepared_create.created_address), - gas: interpreter.gas, + gas, return_value: bytes, }; } if crate::USE_GAS { let gas_for_code = bytes.len() as u64 * gas::CODEDEPOSIT; - if !interpreter.gas.record_cost(gas_for_code) { + if !gas.record_cost(gas_for_code) { // record code deposit gas cost and check if we are out of gas. // EIP-2 point 3: If contract creation does not have enough gas to pay for the // final gas fee for adding the contract code to the state, the contract @@ -538,7 +533,7 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, return CreateResult { result: InstructionResult::OutOfGas, created_address: Some(prepared_create.created_address), - gas: interpreter.gas, + gas, return_value: bytes, }; } else { @@ -560,7 +555,7 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, CreateResult { result: InstructionResult::Return, created_address: Some(prepared_create.created_address), - gas: interpreter.gas, + gas, return_value: bytes, } } @@ -571,22 +566,22 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, CreateResult { result: exit_reason, created_address: Some(prepared_create.created_address), - gas: interpreter.gas, - return_value: interpreter.return_value(), + gas, + return_value: bytes, } } } } /// Create a Interpreter and run it. - /// Returns the exit reason and created interpreter as it contains return values and gas spend. + /// Returns the exit reason, return value and gas from interpreter pub fn run_interpreter( &mut self, contract: Box, gas_limit: u64, is_static: bool, - shared_memory: &Rc>, - ) -> (InstructionResult, Box) { + shared_memory: &mut SharedMemory, + ) -> (InstructionResult, Bytes, Gas) { let mut interpreter = Box::new(Interpreter::new( contract, gas_limit, @@ -594,7 +589,7 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, shared_memory, )); - interpreter.shared_memory.borrow_mut().new_context_memory(); + interpreter.shared_memory.new_context_memory(); if INSPECT { self.inspector @@ -606,7 +601,7 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, interpreter.run::(self) }; - (exit_reason, interpreter) + (exit_reason, interpreter.return_value(), *interpreter.gas()) } /// Call precompile contract @@ -722,11 +717,7 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, } /// Main contract call of the EVM. - fn call_inner( - &mut self, - inputs: &CallInputs, - shared_memory: &Rc>, - ) -> CallResult { + fn call_inner(&mut self, inputs: &CallInputs, shared_memory: &mut SharedMemory) -> CallResult { // Prepare call let prepared_call = match self.prepare_call(inputs) { Ok(o) => o, @@ -737,7 +728,7 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, self.call_precompile(inputs, prepared_call.gas) } else if !prepared_call.contract.bytecode.is_empty() { // Create interpreter and execute subcall - let (exit_reason, interpreter) = self.run_interpreter( + let (exit_reason, bytes, gas) = self.run_interpreter( prepared_call.contract, prepared_call.gas.limit(), inputs.is_static, @@ -745,8 +736,8 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, ); CallResult { result: exit_reason, - gas: interpreter.gas, - return_value: interpreter.return_value(), + gas, + return_value: bytes, } } else { CallResult { @@ -898,7 +889,7 @@ impl<'a, GSPEC: Spec, DB: Database + 'a, const INSPECT: bool> Host fn create( &mut self, inputs: &mut CreateInputs, - shared_memory: &Rc>, + shared_memory: &mut SharedMemory, ) -> (InstructionResult, Option, Gas, Bytes) { // Call inspector if INSPECT { @@ -927,7 +918,7 @@ impl<'a, GSPEC: Spec, DB: Database + 'a, const INSPECT: bool> Host fn call( &mut self, inputs: &mut CallInputs, - shared_memory: &Rc>, + shared_memory: &mut SharedMemory, ) -> (InstructionResult, Gas, Bytes) { if INSPECT { let (ret, gas, out) = self.inspector.call(&mut self.data, inputs); diff --git a/crates/revm/src/inspector/customprinter.rs b/crates/revm/src/inspector/customprinter.rs index 72fd4d25da..4ae2f81e73 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.shared_memory.borrow().len(), + interp.shared_memory.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 216f7447aa..f21c609157 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.shared_memory.borrow().len(); + self.mem_size = interp.shared_memory.len(); self.gas = self.gas_inspector.gas_remaining(); // InstructionResult::Continue From 27978c7b1340df3f3d99f57f8294e81bc09e23f1 Mon Sep 17 00:00:00 2001 From: Lorenzo Feroleto Date: Tue, 5 Sep 2023 16:33:09 +0200 Subject: [PATCH 11/19] chore(shared_memory): dropped f64::sqrt for no_std code --- .../interpreter/src/interpreter/shared_memory.rs | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/crates/interpreter/src/interpreter/shared_memory.rs b/crates/interpreter/src/interpreter/shared_memory.rs index c2e7bd67e1..7c7deae25d 100644 --- a/crates/interpreter/src/interpreter/shared_memory.rs +++ b/crates/interpreter/src/interpreter/shared_memory.rs @@ -199,7 +199,7 @@ impl SharedMemory { /// https://2π.com/22/eth-max-mem #[inline(always)] fn calculate_upper_bound(gas_limit: u64) -> u64 { - 4096 * ((2 * gas_limit) as f64).sqrt() as u64 + 4096 * sqrt(2 * gas_limit) } /// Update the amount of memory left for usage @@ -216,3 +216,17 @@ 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) } + +/// Basic sqrt function using Babylonian method +fn sqrt(n: u64) -> u64 { + if n < 2 { + return n; + } + let mut x = n / 2; + let mut y = (x + n / x) / 2; + while y < x { + x = y; + y = (x + n / x) / 2; + } + x +} From c7945ebcf375bf629d76fa64d04ff979b7cd15d3 Mon Sep 17 00:00:00 2001 From: Lorenzo Feroleto Date: Tue, 26 Sep 2023 18:43:08 +0200 Subject: [PATCH 12/19] fix(shared_memory): restore free_context_memory where needed --- crates/interpreter/src/interpreter.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/interpreter/src/interpreter.rs b/crates/interpreter/src/interpreter.rs index 716d0d661e..01470354a0 100644 --- a/crates/interpreter/src/interpreter.rs +++ b/crates/interpreter/src/interpreter.rs @@ -146,8 +146,10 @@ impl<'a> Interpreter<'a> { /// Returns a copy of the interpreter's return value, if any. #[inline] - pub fn return_value(&self) -> Bytes { - self.return_value_slice().to_vec().into() + pub fn return_value(&mut self) -> Bytes { + let return_data = self.return_value_slice().to_vec().into(); + self.shared_memory.free_context_memory(); + return_data } /// Returns a reference to the interpreter's return value, if any. From 07ece234e2390dca737656219d58c8660004fdc4 Mon Sep 17 00:00:00 2001 From: Lorenzo Feroleto Date: Wed, 27 Sep 2023 14:15:38 +0200 Subject: [PATCH 13/19] chore(SharedMemory): refactoring --- crates/interpreter/src/instructions/macros.rs | 6 +- crates/interpreter/src/interpreter.rs | 4 +- .../src/interpreter/shared_memory.rs | 111 ++++++++---------- crates/revm/src/evm_impl.rs | 6 +- 4 files changed, 58 insertions(+), 69 deletions(-) diff --git a/crates/interpreter/src/instructions/macros.rs b/crates/interpreter/src/instructions/macros.rs index a1cf6343af..ae1242b7e1 100644 --- a/crates/interpreter/src/instructions/macros.rs +++ b/crates/interpreter/src/instructions/macros.rs @@ -52,12 +52,10 @@ macro_rules! gas_or_fail { macro_rules! shared_memory_resize { ($interp:expr, $offset:expr, $len:expr) => { - let len: usize = $len; - let offset: usize = $offset; if let Some(new_size) = - crate::interpreter::shared_memory::next_multiple_of_32(offset.saturating_add(len)) + crate::interpreter::shared_memory::next_multiple_of_32($offset.saturating_add($len)) { - if new_size as u64 > $interp.shared_memory.limit { + if new_size > $interp.shared_memory.limit { $interp.instruction_result = InstructionResult::MemoryLimitOOG; return; } diff --git a/crates/interpreter/src/interpreter.rs b/crates/interpreter/src/interpreter.rs index 01470354a0..0e30170af9 100644 --- a/crates/interpreter/src/interpreter.rs +++ b/crates/interpreter/src/interpreter.rs @@ -147,9 +147,7 @@ impl<'a> Interpreter<'a> { /// Returns a copy of the interpreter's return value, if any. #[inline] pub fn return_value(&mut self) -> Bytes { - let return_data = self.return_value_slice().to_vec().into(); - self.shared_memory.free_context_memory(); - return_data + self.return_value_slice().to_vec().into() } /// Returns a reference to the interpreter's return value, if any. diff --git a/crates/interpreter/src/interpreter/shared_memory.rs b/crates/interpreter/src/interpreter/shared_memory.rs index b04b2e3601..8f9deafc2a 100644 --- a/crates/interpreter/src/interpreter/shared_memory.rs +++ b/crates/interpreter/src/interpreter/shared_memory.rs @@ -1,7 +1,7 @@ use revm_primitives::U256; use crate::alloc::vec; -use crate::alloc::vec::Vec; +use crate::alloc::{boxed::Box, slice, vec::Vec}; use core::{ cmp::min, fmt, @@ -14,16 +14,16 @@ use core::{ /// the `new` static method to ensure memory safety. pub struct SharedMemory { /// Shared buffer - data: Vec, + data: Box<[u8]>, /// Memory checkpoints for each depth checkpoints: Vec, /// Raw pointer used for the portion of memory used /// by the current context - current_slice: *mut [u8], + current_ptr: *mut u8, /// How much memory has been used in the current context current_len: usize, /// Amount of memory left for assignment - pub limit: u64, + pub limit: usize, } impl fmt::Debug for SharedMemory { @@ -31,32 +31,26 @@ impl fmt::Debug for SharedMemory { f.debug_struct("SharedMemory") .field( "current_slice", - &crate::primitives::hex::encode(self.get_current_slice()), + &crate::primitives::hex::encode(self.current_slice()), ) .finish() } } impl SharedMemory { + /// Calculates memory allocation upper bound using + /// https://2π.com/22/eth-max-mem + #[inline] + pub fn calculate_upper_bound(gas_limit: u64) -> u64 { + 4096 * sqrt(2u64.checked_mul(gas_limit).unwrap_or(u64::MAX)) + } + /// Allocate memory to be shared between calls. /// Memory size is estimated using https://2π.com/22/eth-max-mem /// which depends on transaction [gas_limit]. /// Maximum allocation size is 2^32 - 1 bytes; pub fn new(gas_limit: u64) -> Self { - let size = min( - SharedMemory::calculate_upper_bound(gas_limit) as usize, - 2usize.pow(32) - 1, - ); - - let mut data = vec![0; size]; - let current_slice: *mut [u8] = &mut data[..]; - SharedMemory { - data, - checkpoints: Vec::with_capacity(1024), - current_slice, - current_len: 0, - limit: size as u64, - } + Self::new_with_memory_limit(gas_limit, (u32::MAX - 1) as u64) } /// Allocate memory to be shared between calls. @@ -64,69 +58,67 @@ impl SharedMemory { /// which depends on transaction [gas_limit]. /// Uses [memory_limit] as maximum allocation size pub fn new_with_memory_limit(gas_limit: u64, memory_limit: u64) -> Self { - let size = min( - SharedMemory::calculate_upper_bound(gas_limit) as usize, + let limit = min( + Self::calculate_upper_bound(gas_limit) as usize, memory_limit as usize, ); - let mut data = vec![0; size]; - let current_slice: *mut [u8] = &mut data[..]; - SharedMemory { + let mut data = vec![0; limit].into_boxed_slice(); + let current_slice = data.as_mut_ptr(); + Self { data, - checkpoints: Vec::with_capacity(1024), - current_slice, + checkpoints: Vec::with_capacity(32), + current_ptr: current_slice, current_len: 0, - limit: size as u64, + limit, } } /// Prepares the shared memory for a new context pub fn new_context_memory(&mut self) { - let base_offset = self.checkpoints.last().unwrap_or(&0); + let base_offset = self.last_checkpoint(); let new_checkpoint = base_offset + self.current_len; self.checkpoints.push(new_checkpoint); - self.current_slice = &mut self.data[new_checkpoint..]; + self.current_ptr = self.data[new_checkpoint..].as_mut_ptr(); self.current_len = 0; } /// Prepares the shared memory for returning to the previous context pub fn free_context_memory(&mut self) { if let Some(old_checkpoint) = self.checkpoints.pop() { - let last = *self.checkpoints.last().unwrap_or(&0); - self.current_slice = &mut self.data[last..]; - self.current_len = old_checkpoint - last; + let last_checkpoint = self.last_checkpoint(); + self.current_ptr = self.data[last_checkpoint..].as_mut_ptr(); + self.current_len = old_checkpoint - last_checkpoint; self.update_limit(); } } /// Get the length of the current memory range. + #[inline(always)] pub fn len(&self) -> usize { self.current_len } - /// Return true if current effective memory range is zero. + #[inline(always)] pub fn is_empty(&self) -> bool { self.current_len == 0 } - /// Return the memory of the current context - 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 + /// - we have enough gas to resize this vector /// - we made new_size as multiply of 32 /// - [new_size] is greater than `self.len()` + #[inline(always)] pub fn resize(&mut self, new_size: usize) { // extend with zeros let range = self.current_len..new_size; - self.get_current_slice_mut()[range] - .iter_mut() - .for_each(|byte| *byte = 0); + for byte in self.current_slice_mut()[range].iter_mut() { + *byte = 0; + } + self.current_len = new_size; self.update_limit(); } @@ -137,7 +129,7 @@ impl SharedMemory { #[inline(always)] #[cfg_attr(debug_assertions, track_caller)] pub fn slice(&self, offset: usize, size: usize) -> &[u8] { - match self.get_current_slice().get(offset..offset + size) { + match self.current_slice().get(offset..offset + size) { Some(slice) => slice, None => debug_unreachable!("slice OOB: {offset}..{size}; len: {}", self.len()), } @@ -150,7 +142,7 @@ impl SharedMemory { #[cfg_attr(debug_assertions, track_caller)] pub fn slice_mut(&mut self, offset: usize, size: usize) -> &mut [u8] { let len = self.current_len; - match self.get_current_slice_mut().get_mut(offset..offset + size) { + match self.current_slice_mut().get_mut(offset..offset + size) { Some(slice) => slice, None => debug_unreachable!("slice OOB: {offset}..{size}; len: {}", len), } @@ -162,7 +154,7 @@ impl SharedMemory { #[inline(always)] #[cfg_attr(debug_assertions, track_caller)] pub fn set_byte(&mut self, index: usize, byte: u8) { - match self.get_current_slice_mut().get_mut(index) { + match self.current_slice_mut().get_mut(index) { Some(b) => *b = byte, None => debug_unreachable!("set_byte OOB: {index}; len: {}", self.len()), } @@ -219,36 +211,33 @@ impl SharedMemory { #[inline(always)] #[cfg_attr(debug_assertions, track_caller)] pub fn copy(&mut self, dst: usize, src: usize, len: usize) { - self.get_current_slice_mut() - .copy_within(src..src + len, dst); + self.current_slice_mut().copy_within(src..src + len, dst); } - /// Get a refernce to the memory of the current context + /// Get a reference to the memory of the current context #[inline(always)] - fn get_current_slice(&self) -> &[u8] { + fn current_slice(&self) -> &[u8] { // Safety: if it is a valid pointer to a slice of `self.data` - unsafe { &*self.current_slice } + unsafe { slice::from_raw_parts(self.current_ptr, self.limit) } } - /// Get a mutable refernce to the memory of the current context + /// Get a mutable reference to the memory of the current context #[inline(always)] - fn get_current_slice_mut(&mut self) -> &mut [u8] { + fn current_slice_mut(&mut self) -> &mut [u8] { // Safety: it is a valid pointer to a slice of `self.data` - unsafe { &mut *self.current_slice } + unsafe { slice::from_raw_parts_mut(self.current_ptr, self.limit) } } - /// Calculates memory allocation upper bound using - /// https://2π.com/22/eth-max-mem + /// Update the amount of memory left for usage #[inline(always)] - fn calculate_upper_bound(gas_limit: u64) -> u64 { - 4096 * sqrt(2 * gas_limit) + fn update_limit(&mut self) { + self.limit = self.data.len() - self.last_checkpoint() - self.current_len; } - /// Update the amount of memory left for usage + /// Get the last memory checkpoint #[inline(always)] - fn update_limit(&mut self) { - self.limit = - (self.data.len() - *self.checkpoints.last().unwrap_or(&0) - self.current_len) as u64; + fn last_checkpoint(&self) -> usize { + *self.checkpoints.last().unwrap_or(&0) } } diff --git a/crates/revm/src/evm_impl.rs b/crates/revm/src/evm_impl.rs index 121b6e7e9f..c6b3708779 100644 --- a/crates/revm/src/evm_impl.rs +++ b/crates/revm/src/evm_impl.rs @@ -604,7 +604,11 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, interpreter.run::(self) }; - (exit_reason, interpreter.return_value(), *interpreter.gas()) + let (return_value, gas) = (interpreter.return_value(), *interpreter.gas()); + + interpreter.shared_memory.free_context_memory(); + + (exit_reason, return_value, gas) } /// Call precompile contract From 23a11914506baa28bed1d4425ed171cd15843084 Mon Sep 17 00:00:00 2001 From: Lorenzo Feroleto Date: Fri, 29 Sep 2023 19:02:09 +0200 Subject: [PATCH 14/19] chore(SharedMemory): internal refactoring part 2 --- crates/interpreter/src/interpreter.rs | 5 +-- .../src/interpreter/shared_memory.rs | 36 ++++++++++++------- crates/revm/benches/bench.rs | 13 ++----- 3 files changed, 29 insertions(+), 25 deletions(-) diff --git a/crates/interpreter/src/interpreter.rs b/crates/interpreter/src/interpreter.rs index 0e30170af9..59d9ae1ba8 100644 --- a/crates/interpreter/src/interpreter.rs +++ b/crates/interpreter/src/interpreter.rs @@ -88,7 +88,8 @@ impl<'a> Interpreter<'a> { &self.gas } - /// Reference of interpreter stack. + /// Returns a reference to the interpreter's stack. + #[inline] pub fn stack(&self) -> &Stack { &self.stack } @@ -146,7 +147,7 @@ impl<'a> Interpreter<'a> { /// Returns a copy of the interpreter's return value, if any. #[inline] - pub fn return_value(&mut self) -> Bytes { + pub fn return_value(&self) -> Bytes { self.return_value_slice().to_vec().into() } diff --git a/crates/interpreter/src/interpreter/shared_memory.rs b/crates/interpreter/src/interpreter/shared_memory.rs index 8f9deafc2a..af4ed2c236 100644 --- a/crates/interpreter/src/interpreter/shared_memory.rs +++ b/crates/interpreter/src/interpreter/shared_memory.rs @@ -42,7 +42,7 @@ impl SharedMemory { /// https://2π.com/22/eth-max-mem #[inline] pub fn calculate_upper_bound(gas_limit: u64) -> u64 { - 4096 * sqrt(2u64.checked_mul(gas_limit).unwrap_or(u64::MAX)) + 4096 * sqrt(2u64.saturating_mul(gas_limit)) } /// Allocate memory to be shared between calls. @@ -50,7 +50,7 @@ impl SharedMemory { /// which depends on transaction [gas_limit]. /// Maximum allocation size is 2^32 - 1 bytes; pub fn new(gas_limit: u64) -> Self { - Self::new_with_memory_limit(gas_limit, (u32::MAX - 1) as u64) + Self::new_with_memory_limit(gas_limit, u32::MAX as u64) } /// Allocate memory to be shared between calls. @@ -81,7 +81,7 @@ impl SharedMemory { self.checkpoints.push(new_checkpoint); - self.current_ptr = self.data[new_checkpoint..].as_mut_ptr(); + self.set_ptr(new_checkpoint); self.current_len = 0; } @@ -89,7 +89,7 @@ impl SharedMemory { pub fn free_context_memory(&mut self) { if let Some(old_checkpoint) = self.checkpoints.pop() { let last_checkpoint = self.last_checkpoint(); - self.current_ptr = self.data[last_checkpoint..].as_mut_ptr(); + self.set_ptr(last_checkpoint); self.current_len = old_checkpoint - last_checkpoint; self.update_limit(); } @@ -101,6 +101,7 @@ impl SharedMemory { self.current_len } + /// Returns true if the current memory range is empty. #[inline(always)] pub fn is_empty(&self) -> bool { self.current_len == 0 @@ -115,9 +116,7 @@ impl SharedMemory { // extend with zeros let range = self.current_len..new_size; - for byte in self.current_slice_mut()[range].iter_mut() { - *byte = 0; - } + self.current_slice_mut()[range].fill(0); self.current_len = new_size; self.update_limit(); @@ -129,9 +128,10 @@ impl SharedMemory { #[inline(always)] #[cfg_attr(debug_assertions, track_caller)] pub fn slice(&self, offset: usize, size: usize) -> &[u8] { - match self.current_slice().get(offset..offset + size) { + let end = offset + size; + match self.current_slice().get(offset..end) { Some(slice) => slice, - None => debug_unreachable!("slice OOB: {offset}..{size}; len: {}", self.len()), + None => debug_unreachable!("slice OOB: {offset}..{end}; len: {}", self.len()), } } @@ -141,10 +141,11 @@ impl SharedMemory { #[inline(always)] #[cfg_attr(debug_assertions, track_caller)] pub fn slice_mut(&mut self, offset: usize, size: usize) -> &mut [u8] { + let end = offset + size; let len = self.current_len; match self.current_slice_mut().get_mut(offset..offset + size) { Some(slice) => slice, - None => debug_unreachable!("slice OOB: {offset}..{size}; len: {}", len), + None => debug_unreachable!("slice OOB: {offset}..{end}; len: {}", len), } } @@ -218,14 +219,16 @@ impl SharedMemory { #[inline(always)] fn current_slice(&self) -> &[u8] { // Safety: if it is a valid pointer to a slice of `self.data` - unsafe { slice::from_raw_parts(self.current_ptr, self.limit) } + unsafe { slice::from_raw_parts(self.current_ptr, self.data.len() - self.last_checkpoint()) } } /// Get a mutable reference to the memory of the current context #[inline(always)] fn current_slice_mut(&mut self) -> &mut [u8] { // Safety: it is a valid pointer to a slice of `self.data` - unsafe { slice::from_raw_parts_mut(self.current_ptr, self.limit) } + unsafe { + slice::from_raw_parts_mut(self.current_ptr, self.data.len() - self.last_checkpoint()) + } } /// Update the amount of memory left for usage @@ -239,6 +242,15 @@ impl SharedMemory { fn last_checkpoint(&self) -> usize { *self.checkpoints.last().unwrap_or(&0) } + + /// Set memory pointer to checkpoint + #[inline(always)] + fn set_ptr(&mut self, checkpoint: usize) { + if self.data.len() > 0 { + assume!(checkpoint < self.data.len()); + self.current_ptr = unsafe { self.data.as_mut_ptr().add(checkpoint) }; + } + } } /// Rounds up `x` to the closest multiple of 32. If `x % 32 == 0` then `x` is returned. diff --git a/crates/revm/benches/bench.rs b/crates/revm/benches/bench.rs index 736e6f4a1c..e7e057b6db 100644 --- a/crates/revm/benches/bench.rs +++ b/crates/revm/benches/bench.rs @@ -102,18 +102,11 @@ fn bench_transact(g: &mut BenchmarkGroup<'_, WallTime>, evm: &mut Evm) { BytecodeState::Analysed { .. } => "analysed", }; let id = format!("transact/{state}"); - g.bench_function(id, |b| { - b.iter(|| { - // reset gas limit to the right amount before tx - evm.env.tx.gas_limit = 22_000; - evm.transact().unwrap() - }) - }); + g.bench_function(id, |b| b.iter(|| evm.transact().unwrap())); } fn bench_eval(g: &mut BenchmarkGroup<'_, WallTime>, evm: &mut Evm) { - let gas_limit = 22_000; - evm.env.tx.gas_limit = gas_limit; + evm.env.tx.gas_limit = 22_000; let mut shared_memory = SharedMemory::new(evm.env.tx.gas_limit); g.bench_function("eval", |b| { @@ -124,8 +117,6 @@ fn bench_eval(g: &mut BenchmarkGroup<'_, WallTime>, evm: &mut Evm) { }; let mut host = DummyHost::new(evm.env.clone()); b.iter(|| { - // reset gas limit to the right amount before tx - evm.env.tx.gas_limit = gas_limit; let mut interpreter = Interpreter::new( Box::new(contract.clone()), u64::MAX, From 39efbe9be000b1b6e256fae2d041d4e620bcaaed Mon Sep 17 00:00:00 2001 From: Lorenzo Feroleto Date: Wed, 4 Oct 2023 15:28:28 +0200 Subject: [PATCH 15/19] chore(SharedMemory): test with more allocations --- crates/interpreter/src/instructions/macros.rs | 2 +- .../src/interpreter/shared_memory.rs | 72 +++++++------------ crates/revm/src/evm_impl.rs | 7 -- 3 files changed, 26 insertions(+), 55 deletions(-) diff --git a/crates/interpreter/src/instructions/macros.rs b/crates/interpreter/src/instructions/macros.rs index 627fb24b69..723409750e 100644 --- a/crates/interpreter/src/instructions/macros.rs +++ b/crates/interpreter/src/instructions/macros.rs @@ -55,7 +55,7 @@ macro_rules! shared_memory_resize { if let Some(new_size) = crate::interpreter::shared_memory::next_multiple_of_32($offset.saturating_add($len)) { - if new_size > $interp.shared_memory.limit { + if $interp.shared_memory.new_size_over_limit(new_size) { $interp.instruction_result = InstructionResult::MemoryLimitOOG; return; } diff --git a/crates/interpreter/src/interpreter/shared_memory.rs b/crates/interpreter/src/interpreter/shared_memory.rs index af4ed2c236..22312fae62 100644 --- a/crates/interpreter/src/interpreter/shared_memory.rs +++ b/crates/interpreter/src/interpreter/shared_memory.rs @@ -1,7 +1,6 @@ use revm_primitives::U256; -use crate::alloc::vec; -use crate::alloc::{boxed::Box, slice, vec::Vec}; +use crate::alloc::vec::Vec; use core::{ cmp::min, fmt, @@ -14,12 +13,9 @@ use core::{ /// the `new` static method to ensure memory safety. pub struct SharedMemory { /// Shared buffer - data: Box<[u8]>, + data: Vec, /// Memory checkpoints for each depth checkpoints: Vec, - /// Raw pointer used for the portion of memory used - /// by the current context - current_ptr: *mut u8, /// How much memory has been used in the current context current_len: usize, /// Amount of memory left for assignment @@ -50,25 +46,10 @@ impl SharedMemory { /// which depends on transaction [gas_limit]. /// Maximum allocation size is 2^32 - 1 bytes; pub fn new(gas_limit: u64) -> Self { - Self::new_with_memory_limit(gas_limit, u32::MAX as u64) - } - - /// Allocate memory to be shared between calls. - /// Memory size is estimated using https://2π.com/22/eth-max-mem - /// which depends on transaction [gas_limit]. - /// Uses [memory_limit] as maximum allocation size - pub fn new_with_memory_limit(gas_limit: u64, memory_limit: u64) -> Self { - let limit = min( - Self::calculate_upper_bound(gas_limit) as usize, - memory_limit as usize, - ); - - let mut data = vec![0; limit].into_boxed_slice(); - let current_slice = data.as_mut_ptr(); + let limit = Self::calculate_upper_bound(gas_limit) as usize; Self { - data, + data: Vec::with_capacity(1024), checkpoints: Vec::with_capacity(32), - current_ptr: current_slice, current_len: 0, limit, } @@ -80,8 +61,6 @@ impl SharedMemory { let new_checkpoint = base_offset + self.current_len; self.checkpoints.push(new_checkpoint); - - self.set_ptr(new_checkpoint); self.current_len = 0; } @@ -89,9 +68,7 @@ impl SharedMemory { pub fn free_context_memory(&mut self) { if let Some(old_checkpoint) = self.checkpoints.pop() { let last_checkpoint = self.last_checkpoint(); - self.set_ptr(last_checkpoint); self.current_len = old_checkpoint - last_checkpoint; - self.update_limit(); } } @@ -113,13 +90,16 @@ impl SharedMemory { /// - [new_size] is greater than `self.len()` #[inline(always)] pub fn resize(&mut self, new_size: usize) { - // extend with zeros - let range = self.current_len..new_size; + let last_checkpoint = self.last_checkpoint(); + let range = last_checkpoint + self.current_len..last_checkpoint + new_size; - self.current_slice_mut()[range].fill(0); + if let Some(available_memory) = self.data.get_mut(range) { + available_memory.fill(0); + } else { + self.data.resize(last_checkpoint + new_size, 0); + } self.current_len = new_size; - self.update_limit(); } /// Returns a byte slice of the memory region at the given offset. @@ -219,22 +199,29 @@ impl SharedMemory { #[inline(always)] fn current_slice(&self) -> &[u8] { // Safety: if it is a valid pointer to a slice of `self.data` - unsafe { slice::from_raw_parts(self.current_ptr, self.data.len() - self.last_checkpoint()) } + let data = self + .data + .get(self.last_checkpoint()..self.last_checkpoint() + self.current_len) + .unwrap(); + data } /// Get a mutable reference to the memory of the current context #[inline(always)] fn current_slice_mut(&mut self) -> &mut [u8] { + let last_checkpoint = self.last_checkpoint(); + let current_len = self.current_len; // Safety: it is a valid pointer to a slice of `self.data` - unsafe { - slice::from_raw_parts_mut(self.current_ptr, self.data.len() - self.last_checkpoint()) - } + let data = self + .data + .get_mut(last_checkpoint..last_checkpoint + current_len) + .unwrap(); + data } - /// Update the amount of memory left for usage #[inline(always)] - fn update_limit(&mut self) { - self.limit = self.data.len() - self.last_checkpoint() - self.current_len; + pub fn new_size_over_limit(&self, new_size: usize) -> bool { + self.last_checkpoint() + new_size > self.limit } /// Get the last memory checkpoint @@ -242,15 +229,6 @@ impl SharedMemory { fn last_checkpoint(&self) -> usize { *self.checkpoints.last().unwrap_or(&0) } - - /// Set memory pointer to checkpoint - #[inline(always)] - fn set_ptr(&mut self, checkpoint: usize) { - if self.data.len() > 0 { - assume!(checkpoint < self.data.len()); - self.current_ptr = unsafe { self.data.as_mut_ptr().add(checkpoint) }; - } - } } /// Rounds up `x` to the closest multiple of 32. If `x % 32 == 0` then `x` is returned. diff --git a/crates/revm/src/evm_impl.rs b/crates/revm/src/evm_impl.rs index 4545c47094..49cc916a89 100644 --- a/crates/revm/src/evm_impl.rs +++ b/crates/revm/src/evm_impl.rs @@ -189,8 +189,6 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> Transact fn transact_preverified(&mut self) -> EVMResult { let env = &self.data.env; - #[cfg(feature = "memory_limit")] - let memory_limit = env.cfg.memory_limit; let tx_caller = env.tx.caller; let tx_value = env.tx.value; let tx_data = env.tx.data.clone(); @@ -287,11 +285,6 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> Transact let transact_gas_limit = tx_gas_limit - initial_gas_spend; - #[cfg(feature = "memory_limit")] - let mut shared_memory = - SharedMemory::new_with_memory_limit(transact_gas_limit, memory_limit); - - #[cfg(not(feature = "memory_limit"))] let mut shared_memory = SharedMemory::new(transact_gas_limit); // call inner handling of call/create From 986a03ba7faa4938d8857a6ebea746fe88509e6e Mon Sep 17 00:00:00 2001 From: Lorenzo Feroleto Date: Wed, 4 Oct 2023 17:33:47 +0200 Subject: [PATCH 16/19] chore(SharedMemory): refactoring for simplifying logic --- crates/interpreter/src/instructions/macros.rs | 5 -- .../src/interpreter/shared_memory.rs | 84 ++++++++----------- crates/revm/benches/bench.rs | 6 +- crates/revm/src/evm_impl.rs | 2 +- 4 files changed, 35 insertions(+), 62 deletions(-) diff --git a/crates/interpreter/src/instructions/macros.rs b/crates/interpreter/src/instructions/macros.rs index 723409750e..41063fca4a 100644 --- a/crates/interpreter/src/instructions/macros.rs +++ b/crates/interpreter/src/instructions/macros.rs @@ -55,11 +55,6 @@ macro_rules! shared_memory_resize { if let Some(new_size) = crate::interpreter::shared_memory::next_multiple_of_32($offset.saturating_add($len)) { - if $interp.shared_memory.new_size_over_limit(new_size) { - $interp.instruction_result = InstructionResult::MemoryLimitOOG; - return; - } - if new_size > $interp.shared_memory.len() { if crate::USE_GAS { let num_bytes = new_size / 32; diff --git a/crates/interpreter/src/interpreter/shared_memory.rs b/crates/interpreter/src/interpreter/shared_memory.rs index 22312fae62..d076993e9e 100644 --- a/crates/interpreter/src/interpreter/shared_memory.rs +++ b/crates/interpreter/src/interpreter/shared_memory.rs @@ -18,8 +18,6 @@ pub struct SharedMemory { checkpoints: Vec, /// How much memory has been used in the current context current_len: usize, - /// Amount of memory left for assignment - pub limit: usize, } impl fmt::Debug for SharedMemory { @@ -27,31 +25,28 @@ impl fmt::Debug for SharedMemory { f.debug_struct("SharedMemory") .field( "current_slice", - &crate::primitives::hex::encode(self.current_slice()), + &crate::primitives::hex::encode(self.context_memory()), ) .finish() } } -impl SharedMemory { - /// Calculates memory allocation upper bound using - /// https://2π.com/22/eth-max-mem - #[inline] - pub fn calculate_upper_bound(gas_limit: u64) -> u64 { - 4096 * sqrt(2u64.saturating_mul(gas_limit)) +impl Default for SharedMemory { + fn default() -> Self { + Self::new() } +} +impl SharedMemory { /// Allocate memory to be shared between calls. /// Memory size is estimated using https://2π.com/22/eth-max-mem /// which depends on transaction [gas_limit]. /// Maximum allocation size is 2^32 - 1 bytes; - pub fn new(gas_limit: u64) -> Self { - let limit = Self::calculate_upper_bound(gas_limit) as usize; + pub fn new() -> Self { Self { - data: Vec::with_capacity(1024), + data: Vec::with_capacity(4 * 1024), // from evmone checkpoints: Vec::with_capacity(32), current_len: 0, - limit, } } @@ -96,7 +91,8 @@ impl SharedMemory { if let Some(available_memory) = self.data.get_mut(range) { available_memory.fill(0); } else { - self.data.resize(last_checkpoint + new_size, 0); + self.data + .resize(last_checkpoint + usize::max(new_size, 4 * 1024), 0); } self.current_len = new_size; @@ -109,7 +105,12 @@ impl SharedMemory { #[cfg_attr(debug_assertions, track_caller)] pub fn slice(&self, offset: usize, size: usize) -> &[u8] { let end = offset + size; - match self.current_slice().get(offset..end) { + let last_checkpoint = self.last_checkpoint(); + + match self + .data + .get(last_checkpoint + offset..last_checkpoint + offset + size) + { Some(slice) => slice, None => debug_unreachable!("slice OOB: {offset}..{end}; len: {}", self.len()), } @@ -121,9 +122,14 @@ impl SharedMemory { #[inline(always)] #[cfg_attr(debug_assertions, track_caller)] pub fn slice_mut(&mut self, offset: usize, size: usize) -> &mut [u8] { + let len = self.len(); let end = offset + size; - let len = self.current_len; - match self.current_slice_mut().get_mut(offset..offset + size) { + let last_checkpoint = self.last_checkpoint(); + + match self + .data + .get_mut(last_checkpoint + offset..last_checkpoint + offset + size) + { Some(slice) => slice, None => debug_unreachable!("slice OOB: {offset}..{end}; len: {}", len), } @@ -135,7 +141,8 @@ impl SharedMemory { #[inline(always)] #[cfg_attr(debug_assertions, track_caller)] pub fn set_byte(&mut self, index: usize, byte: u8) { - match self.current_slice_mut().get_mut(index) { + let last_checkpoint = self.last_checkpoint(); + match self.data.get_mut(last_checkpoint + index) { Some(b) => *b = byte, None => debug_unreachable!("set_byte OOB: {index}; len: {}", self.len()), } @@ -192,36 +199,25 @@ impl SharedMemory { #[inline(always)] #[cfg_attr(debug_assertions, track_caller)] pub fn copy(&mut self, dst: usize, src: usize, len: usize) { - self.current_slice_mut().copy_within(src..src + len, dst); + self.context_memory_mut().copy_within(src..src + len, dst); } /// Get a reference to the memory of the current context #[inline(always)] - fn current_slice(&self) -> &[u8] { - // Safety: if it is a valid pointer to a slice of `self.data` - let data = self - .data - .get(self.last_checkpoint()..self.last_checkpoint() + self.current_len) - .unwrap(); - data + fn context_memory(&self) -> &[u8] { + let last_checkpoint = self.last_checkpoint(); + let current_len = self.current_len; + // Safety: it is a valid pointer to a slice of `self.data` + &self.data[last_checkpoint..last_checkpoint + current_len] } /// Get a mutable reference to the memory of the current context #[inline(always)] - fn current_slice_mut(&mut self) -> &mut [u8] { + fn context_memory_mut(&mut self) -> &mut [u8] { let last_checkpoint = self.last_checkpoint(); let current_len = self.current_len; // Safety: it is a valid pointer to a slice of `self.data` - let data = self - .data - .get_mut(last_checkpoint..last_checkpoint + current_len) - .unwrap(); - data - } - - #[inline(always)] - pub fn new_size_over_limit(&self, new_size: usize) -> bool { - self.last_checkpoint() + new_size > self.limit + &mut self.data[last_checkpoint..last_checkpoint + current_len] } /// Get the last memory checkpoint @@ -238,20 +234,6 @@ pub(crate) fn next_multiple_of_32(x: usize) -> Option { x.checked_add(r) } -/// Basic sqrt function using Babylonian method -fn sqrt(n: u64) -> u64 { - if n < 2 { - return n; - } - let mut x = n / 2; - let mut y = (x + n / x) / 2; - while y < x { - x = y; - y = (x + n / x) / 2; - } - x -} - #[cfg(test)] mod tests { use super::next_multiple_of_32; diff --git a/crates/revm/benches/bench.rs b/crates/revm/benches/bench.rs index c6d7178645..4dadb40c67 100644 --- a/crates/revm/benches/bench.rs +++ b/crates/revm/benches/bench.rs @@ -16,9 +16,6 @@ type Evm = revm::EVM; fn analysis(c: &mut Criterion) { let mut evm = revm::new(); - // Empirical good value for gas_limit for low shared_memory usage - evm.env.tx.gas_limit = 22_000; - evm.env.tx.caller = address!("1000000000000000000000000000000000000000"); evm.env.tx.transact_to = TransactTo::Call(address!("0000000000000000000000000000000000000000")); // evm.env.tx.data = bytes!("30627b7c"); @@ -90,8 +87,7 @@ fn bench_transact(g: &mut BenchmarkGroup<'_, WallTime>, evm: &mut Evm) { } fn bench_eval(g: &mut BenchmarkGroup<'_, WallTime>, evm: &mut Evm) { - evm.env.tx.gas_limit = 22_000; - let mut shared_memory = SharedMemory::new(evm.env.tx.gas_limit); + let mut shared_memory = SharedMemory::new(); g.bench_function("eval", |b| { let contract = Contract { diff --git a/crates/revm/src/evm_impl.rs b/crates/revm/src/evm_impl.rs index 49cc916a89..792a06c5fc 100644 --- a/crates/revm/src/evm_impl.rs +++ b/crates/revm/src/evm_impl.rs @@ -285,7 +285,7 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> Transact let transact_gas_limit = tx_gas_limit - initial_gas_spend; - let mut shared_memory = SharedMemory::new(transact_gas_limit); + let mut shared_memory = SharedMemory::new(); // call inner handling of call/create let (call_result, ret_gas, output) = match self.data.env.tx.transact_to { From 1da0f38a5e7f7a079c06e0246cae800fe29eb020 Mon Sep 17 00:00:00 2001 From: Lorenzo Feroleto Date: Fri, 6 Oct 2023 10:04:16 +0200 Subject: [PATCH 17/19] chore(SharedMemory): remove memory file from sync --- crates/interpreter/src/interpreter/memory.rs | 233 ------------------- 1 file changed, 233 deletions(-) delete mode 100644 crates/interpreter/src/interpreter/memory.rs diff --git a/crates/interpreter/src/interpreter/memory.rs b/crates/interpreter/src/interpreter/memory.rs deleted file mode 100644 index f0c15bcd2d..0000000000 --- a/crates/interpreter/src/interpreter/memory.rs +++ /dev/null @@ -1,233 +0,0 @@ -use crate::primitives::U256; -use alloc::vec::Vec; -use core::{ - cmp::min, - fmt, - ops::{BitAnd, Not}, -}; - -/// A sequential memory. It uses Rust's `Vec` for internal -/// representation. -#[derive(Clone, Eq, PartialEq)] -#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] -pub struct Memory { - data: Vec, -} - -impl fmt::Debug for Memory { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - f.debug_struct("Memory") - .field("data", &crate::primitives::hex::encode(&self.data)) - .finish() - } -} - -impl Default for Memory { - #[inline] - fn default() -> Self { - Self { - data: Vec::with_capacity(4 * 1024), // took it from evmone - } - } -} - -impl Memory { - /// Create a new memory with the given limit. - #[inline] - pub fn new() -> Self { - Self { - data: Vec::with_capacity(4 * 1024), // took it from evmone - } - } - - #[deprecated = "Use `len` instead"] - #[doc(hidden)] - #[inline] - pub fn effective_len(&self) -> usize { - self.len() - } - - /// Returns the length of the current memory range. - #[inline] - pub fn len(&self) -> usize { - self.data.len() - } - - /// Returns true if current memory range length is zero. - #[inline] - pub fn is_empty(&self) -> bool { - self.len() == 0 - } - - /// Return a reference to the full memory. - #[inline] - pub fn data(&self) -> &Vec { - &self.data - } - - /// Consumes the type and returns the full memory. - #[inline] - pub fn into_data(self) -> Vec { - self.data - } - - /// Shrinks the capacity of the data buffer as much as possible. - #[inline] - pub fn shrink_to_fit(&mut self) { - self.data.shrink_to_fit() - } - - /// Resizes the stack in-place so that then length is equal to `new_size`. - /// - /// `new_size` should be a multiple of 32. - #[inline] - pub fn resize(&mut self, new_size: usize) { - self.data.resize(new_size, 0); - } - - /// Returns a byte slice of the memory region at the given offset. - /// - /// Panics on out of bounds. - #[inline(always)] - #[cfg_attr(debug_assertions, track_caller)] - pub fn slice(&self, offset: usize, size: usize) -> &[u8] { - match self.data.get(offset..offset + size) { - Some(slice) => slice, - None => debug_unreachable!("slice OOB: {offset}..{size}; len: {}", self.len()), - } - } - - #[deprecated = "use `slice` instead"] - #[inline(always)] - #[cfg_attr(debug_assertions, track_caller)] - pub fn get_slice(&self, offset: usize, size: usize) -> &[u8] { - self.slice(offset, size) - } - - /// Returns a mutable byte slice of the memory region at the given offset. - /// - /// Panics on out of bounds. - #[inline(always)] - #[cfg_attr(debug_assertions, track_caller)] - pub fn slice_mut(&mut self, offset: usize, size: usize) -> &mut [u8] { - let _len = self.len(); - match self.data.get_mut(offset..offset + size) { - Some(slice) => slice, - None => debug_unreachable!("slice_mut OOB: {offset}..{size}; len: {_len}"), - } - } - - /// Sets the `byte` at the given `index`. - /// - /// Panics when `index` is out of bounds. - #[inline(always)] - #[cfg_attr(debug_assertions, track_caller)] - pub fn set_byte(&mut self, index: usize, byte: u8) { - match self.data.get_mut(index) { - Some(b) => *b = byte, - None => debug_unreachable!("set_byte OOB: {index}; len: {}", self.len()), - } - } - - /// Sets the given `value` to the memory region at the given `offset`. - /// - /// Panics on out of bounds. - #[inline(always)] - #[cfg_attr(debug_assertions, track_caller)] - pub fn set_u256(&mut self, offset: usize, value: U256) { - self.set(offset, &value.to_be_bytes::<32>()); - } - - /// Set memory region at given `offset`. - /// - /// Panics on out of bounds. - #[inline(always)] - #[cfg_attr(debug_assertions, track_caller)] - pub fn set(&mut self, offset: usize, value: &[u8]) { - if !value.is_empty() { - self.slice_mut(offset, value.len()).copy_from_slice(value); - } - } - - /// Set memory from data. Our memory offset+len is expected to be correct but we - /// are doing bound checks on data/data_offeset/len and zeroing parts that is not copied. - /// - /// Panics on out of bounds. - #[inline(always)] - #[cfg_attr(debug_assertions, track_caller)] - pub fn set_data(&mut self, memory_offset: usize, data_offset: usize, len: usize, data: &[u8]) { - if data_offset >= data.len() { - // nullify all memory slots - self.slice_mut(memory_offset, len).fill(0); - return; - } - let data_end = min(data_offset + len, data.len()); - let data_len = data_end - data_offset; - debug_assert!(data_offset < data.len() && data_end <= data.len()); - let data = unsafe { data.get_unchecked(data_offset..data_end) }; - self.slice_mut(memory_offset, data_len) - .copy_from_slice(data); - - // nullify rest of memory slots - // Safety: Memory is assumed to be valid. And it is commented where that assumption is made - self.slice_mut(memory_offset + data_len, len - data_len) - .fill(0); - } - - /// Copies elements from one part of the memory to another part of itself. - /// - /// Panics on out of bounds. - #[inline(always)] - #[cfg_attr(debug_assertions, track_caller)] - pub fn copy(&mut self, dst: usize, src: usize, len: usize) { - self.data.copy_within(src..src + len, dst); - } -} - -/// Rounds up `x` to the closest multiple of 32. If `x % 32 == 0` then `x` is returned. -#[inline] -pub 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 super::*; - - #[test] - fn test_copy() { - // Create a sample memory instance - let mut memory = Memory::new(); - - // Set up initial memory data - let data: Vec = vec![1, 2, 3, 4, 5, 6, 7, 8, 9, 10]; - memory.resize(data.len()); - memory.set_data(0, 0, data.len(), &data); - - // Perform a copy operation - memory.copy(5, 0, 4); - - // Verify the copied data - let copied_data = memory.slice(5, 4); - assert_eq!(copied_data, &[1, 2, 3, 4]); - } - - #[test] - fn test_next_multiple_of_32() { - // next_multiple_of_32 returns x when it is a multiple of 32 - for i in 0..32 { - let x = i * 32; - assert_eq!(Some(x), next_multiple_of_32(x)); - } - - // next_multiple_of_32 rounds up to the nearest multiple of 32 when `x % 32 != 0` - for x in 0..1024 { - if x % 32 == 0 { - continue; - } - let next_multiple = x + 32 - (x % 32); - assert_eq!(Some(next_multiple), next_multiple_of_32(x)); - } - } -} From b6ed3376b44627843de15826213f92f569db09b5 Mon Sep 17 00:00:00 2001 From: Lorenzo Feroleto Date: Fri, 6 Oct 2023 10:39:00 +0200 Subject: [PATCH 18/19] chore(SharedMemory): docs --- bins/revme/ethtests | 1 - crates/interpreter/src/interpreter/shared_memory.rs | 4 +--- 2 files changed, 1 insertion(+), 4 deletions(-) delete mode 160000 bins/revme/ethtests diff --git a/bins/revme/ethtests b/bins/revme/ethtests deleted file mode 160000 index 661356317a..0000000000 --- a/bins/revme/ethtests +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 661356317ac6df52208d54187e692472a25a01f8 diff --git a/crates/interpreter/src/interpreter/shared_memory.rs b/crates/interpreter/src/interpreter/shared_memory.rs index a449ac474a..8c54ff50e0 100644 --- a/crates/interpreter/src/interpreter/shared_memory.rs +++ b/crates/interpreter/src/interpreter/shared_memory.rs @@ -39,9 +39,7 @@ impl Default for SharedMemory { impl SharedMemory { /// Allocate memory to be shared between calls. - /// Memory size is estimated using https://2π.com/22/eth-max-mem - /// which depends on transaction [gas_limit]. - /// Maximum allocation size is 2^32 - 1 bytes; + /// Initial capacity is 4KiB which is expanded if needed pub fn new() -> Self { Self { data: Vec::with_capacity(4 * 1024), // from evmone From b82ea0d0e99029e94acc92dc19426f1394354901 Mon Sep 17 00:00:00 2001 From: Lorenzo Feroleto Date: Tue, 10 Oct 2023 13:57:55 +0200 Subject: [PATCH 19/19] fix(SharedMemory): restore cfg memory limit --- crates/interpreter/src/instructions/macros.rs | 6 +++++ .../src/interpreter/shared_memory.rs | 25 ++++++++++++++++++- crates/revm/src/evm_impl.rs | 3 +++ 3 files changed, 33 insertions(+), 1 deletion(-) diff --git a/crates/interpreter/src/instructions/macros.rs b/crates/interpreter/src/instructions/macros.rs index 2229ed0ee8..1207c0b392 100644 --- a/crates/interpreter/src/instructions/macros.rs +++ b/crates/interpreter/src/instructions/macros.rs @@ -55,6 +55,12 @@ macro_rules! shared_memory_resize { if let Some(new_size) = crate::interpreter::next_multiple_of_32($offset.saturating_add($len)) { + #[cfg(feature = "memory_limit")] + if $interp.shared_memory.limit_reached(new_size) { + $interp.instruction_result = InstructionResult::MemoryLimitOOG; + return; + } + if new_size > $interp.shared_memory.len() { if crate::USE_GAS { let num_bytes = new_size / 32; diff --git a/crates/interpreter/src/interpreter/shared_memory.rs b/crates/interpreter/src/interpreter/shared_memory.rs index 8c54ff50e0..2b3fbafe51 100644 --- a/crates/interpreter/src/interpreter/shared_memory.rs +++ b/crates/interpreter/src/interpreter/shared_memory.rs @@ -18,6 +18,9 @@ pub struct SharedMemory { checkpoints: Vec, /// How much memory has been used in the current context current_len: usize, + /// Memory limit. See [`crate::CfgEnv`]. + #[cfg(feature = "memory_limit")] + pub memory_limit: u64, } impl fmt::Debug for SharedMemory { @@ -45,9 +48,29 @@ impl SharedMemory { data: Vec::with_capacity(4 * 1024), // from evmone checkpoints: Vec::with_capacity(32), current_len: 0, + #[cfg(feature = "memory_limit")] + memory_limit: u64::MAX, } } + /// Allocate memory to be shared between calls, with `memory_limit` + /// as upper bound for allocation size. + /// Initial capacity is 4KiB which is expanded if needed + #[cfg(feature = "memory_limit")] + pub fn new_with_memory_limit(memory_limit: u64) -> Self { + Self { + memory_limit, + ..Self::new() + } + } + + /// Returns true if the `new_size` for the current context memory will + /// make the shared buffer length exceed the `memory_limit` + #[cfg(feature = "memory_limit")] + pub fn limit_reached(&self, new_size: usize) -> bool { + (self.last_checkpoint() + new_size) as u64 > self.memory_limit + } + /// Prepares the shared memory for a new context pub fn new_context_memory(&mut self) { let base_offset = self.last_checkpoint(); @@ -221,7 +244,7 @@ impl SharedMemory { /// Get the last memory checkpoint #[inline(always)] fn last_checkpoint(&self) -> usize { - *self.checkpoints.last().unwrap_or(&0) + self.checkpoints.last().cloned().unwrap_or_default() } } diff --git a/crates/revm/src/evm_impl.rs b/crates/revm/src/evm_impl.rs index cc95aed045..c395e68bec 100644 --- a/crates/revm/src/evm_impl.rs +++ b/crates/revm/src/evm_impl.rs @@ -288,6 +288,9 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> Transact let transact_gas_limit = tx_gas_limit - initial_gas_spend; + #[cfg(feature = "memory_limit")] + let mut shared_memory = SharedMemory::new_with_memory_limit(self.data.env.cfg.memory_limit); + #[cfg(not(feature = "memory_limit"))] let mut shared_memory = SharedMemory::new(); // call inner handling of call/create