wasm2c: uvwasi adapter layer for wasm2c output#2782
Conversation
e954a44 to
17eb353
Compare
|
@sbc100 Just checking if there is interest in landing this, before i spend time rebasing or cleanups |
|
I'm not against landing this, I'm just a little stretched on review time right now. I can try to get to it soon. We also have @zherczeg who is been quite active recently on the project and could maybe review ? |
|
@sbc100 thanks and no worries! I'll push a rebase soon to keep the diff/merge clean |
|
I can review it, but I don't know the context, so I need to understand the purpose of this code first. It adds several files to gitignore, two files to examples, and an adapter. Is it adds some built ins? How wasm and libuw is connected? Is it something wasm-only, or do you need wasm-host communication as well? |
| return UVWASI_ENOMEM; | ||
| } | ||
|
|
||
| ret = uvwasi_args_get(wasi->uvwasi, uv_argv, buf_base); |
There was a problem hiding this comment.
Where are these calls coming from? Which include provides them?
There was a problem hiding this comment.
This comes from uvwasi.h that is included in wasm-rt-uvwasi-adapter.h
Broadly the idea here is that Wasm programs can be compiled as a "pure compute" Wasm program (target of For a program compiled to the WASI backend, the Wasm compiler includes a wasi-libc that provides the standard C library functions like This PR implements the uvwasi adapter for wasm2c. |
|
Thank you for the explanation. |
zherczeg
left a comment
There was a problem hiding this comment.
The code looks valid. Does the patch contains all wasi 0.1 functions or it is a partial implementation of them?
| hello-wasi | ||
| hello-wasi.wasm | ||
| hello-wasi-wasm.* | ||
| test.out |
There was a problem hiding this comment.
Usually tmp files goes to the bin/ build/ or out/ directories when wabt is compiled. Can these files goes there as well (and no need to add them to gitignore)?
| uvwasi_t* uvwasi; | ||
| }; | ||
|
|
||
| void wasm2c_uvwasi_link(struct w2c_wasi__snapshot__preview1* wasi, |
There was a problem hiding this comment.
Is this an extension to wasi interface? If not, can it be a static func?
| struct w2c_wasi__snapshot__preview1* wasi, | ||
| u32 fd, | ||
| u64 filesize) { | ||
| return uvwasi_fd_filestat_set_size(wasi->uvwasi, fd, filesize); |
There was a problem hiding this comment.
How does this adapter works? Does the wasm file contains external references such as w2c_wasi__snapshot__preview1_fd_filestat_set_size? Then these references are redirected to other functions such as uvwasi_fd_filestat_set_size? Then, when the output .c file is compiled, do you need a wasi library?
| TRAP(OOB); \ | ||
| } while (0); | ||
|
|
||
| static inline void* mem_get_checked_pointer( |
There was a problem hiding this comment.
Does wasi use a linear memory in a .c program? Is it better than raw pointers? Do you have multiple w2c_wasi__snapshot__preview1 structures or is it a global value in the .c implementation of wasi?
|
@zherczeg Thanks for the review. I'll address these shortly I also spoke to @keithw offline and he pointed out that this implementation currently traps on bad parameter rather than returning system call error codes. I will need to address this as well. Moving this to a draft, and will convert back to an active PR once I address this |
Implementation of a uvwasi adapter for wasm2c output
Last discussed in a comment with @sbc100 here a while back
#2395 (comment)
wasm2c can now use uvwasi for the wasi implementation by including
wasm-rt-uvwasi-adapter.c(and of course linking libuv ad libuvwasi)I have added an example of using this adapter in the examples folder.
Full disclosure: While the initial adapter was written by me on an old fork of wasm2c, in a pre-LLM era, I used an LLM to adapt that patch for this PR and the current wasm2c codebase.
LLM Mitigations: I audited every line of the generated code. I fixed several double-fetch bugs, edge cases on assuming null termination and off-by-ones. I also fixed more minor stylistic things to keep the generated code similar to the wasm2c codebase. To the extent possible, I believe my audit has made the code about as secure as it would be if I hand wrote it from scratch.