Wasmi `v0.32.0-beta.13` seems to have broken linking

[tomaka]

Wasmi `v0.32.0-beta.13` seems to have broken linking

[tomaka]

you are not using Linker::func_new

I have no idea if one way to do things is better than the other 🤷

or even better Linker::func_wrap?

I need to be able to accept unresolved imports. If a function name is not recognized by smoldot, the VM should still be able run, and errors should happen only if the function is actually called. To achieve that, I can't know the function signature at compilation time.
And I use the same code for both recognized and unrecognized functions by convenience.

Does using Linker::func_new or Linker::func_wrap solve your issues?

func_new doesn't solve it, no.

[tomaka]

does enabling the new no-hash-maps crate feature of Wasmi solve your issues?

It doesn't compile:

error[E0432]: unresolved imports `std::cmp`, `std::mem`, `std::ops`
 --> /Users/tomaka/.cargo/registry/src/index.crates.io-6f17d22bba15001f/wasmi_collections-0.1.0/src/string_interner/detail.rs:4:5
  |
4 |     cmp::Ordering,
  |     ^^^ could not find `cmp` in `std`
5 |     collections::{btree_map::Entry, BTreeMap},
6 |     mem,
  |     ^^^ no `mem` in the root
7 |     ops::Deref,
  |     ^^^ could not find `ops` in `std`
  |
  = help: consider importing one of these items instead:
          core::mem
          std::mem

For more information about this error, try `rustc --explain E0432`.
error: could not compile `wasmi_collections` (lib) due to 1 previous error

[tomaka]

If I activate both std and no-hash-maps, the issue is fixed.

[tomaka]

smoldot does not propagate its std feature

Smoldot is entirely no-std. Smoldot's std feature is completely disconnected from the VM.

enabling either wasmi/std, wasmi/no-hash-maps

I'm happy to enable no-hash-maps if it compiles.

[Robbepop]

@tomaka I opened this PR to fix no_std+no-hash-maps builds which should help you until the underlying issue is properly fixed.

If you can confirm me that the PR fixes your issue for now I will create another beta release today and work on the proper fix in the meantime.

[Robbepop]

@tomaka I released Wasmi version 0.32.0-beta.14 with the proper fix included.

[Robbepop]

@tomaka thanks for reporting the bug, I am going to investigate it with high priority.

My random guess as what might be causing trouble is the fact that I'm using the generic function constructor:

Is there a specific reason as to why you are not using Linker::func_new or even better Linker::func_wrap?

Does using Linker::func_new or Linker::func_wrap solve your issues?

[Robbepop]

Okay, very weird question but does enabling the new no-hash-maps crate feature of Wasmi solve your issues?

[Robbepop]

smol-dot/smoldot#1817 udpates from beta.10 directly to beta.13. Can you tell me which was the last working beta.N?

I built smoldot locally and can confirm that Wasmi v0.32.0-beta.12 still worked and the problems started with Wasmi v0.32.0-beta.13.

[Robbepop]

@tomaka

I took one of the failing smoldot tests and re-created a regression test in the Wasmi repository, however, I somehow cannot reproduce it there. Can you please take a look and maybe tell me what smoldot is doing differently?

#[test]
fn smoldot_regression() {
    use crate::{Engine, Func, Linker, Memory, Module, Store};
    let wasm = wat::parse_str(
        r#"
        (module
            (import "host" "hello" (func $host_hello (param i32) (result i32)))
            (import "env" "memory" (memory $mem 0 4096))
            (func (export "hello") (result i32)
                (call $host_hello (i32.const 3))
                (i32.const 2)
                i32.add
            )
        )"#,
    )
    .unwrap();
    let engine = Engine::default();
    let mut store = Store::new(&engine, ());
    let module = Module::new(&engine, &wasm[..]).unwrap();
    let mut linker = <Linker<()>>::new(&engine);
    for import_type in module.imports() {
        let name = import_type.import_name();
        match import_type.ty() {
            ExternType::Memory(memory_type) => {
                let memory = Memory::new(&mut store, memory_type.clone()).unwrap();
                linker.define(name.module(), name.name(), memory).unwrap();
            }
            ExternType::Func(func_type) => {
                let func = Func::new(
                    &mut store,
                    func_type.clone(),
                    |_caller, _params, _results| unimplemented!(),
                );
                linker.define(name.module(), name.name(), func).unwrap();
            }
            ExternType::Global(_) => unimplemented!(),
            ExternType::Table(_) => unimplemented!(),
        }
    }
    // In smoldot this call fails ...
    linker.instantiate(&mut store, &module).unwrap();
}

[Robbepop]

I debugged it some more in smoldot:

This call fails:
https://github.com/smol-dot/smoldot/blob/61cf6a956907f384e8ed3225a4145ff47b8bb26b/lib/src/executor/vm/interpreter.rs#L178

Because wasmi::Linker::get("host", "hello") at this point returns None when not enabling wasmi/no-hash-maps crate feature. I have not yet figured out why. My base assumption is that this is connected to the wasmi_collections::StringInterner which returns consecutive indices for Symbol in no-hash-maps mode and non-consecutive Symbol indices otherwise. Unfortunately I am unable to reproduce this at Wasmi locally ...

[Robbepop]

I figured out that I cannot reproduce this directly inside the Wasmi crate test suite since the Wasmi test suite does not compile under no_std but the error in smoldot only happens in Wasmi's no_std mode without the no-hash-maps feature enable. Thus enabling either wasmi/std, wasmi/no-hash-maps or both will fix the issue on the smoldot side.

@tomaka smoldot does not propagate its std feature for its wasmi dependency. Is this an oversight or wanted behavior?

[Robbepop]

This minimized Wasmi test case fails only if both std and no-hash-maps crate features are disabled:

#[test]
fn min_smoldot_regression() {
    let mut interner = crate::collections::StringInterner::new();
    let sym = interner.get_or_intern("hello");
    assert_eq!(interner.get("hello"), Some(sym));
}

Reason being that in the aforementioned mode make_hash calls in StringInterner::{get, get_or_intern} return different hashes for the same string which is obviously incorrect. As of now I have no clue why. On std mode everything seems to work. So my best guess is that it has something to do with how wasmi_collections use ahash in no_std mode.