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

MSVC: Cleanups are run on all faults, like segfaults #33112

Closed
alexcrichton opened this issue Apr 20, 2016 · 2 comments
Closed

MSVC: Cleanups are run on all faults, like segfaults #33112

alexcrichton opened this issue Apr 20, 2016 · 2 comments
Labels
O-windows-msvc Toolchain: MSVC, Operating system: Windows

Comments

@alexcrichton
Copy link
Member

For example:

struct Bomb;

impl Drop for Bomb {
        fn drop(&mut self) {
                println!("foo");
        }
}

fn main() {
        let _a = Bomb;
        foo();
}

fn foo() {
        unsafe { *(0 as *mut i32) = 0; }
}

On Unix this doesn't print anything where as on Windows it will print foo. While initially intentional I'm thinking that this may seem like a bad idea now.

This is happening because in Windows faults like segfaults (or illegal instructions) all go through the same error handling mechanism that normal program exceptions go through. Our cleanups (aka drop code) are registered using cleanuppad instructions with no extra arguments which essentially means "run this cleanup for all exceptions".

This... may or may not be memory safe. I'd personally find it surprising that we keep running code after a segfault or illegal instruction, I'd prefer Unix's semantics where no more Rust code is run at least.

@alexcrichton alexcrichton added the O-windows-msvc Toolchain: MSVC, Operating system: Windows label Apr 20, 2016
@brson
Copy link
Contributor

brson commented Apr 20, 2016

I agree. This behavior is likely to cause security issues.

@alexcrichton
Copy link
Member Author

Looks like this is not the behavior of C++ on using clang on Windows. The IR generated also uses a cleanuppad with no arguments, but the personality function is __CxxFrameHandler3 instead of __C_specific_handler that we use.

alexcrichton added a commit to alexcrichton/rust that referenced this issue May 4, 2016
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 added a commit to alexcrichton/rust that referenced this issue May 5, 2016
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 added a commit to alexcrichton/rust that referenced this issue May 8, 2016
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
@bors bors closed this as completed in 38e6e5d May 10, 2016
alexcrichton added a commit to alexcrichton/rust that referenced this issue Feb 27, 2018
Exception handling on MSVC for Rust has at this point a long and storied
history to it. We currently use the exception mechanisms on MSVC to implement
panics in Rust. On MSVC both 32 and 64-bit exceptions are based on SEH,
structured exception handling. LLVM long ago added special instructions for
MSVC, and we've been using those for as long as MSVC panics have been
implemented!

Exception handling at the system layer is typically guided by "personality
functions" which are in theory intended to be language specific and allow
programming languages to implement custom logic. Since the beginning, however,
LLVM has had a hardcoded list of "known personalities" as they behave quite
differently. As a result, Rust has historically shoehorned our desired panic
behavior into preexisting personality functions defined on MSVC.

Originally Rust actually used the functions `__C_specific_handler` (64-bit) and
`_except_handler3` (32-bit). Using these personalities was relatively easy in
Rust and only required a "filter function" on our "catch" equivalent to only
catch Rust exceptions. Unfortunately these personalities suffered two
[fatal][f1] [flaws][f2], which caused us to subsequently [switch] to the
`__CxxFrameHandler3` personality.

This personality is intended for C++, but we're mostly like C++ in any case so
it worked quite well for a long time! The default C++ personality didn't run
cleanups on faults and LLVM optimized invokes of nounwind functions well. The
only downside at this point was that the support was [sort of scary][scary].

Fast forward to the 1.24.0 release and another [fatal flaw][f3] is found in our
strategy. Historically we've always declared "unwinding into Rust code from
other languages is undefined behavior" (or unwinding out of Rust code). In
1.24.0 we changed `extern` functions defined in Rust to enforce this behavior,
forcibly aborting the process if the function panics. Everything was ship shape
until it was discovered that `longjmp` across Rust frames caused the process to
abort!

It turns out that on MSVC `longjmp` is implemented with SEH! Furthermore it
turns out that the personality we're using, `__CxxFrameHandler3`, recognized the
`longjmp` exception enough to run cleanups. Consequently, when SEH ran past an
`extern` function in Rust it aborted the process. Sounds like "cleanups run on
segfaults" v2!

Well in any case, that's a long-winded way of describing how shoehorning Rust's
desired behavior into existing personality functions is getting more and more
difficult. As a result, this commit starts taking a new strategy of defining
custom personality functions in Rust (like we do for all other platforms) and
teaching LLVM about these personalities so they're classified correctly and
don't suffer from [old bugs][f2].

Alright, so with all that information in your head now this commit can be
described with:

* New personality functions are added for MSVC: `rust_seh{32,64}_personality`.
* These functions are shims around `__C_specific_handler` and `_except_handler3`
  like how on Unix we're typically a shim around a gcc personality.
* Rust's personality functions defer on all exceptions that *aren't*
  Rust-related. We choose an arbitrary code to represent a Rust exception and
  only exceptions with matching codes execute our cleanups/catches. (the
  prevents [previous bugs][f1] with the personalities these functions are
  wrapping).
* LLVM is updated with a small-ish commit to learn about these personality
  functions. The largest change here is, again, [ensuring old bugs don't
  resurface][f2] by teaching LLVM that it can simplify invokes of nounwind
  functions in Rust.
* Finally, bedbad6 is partially reverted to restore the old translation
  behavior of the `try` intrinsic, bringing some scary code back into the
  compiler about `llvm.localescape` and such.

Overall the intent of this commit is to preserve all existing behavior with
panics on MSVC (aka keep old bugs closed and use the same system in general) but
enable longjmps across Rust code. To this effect a test is checked in which
asserts that we can indeed longjmp across Rust code with destructors and such.

[f1]: rust-lang#33112
[f2]: rust-lang#33116
[switch]: rust-lang@38e6e5d0
[scary]: https://github.com/rust-lang/rust/blob/bedbad61195d2eae69b43eca49c6d3e2aee8f208/src/libpanic_unwind/seh.rs#L107-L294
[f3]: rust-lang#48251
alexcrichton added a commit to alexcrichton/rust that referenced this issue Feb 27, 2018
Exception handling on MSVC for Rust has at this point a long and storied
history to it. We currently use the exception mechanisms on MSVC to implement
panics in Rust. On MSVC both 32 and 64-bit exceptions are based on SEH,
structured exception handling. LLVM long ago added special instructions for
MSVC, and we've been using those for as long as MSVC panics have been
implemented!

Exception handling at the system layer is typically guided by "personality
functions" which are in theory intended to be language specific and allow
programming languages to implement custom logic. Since the beginning, however,
LLVM has had a hardcoded list of "known personalities" as they behave quite
differently. As a result, Rust has historically shoehorned our desired panic
behavior into preexisting personality functions defined on MSVC.

Originally Rust actually used the functions `__C_specific_handler` (64-bit) and
`_except_handler3` (32-bit). Using these personalities was relatively easy in
Rust and only required a "filter function" on our "catch" equivalent to only
catch Rust exceptions. Unfortunately these personalities suffered two
[fatal][f1] [flaws][f2], which caused us to subsequently [switch] to the
`__CxxFrameHandler3` personality.

This personality is intended for C++, but we're mostly like C++ in any case so
it worked quite well for a long time! The default C++ personality didn't run
cleanups on faults and LLVM optimized invokes of nounwind functions well. The
only downside at this point was that the support was [sort of scary][scary].

Fast forward to the 1.24.0 release and another [fatal flaw][f3] is found in our
strategy. Historically we've always declared "unwinding into Rust code from
other languages is undefined behavior" (or unwinding out of Rust code). In
1.24.0 we changed `extern` functions defined in Rust to enforce this behavior,
forcibly aborting the process if the function panics. Everything was ship shape
until it was discovered that `longjmp` across Rust frames caused the process to
abort!

It turns out that on MSVC `longjmp` is implemented with SEH! Furthermore it
turns out that the personality we're using, `__CxxFrameHandler3`, recognized the
`longjmp` exception enough to run cleanups. Consequently, when SEH ran past an
`extern` function in Rust it aborted the process. Sounds like "cleanups run on
segfaults" v2!

Well in any case, that's a long-winded way of describing how shoehorning Rust's
desired behavior into existing personality functions is getting more and more
difficult. As a result, this commit starts taking a new strategy of defining
custom personality functions in Rust (like we do for all other platforms) and
teaching LLVM about these personalities so they're classified correctly and
don't suffer from [old bugs][f2].

Alright, so with all that information in your head now this commit can be
described with:

* New personality functions are added for MSVC: `rust_seh{32,64}_personality`.
* These functions are shims around `__C_specific_handler` and `_except_handler3`
  like how on Unix we're typically a shim around a gcc personality.
* Rust's personality functions defer on all exceptions that *aren't*
  Rust-related. We choose an arbitrary code to represent a Rust exception and
  only exceptions with matching codes execute our cleanups/catches. (the
  prevents [previous bugs][f1] with the personalities these functions are
  wrapping).
* LLVM is updated with a small-ish commit to learn about these personality
  functions. The largest change here is, again, [ensuring old bugs don't
  resurface][f2] by teaching LLVM that it can simplify invokes of nounwind
  functions in Rust.
* Finally, bedbad6 is partially reverted to restore the old translation
  behavior of the `try` intrinsic, bringing some scary code back into the
  compiler about `llvm.localescape` and such.

Overall the intent of this commit is to preserve all existing behavior with
panics on MSVC (aka keep old bugs closed and use the same system in general) but
enable longjmps across Rust code. To this effect a test is checked in which
asserts that we can indeed longjmp across Rust code with destructors and such.

[f1]: rust-lang#33112
[f2]: rust-lang#33116
[switch]: rust-lang@38e6e5d0
[scary]: https://github.com/rust-lang/rust/blob/bedbad61195d2eae69b43eca49c6d3e2aee8f208/src/libpanic_unwind/seh.rs#L107-L294
[f3]: rust-lang#48251
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-windows-msvc Toolchain: MSVC, Operating system: Windows
Projects
None yet
Development

No branches or pull requests

2 participants