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

rustc: Implement custom panic runtimes #32900

Merged
merged 2 commits into from
May 10, 2016
Merged

Conversation

alexcrichton
Copy link
Member

This commit is an implementation of RFC 1513 which allows applications to
alter the behavior of panics at compile time. A new compiler flag, -C panic,
is added and accepts the values unwind or panic, with the default being
unwind. This model affects how code is generated for the local crate, skipping
generation of landing pads with -C panic=abort.

Panic implementations are then provided by crates tagged with
#![panic_runtime] and lazily required by crates with
#![needs_panic_runtime]. The panic strategy (-C panic value) of the panic
runtime must match the final product, and if the panic strategy is not abort
then the entire DAG must have the same panic strategy.

With the -C panic=abort strategy, users can expect a stable method to disable
generation of landing pads, improving optimization in niche scenarios,
decreasing compile time, and decreasing output binary size. With the -C panic=unwind strategy users can expect the existing ability to isolate failure
in Rust code from the outside world.

Organizationally, this commit dismantles the sys_common::unwind module in
favor of some bits moving part of it to libpanic_unwind and the rest into the
panicking module in libstd. The custom panic runtime support is pretty similar
to the custom allocator support with the only major difference being how the
panic runtime is injected (takes the -C panic flag into account).

Closes #32837

@alexcrichton
Copy link
Member Author

r? @nikomatsakis

extern "system" {
fn ExitProcess(code: u32) -> !;
}
ExitProcess(101);
Copy link
Member

Choose a reason for hiding this comment

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

This is a rather quiet way of exiting the process. If we want something that debuggers will notice we need a noisier way of killing the process, like say __fastfail.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd be fine doing so, but it doesn't appear that there's a function we can call anywhere? Must we resort to assembly to actually do that?

Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately it is an intrisic so unless LLVM gets an intrinsic for it, there's not much we can do other than using assembly. Alternatively we can raise an uncatchable exception as described here.

Copy link
Member

Choose a reason for hiding this comment

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

Or wait, never mind, there is the function RaiseFailFastException although it requires Windows 7 at a minimum.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I've updated to just use intrinsics::abort for now and we can perhaps look into fastfail later

@brson
Copy link
Contributor

brson commented Apr 12, 2016

Sweet patch.

//
// This... is a bit of a hack how we detect this. Ideally this information
// should be encoded in the crate I guess? Would likely require an RFC
// amendment to RFC 1513, however.
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, isn't this what [profile.dev] panic = 'unwind' is for?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, that's meant for compiling an entire DAG with a particular panic runtime

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah gotcha, because even when building panic_abort alone, there's the deps of libc, alloc, and core in the DAG too. Sorry for the noise.

});

fn get_call<F: FnMut()>(_: &mut F) -> fn(&mut F) {
call
Copy link
Contributor

Choose a reason for hiding this comment

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

<F as FnMut>::call_mut

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm good point! This felt a little weird... The signature of that though is:

extern "rust-call" fn(&mut F, ());

Which I'm slightly hesitant to transmute to fn(*mut u8) due to the second argument...

@bors
Copy link
Contributor

bors commented Apr 12, 2016

☔ The latest upstream changes (presumably #32804) made this pull request unmergeable. Please resolve the merge conflicts.

@alexcrichton alexcrichton force-pushed the panic2abort branch 2 times, most recently from d7e2d4d to 22b5ae0 Compare April 12, 2016 16:25
//! The first one is the personality routine described above. The second one
//! allows compilation target to customize the process of resuming unwind at the
//! end of the landing pads. `eh_unwind_resume` is used only if
//! `custom_unwind_resume` flag in the target options is set.
Copy link
Contributor

Choose a reason for hiding this comment

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

huzzah for comments :)

@nikomatsakis
Copy link
Contributor

r=me once tidy errors are fixed. this is nice.

@alexcrichton
Copy link
Member Author

@bors: r=nikomatsakis a9e4f62

@alexcrichton alexcrichton added the relnotes Marks issues that should be documented in the release notes of the next release. label Apr 12, 2016
@alexcrichton
Copy link
Member Author

@bors: r=nikomatsakis 30fbd36

@bors
Copy link
Contributor

bors commented Apr 14, 2016

⌛ Testing commit 30fbd36 with merge 5702651...

@bors
Copy link
Contributor

bors commented Apr 14, 2016

💔 Test failed - auto-win-msvc-64-cargotest

@alexcrichton
Copy link
Member Author

@bors: r+ 2ef34ad

@alexcrichton
Copy link
Member Author

er,

@bors: r=nikomatsakis 2ef34ad

@bors
Copy link
Contributor

bors commented Apr 14, 2016

💡 This pull request was already approved, no need to approve it again.

@alexcrichton
Copy link
Member Author

@bors: r=nikomatsakis 155a9c6

@bors
Copy link
Contributor

bors commented May 9, 2016

☔ The latest upstream changes (presumably #33360) made this pull request unmergeable. Please resolve the merge conflicts.

@alexcrichton
Copy link
Member Author

@bors: r=nikomatsakis 451c9b4

@bors
Copy link
Contributor

bors commented May 9, 2016

⌛ Testing commit 451c9b4 with merge 39baa87...

@bors
Copy link
Contributor

bors commented May 9, 2016

💔 Test failed - auto-win-gnu-64-opt

This commit is an implementation of [RFC 1513] which allows applications to
alter the behavior of panics at compile time. A new compiler flag, `-C panic`,
is added and accepts the values `unwind` or `panic`, with the default being
`unwind`. This model affects how code is generated for the local crate, skipping
generation of landing pads with `-C panic=abort`.

[RFC 1513]: https://github.com/rust-lang/rfcs/blob/master/text/1513-less-unwinding.md

Panic implementations are then provided by crates tagged with
`#![panic_runtime]` and lazily required by crates with
`#![needs_panic_runtime]`. The panic strategy (`-C panic` value) of the panic
runtime must match the final product, and if the panic strategy is not `abort`
then the entire DAG must have the same panic strategy.

With the `-C panic=abort` strategy, users can expect a stable method to disable
generation of landing pads, improving optimization in niche scenarios,
decreasing compile time, and decreasing output binary size. With the `-C
panic=unwind` strategy users can expect the existing ability to isolate failure
in Rust code from the outside world.

Organizationally, this commit dismantles the `sys_common::unwind` module in
favor of some bits moving part of it to `libpanic_unwind` and the rest into the
`panicking` module in libstd. The custom panic runtime support is pretty similar
to the custom allocator support with the only major difference being how the
panic runtime is injected (takes the `-C panic` flag into account).
@alexcrichton alexcrichton force-pushed the panic2abort branch 2 times, most recently from 16ea0fa to 3ac27d9 Compare May 9, 2016 16:52
#[cfg(not(stage0))]
pub extern fn rust_eh_personality() {}
pub mod peronalities {
Copy link

Choose a reason for hiding this comment

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

s/peronalities/personalities/

Currently the compiler has two relatively critical bugs in the implementation of
MSVC unwinding:

* rust-lang#33112 - faults like segfaults and illegal instructions will run destructors
           in Rust, meaning we keep running code after a super-fatal exception
           has happened.

* rust-lang#33116 - When compiling with LTO plus `-Z no-landing-pads` (or `-C
           panic=abort` with the previous commit) LLVM won't remove all `invoke`
           instructions, meaning that some landing pads stick around and
           cleanups may be run due to the previous bug.

These both stem from the flavor of "personality function" that Rust uses for
unwinding on MSVC. On 32-bit this is `_except_handler3` and on 64-bit this is
`__C_specific_handler`, but they both essentially are the "most generic"
personality functions for catching exceptions and running cleanups. That is,
thse two personalities will run cleanups for all exceptions unconditionally, so
when we use them we run cleanups for **all SEH exceptions** (include things like
segfaults).

Note that this also explains why LLVM won't optimize away `invoke` instructions.
These functions can legitimately still unwind (the `nounwind` attribute only
seems to apply to "C++ exception-like unwining"). Also note that the standard
library only *catches* Rust exceptions, not others like segfaults and illegal
instructions.

LLVM has support for another personality, `__CxxFrameHandler3`, which does not
run cleanups for general exceptions, only C++ exceptions thrown by
`_CxxThrowException`. This essentially ideally matches our use case, so this
commit moves us over to using this well-known personality function as well as
exception-throwing function.

This doesn't *seem* to pull in any extra runtime dependencies just yet, but if
it does we can perhaps try to work out how to implement more of it in Rust
rather than relying on MSVCRT runtime bits.

More details about how this is actually implemented can be found in the changes
itself, but this...

Closes rust-lang#33112
Closes rust-lang#33116
@alexcrichton
Copy link
Member Author

@bors: r=nikomatsakis 38e6e5d

@bors
Copy link
Contributor

bors commented May 10, 2016

⌛ Testing commit 38e6e5d with merge 72ed7e7...

bors added a commit that referenced this pull request May 10, 2016
rustc: Implement custom panic runtimes

This commit is an implementation of [RFC 1513] which allows applications to
alter the behavior of panics at compile time. A new compiler flag, `-C panic`,
is added and accepts the values `unwind` or `panic`, with the default being
`unwind`. This model affects how code is generated for the local crate, skipping
generation of landing pads with `-C panic=abort`.

[RFC 1513]: https://github.com/rust-lang/rfcs/blob/master/text/1513-less-unwinding.md

Panic implementations are then provided by crates tagged with
`#![panic_runtime]` and lazily required by crates with
`#![needs_panic_runtime]`. The panic strategy (`-C panic` value) of the panic
runtime must match the final product, and if the panic strategy is not `abort`
then the entire DAG must have the same panic strategy.

With the `-C panic=abort` strategy, users can expect a stable method to disable
generation of landing pads, improving optimization in niche scenarios,
decreasing compile time, and decreasing output binary size. With the `-C
panic=unwind` strategy users can expect the existing ability to isolate failure
in Rust code from the outside world.

Organizationally, this commit dismantles the `sys_common::unwind` module in
favor of some bits moving part of it to `libpanic_unwind` and the rest into the
`panicking` module in libstd. The custom panic runtime support is pretty similar
to the custom allocator support with the only major difference being how the
panic runtime is injected (takes the `-C panic` flag into account).

Closes #32837
@bors bors merged commit 38e6e5d into rust-lang:master May 10, 2016
@alexcrichton
Copy link
Member Author

19th time's the charm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
relnotes Marks issues that should be documented in the release notes of the next release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.