add windows exception handling in C #175

xmclark · 2019-02-14T02:16:00Z

This PR adds experiemental support for windows. After much trial-and-error, we came up with a pure C solution for doing the exception handling (signal handling) and stack jmping. This is not idea, but it is a stop-gap solution and enables us to move forward on windows support (#51).

syrusakbary · 2019-02-14T04:36:10Z

Love this! ❤️

CryZe · 2019-02-14T05:45:54Z

Is C used here because setjmp is problematic in Rust or what's the reason?

syrusakbary · 2019-02-14T05:48:46Z

I reviewed a bit more. I think naming the package clif-backend-windows it's a bit misleading, since only handles exception handling in windows and have no dependency over clif-backend.

What you think on renaming the package to win-exception-handler ?

lachlansneff · 2019-02-14T06:45:21Z

Look at all of those passing tests 😎

xmclark · 2019-02-14T06:50:01Z

Hey @CryZe yes setjmp/longjmp are a necessary evil here. We actually use them for unix too!

In this case, we had trouble getting the jmps to work across the FFI so this is a stop-gap solution written in pure C (as seen in this PR). We never jmp between Rust and C.

EDIT:
I think I didn't totally answer your question. I was having trouble getting a pure rust solution to work. This is a bit of a hack that is only for windows. We may be able to refactor it in the future. We may also choose to keep it in C or upgrade to C++.

xmclark · 2019-02-14T06:51:25Z

@syrusakbary I think a name change is warranted! I will change it to win-exception-handler. I am open to other suggestions.

syrusakbary

Let’s merge it!

xmclark · 2019-02-14T17:27:48Z

I want 👀 from @bjfish or @lachlansneff before we merge.

lachlansneff · 2019-02-14T17:38:04Z

lib/win-exception-handler/exception_handling/exception_handling.c

+        return EXCEPTION_CONTINUE_SEARCH;
+    }
+    alreadyHandlingException = TRUE;
+    pCONTEXT->Rip = (uintptr_t)(&longjmpOutOfHere);


Add some comments here.

Basically, here, we coerce the os to resume us into a context that calls longjmp instead of just continuing. Presumably, we cannot longjmp out of the signal/exception context, like we can on unix.

1026: Bump smallvec from 0.6.13 to 1.0.0 r=MarkMcCaskey a=dependabot-preview[bot] Bumps [smallvec](https://github.com/servo/rust-smallvec) from 0.6.13 to 1.0.0. <details> <summary>Release notes</summary> *Sourced from [smallvec's releases](https://github.com/servo/rust-smallvec/releases).* > ## v1.0.0 > * Requires Rust 1.36 or later. > * [breaking change] Use `MaybeUninit` to avoid possible undefined behavior ([#162](https://github-redirect.dependabot.com/servo/rust-smallvec/issues/162), [#170](https://github-redirect.dependabot.com/servo/rust-smallvec/issues/170)). > * [breaking change] The `drain` method now takes a range argument, just like the standard `Vec::drain` ([#145](https://github-redirect.dependabot.com/servo/rust-smallvec/issues/145)). > * [breaking change] Remove the `unreachable` function and replace it with the new standard `unreachable_unchecked` function ([#164](https://github-redirect.dependabot.com/servo/rust-smallvec/issues/164)). > * [breaking change] Use `no_std` by default. This crate depends only on `core` and `alloc` by default. If the optional `write` feature is enabled then it depends on `std` so that `SmallVec<[u8;_]>` can implement the `std::io::Write` trait ([#173](https://github-redirect.dependabot.com/servo/rust-smallvec/issues/173)). > * [breaking change] Remove the deprecated `VecLike` trait ([#165](https://github-redirect.dependabot.com/servo/rust-smallvec/issues/165)). > * Add support for 96-element small vectors, `SmallVec<[T; 96]>` ([#163](https://github-redirect.dependabot.com/servo/rust-smallvec/issues/163)). > * Iterators now implement `FusedIterator` ([#172](https://github-redirect.dependabot.com/servo/rust-smallvec/issues/172)). > * Indexing now uses the standard `SliceIndex` trait ([#166](https://github-redirect.dependabot.com/servo/rust-smallvec/issues/166)). > * Add automatic fuzz testing and MIRI testing ([#168](https://github-redirect.dependabot.com/servo/rust-smallvec/issues/168), [#162](https://github-redirect.dependabot.com/servo/rust-smallvec/issues/162)). > * Update syntax and formatting to Rust 2018 standard ([#174](https://github-redirect.dependabot.com/servo/rust-smallvec/issues/174), [#167](https://github-redirect.dependabot.com/servo/rust-smallvec/issues/167)). > </details> <details> <summary>Commits</summary> - [`34c2628`](servo/rust-smallvec@34c2628) Auto merge of [#175](https://github-redirect.dependabot.com/servo/rust-smallvec/issues/175) - mbrubeck:one, r=jdm - [`523a6dc`](servo/rust-smallvec@523a6dc) Version 1.0.0 - [`a2c0504`](servo/rust-smallvec@a2c0504) Auto merge of [#174](https://github-redirect.dependabot.com/servo/rust-smallvec/issues/174) - mbrubeck:2018, r=jdm - [`a51059c`](servo/rust-smallvec@a51059c) Update to Rust 2018 edition - [`01917a6`](servo/rust-smallvec@01917a6) Auto merge of [#168](https://github-redirect.dependabot.com/servo/rust-smallvec/issues/168) - dpc:fuzz, r=mbrubeck - [`a3ba738`](servo/rust-smallvec@a3ba738) Add a fake stub input case for afl fuzzer - [`85c54a5`](servo/rust-smallvec@85c54a5) Add simple fuzzing - [`b2c9c65`](servo/rust-smallvec@b2c9c65) Auto merge of [#173](https://github-redirect.dependabot.com/servo/rust-smallvec/issues/173) - mbrubeck:std, r=emilio - [`fffb185`](servo/rust-smallvec@fffb185) Use no_std by default - [`af73fbd`](servo/rust-smallvec@af73fbd) Auto merge of [#172](https://github-redirect.dependabot.com/servo/rust-smallvec/issues/172) - mbrubeck:fused, r=mbrubeck - Additional commits viewable in [compare view](servo/rust-smallvec@v0.6.13...v1.0.0) </details> <br /> [![Dependabot compatibility score](https://api.dependabot.com/badges/compatibility_score?dependency-name=smallvec&package-manager=cargo&previous-version=0.6.13&new-version=1.0.0)](https://dependabot.com/compatibility-score.html?dependency-name=smallvec&package-manager=cargo&previous-version=0.6.13&new-version=1.0.0) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) - `@dependabot use these labels` will set the current labels as the default for future PRs for this repo and language - `@dependabot use these reviewers` will set the current reviewers as the default for future PRs for this repo and language - `@dependabot use these assignees` will set the current assignees as the default for future PRs for this repo and language - `@dependabot use this milestone` will set the current milestone as the default for future PRs for this repo and language - `@dependabot badge me` will comment on this PR with code to add a "Dependabot enabled" badge to your readme Additionally, you can set the following in your Dependabot [dashboard](https://app.dependabot.com): - Update frequency (including time of day and day of week) - Pull request limits (per update run and/or open at any time) - Automerge options (never/patch/minor, and dev/runtime dependencies) - Out-of-range updates (receive only lockfile updates, if desired) - Security updates (receive only security updates, if desired) </details> Co-authored-by: dependabot-preview[bot] <27856297+dependabot-preview[bot]@users.noreply.github.com>

add windows exception handling in C

4c2cfd5

xmclark changed the title ~~add windows exception handling in C~~ WIP - add windows exception handling in C Feb 14, 2019

xmclark added 4 commits February 13, 2019 18:24

fix the builds

b43d482

cargo fmt

965eb0d

only compile this crate on windows

d1b1991

none of that nonsense

b87bf6c

rename win exception handler module

bf40b3e

xmclark mentioned this pull request Feb 14, 2019

Polyfill Emscripten Syscalls for Windows #176

Closed

16 tasks

xmclark added 2 commits February 14, 2019 08:32

update the readme

ab325d7

cargo fmt

ad2cb4e

xmclark changed the title ~~WIP - add windows exception handling in C~~ add windows exception handling in C Feb 14, 2019

xmclark requested review from syrusakbary, lachlansneff and bjfish and removed request for syrusakbary and lachlansneff February 14, 2019 16:58

syrusakbary approved these changes Feb 14, 2019

View reviewed changes

lachlansneff reviewed Feb 14, 2019

View reviewed changes

add a comment to the c code

f33bddd

xmclark merged commit 6a1fdb7 into master Feb 14, 2019

xmclark deleted the fix/windows-exception-handling-in-c branch February 14, 2019 17:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add windows exception handling in C #175

add windows exception handling in C #175

xmclark commented Feb 14, 2019 •

edited

Loading

syrusakbary commented Feb 14, 2019

CryZe commented Feb 14, 2019

syrusakbary commented Feb 14, 2019

lachlansneff commented Feb 14, 2019

xmclark commented Feb 14, 2019 •

edited

Loading

xmclark commented Feb 14, 2019

syrusakbary left a comment

xmclark commented Feb 14, 2019

lachlansneff Feb 14, 2019

add windows exception handling in C #175

add windows exception handling in C #175

Conversation

xmclark commented Feb 14, 2019 • edited Loading

syrusakbary commented Feb 14, 2019

CryZe commented Feb 14, 2019

syrusakbary commented Feb 14, 2019

lachlansneff commented Feb 14, 2019

xmclark commented Feb 14, 2019 • edited Loading

xmclark commented Feb 14, 2019

syrusakbary left a comment

Choose a reason for hiding this comment

xmclark commented Feb 14, 2019

lachlansneff Feb 14, 2019

Choose a reason for hiding this comment

xmclark commented Feb 14, 2019 •

edited

Loading

xmclark commented Feb 14, 2019 •

edited

Loading