Change std::panicking::try::Data into a union

No longer potentially call `mem::uninitialized::<!>()`

Fixes #39432

diff --git a/src/libstd/lib.rs b/src/libstd/lib.rs
index 3a552c060a9b9066d0031e1007213b15561184e4..070690773b6c4792a05dc525ce790c9d1cdc72ce 100644 (file)
--- a/src/libstd/lib.rs
+++ b/src/libstd/lib.rs
@@ -303,6 +303,7 @@
 #![feature(unboxed_closures)]
 #![feature(unicode)]
 #![feature(unique)]
+#![feature(untagged_unions)]
 #![feature(unwind_attributes)]
 #![feature(vec_push_all)]
 #![feature(zero_one)]

diff --git a/src/libstd/panicking.rs b/src/libstd/panicking.rs
index d76e8816ca45f3932b37baf92a35fcdf8fca5028..3fba49345e63d1e5ee34cf8f63cc483f44492353 100644 (file)
--- a/src/libstd/panicking.rs
+++ b/src/libstd/panicking.rs
@@ -389,28 +389,23 @@ pub fn update_panic_count(amt: isize) -> usize {
 
 /// Invoke a closure, capturing the cause of an unwinding panic if one occurs.
 pub unsafe fn try<R, F: FnOnce() -> R>(f: F) -> Result<R, Box<Any + Send>> {
-    struct Data<F, R> {
+    #[allow(unions_with_drop_fields)]
+    union Data<F, R> {
         f: F,
         r: R,
     }
 
     // We do some sketchy operations with ownership here for the sake of
-    // performance. The `Data` structure is never actually fully valid, but
-    // instead it always contains at least one uninitialized field. We can only
-    // pass pointers down to `__rust_maybe_catch_panic` (can't pass objects by
-    // value), so we do all the ownership tracking here manully.
+    // performance. We can only  pass pointers down to
+    // `__rust_maybe_catch_panic` (can't pass objects by value), so we do all
+    // the ownership tracking here manually using a union.
     //
-    // Note that this is all invalid if any of these functions unwind, but the
-    // whole point of this function is to prevent that! As a result we go
-    // through a transition where:
+    // We go through a transition where:
     //
-    // * First, only the closure we're going to call is initialized. The return
-    //   value is uninitialized.
+    // * First, we set the data to be the closure that we're going to call.
     // * When we make the function call, the `do_call` function below, we take
-    //   ownership of the function pointer, replacing it with uninitialized
-    //   data. At this point the `Data` structure is entirely uninitialized, but
-    //   it won't drop due to an unwind because it's owned on the other side of
-    //   the catch panic.
+    //   ownership of the function pointer. At this point the `Data` union is
+    //   entirely uninitialized.
     // * If the closure successfully returns, we write the return value into the
     //   data's return slot. Note that `ptr::write` is used as it's overwriting
     //   uninitialized data.
@@ -418,11 +413,10 @@ struct Data<F, R> {
     //   in one of two states:
     //
     //      1. The closure didn't panic, in which case the return value was
-    //         filled in. We have to be careful to `forget` the closure,
-    //         however, as ownership was passed to the `do_call` function.
+    //         filled in. We move it out of `data` and return it.
     //      2. The closure panicked, in which case the return value wasn't
-    //         filled in. In this case the entire `data` structure is invalid,
-    //         so we forget the entire thing.
+    //         filled in. In this case the entire `data` union is invalid, so
+    //         there is no need to drop anything.
     //
     // Once we stack all that together we should have the "most efficient'
     // method of calling a catch panic whilst juggling ownership.
@@ -430,7 +424,6 @@ struct Data<F, R> {
     let mut any_vtable = 0;
     let mut data = Data {
         f: f,
-        r: mem::uninitialized(),
     };
 
     let r = __rust_maybe_catch_panic(do_call::<F, R>,
@@ -439,12 +432,9 @@ struct Data<F, R> {
                                      &mut any_vtable);
 
     return if r == 0 {
-        let Data { f, r } = data;
-        mem::forget(f);
         debug_assert!(update_panic_count(0) == 0);
-        Ok(r)
+        Ok(data.r)
     } else {
-        mem::forget(data);
         update_panic_count(-1);
         debug_assert!(update_panic_count(0) == 0);
         Err(mem::transmute(raw::TraitObject {

diff --git a/src/test/run-pass/catch-unwind-bang.rs b/src/test/run-pass/catch-unwind-bang.rs
new file mode 100644 (file)
index 0000000..df54ec9
--- /dev/null
+++ b/src/test/run-pass/catch-unwind-bang.rs
@@ -0,0 +1,17 @@
+// Copyright 2017 The Rust Project Developers. See the COPYRIGHT
+// file at the top-level directory of this distribution and at
+// http://rust-lang.org/COPYRIGHT.
+//
+// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
+// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
+// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
+// option. This file may not be copied, modified, or distributed
+// except according to those terms.
+
+fn worker() -> ! {
+    panic!()
+}
+
+fn main() {
+    std::panic::catch_unwind(worker).unwrap_err();
+}