diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index 32d624e8fbc79..5b71d0b85a03e 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}; @@ -1153,6 +1153,10 @@ impl From> for Rc<[T]> { /// [`None`]: ../../std/option/enum.Option.html#variant.None #[stable(feature = "rc_weak", since = "1.4.0")] pub struct Weak { + // This is a `NonNull` to allow optimizing the size of this type in enums, + // but it is not necessarily a valid pointer. + // `Weak::new` sets this to a dangling pointer so that it doesn’t need + // to allocate space on the heap. ptr: NonNull>, } @@ -1165,8 +1169,8 @@ impl !marker::Sync for Weak {} impl, U: ?Sized> CoerceUnsized> for Weak {} impl Weak { - /// Constructs a new `Weak`, allocating memory for `T` without initializing - /// it. Calling [`upgrade`] on the return value always gives [`None`]. + /// Constructs a new `Weak`, without allocating any memory. + /// Calling [`upgrade`] on the return value always gives [`None`]. /// /// [`upgrade`]: struct.Weak.html#method.upgrade /// [`None`]: ../../std/option/enum.Option.html @@ -1181,18 +1185,18 @@ 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(), - }), - } + Weak { + ptr: NonNull::dangling(), } } } +pub(crate) fn is_dangling(ptr: NonNull) -> bool { + let address = ptr.as_ptr() as *mut () as usize; + let align = align_of_val(unsafe { ptr.as_ref() }); + address == align +} + impl Weak { /// Attempts to upgrade the `Weak` pointer to an [`Rc`], extending /// the lifetime of the value if successful. @@ -1222,13 +1226,25 @@ impl Weak { /// ``` #[stable(feature = "rc_weak", since = "1.4.0")] pub fn upgrade(&self) -> Option> { - if self.strong() == 0 { + let inner = self.inner()?; + if inner.strong() == 0 { None } else { - self.inc_strong(); + inner.inc_strong(); Some(Rc { ptr: self.ptr, phantom: PhantomData }) } } + + /// Return `None` when the pointer is dangling and there is no allocated `RcBox`, + /// i.e. this `Weak` was created by `Weak::new` + #[inline] + fn inner(&self) -> Option<&RcBox> { + if is_dangling(self.ptr) { + None + } else { + Some(unsafe { self.ptr.as_ref() }) + } + } } #[stable(feature = "rc_weak", since = "1.4.0")] @@ -1258,12 +1274,14 @@ impl Drop for Weak { /// assert!(other_weak_foo.upgrade().is_none()); /// ``` fn drop(&mut self) { - unsafe { - self.dec_weak(); + if let Some(inner) = self.inner() { + inner.dec_weak(); // the weak count starts at 1, and will only go to zero if all // the strong pointers have disappeared. - if self.weak() == 0 { - Global.dealloc(self.ptr.cast(), Layout::for_value(self.ptr.as_ref())); + if inner.weak() == 0 { + unsafe { + Global.dealloc(self.ptr.cast(), Layout::for_value(self.ptr.as_ref())); + } } } } @@ -1284,7 +1302,9 @@ impl Clone for Weak { /// ``` #[inline] fn clone(&self) -> Weak { - self.inc_weak(); + if let Some(inner) = self.inner() { + inner.inc_weak() + } Weak { ptr: self.ptr } } } @@ -1317,7 +1337,7 @@ impl Default for Weak { } } -// NOTE: We checked_add here to deal with mem::forget safety. In particular +// NOTE: We checked_add here to deal with mem::forget safely. In particular // if you mem::forget Rcs (or Weaks), the ref-count can overflow, and then // you can free the allocation while outstanding Rcs (or Weaks) exist. // We abort because this is such a degenerate scenario that we don't care about @@ -1370,12 +1390,10 @@ impl RcBoxPtr for Rc { } } -impl RcBoxPtr for Weak { +impl RcBoxPtr for RcBox { #[inline(always)] fn inner(&self) -> &RcBox { - unsafe { - self.ptr.as_ref() - } + self } } diff --git a/src/liballoc/sync.rs b/src/liballoc/sync.rs index 4244b09b18f9c..6710878b31d94 100644 --- a/src/liballoc/sync.rs +++ b/src/liballoc/sync.rs @@ -34,6 +34,7 @@ use core::convert::From; use alloc::{Global, Alloc, Layout, box_free, handle_alloc_error}; use boxed::Box; +use rc::is_dangling; use string::String; use vec::Vec; @@ -43,9 +44,6 @@ use vec::Vec; /// necessarily) at _exactly_ `MAX_REFCOUNT + 1` references. const MAX_REFCOUNT: usize = (isize::MAX) as usize; -/// A sentinel value that is used for the pointer of `Weak::new()`. -const WEAK_EMPTY: usize = 1; - /// A thread-safe reference-counting pointer. 'Arc' stands for 'Atomically /// Reference Counted'. /// @@ -239,9 +237,9 @@ impl, U: ?Sized> CoerceUnsized> for Arc {} #[stable(feature = "arc_weak", since = "1.4.0")] pub struct Weak { // This is a `NonNull` to allow optimizing the size of this type in enums, - // but it is actually not truly "non-null". A `Weak::new()` will set this - // to a sentinel value, instead of needing to allocate some space in the - // heap. + // but it is not necessarily a valid pointer. + // `Weak::new` sets this to a dangling pointer so that it doesn’t need + // to allocate space on the heap. ptr: NonNull>, } @@ -1035,10 +1033,8 @@ impl Weak { /// ``` #[stable(feature = "downgraded_weak", since = "1.10.0")] pub fn new() -> Weak { - unsafe { - Weak { - ptr: NonNull::new_unchecked(WEAK_EMPTY as *mut _), - } + Weak { + ptr: NonNull::dangling(), } } } @@ -1074,11 +1070,7 @@ impl Weak { pub fn upgrade(&self) -> Option> { // We use a CAS loop to increment the strong count instead of a // fetch_add because once the count hits 0 it must never be above 0. - let inner = if self.ptr.as_ptr() as *const u8 as usize == WEAK_EMPTY { - return None; - } else { - unsafe { self.ptr.as_ref() } - }; + let inner = self.inner()?; // Relaxed load because any write of 0 that we can observe // leaves the field in a permanently zero state (so a @@ -1109,6 +1101,17 @@ impl Weak { } } } + + /// Return `None` when the pointer is dangling and there is no allocated `ArcInner`, + /// i.e. this `Weak` was created by `Weak::new` + #[inline] + fn inner(&self) -> Option<&ArcInner> { + if is_dangling(self.ptr) { + None + } else { + Some(unsafe { self.ptr.as_ref() }) + } + } } #[stable(feature = "arc_weak", since = "1.4.0")] @@ -1126,10 +1129,10 @@ impl Clone for Weak { /// ``` #[inline] fn clone(&self) -> Weak { - let inner = if self.ptr.as_ptr() as *const u8 as usize == WEAK_EMPTY { - return Weak { ptr: self.ptr }; + let inner = if let Some(inner) = self.inner() { + inner } else { - unsafe { self.ptr.as_ref() } + return Weak { ptr: self.ptr }; }; // See comments in Arc::clone() for why this is relaxed. This can use a // fetch_add (ignoring the lock) because the weak count is only locked @@ -1204,10 +1207,10 @@ impl Drop for Weak { // weak count can only be locked if there was precisely one weak ref, // meaning that drop could only subsequently run ON that remaining weak // ref, which can only happen after the lock is released. - let inner = if self.ptr.as_ptr() as *const u8 as usize == WEAK_EMPTY { - return; + let inner = if let Some(inner) = self.inner() { + inner } else { - unsafe { self.ptr.as_ref() } + return }; if inner.weak.fetch_sub(1, Release) == 1 { diff --git a/src/liballoc/tests/arc.rs b/src/liballoc/tests/arc.rs new file mode 100644 index 0000000000000..753873dd294ce --- /dev/null +++ b/src/liballoc/tests/arc.rs @@ -0,0 +1,55 @@ +// 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. + +use std::any::Any; +use std::sync::{Arc, Weak}; + +#[test] +fn uninhabited() { + enum Void {} + let mut a = Weak::::new(); + a = a.clone(); + assert!(a.upgrade().is_none()); + + let mut a: Weak = a; // Unsizing + a = a.clone(); + assert!(a.upgrade().is_none()); +} + +#[test] +fn slice() { + let a: Arc<[u32; 3]> = Arc::new([3, 2, 1]); + let a: Arc<[u32]> = a; // Unsizing + let b: Arc<[u32]> = Arc::from(&[3, 2, 1][..]); // Conversion + assert_eq!(a, b); + + // Exercise is_dangling() with a DST + let mut a = Arc::downgrade(&a); + a = a.clone(); + assert!(a.upgrade().is_some()); +} + +#[test] +fn trait_object() { + let a: Arc = Arc::new(4); + let a: Arc = a; // Unsizing + + // Exercise is_dangling() with a DST + let mut a = Arc::downgrade(&a); + a = a.clone(); + assert!(a.upgrade().is_some()); + + let mut b = Weak::::new(); + b = b.clone(); + assert!(b.upgrade().is_none()); + let mut b: Weak = b; // Unsizing + b = b.clone(); + assert!(b.upgrade().is_none()); +} diff --git a/src/liballoc/tests/lib.rs b/src/liballoc/tests/lib.rs index dbac2c0bb18a6..2c361598e8928 100644 --- a/src/liballoc/tests/lib.rs +++ b/src/liballoc/tests/lib.rs @@ -32,12 +32,14 @@ extern crate rand; use std::hash::{Hash, Hasher}; use std::collections::hash_map::DefaultHasher; +mod arc; mod binary_heap; mod btree; mod cow_str; mod fmt; mod heap; mod linked_list; +mod rc; mod slice; mod str; mod string; diff --git a/src/liballoc/tests/rc.rs b/src/liballoc/tests/rc.rs new file mode 100644 index 0000000000000..baa0406acfc3d --- /dev/null +++ b/src/liballoc/tests/rc.rs @@ -0,0 +1,55 @@ +// 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. + +use std::any::Any; +use std::rc::{Rc, Weak}; + +#[test] +fn uninhabited() { + enum Void {} + let mut a = Weak::::new(); + a = a.clone(); + assert!(a.upgrade().is_none()); + + let mut a: Weak = a; // Unsizing + a = a.clone(); + assert!(a.upgrade().is_none()); +} + +#[test] +fn slice() { + let a: Rc<[u32; 3]> = Rc::new([3, 2, 1]); + let a: Rc<[u32]> = a; // Unsizing + let b: Rc<[u32]> = Rc::from(&[3, 2, 1][..]); // Conversion + assert_eq!(a, b); + + // Exercise is_dangling() with a DST + let mut a = Rc::downgrade(&a); + a = a.clone(); + assert!(a.upgrade().is_some()); +} + +#[test] +fn trait_object() { + let a: Rc = Rc::new(4); + let a: Rc = a; // Unsizing + + // Exercise is_dangling() with a DST + let mut a = Rc::downgrade(&a); + a = a.clone(); + assert!(a.upgrade().is_some()); + + let mut b = Weak::::new(); + b = b.clone(); + assert!(b.upgrade().is_none()); + let mut b: Weak = b; // Unsizing + b = b.clone(); + assert!(b.upgrade().is_none()); +} 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..eb57c12ed12c6 --- /dev/null +++ b/src/test/run-pass/weak-new-uninhabited-issue-48493.rs @@ -0,0 +1,15 @@ +// 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(); + std::sync::Weak::::new(); +}