Skip to content

Make <*const T>::is_null const fn #74940

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Aug 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions library/core/src/ptr/const_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,15 @@ impl<T: ?Sized> *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:
Expand All @@ -23,11 +32,12 @@ impl<T: ?Sized> *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.
Expand Down
14 changes: 12 additions & 2 deletions library/core/src/ptr/mut_ptr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,15 @@ impl<T: ?Sized> *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:
Expand All @@ -22,11 +31,12 @@ impl<T: ?Sized> *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.
Expand Down
42 changes: 39 additions & 3 deletions src/librustc_mir/interpret/intrinsics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()?;
Expand Down Expand Up @@ -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<M::PointerTag>, b: Scalar<M::PointerTag>) -> 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<M::PointerTag>, b: Scalar<M::PointerTag>) -> 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>,
Expand Down
78 changes: 78 additions & 0 deletions src/test/ui/consts/ptr_comparisons.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
// 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,
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
47 changes: 47 additions & 0 deletions src/test/ui/consts/ptr_comparisons.stderr
Original file line number Diff line number Diff line change
@@ -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 $TWO_WORDS, but is outside bounds of alloc2 which has size $WORD
| inside `std::ptr::const_ptr::<impl *const usize>::offset` at $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL
| inside `_` at $DIR/ptr_comparisons.rs:62:34
|
::: $DIR/ptr_comparisons.rs:62: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: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 $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:70: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:75: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

16 changes: 16 additions & 0 deletions src/test/ui/consts/ptr_is_null.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// compile-flags: --crate-type=lib
// check-pass
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you sure check-pass is enough for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

considering that it did cause stderr output when I had the asserts inverted, yes.

These constants are evaluated by the lint that used to be const_err, and check runs all lints.


#![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::<usize>().is_null());

pub const _: () = assert!(!("foo" as *const str).is_null());