From b5b6760c03867e2b16862324b4764cf35be8a1cd Mon Sep 17 00:00:00 2001 From: CAD97 Date: Sun, 10 Jan 2021 23:27:32 -0500 Subject: [PATCH] Weak::into_raw shouldn't translate sentinel value --- library/alloc/src/rc.rs | 27 ++++++++++++--------------- library/alloc/src/sync.rs | 21 +++++++++------------ 2 files changed, 21 insertions(+), 27 deletions(-) diff --git a/library/alloc/src/rc.rs b/library/alloc/src/rc.rs index b9f3f357c1a51..43affa396c9d7 100644 --- a/library/alloc/src/rc.rs +++ b/library/alloc/src/rc.rs @@ -848,7 +848,7 @@ impl Rc { pub fn downgrade(this: &Self) -> Weak { this.inner().inc_weak(); // Make sure we do not create a dangling Weak - debug_assert!(!is_dangling(this.ptr)); + debug_assert!(!is_dangling(this.ptr.as_ptr())); Weak { ptr: this.ptr } } @@ -1837,8 +1837,8 @@ impl Weak { } } -pub(crate) fn is_dangling(ptr: NonNull) -> bool { - let address = ptr.as_ptr() as *mut () as usize; +pub(crate) fn is_dangling(ptr: *mut T) -> bool { + let address = ptr as *mut () as usize; address == usize::MAX } @@ -1879,17 +1879,15 @@ impl Weak { pub fn as_ptr(&self) -> *const T { let ptr: *mut RcBox = NonNull::as_ptr(self.ptr); - if is_dangling(self.ptr) { - // If the pointer is dangling, we return a null pointer as the dangling sentinel. - // We can't return the usize::MAX sentinel, as that could valid if T is ZST. - // SAFETY: we have to return a known sentinel here that cannot be produced for - // a valid pointer, so that `from_raw` can reverse this transformation. - (ptr as *mut T).set_ptr_value(ptr::null_mut()) + if is_dangling(ptr) { + // If the pointer is dangling, we return the sentinel directly. This cannot be + // a valid payload address, as it is at least as aligned as RcBox (usize). + ptr as *const T } else { // SAFETY: if is_dangling returns false, then the pointer is dereferencable. // The payload may be dropped at this point, and we have to maintain provenance, // so use raw pointer manipulation. - unsafe { &raw mut (*ptr).value } + unsafe { &raw const (*ptr).value } } } @@ -1973,10 +1971,9 @@ impl Weak { pub unsafe fn from_raw(ptr: *const T) -> Self { // See Weak::as_ptr for context on how the input pointer is derived. - let ptr = if ptr.is_null() { - // If we get a null pointer, this is a dangling weak. - // SAFETY: this is the same sentinel as used in Weak::new and is_dangling - (ptr as *mut RcBox).set_ptr_value(usize::MAX as *mut _) + let ptr = if is_dangling(ptr as *mut T) { + // This is a dangling Weak. + ptr as *mut RcBox } else { // Otherwise, we're guaranteed the pointer came from a nondangling Weak. // SAFETY: data_offset is safe to call, as ptr references a real (potentially dropped) T. @@ -2052,7 +2049,7 @@ impl Weak { /// (i.e., when this `Weak` was created by `Weak::new`). #[inline] fn inner(&self) -> Option> { - if is_dangling(self.ptr) { + if is_dangling(self.ptr.as_ptr()) { None } else { // We are careful to *not* create a reference covering the "data" field, as diff --git a/library/alloc/src/sync.rs b/library/alloc/src/sync.rs index c50f5270a4d10..8917e2d4b400a 100644 --- a/library/alloc/src/sync.rs +++ b/library/alloc/src/sync.rs @@ -885,7 +885,7 @@ impl Arc { match this.inner().weak.compare_exchange_weak(cur, cur + 1, Acquire, Relaxed) { Ok(_) => { // Make sure we do not create a dangling Weak - debug_assert!(!is_dangling(this.ptr)); + debug_assert!(!is_dangling(this.ptr.as_ptr())); return Weak { ptr: this.ptr }; } Err(old) => cur = old, @@ -1664,12 +1664,10 @@ impl Weak { pub fn as_ptr(&self) -> *const T { let ptr: *mut ArcInner = NonNull::as_ptr(self.ptr); - if is_dangling(self.ptr) { - // If the pointer is dangling, we return a null pointer as the dangling sentinel. - // We can't return the usize::MAX sentinel, as that could valid if T is ZST. - // SAFETY: we have to return a known sentinel here that cannot be produced for - // a valid pointer, so that `from_raw` can reverse this transformation. - (ptr as *mut T).set_ptr_value(ptr::null_mut()) + if is_dangling(ptr) { + // If the pointer is dangling, we return the sentinel directly. This cannot be + // a valid payload address, as it is at least as aligned as ArcInner (usize). + ptr as *const T } else { // SAFETY: if is_dangling returns false, then the pointer is dereferencable. // The payload may be dropped at this point, and we have to maintain provenance, @@ -1758,10 +1756,9 @@ impl Weak { pub unsafe fn from_raw(ptr: *const T) -> Self { // See Weak::as_ptr for context on how the input pointer is derived. - let ptr = if ptr.is_null() { - // If we get a null pointer, this is a dangling weak. - // SAFETY: this is the same sentinel as used in Weak::new and is_dangling - (ptr as *mut ArcInner).set_ptr_value(usize::MAX as *mut _) + let ptr = if is_dangling(ptr as *mut T) { + // This is a dangling Weak. + ptr as *mut ArcInner } else { // Otherwise, we're guaranteed the pointer came from a nondangling Weak. // SAFETY: data_offset is safe to call, as ptr references a real (potentially dropped) T. @@ -1877,7 +1874,7 @@ impl Weak { /// (i.e., when this `Weak` was created by `Weak::new`). #[inline] fn inner(&self) -> Option> { - if is_dangling(self.ptr) { + if is_dangling(self.ptr.as_ptr()) { None } else { // We are careful to *not* create a reference covering the "data" field, as