From d98255953e9bfc97f67ca65c6db631401427a6f6 Mon Sep 17 00:00:00 2001 From: Oliver Nordbjerg Date: Mon, 27 Jun 2022 11:00:52 +0200 Subject: [PATCH] refactor: extract `build_ic_map` --- evm/src/executor/inspector/coverage.rs | 42 +++++--------------------- evm/src/executor/inspector/debugger.rs | 31 +++---------------- evm/src/utils.rs | 28 +++++++++++++++++ 3 files changed, 40 insertions(+), 61 deletions(-) diff --git a/evm/src/executor/inspector/coverage.rs b/evm/src/executor/inspector/coverage.rs index 5c4dbeb0af9b..c0baa046321d 100644 --- a/evm/src/executor/inspector/coverage.rs +++ b/evm/src/executor/inspector/coverage.rs @@ -1,10 +1,9 @@ -use crate::{coverage::HitMaps, executor::inspector::utils::get_create_address}; +use crate::{ + coverage::HitMaps, executor::inspector::utils::get_create_address, utils::build_ic_map, +}; use bytes::Bytes; use ethers::types::Address; -use revm::{ - opcode, spec_opcode_gas, CallInputs, CreateInputs, Database, EVMData, Gas, Inspector, - Interpreter, Return, SpecId, -}; +use revm::{CallInputs, CreateInputs, Database, EVMData, Gas, Inspector, Interpreter, Return}; use std::collections::BTreeMap; #[derive(Default, Debug)] @@ -25,34 +24,6 @@ pub struct CoverageCollector { } impl CoverageCollector { - /// Builds the instruction counter map for the given bytecode. - // TODO: Some of the same logic is performed in REVM, but then later discarded. We should - // investigate if we can reuse it - // TODO: Duplicate code of the debugger inspector - pub fn build_ic_map(&mut self, spec: SpecId, code: &Bytes) { - if let Some(context) = self.context.last() { - let opcode_infos = spec_opcode_gas(spec); - let mut ic_map: BTreeMap = BTreeMap::new(); - - let mut i = 0; - let mut cumulative_push_size = 0; - while i < code.len() { - let op = code[i]; - ic_map.insert(i, i - cumulative_push_size); - if opcode_infos[op as usize].is_push { - // Skip the push bytes. - // - // For more context on the math, see: https://github.com/bluealloy/revm/blob/007b8807b5ad7705d3cacce4d92b89d880a83301/crates/revm/src/interpreter/contract.rs#L114-L115 - i += (op - opcode::PUSH1 + 1) as usize; - cumulative_push_size += (op - opcode::PUSH1 + 1) as usize; - } - i += 1; - } - - self.ic_map.insert(*context, ic_map); - } - } - pub fn enter(&mut self, address: Address) { self.context.push(address); } @@ -87,7 +58,10 @@ where // TODO: This is rebuilt for all contracts every time. We should only run this if the IC // map for a given address does not exist, *but* we need to account for the fact that the // code given by the interpreter may either be the contract init code, or the runtime code. - self.build_ic_map(data.env.cfg.spec_id, &interp.contract().code); + if let Some(context) = self.context.last() { + self.ic_map + .insert(*context, build_ic_map(data.env.cfg.spec_id, &interp.contract().code)); + } Return::Continue } diff --git a/evm/src/executor/inspector/debugger.rs b/evm/src/executor/inspector/debugger.rs index c993a9463168..5de2682765f4 100644 --- a/evm/src/executor/inspector/debugger.rs +++ b/evm/src/executor/inspector/debugger.rs @@ -4,13 +4,14 @@ use crate::{ inspector::utils::{gas_used, get_create_address}, CHEATCODE_ADDRESS, }, + utils::build_ic_map, CallKind, }; use bytes::Bytes; use ethers::types::Address; use revm::{ opcode, spec_opcode_gas, CallInputs, CreateInputs, Database, EVMData, Gas, Inspector, - Interpreter, Memory, Return, SpecId, + Interpreter, Memory, Return, }; use std::collections::BTreeMap; @@ -48,31 +49,6 @@ pub struct Debugger { } impl Debugger { - /// Builds the instruction counter map for the given bytecode. - // TODO: Some of the same logic is performed in REVM, but then later discarded. We should - // investigate if we can reuse it - pub fn build_ic_map(&mut self, spec: SpecId, code: &Bytes) { - let opcode_infos = spec_opcode_gas(spec); - let mut ic_map: BTreeMap = BTreeMap::new(); - - let mut i = 0; - let mut cumulative_push_size = 0; - while i < code.len() { - let op = code[i]; - ic_map.insert(i, i - cumulative_push_size); - if opcode_infos[op as usize].is_push { - // Skip the push bytes. - // - // For more context on the math, see: https://github.com/bluealloy/revm/blob/007b8807b5ad7705d3cacce4d92b89d880a83301/crates/revm/src/interpreter/contract.rs#L114-L115 - i += (op - opcode::PUSH1 + 1) as usize; - cumulative_push_size += (op - opcode::PUSH1 + 1) as usize; - } - i += 1; - } - - self.ic_map.insert(self.context, ic_map); - } - /// Enters a new execution context. pub fn enter(&mut self, depth: usize, address: Address, kind: CallKind) { self.context = address; @@ -127,7 +103,8 @@ where // TODO: This is rebuilt for all contracts every time. We should only run this if the IC // map for a given address does not exist, *but* we need to account for the fact that the // code given by the interpreter may either be the contract init code, or the runtime code. - self.build_ic_map(data.env.cfg.spec_id, &interp.contract().code); + self.ic_map + .insert(self.context, build_ic_map(data.env.cfg.spec_id, &interp.contract().code)); self.previous_gas_block = interp.contract.first_gas_block(); Return::Continue } diff --git a/evm/src/utils.rs b/evm/src/utils.rs index 5bcee7044c40..bb7a13c19676 100644 --- a/evm/src/utils.rs +++ b/evm/src/utils.rs @@ -1,4 +1,7 @@ +use bytes::Bytes; use ethers::prelude::{H256, U256}; +use revm::{opcode, spec_opcode_gas, SpecId}; +use std::collections::BTreeMap; /// Small helper function to convert [U256] into [H256]. pub fn u256_to_h256_le(u: U256) -> H256 { @@ -23,3 +26,28 @@ pub fn h256_to_u256_be(storage: H256) -> U256 { pub fn h256_to_u256_le(storage: H256) -> U256 { U256::from_little_endian(storage.as_bytes()) } + +/// Builds the instruction counter map for the given bytecode. +// TODO: Some of the same logic is performed in REVM, but then later discarded. We should +// investigate if we can reuse it +pub fn build_ic_map(spec: SpecId, code: &Bytes) -> BTreeMap { + let opcode_infos = spec_opcode_gas(spec); + let mut ic_map: BTreeMap = BTreeMap::new(); + + let mut i = 0; + let mut cumulative_push_size = 0; + while i < code.len() { + let op = code[i]; + ic_map.insert(i, i - cumulative_push_size); + if opcode_infos[op as usize].is_push { + // Skip the push bytes. + // + // For more context on the math, see: https://github.com/bluealloy/revm/blob/007b8807b5ad7705d3cacce4d92b89d880a83301/crates/revm/src/interpreter/contract.rs#L114-L115 + i += (op - opcode::PUSH1 + 1) as usize; + cumulative_push_size += (op - opcode::PUSH1 + 1) as usize; + } + i += 1; + } + + ic_map +}