Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

no_std support for wasmparser #3495

Closed
ethindp opened this issue Oct 31, 2021 · 10 comments
Closed

no_std support for wasmparser #3495

ethindp opened this issue Oct 31, 2021 · 10 comments

Comments

@ethindp
Copy link

ethindp commented Oct 31, 2021

So, I opened this pull request for wasmparser, making it function in no-std environments (since if someone wants to use (say) cranelift-wasm in a no-std environment it wouldn't build at the moment because wasmparser is not no-std and it requires it). I was politely informed that I would need to open a discussion about merging it here, or else my PR would not be merged (even though this PR does not in fact touch wasmtime in any way and is completely unrelated to it). So, this issue is for the discussion for merging that PR. The PR does not modify any behavior of wasmparser or wasmtime, nor does it alter the development flow of (any) new developers (although they will simply have to avoid certain dependencies). However, that shouldn't be a problem, since wasmtime is an incremental wasm parser, and is media-agnostic as to where the wasm comes from.
The PR alters the dependency tree of wasmparser by adding the no-std-compat and an extern crate declaration above all use declarations in lib.rs. Additionally, I added a feature called std to facilitate the importation of libstd, and enabled it by default. Other than that, the code for the library is identical. This nullifies any reasons not to add no-std support to wasmparser, at minimum. CD/CI builds can be satisfied by adding another task to bench/test the wasmparser crate without any default features and with only the deterministic feature but not default/std.
Crates that should be avoided in wasmparsers development are typically any crates that depend on modules within libstd that aren't also in liballoc or libcore. (This is mitigated a bit by emulating mutexes/locks with spin and hashmaps with hashbrown, but things that touch the filesystem, networking, threads, processes, etc., should be avoided.) However, as I said previously I don't foresee any reason why wasmparser would ever need any of this functionality.

@bacongobbler
Copy link
Contributor

bacongobbler commented Nov 5, 2021

This is an outsider's perspective, so take it with a grain of salt.

(although they will simply have to avoid certain dependencies)

I think that's one of the main issues with supporting #![no_std]. It means that the project can never, EVER, rely on the standard library. That's a pretty severe limitation for any project. No println!, no std::string, nothing.

While I can understand the reasoning behind why #![no_std] support would be desirable for your use case (I'm assuming you want to use wasmparser in an environment where libc is unavailable), I can also sympathize with the developers on this one. Because supporting #![no_std] severely limits what libraries your project can rely on for future feature development. And that can put the maintainers in a very difficult position because they have to say "no" to future development unless there's a #![no_std] equivalent. Things like the JIT compiler in wasmtime do not have a #![no_std] equivalent. It is possible other features developed in the future for wasmparser may also have no #![no_std] equivalent.

I think it's more than fair for the maintainers to be cautious about limiting the project to a #![no_std] compile target for their projects. And given that wasmparser is a bytecodealliance project it's also fair to say that the same policy guiding wasmtime should also influence subprojects.

@bjorn3
Copy link
Contributor

bjorn3 commented Nov 5, 2021

No println!

There is no reason why a compiler backend or wasmparser should ever want to print to stdout. If they ever start doing this I would immediately open an issue for it. During development you can use one of the macros from the log crate.

no std::string

#![no_std] doesn't mean no liballoc. std::string is a re-export of alloc::string.

Things like the JIT compiler in wasmtime do not have a #![no_std] equivalent.

Cranelift can easily get #![no_std] support. In fact in the past it used to have it (there is still a feature flag), but it was left bitrotting. Only the final step to mark the jitted code as executable requires OS support, but the embedder is the one responsible for this anyway. It can do the respective OS call or page table manipulation itself.

It is possible other features developed in the future for wasmparser may also have no #![no_std] equivalent.

I can't imagine what kind of feature for a parser would need an OS to function. In addition having a parser directly interact with the OS seems like a recipe for a security issue to me. In any case it is possible to provide an std feature flag to enable libstd usage and then gate this feature behind the feature flag.

@bacongobbler
Copy link
Contributor

bacongobbler commented Nov 5, 2021

Cranelift can easily get #![no_std] support.

Okay. From a beginner's perspective, the document on no_std support claims that the JIT compiler won't work as well due to its dependency on "mmap for allocating JIT code memory" (quoted straight from the docs). If that's incorrect, perhaps the document needs to be updated or clarified (and I would be happy to do so if I was pointed in the right direction).

But the point about it being as simple as "flipping the switch" on #![no_std] still applies (regardless of my ignorance on this subject). println!/std::string arguments aside, the point I'm trying to make is that choosing to support #![no_std] can limit what libraries the project can rely upon, and it's hard to justify the costs associated in maintaining and developing #![no_std]-compatible code compared to relying on std. Those decisions need to be weighed carefully.

https://docs.wasmtime.dev/stability-platform-support.html#what-about-no_std summarizes the situation better than I can.

@bjorn3
Copy link
Contributor

bjorn3 commented Nov 5, 2021

Okay. From a beginner's perspective, the document on no_std support claims that the JIT compiler won't work as well due to its dependency on "mmap for allocating JIT code memory" (quoted straight from the docs). If that's incorrect, perhaps the document needs to be updated or clarified (and I would be happy to do so if I was pointed in the right direction).

Wasmtime and cranelift-jit don't work, but cranelift-jit is small enough that you can easily write your own replacement that uses a different interface as necessary.

https://docs.wasmtime.dev/stability-platform-support.html#what-about-no_std summarizes the situation better than I can.

I partially agree. Wrt to wasmtime I fully agree, but wrt cranelift-codegen the cost is pretty small IMHO. Not that I think I can convince others except maybe (and only maybe) by forking cranelift with #![no_std] support and updating it for an extended period to the latest version to show that it is easy to update.

@ethindp
Copy link
Author

ethindp commented Nov 6, 2021

I think that's one of the main issues with supporting #![no_std]. It means that the project can never, EVER, rely on the standard library. That's a pretty severe limitation for any project. No println!, no std::string, nothing.

As I said in the opening comment of this issue, why would wasmparser ever need anything within libstd that isn't in liballoc or libcore? Why would wasmparser need println!, processes, threads, networking, or OS-specific functionality? This is a parser, not a JIT compiler or runtime environment (and even a JIT compiler can be made in no_std environments, its just harder).

@iximeow
Copy link
Contributor

iximeow commented Nov 6, 2021

why would wasmparser ever need anything within libstd that isn't in liballoc or libcore?

std::error::Error is in fact in std, not a re-export out of libcore or liballoc. as someone that maintains no_std crates this has been a complication for users that want to seamlessly convert to anyhow::Error (because there is a From for that conversion, and this gets used in ?-unwrapping). wasmparser does use that trait (BinaryReaderError) so this is an impl and behavior that would have to be conditional on std. i think this is the most obvious (and generally problematic) case, but there are other features in libstd that are not related to OS-specifics, but are often reached-for tools in Rust.

i don't see that impl changed in the no_std PR; it looks like this won't build unless the dependency tree accidentally includes std? this is exactly what the third point in the Wasmtime no_std policy document is getting at; testing against regressions in no_std builds is not free, and is jarring to find a local patch that passes local checks fails under a particular CI environment.

i think it would help everyone to understand your use case for a no_std wasmparser. build-std ought to get most uses most of the way, are you unable to use a nightly Cargo? changes to get build-std working for all platforms are changes further upstream in Rust, but do have a wider benefit than getting individual crates working in a particular environment.

@bjorn3
Copy link
Contributor

bjorn3 commented Nov 6, 2021

build-std ought to get most uses most of the way, are you unable to use a nightly Cargo?

-Zbuild-std only gives a non-feature-gated libstd on already existing targets. If you create your own target for say a kernel, you need to change all crates using libstd to add the right #![feature].

changes to get build-std working for all platforms are changes further upstream in Rust, but do have a wider benefit than getting individual crates working in a particular environment.

#![no_std] will get a crate working on all targets, including those which can't be upstreamed like custom kernel targets or other custom targets.

@ethindp
Copy link
Author

ethindp commented Dec 12, 2021

Any update on this?

@alexcrichton
Copy link
Member

I realize it's been years since this was originally opened, but in case anyone cc'd on this issue is still interested I've opened #8341 with a proposal to add no_std support for wasmtime which would address this issue as-stated.

@alexcrichton
Copy link
Member

This is done now in bytecodealliance/wasm-tools#1493, so I'm going to close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants