Wasmtime GitHub notifications bot (Mar 07 2020 at 08:55): Wasmtime GitHub notifications bot (Mar 07 2020 at 08:55):kubkon opened Issue #1252:
Apologies that this didn't click earlier, but is it intentional that we do not support virtual fs in snapshot0 aka wasi_unstable? #701 landed (hooray!) but after careful re-examination I've noticed it never touched any of the old snapshot code. It's not a deal breaker per se unless we perceive anybody in the future wanting to use virtual fs with the old WASI ABI.
The thing that really worries me though is that I've noticed both snapshots (wasi_unstable and wasi_snapshot_preview1) really diverge and in places where they supposedly shouldn't. Virtual fs is one of those places (but there are more!). My question here is, given that
wiggleis hopefully round the corner (I plan to have a fully working port in the coming week), @pchickey and I can start figuring out a way of polyfilling syscall logic to different ABI snapshots which would make the out-of-sync issue essentially disappear (to a degree ofc!).Also, I remember having a discussion about this with @alexcrichton sometime in the past. I think we ought to really figure out a way of testing _all_ supported by Wasmtime WASI snapshots to avoid this type of errors in the future.
Oh man, what started as a simple question, ended up as an essay. Apologies!
cc @iximeow
Wasmtime GitHub notifications bot (Mar 07 2020 at 08:55):kubkon labeled Issue #1252:
Apologies that this didn't click earlier, but is it intentional that we do not support virtual fs in snapshot0 aka wasi_unstable? #701 landed (hooray!) but after careful re-examination I've noticed it never touched any of the old snapshot code. It's not a deal breaker per se unless we perceive anybody in the future wanting to use virtual fs with the old WASI ABI.
The thing that really worries me though is that I've noticed both snapshots (wasi_unstable and wasi_snapshot_preview1) really diverge and in places where they supposedly shouldn't. Virtual fs is one of those places (but there are more!). My question here is, given that
wiggleis hopefully round the corner (I plan to have a fully working port in the coming week), @pchickey and I can start figuring out a way of polyfilling syscall logic to different ABI snapshots which would make the out-of-sync issue essentially disappear (to a degree ofc!).Also, I remember having a discussion about this with @alexcrichton sometime in the past. I think we ought to really figure out a way of testing _all_ supported by Wasmtime WASI snapshots to avoid this type of errors in the future.
Oh man, what started as a simple question, ended up as an essay. Apologies!
cc @iximeow
Wasmtime GitHub notifications bot (Mar 07 2020 at 08:55):kubkon labeled Issue #1252:
Apologies that this didn't click earlier, but is it intentional that we do not support virtual fs in snapshot0 aka wasi_unstable? #701 landed (hooray!) but after careful re-examination I've noticed it never touched any of the old snapshot code. It's not a deal breaker per se unless we perceive anybody in the future wanting to use virtual fs with the old WASI ABI.
The thing that really worries me though is that I've noticed both snapshots (wasi_unstable and wasi_snapshot_preview1) really diverge and in places where they supposedly shouldn't. Virtual fs is one of those places (but there are more!). My question here is, given that
wiggleis hopefully round the corner (I plan to have a fully working port in the coming week), @pchickey and I can start figuring out a way of polyfilling syscall logic to different ABI snapshots which would make the out-of-sync issue essentially disappear (to a degree ofc!).Also, I remember having a discussion about this with @alexcrichton sometime in the past. I think we ought to really figure out a way of testing _all_ supported by Wasmtime WASI snapshots to avoid this type of errors in the future.
Oh man, what started as a simple question, ended up as an essay. Apologies!
cc @iximeow
Wasmtime GitHub notifications bot (Mar 07 2020 at 09:11):kubkon commented on Issue #1252:
Also, while we're here. @sunfishcode @alexcrichton @pchickey do you guys think we should introduce
wigglein _both_ snapshots, or target only the current one (wasi_snapshot_preview1), and after we figure out the polyfill mechanics, backport to wasi_unstable?
Wasmtime GitHub notifications bot (Mar 09 2020 at 15:20):iximeow commented on Issue #1252:
On the topic of virtual fs in in snapshot0 specifically.. I simply didn't think about it. I can't imagine a reason snapshot0 should not be able to use snapshot0. @pchickey mentioned that lucet-wasi has been on snapshot0 for a bit while
wigglehas gotten built out, but at one point while I was building it out I definitely had it using virtual files, so I'm highly suspicious of my understanding of.. something, somewhere.
Wasmtime GitHub notifications bot (Mar 09 2020 at 15:27):kubkon commented on Issue #1252:
Right, OK. I thought I'd double check with you before doing any further refactoring etc. So now the real question is whether we want to backport its functionality as-is to snapshot0 (and this involves a lot of nasty copy-pasting here and there), or if we can wait a little bit for
wiggleto make entrance, figure out the polyfill mechanism between different snapshots, and then backporting is given to us for free. Thoughts? :-)
Wasmtime GitHub notifications bot (Mar 09 2020 at 18:53):pchickey commented on Issue #1252:
The path I was thinking we'd follow here is:
Land wiggle support in snapshot 1 only. Keep the snapshot 0 tree intact, including using the old
wigtool. Since its entirely different code, this should be pretty easy, right?Teach wiggle about the witx polyfill feature for calculating type & func call equivalency. For polyfilled modules, let wiggle_generate spit out a
mod snapshot_0 { mod types { <ts> } <funcs> }where we fill in equivalent types with aliases (e.g.type Foo = super::super::snapshot_1::types::Foo) and equivalent function calls with calls to the snapshot_1 version as well. Require the user provide manual implementations for all non-equivalent function calls.Land that wiggle feature in wasi-common, deleting snapshot 0 subtree and wig.
Wasmtime GitHub notifications bot (Mar 09 2020 at 18:58):pchickey edited a comment on Issue #1252:
The path I was thinking we'd follow here is:
Land wiggle support in snapshot 1 only. Keep the snapshot 0 tree intact, including using the old
wigtool. Since its entirely different code, this should be pretty easy, right?Teach wiggle about the witx polyfill feature for calculating type & func call equivalency. For polyfilled modules, let wiggle_generate spit out a
mod snapshot_0 { mod types { <ts> } <funcs> }where we fill in equivalent types with aliases (e.g.type Foo = super::super::snapshot_1::types::Foo), non-equivalent types with their definition. Fill in the equivalent function calls with the same code we generate in snapshot 1 - validate the inputs, and call the snapshot 1 trait method. The non-equivalent function calls will call a snapshot 0 trait method. The user is responsible for implementing these non-automatic methods in the snapshot 0 trait just like they are the snapshot 1 trait.Land that wiggle feature in wasi-common, deleting snapshot 0 subtree and wig.
Wasmtime GitHub notifications bot (Mar 09 2020 at 20:51):kubkon commented on Issue #1252:
This plan sounds good to me! @sunfishcode I'd love to hear your thoughts on this as well!
Wasmtime GitHub notifications bot (Mar 09 2020 at 21:01):sunfishcode commented on Issue #1252:
That sounds reasonable to me! And of course, it continues to make sense to factor out code into yanix/winx and into modules which aren't per-snapshot (currently we just have sandboxed_tty_writer) so that for the cases where we do have to use manual code, we can share as much as possible.
Wasmtime GitHub notifications bot (Mar 09 2020 at 21:06):kubkon commented on Issue #1252:
Just an FYI that after #1253 lands, I'll restart my efforts at porting
wiggleintowasi-common. If everything goes well, I'm really hopeful to have it fully working by the end of this week at the latest. This would mean we could start looking at polyfills soon.
Last updated: Dec 23 2024 at 12:05 UTC