Auto merge of #1084 - RalfJung:assert-panic, r=RalfJung

proper support for `Assert` MIR terminators

Fixes #1070
Blocked on rust-lang/rust#66874

Author: bors

rust-version

@@ -1 +1 @@
-4af3ee8ee2a2bc1286b021db7600ba990359cf3f
+2da942f32802c8233a09744024dfbc34431adf65

src/eval.rs

@@ -93,7 +93,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
             arg.push(0);
             argvs.push(
                 ecx.memory
-                    .allocate_static_bytes(arg.as_slice(), MiriMemoryKind::Static.into()),
+                    .allocate_static_bytes(arg.as_slice(), MiriMemoryKind::Env.into()),
             );
         }
         // Make an array with all these pointers, in the Miri memory.
@@ -103,7 +103,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
         let argvs_place = ecx.allocate(argvs_layout, MiriMemoryKind::Env.into());
         for (idx, arg) in argvs.into_iter().enumerate() {
             let place = ecx.mplace_field(argvs_place, idx as u64)?;
-            ecx.write_scalar(Scalar::Ptr(arg), place.into())?;
+            ecx.write_scalar(arg, place.into())?;
         }
         ecx.memory
             .mark_immutable(argvs_place.ptr.assert_ptr().alloc_id)?;
@@ -144,19 +144,19 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
     // Return place (in static memory so that it does not count as leak).
     let ret_place = ecx.allocate(
         ecx.layout_of(tcx.types.isize)?,
-        MiriMemoryKind::Static.into(),
+        MiriMemoryKind::Env.into(),
     );
     // Call start function.
     ecx.call_function(
         start_instance,
-        &[main_ptr.into(), argc, argv],
+        &[main_ptr.into(), argc.into(), argv.into()],
         Some(ret_place.into()),
         StackPopCleanup::None { cleanup: true },
     )?;
 
     // Set the last_error to 0
     let errno_layout = ecx.layout_of(tcx.types.u32)?;
-    let errno_place = ecx.allocate(errno_layout, MiriMemoryKind::Static.into());
+    let errno_place = ecx.allocate(errno_layout, MiriMemoryKind::Env.into());
     ecx.write_scalar(Scalar::from_u32(0), errno_place.into())?;
     ecx.machine.last_error = Some(errno_place);
 
@@ -217,16 +217,13 @@ pub fn eval_main<'tcx>(tcx: TyCtxt<'tcx>, main_id: DefId, config: MiriConfig) ->
                 }
                 err_unsup!(NoMirFor(..)) =>
                     format!("{}. Did you set `MIRI_SYSROOT` to a Miri-enabled sysroot? You can prepare one with `cargo miri setup`.", e),
+                InterpError::InvalidProgram(_) =>
+                    bug!("This error should be impossible in Miri: {}", e),
                 _ => e.to_string()
             };
             e.print_backtrace();
             if let Some(frame) = ecx.stack().last() {
-                let block = &frame.body.basic_blocks()[frame.block.unwrap()];
-                let span = if frame.stmt < block.statements.len() {
-                    block.statements[frame.stmt].source_info.span
-                } else {
-                    block.terminator().source_info.span
-                };
+                let span = frame.current_source_info().unwrap().span;
 
                 let msg = format!("Miri evaluation error: {}", msg);
                 let mut err = ecx.tcx.sess.struct_span_err(span, msg.as_str());

src/helpers.rs

@@ -124,17 +124,21 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
     fn call_function(
         &mut self,
         f: ty::Instance<'tcx>,
-        args: &[Scalar<Tag>],
+        args: &[Immediate<Tag>],
         dest: Option<PlaceTy<'tcx, Tag>>,
         stack_pop: StackPopCleanup,
     ) -> InterpResult<'tcx> {
         let this = self.eval_context_mut();
 
         // Push frame.
         let mir = this.load_mir(f.def, None)?;
+        let span = this.stack().last()
+            .and_then(Frame::current_source_info)
+            .map(|si| si.span)
+            .unwrap_or(DUMMY_SP);
         this.push_stack_frame(
             f,
-            DUMMY_SP, // There is no call site.
+            span,
             mir,
             dest,
             stack_pop,
@@ -146,7 +150,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             let callee_arg = this.local_place(
                 callee_args.next().expect("callee has fewer arguments than expected")
             )?;
-            this.write_scalar(*arg, callee_arg)?;
+            this.write_immediate(*arg, callee_arg)?;
         }
         callee_args.next().expect_none("callee has more arguments than expected");
 

src/lib.rs

@@ -38,14 +38,15 @@ pub use crate::shims::dlsym::{Dlsym, EvalContextExt as DlsymEvalContextExt};
 pub use crate::shims::env::{EnvVars, EvalContextExt as EnvEvalContextExt};
 pub use crate::shims::fs::{FileHandler, EvalContextExt as FileEvalContextExt};
 pub use crate::shims::panic::{CatchUnwindData, EvalContextExt as PanicEvalContextExt};
+
 pub use crate::operator::EvalContextExt as OperatorEvalContextExt;
 pub use crate::range_map::RangeMap;
 pub use crate::helpers::{EvalContextExt as HelpersEvalContextExt};
 pub use crate::mono_hash_map::MonoHashMap;
 pub use crate::stacked_borrows::{EvalContextExt as StackedBorEvalContextExt, Tag, Permission, Stack, Stacks, Item};
 pub use crate::machine::{
     PAGE_SIZE, STACK_ADDR, STACK_SIZE, NUM_CPUS,
-    MemoryExtra, AllocExtra, MiriMemoryKind, Evaluator, MiriEvalContext, MiriEvalContextExt,
+    MemoryExtra, AllocExtra, FrameData, MiriMemoryKind, Evaluator, MiriEvalContext, MiriEvalContextExt,
 };
 pub use crate::eval::{eval_main, create_ecx, MiriConfig, TerminationInfo};
 

src/machine.rs

@@ -43,9 +43,9 @@ pub enum MiriMemoryKind {
     C,
     /// Windows `HeapAlloc` memory.
     WinHeap,
-    /// Part of env var emulation.
+    /// Memory for env vars and args, errno and other parts of the machine-managed environment.
     Env,
-    /// Statics.
+    /// Rust statics.
     Static,
 }
 
@@ -215,6 +215,16 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> {
         ecx.call_intrinsic(span, instance, args, ret, unwind)
     }
 
+    #[inline(always)]
+    fn assert_panic(
+        ecx: &mut InterpCx<'mir, 'tcx, Self>,
+        span: Span,
+        msg: &AssertMessage<'tcx>,
+        unwind: Option<mir::BasicBlock>,
+    ) -> InterpResult<'tcx> {
+        ecx.assert_panic(span, msg, unwind)
+    }
+
     #[inline(always)]
     fn binary_ptr_op(
         ecx: &rustc_mir::interpret::InterpCx<'mir, 'tcx, Self>,
@@ -243,7 +253,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> {
         let malloc = ty::Instance::mono(ecx.tcx.tcx, malloc);
         ecx.call_function(
             malloc,
-            &[size, align],
+            &[size.into(), align.into()],
             Some(dest),
             // Don't do anything when we are done. The `statement()` function will increment
             // the old stack frame's stmt counter to the next statement, which means that when
@@ -281,27 +291,25 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> {
         Ok(())
     }
 
-    fn tag_allocation<'b>(
+    fn init_allocation_extra<'b>(
         memory_extra: &MemoryExtra,
         id: AllocId,
         alloc: Cow<'b, Allocation>,
         kind: Option<MemoryKind<Self::MemoryKinds>>,
-    ) -> (
-        Cow<'b, Allocation<Self::PointerTag, Self::AllocExtra>>,
-        Self::PointerTag,
-    ) {
+    ) -> (Cow<'b, Allocation<Self::PointerTag, Self::AllocExtra>>, Self::PointerTag) {
         let kind = kind.expect("we set our STATIC_KIND so this cannot be None");
         let alloc = alloc.into_owned();
-        let (stacks, base_tag) = if !memory_extra.validate {
-            (None, Tag::Untagged)
-        } else {
+        let (stacks, base_tag) = if memory_extra.validate {
             let (stacks, base_tag) = Stacks::new_allocation(
                 id,
                 alloc.size,
                 Rc::clone(&memory_extra.stacked_borrows),
                 kind,
             );
             (Some(stacks), base_tag)
+        } else {
+            // No stacks, no tag.
+            (None, Tag::Untagged)
         };
         let mut stacked_borrows = memory_extra.stacked_borrows.borrow_mut();
         let alloc: Allocation<Tag, Self::AllocExtra> = alloc.with_tags_and_extra(

src/shims/env.rs

@@ -57,7 +57,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         Ok(match this.machine.env_vars.map.get(name) {
             // The offset is used to strip the "{name}=" part of the string.
             Some(var_ptr) => {
-                Scalar::Ptr(var_ptr.offset(Size::from_bytes(name.len() as u64 + 1), this)?)
+                Scalar::from(var_ptr.offset(Size::from_bytes(name.len() as u64 + 1), this)?)
             }
             None => Scalar::ptr_null(&*this.tcx),
         })

src/shims/foreign_items.rs

@@ -205,7 +205,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                         Align::from_bytes(align).unwrap(),
                         MiriMemoryKind::C.into(),
                     );
-                    this.write_scalar(Scalar::Ptr(ptr), ret.into())?;
+                    this.write_scalar(ptr, ret.into())?;
                 }
                 this.write_null(dest)?;
             }
@@ -234,7 +234,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                     Align::from_bytes(align).unwrap(),
                     MiriMemoryKind::Rust.into(),
                 );
-                this.write_scalar(Scalar::Ptr(ptr), dest)?;
+                this.write_scalar(ptr, dest)?;
             }
             "__rust_alloc_zeroed" => {
                 let size = this.read_scalar(args[0])?.to_machine_usize(this)?;
@@ -254,7 +254,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 this.memory
                     .write_bytes(ptr.into(), iter::repeat(0u8).take(size as usize))
                     .unwrap();
-                this.write_scalar(Scalar::Ptr(ptr), dest)?;
+                this.write_scalar(ptr, dest)?;
             }
             "__rust_dealloc" => {
                 let ptr = this.read_scalar(args[0])?.not_undef()?;
@@ -295,7 +295,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                     align,
                     MiriMemoryKind::Rust.into(),
                 )?;
-                this.write_scalar(Scalar::Ptr(new_ptr), dest)?;
+                this.write_scalar(new_ptr, dest)?;
             }
 
             "syscall" => {

src/shims/panic.rs

@@ -11,11 +11,12 @@
 //!   gets popped *during unwinding*, we take the panic payload and store it according to the extra
 //!   metadata we remembered when pushing said frame.
 
+use syntax::source_map::Span;
 use rustc::mir;
-use crate::*;
-use super::machine::FrameData;
+use rustc::ty::{self, layout::LayoutOf};
 use rustc_target::spec::PanicStrategy;
-use crate::rustc_target::abi::LayoutOf;
+
+use crate::*;
 
 /// Holds all of the relevant data for a call to
 /// `__rust_maybe_catch_panic`.
@@ -85,7 +86,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             MPlaceTy::dangling(this.layout_of(tcx.mk_unit())?, this).into();
         this.call_function(
             f_instance,
-            &[f_arg],
+            &[f_arg.into()],
             Some(ret_place),
             // Directly return to caller.
             StackPopCleanup::Goto { ret: Some(ret), unwind: None },
@@ -150,4 +151,58 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         this.memory.extra.stacked_borrows.borrow_mut().end_call(extra.call_id);
         Ok(res)
     }
+
+    fn assert_panic(
+        &mut self,
+        span: Span,
+        msg: &AssertMessage<'tcx>,
+        unwind: Option<mir::BasicBlock>,
+    ) -> InterpResult<'tcx> {
+        use rustc::mir::interpret::PanicInfo::*;
+        let this = self.eval_context_mut();
+
+        match msg {
+            BoundsCheck { ref index, ref len } => {
+                // Forward to `panic_bounds_check` lang item.
+
+                // First arg: Caller location.
+                let location = this.alloc_caller_location_for_span(span);
+                // Second arg: index.
+                let index = this.read_scalar(this.eval_operand(index, None)?)?;
+                // Third arg: len.
+                let len = this.read_scalar(this.eval_operand(len, None)?)?;
+
+                // Call the lang item.
+                let panic_bounds_check = this.tcx.lang_items().panic_bounds_check_fn().unwrap();
+                let panic_bounds_check = ty::Instance::mono(this.tcx.tcx, panic_bounds_check);
+                this.call_function(
+                    panic_bounds_check,
+                    &[location.ptr.into(), index.into(), len.into()],
+                    None,
+                    StackPopCleanup::Goto { ret: None, unwind },
+                )?;
+            }
+            _ => {
+                // Forward everything else to `panic` lang item.
+
+                // First arg: Message.
+                let msg = msg.description();
+                let msg = this.allocate_str(msg, MiriMemoryKind::Env.into());
+
+                // Second arg: Caller location.
+                let location = this.alloc_caller_location_for_span(span);
+
+                // Call the lang item.
+                let panic = this.tcx.lang_items().panic_fn().unwrap();
+                let panic = ty::Instance::mono(this.tcx.tcx, panic);
+                this.call_function(
+                    panic,
+                    &[msg.to_ref(), location.ptr.into()],
+                    None,
+                    StackPopCleanup::Goto { ret: None, unwind },
+                )?;
+            }
+        }
+        Ok(())
+    }
 }

src/shims/tls.rs

@@ -150,7 +150,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             let ret_place = MPlaceTy::dangling(this.layout_of(this.tcx.mk_unit())?, this).into();
             this.call_function(
                 instance,
-                &[ptr],
+                &[ptr.into()],
                 Some(ret_place),
                 StackPopCleanup::None { cleanup: true },
             )?;

src/stacked_borrows.rs

@@ -472,12 +472,17 @@ impl Stacks {
                 // and in particular, *all* raw pointers.
                 (Tag::Tagged(extra.borrow_mut().new_ptr()), Permission::Unique),
             MemoryKind::Machine(MiriMemoryKind::Static) =>
+                // Static memory can be referenced by "global" pointers from `tcx`.
+                // Thus we call `static_base_ptr` such that the global pointers get the same tag
+                // as what we use here.
+                // The base pointer is not unique, so the base permission is `SharedReadWrite`.
                 (extra.borrow_mut().static_base_ptr(id), Permission::SharedReadWrite),
             _ =>
+                // Everything else we handle entirely untagged for now.
+                // FIXME: experiment with more precise tracking.
                 (Tag::Untagged, Permission::SharedReadWrite),
         };
-        let stack = Stacks::new(size, perm, tag, extra);
-        (stack, tag)
+        (Stacks::new(size, perm, tag, extra), tag)
     }
 
     #[inline(always)]
@@ -591,7 +596,12 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
         // Compute new borrow.
         let new_tag = match kind {
+            // Give up tracking for raw pointers.
+            // FIXME: Experiment with more precise tracking. Blocked on `&raw`
+            // because `Rc::into_raw` currently creates intermediate references,
+            // breaking `Rc::from_raw`.
             RefKind::Raw { .. } => Tag::Untagged,
+            // All other pointesr are properly tracked.
             _ => Tag::Tagged(this.memory.extra.stacked_borrows.borrow_mut().new_ptr()),
         };
 

test-cargo-miri/test.stdout.ref

@@ -5,12 +5,13 @@ test test::rng ... ok
 test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
 
 
-running 5 tests
+running 6 tests
 test do_panic ... ok
 test entropy_rng ... ok
+test fail_index_check ... ok
 test num_cpus ... ok
 test simple1 ... ok
 test simple2 ... ok
 
-test result: ok. 5 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
+test result: ok. 6 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
 

test-cargo-miri/test.stdout.ref2

@@ -7,5 +7,5 @@ test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out
 running 1 test
 test simple1 ... ok
 
-test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 4 filtered out
+test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 5 filtered out
 

test-cargo-miri/test.stdout.ref3

@@ -7,5 +7,5 @@ test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out
 running 1 test
 test num_cpus ... ok
 
-test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 4 filtered out
+test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 5 filtered out