pchickey opened PR #8872 from bytecodealliance:pch/upstream_wave
to bytecodealliance:main
:
<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
pchickey updated PR #8872.
pchickey updated PR #8872.
pchickey edited PR #8872:
This PR is upstreaming instances of the
wasm-wave
crate's traits for wasmtime values. However, we won't be able to land this PR in wasmtime untilwasm-wave
is written entirely in safe rust.This PR is blocked on the cargo-vet: I hadn't looked into how
logos
works prior to making this PR, but I have discovered that logos derive macro generates significant amounts ofunsafe
rust. While I don't have any evidencelogos
is unsound, but it seems too risky to me to accept it into our audits, since I also have no reasonable way of ruling out thatlogos
is 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 FuncType
definition in that code by definingcomponent::Func::ty
5918a4b tso that theComponentFunc
impl of WasmType is usable.<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
pchickey edited PR #8872:
This PR is upstreaming instances of the
wasm-wave
crate's traits for wasmtime values. However, we won't be able to land this PR in wasmtime untilwasm-wave
is written entirely in safe rust, which it presently is not, because it useslogos
to derive its lexer here: https://github.com/bytecodealliance/wasm-tools/blob/main/crates/wasm-wave/src/lex.rs#L11This PR is blocked on the cargo-vet: I hadn't looked into how
logos
works prior to making this PR, but I have discovered that logos derive macro generates significant amounts ofunsafe
rust. While I don't have any evidencelogos
is unsound, but it seems too risky to me to accept it into our audits, since I also have no reasonable way of ruling out thatlogos
is 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 FuncType
definition in that code by definingcomponent::Func::ty
5918a4b tso that theComponentFunc
impl of WasmType is usable.<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
pchickey edited PR #8872:
This PR is upstreaming instances of the
wasm-wave
crate's traits for wasmtime values. However, we won't be able to land this PR in wasmtime untilwasm-wave
is written entirely in safe rust, which it presently is not, because it useslogos
to derive its lexer here: https://github.com/bytecodealliance/wasm-tools/blob/main/crates/wasm-wave/src/lex.rs#L11I hadn't looked into how
logos
works prior to making this PR, but when I got to thecargo vet
for thelogos
crate, I discovered that logos derive macro generates significant amounts ofunsafe
rust. While I don't have any evidencelogos
is unsound, but it seems too risky to me to accept it into our audits, since I also have no reasonable way of ruling out thatlogos
is 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 FuncType
definition in that code by definingcomponent::Func::ty
5918a4b tso that theComponentFunc
impl of WasmType is usable.<!--
Please make sure you include the following information:
If this work has been discussed elsewhere, please include a link to that
conversation. If it was discussed in an issue, just mention "issue #...".Explain why this change is needed. If the details are in an issue already,
this can be brief.Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.htmlPlease ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->
Kmeakin commented on PR #8872:
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
forbid-unsafe
feature from Logos: https://logos.maciej.codes/unsafe.html
pchickey commented on PR #8872:
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.
pchickey commented on PR #8872:
Once https://github.com/bytecodealliance/wasm-tools/pull/1874 lands and is in a wasm-tools release, I'll update this PR to use it.
Last updated: Oct 23 2024 at 20:03 UTC