Repository navigation
cranelift-codegen no longer builds with no_std #1158
Copy link
Copy link
Closed
Labels
bugIncorrect behavior in the current implementation that needs fixingIncorrect behavior in the current implementation that needs fixingcraneliftIssues related to the Cranelift code generatorIssues related to the Cranelift code generator
Description
I've tracked down a few things that are preventing this from happening.
packed_structrequires std by default- There are a few uses of std in
cranelift-codegenandcranelift-codegen-sharedthat can be replaced with core or alloc. thiserrorrequires std, because the Error trait doesn't exist in core or alloc.
The first two are trivial to fix, and I'll create a PR with those changes in a second.
However, there are 11 uses of the Display and Into<result::CodegenError> traits that prevent me from placing thiserror behind a feature gate, and I'm not sure what to do about them.
Here are the 11 things that rely on those traits, as reported by rustc (click to expand)
$ cargo build --no-default-features --features core
Compiling cranelift-codegen v0.51.0 (/home/icydefiance/Documents/code/cranelift/cranelift-codegen)
error[E0599]: no method named `to_string` found for type `verifier::VerifierError` in the current scope
--> cranelift-codegen/src/print_errors.rs:218:36
|
218 | writeln!(w, "; error: {}", err.to_string())?;
| ^^^^^^^^^ method not found in `verifier::VerifierError`
|
::: cranelift-codegen/src/verifier/mod.rs:138:1
|
138 | pub struct VerifierError {
| ------------------------ method `to_string` not found for this
|
= help: items from traits can only be used if the trait is implemented and in scope
= note: the following trait defines an item `to_string`, perhaps you need to implement it:
candidate bytecodealliance/cranelift#1: `alloc::string::ToString`
error[E0599]: no method named `to_string` found for type `result::CodegenError` in the current scope
--> cranelift-codegen/src/print_errors.rs:227:13
|
227 | err.to_string()
| ^^^^^^^^^ method not found in `result::CodegenError`
|
::: cranelift-codegen/src/result.rs:12:1
|
12 | pub enum CodegenError {
| --------------------- method `to_string` not found for this
|
= help: items from traits can only be used if the trait is implemented and in scope
= note: the following trait defines an item `to_string`, perhaps you need to implement it:
candidate bytecodealliance/cranelift#1: `alloc::string::ToString`
error[E0277]: `verifier::VerifierError` doesn't implement `core::fmt::Display`
--> cranelift-codegen/src/verifier/mod.rs:235:33
|
235 | writeln!(f, "- {}", err)?;
| ^^^ `verifier::VerifierError` cannot be formatted with the default formatter
|
= help: the trait `core::fmt::Display` is not implemented for `verifier::VerifierError`
= note: in format strings you may be able to use `{:?}` (or {:#?} for pretty-print) instead
= note: required because of the requirements on the impl of `core::fmt::Display` for `&verifier::VerifierError`
= note: required by `core::fmt::Display::fmt`
error[E0277]: the trait bound `result::CodegenError: core::convert::From<verifier::VerifierErrors>` is not satisfied
--> cranelift-codegen/src/context.rs:224:30
|
224 | self.verify(fisa)?;
| ^ the trait `core::convert::From<verifier::VerifierErrors>` is not implemented for `result::CodegenError`
|
= note: required by `core::convert::From::from`
error[E0277]: the trait bound `result::CodegenError: core::convert::From<verifier::VerifierErrors>` is not satisfied
--> cranelift-codegen/src/context.rs:244:39
|
244 | self.verify_locations(isa)?;
| ^ the trait `core::convert::From<verifier::VerifierErrors>` is not implemented for `result::CodegenError`
|
= note: required by `core::convert::From::from`
error[E0277]: the trait bound `result::CodegenError: core::convert::From<verifier::VerifierErrors>` is not satisfied
--> cranelift-codegen/src/regalloc/context.rs:111:28
|
111 | return Err(errors.into());
| ^^^^^^^^^^^^^ the trait `core::convert::From<verifier::VerifierErrors>` is not implemented for `result::CodegenError`
|
= note: required because of the requirements on the impl of `core::convert::Into<result::CodegenError>` for `verifier::VerifierErrors`
error[E0277]: the trait bound `result::CodegenError: core::convert::From<verifier::VerifierErrors>` is not satisfied
--> cranelift-codegen/src/regalloc/context.rs:139:28
|
139 | return Err(errors.into());
| ^^^^^^^^^^^^^ the trait `core::convert::From<verifier::VerifierErrors>` is not implemented for `result::CodegenError`
|
= note: required because of the requirements on the impl of `core::convert::Into<result::CodegenError>` for `verifier::VerifierErrors`
error[E0277]: the trait bound `result::CodegenError: core::convert::From<verifier::VerifierErrors>` is not satisfied
--> cranelift-codegen/src/regalloc/context.rs:168:28
|
168 | return Err(errors.into());
| ^^^^^^^^^^^^^ the trait `core::convert::From<verifier::VerifierErrors>` is not implemented for `result::CodegenError`
|
= note: required because of the requirements on the impl of `core::convert::Into<result::CodegenError>` for `verifier::VerifierErrors`
error[E0277]: the trait bound `result::CodegenError: core::convert::From<verifier::VerifierErrors>` is not satisfied
--> cranelift-codegen/src/regalloc/context.rs:196:28
|
196 | return Err(errors.into());
| ^^^^^^^^^^^^^ the trait `core::convert::From<verifier::VerifierErrors>` is not implemented for `result::CodegenError`
|
= note: required because of the requirements on the impl of `core::convert::Into<result::CodegenError>` for `verifier::VerifierErrors`
error[E0277]: the trait bound `result::CodegenError: core::convert::From<verifier::VerifierErrors>` is not satisfied
--> cranelift-codegen/src/regalloc/context.rs:239:28
|
239 | return Err(errors.into());
| ^^^^^^^^^^^^^ the trait `core::convert::From<verifier::VerifierErrors>` is not implemented for `result::CodegenError`
|
= note: required because of the requirements on the impl of `core::convert::Into<result::CodegenError>` for `verifier::VerifierErrors`
error[E0277]: the trait bound `result::CodegenError: core::convert::From<verifier::VerifierErrors>` is not satisfied
--> cranelift-codegen/src/regalloc/context.rs:248:17
|
248 | Err(errors.into())
| ^^^^^^^^^^^^^ the trait `core::convert::From<verifier::VerifierErrors>` is not implemented for `result::CodegenError`
|
= note: required because of the requirements on the impl of `core::convert::Into<result::CodegenError>` for `verifier::VerifierErrors`
error: aborting due to 11 previous errors
Some errors have detailed explanations: E0277, E0599.
For more information about an error, try `rustc --explain E0277`.
error: could not compile `cranelift-codegen`.
To learn more, run the command again with --verbose.
Additional info:
- The author of
thiserrordoesn't want to add a feature-gate for just the Error trait, but it's possible to provide another implementation of Display (and presumably Into). Option toimpl Errorunder feature flag dtolnay/thiserror#43 - There was some discussion of moving Error to alloc, and a PR was even created to do so, but it was closed because RFC 2504 will add backtrace functionality that requires std.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
bugIncorrect behavior in the current implementation that needs fixingIncorrect behavior in the current implementation that needs fixingcraneliftIssues related to the Cranelift code generatorIssues related to the Cranelift code generator
Activity
IcyDefiance commentedon Dec 1, 2019
In addition to that PR, I started addressing point 3 on the no_std-thiserror branch on my fork. (diff) Building that branch will give you the same errors that I pasted in the above post.
IcyDefiance commentedon Feb 21, 2020
I got cranelift-wasm and its dependencies (including cranelift-codegen) to compile with no_std in my fork. To do so, I had to fork thiserror and v0.48.2 of wasmparser (there have been breaking changes since then).
I also copied the
extern crate alloc as std;line from cranelift-wasm to cranelift-codegen, and eliminated all direct use of alloc there. (It also reduced the changes I had to make to thiserror.) Unfortunately I don't think there's a way to do the same thing with core.Considering the current lack of interest in supporting no_std, as expressed in my PR, I won't bother making any new PRs. However, they're good enough for the project I'm playing with, so they may also be useful to someone else, and they may be informative if there is any future interest in supporting no_std.
iximeow commentedon Feb 21, 2020
I happen to know of at least one project where building cranelift-wasm as no_std is really helpful, so I just want to say I appreciate this work.
[-]cratelift-codegen no longer builds with no_std[/-][+]cranelift-codegen no longer builds with no_std[/+]bnjbvr commentedon Feb 21, 2020
Sorry we didn't get back to you earlier, things have been a bit hectic lately, and no-std support is not checked in automation, so it's definitely something we may break a lot. The main issue here was finding a proper core reviewer for your work; if there's someone who could chime in and review such changes, it would be definitely helpful. And then, we could reopen your PR here and in other places and take your changes, if you were so inclined.
bnjbvr commentedon Feb 21, 2020
(Of course, that also depends on the
thiserrorchanges being properly integrated upstream, which might be another hill to climb.)IcyDefiance commentedon Feb 21, 2020
By "lack of interest" I was referring to Alex's comment in my PR, but if that's not the case then I can make some new PRs later today. I'll start with thiserror and wasmparser and see how it goes.
bnjbvr commentedon Feb 24, 2020
It's nice to be able to experiment on a smaller crate like
wasmparser(since we're the same authors), to have a precise idea of what the code would look like in the end, what's the best strategy to implement this (useno-std-compator something else). Once that's done, we agreed we could also port the whole Cranelift crate to no-std support, using the methods learned during the port ofwasmparser. Hope this makes sense!yurydelendik commentedon Feb 24, 2020
Opened https://github.com/bytecodealliance/wasmparser/issues/198 . I'm proposing not depend on structures that require memory allocations (this means no hashbrown )
These ideas might not be applied here
Lucky4Luuk commentedon Jun 5, 2024
Is this still being worked on? What are the current issues blocking no_std support?
alexcrichton commentedon Jun 5, 2024
I'm at least not personally aware of this being worked on, but no blocker other than "just needs some elbow grease". If you'd be up for it PRs would be most welcome!
marceline-cramer commentedon Jul 18, 2024
I made a pretty half-assed attempt at this but managed to dig up some useful info: a major blocker to getting this to work is getting several dependencies involved in
std::io::Write-ing to objects to work onno_std. Here's a non-exhaustive list (i've been doing this for hours and i am exhausted enough):leb128crate used bywasm-encoderhas a hard dependency onstd::iotermcolorcrate has a hard dependency onstd::ioindexmap/stdis enabled bywasmparser/stdis enabled bywasmprinter(which has no features that could be used to disable the dep feature)gimli/writehas a hard dependency onstd::ioobject/writehas a hard dependency onstd::ioAll of those issues come from the
wasmtime-environcrate with thecompilefeature, which as far as I can tell, is pretty necessary to the functioning ofwasmtime-cranelift. I don't know how necessary that is, but if it is absolutely crucial to havecompileworking, those crates need to be either factored out, guarded behindstd, or in the worst case, modified upstream to not usestd::io. Much bureaucracy ensues.The really obnoxious part about removing
std::iofrom those crates is that there is no equivalentioAPI incoreoralloc, making ergonomicno_stda great endeavor per project.Aside from
wasmtime-environ,wasmtime-codegen/unwindhas a similar breaking dependency ongimli/write, which usesstd::ioto write objects. Disabling theunwindfeature also causes a cascade of missing API items in thewasmtime-codegencrate ANDwasmtime-cranelift. Today, I did manage to getwasmtime-codegencompiling locally inno_stdbut because I hadn't gotten theunwindfeature to work I had to give up.thread_localalso breaks inwasmtime-codegenbecause that'sstd-only. I managed to take care of that with nightly#[thread_local]for now but obviously that's not optimal. More research is needed.OnceLockcan be replaced withonce_cell::OnceCelltrivially, although that uses the scary-soundingcritical-sectionfeature. More research is needed.I haven't tried to get
thiserrorworking yet but I will.I would really appreciate some guidance for what to work towards next. This is clearly going to take a LOT of work to get
wasmtime-craneliftworking onno_stdconsistently on all platforms but I'm pretty invested in it. Also, if I've made any mistakes in my probing (I'm a newcomer to the codebase), please let me know.bjorn3 commentedon Jul 18, 2024
#[thread_local]is generally unavailable on no_std targets too.alexcrichton commentedon Jul 18, 2024
My recommendation would be to first start off by gating "easy" things behind on-by-default Cargo features. For example
wasmprinterdoesn't need to be included in no_std builds (northread_localusage ortermcolor). Basically anything that's optional functionality can be hard-disabled at compile time and that can greatly help to prune the dependency tree of crates that don't support no_std.After that next bit that I would recommend is to do this incrementally. The main crate in question I believe is
cranelift-codegenbut there are other crates as well that it depends on. I'd recommend starting at the leaves (e.g.cranelift-entity) and adding a "ratchet" to add support forno_stdto ensure they don't regress. For example on CI you'd add lines around here where that ensures that crates build on a no_std target. Eventually as you work your way up tocranelift-codegenyou'll already have added support to a number of other crates and handled a number of these issues.At that point I think it might be worth reevaluating taking stock of what's remaining. I think it would be useful to classify dependencies as you've done, but with a refreshed list at that point. Some of these are going to be somewhat hard like
gimli/writeandobject/writebut we might be able to work with upstream to have new crate versions that support no_std needs, but that also sort of depends on what exactly Cranelift needs. That's why I'd recommend doing everything exceptcranelift-codegenfirst if possible so we can get the most precise picture of what Cranelift needs from its dependencies with no_std.marceline-cramer commentedon Jul 21, 2024
I'm having a hard time with removing
thread_localusage fromcranelift/codegen/src/timing.rs. What alternative tothread_localstatics should I use withno_std, if any? As far as I can tell, thetimingmodule and those statics can't be configured out. One idea I have is to just configure out all thetimingusage with API-compatible no-op stub functions onno_std, but I don't know enough about the codebase to understand the consequences of something like that.I should also point out that the
set_thread_profiler()function is not used anywhere in the codebase.marceline-cramer commentedon Jul 21, 2024
For now, I just put no-op stubs in the
timingAPI when thetimingfeature is not enabled and made thetimingfeature enable thestdfeature automatically.thread_local! {}is still in use as long as thetimingfeature is enabled.Right now, my development approach is to hack on all of these intersecting API changes and changes to the dependency trees on a fork, which I have set up to be running the full CI PR test suite: https://github.com/marceline-cramer/wasmtime/pull/1 Once that branch on my fork is to a place I'm happy with and all the pieces fit together practically as a rough sketch, I can start decomposing it into finer-grained, more polished things to implement as upstream-facing PRs. I did implement the "ratcheting" approach to get down to
cranelift-codegenas quickly as I could and discovered all but one of its workspace dependencies already work fine inno_std. The one exception wascranelift-control, which to compile tono_stdI slapped in a Git dep for a pending PR onarbitrarywithno_stdsupport: rust-fuzz/arbitrary#177I added a field to
CodegenOptionsincranelift-isleto emitno_std-compatible code by adding a short prelude to generated source files, because most CI compiles ISLE withstdbutcranelift-codegendoes not. If that's an acceptable change for upstream, I'd be happy to make a little PR to get that moving along.The reason I had gotten as far with
cranelift-codegenas to be hacking onthread_localis because getting it to compile to ano_stdtarget turned out to be pretty easy. I mainly just swapped out all of the uses ofrustc-hashwith a combination ofhashbrownandahashand replaced any lingering uses ofstdwithcoreandalloc. I also replaced any use of floating-point arithmetic in the instrinics code with thecore_mathcrate, which just implementsstd-compatible arithmetic operators oncorefloats usinglibm. There may be some security concerns with regards to the use ofahashin ano_stdenvironment but I have decided that taking a deeper look into that is Tomorrow Marceline's problem.The main kicker with
cranelift-codegenright now is that so far, I've only gottenall-archandtrace-logfeatures, with no others, to compile, and only in nightly. The reason it has to be in nightly for now is because there are some cases ofimpl std::error::Errorthat I'm too afraid tocfgout, so for now I'm using nightly andcore::error::Erroruntil I can figure out what to do with potentialno_stdAPI users ofcranelift-codegen. I figure that the already-discussed use ofthiserroronno_stdwould force us to use nightly for now anyways.That's about where I am now. The CI mostly passes, although Cranelift's unit tests fail with slightly different code output compared to the expected code. I can chalk that up to either an impatient use of
cargo updatethat may have minorly changed some semantics of dependencies involved in codegen, or something related to the switching ofrustc-hashtohashbrown. Both are easy to test.The main issue now is that, by default,
cranelift-codegenis configured to havestd,unwind,trace-logenabled.trace-logworks inno_stdindependently ofstd, butunwindis still blocked by that peskygimli/writedep and its use ofstd::io. I can take a deeper look into how exactly that is used and what an alternative to that either in Cranelift or upstream Gimli might look like when I have more time and can afford to lock in on specific issues like that. I'm becoming a lot more comfortable navigating the codebase but there's still a lot of cognitive overhead and even swapping out dependencies takes a lot of attention.bjorn3 commentedon Jul 21, 2024
It is used by cg_clif to include performance timings collected by Cranelift in rustc self profile profiles. Having thr timing module be a nop for no_std builds is fine.
bjorn3 commentedon Jul 21, 2024
Unconditionally using core instead of std should work fine. In std builds, core is still available.
alexcrichton commentedon Jul 22, 2024
Sounds good! I'd recommend gating
set_thread_profilerbehind this Cargo feature entirely (and having the feature enabled by default probably too). That would avoid the confusion of callingset_thread_profilerand having it do nothing.I'd probably recommend adding a
fuzzfeature or similar to avoid needing to update thearbitrarycrate (although updating that is definitely ok too). Basically gating this functionality on a Cargo feature is fine. (this Cargo feature may even be able to be off-by-default too)This sounds generally fine and possible for us to land. When making PRs if you wouldn't mind leaving
cranelift-codegen's no_std support for a single standalone PR that'll make this easiest to review and discuss.FWIW I think it's definitely ok to not get the entire crate and all its features compiling with no_std. Only getting a subset working (albeit we still need a useful subset, but the one you have is useful) is more than ok. Can always work to improve the subset supported later on.
I'm assuming that you're probably going to add a
stdfeature to thecranelift-codegencrate no matter what, and would it be possible to gate these impls on thestdfeature? That is, "just" omit them for a no_std build?In the spirit of ratcheting and progress over time I'll reiterate it's completely ok to get only some features working and not others. In this workspace most dependencies disable default features and then individual crates will enable features in addition to specifying
workspace = true. That would enable a state wherecranelift-codegenitself builds with no_std butwasmtime-craneliftdoesn't work just yet. That'd help figure out all the dependencies as we go along and get to a working state at least for one crate while the blockers of the next crate are tackled.One example here is that the
unwindfeature is only used in Wasmtime forConfig::native_unwind_infowhich would be easy to gate behind a Cargo feature. In that sense there's no need to unconditionally enableunwindin Cranelift given possible future changes.If your target is
wasmtime-craneliftthe bigger problem is probably going to beobject::writeas that's pretty intrinsically depended on. It's probably best to cross that bridge when we get there though and it's the "only" remaining issue. If your target is justcranelift-codegenonno_stdthen you can disregard this last part from me :)marceline-cramer commentedon Jul 23, 2024
It would work fine if not for the
impl Length for Vecin the codegen. In no_std, that would have to be replaced with thealloccrate, and no_std or not, thealloccrate has to be manually declaredextern crate alloc;.marceline-cramer commentedon Jul 23, 2024
Sounds great, I'll remember that when I open the
cranelift-codegenno_stdPR.Not including the
arbitrarycrate breaksControlPlane::arbitrary()andControlPlane::get_arbitrary(), so I've made it on by default so that none of the dependencies need to manually enable it. We can discuss that more in thecranelift-controlPR I'm about to open. :)Yeah, I suppose that is definitely the easiest way of going about it. I would definitely be glad to have just stripped-down
cranelift-codegenworking inno_stdupstream for now.I was not aware that the dependency on
unwindwas so light, glad to hear! Yes, I am most interested in using the entirety of Wasmtime JIT functionality itself in ano_stdenvironment. Once I can get the lower-level features working though I'll see what I can get done with theobjectcrate and I'll come back here to make the rest of the push. :)the-ssd commentedon Jan 24, 2026
codegen still doesn't build with no_std, so maybe this should be reopened