Stream: git-wasmtime

Topic: wasmtime / PR #3864 Unwind: separate the "set frame reg" ...

Wasmtime GitHub notifications bot (Feb 28 2022 at 23:29):

cfallin opened PR #3864 from windows-unwind to main:

In #3859 we discovered that the unwind metadata info on Windows is
placed slightly incorrectly in the presence of explicit stack checks in
function prologues. In particular, if the stack check fails, then we
start the process of backtracing from a state where we have actually
updated rbp (the frame pointer) but we have not emitted the metadata
saying we have done so.

The fix is to emit the SetFPReg Win32 unwind op exactly at the offset
of the mov rbp, rsp instruction, not after the stack check. However,
actually achieving this output given our current unwind design (which
abstracts over SysV and Win32) is slightly tricky, because:

• We previously had a single DefineNewFrame op that both indicated
  where the FP was set, and gave the size of the clobbers;

• We don't know the size of the clobbers until we generate the
  clobber-save sequence; and

• The clobber-save sequence is generated after any explicit stack check.

To resolve the problem, this PR splits the DefineNewFrame op into two
pieces: SetFrameReg and DefineNewFrame. On x86-64 and aarch64 this
is emitted in the appropriate place. (s390x does not use a frame
register, so has no need for this op.) This should resolve the issue we
were seeing with Windows-2022 and unwinding.

(Thanks to @alexcrichton, @iximeow and @peterhuene for help debugging
this!)

I've switched the main Windows CI jobs back to Windows-2022 (naming this explicitly rather than windows-latest to pin our CI on a known version -- happy to switch that to -latest though if we think that's better). I've also added a job that explicitly tests Windows-2019 to ensure we don't have any other regressions. If the overhead for this is too high then maybe we can remove it after seeing a clean pass at least on this fix.

Fixes #3859.

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Feb 28 2022 at 23:29):

cfallin requested alexcrichton for a review on PR #3864.

Wasmtime GitHub notifications bot (Feb 28 2022 at 23:32):

cfallin updated PR #3864 from windows-unwind to main.

Wasmtime GitHub notifications bot (Feb 28 2022 at 23:34):

cfallin updated PR #3864 from windows-unwind to main.

Wasmtime GitHub notifications bot (May 09 2022 at 23:59):

cfallin updated PR #3864 from windows-unwind to main.

Wasmtime GitHub notifications bot (Aug 02 2022 at 07:03):

cfallin closed without merge PR #3864.

Last updated: Dec 23 2024 at 12:05 UTC