Merge pull request #623 from RalfJung/fat-ptr-eq

Fix comparing fat pointers

rust-version

@@ -1 +1 @@
-nightly-2019-02-14
+nightly-2019-02-15

src/bin/cargo-miri.rs

@@ -266,7 +266,7 @@ path = "lib.rs"
     let sysroot = if is_host { dir.join("HOST") } else { PathBuf::from(dir) };
     std::env::set_var("MIRI_SYSROOT", &sysroot);
     if !ask_user {
-        println!("A libstd for miri is now available in `{}`", sysroot.display());
+        println!("A libstd for Miri is now available in `{}`", sysroot.display());
     }
 }
 

src/intrinsic.rs

@@ -5,7 +5,7 @@ use rustc::ty;
 use rustc::mir::interpret::{EvalResult, PointerArithmetic};
 
 use crate::{
-    PlaceTy, OpTy, Immediate, Scalar, ScalarMaybeUndef, Borrow,
+    PlaceTy, OpTy, ImmTy, Immediate, Scalar, ScalarMaybeUndef, Borrow,
     OperatorEvalContextExt
 };
 
@@ -80,11 +80,11 @@ pub trait EvalContextExt<'a, 'mir, 'tcx: 'a+'mir>: crate::MiriEvalContextExt<'a,
 
             _ if intrinsic_name.starts_with("atomic_cxchg") => {
                 let ptr = this.deref_operand(args[0])?;
-                let expect_old = this.read_immediate(args[1])?; // read as immediate for the sake of `binary_op_imm()`
+                let expect_old = this.read_immediate(args[1])?; // read as immediate for the sake of `binary_op()`
                 let new = this.read_scalar(args[2])?;
-                let old = this.read_immediate(ptr.into())?; // read as immediate for the sake of `binary_op_imm()`
-                // binary_op_imm will bail if either of them is not a scalar
-                let (eq, _) = this.binary_op_imm(mir::BinOp::Eq, old, expect_old)?;
+                let old = this.read_immediate(ptr.into())?; // read as immediate for the sake of `binary_op()`
+                // binary_op will bail if either of them is not a scalar
+                let (eq, _) = this.binary_op(mir::BinOp::Eq, old, expect_old)?;
                 let res = Immediate::ScalarPair(old.to_scalar_or_undef(), eq.into());
                 this.write_immediate(res, dest)?; // old value is returned
                 // update ptr depending on comparison
@@ -140,9 +140,9 @@ pub trait EvalContextExt<'a, 'mir, 'tcx: 'a+'mir>: crate::MiriEvalContextExt<'a,
                     _ => bug!(),
                 };
                 // Atomics wrap around on overflow.
-                let (val, _overflowed) = this.binary_op_imm(op, old, rhs)?;
+                let (val, _overflowed) = this.binary_op(op, old, rhs)?;
                 let val = if neg {
-                    this.unary_op(mir::UnOp::Not, val, old.layout)?
+                    this.unary_op(mir::UnOp::Not, ImmTy::from_scalar(val, old.layout))?
                 } else {
                     val
                 };
@@ -239,7 +239,7 @@ pub trait EvalContextExt<'a, 'mir, 'tcx: 'a+'mir>: crate::MiriEvalContextExt<'a,
                 let a = this.read_immediate(args[0])?;
                 let b = this.read_immediate(args[1])?;
                 // check x % y != 0
-                if this.binary_op_imm(mir::BinOp::Rem, a, b)?.0.to_bits(dest.layout.size)? != 0 {
+                if this.binary_op(mir::BinOp::Rem, a, b)?.0.to_bits(dest.layout.size)? != 0 {
                     return err!(ValidationFailure(format!("exact_div: {:?} cannot be divided by {:?}", a, b)));
                 }
                 this.binop_ignore_overflow(mir::BinOp::Div, a, b, dest)?;

src/lib.rs

@@ -17,7 +17,7 @@ use std::collections::HashMap;
 use std::borrow::Cow;
 
 use rustc::ty::{self, TyCtxt, query::TyCtxtAt};
-use rustc::ty::layout::{TyLayout, LayoutOf, Size, Align};
+use rustc::ty::layout::{LayoutOf, Size, Align};
 use rustc::hir::{self, def_id::DefId};
 use rustc::mir;
 
@@ -406,12 +406,10 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> {
     fn ptr_op(
         ecx: &rustc_mir::interpret::EvalContext<'a, 'mir, 'tcx, Self>,
         bin_op: mir::BinOp,
-        left: Scalar<Borrow>,
-        left_layout: TyLayout<'tcx>,
-        right: Scalar<Borrow>,
-        right_layout: TyLayout<'tcx>,
+        left: ImmTy<'tcx, Borrow>,
+        right: ImmTy<'tcx, Borrow>,
     ) -> EvalResult<'tcx, (Scalar<Borrow>, bool)> {
-        ecx.ptr_op(bin_op, left, left_layout, right, right_layout)
+        ecx.ptr_op(bin_op, left, right)
     }
 
     fn box_alloc(

src/operator.rs

@@ -1,4 +1,4 @@
-use rustc::ty::{Ty, layout::TyLayout};
+use rustc::ty::Ty;
 use rustc::mir;
 
 use crate::*;
@@ -7,10 +7,8 @@ pub trait EvalContextExt<'tcx> {
     fn ptr_op(
         &self,
         bin_op: mir::BinOp,
-        left: Scalar<Borrow>,
-        left_layout: TyLayout<'tcx>,
-        right: Scalar<Borrow>,
-        right_layout: TyLayout<'tcx>,
+        left: ImmTy<'tcx, Borrow>,
+        right: ImmTy<'tcx, Borrow>,
     ) -> EvalResult<'tcx, (Scalar<Borrow>, bool)>;
 
     fn ptr_int_arithmetic(
@@ -25,7 +23,6 @@ pub trait EvalContextExt<'tcx> {
         &self,
         left: Scalar<Borrow>,
         right: Scalar<Borrow>,
-        size: Size,
     ) -> EvalResult<'tcx, bool>;
 
     fn pointer_offset_inbounds(
@@ -40,14 +37,34 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, '
     fn ptr_op(
         &self,
         bin_op: mir::BinOp,
-        left: Scalar<Borrow>,
-        left_layout: TyLayout<'tcx>,
-        right: Scalar<Borrow>,
-        right_layout: TyLayout<'tcx>,
+        left: ImmTy<'tcx, Borrow>,
+        right: ImmTy<'tcx, Borrow>,
     ) -> EvalResult<'tcx, (Scalar<Borrow>, bool)> {
         use rustc::mir::BinOp::*;
 
-        trace!("ptr_op: {:?} {:?} {:?}", left, bin_op, right);
+        trace!("ptr_op: {:?} {:?} {:?}", *left, bin_op, *right);
+
+        // Operations that support fat pointers
+        match bin_op {
+            Eq | Ne => {
+                let eq = match (*left, *right) {
+                    (Immediate::Scalar(left), Immediate::Scalar(right)) =>
+                        self.ptr_eq(left.not_undef()?, right.not_undef()?)?,
+                    (Immediate::ScalarPair(left1, left2), Immediate::ScalarPair(right1, right2)) =>
+                        self.ptr_eq(left1.not_undef()?, right1.not_undef()?)? &&
+                        self.ptr_eq(left2.not_undef()?, right2.not_undef()?)?,
+                    _ => bug!("Type system should not allow comparing Scalar with ScalarPair"),
+                };
+                return Ok((Scalar::from_bool(if bin_op == Eq { eq } else { !eq }), false));
+            }
+            _ => {},
+        }
+
+        // Now we expect no more fat pointers
+        let left_layout = left.layout;
+        let left = left.to_scalar()?;
+        let right_layout = right.layout;
+        let right = right.to_scalar()?;
         debug_assert!(left.is_ptr() || right.is_ptr() || bin_op == Offset);
 
         match bin_op {
@@ -63,11 +80,6 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, '
                 )?;
                 Ok((ptr, false))
             }
-            // These work on anything
-            Eq =>
-                Ok((Scalar::from_bool(self.ptr_eq(left, right, left_layout.size)?), false)),
-            Ne =>
-                Ok((Scalar::from_bool(!self.ptr_eq(left, right, left_layout.size)?), false)),
             // These need both to be pointer, and fail if they are not in the same location
             Lt | Le | Gt | Ge | Sub if left.is_ptr() && right.is_ptr() => {
                 let left = left.to_ptr().expect("we checked is_ptr");
@@ -85,8 +97,8 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, '
                             let layout = self.layout_of(self.tcx.types.usize)?;
                             return self.binary_op(
                                 Sub,
-                                left_offset, layout,
-                                right_offset, layout,
+                                ImmTy::from_scalar(left_offset, layout),
+                                ImmTy::from_scalar(right_offset, layout),
                             )
                         }
                         _ => bug!("We already established it has to be one of these operators."),
@@ -126,8 +138,8 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, '
         &self,
         left: Scalar<Borrow>,
         right: Scalar<Borrow>,
-        size: Size,
     ) -> EvalResult<'tcx, bool> {
+        let size = self.pointer_size();
         Ok(match (left, right) {
             (Scalar::Bits { .. }, Scalar::Bits { .. }) =>
                 left.to_bits(size)? == right.to_bits(size)?,

tests/run-pass/async-fn.rs

@@ -4,7 +4,8 @@
     futures_api,
 )]
 
-use std::{future::Future, pin::Pin, task::Poll};
+use std::{future::Future, pin::Pin, task::Poll, ptr};
+use std::task::{Waker, RawWaker, RawWakerVTable};
 
 // See if we can run a basic `async fn`
 pub async fn foo(x: &u32, y: u32) -> u32 {
@@ -17,18 +18,23 @@ pub async fn foo(x: &u32, y: u32) -> u32 {
     *x + y + *a
 }
 
-fn main() {
-    use std::{sync::Arc, task::{Wake, local_waker}};
+fn raw_waker_clone(_this: *const ()) -> RawWaker {
+    panic!("unimplemented");
+}
+fn raw_waker_wake(_this: *const ()) {
+    panic!("unimplemented");
+}
+fn raw_waker_drop(_this: *const ()) {}
 
-    struct NoWake;
-    impl Wake for NoWake {
-        fn wake(_arc_self: &Arc<Self>) {
-            panic!();
-        }
-    }
+static RAW_WAKER: RawWakerVTable = RawWakerVTable {
+    clone: raw_waker_clone,
+    wake: raw_waker_wake,
+    drop: raw_waker_drop,
+};
 
-    let lw = unsafe { local_waker(Arc::new(NoWake)) };
+fn main() {
     let x = 5;
     let mut fut = foo(&x, 7);
-    assert_eq!(unsafe { Pin::new_unchecked(&mut fut) }.poll(&lw), Poll::Ready(31));
+    let waker = unsafe { Waker::new_unchecked(RawWaker::new(ptr::null(), &RAW_WAKER)) };
+    assert_eq!(unsafe { Pin::new_unchecked(&mut fut) }.poll(&waker), Poll::Ready(31));
 }

tests/run-pass/hashmap.rs

@@ -21,5 +21,5 @@ fn main() {
     }
     assert_eq!(map.values().fold(0, |x, y| x+y), num*(num-1)/2);
 
-    // TODO: Test Entry API
+    // TODO: Test Entry API, Iterators, ...
 }

tests/run-pass/rc.rs

@@ -1,6 +1,7 @@
 use std::cell::{Cell, RefCell};
 use std::rc::Rc;
 use std::sync::Arc;
+use std::fmt::Debug;
 
 fn rc_refcell() {
     let r = Rc::new(RefCell::new(42));
@@ -60,7 +61,16 @@ fn rc_from() {
     check_unique_rc::<str>(Rc::from("Hello, World!"));
 }
 
+fn rc_fat_ptr_eq() {
+    let p = Rc::new(1) as Rc<Debug>;
+    let a: *const Debug = &*p;
+    let r = Rc::into_raw(p);
+    assert!(a == r);
+    drop(unsafe { Rc::from_raw(r) });
+}
+
 fn main() {
+    rc_fat_ptr_eq();
     rc_refcell();
     rc_refcell2();
     rc_cell();