Skip to content

Commit ca92c51

Browse files
author
whataloadofwhat
committed
Change std::panicking::try::Data into a union
No longer potentially call `mem::uninitialized::<!>()` Fixes #39432
1 parent f805144 commit ca92c51

File tree

3 files changed

+31
-23
lines changed

3 files changed

+31
-23
lines changed

Diff for: src/libstd/lib.rs

+1
Original file line numberDiff line numberDiff line change
@@ -303,6 +303,7 @@
303303
#![feature(unboxed_closures)]
304304
#![feature(unicode)]
305305
#![feature(unique)]
306+
#![feature(untagged_unions)]
306307
#![feature(unwind_attributes)]
307308
#![feature(vec_push_all)]
308309
#![feature(zero_one)]

Diff for: src/libstd/panicking.rs

+13-23
Original file line numberDiff line numberDiff line change
@@ -389,48 +389,41 @@ pub use realstd::rt::update_panic_count;
389389

390390
/// Invoke a closure, capturing the cause of an unwinding panic if one occurs.
391391
pub unsafe fn try<R, F: FnOnce() -> R>(f: F) -> Result<R, Box<Any + Send>> {
392-
struct Data<F, R> {
392+
#[allow(unions_with_drop_fields)]
393+
union Data<F, R> {
393394
f: F,
394395
r: R,
395396
}
396397

397398
// We do some sketchy operations with ownership here for the sake of
398-
// performance. The `Data` structure is never actually fully valid, but
399-
// instead it always contains at least one uninitialized field. We can only
400-
// pass pointers down to `__rust_maybe_catch_panic` (can't pass objects by
401-
// value), so we do all the ownership tracking here manully.
399+
// performance. We can only pass pointers down to
400+
// `__rust_maybe_catch_panic` (can't pass objects by value), so we do all
401+
// the ownership tracking here manually using a union.
402402
//
403-
// Note that this is all invalid if any of these functions unwind, but the
404-
// whole point of this function is to prevent that! As a result we go
405-
// through a transition where:
403+
// We go through a transition where:
406404
//
407-
// * First, only the closure we're going to call is initialized. The return
408-
// value is uninitialized.
405+
// * First, we set the data to be the closure that we're going to call.
409406
// * When we make the function call, the `do_call` function below, we take
410-
// ownership of the function pointer, replacing it with uninitialized
411-
// data. At this point the `Data` structure is entirely uninitialized, but
412-
// it won't drop due to an unwind because it's owned on the other side of
413-
// the catch panic.
407+
// ownership of the function pointer. At this point the `Data` union is
408+
// entirely uninitialized.
414409
// * If the closure successfully returns, we write the return value into the
415410
// data's return slot. Note that `ptr::write` is used as it's overwriting
416411
// uninitialized data.
417412
// * Finally, when we come back out of the `__rust_maybe_catch_panic` we're
418413
// in one of two states:
419414
//
420415
// 1. The closure didn't panic, in which case the return value was
421-
// filled in. We have to be careful to `forget` the closure,
422-
// however, as ownership was passed to the `do_call` function.
416+
// filled in. We move it out of `data` and return it.
423417
// 2. The closure panicked, in which case the return value wasn't
424-
// filled in. In this case the entire `data` structure is invalid,
425-
// so we forget the entire thing.
418+
// filled in. In this case the entire `data` union is invalid, so
419+
// there is no need to drop anything.
426420
//
427421
// Once we stack all that together we should have the "most efficient'
428422
// method of calling a catch panic whilst juggling ownership.
429423
let mut any_data = 0;
430424
let mut any_vtable = 0;
431425
let mut data = Data {
432426
f: f,
433-
r: mem::uninitialized(),
434427
};
435428

436429
let r = __rust_maybe_catch_panic(do_call::<F, R>,
@@ -439,12 +432,9 @@ pub unsafe fn try<R, F: FnOnce() -> R>(f: F) -> Result<R, Box<Any + Send>> {
439432
&mut any_vtable);
440433

441434
return if r == 0 {
442-
let Data { f, r } = data;
443-
mem::forget(f);
444435
debug_assert!(update_panic_count(0) == 0);
445-
Ok(r)
436+
Ok(data.r)
446437
} else {
447-
mem::forget(data);
448438
update_panic_count(-1);
449439
debug_assert!(update_panic_count(0) == 0);
450440
Err(mem::transmute(raw::TraitObject {

Diff for: src/test/run-pass/catch-unwind-bang.rs

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
fn worker() -> ! {
12+
panic!()
13+
}
14+
15+
fn main() {
16+
std::panic::catch_unwind(worker).unwrap_err();
17+
}

0 commit comments

Comments
 (0)