From 7dc00b52e71dd8c6a7d22245d90dae151bc0391a Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Wed, 27 Jun 2018 18:09:21 +0200 Subject: [PATCH 1/2] Remove use of uninitialized() in Weak::new() This avoids the question of whether `uninitialized::()` is necessarily UB when `T = !` --- src/liballoc/arc.rs | 16 ++++++++-------- src/liballoc/rc.rs | 16 ++++++++-------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/liballoc/arc.rs b/src/liballoc/arc.rs index 0fbd1408f644f..79f4515121284 100644 --- a/src/liballoc/arc.rs +++ b/src/liballoc/arc.rs @@ -23,7 +23,7 @@ use core::borrow; use core::fmt; use core::cmp::Ordering; use core::intrinsics::abort; -use core::mem::{self, align_of_val, size_of_val, uninitialized}; +use core::mem::{self, align_of_val, size_of_val}; use core::ops::Deref; use core::ops::CoerceUnsized; use core::ptr::{self, NonNull}; @@ -1028,13 +1028,13 @@ impl Weak { #[stable(feature = "downgraded_weak", since = "1.10.0")] pub fn new() -> Weak { unsafe { - Weak { - ptr: Box::into_raw_non_null(box ArcInner { - strong: atomic::AtomicUsize::new(0), - weak: atomic::AtomicUsize::new(1), - data: uninitialized(), - }), - } + let layout = Layout::new::>(); + let mut ptr = Global.alloc(layout) + .unwrap_or_else(|_| handle_alloc_error(layout)) + .cast::>(); + ptr::write(&mut ptr.as_mut().strong, atomic::AtomicUsize::new(0)); + ptr::write(&mut ptr.as_mut().weak, atomic::AtomicUsize::new(1)); + Weak { ptr } } } } diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index 32d624e8fbc79..913a285b6bb8d 100644 --- a/src/liballoc/rc.rs +++ b/src/liballoc/rc.rs @@ -253,7 +253,7 @@ use core::hash::{Hash, Hasher}; use core::intrinsics::abort; use core::marker; use core::marker::{Unsize, PhantomData}; -use core::mem::{self, align_of_val, forget, size_of_val, uninitialized}; +use core::mem::{self, align_of_val, forget, size_of_val}; use core::ops::Deref; use core::ops::CoerceUnsized; use core::ptr::{self, NonNull}; @@ -1182,13 +1182,13 @@ impl Weak { #[stable(feature = "downgraded_weak", since = "1.10.0")] pub fn new() -> Weak { unsafe { - Weak { - ptr: Box::into_raw_non_null(box RcBox { - strong: Cell::new(0), - weak: Cell::new(1), - value: uninitialized(), - }), - } + let layout = Layout::new::>(); + let mut ptr = Global.alloc(layout) + .unwrap_or_else(|_| handle_alloc_error(layout)) + .cast::>(); + ptr::write(&mut ptr.as_mut().strong, Cell::new(0)); + ptr::write(&mut ptr.as_mut().weak, Cell::new(1)); + Weak { ptr } } } } From 28b774edfc96d8c392ccaea7c1b8be72158b8103 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Wed, 27 Jun 2018 21:36:29 +0200 Subject: [PATCH 2/2] Add a test and static assertions for the soundness of Weak::new() --- src/liballoc/arc.rs | 9 +++++++++ src/liballoc/lib.rs | 1 + src/liballoc/rc.rs | 9 +++++++++ .../run-pass/weak-new-uninhabited-issue-48493.rs | 14 ++++++++++++++ 4 files changed, 33 insertions(+) create mode 100644 src/test/run-pass/weak-new-uninhabited-issue-48493.rs diff --git a/src/liballoc/arc.rs b/src/liballoc/arc.rs index 79f4515121284..2cae57522ab2f 100644 --- a/src/liballoc/arc.rs +++ b/src/liballoc/arc.rs @@ -1028,6 +1028,15 @@ impl Weak { #[stable(feature = "downgraded_weak", since = "1.10.0")] pub fn new() -> Weak { unsafe { + // Statically assert that we’re not about to allocate zero bytes + // but at least enough space for the `strong` and `weak` fields + // even if `T` is uninhabited. + // https://github.com/rust-lang/rust/issues/48493#issuecomment-372438465 + // https://github.com/rust-lang/rust/pull/50622 + #[cfg(not(stage0))] enum Void {} + #[cfg(not(stage0))] let _ = mem::transmute::, ArcInner<()>>; + #[cfg(not(stage0))] let _ = mem::transmute::, ArcInner<()>>; + let layout = Layout::new::>(); let mut ptr = Global.alloc(layout) .unwrap_or_else(|_| handle_alloc_error(layout)) diff --git a/src/liballoc/lib.rs b/src/liballoc/lib.rs index e25742a4a61eb..365b9370ce344 100644 --- a/src/liballoc/lib.rs +++ b/src/liballoc/lib.rs @@ -100,6 +100,7 @@ #![feature(lang_items)] #![feature(libc)] #![feature(needs_allocator)] +#![feature(never_type)] #![feature(optin_builtin_traits)] #![feature(pattern)] #![feature(pin)] diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index 913a285b6bb8d..d56935550865a 100644 --- a/src/liballoc/rc.rs +++ b/src/liballoc/rc.rs @@ -1182,6 +1182,15 @@ impl Weak { #[stable(feature = "downgraded_weak", since = "1.10.0")] pub fn new() -> Weak { unsafe { + // Assert that we’re not about to allocate zero bytes + // but at least enough space for the `strong` and `weak` fields + // even if `T` is uninhabited. + // https://github.com/rust-lang/rust/issues/48493#issuecomment-372438465 + // https://github.com/rust-lang/rust/pull/50622 + #[cfg(not(stage0))] enum Void {} + #[cfg(not(stage0))] let _ = mem::transmute::, RcBox<()>>; + #[cfg(not(stage0))] let _ = mem::transmute::, RcBox<()>>; + let layout = Layout::new::>(); let mut ptr = Global.alloc(layout) .unwrap_or_else(|_| handle_alloc_error(layout)) diff --git a/src/test/run-pass/weak-new-uninhabited-issue-48493.rs b/src/test/run-pass/weak-new-uninhabited-issue-48493.rs new file mode 100644 index 0000000000000..4b40893b40442 --- /dev/null +++ b/src/test/run-pass/weak-new-uninhabited-issue-48493.rs @@ -0,0 +1,14 @@ +// Copyright 2018 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 or the MIT license +// , at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +fn main() { + enum Void {} + std::rc::Weak::::new(); +}