From becebf3106407b892f25807473e23d6e8f116a1e Mon Sep 17 00:00:00 2001 From: Lukas Lueg Date: Fri, 10 Jan 2020 19:39:01 +0100 Subject: [PATCH 1/3] Ammend Rc/Arc::from_raw() docs regarding unsafety Constructing an Rc/Arc is unsafe even if the wrapped `T` is never dereferenced. --- src/liballoc/rc.rs | 7 ++++--- src/liballoc/sync.rs | 7 ++++--- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index 3080a8bf45966..60a8e1714b644 100644 --- a/src/liballoc/rc.rs +++ b/src/liballoc/rc.rs @@ -573,10 +573,11 @@ impl Rc { /// Constructs an `Rc` from a raw pointer. /// /// The raw pointer must have been previously returned by a call to a - /// [`Rc::into_raw`][into_raw]. + /// [`Rc::into_raw`][into_raw] using the same `T`. /// - /// This function is unsafe because improper use may lead to memory problems. For example, a - /// double-free may occur if the function is called twice on the same raw pointer. + /// This function is unsafe because improper use may lead to memory unsafety, + /// even if `T` is never accessed. For example, a double-free may occur if the function is + /// called twice on the same raw pointer. /// /// [into_raw]: struct.Rc.html#method.into_raw /// diff --git a/src/liballoc/sync.rs b/src/liballoc/sync.rs index dc53ad2840727..024f9407604ff 100644 --- a/src/liballoc/sync.rs +++ b/src/liballoc/sync.rs @@ -553,10 +553,11 @@ impl Arc { /// Constructs an `Arc` from a raw pointer. /// /// The raw pointer must have been previously returned by a call to a - /// [`Arc::into_raw`][into_raw]. + /// [`Arc::into_raw`][into_raw], using the same `T`. /// - /// This function is unsafe because improper use may lead to memory problems. For example, a - /// double-free may occur if the function is called twice on the same raw pointer. + /// This function is unsafe because improper use may lead to memory unsafety, + /// even if `T` is never accessed. For example, a double-free may occur if the function is + /// called twice on the same raw pointer. /// /// [into_raw]: struct.Arc.html#method.into_raw /// From b4c96a9199f13c5c1c2afa6258d2b9206c151d9f Mon Sep 17 00:00:00 2001 From: Lukas Lueg Date: Tue, 28 Jan 2020 22:28:13 +0100 Subject: [PATCH 2/3] Refine [Arc/Rc]::from_raw() docs --- src/liballoc/rc.rs | 18 +++++++++++++----- src/liballoc/sync.rs | 18 +++++++++++++----- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index 60a8e1714b644..1d2222adb9da4 100644 --- a/src/liballoc/rc.rs +++ b/src/liballoc/rc.rs @@ -570,16 +570,24 @@ impl Rc { ptr } - /// Constructs an `Rc` from a raw pointer. + /// Constructs an `Rc` from a raw pointer. /// - /// The raw pointer must have been previously returned by a call to a - /// [`Rc::into_raw`][into_raw] using the same `T`. + /// The raw pointer must have been previously returned by a call to + /// [`Rc::into_raw`][into_raw] where `U` must have the same size + /// and alignment as `T`. This is trivially true if `U` is `T`. + /// Note that if `U` is not `T` but has the same size and alignment, this is + /// basically like transmuting references of different types. See + /// [`mem::transmute`][transmute] for more information on what + /// restrictions apply in this case. + /// + /// The user of `from_raw` has to make sure a specific value of `T` is only + /// dropped once. /// /// This function is unsafe because improper use may lead to memory unsafety, - /// even if `T` is never accessed. For example, a double-free may occur if the function is - /// called twice on the same raw pointer. + /// even if the returned `Rc` is never accessed. /// /// [into_raw]: struct.Rc.html#method.into_raw + /// [transmute]: ../../std/mem/fn.transmute.html /// /// # Examples /// diff --git a/src/liballoc/sync.rs b/src/liballoc/sync.rs index 024f9407604ff..f9c8da58c75c7 100644 --- a/src/liballoc/sync.rs +++ b/src/liballoc/sync.rs @@ -550,16 +550,24 @@ impl Arc { ptr } - /// Constructs an `Arc` from a raw pointer. + /// Constructs an `Arc` from a raw pointer. /// - /// The raw pointer must have been previously returned by a call to a - /// [`Arc::into_raw`][into_raw], using the same `T`. + /// The raw pointer must have been previously returned by a call to + /// [`Arc::into_raw`][into_raw] where `U` must have the same size and + /// alignment as `T`. This is trivially true if `U` is `T`. + /// Note that if `U` is not `T` but has the same size and alignment, this is + /// basically like transmuting references of different types. See + /// [`mem::transmute`][transmute] for more information on what + /// restrictions apply in this case. + /// + /// The user of `from_raw` has to make sure a specific value of `T` is only + /// dropped once. /// /// This function is unsafe because improper use may lead to memory unsafety, - /// even if `T` is never accessed. For example, a double-free may occur if the function is - /// called twice on the same raw pointer. + /// even if the returned `Arc` is never accessed. /// /// [into_raw]: struct.Arc.html#method.into_raw + /// [transmute]: ../../std/mem/fn.transmute.html /// /// # Examples /// From 586c7e3907738938db7a6730fd70d7125f5925fa Mon Sep 17 00:00:00 2001 From: Lukas Lueg Date: Mon, 3 Feb 2020 18:14:31 +0100 Subject: [PATCH 3/3] Make rc::RcBox and sync::ArcInner repr(C) Future-proof these types in case rustc reorders the inner fields. As per discussion in PR #68099. --- src/liballoc/rc.rs | 4 ++++ src/liballoc/sync.rs | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/src/liballoc/rc.rs b/src/liballoc/rc.rs index 1d2222adb9da4..751efe0e71ec1 100644 --- a/src/liballoc/rc.rs +++ b/src/liballoc/rc.rs @@ -259,6 +259,10 @@ use crate::vec::Vec; #[cfg(test)] mod tests; +// This is repr(C) to future-proof against possible field-reordering, which +// would interfere with otherwise safe [into|from]_raw() of transmutable +// inner types. +#[repr(C)] struct RcBox { strong: Cell, weak: Cell, diff --git a/src/liballoc/sync.rs b/src/liballoc/sync.rs index f9c8da58c75c7..b44d78f31ec9d 100644 --- a/src/liballoc/sync.rs +++ b/src/liballoc/sync.rs @@ -270,6 +270,10 @@ impl fmt::Debug for Weak { } } +// This is repr(C) to future-proof against possible field-reordering, which +// would interfere with otherwise safe [into|from]_raw() of transmutable +// inner types. +#[repr(C)] struct ArcInner { strong: atomic::AtomicUsize,