Better documentation needed: reentrancy through the panic hook

[RalfJung]

The following function may seem sound:

fn definitely_safe(b: bool) {
    static mut S: i32 = 0;
    
    struct Bomb<'a>(&'a mut i32);
    impl<'a> Drop for Bomb<'a> {
        fn drop(&mut self) {
            *self.0 = 42;
        }
    }
    
    // SAEFTY: we're not creating a second reference while
    // this one is live.
    let _bomb = Bomb(unsafe { &mut *ptr::addr_of_mut!(S) });
    if b {
        panic!("Oopsie, something went wrong!");
    }
}

But actually it is not. The panic can call arbitrary user-defined code via a panic hook, and that can do a reentrant call do definitely_safe, and then we have UB.

I don't think there is any way to say that definitely_safe is sound, so the main question is -- how to we raise awareness of this reentrancy issue so that unsafe code authors don't stumble into it?

[RalfJung]

I wonder if swc-project/swc#8362 can be turned into an unsoundness on stable via a custom panic hook.

EDIT: I was not able to find any way to make that code panic, other than via allocation failure. But if/when allocation failure can lead to a panic, then this definitely becomes an unsoundness.

[RalfJung]

@rust-lang/libs-api @rust-lang/lang I wonder what your take on this is -- do crates in general have to consider every panic as a possible point of reentrancy? That seems really hard to reason about as panics can occur in a lot of places. It's also unclear what the alternative is though, since panic::set_hook is stable. (Making that hook require a Send closure is something that could have helped here, but it's probably too late for that. It's also kind of a hack.)

Also note the related #506 -- if allocation or allocation failure can lead to a panic, then every allocation site also necessarily has to be considered a possible point of reentrancy (i.e., a point where arbitrary safe code is executed, including code that may call back into the current library -- even for !Sync types).

[nikomatsakis]

Egads. This is a challenge, yes.

…

On Mon, Apr 22, 2024, at 4:01 AM, Ralf Jung wrote: @rust-lang/libs-api <https://github.com/orgs/rust-lang/teams/libs-api> @rust-lang/lang <https://github.com/orgs/rust-lang/teams/lang> I wonder what your take on this is -- do crates in general have to consider every panic as a possible point of reentrancy? That seems really hard to reason about as panics can occur in a lot of places. It's also unclear what the alternative is though, since `panic::set_hook` is stable. — Reply to this email directly, view it on GitHub <#505 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABF4ZUOLMRCZQMFSBS4VSTY6S7TZAVCNFSM6AAAAABGK2CDZOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANRYG42DEMJYG4>. You are receiving this because you are on a team that was mentioned.Message ID: ***@***.***>