feat: resolve the memory once

fvm/src/call_manager/default.rs

@@ -12,7 +12,7 @@ use super::{Backtrace, CallManager, InvocationResult, NO_DATA_BLOCK_ID};
 use crate::call_manager::backtrace::Frame;
 use crate::call_manager::FinishRet;
 use crate::gas::GasTracker;
-use crate::kernel::{ClassifyResult, ExecutionError, Kernel, Result, SyscallError};
+use crate::kernel::{ExecutionError, Kernel, Result, SyscallError};
 use crate::machine::Machine;
 use crate::syscalls::error::Abort;
 use crate::syscalls::{charge_for_exec, update_gas_available};
@@ -333,18 +333,21 @@ where
             // Make a store.
             let mut store = engine.new_store(kernel);
 
-            // Instantiate the module.
-            let instance = match engine
-                .get_instance(&mut store, &state.code)
-                .and_then(|i| i.context("actor code not found"))
-                .or_fatal()
-            {
-                Ok(ret) => ret,
-                Err(err) => return (Err(err), store.into_data().kernel.into_call_manager()),
-            };
-
             // From this point on, there are no more syscall errors, only aborts.
             let result: std::result::Result<RawBytes, Abort> = (|| {
+                // Instantiate the module.
+                let instance = engine
+                    .get_instance(&mut store, &state.code)
+                    .and_then(|i| i.context("actor code not found"))
+                    .map_err(Abort::Fatal)?;
+
+                // Resolve and store a reference to the exported memory.
+                let memory = instance
+                    .get_memory(&mut store, "memory")
+                    .context("actor has no memory export")
+                    .map_err(Abort::Fatal)?;
+                store.data_mut().memory = memory;
+
                 // Lookup the invoke method.
                 let invoke: wasmtime::TypedFunc<(u32,), u32> = instance
                     .get_typed_func(&mut store, "invoke")

fvm/src/machine/engine.rs

@@ -7,7 +7,7 @@ use anyhow::{anyhow, Context};
 use cid::Cid;
 use fvm_ipld_blockstore::Blockstore;
 use fvm_wasm_instrument::gas_metering::GAS_COUNTER_NAME;
-use wasmtime::{Global, GlobalType, Linker, Module, Mutability, Val, ValType};
+use wasmtime::{Global, GlobalType, Linker, Memory, MemoryType, Module, Mutability, Val, ValType};
 
 use crate::gas::WasmGasPrices;
 use crate::machine::NetworkConfig;
@@ -119,9 +119,13 @@ pub fn default_wasmtime_config() -> wasmtime::Config {
 struct EngineInner {
     engine: wasmtime::Engine,
 
-    /// dummy gas global used in store costructor to avoid making
-    /// InvocationData.avail_gas_global an Option
+    /// These two fields are used used in the store constructor to avoid resolve a chicken & egg
+    /// situation: We need the store before we can get the real values, but we need to create the
+    /// `InvocationData` before we can make the store.
+    ///
+    /// Alternatively, we could use `Option`s. But then we need to unwrap everywhere.
     dummy_gas_global: Global,
+    dummy_memory: Memory,
 
     module_cache: Mutex<HashMap<Cid, Module>>,
     instance_cache: Mutex<anymap::Map<dyn anymap::any::Any + Send>>,
@@ -150,8 +154,12 @@ impl Engine {
         let dummy_gg = Global::new(&mut dummy_store, gg_type, Val::I64(0))
             .expect("failed to create dummy gas global");
 
+        let dummy_memory = Memory::new(&mut dummy_store, MemoryType::new(0, Some(0)))
+            .expect("failed to create dummy memory");
+
         Ok(Engine(Arc::new(EngineInner {
             engine,
+            dummy_memory,
             dummy_gas_global: dummy_gg,
             module_cache: Default::default(),
             instance_cache: Mutex::new(anymap::Map::new()),
@@ -299,6 +307,7 @@ impl Engine {
             None => return Ok(None),
         };
         let instance = cache.linker.instantiate(&mut *store, module)?;
+
         Ok(Some(instance))
     }
 
@@ -309,6 +318,7 @@ impl Engine {
             last_error: None,
             avail_gas_global: self.0.dummy_gas_global,
             last_milligas_available: 0,
+            memory: self.0.dummy_memory,
         };
 
         let mut store = wasmtime::Store::new(&self.0.engine, id);

fvm/src/syscalls/bind.rs

@@ -2,7 +2,7 @@ use std::mem;
 
 use fvm_shared::error::ErrorNumber;
 use fvm_shared::sys::SyscallSafe;
-use wasmtime::{Caller, Linker, Trap, WasmTy};
+use wasmtime::{Caller, Linker, WasmTy};
 
 use super::context::Memory;
 use super::error::Abort;
@@ -89,13 +89,10 @@ where
 
 fn memory_and_data<'a, K: Kernel>(
     caller: &'a mut Caller<'_, InvocationData<K>>,
-) -> Result<(&'a mut Memory, &'a mut InvocationData<K>), Trap> {
-    let (mem, data) = caller
-        .get_export("memory")
-        .and_then(|m| m.into_memory())
-        .ok_or_else(|| Trap::new("failed to lookup actor memory"))?
-        .data_and_store_mut(caller);
-    Ok((Memory::new(mem), data))
+) -> (&'a mut Memory, &'a mut InvocationData<K>) {
+    let memory_handle = caller.data().memory;
+    let (mem, data) = memory_handle.data_and_store_mut(caller);
+    (Memory::new(mem), data)
 }
 
 // Unfortunately, we can't implement this for _all_ functions. So we implement it for functions of up to 6 arguments.
@@ -120,7 +117,7 @@ macro_rules! impl_bind_syscalls {
                     self.func_wrap(module, name, move |mut caller: Caller<'_, InvocationData<K>> $(, $t: $t)*| {
                         charge_for_exec(&mut caller)?;
 
-                        let (mut memory, mut data) = memory_and_data(&mut caller)?;
+                        let (mut memory, mut data) = memory_and_data(&mut caller);
                         let ctx = Context{kernel: &mut data.kernel, memory: &mut memory};
                         let out = syscall(ctx $(, $t)*).into();
 
@@ -148,7 +145,7 @@ macro_rules! impl_bind_syscalls {
                     self.func_wrap(module, name, move |mut caller: Caller<'_, InvocationData<K>>, ret: u32 $(, $t: $t)*| {
                         charge_for_exec(&mut caller)?;
 
-                        let (mut memory, mut data) = memory_and_data(&mut caller)?;
+                        let (mut memory, mut data) = memory_and_data(&mut caller);
 
                         // We need to check to make sure we can store the return value _before_ we do anything.
                         if (ret as u64) > (memory.len() as u64)

fvm/src/syscalls/mod.rs

@@ -2,7 +2,7 @@ use std::mem;
 
 use anyhow::{anyhow, Context as _};
 use cid::Cid;
-use wasmtime::{AsContextMut, Global, Linker, Val};
+use wasmtime::{AsContextMut, Global, Linker, Memory, Val};
 
 use crate::call_manager::backtrace;
 use crate::kernel::ExecutionError;
@@ -30,16 +30,21 @@ pub(self) use context::Context;
 pub struct InvocationData<K> {
     /// The kernel on which this actor is being executed.
     pub kernel: K,
+
     /// The last-seen syscall error. This error is considered the abort "cause" if an actor aborts
     /// after receiving this error without calling any other syscalls.
     pub last_error: Option<backtrace::Cause>,
 
     /// The global containing remaining available gas.
     pub avail_gas_global: Global,
+
     /// The last-set milligas limit. When `charge_for_exec` is called, we charge for the
     /// _difference_ between the current gas available (the wasm global) and the
     /// `last_milligas_available`.
     pub last_milligas_available: i64,
+
+    /// The invocation's imported "memory".
+    pub memory: Memory,
 }
 
 pub fn update_gas_available(

testing/integration/tests/lib.rs

@@ -73,6 +73,7 @@ fn out_of_gas() {
     const WAT: &str = r#"
     ;; Mock invoke function
     (module
+      (memory (export "memory") 1)
       (func (export "invoke") (param $x i32) (result i32)
         (loop
             (br 0)
@@ -127,6 +128,7 @@ fn out_of_stack() {
     const WAT: &str = r#"
     ;; Mock invoke function
     (module
+      (memory (export "memory") 1)
       (func (export "invoke") (param $x i32) (result i32)
         (i64.const 123)
         (call 1)