wasm-tools
wasm-tools copied to clipboard
Add no-std support to wasmparser
Thanks for the PR, but this library doesn't support #![no_std] at this time for the same reasons as Wasmtime, so to add something like this would warrant discussion and agreement for Wasmtime first.
@alexcrichton Why would whether this supports no_std or not be dependent on what wasmtime supports? All this does is parse wasm -- it doesn't actually execute or JIT it, and it (by default) includes the standard library. The change was a trivial one. Merging this functionality won't affect wasmtime in any way, and the same patterns that've been used thus far can continue to be used in this library since I doubt you'll need threading or anything like that if your just parsing Wasm binaries and letting the end-developer handle the structures generated from them. If wasmparser -- which is what changed -- won't ever depend on things that you'll only find in a C library, which appears to be the case since everything that the library does is portable, I'm quite confused on the refusal to merge this PR.
Well in some sense I'm one of the primary maintainers of this crate and I'm also the one who wrote up the policy for wasmtime of not supporting #![no_std]. In another sense one of this crate's main purposes is to serve Wasmtime and if no_std makes development on Wasmtime more difficult then that applies to this crate as well and we don't want that.
If you'd like to discuss the no_std policy I'd recommend opening a specific issue on Wasmtime than continuing here in a PR on a different repo.
@alexcrichton That's the thing: this PR doesn't actually do that. It will not make wasmtime development harder or anything. So long as you don't go importing (for example) std::thread::* in wasmparser (which you shouldn't because you shouldn't need any networking/threading/process handling functions in a Wasm parser) you shouldn't need to do anything. Even std::sync::* will work fine (including mutexes/spinlocks/reader-writer locks). Wasmtime shouldn't need to do anything on its end to use this PR.
Sorry all I can recommend is reiterating my above point, this isn't the place to discuss the bytecodealliance position on #![no_std], but rather that should happen as an issue on Wasmtime or an RFC even.
For me a no_std wasmparser would also be nice since I require Wasm compilation support without WASI.
So I gave it a try today myself and have to conclude that wasmparser being a crate in a workspace makes this task more difficult since other workspace members fiddle with Cargo features of wasmparser itself.
You can inspect this by adding --verbose to cargo build commands.
This PR also has some issues, for example the use std::error::Error is invalid in no_std contexts which is undiscovered since this PR does not fix the aforementioned Cargo issues with feature resolution in workspaces.
tldr; std feature is always included in wasmparser builds due to the workspace.
To work around this you either have to convert the entire workspace to support no_std or move wasmparser into its own crate or at least exclude it from the workspace.
At a minimum it is enough to support no_std for wasmparser-dump crate since it is directly depended on (dev-dependency) by wasmparser itself. However, I am not sure a no_std version of wasmparser-dump is actually useful.
@ethindp FYI I published a no_std compatible wasmparser crate here: https://crates.io/crates/wasmparser-nostd
It also switches out Hash{Map,Set} with BTree{Map,Set} internally so that the implementation is not attackable in no_std environment. Enable no_std by disabling default features, e.g. cargo build --no-default-features.
I think we can close this PR.
- It is outdated.
- We already have an open issue that informs people about
no_stdsupport inwasmparseret. al. - The issue contains a link to the actual
wasmparser-nostdfork that I maintain since a while.
Sounds good.