Simplify memory & table exports

lib/api/src/externals/memory.rs

@@ -3,19 +3,15 @@ use crate::externals::Extern;
 use crate::memory_view::MemoryView;
 use crate::store::Store;
 use crate::MemoryType;
-use std::ptr::NonNull;
 use std::slice;
+use std::sync::Arc;
 use wasm_common::{Pages, ValueType};
-use wasmer_runtime::{
+use wasmer_runtime::{Export, ExportMemory, Memory as RuntimeMemory, MemoryError};
-    Export, ExportMemory, Memory as MemoryTrait, MemoryError, VMMemoryDefinition,
-};
 
 #[derive(Clone)]
 pub struct Memory {
     store: Store,
-    // If the Memory is owned by the Store, not the instance
+    memory: Arc<dyn RuntimeMemory>,
-    owned_by_store: bool,
-    exported: ExportMemory,
 }
 
 impl Memory {
@@ -23,24 +19,15 @@ impl Memory {
         let tunables = store.tunables();
         let style = tunables.memory_style(&ty);
         let memory = tunables.create_memory(&ty, &style)?;
-        let definition = memory.vmmemory();
 
         Ok(Memory {
             store: store.clone(),
-            owned_by_store: true,
+            memory,
-            exported: ExportMemory {
-                from: memory,
-                definition,
-            },
         })
     }
 
-    fn definition(&self) -> NonNull<VMMemoryDefinition> {
-        self.memory().vmmemory()
-    }
-
     pub fn ty(&self) -> &MemoryType {
-        self.exported.ty()
+        self.memory.ty()
     }
 
     pub fn store(&self) -> &Store {
@@ -64,36 +51,32 @@ impl Memory {
     /// To be defined (TODO).
     #[allow(clippy::mut_from_ref)]
     pub unsafe fn data_unchecked_mut(&self) -> &mut [u8] {
-        let definition = self.definition();
+        let definition = self.memory.vmmemory();
         let def = definition.as_ref();
         slice::from_raw_parts_mut(def.base, def.current_length)
     }
 
     pub fn data_ptr(&self) -> *mut u8 {
-        let definition = self.definition();
+        let definition = self.memory.vmmemory();
         let def = unsafe { definition.as_ref() };
         def.base
     }
 
     pub fn data_size(&self) -> usize {
-        let definition = self.definition();
+        let definition = self.memory.vmmemory();
         let def = unsafe { definition.as_ref() };
         def.current_length
     }
 
     pub fn size(&self) -> Pages {
-        self.memory().size()
+        self.memory.size()
-    }
-
-    fn memory(&self) -> &dyn MemoryTrait {
-        &*self.exported.from
     }
 
     pub fn grow<IntoPages>(&self, delta: IntoPages) -> Result<Pages, MemoryError>
     where
         IntoPages: Into<Pages>,
     {
-        self.memory().grow(delta.into())
+        self.memory.grow(delta.into())
     }
 
     /// Return a "view" of the currently accessible memory. By
@@ -138,21 +121,24 @@ impl Memory {
     pub(crate) fn from_export(store: &Store, wasmer_export: ExportMemory) -> Memory {
         Memory {
             store: store.clone(),
-            owned_by_store: false,
+            memory: wasmer_export.from,
-            exported: wasmer_export,
         }
     }
 
     /// Returns whether or not these two globals refer to the same data.
     pub fn same(&self, other: &Memory) -> bool {
-        self.exported.same(&other.exported)
+        Arc::ptr_eq(&self.memory, &other.memory)
     }
 }
 
 impl<'a> Exportable<'a> for Memory {
     fn to_export(&self) -> Export {
-        self.exported.clone().into()
+        ExportMemory {
+            from: self.memory.clone(),
+        }
+        .into()
     }
+
     fn get_self_from_extern(_extern: &'a Extern) -> Result<&'a Self, ExportError> {
         match _extern {
             Extern::Memory(memory) => Ok(memory),
@@ -160,12 +146,3 @@ impl<'a> Exportable<'a> for Memory {
         }
     }
 }
-
-impl Drop for Memory {
-    fn drop(&mut self) {
-        if self.owned_by_store {
-            // let r = unsafe { libc::munmap(self.ptr as *mut libc::c_void, self.len) };
-            // assert_eq!(r, 0, "munmap failed: {}", std::io::Error::last_os_error());
-        }
-    }
-}

lib/api/src/externals/table.rs

@@ -4,6 +4,7 @@ use crate::store::Store;
 use crate::types::{Val, ValFuncRef};
 use crate::RuntimeError;
 use crate::TableType;
+use std::sync::Arc;
 use wasmer_runtime::{Export, ExportTable, Table as RuntimeTable, VMCallerCheckedAnyfunc};
 
 /// The `Table` struct is an array-like structure representing a WebAssembly Table,
@@ -14,9 +15,7 @@ use wasmer_runtime::{Export, ExportTable, Table as RuntimeTable, VMCallerChecked
 #[derive(Clone)]
 pub struct Table {
     store: Store,
-    // If the Table is owned by the Store, not the instance
+    table: Arc<dyn RuntimeTable>,
-    owned_by_store: bool,
-    exported: ExportTable,
 }
 
 fn set_table_item(
@@ -44,24 +43,15 @@ impl Table {
             set_table_item(table.as_ref(), i, item.clone())?;
         }
 
-        let definition = table.vmtable();
         Ok(Table {
             store: store.clone(),
-            owned_by_store: true,
+            table,
-            exported: ExportTable {
-                from: table,
-                definition,
-            },
         })
     }
 
-    fn table(&self) -> &dyn RuntimeTable {
-        &*self.exported.from
-    }
-
     /// Gets the underlying [`TableType`].
     pub fn ty(&self) -> &TableType {
-        self.exported.ty()
+        self.table.ty()
     }
 
     pub fn store(&self) -> &Store {
@@ -70,19 +60,19 @@ impl Table {
 
     /// Retrieves an element of the table at the provided `index`.
     pub fn get(&self, index: u32) -> Option<Val> {
-        let item = self.table().get(index)?;
+        let item = self.table.get(index)?;
         Some(ValFuncRef::from_checked_anyfunc(item, &self.store))
     }
 
     /// Sets an element `val` in the Table at the provided `index`.
     pub fn set(&self, index: u32, val: Val) -> Result<(), RuntimeError> {
         let item = val.into_checked_anyfunc(&self.store)?;
-        set_table_item(self.table(), index, item)
+        set_table_item(self.table.as_ref(), index, item)
     }
 
     /// Retrieves the size of the `Table` (in elements)
     pub fn size(&self) -> u32 {
-        self.table().size()
+        self.table.size()
     }
 
     /// Grows the size of the `Table` by `delta`, initializating
@@ -96,11 +86,10 @@ impl Table {
     /// Returns an error if the `delta` is out of bounds for the table.
     pub fn grow(&self, delta: u32, init: Val) -> Result<u32, RuntimeError> {
         let item = init.into_checked_anyfunc(&self.store)?;
-        let table = self.table();
+        match self.table.grow(delta) {
-        match table.grow(delta) {
             Some(len) => {
                 for i in 0..delta {
-                    set_table_item(table, len + i, item.clone())?;
+                    set_table_item(self.table.as_ref(), len + i, item.clone())?;
                 }
                 Ok(len)
             }
@@ -131,8 +120,8 @@ impl Table {
             ));
         }
         RuntimeTable::copy(
-            dst_table.table(),
+            dst_table.table.as_ref(),
-            src_table.table(),
+            src_table.table.as_ref(),
             dst_index,
             src_index,
             len,
@@ -144,21 +133,24 @@ impl Table {
     pub(crate) fn from_export(store: &Store, wasmer_export: ExportTable) -> Table {
         Table {
             store: store.clone(),
-            owned_by_store: false,
+            table: wasmer_export.from,
-            exported: wasmer_export,
         }
     }
 
     /// Returns whether or not these two tables refer to the same data.
     pub fn same(&self, other: &Self) -> bool {
-        self.exported.same(&other.exported)
+        Arc::ptr_eq(&self.table, &other.table)
     }
 }
 
 impl<'a> Exportable<'a> for Table {
     fn to_export(&self) -> Export {
-        self.exported.clone().into()
+        ExportTable {
+            from: self.table.clone(),
+        }
+        .into()
     }
+
     fn get_self_from_extern(_extern: &'a Extern) -> Result<&'a Self, ExportError> {
         match _extern {
             Extern::Table(table) => Ok(table),

lib/engine/src/resolver.rs

@@ -168,7 +168,7 @@ pub fn resolve_imports(
             }
             Export::Table(ref t) => {
                 table_imports.push(VMTableImport {
-                    definition: t.definition,
+                    definition: t.from.vmtable(),
                     from: t.from.clone(),
                 });
             }
@@ -202,7 +202,7 @@ pub fn resolve_imports(
                 }
 
                 memory_imports.push(VMMemoryImport {
-                    definition: m.definition,
+                    definition: m.from.vmmemory(),
                     from: m.from.clone(),
                 });
             }

lib/runtime/src/export.rs

@@ -3,11 +3,7 @@
 
 use crate::memory::{Memory, MemoryStyle};
 use crate::table::{Table, TableStyle};
-use crate::vmcontext::{
+use crate::vmcontext::{VMContext, VMFunctionBody, VMFunctionKind, VMGlobalDefinition};
-    VMContext, VMFunctionBody, VMFunctionKind, VMGlobalDefinition, VMMemoryDefinition,
-    VMTableDefinition,
-};
-use std::ptr::NonNull;
 use std::sync::Arc;
 use wasm_common::{FunctionType, GlobalType, MemoryType, TableType};
 
@@ -49,14 +45,6 @@ impl From<ExportFunction> for Export {
 /// A table export value.
 #[derive(Debug, Clone)]
 pub struct ExportTable {
-    /// The address of the table descriptor.
-    ///
-    /// The `VMTableDefinition` this points to should be considered immutable from
-    /// this pointer.  The data may be updated though.
-    /// TODO: better define this behavior and document it
-    // TODO: consider a special wrapper pointer type for this kind of logic
-    // (so we don't need to `unsafe impl Send` in the places that use it)
-    pub definition: NonNull<VMTableDefinition>,
     /// Pointer to the containing `Table`.
     pub from: Arc<dyn Table>,
 }
@@ -83,8 +71,7 @@ impl ExportTable {
 
     /// Returns whether or not the two `ExportTable`s refer to the same Memory.
     pub fn same(&self, other: &Self) -> bool {
-        // TODO: comparing
+        Arc::ptr_eq(&self.from, &other.from)
-        self.definition == other.definition //&& self.from == other.from
     }
 }
 
@@ -97,14 +84,6 @@ impl From<ExportTable> for Export {
 /// A memory export value.
 #[derive(Debug, Clone)]
 pub struct ExportMemory {
-    /// The address of the memory descriptor.
-    ///
-    /// The `VMMemoryDefinition` this points to should be considered immutable from
-    /// this pointer.  The data may be updated though.
-    /// TODO: better define this behavior and document it
-    // TODO: consider a special wrapper pointer type for this kind of logic
-    // (so we don't need to `unsafe impl Send` in the places that use it)
-    pub definition: NonNull<VMMemoryDefinition>,
     /// Pointer to the containing `Memory`.
     pub from: Arc<dyn Memory>,
 }
@@ -131,8 +110,7 @@ impl ExportMemory {
 
     /// Returns whether or not the two `ExportMemory`s refer to the same Memory.
     pub fn same(&self, other: &Self) -> bool {
-        // TODO: implement comparison
+        Arc::ptr_eq(&self.from, &other.from)
-        self.definition == other.definition //&& self.from == other.from
     }
 }
 

lib/runtime/src/instance.rs

@@ -311,24 +311,22 @@ impl Instance {
                 .into()
             }
             ExportIndex::Table(index) => {
-                let (definition, from) =
+                let from = if let Some(def_index) = self.module.local_table_index(*index) {
-                    if let Some(def_index) = self.module.local_table_index(*index) {
+                    self.tables[def_index].clone()
-                        (self.table_ptr(def_index), self.tables[def_index].clone())
+                } else {
-                    } else {
+                    let import = self.imported_table(*index);
-                        let import = self.imported_table(*index);
+                    import.from.clone()
-                        (import.definition, import.from.clone())
+                };
-                    };
+                ExportTable { from }.into()
-                ExportTable { definition, from }.into()
             }
             ExportIndex::Memory(index) => {
-                let (definition, from) =
+                let from = if let Some(def_index) = self.module.local_memory_index(*index) {
-                    if let Some(def_index) = self.module.local_memory_index(*index) {
+                    self.memories[def_index].clone()
-                        (self.memory_ptr(def_index), self.memories[def_index].clone())
+                } else {
-                    } else {
+                    let import = self.imported_memory(*index);
-                        let import = self.imported_memory(*index);
+                    import.from.clone()
-                        (import.definition, import.from.clone())
+                };
-                    };
+                ExportMemory { from }.into()
-                ExportMemory { definition, from }.into()
             }
             ExportIndex::Global(index) => ExportGlobal {
                 definition: if let Some(def_index) = self.module.local_global_index(*index) {