From 3580c4c58940ad5d62a068609b9b696e8eb31309 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Fri, 29 Sep 2017 12:36:32 -0700 Subject: [PATCH 1/5] Remove `T: Sized` on `ptr::is_null()`, `as_ref()`, `as_mut()` `NonZero::is_zero()` was already casting all pointers to thin `*mut u8` to check for null. It seems reasonable to apply that for `is_null()` in general, and then unsized fat pointers can also be used with `as_ref()` and `as_mut()` to get fat references. --- src/libcore/nonzero.rs | 3 +-- src/libcore/ptr.rs | 18 ++++++++------ src/libcore/tests/ptr.rs | 54 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 9 deletions(-) diff --git a/src/libcore/nonzero.rs b/src/libcore/nonzero.rs index f075d825f5d53..8271be5d38f88 100644 --- a/src/libcore/nonzero.rs +++ b/src/libcore/nonzero.rs @@ -28,8 +28,7 @@ macro_rules! impl_zeroable_for_pointer_types { unsafe impl Zeroable for $Ptr { #[inline] fn is_zero(&self) -> bool { - // Cast because `is_null` is only available on thin pointers - (*self as *mut u8).is_null() + (*self).is_null() } } )+ diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index 4041a3760e5ca..dddeda7f02dec 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -487,8 +487,10 @@ impl *const T { /// ``` #[stable(feature = "rust1", since = "1.0.0")] #[inline] - pub fn is_null(self) -> bool where T: Sized { - self == null() + pub fn is_null(self) -> bool { + // Compare via a cast to a thin pointer, so fat pointers are only + // considering their "data" part for null-ness. + (self as *const u8) == null() } /// Returns `None` if the pointer is null, or else returns a reference to @@ -519,7 +521,7 @@ impl *const T { /// ``` #[stable(feature = "ptr_as_ref", since = "1.9.0")] #[inline] - pub unsafe fn as_ref<'a>(self) -> Option<&'a T> where T: Sized { + pub unsafe fn as_ref<'a>(self) -> Option<&'a T> { if self.is_null() { None } else { @@ -1118,8 +1120,10 @@ impl *mut T { /// ``` #[stable(feature = "rust1", since = "1.0.0")] #[inline] - pub fn is_null(self) -> bool where T: Sized { - self == null_mut() + pub fn is_null(self) -> bool { + // Compare via a cast to a thin pointer, so fat pointers are only + // considering their "data" part for null-ness. + (self as *mut u8) == null_mut() } /// Returns `None` if the pointer is null, or else returns a reference to @@ -1150,7 +1154,7 @@ impl *mut T { /// ``` #[stable(feature = "ptr_as_ref", since = "1.9.0")] #[inline] - pub unsafe fn as_ref<'a>(self) -> Option<&'a T> where T: Sized { + pub unsafe fn as_ref<'a>(self) -> Option<&'a T> { if self.is_null() { None } else { @@ -1274,7 +1278,7 @@ impl *mut T { /// ``` #[stable(feature = "ptr_as_ref", since = "1.9.0")] #[inline] - pub unsafe fn as_mut<'a>(self) -> Option<&'a mut T> where T: Sized { + pub unsafe fn as_mut<'a>(self) -> Option<&'a mut T> { if self.is_null() { None } else { diff --git a/src/libcore/tests/ptr.rs b/src/libcore/tests/ptr.rs index c2d53840f8f57..4bd725f828f02 100644 --- a/src/libcore/tests/ptr.rs +++ b/src/libcore/tests/ptr.rs @@ -9,6 +9,7 @@ // except according to those terms. use core::ptr::*; +use core::slice; use core::cell::RefCell; #[test] @@ -62,6 +63,28 @@ fn test_is_null() { let mq = unsafe { mp.offset(1) }; assert!(!mq.is_null()); + + // Pointers to unsized types + let s: &mut [u8] = &mut [1, 2, 3]; + let cs: *const [u8] = s; + assert!(!cs.is_null()); + + let ms: *mut [u8] = s; + assert!(!ms.is_null()); + + let cz: *const [u8] = &[]; + assert!(!cz.is_null()); + + let mz: *mut [u8] = &mut []; + assert!(!mz.is_null()); + + unsafe { + let ncs: *const [u8] = slice::from_raw_parts(null(), 0); + assert!(ncs.is_null()); + + let nms: *mut [u8] = slice::from_raw_parts_mut(null_mut(), 0); + assert!(nms.is_null()); + } } #[test] @@ -85,6 +108,26 @@ fn test_as_ref() { let p = &u as *const isize; assert_eq!(p.as_ref().unwrap(), &2); } + + // Pointers to unsized types + let s: &mut [u8] = &mut [1, 2, 3]; + let cs: *const [u8] = s; + assert_eq!(cs.as_ref(), Some(&*s)); + + let ms: *mut [u8] = s; + assert_eq!(ms.as_ref(), Some(&*s)); + + let cz: *const [u8] = &[]; + assert_eq!(cz.as_ref(), Some(&[][..])); + + let mz: *mut [u8] = &mut []; + assert_eq!(mz.as_ref(), Some(&[][..])); + + let ncs: *const [u8] = slice::from_raw_parts(null(), 0); + assert_eq!(ncs.as_ref(), None); + + let nms: *mut [u8] = slice::from_raw_parts_mut(null_mut(), 0); + assert_eq!(nms.as_ref(), None); } } @@ -103,6 +146,17 @@ fn test_as_mut() { let p = &mut u as *mut isize; assert!(p.as_mut().unwrap() == &mut 2); } + + // Pointers to unsized types + let s: &mut [u8] = &mut [1, 2, 3]; + let ms: *mut [u8] = s; + assert_eq!(ms.as_mut(), Some(s)); + + let mz: *mut [u8] = &mut []; + assert_eq!(mz.as_mut(), Some(&mut [][..])); + + let nms: *mut [u8] = slice::from_raw_parts_mut(null_mut(), 0); + assert_eq!(nms.as_mut(), None); } } From 8b2e09ffbbb69402f8881824f8689f785695a780 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Fri, 29 Sep 2017 13:19:20 -0700 Subject: [PATCH 2/5] Avoid the UB of a null reference to a slice --- src/libcore/tests/ptr.rs | 34 ++++++++++++++++++++++++---------- 1 file changed, 24 insertions(+), 10 deletions(-) diff --git a/src/libcore/tests/ptr.rs b/src/libcore/tests/ptr.rs index 4bd725f828f02..6af74eb532b9d 100644 --- a/src/libcore/tests/ptr.rs +++ b/src/libcore/tests/ptr.rs @@ -8,10 +8,26 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +use core::mem; use core::ptr::*; -use core::slice; use core::cell::RefCell; + +/// Create a null pointer to a mutable slice. This is implemented like +/// `slice::from_raw_parts_mut`, which we can't use directly because +/// having a null `&mut [T]` even temporarily is UB. +fn null_slice() -> *mut [T] { + unsafe { + #[repr(C)] + struct Repr { + pub data: *mut T, + pub len: usize, + } + + mem::transmute(Repr { data: null_mut::(), len: 0 }) + } +} + #[test] fn test() { unsafe { @@ -78,13 +94,11 @@ fn test_is_null() { let mz: *mut [u8] = &mut []; assert!(!mz.is_null()); - unsafe { - let ncs: *const [u8] = slice::from_raw_parts(null(), 0); - assert!(ncs.is_null()); + let ncs: *const [u8] = null_slice(); + assert!(ncs.is_null()); - let nms: *mut [u8] = slice::from_raw_parts_mut(null_mut(), 0); - assert!(nms.is_null()); - } + let nms: *mut [u8] = null_slice(); + assert!(nms.is_null()); } #[test] @@ -123,10 +137,10 @@ fn test_as_ref() { let mz: *mut [u8] = &mut []; assert_eq!(mz.as_ref(), Some(&[][..])); - let ncs: *const [u8] = slice::from_raw_parts(null(), 0); + let ncs: *const [u8] = null_slice(); assert_eq!(ncs.as_ref(), None); - let nms: *mut [u8] = slice::from_raw_parts_mut(null_mut(), 0); + let nms: *mut [u8] = null_slice(); assert_eq!(nms.as_ref(), None); } } @@ -155,7 +169,7 @@ fn test_as_mut() { let mz: *mut [u8] = &mut []; assert_eq!(mz.as_mut(), Some(&mut [][..])); - let nms: *mut [u8] = slice::from_raw_parts_mut(null_mut(), 0); + let nms: *mut [u8] = null_slice(); assert_eq!(nms.as_mut(), None); } } From 5c6118339a6d143983c0096b3053aefca694352b Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Fri, 29 Sep 2017 13:59:53 -0700 Subject: [PATCH 3/5] Document that there are many possible null pointers --- src/libcore/ptr.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index dddeda7f02dec..474658b65a154 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -476,6 +476,11 @@ pub unsafe fn write_volatile(dst: *mut T, src: T) { impl *const T { /// Returns `true` if the pointer is null. /// + /// Note that unsized types have many possible null pointers, as only the + /// raw data pointer is considered, not their length, vtable, etc. + /// Therefore, two pointers that are null may still not compare equal to + /// each other. + /// /// # Examples /// /// Basic usage: @@ -1109,6 +1114,11 @@ impl *const T { impl *mut T { /// Returns `true` if the pointer is null. /// + /// Note that unsized types have many possible null pointers, as only the + /// raw data pointer is considered, not their length, vtable, etc. + /// Therefore, two pointers that are null may still not compare equal to + /// each other. + /// /// # Examples /// /// Basic usage: From 40a678d8dbaea49a2e9f5451f4db68359a09f67a Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Sat, 30 Sep 2017 14:45:18 -0700 Subject: [PATCH 4/5] Use unsized coercions for null ptr tests --- src/libcore/tests/ptr.rs | 66 +++++++++++++++++++++++++--------------- 1 file changed, 41 insertions(+), 25 deletions(-) diff --git a/src/libcore/tests/ptr.rs b/src/libcore/tests/ptr.rs index 6af74eb532b9d..98436f0e1d1cd 100644 --- a/src/libcore/tests/ptr.rs +++ b/src/libcore/tests/ptr.rs @@ -8,26 +8,9 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use core::mem; use core::ptr::*; use core::cell::RefCell; - -/// Create a null pointer to a mutable slice. This is implemented like -/// `slice::from_raw_parts_mut`, which we can't use directly because -/// having a null `&mut [T]` even temporarily is UB. -fn null_slice() -> *mut [T] { - unsafe { - #[repr(C)] - struct Repr { - pub data: *mut T, - pub len: usize, - } - - mem::transmute(Repr { data: null_mut::(), len: 0 }) - } -} - #[test] fn test() { unsafe { @@ -80,7 +63,7 @@ fn test_is_null() { let mq = unsafe { mp.offset(1) }; assert!(!mq.is_null()); - // Pointers to unsized types + // Pointers to unsized types -- slices let s: &mut [u8] = &mut [1, 2, 3]; let cs: *const [u8] = s; assert!(!cs.is_null()); @@ -94,11 +77,24 @@ fn test_is_null() { let mz: *mut [u8] = &mut []; assert!(!mz.is_null()); - let ncs: *const [u8] = null_slice(); + let ncs: *const [u8] = null::<[u8; 3]>(); assert!(ncs.is_null()); - let nms: *mut [u8] = null_slice(); + let nms: *mut [u8] = null_mut::<[u8; 3]>(); assert!(nms.is_null()); + + // Pointers to unsized types -- trait objects + let ci: *const ToString = &3; + assert!(!ci.is_null()); + + let mi: *mut ToString = &mut 3; + assert!(!mi.is_null()); + + let nci: *const ToString = null::(); + assert!(nci.is_null()); + + let nmi: *mut ToString = null_mut::(); + assert!(nmi.is_null()); } #[test] @@ -123,7 +119,7 @@ fn test_as_ref() { assert_eq!(p.as_ref().unwrap(), &2); } - // Pointers to unsized types + // Pointers to unsized types -- slices let s: &mut [u8] = &mut [1, 2, 3]; let cs: *const [u8] = s; assert_eq!(cs.as_ref(), Some(&*s)); @@ -137,11 +133,24 @@ fn test_as_ref() { let mz: *mut [u8] = &mut []; assert_eq!(mz.as_ref(), Some(&[][..])); - let ncs: *const [u8] = null_slice(); + let ncs: *const [u8] = null::<[u8; 3]>(); assert_eq!(ncs.as_ref(), None); - let nms: *mut [u8] = null_slice(); + let nms: *mut [u8] = null_mut::<[u8; 3]>(); assert_eq!(nms.as_ref(), None); + + // Pointers to unsized types -- trait objects + let ci: *const ToString = &3; + assert!(ci.as_ref().is_some()); + + let mi: *mut ToString = &mut 3; + assert!(mi.as_ref().is_some()); + + let nci: *const ToString = null::(); + assert!(nci.as_ref().is_none()); + + let nmi: *mut ToString = null_mut::(); + assert!(nmi.as_ref().is_none()); } } @@ -161,7 +170,7 @@ fn test_as_mut() { assert!(p.as_mut().unwrap() == &mut 2); } - // Pointers to unsized types + // Pointers to unsized types -- slices let s: &mut [u8] = &mut [1, 2, 3]; let ms: *mut [u8] = s; assert_eq!(ms.as_mut(), Some(s)); @@ -169,8 +178,15 @@ fn test_as_mut() { let mz: *mut [u8] = &mut []; assert_eq!(mz.as_mut(), Some(&mut [][..])); - let nms: *mut [u8] = null_slice(); + let nms: *mut [u8] = null_mut::<[u8; 3]>(); assert_eq!(nms.as_mut(), None); + + // Pointers to unsized types -- trait objects + let mi: *mut ToString = &mut 3; + assert!(mi.as_mut().is_some()); + + let nmi: *mut ToString = null_mut::(); + assert!(nmi.as_mut().is_none()); } } From 604f049cd5060129cf14f7bd340d442811345ea8 Mon Sep 17 00:00:00 2001 From: Josh Stone Date: Tue, 10 Oct 2017 11:35:41 -0700 Subject: [PATCH 5/5] Restore `T: Sized` on `ptr::is_null` The exact semantics of `is_null` on unsized pointers are still debatable, especially for trait objects. It may be legal to call `*mut self` methods on a trait object someday, as with Go interfaces, so `is_null` might need to validate the vtable pointer too. For `as_ref` and `as_mut`, we're assuming that you cannot have a non-null data pointer with a null vtable, so casting the unsized check is fine. --- src/libcore/nonzero.rs | 3 ++- src/libcore/ptr.rs | 34 +++++++++++++--------------------- src/libcore/tests/ptr.rs | 33 --------------------------------- 3 files changed, 15 insertions(+), 55 deletions(-) diff --git a/src/libcore/nonzero.rs b/src/libcore/nonzero.rs index 8271be5d38f88..f075d825f5d53 100644 --- a/src/libcore/nonzero.rs +++ b/src/libcore/nonzero.rs @@ -28,7 +28,8 @@ macro_rules! impl_zeroable_for_pointer_types { unsafe impl Zeroable for $Ptr { #[inline] fn is_zero(&self) -> bool { - (*self).is_null() + // Cast because `is_null` is only available on thin pointers + (*self as *mut u8).is_null() } } )+ diff --git a/src/libcore/ptr.rs b/src/libcore/ptr.rs index 474658b65a154..7d0cdc27571f5 100644 --- a/src/libcore/ptr.rs +++ b/src/libcore/ptr.rs @@ -476,11 +476,6 @@ pub unsafe fn write_volatile(dst: *mut T, src: T) { impl *const T { /// Returns `true` if the pointer is null. /// - /// Note that unsized types have many possible null pointers, as only the - /// raw data pointer is considered, not their length, vtable, etc. - /// Therefore, two pointers that are null may still not compare equal to - /// each other. - /// /// # Examples /// /// Basic usage: @@ -492,10 +487,8 @@ impl *const T { /// ``` #[stable(feature = "rust1", since = "1.0.0")] #[inline] - pub fn is_null(self) -> bool { - // Compare via a cast to a thin pointer, so fat pointers are only - // considering their "data" part for null-ness. - (self as *const u8) == null() + pub fn is_null(self) -> bool where T: Sized { + self == null() } /// Returns `None` if the pointer is null, or else returns a reference to @@ -527,7 +520,9 @@ impl *const T { #[stable(feature = "ptr_as_ref", since = "1.9.0")] #[inline] pub unsafe fn as_ref<'a>(self) -> Option<&'a T> { - if self.is_null() { + // Check for null via a cast to a thin pointer, so fat pointers are only + // considering their "data" part for null-ness. + if (self as *const u8).is_null() { None } else { Some(&*self) @@ -1114,11 +1109,6 @@ impl *const T { impl *mut T { /// Returns `true` if the pointer is null. /// - /// Note that unsized types have many possible null pointers, as only the - /// raw data pointer is considered, not their length, vtable, etc. - /// Therefore, two pointers that are null may still not compare equal to - /// each other. - /// /// # Examples /// /// Basic usage: @@ -1130,10 +1120,8 @@ impl *mut T { /// ``` #[stable(feature = "rust1", since = "1.0.0")] #[inline] - pub fn is_null(self) -> bool { - // Compare via a cast to a thin pointer, so fat pointers are only - // considering their "data" part for null-ness. - (self as *mut u8) == null_mut() + pub fn is_null(self) -> bool where T: Sized { + self == null_mut() } /// Returns `None` if the pointer is null, or else returns a reference to @@ -1165,7 +1153,9 @@ impl *mut T { #[stable(feature = "ptr_as_ref", since = "1.9.0")] #[inline] pub unsafe fn as_ref<'a>(self) -> Option<&'a T> { - if self.is_null() { + // Check for null via a cast to a thin pointer, so fat pointers are only + // considering their "data" part for null-ness. + if (self as *const u8).is_null() { None } else { Some(&*self) @@ -1289,7 +1279,9 @@ impl *mut T { #[stable(feature = "ptr_as_ref", since = "1.9.0")] #[inline] pub unsafe fn as_mut<'a>(self) -> Option<&'a mut T> { - if self.is_null() { + // Check for null via a cast to a thin pointer, so fat pointers are only + // considering their "data" part for null-ness. + if (self as *mut u8).is_null() { None } else { Some(&mut *self) diff --git a/src/libcore/tests/ptr.rs b/src/libcore/tests/ptr.rs index 98436f0e1d1cd..e93e9be0cd50b 100644 --- a/src/libcore/tests/ptr.rs +++ b/src/libcore/tests/ptr.rs @@ -62,39 +62,6 @@ fn test_is_null() { let mq = unsafe { mp.offset(1) }; assert!(!mq.is_null()); - - // Pointers to unsized types -- slices - let s: &mut [u8] = &mut [1, 2, 3]; - let cs: *const [u8] = s; - assert!(!cs.is_null()); - - let ms: *mut [u8] = s; - assert!(!ms.is_null()); - - let cz: *const [u8] = &[]; - assert!(!cz.is_null()); - - let mz: *mut [u8] = &mut []; - assert!(!mz.is_null()); - - let ncs: *const [u8] = null::<[u8; 3]>(); - assert!(ncs.is_null()); - - let nms: *mut [u8] = null_mut::<[u8; 3]>(); - assert!(nms.is_null()); - - // Pointers to unsized types -- trait objects - let ci: *const ToString = &3; - assert!(!ci.is_null()); - - let mi: *mut ToString = &mut 3; - assert!(!mi.is_null()); - - let nci: *const ToString = null::(); - assert!(nci.is_null()); - - let nmi: *mut ToString = null_mut::(); - assert!(nmi.is_null()); } #[test]