Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open code the __fastfail intrinsic for rtabort! on windows #33814

Merged
merged 1 commit into from
Jun 1, 2016

Conversation

lambda
Copy link
Contributor

@lambda lambda commented May 23, 2016

As described https://msdn.microsoft.com/en-us/library/dn774154.aspx

This is a Windows 8+ mechanism for terminating the process quickly,
which degrades to either an access violation or bugcheck in older versions.

I'm not sure this is better the the current mechanism of terminating
with an illegal instruction, but we recently converted unix to
terminate more correctly with SIGABORT, and this seems more correct
for windows.

[breaking-change]

@rust-highfive
Copy link
Collaborator

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

@lambda
Copy link
Contributor Author

lambda commented May 23, 2016

This is essentially a rebase of #31519 from @brson on top of the latest version of #31457 which was merged.

r? @alexcrichton

@rust-highfive rust-highfive assigned alexcrichton and unassigned aturon May 23, 2016
@lambda lambda force-pushed the rtabort-use-platform-abort branch from 033d005 to ac8b0ea Compare May 23, 2016 14:02
@lambda
Copy link
Contributor Author

lambda commented May 23, 2016

cc @retep998

// debateable; see https://github.com/rust-lang/rust/pull/31519 for details.
#[cfg(not(unix))]
// On windows, use the processor-specific __fastfail mechanism
// https://msdn.microsoft.com/en-us/library/dn774154.aspx
Copy link
Member

Choose a reason for hiding this comment

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

Could this elaborate on why we're using this? For example mention that exception handlers are removed and such, and also that on Windows 7 and prior we do not see this behavior but it largely equates to the same semantics.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added an expanded explanation.

@lambda lambda force-pushed the rtabort-use-platform-abort branch from ac8b0ea to 368e84f Compare May 24, 2016 12:50
As described https://msdn.microsoft.com/en-us/library/dn774154.aspx

This is a Windows 8+ mechanism for terminating the process quickly,
which degrades to either an access violation or bugcheck in older versions.

I'm not sure this is better the the current mechanism of terminating
with an illegal instruction, but we recently converted unix to
terminate more correctly with SIGABORT, and this *seems* more correct
for windows.

[breaking-change]
@lambda lambda force-pushed the rtabort-use-platform-abort branch from 368e84f to 696a570 Compare May 24, 2016 12:56
@lambda lambda changed the title Open code the __failfast intrinsic for rtabort! on windows Open code the __fastfail intrinsic for rtabort! on windows May 24, 2016
@alexcrichton
Copy link
Member

@bors: r+ 696a570

Thanks!

@durka
Copy link
Contributor

durka commented May 24, 2016

What does "open code" mean, btw? Is it a synonym for "hard code"?

@lambda
Copy link
Contributor Author

lambda commented May 24, 2016

Open-code is compiler terminology for directly generating the instructions to perform a primitive operation rather than calling a function to do so. See this exercise in SICP for example. I think "inline the assembly" might be a better translation than "hard code".

@bors
Copy link
Contributor

bors commented May 28, 2016

⌛ Testing commit 696a570 with merge 13dbf51...

@bors
Copy link
Contributor

bors commented May 28, 2016

💔 Test failed - auto-win-gnu-32-opt-rustbuild

@retep998
Copy link
Member

Again with the spurious failure due to arguments to Command being too long.

@alexcrichton Should I submit a PR that makes rustc print out the linker arguments whenever it hits error 206 on Windows?

@alexcrichton
Copy link
Member

@retep998 sure!

@bors: retry

@bors
Copy link
Contributor

bors commented May 31, 2016

⌛ Testing commit 696a570 with merge 2ec427b...

@bors
Copy link
Contributor

bors commented May 31, 2016

💔 Test failed - auto-linux-64-cross-netbsd

@retep998
Copy link
Member

Test failure is #33778

@alexcrichton
Copy link
Member

@bors: retry

On Tue, May 31, 2016 at 3:28 AM, Peter Atashian notifications@github.com
wrote:

Test failure is #33778 #33778


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#33814 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAD95J3c0luny9zvtCWpjN1I8XbVaa47ks5qHA1CgaJpZM4Iki6K
.

@bors
Copy link
Contributor

bors commented May 31, 2016

⌛ Testing commit 696a570 with merge a287eec...

@bors
Copy link
Contributor

bors commented May 31, 2016

💔 Test failed - auto-linux-64-opt-rustbuild

@alexcrichton
Copy link
Member

@bors: retry

@bors
Copy link
Contributor

bors commented Jun 1, 2016

⌛ Testing commit 696a570 with merge 42fcc92...

@bors
Copy link
Contributor

bors commented Jun 1, 2016

⛄ The build was interrupted to prioritize another pull request.

@bors
Copy link
Contributor

bors commented Jun 1, 2016

⌛ Testing commit 696a570 with merge 7161ade...

@bors
Copy link
Contributor

bors commented Jun 1, 2016

⛄ The build was interrupted to prioritize another pull request.

@bors
Copy link
Contributor

bors commented Jun 1, 2016

⌛ Testing commit 696a570 with merge 12d1659...

bors added a commit that referenced this pull request Jun 1, 2016
Open code the __fastfail intrinsic for rtabort! on windows

As described https://msdn.microsoft.com/en-us/library/dn774154.aspx

This is a Windows 8+ mechanism for terminating the process quickly,
which degrades to either an access violation or bugcheck in older versions.

I'm not sure this is better the the current mechanism of terminating
with an illegal instruction, but we recently converted unix to
terminate more correctly with SIGABORT, and this *seems* more correct
for windows.

[breaking-change]
@bors bors merged commit 696a570 into rust-lang:master Jun 1, 2016
@pravic
Copy link
Contributor

pravic commented Jun 6, 2016

User-mode fast fail requests appear as a second chance non-continuable exception

Windows operating systems that do not support the fast fail instruction natively will typically treat a fast fail request as an access violation, or as an UNEXPECTED_KERNEL_MODE_TRAP bugcheck. In these cases, the program is still terminated, but not necessarily as quickly.

In Windows 7 this is just a regular continuable exception, so it can be catched, suppressed or recovered (for example, by SEH/VEH directly or undirectly).

@retep998
Copy link
Member

retep998 commented Jun 6, 2016

@pravic The previous solution of generating an illegal instruction exception was also a regular continuable exception, so there is no regression here on Windows 7, but there is an improvement on newer versions.

@pravic
Copy link
Contributor

pravic commented Jun 7, 2016

So why not to raise a noncontinuable exception? Or no. Why not to reuse the ucrt behavior? Looks hastily.

@retep998
Copy link
Member

retep998 commented Jun 7, 2016

@pravic By all means feel free to submit a PR that raises a noncontinuable exception on Windows 7 and older. This PR does not prevent such a change being made later on, and is simply a step in the right direction.

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

Successfully merging this pull request may close these issues.

8 participants