Refactor runtime Table to support static storage.

This commit refactors `Table` in the runtime such that it can be created from a
pointer to existing table data.

The current `Vec` backing of the `Table` is considered to be "dynamic" storage.

This will be used for the upcoming pooling allocator where table memory is
managed externally to the instance.

The `table.copy` implementation was improved to use slice primitives for doing
the copying.

Fixes #983.

crates/runtime/src/instance/allocator.rs

@@ -290,7 +290,7 @@ impl OnDemandInstanceAllocator {
         let mut tables: PrimaryMap<DefinedTableIndex, _> =
             PrimaryMap::with_capacity(module.table_plans.len() - num_imports);
         for table in &module.table_plans.values().as_slice()[num_imports..] {
-            tables.push(Table::new(table));
+            tables.push(Table::new_dynamic(table));
         }
         tables.into_boxed_slice()
     }

crates/runtime/src/table.rs

@@ -4,19 +4,13 @@
 
 use crate::vmcontext::{VMCallerCheckedAnyfunc, VMTableDefinition};
 use crate::{Trap, VMExternRef};
-use std::cell::RefCell;
+use std::cell::{Cell, RefCell};
+use std::cmp::min;
 use std::convert::{TryFrom, TryInto};
 use std::ptr;
 use wasmtime_environ::wasm::TableElementType;
 use wasmtime_environ::{ir, TablePlan, TableStyle};
 
-/// A table instance.
-#[derive(Debug)]
-pub struct Table {
-    elements: RefCell<TableElements>,
-    maximum: Option<u32>,
-}
-
 /// An element going into or coming out of a table.
 #[derive(Clone, Debug)]
 pub enum TableElement {
@@ -26,15 +20,75 @@ pub enum TableElement {
     ExternRef(Option<VMExternRef>),
 }
 
+impl TryFrom<TableElement> for *mut VMCallerCheckedAnyfunc {
+    type Error = ();
+
+    fn try_from(e: TableElement) -> Result<Self, Self::Error> {
+        match e {
+            TableElement::FuncRef(f) => Ok(f),
+            _ => Err(()),
+        }
+    }
+}
+
+impl TryFrom<TableElement> for Option<VMExternRef> {
+    type Error = ();
+
+    fn try_from(e: TableElement) -> Result<Self, Self::Error> {
+        match e {
+            TableElement::ExternRef(x) => Ok(x),
+            _ => Err(()),
+        }
+    }
+}
+
+impl From<*mut VMCallerCheckedAnyfunc> for TableElement {
+    fn from(f: *mut VMCallerCheckedAnyfunc) -> TableElement {
+        TableElement::FuncRef(f)
+    }
+}
+
+impl From<Option<VMExternRef>> for TableElement {
+    fn from(x: Option<VMExternRef>) -> TableElement {
+        TableElement::ExternRef(x)
+    }
+}
+
+impl From<VMExternRef> for TableElement {
+    fn from(x: VMExternRef) -> TableElement {
+        TableElement::ExternRef(Some(x))
+    }
+}
+
 #[derive(Debug)]
 enum TableElements {
     FuncRefs(Vec<*mut VMCallerCheckedAnyfunc>),
     ExternRefs(Vec<Option<VMExternRef>>),
 }
 
+#[derive(Debug)]
+enum TableStorage {
+    Static {
+        data: *mut u8,
+        size: Cell<u32>,
+        ty: TableElementType,
+        maximum: u32,
+    },
+    Dynamic {
+        elements: RefCell<TableElements>,
+        maximum: Option<u32>,
+    },
+}
+
+/// Represents an instance's table.
+#[derive(Debug)]
+pub struct Table {
+    storage: TableStorage,
+}
+
 impl Table {
-    /// Create a new table instance with specified minimum and maximum number of elements.
+    /// Create a new dynamic (movable) table instance for the specified table plan.
-    pub fn new(plan: &TablePlan) -> Self {
+    pub fn new_dynamic(plan: &TablePlan) -> Self {
         let min = usize::try_from(plan.table.minimum).unwrap();
         let elements = RefCell::new(match plan.table.ty {
             TableElementType::Func => TableElements::FuncRefs(vec![ptr::null_mut(); min]),
@@ -43,27 +97,58 @@ impl Table {
                 TableElements::ExternRefs(vec![None; min])
             }
         });
+
         match plan.style {
             TableStyle::CallerChecksSignature => Self {
-                elements,
+                storage: TableStorage::Dynamic {
-                maximum: plan.table.maximum,
+                    elements,
+                    maximum: plan.table.maximum,
+                },
+            },
+        }
+    }
+
+    /// Create a new static (immovable) table instance for the specified table plan.
+    pub fn new_static(plan: &TablePlan, data: *mut u8, maximum: u32) -> Self {
+        match plan.style {
+            TableStyle::CallerChecksSignature => Self {
+                storage: TableStorage::Static {
+                    data,
+                    size: Cell::new(plan.table.minimum),
+                    ty: plan.table.ty.clone(),
+                    maximum: min(plan.table.maximum.unwrap_or(maximum), maximum),
+                },
             },
         }
     }
 
     /// Returns the type of the elements in this table.
     pub fn element_type(&self) -> TableElementType {
-        match &*self.elements.borrow() {
+        match &self.storage {
-            TableElements::FuncRefs(_) => TableElementType::Func,
+            TableStorage::Static { ty, .. } => *ty,
-            TableElements::ExternRefs(_) => TableElementType::Val(crate::ref_type()),
+            TableStorage::Dynamic { elements, .. } => match &*elements.borrow() {
+                TableElements::FuncRefs(_) => TableElementType::Func,
+                TableElements::ExternRefs(_) => TableElementType::Val(crate::ref_type()),
+            },
         }
     }
 
     /// Returns the number of allocated elements.
     pub fn size(&self) -> u32 {
-        match &*self.elements.borrow() {
+        match &self.storage {
-            TableElements::FuncRefs(x) => x.len().try_into().unwrap(),
+            TableStorage::Static { size, .. } => size.get(),
-            TableElements::ExternRefs(x) => x.len().try_into().unwrap(),
+            TableStorage::Dynamic { elements, .. } => match &*elements.borrow() {
+                TableElements::FuncRefs(x) => x.len().try_into().unwrap(),
+                TableElements::ExternRefs(x) => x.len().try_into().unwrap(),
+            },
+        }
+    }
+
+    /// Returns the maximum number of elements.
+    pub fn maximum(&self) -> Option<u32> {
+        match &self.storage {
+            TableStorage::Static { maximum, .. } => Some(*maximum),
+            TableStorage::Dynamic { maximum, .. } => maximum.clone(),
         }
     }
 
@@ -80,8 +165,31 @@ impl Table {
             return Err(Trap::wasm(ir::TrapCode::TableOutOfBounds));
         }
 
-        for i in start..end {
+        match val {
-            self.set(i, val.clone()).unwrap();
+            TableElement::FuncRef(r) => {
+                unsafe {
+                    self.with_funcrefs_mut(move |elements| {
+                        let elements = elements.unwrap();
+
+                        // TODO: replace this with slice::fill (https://github.com/rust-lang/rust/issues/70758) when stabilized
+                        for e in &mut elements[start as usize..end as usize] {
+                            *e = r;
+                        }
+                    });
+                }
+            }
+            TableElement::ExternRef(r) => {
+                unsafe {
+                    self.with_externrefs_mut(move |elements| {
+                        let elements = elements.unwrap();
+
+                        // TODO: replace this with slice::fill (https://github.com/rust-lang/rust/issues/70758) when stabilized
+                        for e in &mut elements[start as usize..end as usize] {
+                            *e = r.clone();
+                        }
+                    });
+                }
+            }
         }
 
         Ok(())
@@ -104,38 +212,48 @@ impl Table {
     /// Generally, prefer using `InstanceHandle::table_grow`, which encapsulates
     /// this unsafety.
     pub unsafe fn grow(&self, delta: u32, init_value: TableElement) -> Option<u32> {
-        let size = self.size();
+        let old_size = self.size();
 
-        let new_len = size.checked_add(delta)?;
+        let new_size = old_size.checked_add(delta)?;
-        if let Some(max) = self.maximum {
+        if let Some(max) = self.maximum() {
-            if new_len > max {
+            if new_size > max {
                 return None;
             }
         }
-        let new_len = usize::try_from(new_len).unwrap();
 
-        match &mut *self.elements.borrow_mut() {
+        match &self.storage {
-            TableElements::FuncRefs(x) => {
+            TableStorage::Static { size, .. } => {
-                let init_value = init_value.try_into().ok()?;
+                size.set(new_size);
-                x.resize(new_len, init_value)
+                self.fill(old_size, init_value, delta)
+                    .ok()
+                    .map(|_| old_size)
             }
-            TableElements::ExternRefs(x) => {
+            TableStorage::Dynamic { elements, .. } => {
-                let init_value = init_value.try_into().ok()?;
+                let new_len = usize::try_from(new_size).unwrap();
-                x.resize(new_len, init_value)
+
+                match &mut *elements.borrow_mut() {
+                    TableElements::FuncRefs(x) => x.resize(new_len, init_value.try_into().ok()?),
+                    TableElements::ExternRefs(x) => x.resize(new_len, init_value.try_into().ok()?),
+                }
+
+                Some(old_size)
             }
         }
-
-        Some(size)
     }
 
     /// Get reference to the specified element.
     ///
     /// Returns `None` if the index is out of bounds.
     pub fn get(&self, index: u32) -> Option<TableElement> {
-        match &*self.elements.borrow() {
+        unsafe {
-            TableElements::FuncRefs(x) => x.get(index as usize).cloned().map(TableElement::FuncRef),
+            match self.element_type() {
-            TableElements::ExternRefs(x) => {
+                TableElementType::Func => self.with_funcrefs(|elements| {
-                x.get(index as usize).cloned().map(TableElement::ExternRef)
+                    elements.and_then(|e| e.get(index as usize).cloned().map(TableElement::FuncRef))
+                }),
+                TableElementType::Val(_) => self.with_externrefs(|elements| {
+                    elements
+                        .and_then(|e| e.get(index as usize).cloned().map(TableElement::ExternRef))
+                }),
             }
         }
     }
@@ -147,18 +265,22 @@ impl Table {
     /// Returns an error if `index` is out of bounds or if this table type does
     /// not match the element type.
     pub fn set(&self, index: u32, elem: TableElement) -> Result<(), ()> {
-        let mut elems = self.elements.borrow_mut();
+        unsafe {
-        match &mut *elems {
+            match self.element_type() {
-            TableElements::FuncRefs(x) => {
+                TableElementType::Func => self.with_funcrefs_mut(move |elements| {
-                let slot = x.get_mut(index as usize).ok_or(())?;
+                    let elements = elements.ok_or(())?;
-                *slot = elem.try_into().or(Err(()))?;
+                    let e = elements.get_mut(index as usize).ok_or(())?;
-            }
+                    *e = elem.try_into()?;
-            TableElements::ExternRefs(x) => {
+                    Ok(())
-                let slot = x.get_mut(index as usize).ok_or(())?;
+                }),
-                *slot = elem.try_into().or(Err(()))?;
+                TableElementType::Val(_) => self.with_externrefs_mut(move |elements| {
+                    let elements = elements.ok_or(())?;
+                    let e = elements.get_mut(index as usize).ok_or(())?;
+                    *e = elem.try_into()?;
+                    Ok(())
+                }),
             }
         }
-        Ok(())
     }
 
     /// Copy `len` elements from `src_table[src_index..]` into `dst_table[dst_index..]`.
@@ -186,20 +308,48 @@ impl Table {
             return Err(Trap::wasm(ir::TrapCode::TableOutOfBounds));
         }
 
-        let srcs = src_index..src_index + len;
+        // Check if the source and destination are the same table
-        let dsts = dst_index..dst_index + len;
+        // This ensures we don't `borrow` and `borrow_mut` the same underlying RefCell
+        let same_table = ptr::eq(dst_table, src_table);
 
-        // Note on the unwraps: the bounds check above means that these will
+        let src_range = src_index as usize..src_index as usize + len as usize;
-        // never panic.
+        let dst_range = dst_index as usize..dst_index as usize + len as usize;
-        //
+
-        // TODO(#983): investigate replacing this get/set loop with a `memcpy`.
+        unsafe {
-        if dst_index <= src_index {
+            match dst_table.element_type() {
-            for (s, d) in (srcs).zip(dsts) {
+                TableElementType::Func => dst_table.with_funcrefs_mut(|dst| {
-                dst_table.set(d, src_table.get(s).unwrap()).unwrap();
+                    let dst = dst.unwrap();
-            }
+
-        } else {
+                    if same_table {
-            for (s, d) in srcs.rev().zip(dsts.rev()) {
+                        dst.copy_within(src_range, dst_index as usize);
-                dst_table.set(d, src_table.get(s).unwrap()).unwrap();
+                    } else {
+                        src_table.with_funcrefs(|src| {
+                            let src = src.unwrap();
+                            dst[dst_range].copy_from_slice(&src[src_range]);
+                        })
+                    }
+                }),
+                TableElementType::Val(_) => dst_table.with_externrefs_mut(|dst| {
+                    let dst = dst.unwrap();
+
+                    if same_table {
+                        // As there's no `slice::clone_within` because cloning can't be done with memmove, use a loop
+                        if dst_index <= src_index {
+                            for (s, d) in (src_range).zip(dst_range) {
+                                dst[d] = dst[s].clone();
+                            }
+                        } else {
+                            for (s, d) in src_range.rev().zip(dst_range.rev()) {
+                                dst[d] = dst[s].clone();
+                            }
+                        }
+                    } else {
+                        src_table.with_externrefs(|src| {
+                            let src = src.unwrap();
+                            dst[dst_range].clone_from_slice(&src[src_range]);
+                        })
+                    }
+                }),
             }
         }
 
@@ -208,55 +358,97 @@ impl Table {
 
     /// Return a `VMTableDefinition` for exposing the table to compiled wasm code.
     pub fn vmtable(&self) -> VMTableDefinition {
-        match &*self.elements.borrow() {
+        match &self.storage {
-            TableElements::FuncRefs(x) => VMTableDefinition {
+            TableStorage::Static { data, size, .. } => VMTableDefinition {
-                base: x.as_ptr() as *const u8 as *mut u8,
+                base: *data,
-                current_elements: x.len().try_into().unwrap(),
+                current_elements: size.get(),
             },
-            TableElements::ExternRefs(x) => VMTableDefinition {
+            TableStorage::Dynamic { elements, .. } => match &*elements.borrow() {
-                base: x.as_ptr() as *const u8 as *mut u8,
+                TableElements::FuncRefs(x) => VMTableDefinition {
-                current_elements: x.len().try_into().unwrap(),
+                    base: x.as_ptr() as *const u8 as _,
+                    current_elements: x.len().try_into().unwrap(),
+                },
+                TableElements::ExternRefs(x) => VMTableDefinition {
+                    base: x.as_ptr() as *const u8 as _,
+                    current_elements: x.len().try_into().unwrap(),
+                },
+            },
+        }
+    }
+
+    unsafe fn with_funcrefs<F, R>(&self, with: F) -> R
+    where
+        F: FnOnce(Option<&[*mut VMCallerCheckedAnyfunc]>) -> R,
+    {
+        match &self.storage {
+            TableStorage::Static { data, size, ty, .. } => match ty {
+                TableElementType::Func => with(Some(std::slice::from_raw_parts(
+                    *data as *const _,
+                    size.get() as usize,
+                ))),
+                _ => with(None),
+            },
+            TableStorage::Dynamic { elements, .. } => match &*elements.borrow() {
+                TableElements::FuncRefs(x) => with(Some(x.as_slice())),
+                _ => with(None),
+            },
+        }
+    }
+
+    unsafe fn with_funcrefs_mut<F, R>(&self, with: F) -> R
+    where
+        F: FnOnce(Option<&mut [*mut VMCallerCheckedAnyfunc]>) -> R,
+    {
+        match &self.storage {
+            TableStorage::Static { data, size, ty, .. } => match ty {
+                TableElementType::Func => with(Some(std::slice::from_raw_parts_mut(
+                    *data as *mut _,
+                    size.get() as usize,
+                ))),
+                _ => with(None),
+            },
+            TableStorage::Dynamic { elements, .. } => match &mut *elements.borrow_mut() {
+                TableElements::FuncRefs(x) => with(Some(x.as_mut_slice())),
+                _ => with(None),
+            },
+        }
+    }
+
+    unsafe fn with_externrefs<F, R>(&self, with: F) -> R
+    where
+        F: FnOnce(Option<&[Option<VMExternRef>]>) -> R,
+    {
+        match &self.storage {
+            TableStorage::Static { data, size, ty, .. } => match ty {
+                TableElementType::Val(_) => with(Some(std::slice::from_raw_parts(
+                    *data as *const _,
+                    size.get() as usize,
+                ))),
+                _ => with(None),
+            },
+            TableStorage::Dynamic { elements, .. } => match &*elements.borrow() {
+                TableElements::ExternRefs(x) => with(Some(x.as_slice())),
+                _ => with(None),
+            },
+        }
+    }
+
+    unsafe fn with_externrefs_mut<F, R>(&self, with: F) -> R
+    where
+        F: FnOnce(Option<&mut [Option<VMExternRef>]>) -> R,
+    {
+        match &self.storage {
+            TableStorage::Static { data, size, ty, .. } => match ty {
+                TableElementType::Val(_) => with(Some(std::slice::from_raw_parts_mut(
+                    *data as *mut _,
+                    size.get() as usize,
+                ))),
+                _ => with(None),
+            },
+            TableStorage::Dynamic { elements, .. } => match &mut *elements.borrow_mut() {
+                TableElements::ExternRefs(x) => with(Some(x.as_mut_slice())),
+                _ => with(None),
             },
         }
     }
 }
-
-impl TryFrom<TableElement> for *mut VMCallerCheckedAnyfunc {
-    type Error = TableElement;
-
-    fn try_from(e: TableElement) -> Result<Self, Self::Error> {
-        match e {
-            TableElement::FuncRef(f) => Ok(f),
-            _ => Err(e),
-        }
-    }
-}
-
-impl TryFrom<TableElement> for Option<VMExternRef> {
-    type Error = TableElement;
-
-    fn try_from(e: TableElement) -> Result<Self, Self::Error> {
-        match e {
-            TableElement::ExternRef(x) => Ok(x),
-            _ => Err(e),
-        }
-    }
-}
-
-impl From<*mut VMCallerCheckedAnyfunc> for TableElement {
-    fn from(f: *mut VMCallerCheckedAnyfunc) -> TableElement {
-        TableElement::FuncRef(f)
-    }
-}
-
-impl From<Option<VMExternRef>> for TableElement {
-    fn from(x: Option<VMExternRef>) -> TableElement {
-        TableElement::ExternRef(x)
-    }
-}
-
-impl From<VMExternRef> for TableElement {
-    fn from(x: VMExternRef) -> TableElement {
-        TableElement::ExternRef(Some(x))
-    }
-}