Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Execution trace recording #881

Closed
wants to merge 13 commits into from
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

## [Unreleased]

### Added

- [881](https://github.com/FuelLabs/fuel-vm/pull/881): Support for capturing execution traces.

### Fixed
- [879](https://github.com/FuelLabs/fuel-vm/pull/879): Debugger state wasn't propagated in contract contexts.
- [878](https://github.com/FuelLabs/fuel-vm/pull/878): Fix the transaction de/serialization that wasn't backward compatible with the addition of the new policy.
Expand Down
2 changes: 2 additions & 0 deletions fuel-vm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ percent-encoding = { version = "2.3", features = [
primitive-types = { version = "0.12", default-features = false }
rand = { version = "0.8", optional = true }
serde = { version = "1.0", features = ["derive", "rc"], optional = true }
serde-big-array = { version = "0.5.1", optional = true }
serde_with = { version = "3.7", optional = true }
sha3 = { version = "0.10", default-features = false }
static_assertions = "1.1"
Expand Down Expand Up @@ -95,6 +96,7 @@ da-compression = ["fuel-compression", "fuel-tx/da-compression"]
serde = [
"dep:serde",
"dep:serde_with",
"dep:serde-big-array",
"hashbrown/serde",
"fuel-asm/serde",
"fuel-types/serde",
Expand Down
5 changes: 5 additions & 0 deletions fuel-vm/src/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ use core::{
mem,
ops::Index,
};
use trace::ExecutionTracer;

use fuel_asm::{
Flags,
Expand Down Expand Up @@ -67,6 +68,7 @@ mod memory;
mod metadata;
mod post_execution;
mod receipts;
pub mod trace;

mod debug;
mod ecal;
Expand All @@ -86,6 +88,7 @@ pub use memory::{
Memory,
MemoryInstance,
MemoryRange,
MemorySliceChange,
};

use crate::checked_transaction::{
Expand Down Expand Up @@ -132,6 +135,8 @@ pub struct Interpreter<M, S, Tx = (), Ecal = NotSupportedEcal> {
context: Context,
balances: RuntimeBalances,
profiler: Profiler,
/// `None` if the excution trace is not enabled.
trace: Option<ExecutionTracer<M>>,
interpreter_params: InterpreterParams,
/// `PanicContext` after the latest execution. It is consumed by
/// `append_panic_receipt` and is `PanicContext::None` after consumption.
Expand Down
1 change: 1 addition & 0 deletions fuel-vm/src/interpreter/constructors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ where
context: Context::default(),
balances: RuntimeBalances::default(),
profiler: Profiler::default(),
trace: None,
interpreter_params,
panic_context: PanicContext::None,
ecal_state,
Expand Down
2 changes: 2 additions & 0 deletions fuel-vm/src/interpreter/diff/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ where
balances: self.balances,
panic_context: self.panic_context,
profiler: self.profiler,
trace: self.trace,
interpreter_params: self.interpreter_params,
ecal_state: self.ecal_state,
}
Expand Down Expand Up @@ -199,6 +200,7 @@ where
balances: self.balances,
panic_context: self.panic_context,
profiler: self.profiler,
trace: self.trace,
interpreter_params: self.interpreter_params,
ecal_state: self.ecal_state,
}
Expand Down
11 changes: 9 additions & 2 deletions fuel-vm/src/interpreter/executors/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,15 @@ where
}
}

self.instruction_inner(raw.into())
.map_err(|e| InterpreterError::from_runtime(e, raw.into()))
let result = self
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it will be more beneficial if, on the creation of the interpreter, we will pass some object that implements traits like Tracer or something.

trait Tracer<VM> {
    fn trace_instruction_before_execution(
        &mut self,
        instruction: &Intstuction,
        vm: &mut VM,
    );

    fn trace_instruction_after_execution(
        &mut self,
        instruction: &Intstuction,
        vm: &mut VM,
    );
}

In this case, we will have more flexibility in the implementation and can define logic to get traces on the fuel-core side. In the case of tooling, they can maybe create a local debugger. In the case of indexers, they can index something inside of these functions.

Of course, this solution means that we need to have getters for all internal fields, including MemoryInstance.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think nobody is actually going to use this, and it's an unnecessary complicaton. But it would be nice to be wrong about this, and it's not that much extra complication.

Fortunately we already support almost all fields this needs, as ECAL support is also done like this.

.instruction_inner(raw.into())
.map_err(|e| InterpreterError::from_runtime(e, raw.into()));

if !matches!(result, Err(_) | Ok(ExecuteState::DebugEvent(_))) {
self.record_trace_after_instruction();
}

result
}

fn instruction_inner(
Expand Down
61 changes: 56 additions & 5 deletions fuel-vm/src/interpreter/memory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,10 @@ use crate::error::{
IoResult,
RuntimeError,
};
use alloc::vec::Vec;
use alloc::{
vec,
vec::Vec,
};
use fuel_storage::{
Mappable,
StorageRead,
Expand All @@ -54,6 +57,9 @@ mod impl_tests;
#[cfg(test)]
mod allocation_tests;

#[cfg(test)]
mod diff_tests;

#[cfg(test)]
mod stack_tests;

Expand Down Expand Up @@ -130,6 +136,15 @@ impl MemoryInstance {
self.hp = MEM_SIZE;
}

/// Make memory equal to another instance, keeping the original allocations.
pub fn make_equal(&mut self, other: &Self) {
self.stack.truncate(0);
self.stack.extend_from_slice(&other.stack);
self.hp = other.hp;
self.heap.truncate(0);
self.heap.extend_from_slice(&other.heap);
}

/// Offset of the heap section
fn heap_offset(&self) -> usize {
MEM_SIZE.saturating_sub(self.heap.len())
Expand Down Expand Up @@ -403,6 +418,37 @@ impl MemoryInstance {
&self.heap
}

/// Diff of from `self` to `new` memory state.
/// Panics if new instance is not possible to reach from the current one,
/// for instance if it has smaller stack or heap allocation.
pub fn diff_patches(&self, new: &MemoryInstance) -> Vec<MemorySliceChange> {
let mut changes = Vec::new();
let mut current_change: Option<MemorySliceChange> = None;

for i in 0..MEM_SIZE {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh, it will be very expensive to do 10^7 iterations per each instruction=D It would be nice to have something more performant=)

If we go for now with Tracer approach, we can decide later in the fuel-core how do we want to do that in an optimal way

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm very aware of this. I have lots of ideas on how to optimize this, and I'm going to write some benchmarks to see what approach is the right one.

let [old_value] = self.read_bytes(i).unwrap_or([0]);
let [new_value] = new.read_bytes(i).unwrap_or([0]);

if old_value != new_value {
if let Some(change) = current_change.as_mut() {
change.data.push(new_value);
} else {
current_change = Some(MemorySliceChange {
global_start: i,
data: vec![new_value],
});
}
} else if let Some(change) = current_change.take() {
changes.push(change);
}
}
if let Some(change) = current_change.take() {
changes.push(change);
}

changes
}

/// Returns a `MemoryRollbackData` that can be used to achieve the state of the
/// `desired_memory_state` instance.
pub fn collect_rollback_data(
Expand Down Expand Up @@ -499,14 +545,19 @@ fn get_changes(
changes
}

#[derive(Debug, Clone)]
struct MemorySliceChange {
global_start: usize,
data: Vec<u8>,
/// Memory change at a specific location.
#[derive(Debug, Clone, PartialEq)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub struct MemorySliceChange {
/// Start address of the change. Global address.
pub global_start: usize,
/// Data that was changed.
pub data: Vec<u8>,
}

/// The container for the data used to rollback memory changes.
#[derive(Debug, Clone)]
#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
pub struct MemoryRollbackData {
/// Desired stack pointer.
sp: usize,
Expand Down
Loading
Loading