Fix tables holding their registered types (#11106)

This commit fixes an issue where host-created tables with concrete
reference types previously did not keep their associated type
registrations alive for the duration of the table itself. This could
lead to runtime panics when reflecting on their type and additionally
lead to some type confusion about the table itself.  As described
in #11102 this is not a security issue, just a bug that needs fixing.

Closes #11102

Author: alexcrichton

crates/wasmtime/src/runtime/trampoline.rs

@@ -12,47 +12,10 @@ pub(crate) use memory::MemoryCreatorProxy;
 use self::memory::create_memory;
 use self::table::create_table;
 use crate::prelude::*;
-use crate::runtime::vm::{
-    Imports, InstanceAllocationRequest, InstanceAllocator, ModuleRuntimeInfo,
-    OnDemandInstanceAllocator, SharedMemory, StorePtr, VMFunctionImport,
-};
-use crate::store::{InstanceId, StoreOpaque};
+use crate::runtime::vm::SharedMemory;
+use crate::store::StoreOpaque;
 use crate::{MemoryType, TableType};
-use alloc::sync::Arc;
-use core::any::Any;
-use wasmtime_environ::{MemoryIndex, Module, TableIndex, VMSharedTypeIndex};
-
-fn create_handle(
-    module: Module,
-    store: &mut StoreOpaque,
-    host_state: Box<dyn Any + Send + Sync>,
-    func_imports: &[VMFunctionImport],
-    one_signature: Option<VMSharedTypeIndex>,
-) -> Result<InstanceId> {
-    let mut imports = Imports::default();
-    imports.functions = func_imports;
-
-    unsafe {
-        let config = store.engine().config();
-        // Use the on-demand allocator when creating handles associated with host objects
-        // The configured instance allocator should only be used when creating module instances
-        // as we don't want host objects to count towards instance limits.
-        let module = Arc::new(module);
-        let runtime_info = &ModuleRuntimeInfo::bare_maybe_imported_func(module, one_signature);
-        let allocator = OnDemandInstanceAllocator::new(config.mem_creator.clone(), 0, false);
-        let handle = allocator.allocate_module(InstanceAllocationRequest {
-            imports,
-            host_state,
-            store: StorePtr::new(store.traitobj()),
-            runtime_info,
-            wmemcheck: false,
-            pkey: None,
-            tunables: store.engine().tunables(),
-        })?;
-
-        Ok(store.add_dummy_instance(handle))
-    }
-}
+use wasmtime_environ::{MemoryIndex, TableIndex};
 
 pub fn generate_memory_export(
     store: &mut StoreOpaque,

crates/wasmtime/src/runtime/trampoline/memory.rs

@@ -47,7 +47,7 @@ pub fn create_memory(
     // associated with external objects. The configured instance allocator
     // should only be used when creating module instances as we don't want host
     // objects to count towards instance limits.
-    let runtime_info = &ModuleRuntimeInfo::bare_maybe_imported_func(Arc::new(module), None);
+    let runtime_info = &ModuleRuntimeInfo::bare(Arc::new(module));
     let host_state = Box::new(());
     let imports = Imports::default();
     let request = InstanceAllocationRequest {

crates/wasmtime/src/runtime/trampoline/table.rs

@@ -1,7 +1,11 @@
 use crate::prelude::*;
+use crate::runtime::vm::{
+    Imports, InstanceAllocationRequest, InstanceAllocator, ModuleRuntimeInfo,
+    OnDemandInstanceAllocator, StorePtr,
+};
 use crate::store::{InstanceId, StoreOpaque};
-use crate::trampoline::create_handle;
 use crate::TableType;
+use alloc::sync::Arc;
 use wasmtime_environ::{EntityIndex, Module, TypeTrace};
 
 pub fn create_table(store: &mut StoreOpaque, table: &TableType) -> Result<InstanceId> {
@@ -22,5 +26,29 @@ pub fn create_table(store: &mut StoreOpaque, table: &TableType) -> Result<Instan
         .exports
         .insert(String::new(), EntityIndex::Table(table_id));
 
-    create_handle(module, store, Box::new(()), &[], None)
+    let imports = Imports::default();
+
+    unsafe {
+        let config = store.engine().config();
+        // Use the on-demand allocator when creating handles associated with host objects
+        // The configured instance allocator should only be used when creating module instances
+        // as we don't want host objects to count towards instance limits.
+        let module = Arc::new(module);
+        let runtime_info = &ModuleRuntimeInfo::bare_with_registered_type(
+            module,
+            table.element().clone().into_registered_type(),
+        );
+        let allocator = OnDemandInstanceAllocator::new(config.mem_creator.clone(), 0, false);
+        let handle = allocator.allocate_module(InstanceAllocationRequest {
+            imports,
+            host_state: Box::new(()),
+            store: StorePtr::new(store.traitobj()),
+            runtime_info,
+            wmemcheck: false,
+            pkey: None,
+            tunables: store.engine().tunables(),
+        })?;
+
+        Ok(store.add_dummy_instance(handle))
+    }
 }

crates/wasmtime/src/runtime/types.rs

@@ -536,6 +536,10 @@ impl RefType {
     pub(crate) fn is_vmgcref_type_and_points_to_object(&self) -> bool {
         self.heap_type().is_vmgcref_type_and_points_to_object()
     }
+
+    pub(crate) fn into_registered_type(self) -> Option<RegisteredType> {
+        self.heap_type.into_registered_type()
+    }
 }
 
 /// The heap types that can Wasm can have references to.
@@ -1160,6 +1164,18 @@ impl HeapType {
                 HeapType::I31 | HeapType::NoExtern | HeapType::NoFunc | HeapType::None
             )
     }
+
+    pub(crate) fn into_registered_type(self) -> Option<RegisteredType> {
+        use HeapType::*;
+        match self {
+            ConcreteFunc(ty) => Some(ty.registered_type),
+            ConcreteArray(ty) => Some(ty.registered_type),
+            ConcreteStruct(ty) => Some(ty.registered_type),
+            Extern | NoExtern | Func | NoFunc | Any | Eq | I31 | Array | Struct | None => {
+                Option::None
+            }
+        }
+    }
 }
 
 // External Types

crates/wasmtime/src/runtime/vm.rs

@@ -7,6 +7,7 @@
 
 use crate::prelude::*;
 use crate::store::StoreOpaque;
+use crate::type_registry::RegisteredType;
 use alloc::sync::Arc;
 use core::fmt;
 use core::ops::Deref;
@@ -275,23 +276,23 @@ pub enum ModuleRuntimeInfo {
 #[derive(Clone)]
 pub struct BareModuleInfo {
     module: Arc<wasmtime_environ::Module>,
-    one_signature: Option<VMSharedTypeIndex>,
     offsets: VMOffsets<HostPtr>,
+    _registered_type: Option<RegisteredType>,
 }
 
 impl ModuleRuntimeInfo {
     pub(crate) fn bare(module: Arc<wasmtime_environ::Module>) -> Self {
-        ModuleRuntimeInfo::bare_maybe_imported_func(module, None)
+        ModuleRuntimeInfo::bare_with_registered_type(module, None)
     }
 
-    pub(crate) fn bare_maybe_imported_func(
+    pub(crate) fn bare_with_registered_type(
         module: Arc<wasmtime_environ::Module>,
-        one_signature: Option<VMSharedTypeIndex>,
+        registered_type: Option<RegisteredType>,
     ) -> Self {
         ModuleRuntimeInfo::Bare(Box::new(BareModuleInfo {
             offsets: VMOffsets::new(HostPtr, &module),
             module,
-            one_signature,
+            _registered_type: registered_type,
         }))
     }
 
@@ -393,10 +394,7 @@ impl ModuleRuntimeInfo {
                 .as_module_map()
                 .values()
                 .as_slice(),
-            ModuleRuntimeInfo::Bare(b) => match &b.one_signature {
-                Some(s) => core::slice::from_ref(s),
-                None => &[],
-            },
+            ModuleRuntimeInfo::Bare(_) => &[],
         }
     }
 

tests/all/globals.rs

@@ -428,3 +428,35 @@ fn instantiate_global_with_subtype() -> Result<()> {
 
     Ok(())
 }
+
+#[test]
+fn host_globals_keep_type_registration() -> Result<()> {
+    let engine = Engine::default();
+    let mut store = Store::new(&engine, ());
+
+    let ty = FuncType::new(&engine, [], []);
+
+    let g = Global::new(
+        &mut store,
+        GlobalType::new(
+            RefType::new(true, HeapType::ConcreteFunc(ty)).into(),
+            Mutability::Const,
+        ),
+        Val::FuncRef(None),
+    )?;
+
+    {
+        let _ty2 = FuncType::new(&engine, [ValType::I32], [ValType::I32]);
+        let ty = g.ty(&store);
+        let fty = ty.content().unwrap_ref().heap_type().unwrap_concrete_func();
+        assert!(fty.params().len() == 0);
+        assert!(fty.results().len() == 0);
+    }
+
+    let ty = g.ty(&store);
+    let fty = ty.content().unwrap_ref().heap_type().unwrap_concrete_func();
+    assert!(fty.params().len() == 0);
+    assert!(fty.results().len() == 0);
+
+    Ok(())
+}

tests/all/table.rs

@@ -337,3 +337,32 @@ fn i31ref_table_copy() -> Result<()> {
 
     Ok(())
 }
+
+#[test]
+fn host_table_keep_type_registration() -> Result<()> {
+    let engine = Engine::default();
+    let mut store = Store::new(&engine, ());
+
+    let ty = FuncType::new(&engine, [], []);
+
+    let t = Table::new(
+        &mut store,
+        TableType::new(RefType::new(true, HeapType::ConcreteFunc(ty)), 1, None),
+        Ref::Func(None),
+    )?;
+
+    {
+        let _ty2 = FuncType::new(&engine, [ValType::I32], [ValType::I32]);
+        let ty = t.ty(&store);
+        let fty = ty.element().heap_type().unwrap_concrete_func();
+        assert!(fty.params().len() == 0);
+        assert!(fty.results().len() == 0);
+    }
+
+    let ty = t.ty(&store);
+    let fty = ty.element().heap_type().unwrap_concrete_func();
+    assert!(fty.params().len() == 0);
+    assert!(fty.results().len() == 0);
+
+    Ok(())
+}