Skip to content
This repository has been archived by the owner on Jun 26, 2020. It is now read-only.

Implement emitting Windows unwind information for fastcall functions. #1155

Merged
merged 3 commits into from
Nov 5, 2019

Conversation

peterhuene
Copy link
Member

This PR implements emitting Windows unwind information for x64 fastcall
calling convention functions.

The unwind information can be used to construct a Windows function table at
runtime for JIT'd code, enabling stack walking and unwinding by the operating
system.

@peterhuene
Copy link
Member Author

I've marked this as a draft so I can get early feedback of the changes.

I'm still working on consuming this change from Wasmtime so that Windows can walk/unwind across JIT frames. I'll make this PR ready when those changes have been properly tested.

@iximeow
Copy link
Collaborator

iximeow commented Oct 18, 2019

Woah! This is really cool! I think some of the stack layout information you're collecting in UnwindInfo::try_from_func is also collected in #679 (which my ELF unwind information PR builds on: #902). #679 should be close to landing, it's just waiting on someone to have review bandwidth.

How does Windows unwind information handle functions with multiple epilogues, if at all? The Preserve and Restore frame layout changes map directly to equivalent DWARF unwind information, but I dunno if Windows has similar operators.

Upside, if you can phrase this as information built on top of FrameLayoutChange, you can use whatever frame register the layout happens to have encoded. It should correctly describe rsp for functions that elide frame pointer as it is today, even though Cranelift doesn't do that yet.

Copy link
Contributor

@bjorn3 bjorn3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please dont use unsafe code when not necessary.

cranelift-codegen/src/isa/x86/unwind.rs Outdated Show resolved Hide resolved
cranelift-codegen/src/isa/x86/unwind.rs Outdated Show resolved Hide resolved
cranelift-codegen/src/isa/x86/unwind.rs Show resolved Hide resolved
cranelift-codegen/src/isa/x86/unwind.rs Outdated Show resolved Hide resolved
cranelift-filetests/src/test_unwind.rs Outdated Show resolved Hide resolved
cranelift-filetests/src/test_unwind.rs Outdated Show resolved Hide resolved
cranelift-filetests/src/test_unwind.rs Outdated Show resolved Hide resolved
@peterhuene
Copy link
Member Author

Epilogues on Windows x64 ABI are strictly defined, so the OS can tell if it's in an epilogue by disassembling forward a few bytes from the frame IP. Functions are allowed to have multiple epilogues. For unwind, if it detects that the frame IP is in the epilogue, it will simulate the remaining execution of the epilogue to unwind the frame; thus the OS needs no description of epilogues in the unwind information.

I did look at #679 and I think there's work there I can piggy back off of, although it does seem to be DWARF-tailored. For Windows x64 (we'll need an Windows ARM implementation eventually, but unwind information on ARM is quite different), we just need information about the prologues for unwind; at least until such time where a Cranelift function can have an exception handler. Still, I would like #679 to go in first and then we can figure out what makes sense to merge this in with the FrameLayoutChange information, I think.

@peterhuene peterhuene marked this pull request as ready for review October 22, 2019 03:03
@peterhuene
Copy link
Member Author

peterhuene commented Oct 22, 2019

I've tested these changes with corresponding changes to Wasmtime and was able to get successful stack walks (by Visual Studio's debugger) and unwinds through JIT frames on Windows.

I think we may want to focus on getting #679 in first and then I can rebase this on top of those changes and see what makes sense to merge in with those changes.

Copy link
Member

@sunfishcode sunfishcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, this looks good!

cranelift-codegen/Cargo.toml Outdated Show resolved Hide resolved
cranelift-filetests/src/test_unwind.rs Show resolved Hide resolved
cranelift-codegen/src/isa/x86/unwind.rs Show resolved Hide resolved
cranelift-codegen/src/isa/x86/unwind.rs Outdated Show resolved Hide resolved
This commit implements emitting Windows unwind information for x64 fastcall
calling convention functions.

The unwind information can be used to construct a Windows function table at
runtime for JIT'd code, enabling stack walking and unwinding by the operating
system.
This commit addresses code review feedback:

* Remove unnecessary unsafe code.
* Emit the unwind information always as little endian.
* Fix comments.

A dependency from cranelift-codegen to the byteorder crate was added.
The byteorder crate is a no-dependencies crate with a reasonable
abstraction for writing binary data for a specific endianness.
* Disable default features for the `byteorder` crate.
* Add a comment regarding the Windows ABI unwind code numerical values.
* Panic if we encounter a Windows function with a prologue greater than 256
  bytes in size.
@peterhuene
Copy link
Member Author

peterhuene commented Nov 1, 2019

Rather than waiting on #679 to merge first (which hasn't had much progress in the past week), I think we may want to get this merged (pending approval) to unblock getting the Wasm test suites passing on Windows for Wasmtime. The Windows unwind information is intentionally isolated from the rest of Cranelift code generation and I can easily refactor this, as needed, once the tracking frame layout changes PR is merged.

Copy link
Member

@sunfishcode sunfishcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good!

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

Successfully merging this pull request may close these issues.

4 participants