Stream: git-wasmtime

Topic: wasmtime / Issue #1181 Improve support for bulk function ...

view this post on Zulip Wasmtime GitHub notifications bot (Apr 01 2020 at 22:15):

peterhuene commented on Issue #1181:

Update on this issue: I'm currently working on this again and am refactoring both dwarf and windows unwind information so that it will be much easier to generate single .eh_frame sections from multiple functions.

I have updated cranelift-codegen and the unit tests and CLIF file tests are passing again (I've also added additional tests for dwarf and added additional call frame instructions to better account for a few things, such as a future FPO implementation and epilogues not describing the CSR restores).

I'm now updating the dependent crates, including Wasmtime. We'll now be generating an .eh_frame per module rather than per function.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 04 2020 at 20:18):

peterhuene edited a comment on Issue #1181:

Update on this issue: I'm currently working on this again and am refactoring both dwarf and windows unwind information so that it will be much easier to generate single .eh_frame sections from multiple functions.

I have updated cranelift-codegen and the unit tests and CLIF file tests are passing again (I've also added additional tests for dwarf and added additional call frame instructions to better account for a few things, such as a future FPO implementation and epilogues not describing the CSR restores).

I'm now updating the dependent crates, including Wasmtime. We'll now be generating an .eh_frame per module rather than per function; it should save space on not having so many CIEs.

view this post on Zulip Wasmtime GitHub notifications bot (Apr 16 2020 at 20:34):

peterhuene closed Issue #1181:

I'd like FrameUnwindSink to allow collecting records for multiple functions into one sink. Currently, TargetIsa::emit_unwind_info is designed around emitting one full record (a whole .eh_frame or the UnwindInfo part of a RUNTIME_FUNCTION) per function, which is awkward to use in the face of multiple functions. In squaring away this interface with bytecodealliance/cranelift#902, I've realized that to generate good unwind information through cranelift-module, we would have to do a lot of legwork already done by gimli to join all distinct .eh_frame, so I'd rather just build the right structures in the first place. As @philipc commented the existing interface isn't entirely how gimli is intended to be used, either.

Functionally, I want isa::x86::fde::emit_fde to be designed around adding to a FrameTable provided by sink, rather than constructing and writing an .eh_frame section for the function being described. Windows unwind info generation wouldn't have to change much, with implementors of FrameUnwindSink collecting an array of UnwindInfo rather than just one.

I imagine this looking like FrameUnwindSink impls building a collection of records, either a FrameTable for Libunwind or a vec of UnwindInfo for Fastcall. To go along with this, I expect such impls would be added in cranelift-module. These may or may not be suitable for use from wasmtime, I'm optimistic that they could because in the worst case a FrameUnwindSink can be created, used for one function, and then discarded, preserving the current behavior.

My expectation is that a trait like

trait FrameUnwindSink {
    /// Create an instance of the implementor for the provided unwind style.
    fn for_style(kind: FrameUnwindKind) -> Self;

    /// Add a function to the sink. May panic if the function's calling convention
    /// is not compatible with the unwind style this sink was created with.
    fn add_function(&mut self, func: &Function, isa: &dyn TargetIsa);

    /// Serialize this sink's contents in a manner appropriate for this sink's
    /// unwind style.
    fn serialize(&self) -> Vec<u8>;

should satisfy everyone's needs?

The notable benefit of this is making it easy to generate a single .eh_frame section when using cranelift-faerie or cranelift-object, meaning lucet can unwind wasm, have backtraces, all that good stuff :) I imagine @bjorn3 might like this for having unwind information when using Cranelift as a rustc backend as well?

This also seems like a good excuse to move isa::x86::fde and isa::x86::unwind to isa::x86::unwind::windows and isa::x86::unwind::libunwind respectively?

cc @yurydelendik as I'd like your thoughts on this idea, and @peterhuene as I know you've also worked on unwind information here.

Opening an issue to make sure this gets a design friendly to both lucet and wasmtime, rather than just tweaking bytecodealliance/cranelift#902 until something looks right :)

Last updated: Nov 22 2024 at 16:03 UTC