Merge pull request #526 from RalfJung/mut-visitor

Retagging: Recurse into compound values

Author: RalfJung

appveyor.yml

@@ -37,7 +37,7 @@ build: false
 test_script:
     - set RUSTFLAGS=-g
     - set RUST_BACKTRACE=1
-    - cargo build --release --all-targets --all-features
+    - cargo build --release --all-features --all-targets
     - cargo test --release --all-features
     - set MIRI_SYSROOT=%USERPROFILE%\.xargo\HOST
     - cargo test --release --all-features

rust-version

@@ -1 +1 @@
-nightly-2018-11-16
+nightly-2018-11-20

src/fn_call.rs

@@ -131,10 +131,10 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo
             }
 
             "free" => {
-                let ptr = self.read_scalar(args[0])?.not_undef()?.erase_tag(); // raw ptr operation, no tag
+                let ptr = self.read_scalar(args[0])?.not_undef()?;
                 if !ptr.is_null_ptr(self) {
                     self.memory_mut().deallocate(
-                        ptr.to_ptr()?.with_default_tag(),
+                        ptr.to_ptr()?,
                         None,
                         MiriMemoryKind::C.into(),
                     )?;
@@ -179,7 +179,7 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo
                 self.write_scalar(Scalar::Ptr(ptr), dest)?;
             }
             "__rust_dealloc" => {
-                let ptr = self.read_scalar(args[0])?.to_ptr()?.erase_tag(); // raw ptr operation, no tag
+                let ptr = self.read_scalar(args[0])?.to_ptr()?;
                 let old_size = self.read_scalar(args[1])?.to_usize(self)?;
                 let align = self.read_scalar(args[2])?.to_usize(self)?;
                 if old_size == 0 {
@@ -189,13 +189,13 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo
                     return err!(HeapAllocNonPowerOfTwoAlignment(align));
                 }
                 self.memory_mut().deallocate(
-                    ptr.with_default_tag(),
+                    ptr,
                     Some((Size::from_bytes(old_size), Align::from_bytes(align, align).unwrap())),
                     MiriMemoryKind::Rust.into(),
                 )?;
             }
             "__rust_realloc" => {
-                let ptr = self.read_scalar(args[0])?.to_ptr()?.erase_tag(); // raw ptr operation, no tag
+                let ptr = self.read_scalar(args[0])?.to_ptr()?;
                 let old_size = self.read_scalar(args[1])?.to_usize(self)?;
                 let align = self.read_scalar(args[2])?.to_usize(self)?;
                 let new_size = self.read_scalar(args[3])?.to_usize(self)?;
@@ -206,7 +206,7 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo
                     return err!(HeapAllocNonPowerOfTwoAlignment(align));
                 }
                 let new_ptr = self.memory_mut().reallocate(
-                    ptr.with_default_tag(),
+                    ptr,
                     Size::from_bytes(old_size),
                     Align::from_bytes(align, align).unwrap(),
                     Size::from_bytes(new_size),
@@ -238,8 +238,8 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo
 
             "dlsym" => {
                 let _handle = self.read_scalar(args[0])?;
-                let symbol = self.read_scalar(args[1])?.to_ptr()?.erase_tag();
-                let symbol_name = self.memory().read_c_str(symbol.with_default_tag())?;
+                let symbol = self.read_scalar(args[1])?.to_ptr()?;
+                let symbol_name = self.memory().read_c_str(symbol)?;
                 let err = format!("bad c unicode symbol: {:?}", symbol_name);
                 let symbol_name = ::std::str::from_utf8(symbol_name).unwrap_or(&err);
                 return err!(Unimplemented(format!(
@@ -292,13 +292,13 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo
                 return err!(MachineError("the evaluated program panicked".to_string())),
 
             "memcmp" => {
-                let left = self.read_scalar(args[0])?.not_undef()?.erase_tag(); // raw ptr operation
-                let right = self.read_scalar(args[1])?.not_undef()?.erase_tag(); // raw ptr operation
+                let left = self.read_scalar(args[0])?.not_undef()?;
+                let right = self.read_scalar(args[1])?.not_undef()?;
                 let n = Size::from_bytes(self.read_scalar(args[2])?.to_usize(self)?);
 
                 let result = {
-                    let left_bytes = self.memory().read_bytes(left.with_default_tag(), n)?;
-                    let right_bytes = self.memory().read_bytes(right.with_default_tag(), n)?;
+                    let left_bytes = self.memory().read_bytes(left, n)?;
+                    let right_bytes = self.memory().read_bytes(right, n)?;
 
                     use std::cmp::Ordering::*;
                     match left_bytes.cmp(right_bytes) {
@@ -315,8 +315,7 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo
             }
 
             "memrchr" => {
-                let ptr = self.read_scalar(args[0])?.not_undef()?.erase_tag(); // raw ptr operation
-                let ptr = ptr.with_default_tag();
+                let ptr = self.read_scalar(args[0])?.not_undef()?;
                 let val = self.read_scalar(args[1])?.to_bytes()? as u8;
                 let num = self.read_scalar(args[2])?.to_usize(self)?;
                 if let Some(idx) = self.memory().read_bytes(ptr, Size::from_bytes(num))?
@@ -330,8 +329,7 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo
             }
 
             "memchr" => {
-                let ptr = self.read_scalar(args[0])?.not_undef()?.erase_tag(); // raw ptr operation
-                let ptr = ptr.with_default_tag();
+                let ptr = self.read_scalar(args[0])?.not_undef()?;
                 let val = self.read_scalar(args[1])?.to_bytes()? as u8;
                 let num = self.read_scalar(args[2])?.to_usize(self)?;
                 if let Some(idx) = self.memory().read_bytes(ptr, Size::from_bytes(num))?.iter().position(
@@ -347,8 +345,8 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo
 
             "getenv" => {
                 let result = {
-                    let name_ptr = self.read_scalar(args[0])?.to_ptr()?.erase_tag(); // raw ptr operation
-                    let name = self.memory().read_c_str(name_ptr.with_default_tag())?;
+                    let name_ptr = self.read_scalar(args[0])?.to_ptr()?;
+                    let name = self.memory().read_c_str(name_ptr)?;
                     match self.machine.env_vars.get(name) {
                         Some(&var) => Scalar::Ptr(var),
                         None => Scalar::ptr_null(&*self.tcx),
@@ -360,10 +358,10 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo
             "unsetenv" => {
                 let mut success = None;
                 {
-                    let name_ptr = self.read_scalar(args[0])?.not_undef()?.erase_tag(); // raw ptr operation
+                    let name_ptr = self.read_scalar(args[0])?.not_undef()?;
                     if !name_ptr.is_null_ptr(self) {
                         let name = self.memory().read_c_str(name_ptr.to_ptr()?
-                            .with_default_tag())?.to_owned();
+                            )?.to_owned();
                         if !name.is_empty() && !name.contains(&b'=') {
                             success = Some(self.machine.env_vars.remove(&name));
                         }
@@ -382,11 +380,11 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo
             "setenv" => {
                 let mut new = None;
                 {
-                    let name_ptr = self.read_scalar(args[0])?.not_undef()?.erase_tag(); // raw ptr operation
-                    let value_ptr = self.read_scalar(args[1])?.to_ptr()?.erase_tag(); // raw ptr operation
-                    let value = self.memory().read_c_str(value_ptr.with_default_tag())?;
+                    let name_ptr = self.read_scalar(args[0])?.not_undef()?;
+                    let value_ptr = self.read_scalar(args[1])?.to_ptr()?;
+                    let value = self.memory().read_c_str(value_ptr)?;
                     if !name_ptr.is_null_ptr(self) {
-                        let name = self.memory().read_c_str(name_ptr.to_ptr()?.with_default_tag())?;
+                        let name = self.memory().read_c_str(name_ptr.to_ptr()?)?;
                         if !name.is_empty() && !name.contains(&b'=') {
                             new = Some((name.to_owned(), value.to_owned()));
                         }
@@ -417,14 +415,14 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo
 
             "write" => {
                 let fd = self.read_scalar(args[0])?.to_bytes()?;
-                let buf = self.read_scalar(args[1])?.not_undef()?.erase_tag();
+                let buf = self.read_scalar(args[1])?.not_undef()?;
                 let n = self.read_scalar(args[2])?.to_bytes()? as u64;
                 trace!("Called write({:?}, {:?}, {:?})", fd, buf, n);
                 let result = if fd == 1 || fd == 2 {
                     // stdout/stderr
                     use std::io::{self, Write};
 
-                    let buf_cont = self.memory().read_bytes(buf.with_default_tag(), Size::from_bytes(n))?;
+                    let buf_cont = self.memory().read_bytes(buf, Size::from_bytes(n))?;
                     let res = if fd == 1 {
                         io::stdout().write(buf_cont)
                     } else {
@@ -445,8 +443,8 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo
             }
 
             "strlen" => {
-                let ptr = self.read_scalar(args[0])?.to_ptr()?.erase_tag();
-                let n = self.memory().read_c_str(ptr.with_default_tag())?.len();
+                let ptr = self.read_scalar(args[0])?.to_ptr()?;
+                let n = self.memory().read_c_str(ptr)?.len();
                 self.write_scalar(Scalar::from_uint(n as u64, dest.layout.size), dest)?;
             }
 
@@ -492,7 +490,7 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo
 
             // Hook pthread calls that go to the thread-local storage memory subsystem
             "pthread_key_create" => {
-                let key_ptr = self.read_scalar(args[0])?.to_ptr()?.erase_tag(); // raw ptr operation
+                let key_ptr = self.read_scalar(args[0])?.to_ptr()?;
 
                 // Extract the function type out of the signature (that seems easier than constructing it ourselves...)
                 let dtor = match self.read_scalar(args[1])?.not_undef()? {
@@ -515,7 +513,7 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for super::MiriEvalCo
                     return err!(OutOfTls);
                 }
                 self.memory_mut().write_scalar(
-                    key_ptr.with_default_tag(),
+                    key_ptr,
                     key_layout.align,
                     Scalar::from_uint(key, key_layout.size).into(),
                     key_layout.size,

src/helpers.rs

@@ -158,8 +158,10 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super:
             unsafe_cell_action: F,
         }
 
-        impl<'ecx, 'a, 'mir, 'tcx, F> ValueVisitor<'a, 'mir, 'tcx, Evaluator<'tcx>>
-        for UnsafeCellVisitor<'ecx, 'a, 'mir, 'tcx, F>
+        impl<'ecx, 'a, 'mir, 'tcx, F>
+            ValueVisitor<'a, 'mir, 'tcx, Evaluator<'tcx>>
+        for
+            UnsafeCellVisitor<'ecx, 'a, 'mir, 'tcx, F>
         where
             F: FnMut(MPlaceTy<'tcx, Borrow>) -> EvalResult<'tcx>
         {
@@ -230,7 +232,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super:
             }
 
             // We should never get to a primitive, but always short-circuit somewhere above
-            fn visit_primitive(&mut self, _val: ImmTy<'tcx, Borrow>) -> EvalResult<'tcx>
+            fn visit_primitive(&mut self, _v: MPlaceTy<'tcx, Borrow>) -> EvalResult<'tcx>
             {
                 bug!("We should always short-circit before coming to a primitive")
             }

src/intrinsic.rs

@@ -154,12 +154,12 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, '
                 let count = self.read_scalar(args[2])?.to_usize(self)?;
                 let elem_align = elem_layout.align;
                 // erase tags: this is a raw ptr operation
-                let src = self.read_scalar(args[0])?.not_undef()?.erase_tag();
-                let dest = self.read_scalar(args[1])?.not_undef()?.erase_tag();
+                let src = self.read_scalar(args[0])?.not_undef()?;
+                let dest = self.read_scalar(args[1])?.not_undef()?;
                 self.memory_mut().copy(
-                    src.with_default_tag(),
+                    src,
                     elem_align,
-                    dest.with_default_tag(),
+                    dest,
                     elem_align,
                     Size::from_bytes(count * elem_size),
                     intrinsic_name.ends_with("_nonoverlapping"),
@@ -436,7 +436,7 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, '
                 let ty = substs.type_at(0);
                 let ty_layout = self.layout_of(ty)?;
                 let val_byte = self.read_scalar(args[1])?.to_u8()?;
-                let ptr = self.read_scalar(args[0])?.not_undef()?.erase_tag().with_default_tag();
+                let ptr = self.read_scalar(args[0])?.not_undef()?;
                 let count = self.read_scalar(args[2])?.to_usize(self)?;
                 self.memory().check_align(ptr, ty_layout.align)?;
                 self.memory_mut().write_repeat(ptr, val_byte, ty_layout.size * count)?;

src/lib.rs

@@ -23,7 +23,7 @@ use rustc::hir::{self, def_id::DefId};
 use rustc::mir;
 
 use syntax::attr;
-
+use syntax::source_map::DUMMY_SP;
 
 pub use rustc_mir::interpret::*;
 pub use rustc_mir::interpret::{self, AllocMap, PlaceTy}; // resolve ambiguity
@@ -113,7 +113,7 @@ pub fn create_ecx<'a, 'mir: 'a, 'tcx: 'mir>(
         // Push our stack frame
         ecx.push_stack_frame(
             start_instance,
-            start_mir.span,
+            DUMMY_SP, // there is no call site, we want no span
             start_mir,
             Some(ret_ptr.into()),
             StackPopCleanup::None { cleanup: true },
@@ -146,7 +146,7 @@ pub fn create_ecx<'a, 'mir: 'a, 'tcx: 'mir>(
         let ret_place = MPlaceTy::dangling(ecx.layout_of(tcx.mk_unit())?, &ecx).into();
         ecx.push_stack_frame(
             main_instance,
-            main_mir.span,
+            DUMMY_SP, // there is no call site, we want no span
             main_mir,
             Some(ret_place),
             StackPopCleanup::None { cleanup: true },
@@ -185,7 +185,7 @@ pub fn eval_main<'a, 'tcx: 'a>(
     match res {
         Ok(()) => {
             let leaks = ecx.memory().leak_report();
-            // Disable the leak test on some platforms where we likely do not
+            // Disable the leak test on some platforms where we do not
             // correctly implement TLS destructors.
             let target_os = ecx.tcx.tcx.sess.target.target.target_os.to_lowercase();
             let ignore_leaks = target_os == "windows" || target_os == "macos";
@@ -208,8 +208,16 @@ pub fn eval_main<'a, 'tcx: 'a>(
                 let mut err = struct_error(ecx.tcx.tcx.at(span), msg.as_str());
                 let frames = ecx.generate_stacktrace(None);
                 err.span_label(span, e);
-                for FrameInfo { span, location, .. } in frames {
-                    err.span_note(span, &format!("inside call to `{}`", location));
+                // we iterate with indices because we need to look at the next frame (the caller)
+                for idx in 0..frames.len() {
+                    let frame_info = &frames[idx];
+                    let call_site_is_local = frames.get(idx+1).map_or(false,
+                        |caller_info| caller_info.instance.def_id().is_local());
+                    if call_site_is_local {
+                        err.span_note(frame_info.call_site, &frame_info.to_string());
+                    } else {
+                        err.note(&frame_info.to_string());
+                    }
                 }
                 err.emit();
             } else {
@@ -312,8 +320,6 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> {
             // Uses mem::uninitialized
             ("std::ptr::read", ""),
             ("std::sys::windows::mutex::Mutex::", ""),
-            // Should directly take a raw reference
-            ("<std::cell::UnsafeCell<T>>", "::get"),
         ];
         for frame in ecx.stack().iter()
             .rev().take(3)
@@ -461,9 +467,9 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> {
             // No tracking
             Ok(place.ptr)
         } else {
-            let ptr = place.ptr.to_ptr()?; // assert this is not a scalar
-            let tag = ecx.tag_dereference(place, size, mutability.into())?;
-            Ok(Scalar::Ptr(Pointer::new_with_tag(ptr.alloc_id, ptr.offset, tag)))
+            ecx.ptr_dereference(place, size, mutability.into())?;
+            // We never change the pointer
+            Ok(place.ptr)
         }
     }
 

src/operator.rs

@@ -142,8 +142,9 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, '
                     // 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".
-                    self.memory().check_bounds_ptr(left, false)?;
-                    self.memory().check_bounds_ptr(right, false)?;
+                    // We require liveness, as dead allocations can of course overlap.
+                    self.memory().check_bounds_ptr(left, InboundsCheck::Live)?;
+                    self.memory().check_bounds_ptr(right, InboundsCheck::Live)?;
                     // Two live in-bounds pointers, we can compare across allocations
                     left == right
                 }
@@ -153,15 +154,17 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, '
             (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 {
                     // Test if the ptr is in-bounds. Then it cannot be NULL.
-                    if ptr.offset <= alloc_size {
+                    if self.memory().check_bounds_ptr(ptr, InboundsCheck::MaybeDead).is_ok() {
                         return Ok(false);
                     }
                 }
+
+                let (alloc_size, alloc_align) = self.memory().get_size_and_align(ptr.alloc_id);
+
                 // 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`
@@ -293,11 +296,11 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for super::MiriEvalContext<'a, 'mir, '
         let offset = offset.checked_mul(pointee_size).ok_or_else(|| EvalErrorKind::Overflow(mir::BinOp::Mul))?;
         // Now let's see what kind of pointer this is
         if let Scalar::Ptr(ptr) = ptr {
-            // Both old and new pointer must be in-bounds.
+            // Both old and new pointer must be in-bounds of a *live* allocation.
             // (Of the same allocation, but that part is trivial with our representation.)
-            self.memory().check_bounds_ptr(ptr, false)?;
+            self.memory().check_bounds_ptr(ptr, InboundsCheck::Live)?;
             let ptr = ptr.signed_offset(offset, self)?;
-            self.memory().check_bounds_ptr(ptr, false)?;
+            self.memory().check_bounds_ptr(ptr, InboundsCheck::Live)?;
             Ok(Scalar::Ptr(ptr))
         } else {
             // An integer pointer. They can only be offset by 0, and we pretend there