Skip to content

be pragmatic about ptr-int comparisons, for now #691

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
Apr 16, 2019
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
18 changes: 14 additions & 4 deletions src/operator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,9 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, '
// This accepts one-past-the end. Thus, there is still technically
// some non-determinism that we do not fully rule out when two
// allocations sit right next to each other. The C/C++ standards are
// somewhat fuzzy about this case, so I think for now this check is
// "good enough".
// somewhat fuzzy about this case, so pragmatically speaking I think
// for now this check is "good enough".
// FIXME: Once we support intptrcast, we could try to fix these holes.
// Dead allocations in miri cannot overlap with live allocations, but
// on read hardware this can easily happen. Thus for comparisons we require
// both pointers to be live.
Expand All @@ -169,8 +170,17 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, '
assert_eq!(size as u64, self.pointer_size().bytes());
let bits = bits as u64;

// Case I: Comparing with NULL.
if bits == 0 {
// Case I: Comparing real pointers with "small" integers.
// Really we should only do this for NULL, but pragmatically speaking on non-bare-metal systems,
// an allocation will never be at the very bottom of the address space.
// Such comparisons can arise when comparing empty slices, which sometimes are "fake"
// integer pointers (okay because the slice is empty) and sometimes point into a
// real allocation.
// The most common source of such integer pointers is `NonNull::dangling()`, which
// equals the type's alignment. i128 might have an alignment of 16 bytes, but few types have
// alignment 32 or higher, hence the limit of 32.
// FIXME: Once we support intptrcast, we could try to fix these holes.
if bits < 32 {
// Test if the ptr is in-bounds. Then it cannot be NULL.
// Even dangling pointers cannot be NULL.
if self.memory().check_bounds_ptr(ptr, InboundsCheck::MaybeDead).is_ok() {
Expand Down
4 changes: 1 addition & 3 deletions tests/compile-fail/ptr_eq_integer.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
use std::mem;

fn main() {
let b = Box::new(0);
let x = &*b as *const i32;
// We cannot compare this with a non-NULL integer. After all, these *could* be equal (with the right base address).
assert!(x != mem::align_of::<i32>() as *const i32); //~ ERROR invalid arithmetic on pointers
assert!(x != 64 as *const i32); //~ ERROR invalid arithmetic on pointers
}
4 changes: 4 additions & 0 deletions tests/run-pass/vecs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,4 +85,8 @@ fn main() {
assert_eq!(make_vec_macro(), [1, 2]);
assert_eq!(make_vec_macro_repeat(), [42; 5]);
assert_eq!(make_vec_macro_repeat_zeroed(), [0; 7]);

// Test interesting empty slice comparison
// (one is a real pointer, one an integer pointer).
assert_eq!((200..-5).step_by(1).collect::<Vec<isize>>(), []);
}