Skip to content

Commit c00b4b3

Browse files
borsbors
committed
Auto merge of #1084 - RalfJung:assert-panic, r=RalfJung
proper support for `Assert` MIR terminators Fixes #1070 Blocked on rust-lang/rust#66874
2 parents 3c0d343 + 2ef5ac1 commit c00b4b3

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+195
-69
lines changed

‎rust-version

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
4af3ee8ee2a2bc1286b021db7600ba990359cf3f
1+
2da942f32802c8233a09744024dfbc34431adf65

‎src/eval.rs

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
9393
arg.push(0);
9494
argvs.push(
9595
ecx.memory
96-
.allocate_static_bytes(arg.as_slice(), MiriMemoryKind::Static.into()),
96+
.allocate_static_bytes(arg.as_slice(), MiriMemoryKind::Env.into()),
9797
);
9898
}
9999
// Make an array with all these pointers, in the Miri memory.
@@ -103,7 +103,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
103103
let argvs_place = ecx.allocate(argvs_layout, MiriMemoryKind::Env.into());
104104
for (idx, arg) in argvs.into_iter().enumerate() {
105105
let place = ecx.mplace_field(argvs_place, idx as u64)?;
106-
ecx.write_scalar(Scalar::Ptr(arg), place.into())?;
106+
ecx.write_scalar(arg, place.into())?;
107107
}
108108
ecx.memory
109109
.mark_immutable(argvs_place.ptr.assert_ptr().alloc_id)?;
@@ -144,19 +144,19 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
144144
// Return place (in static memory so that it does not count as leak).
145145
let ret_place = ecx.allocate(
146146
ecx.layout_of(tcx.types.isize)?,
147-
MiriMemoryKind::Static.into(),
147+
MiriMemoryKind::Env.into(),
148148
);
149149
// Call start function.
150150
ecx.call_function(
151151
start_instance,
152-
&[main_ptr.into(), argc, argv],
152+
&[main_ptr.into(), argc.into(), argv.into()],
153153
Some(ret_place.into()),
154154
StackPopCleanup::None { cleanup: true },
155155
)?;
156156

157157
// Set the last_error to 0
158158
let errno_layout = ecx.layout_of(tcx.types.u32)?;
159-
let errno_place = ecx.allocate(errno_layout, MiriMemoryKind::Static.into());
159+
let errno_place = ecx.allocate(errno_layout, MiriMemoryKind::Env.into());
160160
ecx.write_scalar(Scalar::from_u32(0), errno_place.into())?;
161161
ecx.machine.last_error = Some(errno_place);
162162

@@ -217,16 +217,13 @@ pub fn eval_main<'tcx>(tcx: TyCtxt<'tcx>, main_id: DefId, config: MiriConfig) ->
217217
}
218218
err_unsup!(NoMirFor(..)) =>
219219
format!("{}. Did you set `MIRI_SYSROOT` to a Miri-enabled sysroot? You can prepare one with `cargo miri setup`.", e),
220+
InterpError::InvalidProgram(_) =>
221+
bug!("This error should be impossible in Miri: {}", e),
220222
_ => e.to_string()
221223
};
222224
e.print_backtrace();
223225
if let Some(frame) = ecx.stack().last() {
224-
let block = &frame.body.basic_blocks()[frame.block.unwrap()];
225-
let span = if frame.stmt < block.statements.len() {
226-
block.statements[frame.stmt].source_info.span
227-
} else {
228-
block.terminator().source_info.span
229-
};
226+
let span = frame.current_source_info().unwrap().span;
230227

231228
let msg = format!("Miri evaluation error: {}", msg);
232229
let mut err = ecx.tcx.sess.struct_span_err(span, msg.as_str());

‎src/helpers.rs

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,17 +124,21 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
124124
fn call_function(
125125
&mut self,
126126
f: ty::Instance<'tcx>,
127-
args: &[Scalar<Tag>],
127+
args: &[Immediate<Tag>],
128128
dest: Option<PlaceTy<'tcx, Tag>>,
129129
stack_pop: StackPopCleanup,
130130
) -> InterpResult<'tcx> {
131131
let this = self.eval_context_mut();
132132

133133
// Push frame.
134134
let mir = this.load_mir(f.def, None)?;
135+
let span = this.stack().last()
136+
.and_then(Frame::current_source_info)
137+
.map(|si| si.span)
138+
.unwrap_or(DUMMY_SP);
135139
this.push_stack_frame(
136140
f,
137-
DUMMY_SP, // There is no call site.
141+
span,
138142
mir,
139143
dest,
140144
stack_pop,
@@ -146,7 +150,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
146150
let callee_arg = this.local_place(
147151
callee_args.next().expect("callee has fewer arguments than expected")
148152
)?;
149-
this.write_scalar(*arg, callee_arg)?;
153+
this.write_immediate(*arg, callee_arg)?;
150154
}
151155
callee_args.next().expect_none("callee has more arguments than expected");
152156

‎src/lib.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,15 @@ pub use crate::shims::dlsym::{Dlsym, EvalContextExt as DlsymEvalContextExt};
3838
pub use crate::shims::env::{EnvVars, EvalContextExt as EnvEvalContextExt};
3939
pub use crate::shims::fs::{FileHandler, EvalContextExt as FileEvalContextExt};
4040
pub use crate::shims::panic::{CatchUnwindData, EvalContextExt as PanicEvalContextExt};
41+
4142
pub use crate::operator::EvalContextExt as OperatorEvalContextExt;
4243
pub use crate::range_map::RangeMap;
4344
pub use crate::helpers::{EvalContextExt as HelpersEvalContextExt};
4445
pub use crate::mono_hash_map::MonoHashMap;
4546
pub use crate::stacked_borrows::{EvalContextExt as StackedBorEvalContextExt, Tag, Permission, Stack, Stacks, Item};
4647
pub use crate::machine::{
4748
PAGE_SIZE, STACK_ADDR, STACK_SIZE, NUM_CPUS,
48-
MemoryExtra, AllocExtra, MiriMemoryKind, Evaluator, MiriEvalContext, MiriEvalContextExt,
49+
MemoryExtra, AllocExtra, FrameData, MiriMemoryKind, Evaluator, MiriEvalContext, MiriEvalContextExt,
4950
};
5051
pub use crate::eval::{eval_main, create_ecx, MiriConfig, TerminationInfo};
5152

‎src/machine.rs

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,9 @@ pub enum MiriMemoryKind {
4343
C,
4444
/// Windows `HeapAlloc` memory.
4545
WinHeap,
46-
/// Part of env var emulation.
46+
/// Memory for env vars and args, errno and other parts of the machine-managed environment.
4747
Env,
48-
/// Statics.
48+
/// Rust statics.
4949
Static,
5050
}
5151

@@ -215,6 +215,16 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> {
215215
ecx.call_intrinsic(span, instance, args, ret, unwind)
216216
}
217217

218+
#[inline(always)]
219+
fn assert_panic(
220+
ecx: &mut InterpCx<'mir, 'tcx, Self>,
221+
span: Span,
222+
msg: &AssertMessage<'tcx>,
223+
unwind: Option<mir::BasicBlock>,
224+
) -> InterpResult<'tcx> {
225+
ecx.assert_panic(span, msg, unwind)
226+
}
227+
218228
#[inline(always)]
219229
fn binary_ptr_op(
220230
ecx: &rustc_mir::interpret::InterpCx<'mir, 'tcx, Self>,
@@ -243,7 +253,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> {
243253
let malloc = ty::Instance::mono(ecx.tcx.tcx, malloc);
244254
ecx.call_function(
245255
malloc,
246-
&[size, align],
256+
&[size.into(), align.into()],
247257
Some(dest),
248258
// Don't do anything when we are done. The `statement()` function will increment
249259
// 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> {
281291
Ok(())
282292
}
283293

284-
fn tag_allocation<'b>(
294+
fn init_allocation_extra<'b>(
285295
memory_extra: &MemoryExtra,
286296
id: AllocId,
287297
alloc: Cow<'b, Allocation>,
288298
kind: Option<MemoryKind<Self::MemoryKinds>>,
289-
) -> (
290-
Cow<'b, Allocation<Self::PointerTag, Self::AllocExtra>>,
291-
Self::PointerTag,
292-
) {
299+
) -> (Cow<'b, Allocation<Self::PointerTag, Self::AllocExtra>>, Self::PointerTag) {
293300
let kind = kind.expect("we set our STATIC_KIND so this cannot be None");
294301
let alloc = alloc.into_owned();
295-
let (stacks, base_tag) = if !memory_extra.validate {
296-
(None, Tag::Untagged)
297-
} else {
302+
let (stacks, base_tag) = if memory_extra.validate {
298303
let (stacks, base_tag) = Stacks::new_allocation(
299304
id,
300305
alloc.size,
301306
Rc::clone(&memory_extra.stacked_borrows),
302307
kind,
303308
);
304309
(Some(stacks), base_tag)
310+
} else {
311+
// No stacks, no tag.
312+
(None, Tag::Untagged)
305313
};
306314
let mut stacked_borrows = memory_extra.stacked_borrows.borrow_mut();
307315
let alloc: Allocation<Tag, Self::AllocExtra> = alloc.with_tags_and_extra(

‎src/shims/env.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
5757
Ok(match this.machine.env_vars.map.get(name) {
5858
// The offset is used to strip the "{name}=" part of the string.
5959
Some(var_ptr) => {
60-
Scalar::Ptr(var_ptr.offset(Size::from_bytes(name.len() as u64 + 1), this)?)
60+
Scalar::from(var_ptr.offset(Size::from_bytes(name.len() as u64 + 1), this)?)
6161
}
6262
None => Scalar::ptr_null(&*this.tcx),
6363
})

‎src/shims/foreign_items.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
205205
Align::from_bytes(align).unwrap(),
206206
MiriMemoryKind::C.into(),
207207
);
208-
this.write_scalar(Scalar::Ptr(ptr), ret.into())?;
208+
this.write_scalar(ptr, ret.into())?;
209209
}
210210
this.write_null(dest)?;
211211
}
@@ -234,7 +234,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
234234
Align::from_bytes(align).unwrap(),
235235
MiriMemoryKind::Rust.into(),
236236
);
237-
this.write_scalar(Scalar::Ptr(ptr), dest)?;
237+
this.write_scalar(ptr, dest)?;
238238
}
239239
"__rust_alloc_zeroed" => {
240240
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
254254
this.memory
255255
.write_bytes(ptr.into(), iter::repeat(0u8).take(size as usize))
256256
.unwrap();
257-
this.write_scalar(Scalar::Ptr(ptr), dest)?;
257+
this.write_scalar(ptr, dest)?;
258258
}
259259
"__rust_dealloc" => {
260260
let ptr = this.read_scalar(args[0])?.not_undef()?;
@@ -295,7 +295,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
295295
align,
296296
MiriMemoryKind::Rust.into(),
297297
)?;
298-
this.write_scalar(Scalar::Ptr(new_ptr), dest)?;
298+
this.write_scalar(new_ptr, dest)?;
299299
}
300300

301301
"syscall" => {

‎src/shims/panic.rs

Lines changed: 59 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,12 @@
1111
//! gets popped *during unwinding*, we take the panic payload and store it according to the extra
1212
//! metadata we remembered when pushing said frame.
1313
14+
use syntax::source_map::Span;
1415
use rustc::mir;
15-
use crate::*;
16-
use super::machine::FrameData;
16+
use rustc::ty::{self, layout::LayoutOf};
1717
use rustc_target::spec::PanicStrategy;
18-
use crate::rustc_target::abi::LayoutOf;
18+
19+
use crate::*;
1920

2021
/// Holds all of the relevant data for a call to
2122
/// `__rust_maybe_catch_panic`.
@@ -85,7 +86,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
8586
MPlaceTy::dangling(this.layout_of(tcx.mk_unit())?, this).into();
8687
this.call_function(
8788
f_instance,
88-
&[f_arg],
89+
&[f_arg.into()],
8990
Some(ret_place),
9091
// Directly return to caller.
9192
StackPopCleanup::Goto { ret: Some(ret), unwind: None },
@@ -150,4 +151,58 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
150151
this.memory.extra.stacked_borrows.borrow_mut().end_call(extra.call_id);
151152
Ok(res)
152153
}
154+
155+
fn assert_panic(
156+
&mut self,
157+
span: Span,
158+
msg: &AssertMessage<'tcx>,
159+
unwind: Option<mir::BasicBlock>,
160+
) -> InterpResult<'tcx> {
161+
use rustc::mir::interpret::PanicInfo::*;
162+
let this = self.eval_context_mut();
163+
164+
match msg {
165+
BoundsCheck { ref index, ref len } => {
166+
// Forward to `panic_bounds_check` lang item.
167+
168+
// First arg: Caller location.
169+
let location = this.alloc_caller_location_for_span(span);
170+
// Second arg: index.
171+
let index = this.read_scalar(this.eval_operand(index, None)?)?;
172+
// Third arg: len.
173+
let len = this.read_scalar(this.eval_operand(len, None)?)?;
174+
175+
// Call the lang item.
176+
let panic_bounds_check = this.tcx.lang_items().panic_bounds_check_fn().unwrap();
177+
let panic_bounds_check = ty::Instance::mono(this.tcx.tcx, panic_bounds_check);
178+
this.call_function(
179+
panic_bounds_check,
180+
&[location.ptr.into(), index.into(), len.into()],
181+
None,
182+
StackPopCleanup::Goto { ret: None, unwind },
183+
)?;
184+
}
185+
_ => {
186+
// Forward everything else to `panic` lang item.
187+
188+
// First arg: Message.
189+
let msg = msg.description();
190+
let msg = this.allocate_str(msg, MiriMemoryKind::Env.into());
191+
192+
// Second arg: Caller location.
193+
let location = this.alloc_caller_location_for_span(span);
194+
195+
// Call the lang item.
196+
let panic = this.tcx.lang_items().panic_fn().unwrap();
197+
let panic = ty::Instance::mono(this.tcx.tcx, panic);
198+
this.call_function(
199+
panic,
200+
&[msg.to_ref(), location.ptr.into()],
201+
None,
202+
StackPopCleanup::Goto { ret: None, unwind },
203+
)?;
204+
}
205+
}
206+
Ok(())
207+
}
153208
}

‎src/shims/tls.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
150150
let ret_place = MPlaceTy::dangling(this.layout_of(this.tcx.mk_unit())?, this).into();
151151
this.call_function(
152152
instance,
153-
&[ptr],
153+
&[ptr.into()],
154154
Some(ret_place),
155155
StackPopCleanup::None { cleanup: true },
156156
)?;

‎src/stacked_borrows.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -472,12 +472,17 @@ impl Stacks {
472472
// and in particular, *all* raw pointers.
473473
(Tag::Tagged(extra.borrow_mut().new_ptr()), Permission::Unique),
474474
MemoryKind::Machine(MiriMemoryKind::Static) =>
475+
// Static memory can be referenced by "global" pointers from `tcx`.
476+
// Thus we call `static_base_ptr` such that the global pointers get the same tag
477+
// as what we use here.
478+
// The base pointer is not unique, so the base permission is `SharedReadWrite`.
475479
(extra.borrow_mut().static_base_ptr(id), Permission::SharedReadWrite),
476480
_ =>
481+
// Everything else we handle entirely untagged for now.
482+
// FIXME: experiment with more precise tracking.
477483
(Tag::Untagged, Permission::SharedReadWrite),
478484
};
479-
let stack = Stacks::new(size, perm, tag, extra);
480-
(stack, tag)
485+
(Stacks::new(size, perm, tag, extra), tag)
481486
}
482487

483488
#[inline(always)]
@@ -591,7 +596,12 @@ trait EvalContextPrivExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
591596

592597
// Compute new borrow.
593598
let new_tag = match kind {
599+
// Give up tracking for raw pointers.
600+
// FIXME: Experiment with more precise tracking. Blocked on `&raw`
601+
// because `Rc::into_raw` currently creates intermediate references,
602+
// breaking `Rc::from_raw`.
594603
RefKind::Raw { .. } => Tag::Untagged,
604+
// All other pointesr are properly tracked.
595605
_ => Tag::Tagged(this.memory.extra.stacked_borrows.borrow_mut().new_ptr()),
596606
};
597607

‎test-cargo-miri/test.stdout.ref

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,13 @@ test test::rng ... ok
55
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
66

77

8-
running 5 tests
8+
running 6 tests
99
test do_panic ... ok
1010
test entropy_rng ... ok
11+
test fail_index_check ... ok
1112
test num_cpus ... ok
1213
test simple1 ... ok
1314
test simple2 ... ok
1415

15-
test result: ok. 5 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
16+
test result: ok. 6 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out
1617

‎test-cargo-miri/test.stdout.ref2

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,5 @@ test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out
77
running 1 test
88
test simple1 ... ok
99

10-
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 4 filtered out
10+
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 5 filtered out
1111

‎test-cargo-miri/test.stdout.ref3

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,5 +7,5 @@ test result: ok. 0 passed; 0 failed; 0 ignored; 0 measured; 1 filtered out
77
running 1 test
88
test num_cpus ... ok
99

10-
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 4 filtered out
10+
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 5 filtered out
1111

0 commit comments

Comments
 (0)