witx tool can document how to polyfill some interfaces

[pchickey]

Can we automatically generate polyfills as we upgrade witx? Maybe. Is it worth the trouble? Not sure yet! Seems interesting, though.

[MarkMcCaskey]

When implementing the polyfill in Wasmer I ran into an issue which seem fundamentally unsolvable: when sizes of caller-allocated objects shrink from version n to n+1, it requires the polyfill to do extra memory allocation, or to do "clever" things like saving the state of the current memory, acting like the memory off the end of the allocation is its own, executing the new call, saving the resulting memory, rewriting the old memory, and then rewriting the new memory within the bounds of the existing allocation. The hack also breaks if the host-call accesses memory which is now aliased by the "extended" struct.

In both of these cases WASI programs that worked before can fail: due to either no memory left to allocate or accessing beyond the end of the allocation which causes memory to be out of bounds or aliased in a bad way.

I found the first way difficult due to not having a way to allocate Wasm memory safely from the host. If such a thing exists for WASI, I'm not aware of it.

Perhaps this is an acceptable tradeoff, but I wish there was a cleaner way to do this

[pchickey]

I am aiming to make polyfilling in wasi-common work without ever manipulating Wasm memory like you describe, and do all conversions in regular host code. However, I haven't actually gotten to that point yet, so I'm not yet sure if it is going to work.

I do not know of any mechanism to safely interact with a guest memory allocator from the host. We could possibly come up with one that works for one particular guest language ABI, but a portable cross-language solution would be quite difficult.

Thanks for sharing your experience with this! I'll report back with how things turn out.

[pchickey]

Current ideas on this work:

• I need to do a bunch of stuff in wasi-common before I can actually generate polyfill code, so I'm going to focus over there for a bit
• The witx ast wasn't designed to keep track of which Document one of its members (e.g. a Type) belongs to, because previously we only ever dealt with one Document at a time. This work compares two, and so its hard to give distinct names to a type defined in one document that is structurally different from a type of the same name in another. witx should permit defining typename inside modules #157 may provide a way out here.

[pchickey]

This code can now emit a markdown doc describing polyfill requirements between snapshot 1 and 0, see below. The -m command line argument maps a module name in the new spec to a module name in the old spec.

The more interesting polyfill to document is the mapping between ephemeral and snapshot 1, now that modularization has landed. To do so, this tool needs to understand function name mappings. Providing all of the mappings as command line arguments would be very long, and the wrong place to specify this sort of important information. So, I'm going to create a .witxpolyfill document parser so that we can describe these name mappings as s-expressions.

cargo run -- ../../phases/snapshot/witx/wasi_snapshot_preview1.witx polyfill ../../phases/old/snapshot_0/witx/wasi_unstable.witx -mwasi_snapshot_preview1=wasi_unstable

# Modules
## `wasi_snapshot_preview1` in terms of `wasi_unstable`
* `args_get`: full compatibility
* `args_sizes_get`: full compatibility
* `environ_get`: full compatibility
* `environ_sizes_get`: full compatibility
* `clock_res_get`: full compatibility
* `clock_time_get`: full compatibility
* `fd_advise`: full compatibility
* `fd_allocate`: full compatibility
* `fd_close`: full compatibility
* `fd_datasync`: full compatibility
* `fd_fdstat_get`: full compatibility
* `fd_fdstat_set_flags`: full compatibility
* `fd_fdstat_set_rights`: full compatibility
* `fd_filestat_get`:
    - param `fd`: compatible
    - result `error`: compatible
    - result `buf`: `filestat` is incompatible with new `filestat`
* `fd_filestat_set_size`: full compatibility
* `fd_filestat_set_times`: full compatibility
* `fd_pread`: full compatibility
* `fd_prestat_get`: full compatibility
* `fd_prestat_dir_name`: full compatibility
* `fd_pwrite`: full compatibility
* `fd_read`: full compatibility
* `fd_readdir`: full compatibility
* `fd_renumber`: full compatibility
* `fd_seek`:
    - param `fd`: compatible
    - param `offset`: compatible
    - param `whence`: `whence` is incompatible with new `whence`
    - result `error`: compatible
    - result `newoffset`: compatible
* `fd_sync`: full compatibility
* `fd_tell`: full compatibility
* `fd_write`: full compatibility
* `path_create_directory`: full compatibility
* `path_filestat_get`:
    - param `fd`: compatible
    - param `flags`: compatible
    - param `path`: compatible
    - result `error`: compatible
    - result `buf`: `filestat` is incompatible with new `filestat`
* `path_filestat_set_times`: full compatibility
* `path_link`: full compatibility
* `path_open`: full compatibility
* `path_readlink`: full compatibility
* `path_remove_directory`: full compatibility
* `path_rename`: full compatibility
* `path_symlink`: full compatibility
* `path_unlink_file`: full compatibility
* `poll_oneoff`:
    - param `in`: `ConstPointer<subscription>` is incompatible with new `ConstPointer<subscription>`
    - param `out`: compatible
    - param `nsubscriptions`: compatible
    - result `error`: compatible
    - result `nevents`: compatible
* `proc_exit`: full compatibility
* `proc_raise`: full compatibility
* `sched_yield`: full compatibility
* `random_get`: full compatibility
* `sock_recv`: full compatibility
* `sock_send`: full compatibility
* `sock_shutdown`: full compatibility

[MarkMcCaskey]

Nice! That sounds good! Having these migrations automated is huge

[sbc100]

Just to be clear this tool shows you where there is incompatibility in ABI in terms of the argument and structures passed across the boundary? Since we don't encode any semantics I assume it can't tell you when the semantics differ?

[pchickey]

Right, this tool makes no attempt to reason about semantics. Its strictly about the ABI.

We haven't gone and made any major semantic changes yet (that I'm aware of) but when we do, perhaps the name mapping language will have a way to indicate that, although a pair of functions may appear to be ABI compatible, they are not semantically compatible. We can cross that bridge when we come to it.

[pchickey]

I think this is at a reasonable point where we can merge it, and come up with a way to describe more complex mappings as a follow-on PR. I'd also be happy to pass that task to someone else if they want to contribute!

[alexcrichton]

This all seems pretty reasonable to me. I'm not really doing an exhaustive review per se since I imagine any bugs will become self-evident pretty quickly.

Could you expand a bit on how you think this'll be used in the tooling? For example would we use this to automatically generate polyfills which implement the old function as a call to the new? For cases where it doesn't match up are you thinking we'd automatically polyfill that as well or would we have manually written bindings?

[pchickey]

Thanks, I just wanted to get some sort of review so we could get it merged and keep iterating.

For tooling, I envision this being used to automatically generate old functions as a call to the new, for cases where it is still compatible. For cases where it doesn't match up, I expect tool users will still have to write bindings by hand - I don't want to take on that more general problem of mapping incompatible calls.

I designed this primarily to use in the wig tool over in wasmtime, but the markdown output is also useful for now. I haven't actually integrated it with that tool yet, because I'm shaving other parts of the yak.

[alexcrichton]

👍