Panic about missing trampolines with subtyping in signatures

[alexcrichton]

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]

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]

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]

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:
  1. (new) get the VMSharedTypeIndex of this function's type's trampoline signature
  2. (existing) get the trampoline associated with that type index

[alexcrichton]

I like the sound of that yeah and it makes sense to me!

[fitzgen]

(I've been working on a patch, still have a few kinks to work out)