Refactor unwind generation in Cranelift.

[peterhuene]

This PR makes the following changes to unwind information generation in
Cranelift:

• Remove frame layout change implementation in favor of processing the prologue
  and epilogue instructions when unwind information is requested. This also
  means this work is no longer performed for Windows, which didn't utilize it.
  It also helps simplify the prologue and epilogue generation code.

• Remove the unwind sink implementation that required each unwind information
  to be represented in final form. For FDEs, this meant writing a
  complete frame table per function, which wastes 20 bytes or so for each
  function with duplicate CIEs. This also enables Cranelift users to collect the
  unwind information and write it as a single frame table.

• For System V calling convention in Wasmtime, the unwind information is no longer stored
  in code memory (it's only a requirement for Windows ABI to do so). This allows
  for more compact code memory for modules with a lot of functions.

• Deletes some duplicate code relating to frame table generation. Users can
  now simply use gimli to create a frame table from each function's unwind
  information.

Fixes #1181.

[github-actions]

Subscribe to Label Action

This issue or pull request has been labeled: "cranelift", "wasmtime:api"

Users Subscribed to "cranelift"

• @bnjbvr

Users Subscribed to "wasmtime:api"

• @peterhuene

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

Learn more.

[peterhuene]

Refactoring to fix the arm64 build error in the file test utility.

[peterhuene]

As this will conflict with #1216 (it adds additional unwind information on Windows for FPR restores), this will need to wait until #1216 goes in then I'll merge the changes.

[peterhuene]

This will need some additional changes with recently merged PRs to get working again. I'll see to it on Monday.

[peterhuene]

This should now be ready for review again after merging in the FPR changes for Windows unwind information.

[yurydelendik]

Overall, it looks fine, except __register_frame piece. @alexcrichton and I talked about using perhaps __register_frame_table where it is available (?), e.g. macosx source code says it is not available there.

[peterhuene]

I'll see about implementing __register_frame_table on platforms where it can be used so we only call it once. On platforms where we can't use it, we'll walk the table registering each entry like we're doing now.

[bnjbvr]

(Redirecting review to @yurydelendik who seemed to have a first look already; please let me know if I need to look at this as well!)

[yurydelendik]

LGTM

The change is required around __deregister_frame as well.

[peterhuene]

@yurydelendik is it required around __deregister_frame though? In the non-macOS case, we only register the entire frame table once and deregister once because only one element was added to the registrations vec.

[yurydelendik]

@yurydelendik is it required around __deregister_frame though?

Good question. I would expect the api be "symmetrical", means that if whatever is fed to __register_frame shall be passed to __deregister_frame. (FWIW this hole API confusing)

Is it possible to check if 1) the current __deregister_frame code de-registers FDEs; 2) does not add additional O(n^2) complexity for gcc unwind ?

[peterhuene]

I agree that the underlying API here is a mess. The code I have now should cause symmetrical invocations of __register_frame and __deregister_frame, but in the case of libgcc, only one invocation of each per code memory entry.

From what I can decipher of the registration code in libgcc, __register_frame will walk all of the FDEs from the given "base entry" (skipping over CIEs), but it only stores the "base entry" address internally for deregistration and therefore only needs the single matching __deregister_frame call. It appears that __register_frame effectively accepts what we (and gimli) call a frame table.

What libgcc is calling a "frame table" for use with __register_frame_table seems to be an array of pointers to those "base entries", allowing you to register frames from multiple "translation units" with a single call. To put it another way, this function would be used if we wanted to register multiple frame tables from our perspective. Technically we could use __register_frame_table to register the frame tables for all code memory entries in a single call, but I think that would require a bit of refactoring to make work.

[yurydelendik]

_register_frame will walk all of the FDEs from the given "base entry" (skipping over CIEs)

Yeah, that confirms my analysis as well. Thanks. I guess we good to go. 👍

[philipc]

FYI, gimli::write::Expression is no longer a simple Vec<u8> after gimli-rs/gimli#479, so this isn't going to work.

It seems like you don't actually need impl From<gimli::write::CallFrameInstruction> for CallFrameInstruction though, so it would be possible to simply delete all the Expression support. If you do need to support expressions, then you'll need to come up with your own serializable version of them. gimli's expressions are used for other debug sections too, thus they potentially have references to DIEs and can't be individually serialized.

[philipc]

See philipc@103d68d for the changes I think are needed. I can open a PR once a new gimli version is released.

[peterhuene]

@philipc thanks for the heads up! When we update gimli, I think we can work around this by removing expression support from this representation since we don't need it to describe our frames currently.