alexcrichton added the bug label to Issue #8432.
alexcrichton added the wasmtime:api label to Issue #8432.
alexcrichton opened issue #8432:
This program:
use wasmtime::*; fn main() { let engine = Engine::default(); let module = Module::new( &engine, r#" (module (import "" "a" (func $a (result externref))) (func (export "a") (drop (call $a)) ) ) "#, ) .unwrap(); let mut linker = Linker::new(&engine); linker .func_wrap("", "a", |mut caller: Caller<'_, ()>| { ExternRef::new(&mut caller, 100) }) .unwrap(); let mut store = Store::new(&engine, ()); let i = linker.instantiate(&mut store, &module).unwrap(); }
compiled against the
main
branch currently runs as:$ cargo run -q thread 'main' panicked at /home/alex/.cargo/git/checkouts/wasmtime-41807828cb3a7a7e/4b9f53a/crates/wasmtime/src/runtime/func.rs:1306:74: must have a wasm-to-native trampoline for this signature if the Wasm module is importing a function of this signature note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
cc @fitzgen
fitzgen commented on issue #8432:
As discussed offline, we should look up the trampoline based on the import signature, not the wrapped function's type (which can be a subtype of the import signature).
shamiao commented on issue #8432:
also happens when
externref
appeared as a func param.
shamiao edited a comment on issue #8432:
also happens when
externref
appeared as a func param.application note: unsafe fn
Linker::func_new_unchecked
can be used to bypass this panic.
fitzgen commented on issue #8432:
I started poking at this. Things are a bit hairy because we update
FuncData
based on whether we have found a trampoline for a host function or not yet, but we could update that to hold onto a trampoline for some supertype of the function which is compatible with that particular use but "incompatible" with some other use that uses the function as its precise type rather than the previous super type. I put "incompatible" in scare quotes because it isn't actually incompatible because the trampolines aren't doing anything with these values other than shuffling them around to different ABI argument/return registers and stack slots. So any trampoline we find would be valid for all future uses, at least as the system currently exists, but this is a pretty subtle invariant to rely upon almost by accident.I think the best way to move forward is to center that invariant, and not make it an accidental after thought, by
- keeping track of every function type's "trampoline signature" in
ModuleTypes
or somewhere around there- a "trampoline signature" is where we map all params/results from
(ref null? <heapty>)
to(ref null <top(heapty)>)
- (note that this is slightly different from the least-upper-bound of all concrete function types that this function type has a partial ordering with; that would require mapping params to bottom and results to top, which seems like a bit of a footgun if we ever give cranelift more knowledge of uninhabited types. instead, we are relying on implicit unsafe downcasts from the top type to the function's actual type for arguments here, which seems fine given that we control representation and otherwise have type/safety checks on calls)
- we only generate trampolines for each trampoline signature, not every function signature
- for modules without any reference types and subtyping, which is 99.9999% of modules, this will be the exact same number of trampolines as today
- for modules with lots of reference types and subtyping, this will actually dedupe trampolines that are byte-for-byte identical today
looking up a trampoline is now a two-step process, where we introduce a step just before our existing type-to-trampoline lookup:
- (new) get the
VMSharedTypeIndex
of this function's type's trampoline signature- (existing) get the trampoline associated with that type index
alexcrichton commented on issue #8432:
I like the sound of that yeah and it makes sense to me!
fitzgen commented on issue #8432:
(I've been working on a patch, still have a few kinks to work out)
fitzgen closed issue #8432:
This program:
use wasmtime::*; fn main() { let engine = Engine::default(); let module = Module::new( &engine, r#" (module (import "" "a" (func $a (result externref))) (func (export "a") (drop (call $a)) ) ) "#, ) .unwrap(); let mut linker = Linker::new(&engine); linker .func_wrap("", "a", |mut caller: Caller<'_, ()>| { ExternRef::new(&mut caller, 100) }) .unwrap(); let mut store = Store::new(&engine, ()); let i = linker.instantiate(&mut store, &module).unwrap(); }
compiled against the
main
branch currently runs as:$ cargo run -q thread 'main' panicked at /home/alex/.cargo/git/checkouts/wasmtime-41807828cb3a7a7e/4b9f53a/crates/wasmtime/src/runtime/func.rs:1306:74: must have a wasm-to-native trampoline for this signature if the Wasm module is importing a function of this signature note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
cc @fitzgen
Last updated: Dec 23 2024 at 12:05 UTC