Merge pull request #326 from dusk-network/single-instance-per-call

One instance per contract function call

Author: Eduardo Leegwater Simões

piecrust/CHANGELOG.md

@@ -13,10 +13,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- Change to have one instance per contract function call [#325]
 - Upgrade `dusk-wasmtime` to version `17`
 
 ### Fixed
 
+- Fix bad dereferences in caused by instance reclamation [#325]
 - Fix overflow in gas limit calculation in inter-contract call
 
 ## [0.15.0] - 2024-01-24
@@ -357,6 +359,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 [#234]: https://github.com/dusk-network/piecrust/pull/234
 
 <!-- ISSUES -->
+[#325]: https://github.com/dusk-network/piecrust/issues/325
 [#301]: https://github.com/dusk-network/piecrust/issues/313
 [#301]: https://github.com/dusk-network/piecrust/issues/301
 [#296]: https://github.com/dusk-network/piecrust/issues/296

piecrust/src/imports.rs

@@ -138,12 +138,12 @@ pub(crate) fn hq(
 ) -> WasmtimeResult<u32> {
     let env = fenv.data_mut();
 
-    let instance = env.self_instance();
+    let mut instance = env.self_instance();
 
     let name_len = name_len as usize;
 
-    check_ptr(instance, name_ofs, name_len)?;
-    check_arg(instance, arg_len)?;
+    check_ptr(&instance, name_ofs, name_len)?;
+    check_arg(&instance, arg_len)?;
 
     let name = instance.with_memory(|buf| {
         // performance: use a dedicated buffer here?
@@ -163,11 +163,11 @@ pub(crate) fn hd(
 ) -> WasmtimeResult<u32> {
     let env = fenv.data_mut();
 
-    let instance = env.self_instance();
+    let mut instance = env.self_instance();
 
     let name_len = name_len as usize;
 
-    check_ptr(instance, name_ofs, name_len)?;
+    check_ptr(&instance, name_ofs, name_len)?;
 
     let name = instance.with_memory(|buf| {
         // performance: use a dedicated buffer here?
@@ -194,13 +194,13 @@ pub(crate) fn c(
 ) -> WasmtimeResult<i32> {
     let env = fenv.data_mut();
 
-    let instance = env.self_instance();
+    let mut instance = env.self_instance();
 
     let name_len = name_len as usize;
 
-    check_ptr(instance, mod_id_ofs, CONTRACT_ID_BYTES)?;
-    check_ptr(instance, name_ofs, name_len)?;
-    check_arg(instance, arg_len)?;
+    check_ptr(&instance, mod_id_ofs, CONTRACT_ID_BYTES)?;
+    check_ptr(&instance, name_ofs, name_len)?;
+    check_arg(&instance, arg_len)?;
 
     let argbuf_ofs = instance.arg_buffer_offset();
 
@@ -222,12 +222,8 @@ pub(crate) fn c(
             &memory[mod_id_ofs..][..std::mem::size_of::<ContractId>()],
         );
 
-        let callee_stack_element = env
-            .push_callstack(mod_id, callee_limit)
-            .expect("pushing to the callstack should succeed");
-        let callee = env
-            .instance(&callee_stack_element.contract_id)
-            .expect("callee instance should exist");
+        let mut callee = env.instance(mod_id)?;
+        env.push_callstack(mod_id, callee_limit, callee.mem_len());
 
         callee.snap().map_err(|err| Error::MemorySnapshotFailure {
             reason: None,
@@ -242,7 +238,7 @@ pub(crate) fn c(
         let ret_len = callee
             .call(name, arg.len() as u32, callee_limit)
             .map_err(Error::normalize)?;
-        check_arg(callee, ret_len as u32)?;
+        check_arg(&callee, ret_len as u32)?;
 
         // copy back result
         callee.read_argument(&mut memory[argbuf_ofs..][..ret_len as usize]);
@@ -291,8 +287,8 @@ pub(crate) fn emit(
 
     let topic_len = topic_len as usize;
 
-    check_ptr(instance, topic_ofs, topic_len)?;
-    check_arg(instance, arg_len)?;
+    check_ptr(&instance, topic_ofs, topic_len)?;
+    check_arg(&instance, arg_len)?;
 
     let data = instance.with_arg_buf(|buf| {
         let arg_len = arg_len as usize;
@@ -327,7 +323,7 @@ fn feed(mut fenv: Caller<Env>, arg_len: u32) -> WasmtimeResult<()> {
     let env = fenv.data_mut();
     let instance = env.self_instance();
 
-    check_arg(instance, arg_len)?;
+    check_arg(&instance, arg_len)?;
 
     let data = instance.with_arg_buf(|buf| {
         let arg_len = arg_len as usize;
@@ -342,7 +338,7 @@ fn hdebug(mut fenv: Caller<Env>, msg_len: u32) -> WasmtimeResult<()> {
     let env = fenv.data_mut();
     let instance = env.self_instance();
 
-    check_arg(instance, msg_len)?;
+    check_arg(&instance, msg_len)?;
 
     Ok(instance.with_arg_buf(|buf| {
         let slice = &buf[..msg_len as usize];
@@ -365,7 +361,7 @@ fn limit(fenv: Caller<Env>) -> u64 {
 
 fn spent(fenv: Caller<Env>) -> u64 {
     let env = fenv.data();
-    let instance = env.self_instance();
+    let mut instance = env.self_instance();
 
     let limit = env.limit();
     let remaining = instance.get_remaining_gas();
@@ -377,7 +373,7 @@ fn panic(fenv: Caller<Env>, arg_len: u32) -> WasmtimeResult<()> {
     let env = fenv.data();
     let instance = env.self_instance();
 
-    check_arg(instance, arg_len)?;
+    check_arg(&instance, arg_len)?;
 
     Ok(instance.with_arg_buf(|buf| {
         let slice = &buf[..arg_len as usize];
@@ -392,7 +388,7 @@ fn panic(fenv: Caller<Env>, arg_len: u32) -> WasmtimeResult<()> {
 }
 
 fn owner(fenv: Caller<Env>, mod_id_ofs: usize) -> WasmtimeResult<i32> {
-    check_ptr(fenv.data().self_instance(), mod_id_ofs, CONTRACT_ID_BYTES)?;
+    check_ptr(&fenv.data().self_instance(), mod_id_ofs, CONTRACT_ID_BYTES)?;
 
     let env = fenv.data();
 

piecrust/src/instance.rs

@@ -5,6 +5,7 @@
 // Copyright (c) DUSK NETWORK. All rights reserved.
 
 use std::io;
+use std::mem::MaybeUninit;
 use std::ops::{Deref, DerefMut};
 
 use dusk_wasmtime::{Instance, Module, Mutability, Store, ValType};
@@ -17,15 +18,47 @@ use crate::store::Memory;
 use crate::Error;
 
 pub struct WrappedInstance {
-    instance: Instance,
-    arg_buf_ofs: usize,
-    store: Store<Env>,
-    memory: Memory,
+    inner: *mut WrappedInstanceInner,
+    original: bool,
+}
+
+impl Clone for WrappedInstance {
+    fn clone(&self) -> Self {
+        Self {
+            inner: self.inner,
+            original: false,
+        }
+    }
+}
+
+impl Drop for WrappedInstance {
+    fn drop(&mut self) {
+        if self.original {
+            unsafe {
+                let _ = Box::from_raw(self.inner);
+            }
+        }
+    }
+}
+
+impl Deref for WrappedInstance {
+    type Target = WrappedInstanceInner;
+
+    fn deref(&self) -> &Self::Target {
+        unsafe { &*self.inner }
+    }
+}
+
+impl DerefMut for WrappedInstance {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        unsafe { &mut *self.inner }
+    }
 }
 
 pub(crate) struct Env {
     self_id: ContractId,
     session: Session,
+    instance: MaybeUninit<WrappedInstance>,
 }
 
 impl Deref for Env {
@@ -43,20 +76,8 @@ impl DerefMut for Env {
 }
 
 impl Env {
-    pub fn self_instance<'b>(&self) -> &'b mut WrappedInstance {
-        let stack_element = self
-            .session
-            .nth_from_top(0)
-            .expect("there should be at least one element in the call stack");
-        self.instance(&stack_element.contract_id)
-            .expect("instance should exist")
-    }
-
-    pub fn instance<'b>(
-        &self,
-        contract_id: &ContractId,
-    ) -> Option<&'b mut WrappedInstance> {
-        self.session.instance(contract_id)
+    pub fn self_instance(&self) -> WrappedInstance {
+        unsafe { self.instance.assume_init_ref().clone() }
     }
 
     pub fn limit(&self) -> u64 {
@@ -94,6 +115,7 @@ impl WrappedInstance {
         let env = Env {
             self_id: contract_id,
             session,
+            instance: MaybeUninit::uninit(),
         };
 
         let module =
@@ -177,28 +199,35 @@ impl WrappedInstance {
         // A memory is no longer new after one instantiation
         memory.is_new = false;
 
-        let wrapped = WrappedInstance {
+        let inner = WrappedInstanceInner {
             store,
             instance,
             arg_buf_ofs,
             memory,
         };
 
-        Ok(wrapped)
-    }
+        let mut instance = WrappedInstance {
+            inner: Box::into_raw(Box::new(inner)),
+            original: true,
+        };
+        let instance_clone = instance.clone();
 
-    pub(crate) fn snap(&mut self) -> io::Result<()> {
-        self.memory.snap()?;
-        Ok(())
-    }
+        instance.store.data_mut().instance = MaybeUninit::new(instance_clone);
 
-    pub(crate) fn revert(&mut self) -> io::Result<()> {
-        self.memory.revert()?;
-        Ok(())
+        Ok(instance)
     }
+}
 
-    pub(crate) fn apply(&mut self) -> io::Result<()> {
-        self.memory.apply()?;
+pub struct WrappedInstanceInner {
+    instance: Instance,
+    arg_buf_ofs: usize,
+    store: Store<Env>,
+    memory: Memory,
+}
+
+impl WrappedInstanceInner {
+    pub(crate) fn snap(&mut self) -> io::Result<()> {
+        self.memory.snap()?;
         Ok(())
     }
 
@@ -238,11 +267,6 @@ impl WrappedInstance {
         self.memory.current_len
     }
 
-    /// Sets the length of the memory.
-    pub(crate) fn set_len(&mut self, len: usize) {
-        self.memory.current_len = len;
-    }
-
     pub(crate) fn with_arg_buf<F, R>(&self, f: F) -> R
     where
         F: FnOnce(&[u8]) -> R,
@@ -342,7 +366,7 @@ impl WrappedInstance {
 }
 
 fn map_call_err(
-    instance: &mut WrappedInstance,
+    instance: &mut WrappedInstanceInner,
     err: dusk_wasmtime::Error,
 ) -> Error {
     if instance.get_remaining_gas() == 0 {