From cd138bcd0b30b7af40fe0c13ce4f71f49cb59d3c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 15 Sep 2018 16:35:37 +0200 Subject: [PATCH 1/3] test more operations on dangling ZST pointers --- .../maybe_null_pointer_deref_zst.rs | 5 +++++ .../maybe_null_pointer_write_zst.rs | 8 ++++++++ tests/compile-fail/null_pointer_deref_zst.rs | 4 ++++ tests/compile-fail/null_pointer_write.rs | 3 +++ tests/compile-fail/null_pointer_write_zst.rs | 6 ++++++ tests/run-pass/cast-rfc0401-vtable-kinds.rs | 4 ---- tests/run-pass/const-vec-of-fns.rs | 2 -- tests/run-pass/issue-20575.rs | 2 -- tests/run-pass/pointers.rs | 6 +++++- .../regions-lifetime-nonfree-late-bound.rs | 2 -- tests/run-pass/sendable-class.rs | 2 -- tests/run-pass/zst.rs | 19 +++++++++++++++++-- tests/run-pass/zst2.rs | 12 ------------ 13 files changed, 48 insertions(+), 27 deletions(-) create mode 100644 tests/compile-fail/maybe_null_pointer_deref_zst.rs create mode 100644 tests/compile-fail/maybe_null_pointer_write_zst.rs create mode 100644 tests/compile-fail/null_pointer_deref_zst.rs create mode 100644 tests/compile-fail/null_pointer_write.rs create mode 100644 tests/compile-fail/null_pointer_write_zst.rs delete mode 100644 tests/run-pass/zst2.rs diff --git a/tests/compile-fail/maybe_null_pointer_deref_zst.rs b/tests/compile-fail/maybe_null_pointer_deref_zst.rs new file mode 100644 index 000000000000..d9f5ad4c696e --- /dev/null +++ b/tests/compile-fail/maybe_null_pointer_deref_zst.rs @@ -0,0 +1,5 @@ +fn main() { + // This pointer *could* be NULL so we cannot load from it, not even at ZST + let ptr = (&0u8 as *const u8).wrapping_sub(0x800) as *const (); + let _x: () = unsafe { *ptr }; //~ ERROR outside bounds +} diff --git a/tests/compile-fail/maybe_null_pointer_write_zst.rs b/tests/compile-fail/maybe_null_pointer_write_zst.rs new file mode 100644 index 000000000000..ef46a469c3ad --- /dev/null +++ b/tests/compile-fail/maybe_null_pointer_write_zst.rs @@ -0,0 +1,8 @@ +fn main() { + // This pointer *could* be NULL so we cannot load from it, not even at ZST. + // Not using the () type here, as writes of that type do not even have MIR generated. + // Also not assigning directly as that's array initialization, not assignment. + let zst_val = [1u8; 0]; + let ptr = (&0u8 as *const u8).wrapping_sub(0x800) as *mut [u8; 0]; + unsafe { *ptr = zst_val; } //~ ERROR outside bounds +} diff --git a/tests/compile-fail/null_pointer_deref_zst.rs b/tests/compile-fail/null_pointer_deref_zst.rs new file mode 100644 index 000000000000..a8b23368616e --- /dev/null +++ b/tests/compile-fail/null_pointer_deref_zst.rs @@ -0,0 +1,4 @@ +fn main() { + let x: () = unsafe { *std::ptr::null() }; //~ ERROR constant evaluation error: invalid use of NULL pointer + panic!("this should never print: {:?}", x); +} diff --git a/tests/compile-fail/null_pointer_write.rs b/tests/compile-fail/null_pointer_write.rs new file mode 100644 index 000000000000..affb040bdedf --- /dev/null +++ b/tests/compile-fail/null_pointer_write.rs @@ -0,0 +1,3 @@ +fn main() { + unsafe { *std::ptr::null_mut() = 0i32 }; //~ ERROR constant evaluation error: invalid use of NULL pointer +} diff --git a/tests/compile-fail/null_pointer_write_zst.rs b/tests/compile-fail/null_pointer_write_zst.rs new file mode 100644 index 000000000000..433c69dbb032 --- /dev/null +++ b/tests/compile-fail/null_pointer_write_zst.rs @@ -0,0 +1,6 @@ +fn main() { + // Not using the () type here, as writes of that type do not even have MIR generated. + // Also not assigning directly as that's array initialization, not assignment. + let zst_val = [1u8; 0]; + unsafe { *std::ptr::null_mut() = zst_val }; //~ ERROR constant evaluation error: invalid use of NULL pointer +} diff --git a/tests/run-pass/cast-rfc0401-vtable-kinds.rs b/tests/run-pass/cast-rfc0401-vtable-kinds.rs index afbd4760a3c9..ebae26996b7b 100644 --- a/tests/run-pass/cast-rfc0401-vtable-kinds.rs +++ b/tests/run-pass/cast-rfc0401-vtable-kinds.rs @@ -8,10 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. - -// FIXME: remove the next line when https://github.com/rust-lang/rust/issues/43358 is resolved -// compile-flags: -Zmir-opt-level=0 - // Check that you can cast between different pointers to trait objects // whose vtable have the same kind (both lengths, or both trait pointers). diff --git a/tests/run-pass/const-vec-of-fns.rs b/tests/run-pass/const-vec-of-fns.rs index 0338a766e262..e100ad5f4692 100644 --- a/tests/run-pass/const-vec-of-fns.rs +++ b/tests/run-pass/const-vec-of-fns.rs @@ -8,8 +8,6 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// pretty-expanded FIXME #23616 - /*! * Try to double-check that static fns have the right size (with or * without dummy env ptr, as appropriate) by iterating a size-2 array. diff --git a/tests/run-pass/issue-20575.rs b/tests/run-pass/issue-20575.rs index 7db7e3b28e8e..137d84c256be 100644 --- a/tests/run-pass/issue-20575.rs +++ b/tests/run-pass/issue-20575.rs @@ -10,8 +10,6 @@ // Test that overloaded calls work with zero arity closures -// pretty-expanded FIXME #23616 - fn main() { let functions: [Box Option<()>>; 1] = [Box::new(|| None)]; diff --git a/tests/run-pass/pointers.rs b/tests/run-pass/pointers.rs index 936ec73bcb8c..d8f030de0723 100644 --- a/tests/run-pass/pointers.rs +++ b/tests/run-pass/pointers.rs @@ -55,7 +55,11 @@ fn main() { assert_eq!(basic_ref_mut_var(), 3); assert_eq!(tuple_ref_mut(), (10, 22)); assert_eq!(match_ref_mut(), 42); - // FIXME: improve this test... how? + + // Compare even dangling pointers with NULL, and with others in the same allocation. assert!(dangling_pointer() != std::ptr::null()); assert!(match dangling_pointer() as usize { 0 => false, _ => true }); + let dangling = dangling_pointer(); + assert!(dangling == dangling); + assert!(dangling.wrapping_add(1) != dangling); } diff --git a/tests/run-pass/regions-lifetime-nonfree-late-bound.rs b/tests/run-pass/regions-lifetime-nonfree-late-bound.rs index 1aef95d8a3f3..96f1217a254e 100644 --- a/tests/run-pass/regions-lifetime-nonfree-late-bound.rs +++ b/tests/run-pass/regions-lifetime-nonfree-late-bound.rs @@ -22,8 +22,6 @@ // doing region-folding, when really all clients of the region-folding // case only want to see FREE lifetime variables, not bound ones. -// pretty-expanded FIXME #23616 - #![allow(unused_features)] #![feature(box_syntax)] diff --git a/tests/run-pass/sendable-class.rs b/tests/run-pass/sendable-class.rs index b3e07d00f010..66f0c84e23c1 100644 --- a/tests/run-pass/sendable-class.rs +++ b/tests/run-pass/sendable-class.rs @@ -10,8 +10,6 @@ // Test that a class with only sendable fields can be sent -// pretty-expanded FIXME #23616 - use std::sync::mpsc::channel; #[allow(dead_code)] diff --git a/tests/run-pass/zst.rs b/tests/run-pass/zst.rs index c1c88875c5c8..c3bae4062fc2 100644 --- a/tests/run-pass/zst.rs +++ b/tests/run-pass/zst.rs @@ -11,8 +11,23 @@ fn use_zst() -> A { } fn main() { + // Not using the () type here, as writes of that type do not even have MIR generated. + // Also not assigning directly as that's array initialization, not assignment. + let zst_val = [1u8; 0]; + assert_eq!(zst_ret(), A); assert_eq!(use_zst(), A); - let x = 42 as *mut (); - unsafe { *x = (); } + let x = 42 as *mut [u8; 0]; + // reading and writing is okay + unsafe { *x = zst_val; } + unsafe { let _y = *x; } + + // We should even be able to use "true" pointers for ZST when the allocation has been + // removed already. The box is for a non-ZST to make sure there actually is an allocation. + let mut x_box = Box::new(((), 1u8)); + let x = &mut x_box.0 as *mut _ as *mut [u8; 0]; + drop(x_box); + // reading and writing is okay + unsafe { *x = zst_val; } + unsafe { let _y = *x; } } diff --git a/tests/run-pass/zst2.rs b/tests/run-pass/zst2.rs deleted file mode 100644 index c2d7b88ea075..000000000000 --- a/tests/run-pass/zst2.rs +++ /dev/null @@ -1,12 +0,0 @@ -#![allow(dead_code)] - -#[derive(Debug)] -struct A; - -fn main() { - // can't use assert_eq, b/c that will try to print the pointer addresses with full MIR enabled - - // FIXME: Test disabled for now, see . - //assert!(&A as *const A as *const () == &() as *const _); - //assert!(&A as *const A == &A as *const A); -} From 18d7394071713092fd6d491fb7bcc3d98bb9a621 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 15 Sep 2018 17:12:48 +0200 Subject: [PATCH 2/3] more compile-fail ptr equality tests, to rule out any non-determinism; and fix ptr equality to fail all of them. At least these are the cases I can think of right now. --- src/memory.rs | 2 +- src/operator.rs | 50 +++++++++++++++---- tests/compile-fail/ptr_eq_dangling.rs | 10 ++++ tests/compile-fail/ptr_eq_integer.rs | 8 +++ tests/compile-fail/ptr_eq_out_of_bounds.rs | 9 ++++ .../compile-fail/ptr_eq_out_of_bounds_null.rs | 6 +++ tests/run-pass-fullmir/loop-break-value.rs | 4 +- tests/run-pass/pointers.rs | 27 ++++++++-- 8 files changed, 102 insertions(+), 14 deletions(-) create mode 100644 tests/compile-fail/ptr_eq_dangling.rs create mode 100644 tests/compile-fail/ptr_eq_integer.rs create mode 100644 tests/compile-fail/ptr_eq_out_of_bounds.rs create mode 100644 tests/compile-fail/ptr_eq_out_of_bounds_null.rs diff --git a/src/memory.rs b/src/memory.rs index 9f8118a223bc..4e0fcd4f511f 100644 --- a/src/memory.rs +++ b/src/memory.rs @@ -46,7 +46,7 @@ pub enum MemoryKind { C, /// Part of env var emulation Env, - // mutable statics + /// mutable statics MutStatic, } diff --git a/src/operator.rs b/src/operator.rs index 4f697dbd5b74..cf416c44c11f 100644 --- a/src/operator.rs +++ b/src/operator.rs @@ -152,18 +152,50 @@ fn ptr_eq( (Scalar::Ptr(ptr), Scalar::Bits { bits, size }) | (Scalar::Bits { bits, size }, Scalar::Ptr(ptr)) => { assert_eq!(size as u64, self.pointer_size().bytes()); + let bits = bits as u64; + let (alloc_size, alloc_align) = self.memory.get_size_and_align(ptr.alloc_id)?; + // Case I: Comparing with NULL if bits == 0 { - // Nothing equals 0, not even dangling pointers. Ideally we would - // require them to be in-bounds of their (possilby dead) allocation, - // but with the allocation gonew e cannot check that. - false - } else { - // Live pointers cannot equal an integer, but again do not - // allow comparing dead pointers. - self.memory.check_bounds(ptr, false)?; - false + // Test if the ptr is in-bounds. Then it cannot be NULL. + if ptr.offset <= alloc_size { + return Ok(false); + } } + // Case II: Alignment gives it away + if ptr.offset.bytes() % alloc_align.abi() == 0 { + // The offset maintains the allocation alignment, so we know `base+offset` + // is aligned by `alloc_align`. + // FIXME: We could be even more general, e.g. offset 2 into a 4-aligned + // allocation cannot equal 3. + if bits % alloc_align.abi() != 0 { + // The integer is *not* aligned. So they cannot be equal. + return Ok(false); + } + } + // Case III: The integer is too big, and the allocation goes on a bit + // without wrapping around the address space. + { + // Compute the highest address at which this allocation could live. + // Substract one more, because it must be possible to add the size + // to the base address without overflowing -- IOW, the very last address + // of the address space is never dereferencable (but it can be in-bounds, i.e., + // one-past-the-end). + let max_base_addr = + ((1u128 << self.pointer_size().bits()) + - u128::from(alloc_size.bytes()) + - 1 + ) as u64; + if let Some(max_addr) = max_base_addr.checked_add(ptr.offset.bytes()) { + if bits > max_addr { + // The integer is too big, this cannot possibly be equal + return Ok(false) + } + } + } + + // None of the supported cases. + return err!(InvalidPointerMath); } }) } diff --git a/tests/compile-fail/ptr_eq_dangling.rs b/tests/compile-fail/ptr_eq_dangling.rs new file mode 100644 index 000000000000..d05996a13d56 --- /dev/null +++ b/tests/compile-fail/ptr_eq_dangling.rs @@ -0,0 +1,10 @@ +fn main() { + let b = Box::new(0); + let x = &*b as *const i32; // soon-to-be dangling + drop(b); + let b = Box::new(0); + let y = &*b as *const i32; // different allocation + // We cannot compare these even though both are inbounds -- they *could* be + // equal if memory was reused. + assert!(x != y); //~ ERROR dangling pointer +} diff --git a/tests/compile-fail/ptr_eq_integer.rs b/tests/compile-fail/ptr_eq_integer.rs new file mode 100644 index 000000000000..10d5fbd517a3 --- /dev/null +++ b/tests/compile-fail/ptr_eq_integer.rs @@ -0,0 +1,8 @@ +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::() as *const i32); //~ ERROR invalid arithmetic on pointers +} diff --git a/tests/compile-fail/ptr_eq_out_of_bounds.rs b/tests/compile-fail/ptr_eq_out_of_bounds.rs new file mode 100644 index 000000000000..af4eed8d4e32 --- /dev/null +++ b/tests/compile-fail/ptr_eq_out_of_bounds.rs @@ -0,0 +1,9 @@ +fn main() { + let b = Box::new(0); + let x = (&*b as *const i32).wrapping_sub(0x800); // out-of-bounds + let b = Box::new(0); + let y = &*b as *const i32; // different allocation + // We cannot compare these even though both allocations are live -- they *could* be + // equal (with the right base addresses). + assert!(x != y); //~ ERROR outside bounds +} diff --git a/tests/compile-fail/ptr_eq_out_of_bounds_null.rs b/tests/compile-fail/ptr_eq_out_of_bounds_null.rs new file mode 100644 index 000000000000..3b7b51fc1995 --- /dev/null +++ b/tests/compile-fail/ptr_eq_out_of_bounds_null.rs @@ -0,0 +1,6 @@ +fn main() { + let b = Box::new(0); + let x = (&*b as *const i32).wrapping_sub(0x800); // out-of-bounds + // We cannot compare this with NULL. After all, this *could* be NULL (with the right base address). + assert!(x != std::ptr::null()); //~ ERROR invalid arithmetic on pointers +} diff --git a/tests/run-pass-fullmir/loop-break-value.rs b/tests/run-pass-fullmir/loop-break-value.rs index 8631909a2a96..ab79a64b56e2 100644 --- a/tests/run-pass-fullmir/loop-break-value.rs +++ b/tests/run-pass-fullmir/loop-break-value.rs @@ -61,7 +61,9 @@ pub fn main() { break Default::default() }; }; - assert_eq!(trait_unified_2, [""]); + // compare lengths; ptr comparison is not deterministic + assert_eq!(trait_unified_2.len(), 1); + assert_eq!(trait_unified_2[0].len(), 0); let trait_unified_3 = loop { break if false { diff --git a/tests/run-pass/pointers.rs b/tests/run-pass/pointers.rs index d8f030de0723..2b26791328df 100644 --- a/tests/run-pass/pointers.rs +++ b/tests/run-pass/pointers.rs @@ -1,3 +1,5 @@ +use std::usize; + fn one_line_ref() -> i16 { *&1 } @@ -44,8 +46,8 @@ fn match_ref_mut() -> i8 { } fn dangling_pointer() -> *const i32 { - let b = Box::new(42); - &*b as *const i32 + let b = Box::new((42, 42)); // make it bigger than the alignment, so that there is some "room" after this pointer + &b.0 as *const i32 } fn main() { @@ -56,10 +58,29 @@ fn main() { assert_eq!(tuple_ref_mut(), (10, 22)); assert_eq!(match_ref_mut(), 42); - // Compare even dangling pointers with NULL, and with others in the same allocation. + // Compare even dangling pointers with NULL, and with others in the same allocation, including + // out-of-bounds. assert!(dangling_pointer() != std::ptr::null()); assert!(match dangling_pointer() as usize { 0 => false, _ => true }); let dangling = dangling_pointer(); assert!(dangling == dangling); assert!(dangling.wrapping_add(1) != dangling); + assert!(dangling.wrapping_sub(1) != dangling); + + // Compare pointer with BIG integers + let dangling = dangling as usize; + assert!(dangling != usize::MAX); + assert!(dangling != usize::MAX - 1); + assert!(dangling != usize::MAX - 2); + assert!(dangling != usize::MAX - 3); // this is even 4-aligned, but it still cannot be equal because of the extra "room" after this pointer + assert_eq!((usize::MAX - 3) % 4, 0); // just to be sure we got this right + + // Compare pointer with unaligned integers + assert!(dangling != 1usize); + assert!(dangling != 2usize); + assert!(dangling != 3usize); + // 4 is a possible choice! So we cannot compare with that. + assert!(dangling != 5usize); + assert!(dangling != 6usize); + assert!(dangling != 7usize); } From 83ae0530a71ad21b13cb9a1acd3ce14e8d6688d4 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 18 Sep 2018 08:32:23 +0200 Subject: [PATCH 3/3] update toolchain --- rust-toolchain | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rust-toolchain b/rust-toolchain index 60c47a930576..48282503c507 100644 --- a/rust-toolchain +++ b/rust-toolchain @@ -1 +1 @@ -nightly-2018-09-17 +nightly-2018-09-18