Cranelift 0.63: Breaking Unwind API

[syrusakbary]

In the latest published version of Cranelift (0.63), there has been breaking changes that are not documented anywhere, and with no alternative ways of using them.

• Frame information has been deleted (as part of Refactor unwind generation in Cranelift. #1466)
• Frame Sink doesn't exist any longer (this can be good, as it makes unwind generation lazy, however the next point is crucial if the sink doesn't exist)
• New Frame info structure fields can't be accessed outside of Cranelift. As such, UnwindCode , UnwindInfo or CallFrameInstruction data can't be retrieved outside of Cranelift.

The last point is specially sensitive, as implementors that want to access the unwind information generated by the Cranelift IR right now are unable to do it with the latest version of Cranelift.

error[E0432]: unresolved imports `cranelift_codegen::binemit::FrameUnwindKind`, `cranelift_codegen::binemit::FrameUnwindOffset`, `cranelift_codegen::binemit::FrameUnwindSink`
  --> lib/cranelift-backend/src/unwind.rs:12:38
   |
12 |     use cranelift_codegen::binemit::{FrameUnwindKind, FrameUnwindOffset, FrameUnwindSink, Reloc};
   |                                      ^^^^^^^^^^^^^^^  ^^^^^^^^^^^^^^^^^  ^^^^^^^^^^^^^^^ no `FrameUnwindSink` in `binemit`
   |                                      |                |
   |                                      |                no `FrameUnwindOffset` in `binemit`
   |                                      no `FrameUnwindKind` in `binemit`

error: aborting due to 2 previous errors

[Edit: removed assumptions]

[github-actions]

Subscribe to Label Action

cc @bnjbvr

This issue or pull request has been labeled: "cranelift"

Thus the following users have been cc'd because of the following labels:

• bnjbvr: cranelift

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

[iximeow]

New Frame info structure fields can't be accessed outside of Cranelift. As such, UnwindCode , UnwindInfo or CallFrameInstruction data can't be retrieved outside of Cranelift.

This is true. Were you manually processing frame layout changes? I would expect typical use to be translating them to the appropriate platform unwind information and little else. This is still quite possible through the public Cranelift APIs, you can see an example of this in wasmtime's use to generate .eh_frame tables.

In fact, this change in interface allows straightforward generation of a single eh_frame table with shared CIEs, which before would have required users editing bytes for eh_frame FDE records after emitting them, or walking each function's FrameLayoutChange and translating to the appropriate CFA directives when assembling .eh_frame itself. This made unwind generation for Cranelift users who are not JIT engines fairly tricky - restructuring these APIs is partially to make Cranelift more amenable to non-wasmtime uses. I've looked at the Windows APIs somewhat less and my understanding is that this was less a concern before, but is largely unimpacted by this change either way?

breaking changes that are not documented anywhere,

You've linked to the wasmtime release notes here, not cranelift release notes - I'm not aware of a specific CHANGELOG-style list of changes in Cranelift, but I wouldn't expect Cranelift changes to show up there now or in the future. I would note that Cranelift is not 1.0+, so breaking changes alone are semver-compliant on minor version bumps. They can and do occur, and are typically paired with functionality-preserving changes. If your use-case is not simply generating platform unwind information, that's both surprising and probably deserving of some tweaks to Cranelift interfaces.

I see you edited back a comment suggesting that merging repos might have yielded a Wasmtime-biased change, but I want to address it anyway: Wasmtime has no special access to Cranelift-internal functionality. Published crate versions must build from published crates, so there's no off-release shenanigans here. If unwind information were no longer available in Cranelift public APIs, this would be a showstopper for Wasmtime's usage of that functionality as well.

[syrusakbary]

Thanks for the quick reply.

This is true. Were you manually processing frame layout changes?

Yes, in the case of SystemV, it's already possible to access the gimli struct via the to_fde field, however for the Windows Unwind info it's currently impossible and it will be useful to have access to internal fields.

You've linked to the wasmtime release notes here, not cranelift release notes - I'm not aware of a specific CHANGELOG-style list of changes in Cranelift, but I wouldn't expect Cranelift changes to show up there now or in the future

I think it will be a good practice to start documenting Cranelift changes in a CHANGELOG-style format. As for the Wasmer use case, I think there are more users just interested on the Cranelift IR itself, and it would be useful to have a list of changes (new features & breaking changes) for it, so we can keep up to date with the progress.

Right now our process to keep up to date with the progress in Cranelift is very tedious: we have to scrape all commits and investigate each of the changes in a daily basis, which is really non-ideal.

Published crate versions must build from published crates, so there's no off-release shenanigans here. If unwind information were no longer available in Cranelift public APIs, this would be a showstopper for Wasmtime's usage of that functionality as well.

Yeah, you are completely right. However, I feel some of the breaking changes in Cranelift have been taken with only wasmtime in mind and no input from external users.
We are very proud to be a big driver of adoption for Cranelift, and would love be taken in consideration (just a bit) when this kind of breaking changes happen, if possible.

I didn't mean to flame the discussion, so that's why I edited the comment :)

[tschneidereit]

However, I feel some of the breaking changes in Cranelift have been taken with only wasmtime in mind and no input from external users.

I will repeat what I said in that tweet: even if we wanted to, which we don't, we couldn't possibly only take Wasmtime into consideration when making changes to Cranelift, because we have other projects that are very high priority, with Firefox being one of them.

As you can no doubt tell, Cranelift is undergoing significant changes right now. I appreciate that those cause work for downstream projects. Given that we're very much not making any API (or behavior) stability guarantees yet, I don't think us putting in the extra work of breaking out documentation of larger changes into a changelog would make a meaningful difference to the amount of work required to keep up, either.

One way to avoid that churn is to wait it out and stick to a specific version of Cranelift until things have settled down a bit.

[iximeow]

however for the Windows Unwind info it's currently impossible

You ought to be able to use winx64::UnwindInfo (in UnwindInfo::WindowsX64)'s emit_size and emit functions (see here) to produce Windows x86_64 SEH information. You'll still have to build the appropriate RUNTIME_FUNCTION struct tables and inform Windows of them (or stick 'em in a .pdata section for an on-disk PE), but that shouldn't require looking at implementation details of Cranelift I'd hope.

I took a quick look at what I think your cranelift-backend rustc output references, but I don't see an unwind.rs in there. (and it's clif-backend not cranelift-backend, different branch maybe?) If winx64::UnwindInfo's emit is insufficient for your use case, can you please expand a little on what processing you're doing? That would help a lot in ensuring we can design a public interface that is flexible for different uses and can stay more stable.

[syrusakbary]

I don't think us putting in the extra work of breaking out documentation of larger changes into a changelog would make a meaningful difference to the amount of work required to keep up, either

It makes me a bit sad to hear that, specially because I believe having a CHANGELOG will benefit the Cranelift project as a whole.

I took a quick look at what I think your cranelift-backend rustc output references, but I don't see an unwind.rs in there

We are going some code changes for unwinding which are not yet published. Thanks for taking a look anyway!

That would help a lot in ensuring we can design a public interface that is flexible for different uses and can stay more stable

Thanks! ❤️ I will ping you once is ready so we can discuss the API from a practical perspective

[jyn514]

I'm not aware of a specific CHANGELOG-style list of changes in Cranelift, but I wouldn't expect Cranelift changes to show up there now or in the future. I would note that Cranelift is not 1.0+, so breaking changes alone are semver-compliant on minor version bumps.

One way to avoid that churn is to wait it out and stick to a specific version of Cranelift until things have settled down a bit.

Cranelift has been changing continuously at least since I started using it in July 2019. I understand that minor version bumps are semver-compliant, but since there is no plan (that I know of) to go 1.0 any time soon, waiting for 1.0 isn't really feasible. Also, I doubt Cranelift would be able to document every breaking change since 0.1 even if they did release 1.0, so we'd still have to deal with it at some point.

I don't think us putting in the extra work of breaking out documentation of larger changes into a changelog would make a meaningful difference to the amount of work required to keep up, either.

It makes me a bit sad to hear that, specially because I believe having a CHANGELOG will benefit the Cranelift project as a whole.

I agree. The difficult part of using cranelift is not the rapid change but the lack of documentation (both for the changes and for cranelift as a whole). Take for example 0.62, which changed cranelift_module::Module::declare_data to take a new tls argument. There is no indication of what tls stands for or what argument should be passed to keep the previous behavior - the only way I knew I should pass false is because I'd been following bytecodealliance/cranelift#1174 (which is now very difficult to find since it's in a different repo).

I think it will be a good practice to start documenting Cranelift changes in a CHANGELOG-style format. As for the Wasmer use case, I think there are more users just interested on the Cranelift IR itself, and it would be useful to have a list of changes (new features & breaking changes) for it, so we can keep up to date with the progress.

+1

[tschneidereit]

To clarify, I do agree that Cranelift should eventually get a better story for documenting breaking changes. More importantly, it should get a somewhat more stable API and reduce the amount of breaking changes. That should probably happen before a 1.0 release, but we don't have firm plans for either at this time.

@jyn514, in the meantime, I'm curious to hear more about how you think a change log would help. The tls example seems like it'd be addressed better by improving the API docs, and I'm not entirely sure what a change log would've contained that'd have helped here.

The best I can come up with is something like "Add TLS (thread local storage) support for ELF targets to Cranelift", as the PR says, but would that have helped in figuring out what to pass for your usage of declare_data? ISTM that anything beyond that should really be API docs, but perhaps there's a way to address this that's reasonably low-effort and better than API docs?

[jyn514]

The best I can come up with is something like "Add TLS (thread local storage) support for ELF targets to Cranelift", as the PR says, but would that have helped in figuring out what to pass for your usage of declare_data?

Absolutely. Now that I know TLS is thread local storage, I can search for it, find the PR, find resources on Wikipedia. tls alone doesn't give me enough information to do any of that.

In general I find Cranelift assumes a lot of background knowledge, which would be fine except that it doesn't give you any links where you can learn more. Take all the ebb docs: the first result for ebb on Google is for the movement of the sea:

I'm not asking for in depth explanations of all these concepts, just the bare minimum so that I can find out more on my own.

[philipc]

Try to restrict your search to the domain you are interested in: "compiler tls" or "compiler ebb". For me, this gives the correct wikipedia article as the top result.

[bjorn3]

I think there is little value in keeping this issue open.

[syrusakbary]

We were able to adapt to newer APIs. I think it will still be useful to implement the last suggestion, but we have no exact need for it now.

New Frame info structure fields can't be accessed outside of Cranelift. As such, UnwindCode , UnwindInfo or CallFrameInstruction data can't be retrieved outside of Cranelift.

Closing the issue for now