Guaging receptibility: allowing nested unwinds

language design
1

That is, the following would no longer abort: [playground]

struct Bomb;

impl Drop for Bomb {
    fn drop(&mut self) {
        let _ = catch_unwind(|| panic!());
    }
}

fn main() {
    let _ = catch_unwind(|| {
        let _bomb = Bomb;
        panic!();
    });
}

Immediate response: that would make the traditional unwind-to-abort shim[1] fail:

struct UnwindAbortBomb;
impl Drop for UnwindAbortBomb {
    fn drop(&mut self) {
        panic!("causing panic while panicking abort");
    }
}
let bomb = UnwindAbortBomb;
/* must not unwind stuff */
panic!("whoops");
// end must-not-unwind stuff
forget(bomb);

So obviously just allowing nested unwinds isn't really possible... but perhaps, that's because we haven't properly defined what a "nested" unwind is.

For an unwind to be nested, it must not escape the drop glue. An unwind is improperly nested / unhandled and must abort the process if the unwind reaches a landing pad dealing with an outer unwind (or escapes the thread's main).

In the most permissive, this would be a properly nested unwind:

struct Bomb;
impl Drop for Bomb { fn drop(&mut self) { panic!("bomb"); } }
catch_unwind(|| drop(Bomb));

However, this means that the simple panic-in-unwind abort bomb would not cause an immediate abort (instead unwinding to the landing pad handling the outer unwind, continuing to run destructors). This makes this most permissive form of the change a nonstarter. There are two ways to preserve the immediate abort of the bomb:

  • The simple approach is the one taken by RFC #3288: prohibit unwinding out of Drop::drop. Every Drop::drop gets a shim to convert unwinds to aborts, like is (will eventually be) done for extern "C".
  • The much more refined approach is to refine the behavior of improperly nested unwinds. Instead of unwinding to the landing pad handling the outer unwind, we just say that the improperly nested unwind immediately aborts without unwinding. This is then implemented by using a tracker like the existing is panicking count, but which tracks if a catch_unwind handler is registered. This may be natively implementable on the Itanium psABI by determining no fresh handler is available in the initial search phase and aborting. I am unsure about natively implementing the properly nested exceptions semantics natively on top of Windows SEH.

(Quick proof-lite that the latter is enough to ensure a naive panic-in-unwind abort bomb would still immediately abort: 1) the code is unwinding 2) the immediately containing catch_unwind is handling that unwind 3) causing a new unwind will be caught by the same handler 4) we recognize the handler as accounted for 5) thus this unwind is improperly nested 6) thus we immediately abort, QED.)

  1. At least partially because of this, I would prefer if such shims were written with an actual panic_abort! which calls the panic hook and then aborts. Specifically, panic_abort! would be more properly written in user code as

    let _unwind = catch_unwind(|| panic!($($args)*));
// NB: Leak _unwind, as dropping it might panic: rust-lang/rust#86027, #99032
//     Plus, we're about to abort anyway, so it shouldn't matter.
abort();
    ↩︎
2

I'm tracking this as an alternative in my counter-proposal RFC to the drops must not unwind RFC.

If this approach is a reasonable approach which

  1. can be implemented and
  2. does not cause existing sound code to become unsound

then I am strongly considering moving it into the main text's proposal, because it increases application robustness to aborts from panics by allowing catching properly nested unwinds in more situations.

3

Can't we just have catch_unwind decrease the panic count?

4

The problem with this as a naive implementation is that it needs to handle the case where no panic is currently in progress as well. However, yes, this is potentially a valid Implementation strategy (that I probably overlooked by brain poison trying to digest the Itanium eh spec).

5

Another potential issue with that approach is std::thread::panicking; is its semantics std::panic::would_abort, or should it remain true because we are still in a panic?

6

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.