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

Revert "One instance per contract function call"

Author: Eduardo Leegwater Simões

piecrust/CHANGELOG.md

@@ -17,12 +17,10 @@ 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

piecrust/src/imports.rs

@@ -138,12 +138,12 @@ pub(crate) fn hq(
 ) -> WasmtimeResult<u32> {
     let env = fenv.data_mut();
 
-    let mut instance = env.self_instance();
+    let 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 mut instance = env.self_instance();
+    let 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 mut instance = env.self_instance();
+    let 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,8 +222,12 @@ pub(crate) fn c(
             &memory[mod_id_ofs..][..std::mem::size_of::<ContractId>()],
         );
 
-        let mut callee = env.instance(mod_id)?;
-        env.push_callstack(mod_id, callee_limit, callee.mem_len());
+        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");
 
         callee.snap().map_err(|err| Error::MemorySnapshotFailure {
             reason: None,
@@ -238,7 +242,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]);
@@ -287,8 +291,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;
@@ -323,7 +327,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;
@@ -338,7 +342,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];
@@ -361,7 +365,7 @@ fn limit(fenv: Caller<Env>) -> u64 {
 
 fn spent(fenv: Caller<Env>) -> u64 {
     let env = fenv.data();
-    let mut instance = env.self_instance();
+    let instance = env.self_instance();
 
     let limit = env.limit();
     let remaining = instance.get_remaining_gas();
@@ -373,7 +377,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];
@@ -388,7 +392,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,7 +5,6 @@
 // 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};
@@ -18,47 +17,15 @@ use crate::store::Memory;
 use crate::Error;
 
 pub struct WrappedInstance {
-    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 }
-    }
+    instance: Instance,
+    arg_buf_ofs: usize,
+    store: Store<Env>,
+    memory: Memory,
 }
 
 pub(crate) struct Env {
     self_id: ContractId,
     session: Session,
-    instance: MaybeUninit<WrappedInstance>,
 }
 
 impl Deref for Env {
@@ -76,8 +43,20 @@ impl DerefMut for Env {
 }
 
 impl Env {
-    pub fn self_instance(&self) -> WrappedInstance {
-        unsafe { self.instance.assume_init_ref().clone() }
+    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 limit(&self) -> u64 {
@@ -115,7 +94,6 @@ impl WrappedInstance {
         let env = Env {
             self_id: contract_id,
             session,
-            instance: MaybeUninit::uninit(),
         };
 
         let module =
@@ -199,38 +177,31 @@ impl WrappedInstance {
         // A memory is no longer new after one instantiation
         memory.is_new = false;
 
-        let inner = WrappedInstanceInner {
+        let wrapped = WrappedInstance {
             store,
             instance,
             arg_buf_ofs,
             memory,
         };
 
-        let mut instance = WrappedInstance {
-            inner: Box::into_raw(Box::new(inner)),
-            original: true,
-        };
-        let instance_clone = instance.clone();
-
-        instance.store.data_mut().instance = MaybeUninit::new(instance_clone);
-
-        Ok(instance)
+        Ok(wrapped)
     }
-}
 
-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(())
     }
 
+    pub(crate) fn revert(&mut self) -> io::Result<()> {
+        self.memory.revert()?;
+        Ok(())
+    }
+
+    pub(crate) fn apply(&mut self) -> io::Result<()> {
+        self.memory.apply()?;
+        Ok(())
+    }
+
     // Write argument into instance
     pub(crate) fn write_argument(&mut self, arg: &[u8]) {
         self.with_arg_buf_mut(|buf| buf[..arg.len()].copy_from_slice(arg))
@@ -267,6 +238,11 @@ impl WrappedInstanceInner {
         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,
@@ -366,7 +342,7 @@ impl WrappedInstanceInner {
 }
 
 fn map_call_err(
-    instance: &mut WrappedInstanceInner,
+    instance: &mut WrappedInstance,
     err: dusk_wasmtime::Error,
 ) -> Error {
     if instance.get_remaining_gas() == 0 {