Allow missing imports in WASM module through a feature flag

[AlexEne]

I've added the possibility to allow missing imports in modules. This has been discussed here.

To re-iterate the need for this, there are use-cases where not all host programs VMs have been updated to the latest version that exports all functions required by modules that execute on these hosts.

In such instances it's more convenient to dynamically check a feature flag and call the native methods only if they exist vs emitting different binaries for each VM version.

For example, this allows your WASM program to use this type of pattern:

// call specific function to check for functionality before calling a method that might not exist
if (vm_host_has_feature("feature_name") {
     call_function_from_vm_because_it_should_exist_here();
} else {
     fallback_bhv();
}

If the you call a missing import it will cause the VM issue a trap.

Once optional imports are standard and implemented, the escape hatch can be removed, in the meantime it's a really useful feature to have.

Maybe @tschneidereit can review this since we discussed it on zulip?

[github-actions]

Subscribe to Label Action

cc @peterhuene

This issue or pull request has been labeled: "wasmtime:api"

Thus the following users have been cc'd because of the following labels:

• peterhuene: wasmtime:api

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

[alexcrichton]

API-wise I personally feel like this would instead be a flag on Linker rather than a compile-time option, but the code is also so small here to support this I think it may be worthwhile exploring how we might be able to support this externally. Creating a Func::new which simply panics or returns a trap isn't so hard, and ideally clients that would specifically like to allow imports they don't allow could opt-in easily by implementing such behavior.

For example one option to implement this externally today is that instead of Linker::instantiate you'd use Linker::get manually on the module's imports, filling in missing ones as necessary. Put another way I think you could write a function that takes &Linker and &Module and instantiates the module with this behavior only using the public API of the wasmtime crate.

One worry I'd have with this as-is as well is that it assumes all missing imports are missing function imports, but it could be a missing import of any type and it's not entirely clear what you'd do for the other types of missing imports (e.g. a missing imported table for example).

[AlexEne]

Yes, it just allows functions to be missing, for other things it still has the behavior it had today, as I'm not sure what a "sane" alternative would be for missing non-functions.

I like your alternative, I wasn't aware it's possible. do you have an example where those APIs are used that I could start from?

[alexcrichton]

I believe a function like this should work for your purposes:

fn instantiate(linker: &Linker, module: &Module) -> Result<Instance> {
    let mut imports = Vec::new();
    for import in module.imports() {
        match linker.get(&import) {
            Some(value) => imports.push(value),
            None => match import.ty() {
                ExternType::Func(ty) => imports.push(
                    Func::new(linker.store(), ty, |_, _, _| {
                        Err(Trap::new("function not implemented"))
                    })
                    .into(),
                ),
                _ => anyhow::bail!("import not defined"),
            },
        }
    }
    Instance::new(linker.store(), module, &imports)
}

[tschneidereit]

I do feel that having a Linker flag for this would be good, perhaps called something like unstable_allow_missing_function_imports or so, to make the "functions only" part clear.

Additionally, it'd be great to also add this as an option --unstable-allow-missing-function-imports to the CLI. @AlexEne, do you think that's something you could add?

[AlexEne]

Yes, I can check how to add that to the CLI too. (and fix the name)

[AlexEne]

@tschneidereit on the command line option. I've added a unstable_allow_missing_function_imports in wasmtime/src/lib.rs in CommonOptions, and in the Config class from wasmtime/src/config.rs.

I've also added a boolean setting in the linker, but it's unclear to me where I should configure the linker to use that value from Config.

@alexcrichton: On the manual specification of missing methods, the code does look promising, I did translate it to C as that's how i'm using wasmtime, so it will require some changes to expose the linker.get( methods through the c-api crate as it's not public now. This also requires making ImportType public as now it's pub(crate).
In adition to that, crafting the trap message from C is a bit clunky as wasm_trap_new requires the store as a parameter. This means that I have to bypass it by using a global static wasm_trap_t* as the store can't be captured in a C++ lambda in order to dynamically create the trap when the missing method is invoked from wasm.

[alexcrichton]

I don't think that this needs to be in Config, at most I think it just needs a flag on Linker. @tschneidereit would you be ok with just a CLI flag, though? Since this is easy enough to implement externally and since this is in theory behavior that will get subsumed by wasm standards in the future, I'd prefer to avoid changing the Linker type if we can.

@AlexEne for the C API I think wasm_importtype_t already exists, and for acquiring the store you could either store that in a global variable or use the wasmtime_* APIs for functions which give you a wasmtime_caller_t which you can acquire the store from. We may not have the method in the C API yet to get the wasm_store_t from that but it's possible to implement.

[AlexEne]

Yes, wasm_importtype_t exists, the one that I mentioned was ImportType (the rust counterpart and parameter to linker.get( as that's currently defined as pub(crate) and can't be created in order to call the linker current method. It's not a big deal, I can shuffle things around so it's possible to create it from the c-api crate.

[AlexEne]

It seems that there isn't really an consensus on how to proceed with this, so I will close this PR for now, and will handle this in my mirror as my use-case can be solved by a patch of a couple of lines in the linker that makes this behavior default.

Once optional imports are implemented this would not be needed anyway.