Stream: git-wasmtime

Topic: wasmtime / Issue #1237 Add a first-class way of accessing...

Wasmtime GitHub notifications bot (Mar 09 2020 at 19:40):

sunfishcode commented on Issue #1237:

The part about this that makes me nervous is being able to access a module's memory without it being exported.

Obviously, requiring modules to export their memory is unfortunate, and the simple name "memory" isn't ideal, and we should do something better, but for now, it does help to model what's going on. You're calling out with i32 values which are pointers to a memory accessed via the "memory" export. We don't want Wasmtime or any tools assuming that a non-exported memory isn't mutated by anything outside the module. The export also helps in the case of multiple memories, because you can pick which one to export as "memory".

Would it still achieve your goal here if you reinstated the lookup("memory") logic in from_abi, keeping the rest of the feature?

Wasmtime GitHub notifications bot (Mar 10 2020 at 06:49):

lostman commented on Issue #1237:

Typically you'll have to set up an Rc<RefCell<Option<Memory>>>, close over that in your function imports, and then fill it in once the instance is created.

When an instance is created it automatically runs the (start) function. If the Rc<...> was the only way to access memory then it wouldn't be possible to use a memory-accessing host call inside (start).

Is that the intended behavior?

Alternatively, if one could make an instance, tie the knot, and only then explicitly run (start), the Rc<...> approach would work.

How would interface types solve this problem?

Wasmtime GitHub notifications bot (Mar 10 2020 at 15:55):

alexcrichton commented on Issue #1237:

@sunfishcode yeah we could definitely go that route, only giving access to exported memories. I would prefer to not hardcode the string "memory" , though, so we could switch to something like:

Func::wrap1(&store, |mem: wasmtime::Caller| {
    let mem: Option<&Memory> = mem.get_export("memory").and_then(|e| e.memory());
    // ...
});

That way the string "memory" would still be passed in by the caller, and we could enhance CallerMemory over time with other stuff if we really wanted. For now it'd just be a thin wrapper around *mut VMContext.

@lostman yes the current way to implement this today with Rc doesn't support calling imports in the start function, but this is basically the same limitation of the JS API and is one of the known caveats of the start function.

Interface types would solve this issue because callers would simply say "I need a string", and it's the job of wasmtime to figure out how to give you that string, not the caller to figure out how to read the string out of linear memory.

Wasmtime GitHub notifications bot (Mar 10 2020 at 16:17):

sunfishcode commented on Issue #1237:

Passing in the "memory" string sounds good. Maybe we could also add a pub const WASM_MEMORY_EXPORT_NAME = "memory" or some such thing that we could recommend people use with this API, so that when we do change it, we have more options for helping them avoid silent breakage.

Wasmtime GitHub notifications bot (Mar 11 2020 at 05:26):

lostman commented on Issue #1237:

What about multiple memories? Is that anywhere on the road map? If it is, any API that relies on specific names will lead to trouble.

The current situation is already a bit messy. What if a module exports memory but calls it "mem"? I recall compiling C++ to Wasm while back and there was no option to chose the name. Not sure what the current state of Rust toolchain is either.

Wasmtime GitHub notifications bot (Mar 11 2020 at 14:29):

alexcrichton commented on Issue #1237:

@lostman with @sunfishcode's suggestion nothing will be hardcoded here any more, so we'll have an easy way to specify which memory is being accessed (via an exported name). I'm working on an update to the various traits here to support all this.

Wasmtime GitHub notifications bot (Mar 11 2020 at 14:44):

sunfishcode commented on Issue #1237:

As additional background, the "memory" export name is currently hard-wired into lld, and it's become a de-facto convention that engines assume now. This is obviously not ideal, but I am optimistic that we'll have chances to move away from programs exporting their entire memory to the world, which will be opportunities to fix this.

Wasmtime GitHub notifications bot (Mar 11 2020 at 14:50):

alexcrichton commented on Issue #1237:

I'm gonna end up taking a pretty different approach to doing this, so I'm going to close this.

Wasmtime GitHub notifications bot (Mar 11 2020 at 19:14):

alexcrichton commented on Issue #1237:

Ok I've followed up with https://github.com/bytecodealliance/wasmtime/pull/1290 which I think is a better way to tackle this, thanks for the suggestion @sunfishcode!

Last updated: Oct 23 2024 at 20:03 UTC