Skip to content

Commit

Permalink
Require declared subtyping when linking module import/export functions
Browse files Browse the repository at this point in the history 
We were previously incorrectly doing a (shallow) match on structure when we
should have been doing a subtyping check.
  • Loading branch information
@fitzgen
fitzgen committed Oct 10, 2024
1 parent a96e33e commit 7e78047
Show file tree
Hide file tree
Showing 6 changed files with 166 additions and 136 deletions.
2 changes: 0 additions & 2 deletions crates/environ/src/module_types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,6 @@ impl ModuleTypes {
#[cfg(feature = "compile")]
impl ModuleTypes {
/// Associate `trampoline_ty` as the trampoline type for `for_ty`.
///
/// This is really only for use by the `ModuleTypesBuilder`.
pub fn set_trampoline_type(
&mut self,
for_ty: ModuleInternedTypeIndex,
Expand Down
38 changes: 29 additions & 9 deletions crates/wasmtime/src/runtime/type_registry.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,12 +119,15 @@ impl TypeCollection {
.write()
.register_module_types(&**gc_runtime, module_types);

log::trace!("Begin building module's shared-to-module-trampoline-types map");
let mut trampolines = SecondaryMap::with_capacity(types.len());
for (module_ty, trampoline) in module_types.trampoline_types() {
for (module_ty, module_trampoline_ty) in module_types.trampoline_types() {
let shared_ty = types[module_ty];
let trampoline_ty = registry.trampoline_type(shared_ty);
trampolines[trampoline_ty] = Some(trampoline).into();
let trampoline_shared_ty = registry.trampoline_type(shared_ty);
trampolines[trampoline_shared_ty] = Some(module_trampoline_ty).into();
log::trace!("--> shared_to_module_trampolines[{trampoline_shared_ty:?}] = {module_trampoline_ty:?}");
}
log::trace!("Done building module's shared-to-module-trampoline-types map");

Self {
engine,
Expand Down Expand Up @@ -711,14 +714,21 @@ impl TypeRegistryInner {
// type in the rec group.
for shared_type_index in entry.0.shared_type_indices.iter().copied() {
let slab_id = shared_type_index_to_slab_id(shared_type_index);
if let Some(f) = self.types[slab_id].as_func() {
match f.trampoline_type() {
Cow::Borrowed(_) => {
let sub_ty = &self.types[slab_id];
if let Some(f) = sub_ty.as_func() {
let trampoline = f.trampoline_type();
match &trampoline {
Cow::Borrowed(_) if sub_ty.is_final && sub_ty.supertype.is_none() => {
// The function type is its own trampoline type. Leave
// its entry in `type_to_trampoline` empty to signal
// this.
log::trace!(
"function type is its own trampoline type: \n\
--> trampoline_type[{shared_type_index:?}] = {shared_type_index:?}\n\
--> trampoline_type[{f}] = {f}"
);
}
Cow::Owned(trampoline) => {
Cow::Borrowed(_) | Cow::Owned(_) => {
// This will recursively call into rec group
// registration, but at most once since trampoline
// function types are their own trampoline type.
Expand All @@ -728,13 +738,23 @@ impl TypeRegistryInner {
is_final: true,
supertype: None,
composite_type: wasmtime_environ::WasmCompositeType::Func(
trampoline,
trampoline.into_owned(),
),
},
);
let trampoline_index = trampoline_entry.0.shared_type_indices[0];
log::trace!(
"Registering trampoline {trampoline_index:?} for function type {shared_type_index:?}"
"Registering trampoline type:\n\
--> trampoline_type[{shared_type_index:?}] = {trampoline_index:?}\n\
--> trampoline_type[{f}] = {g}",
f = {
let slab_id = shared_type_index_to_slab_id(shared_type_index);
self.types[slab_id].unwrap_func()
},
g = {
let slab_id = shared_type_index_to_slab_id(trampoline_index);
self.types[slab_id].unwrap_func()
}
);
debug_assert_ne!(shared_type_index, trampoline_index);
self.type_to_trampoline[shared_type_index] = Some(trampoline_index).into();
Expand Down
29 changes: 4 additions & 25 deletions crates/wasmtime/src/runtime/types/matching.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
use crate::type_registry::RegisteredType;
use crate::{linker::DefinitionType, Engine, FuncType};
use crate::{prelude::*, ArrayType, StructType};
use crate::prelude::*;
use crate::{linker::DefinitionType, Engine};
use wasmtime_environ::{
EntityType, Global, IndexType, Memory, ModuleTypes, Table, TypeTrace, VMSharedTypeIndex,
WasmHeapType, WasmRefType, WasmSubType, WasmValType,
Expand All @@ -17,30 +16,10 @@ impl MatchCx<'_> {
}

fn type_reference(&self, expected: VMSharedTypeIndex, actual: VMSharedTypeIndex) -> Result<()> {
// Avoid matching on structure for subtyping checks when we have
// precisely the same type.
let matches = expected == actual || {
let expected = RegisteredType::root(self.engine, expected).unwrap();
let actual = RegisteredType::root(self.engine, actual).unwrap();
if expected.is_array() && actual.is_array() {
let expected = ArrayType::from_registered_type(expected);
let actual = ArrayType::from_registered_type(actual);
actual.matches(&expected)
} else if expected.is_func() && actual.is_func() {
let expected = FuncType::from_registered_type(expected);
let actual = FuncType::from_registered_type(actual);
actual.matches(&expected)
} else if expected.is_struct() && actual.is_struct() {
let expected = StructType::from_registered_type(expected);
let actual = StructType::from_registered_type(actual);
actual.matches(&expected)
} else {
false
}
};
if matches {
if self.engine.signatures().is_subtype(actual, expected) {
return Ok(());
}

let msg = "types incompatible";
let expected = match self.engine.signatures().borrow(expected) {
Some(ty) => ty,
Expand Down
144 changes: 71 additions & 73 deletions tests/all/func.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2147,107 +2147,105 @@ fn wasm_to_host_trampolines_and_subtyping(config: &mut Config) -> Result<()> {

let engine = Engine::new(&config)?;

let ft0 = FuncType::with_finality_and_supertype(
&engine,
Finality::NonFinal,
None,
[],
[ValType::Ref(RefType::new(true, HeapType::Extern))],
)?;
let ft1 = FuncType::with_finality_and_supertype(
&engine,
Finality::NonFinal,
Some(&ft0),
[],
[ValType::Ref(RefType::new(false, HeapType::Extern))],
)?;

let module = Module::new(
&engine,
r#"
(module
(type $ty (func (result (ref null extern))))
(type $ft0 (sub (func (result (ref null extern)))))
(import "" "a" (func $imported_func (type $ty)))
(import "" "return_func" (func $return_func (result (ref $ty))))
(global $g (export "g") (mut (ref null $ty)) (ref.null $ty))
(table $t (export "t") 10 10 (ref null $ty))
(import "" "a" (func $imported_func (type $ft0)))
(import "" "return_func" (func $return_func (result (ref $ft0))))
(global $g (export "g") (mut (ref null $ft0)) (ref.null $ft0))
(table $t (export "t") 10 10 (ref null $ft0))
(func (export "f")
(drop (call $imported_func))
(drop (call_ref $ty (call $return_func)))
(drop (call_ref $ty (global.get $g)))
(drop (call_ref $ty (table.get $t (i32.const 0))))
(drop (call_ref $ft0 (call $return_func)))
(drop (call_ref $ft0 (global.get $g)))
(drop (call_ref $ft0 (table.get $t (i32.const 0))))
)
)
"#,
)?;

// This defines functions of type `(func (result (ref extern)))` and not of
// This defines a function of type `(func (result (ref extern)))` and not of
// type `(func (result (ref null extern)))` that the Wasm imports. But it is
// still a subtype of the Wasm import's type, so we should be able to use
// the Wasm's precompiled trampolines with this host function.
//
// But this means we also need to look up the trampoline by the caller's
// type, rather than the callee's type.
//
// So take each kind of function we can define, and give it to Wasm in every
// way that Wasm can receive a function, and make sure we find the correct
// trampolines in all cases and everything works.
//
// Note that we create multiple versions of the same function by calling
// these constructors multiple times in the loop below. This is to avoid the
// scenario where we reuse the function, the first time it is exposed to
// Wasm its trampoline is filled in, and then all subsequent uses don't
// actually exercise their associated code paths for finding and
// initializing their trampoline.
let func_ty = FuncType::new(&engine, [], [RefType::new(false, HeapType::Extern).into()]);
let func_ctors: Vec<Box<dyn Fn(&mut Store<_>) -> Func>> = vec![
Box::new(|store| {
Func::wrap(store, |mut caller: Caller<'_, ()>| {
ExternRef::new(&mut caller, 100)
})
}),
Box::new(|store| {
Func::new(
store,
func_ty.clone(),
|mut caller: Caller<'_, ()>, _args, results| {
results[0] = ExternRef::new(&mut caller, 200)?.into();
Ok(())
},
)
}),
];
// So give it to Wasm in every way that Wasm can receive a function, and
// make sure we find the correct trampolines in all cases and everything
// works.
let make_func = |store: &mut Store<_>| {
Func::new(
store,
ft1.clone(),
|mut caller: Caller<'_, ()>, _args, results| {
results[0] = ExternRef::new(&mut caller, 200)?.into();
Ok(())
},
)
};

let return_func_ty = module.imports().nth(1).unwrap().ty().unwrap_func().clone();

for make_func in func_ctors {
let mut store = Store::new(&engine, ());
let mut store = Store::new(&engine, ());

let imported_func = make_func(&mut store);
assert!(FuncType::eq(&imported_func.ty(&store), &func_ty));
let imported_func = make_func(&mut store);
assert!(FuncType::eq(&imported_func.ty(&store), &ft1));

let returned_func = make_func(&mut store);
assert!(FuncType::eq(&returned_func.ty(&store), &func_ty));
let returned_func = make_func(&mut store);
assert!(FuncType::eq(&returned_func.ty(&store), &ft1));

let return_func = Func::new(
&mut store,
return_func_ty.clone(),
move |_caller, _args, results| {
results[0] = returned_func.into();
Ok(())
},
);
let return_func = Func::new(
&mut store,
return_func_ty.clone(),
move |_caller, _args, results| {
results[0] = returned_func.into();
Ok(())
},
);

let instance = Instance::new(
&mut store,
&module,
&[imported_func.into(), return_func.into()],
)?;
let instance = Instance::new(
&mut store,
&module,
&[imported_func.into(), return_func.into()],
)?;

let g = make_func(&mut store);
assert!(FuncType::eq(&g.ty(&store), &func_ty));
instance
.get_global(&mut store, "g")
.unwrap()
.set(&mut store, g.into())?;

let t = make_func(&mut store);
assert!(FuncType::eq(&t.ty(&store), &func_ty));
instance
.get_table(&mut store, "t")
.unwrap()
.set(&mut store, 0, t.into())?;

let f = instance.get_typed_func::<(), ()>(&mut store, "f")?;
f.call(&mut store, ())?;
}
let g = make_func(&mut store);
assert!(FuncType::eq(&g.ty(&store), &ft1));
instance
.get_global(&mut store, "g")
.unwrap()
.set(&mut store, g.into())?;

let t = make_func(&mut store);
assert!(FuncType::eq(&t.ty(&store), &ft1));
instance
.get_table(&mut store, "t")
.unwrap()
.set(&mut store, 0, t.into())?;

let f = instance.get_typed_func::<(), ()>(&mut store, "f")?;
f.call(&mut store, ())?;

Ok(())
}
Loading

0 comments on commit 7e78047

Please sign in to comment.