Upstream wasm-wave instances in wasmtime#8872
Conversation
|
I'd be interested in contributing to this, but I'll have to check with my employer first since it is under a separate repo. In the meantime, you can use the |
|
Thanks! Yes, the plan is to merge this by enabling forbid-unsafe logos over in wasm tools, however I just haven't taken the time to get back to this because it fell off my radar. I don't think we should write a new lexer. |
|
Once bytecodealliance/wasm-tools#1874 lands and is in a wasm-tools release, I'll update this PR to use it. |
7f37566 to
9b718b1
Compare
|
This PR is now a single commit on top of #9601 update and now, 9601 has landed, so its a single commit on main |
9b718b1 to
076a6f6
Compare
|
@alexcrichton Can you please double-check my safe-to-deploy audit of https://diff.rs/beef/0.5.2/0.5.2 as part of reviewing this PR? |
alexcrichton
left a comment
There was a problem hiding this comment.
Looks good to me 👍
To help with version alignment of the wasm_wave crate, how about:
- Reexporting the
wasm_wavecrate somewhere (e.g.wasmtime::component::wasm_wave) - Adding some top-level conveniences (e.g.
Val::{from_wave,to_wave})
and the docs for those helper methods could point to the wasm_wave reexport and additionally to the wave docs?
|
Suggestions done. Thanks! |
This PR is upstreaming instances of the
wasm-wavecrate's traits for wasmtime values. However, we won't be able to land this PR in wasmtime untilwasm-waveis written entirely in safe rust, which it presently is not, because it useslogosto derive its lexer here: https://github.com/bytecodealliance/wasm-tools/blob/main/crates/wasm-wave/src/lex.rs#L11I hadn't looked into how
logosworks prior to making this PR, but when I got to thecargo vetfor thelogoscrate, I discovered that logos derive macro generates significant amounts ofunsaferust. While I don't have any evidencelogosis unsound, but it seems too risky to me to accept it into our audits, since I also have no reasonable way of ruling out thatlogosis unsound. In the particular case ofwasm-wave, we should expect that strings accepted by the crate come from untrusted sources, so we want to be very rigorous on how they are parsed into values.The particular bits of Lann's root wasm-wave repo this PR is upstreaming are found in https://github.com/lann/wasm-wave/tree/main/src/wasmtime. I was able to eliminate the
struct FuncTypedefinition in that code by definingcomponent::Func::ty5918a4b tso that theComponentFuncimpl of WasmType is usable.