From 34c3c0dae5389b345e2a53991425972c02a98abc Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Thu, 30 Jul 2020 12:27:34 +0200 Subject: [PATCH 1/2] Make `<*const T>::is_null` const fn --- library/core/src/ptr/const_ptr.rs | 14 ++++- library/core/src/ptr/mut_ptr.rs | 14 ++++- src/librustc_mir/interpret/intrinsics.rs | 42 ++++++++++++- src/test/ui/consts/ptr_comparisons.rs | 74 +++++++++++++++++++++++ src/test/ui/consts/ptr_comparisons.stderr | 47 ++++++++++++++ src/test/ui/consts/ptr_is_null.rs | 16 +++++ 6 files changed, 200 insertions(+), 7 deletions(-) create mode 100644 src/test/ui/consts/ptr_comparisons.rs create mode 100644 src/test/ui/consts/ptr_comparisons.stderr create mode 100644 src/test/ui/consts/ptr_is_null.rs diff --git a/library/core/src/ptr/const_ptr.rs b/library/core/src/ptr/const_ptr.rs index a16970e9fd180..edd29f6a729cb 100644 --- a/library/core/src/ptr/const_ptr.rs +++ b/library/core/src/ptr/const_ptr.rs @@ -13,6 +13,15 @@ impl *const T { /// Therefore, two pointers that are null may still not compare equal to /// each other. /// + /// ## Behavior during const evaluation + /// + /// When this function is used during const evaluation, it may return `false` for pointers + /// that turn out to be null at runtime. Specifically, when a pointer to some memory + /// is offset beyond its bounds in such a way that the resulting pointer is null, + /// the function will still return `false`. There is no way for CTFE to know + /// the absolute position of that memory, so we cannot tell if the pointer is + /// null or not. + /// /// # Examples /// /// Basic usage: @@ -23,11 +32,12 @@ impl *const T { /// assert!(!ptr.is_null()); /// ``` #[stable(feature = "rust1", since = "1.0.0")] + #[rustc_const_unstable(feature = "const_ptr_is_null", issue = "74939")] #[inline] - pub fn is_null(self) -> bool { + pub const 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() + (self as *const u8).guaranteed_eq(null()) } /// Casts to a pointer of another type. diff --git a/library/core/src/ptr/mut_ptr.rs b/library/core/src/ptr/mut_ptr.rs index b47f90c599629..09438c89b6ab3 100644 --- a/library/core/src/ptr/mut_ptr.rs +++ b/library/core/src/ptr/mut_ptr.rs @@ -12,6 +12,15 @@ impl *mut T { /// Therefore, two pointers that are null may still not compare equal to /// each other. /// + /// ## Behavior during const evaluation + /// + /// When this function is used during const evaluation, it may return `false` for pointers + /// that turn out to be null at runtime. Specifically, when a pointer to some memory + /// is offset beyond its bounds in such a way that the resulting pointer is null, + /// the function will still return `false`. There is no way for CTFE to know + /// the absolute position of that memory, so we cannot tell if the pointer is + /// null or not. + /// /// # Examples /// /// Basic usage: @@ -22,11 +31,12 @@ impl *mut T { /// assert!(!ptr.is_null()); /// ``` #[stable(feature = "rust1", since = "1.0.0")] + #[rustc_const_unstable(feature = "const_ptr_is_null", issue = "74939")] #[inline] - pub fn is_null(self) -> bool { + pub const 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() + (self as *mut u8).guaranteed_eq(null_mut()) } /// Casts to a pointer of another type. diff --git a/src/librustc_mir/interpret/intrinsics.rs b/src/librustc_mir/interpret/intrinsics.rs index b45045716d18c..2c0a42a9bf326 100644 --- a/src/librustc_mir/interpret/intrinsics.rs +++ b/src/librustc_mir/interpret/intrinsics.rs @@ -329,9 +329,14 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { self.write_scalar(offset_ptr, dest)?; } sym::ptr_guaranteed_eq | sym::ptr_guaranteed_ne => { - // FIXME: return `true` for at least some comparisons where we can reliably - // determine the result of runtime (in)equality tests at compile-time. - self.write_scalar(Scalar::from_bool(false), dest)?; + let a = self.read_immediate(args[0])?.to_scalar()?; + let b = self.read_immediate(args[1])?.to_scalar()?; + let cmp = if intrinsic_name == sym::ptr_guaranteed_eq { + self.guaranteed_eq(a, b) + } else { + self.guaranteed_ne(a, b) + }; + self.write_scalar(Scalar::from_bool(cmp), dest)?; } sym::ptr_offset_from => { let a = self.read_immediate(args[0])?.to_scalar()?; @@ -448,6 +453,37 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { Ok(true) } + fn guaranteed_eq(&mut self, a: Scalar, b: Scalar) -> bool { + match (a, b) { + // Comparisons between integers are always known. + (Scalar::Raw { .. }, Scalar::Raw { .. }) => a == b, + // Equality with integers can never be known for sure. + (Scalar::Raw { .. }, Scalar::Ptr(_)) | (Scalar::Ptr(_), Scalar::Raw { .. }) => false, + // FIXME: return `true` for when both sides are the same pointer, *except* that + // some things (like functions and vtables) do not have stable addresses + // so we need to be careful around them. + (Scalar::Ptr(_), Scalar::Ptr(_)) => false, + } + } + + fn guaranteed_ne(&mut self, a: Scalar, b: Scalar) -> bool { + match (a, b) { + // Comparisons between integers are always known. + (Scalar::Raw { .. }, Scalar::Raw { .. }) => a != b, + // Comparisons of abstract pointers with null pointers are known if the pointer + // is in bounds, because if they are in bounds, the pointer can't be null. + (Scalar::Raw { data: 0, .. }, Scalar::Ptr(ptr)) + | (Scalar::Ptr(ptr), Scalar::Raw { data: 0, .. }) => !self.memory.ptr_may_be_null(ptr), + // Inequality with integers other than null can never be known for sure. + (Scalar::Raw { .. }, Scalar::Ptr(_)) | (Scalar::Ptr(_), Scalar::Raw { .. }) => false, + // FIXME: return `true` for at least some comparisons where we can reliably + // determine the result of runtime inequality tests at compile-time. + // Examples include comparison of addresses in static items, for these we can + // give reliable results. + (Scalar::Ptr(_), Scalar::Ptr(_)) => false, + } + } + pub fn exact_div( &mut self, a: ImmTy<'tcx, M::PointerTag>, diff --git a/src/test/ui/consts/ptr_comparisons.rs b/src/test/ui/consts/ptr_comparisons.rs new file mode 100644 index 0000000000000..de7aeaad805cd --- /dev/null +++ b/src/test/ui/consts/ptr_comparisons.rs @@ -0,0 +1,74 @@ +// compile-flags: --crate-type=lib + +#![feature( + const_panic, + core_intrinsics, + const_raw_ptr_comparison, + const_ptr_offset, + const_raw_ptr_deref, + raw_ref_macros +)] + +const FOO: &usize = &42; + +macro_rules! check { + (eq, $a:expr, $b:expr) => { + pub const _: () = + assert!(std::intrinsics::ptr_guaranteed_eq($a as *const u8, $b as *const u8)); + }; + (ne, $a:expr, $b:expr) => { + pub const _: () = + assert!(std::intrinsics::ptr_guaranteed_ne($a as *const u8, $b as *const u8)); + }; + (!eq, $a:expr, $b:expr) => { + pub const _: () = + assert!(!std::intrinsics::ptr_guaranteed_eq($a as *const u8, $b as *const u8)); + }; + (!ne, $a:expr, $b:expr) => { + pub const _: () = + assert!(!std::intrinsics::ptr_guaranteed_ne($a as *const u8, $b as *const u8)); + }; +} + +check!(eq, 0, 0); +check!(ne, 0, 1); +check!(!eq, 0, 1); +check!(!ne, 0, 0); +check!(ne, FOO as *const _, 0); +check!(!eq, FOO as *const _, 0); +// We want pointers to be equal to themselves, but aren't checking this yet because +// there are some open questions (e.g. whether function pointers to the same function +// compare equal, they don't necessarily at runtime). +// The case tested here should work eventually, but does not work yet. +check!(!eq, FOO as *const _, FOO as *const _); +check!(ne, unsafe { (FOO as *const usize).offset(1) }, 0); +check!(!eq, unsafe { (FOO as *const usize).offset(1) }, 0); + +check!(ne, unsafe { (FOO as *const usize as *const u8).offset(3) }, 0); +check!(!eq, unsafe { (FOO as *const usize as *const u8).offset(3) }, 0); + +/////////////////////////////////////////////////////////////////////////////// +// If any of the below start compiling, make sure to add a `check` test for it. +// These invocations exist as canaries so we don't forget to check that the +// behaviour of `guaranteed_eq` and `guaranteed_ne` is still correct. +// All of these try to obtain an out of bounds pointer in some manner. If we +// can create out of bounds pointers, we can offset a pointer far enough that +// at runtime it would be zero and at compile-time it would not be zero. + +const _: *const usize = unsafe { (FOO as *const usize).offset(2) }; +//~^ NOTE + +const _: *const u8 = +//~^ NOTE + unsafe { std::ptr::raw_const!((*(FOO as *const usize as *const [u8; 1000]))[999]) }; +//~^ ERROR any use of this value will cause an error + +const _: usize = unsafe { std::mem::transmute::<*const usize, usize>(FOO) + 4 }; +//~^ ERROR any use of this value will cause an error +//~| NOTE "pointer-to-integer cast" needs an rfc +//~| NOTE + +const _: usize = unsafe { *std::mem::transmute::<&&usize, &usize>(&FOO) + 4 }; +//~^ ERROR any use of this value will cause an error +//~| NOTE "pointer-to-integer cast" needs an rfc +//~| NOTE diff --git a/src/test/ui/consts/ptr_comparisons.stderr b/src/test/ui/consts/ptr_comparisons.stderr new file mode 100644 index 0000000000000..c3e4b6abfcd23 --- /dev/null +++ b/src/test/ui/consts/ptr_comparisons.stderr @@ -0,0 +1,47 @@ +error: any use of this value will cause an error + --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL + | +LL | unsafe { intrinsics::offset(self, count) } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | | + | inbounds test failed: pointer must be in-bounds at offset 16, but is outside bounds of alloc2 which has size 8 + | inside `std::ptr::const_ptr::::offset` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL + | inside `_` at $DIR/ptr_comparisons.rs:58:34 + | + ::: $DIR/ptr_comparisons.rs:58:1 + | +LL | const _: *const usize = unsafe { (FOO as *const usize).offset(2) }; + | ------------------------------------------------------------------- + | + = note: `#[deny(const_err)]` on by default + +error: any use of this value will cause an error + --> $DIR/ptr_comparisons.rs:63:14 + | +LL | / const _: *const u8 = +LL | | +LL | | unsafe { std::ptr::raw_const!((*(FOO as *const usize as *const [u8; 1000]))[999]) }; + | |______________^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^__- + | | + | memory access failed: pointer must be in-bounds at offset 1000, but is outside bounds of alloc2 which has size 8 + | + = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) + +error: any use of this value will cause an error + --> $DIR/ptr_comparisons.rs:66:27 + | +LL | const _: usize = unsafe { std::mem::transmute::<*const usize, usize>(FOO) + 4 }; + | --------------------------^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^--- + | | + | "pointer-to-integer cast" needs an rfc before being allowed inside constants + +error: any use of this value will cause an error + --> $DIR/ptr_comparisons.rs:71:27 + | +LL | const _: usize = unsafe { *std::mem::transmute::<&&usize, &usize>(&FOO) + 4 }; + | --------------------------^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^--- + | | + | "pointer-to-integer cast" needs an rfc before being allowed inside constants + +error: aborting due to 4 previous errors + diff --git a/src/test/ui/consts/ptr_is_null.rs b/src/test/ui/consts/ptr_is_null.rs new file mode 100644 index 0000000000000..324f59443f721 --- /dev/null +++ b/src/test/ui/consts/ptr_is_null.rs @@ -0,0 +1,16 @@ +// compile-flags: --crate-type=lib +// check-pass + +#![feature(const_ptr_is_null, const_panic)] + +const FOO: &usize = &42; + +pub const _: () = assert!(!(FOO as *const usize).is_null()); + +pub const _: () = assert!(!(42 as *const usize).is_null()); + +pub const _: () = assert!((0 as *const usize).is_null()); + +pub const _: () = assert!(std::ptr::null::().is_null()); + +pub const _: () = assert!(!("foo" as *const str).is_null()); From daf7a375105b55de7a808aa096194280249f0fc5 Mon Sep 17 00:00:00 2001 From: Oliver Scherer Date: Mon, 17 Aug 2020 15:31:48 +0200 Subject: [PATCH 2/2] Make a test platform independent --- src/test/ui/consts/ptr_comparisons.rs | 4 ++++ src/test/ui/consts/ptr_comparisons.stderr | 14 +++++++------- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/src/test/ui/consts/ptr_comparisons.rs b/src/test/ui/consts/ptr_comparisons.rs index de7aeaad805cd..be998c800381d 100644 --- a/src/test/ui/consts/ptr_comparisons.rs +++ b/src/test/ui/consts/ptr_comparisons.rs @@ -1,4 +1,8 @@ // compile-flags: --crate-type=lib +// normalize-stderr-32bit: "offset 8" -> "offset $$TWO_WORDS" +// normalize-stderr-64bit: "offset 16" -> "offset $$TWO_WORDS" +// normalize-stderr-32bit: "size 4" -> "size $$WORD" +// normalize-stderr-64bit: "size 8" -> "size $$WORD" #![feature( const_panic, diff --git a/src/test/ui/consts/ptr_comparisons.stderr b/src/test/ui/consts/ptr_comparisons.stderr index c3e4b6abfcd23..493d9be8f71b7 100644 --- a/src/test/ui/consts/ptr_comparisons.stderr +++ b/src/test/ui/consts/ptr_comparisons.stderr @@ -4,11 +4,11 @@ error: any use of this value will cause an error LL | unsafe { intrinsics::offset(self, count) } | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | | - | inbounds test failed: pointer must be in-bounds at offset 16, but is outside bounds of alloc2 which has size 8 + | inbounds test failed: pointer must be in-bounds at offset $TWO_WORDS, but is outside bounds of alloc2 which has size $WORD | inside `std::ptr::const_ptr::::offset` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL - | inside `_` at $DIR/ptr_comparisons.rs:58:34 + | inside `_` at $DIR/ptr_comparisons.rs:62:34 | - ::: $DIR/ptr_comparisons.rs:58:1 + ::: $DIR/ptr_comparisons.rs:62:1 | LL | const _: *const usize = unsafe { (FOO as *const usize).offset(2) }; | ------------------------------------------------------------------- @@ -16,19 +16,19 @@ LL | const _: *const usize = unsafe { (FOO as *const usize).offset(2) }; = note: `#[deny(const_err)]` on by default error: any use of this value will cause an error - --> $DIR/ptr_comparisons.rs:63:14 + --> $DIR/ptr_comparisons.rs:67:14 | LL | / const _: *const u8 = LL | | LL | | unsafe { std::ptr::raw_const!((*(FOO as *const usize as *const [u8; 1000]))[999]) }; | |______________^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^__- | | - | memory access failed: pointer must be in-bounds at offset 1000, but is outside bounds of alloc2 which has size 8 + | memory access failed: pointer must be in-bounds at offset 1000, but is outside bounds of alloc2 which has size $WORD | = note: this error originates in a macro (in Nightly builds, run with -Z macro-backtrace for more info) error: any use of this value will cause an error - --> $DIR/ptr_comparisons.rs:66:27 + --> $DIR/ptr_comparisons.rs:70:27 | LL | const _: usize = unsafe { std::mem::transmute::<*const usize, usize>(FOO) + 4 }; | --------------------------^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^--- @@ -36,7 +36,7 @@ LL | const _: usize = unsafe { std::mem::transmute::<*const usize, usize>(FOO) + | "pointer-to-integer cast" needs an rfc before being allowed inside constants error: any use of this value will cause an error - --> $DIR/ptr_comparisons.rs:71:27 + --> $DIR/ptr_comparisons.rs:75:27 | LL | const _: usize = unsafe { *std::mem::transmute::<&&usize, &usize>(&FOO) + 4 }; | --------------------------^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^---