[34.0.0] Fix globals/tables holding their registered types (#11104)

* Fix globals/tables holding their registered types

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

Closes #11102

* Fix compile

Author: alexcrichton

crates/wasmtime/src/runtime/trampoline.rs

@@ -12,36 +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, ModuleRuntimeInfo, OnDemandInstanceAllocator, SharedMemory, VMFunctionImport,
-};
-use crate::store::{AllocateInstanceKind, InstanceId, StoreOpaque};
+use crate::runtime::vm::SharedMemory;
+use crate::store::StoreOpaque;
 use crate::{MemoryType, TableType};
-use alloc::sync::Arc;
-use wasmtime_environ::{MemoryIndex, Module, TableIndex, VMSharedTypeIndex};
-
-fn create_handle(
-    module: Module,
-    store: &mut StoreOpaque,
-    func_imports: &[VMFunctionImport],
-    one_signature: Option<VMSharedTypeIndex>,
-) -> Result<InstanceId> {
-    let mut imports = Imports::default();
-    imports.functions = func_imports;
-
-    unsafe {
-        let allocator =
-            OnDemandInstanceAllocator::new(store.engine().config().mem_creator.clone(), 0, false);
-        let module = Arc::new(module);
-        store.allocate_instance(
-            AllocateInstanceKind::Dummy {
-                allocator: &allocator,
-            },
-            &ModuleRuntimeInfo::bare_maybe_imported_func(module, one_signature),
-            imports,
-        )
-    }
-}
+use wasmtime_environ::{MemoryIndex, TableIndex};
 
 pub fn generate_memory_export(
     store: &mut StoreOpaque,

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

@@ -1,5 +1,6 @@
 use crate::runtime::vm::{ExportGlobalKind, StoreBox, VMGlobalDefinition};
 use crate::store::{AutoAssertNoGc, StoreOpaque};
+use crate::type_registry::RegisteredType;
 use crate::{GlobalType, Mutability, Result, RootedGcRefImpl, Val};
 use core::ptr::{self, NonNull};
 use wasmtime_environ::{DefinedGlobalIndex, EntityRef, Global};
@@ -8,6 +9,8 @@ use wasmtime_environ::{DefinedGlobalIndex, EntityRef, Global};
 pub struct VMHostGlobalContext {
     pub(crate) ty: Global,
     pub(crate) global: VMGlobalDefinition,
+
+    _registered_type: Option<RegisteredType>,
 }
 
 pub fn generate_global_export(
@@ -25,6 +28,7 @@ pub fn generate_global_export(
     let ctx = StoreBox::new(VMHostGlobalContext {
         ty: global,
         global: VMGlobalDefinition::new(),
+        _registered_type: ty.into_registered_type(),
     });
 
     let mut store = AutoAssertNoGc::new(store);

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

@@ -56,7 +56,7 @@ pub fn create_memory(
             AllocateInstanceKind::Dummy {
                 allocator: &allocator,
             },
-            &ModuleRuntimeInfo::bare_maybe_imported_func(Arc::new(module), None),
+            &ModuleRuntimeInfo::bare(Arc::new(module)),
             Default::default(),
         )
     }

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

@@ -1,7 +1,8 @@
 use crate::TableType;
 use crate::prelude::*;
-use crate::store::{InstanceId, StoreOpaque};
-use crate::trampoline::create_handle;
+use crate::runtime::vm::{Imports, ModuleRuntimeInfo, OnDemandInstanceAllocator};
+use crate::store::{AllocateInstanceKind, InstanceId, StoreOpaque};
+use alloc::sync::Arc;
 use wasmtime_environ::{EntityIndex, Module, TypeTrace};
 
 pub fn create_table(store: &mut StoreOpaque, table: &TableType) -> Result<InstanceId> {
@@ -22,5 +23,21 @@ pub fn create_table(store: &mut StoreOpaque, table: &TableType) -> Result<Instan
         .exports
         .insert(String::new(), EntityIndex::Table(table_id));
 
-    create_handle(module, store, &[], None)
+    let imports = Imports::default();
+
+    unsafe {
+        let allocator =
+            OnDemandInstanceAllocator::new(store.engine().config().mem_creator.clone(), 0, false);
+        let module = Arc::new(module);
+        store.allocate_instance(
+            AllocateInstanceKind::Dummy {
+                allocator: &allocator,
+            },
+            &ModuleRuntimeInfo::bare_with_registered_type(
+                module,
+                table.element().clone().into_registered_type(),
+            ),
+            imports,
+        )
+    }
 }

crates/wasmtime/src/runtime/types.rs

@@ -362,6 +362,13 @@ impl ValType {
             }
         }
     }
+
+    pub(crate) fn into_registered_type(self) -> Option<RegisteredType> {
+        match self {
+            ValType::Ref(ty) => ty.into_registered_type(),
+            _ => None,
+        }
+    }
 }
 
 /// Opaque references to data in the Wasm heap or to host data.
@@ -536,6 +543,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 +1171,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
@@ -2502,6 +2525,10 @@ impl GlobalType {
             .ok_or_else(|| anyhow!("global type has no default value"))?;
         RuntimeGlobal::new(store, self.clone(), val)
     }
+
+    pub(crate) fn into_registered_type(self) -> Option<RegisteredType> {
+        self.content.into_registered_type()
+    }
 }
 
 // Tag Types

crates/wasmtime/src/runtime/vm.rs

@@ -35,6 +35,7 @@ pub(crate) struct f64x2(crate::uninhabited::Uninhabited);
 
 use crate::prelude::*;
 use crate::store::StoreOpaque;
+use crate::type_registry::RegisteredType;
 use alloc::sync::Arc;
 use core::fmt;
 use core::ops::Deref;
@@ -313,23 +314,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,
         }))
     }
 
@@ -431,10 +432,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

@@ -432,3 +432,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(())
+}