From 44c00b1f74feee8f809f47e82d6f07086d49478f Mon Sep 17 00:00:00 2001 From: Andronik Ordian Date: Thu, 5 Sep 2019 16:11:51 +0200 Subject: [PATCH] [trace] introduce trace failed to Ext (#11019) * [trace] add trace_failed to Ext, manage stack of trace data * [evm] call trace_failed only if self.do_trace * [evm] add a comment about do_trace set to true * [vm] improve the doc in trace_prepare_execute * [trace] add the bounds check back --- ethcore/evm/src/interpreter/mod.rs | 12 ++++++++- ethcore/machine/src/externalities.rs | 4 +++ ethcore/trace/src/executive_tracer.rs | 39 ++++++++++++++------------- ethcore/trace/src/lib.rs | 3 +++ ethcore/vm/src/ext.rs | 4 +++ 5 files changed, 42 insertions(+), 20 deletions(-) diff --git a/ethcore/evm/src/interpreter/mod.rs b/ethcore/evm/src/interpreter/mod.rs index 472601395..ba64dbaed 100644 --- a/ethcore/evm/src/interpreter/mod.rs +++ b/ethcore/evm/src/interpreter/mod.rs @@ -279,6 +279,8 @@ impl Interpreter { cache, params, reader, informant, valid_jump_destinations, gasometer, stack, done: false, + // Overridden in `step_inner` based on + // the result of `ext.trace_next_instruction`. do_trace: true, mem: Vec::new(), return_data: ReturnData::empty(), @@ -353,6 +355,9 @@ impl Interpreter { ext.trace_prepare_execute(self.reader.position - 1, opcode, requirements.gas_cost.as_u256(), Self::mem_written(instruction, &self.stack), Self::store_written(instruction, &self.stack)); } if let Err(e) = self.gasometer.as_mut().expect(GASOMETER_PROOF).verify_gas(&requirements.gas_cost) { + if self.do_trace { + ext.trace_failed(); + } return InterpreterResult::Done(Err(e)); } self.mem.expand(requirements.memory_required_size); @@ -366,7 +371,12 @@ impl Interpreter { let result = match self.exec_instruction( current_gas, ext, instruction, requirements.provide_gas ) { - Err(x) => return InterpreterResult::Done(Err(x)), + Err(x) => { + if self.do_trace { + ext.trace_failed(); + } + return InterpreterResult::Done(Err(x)); + }, Ok(x) => x, }; evm_debug!({ self.informant.after_instruction(instruction) }); diff --git a/ethcore/machine/src/externalities.rs b/ethcore/machine/src/externalities.rs index b951eba16..adab87b00 100644 --- a/ethcore/machine/src/externalities.rs +++ b/ethcore/machine/src/externalities.rs @@ -440,6 +440,10 @@ impl<'a, T: 'a, V: 'a, B: 'a> Ext for Externalities<'a, T, V, B> self.vm_tracer.trace_prepare_execute(pc, instruction, gas_cost, mem_written, store_written) } + fn trace_failed(&mut self) { + self.vm_tracer.trace_failed(); + } + fn trace_executed(&mut self, gas_used: U256, stack_push: &[U256], mem: &[u8]) { self.vm_tracer.trace_executed(gas_used, stack_push, mem) } diff --git a/ethcore/trace/src/executive_tracer.rs b/ethcore/trace/src/executive_tracer.rs index 23739a34c..7c6dbc662 100644 --- a/ethcore/trace/src/executive_tracer.rs +++ b/ethcore/trace/src/executive_tracer.rs @@ -16,6 +16,7 @@ //! Simple executive tracer. +use std::cmp::min; use ethereum_types::{U256, Address}; use vm::{Error as VmError, ActionParams}; use log::{debug, warn}; @@ -196,12 +197,16 @@ impl Tracer for ExecutiveTracer { } } +struct TraceData { + mem_written: Option<(usize, usize)>, + store_written: Option<(U256, U256)>, +} + /// Simple VM tracer. Traces all operations. pub struct ExecutiveVMTracer { data: VMTrace, depth: usize, - last_mem_written: Option<(usize, usize)>, - last_store_written: Option<(U256, U256)>, + trace_stack: Vec, } impl ExecutiveVMTracer { @@ -215,8 +220,7 @@ impl ExecutiveVMTracer { subs: vec![], }, depth: 0, - last_mem_written: None, - last_store_written: None, + trace_stack: vec![], } } @@ -243,30 +247,27 @@ impl VMTracer for ExecutiveVMTracer { executed: None, }); }); - self.last_mem_written = mem_written; - self.last_store_written = store_written; + self.trace_stack.push(TraceData { mem_written, store_written }); + } + + fn trace_failed(&mut self) { + let _ = self.trace_stack.pop().expect("pushed in trace_prepare_execute; qed"); } fn trace_executed(&mut self, gas_used: U256, stack_push: &[U256], mem: &[u8]) { - let mem_diff = self.last_mem_written.take().map(|(o, s)| { + let TraceData { mem_written, store_written } = self.trace_stack.pop().expect("pushed in trace_prepare_execute; qed"); + let mem_diff = mem_written.map(|(o, s)| { if o + s > mem.len() { - warn!( - target: "trace", - "Last mem written is out of bounds {} (mem is {})", - o + s, - mem.len(), - ); - (o, &[][..]) - } else { - (o, &(mem[o..o+s])) + warn!(target: "trace", "mem_written is out of bounds"); } + (o, &mem[min(mem.len(), o)..min(o + s, mem.len())]) }); - let store_diff = self.last_store_written.take(); + let store_diff = store_written; Self::with_trace_in_depth(&mut self.data, self.depth, move |trace| { let ex = VMExecutedOperation { gas_used: gas_used, - stack_push: stack_push.iter().cloned().collect(), - mem_diff: mem_diff.map(|(s, r)| MemoryDiff { offset: s, data: r.iter().cloned().collect() }), + stack_push: stack_push.to_vec(), + mem_diff: mem_diff.map(|(s, r)| MemoryDiff { offset: s, data: r.to_vec() }), store_diff: store_diff.map(|(l, v)| StorageDiff { location: l, value: v }), }; trace.operations.last_mut().expect("trace_executed is always called after a trace_prepare_execute; trace.operations cannot be empty; qed").executed = Some(ex); diff --git a/ethcore/trace/src/lib.rs b/ethcore/trace/src/lib.rs index 91fead6a8..9ff02a7a7 100644 --- a/ethcore/trace/src/lib.rs +++ b/ethcore/trace/src/lib.rs @@ -93,6 +93,9 @@ pub trait VMTracer: Send { /// Trace the preparation to execute a single valid instruction. fn trace_prepare_execute(&mut self, _pc: usize, _instruction: u8, _gas_cost: U256, _mem_written: Option<(usize, usize)>, _store_written: Option<(U256, U256)>) {} + /// Trace the execution failure of a single instruction. + fn trace_failed(&mut self) {} + /// Trace the finalised execution of a single valid instruction. fn trace_executed(&mut self, _gas_used: U256, _stack_push: &[U256], _mem: &[u8]) {} diff --git a/ethcore/vm/src/ext.rs b/ethcore/vm/src/ext.rs index 68e5822e4..bb153acff 100644 --- a/ethcore/vm/src/ext.rs +++ b/ethcore/vm/src/ext.rs @@ -164,8 +164,12 @@ pub trait Ext { fn trace_next_instruction(&mut self, _pc: usize, _instruction: u8, _current_gas: U256) -> bool { false } /// Prepare to trace an operation. Passthrough for the VM trace. + /// For each call of `trace_prepare_execute` either `trace_failed` or `trace_executed` MUST be called. fn trace_prepare_execute(&mut self, _pc: usize, _instruction: u8, _gas_cost: U256, _mem_written: Option<(usize, usize)>, _store_written: Option<(U256, U256)>) {} + /// Trace the execution failure of a single instruction. + fn trace_failed(&mut self) {} + /// Trace the finalised execution of a single instruction. fn trace_executed(&mut self, _gas_used: U256, _stack_push: &[U256], _mem: &[u8]) {}