Merge pull request #473 from solson/rustup

rustup: Update for rustc validation fixes
This commit is contained in:
Ralf Jung 2018-10-14 11:29:14 +02:00 committed by GitHub
commit 8b14b03368
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
28 changed files with 167 additions and 83 deletions

View File

@ -1 +1 @@
nightly-2018-10-11
nightly-2018-10-14

View File

@ -95,7 +95,7 @@ fn visit_item(&mut self, i: &'hir hir::Item) {
if i.attrs.iter().any(|attr| attr.name() == "test") {
let did = self.0.hir.body_owner_def_id(body_id);
println!("running test: {}", self.0.def_path_debug_str(did));
miri::eval_main(self.0, did, None);
miri::eval_main(self.0, did, None, /*validate*/true);
self.1.session.abort_if_errors();
}
}
@ -106,7 +106,7 @@ fn visit_impl_item(&mut self, _impl_item: &'hir hir::ImplItem) {}
state.hir_crate.unwrap().visit_all_item_likes(&mut Visitor(tcx, state));
} else if let Some((entry_node_id, _, _)) = *state.session.entry_fn.borrow() {
let entry_def_id = tcx.hir.local_def_id(entry_node_id);
miri::eval_main(tcx, entry_def_id, None);
miri::eval_main(tcx, entry_def_id, None, /*validate*/true);
state.session.abort_if_errors();
} else {

View File

@ -25,10 +25,14 @@
struct MiriCompilerCalls {
default: Box<RustcDefaultCalls>,
/// Whether to begin interpretation at the start_fn lang item or not
/// Whether to begin interpretation at the start_fn lang item or not.
///
/// If false, the interpretation begins at the `main` function
/// If false, the interpretation begins at the `main` function.
start_fn: bool,
/// Whether to enforce the validity invariant.
validate: bool,
}
impl<'a> CompilerCalls<'a> for MiriCompilerCalls {
@ -87,7 +91,9 @@ fn build_controller(
let mut control = this.default.build_controller(sess, matches);
control.after_hir_lowering.callback = Box::new(after_hir_lowering);
let start_fn = this.start_fn;
control.after_analysis.callback = Box::new(move |state| after_analysis(state, start_fn));
let validate = this.validate;
control.after_analysis.callback =
Box::new(move |state| after_analysis(state, start_fn, validate));
control.after_analysis.stop = Compilation::Stop;
control
}
@ -101,16 +107,21 @@ fn after_hir_lowering(state: &mut CompileState) {
state.session.plugin_attributes.borrow_mut().push(attr);
}
fn after_analysis<'a, 'tcx>(state: &mut CompileState<'a, 'tcx>, use_start_fn: bool) {
fn after_analysis<'a, 'tcx>(
state: &mut CompileState<'a, 'tcx>,
use_start_fn: bool,
validate: bool,
) {
state.session.abort_if_errors();
let tcx = state.tcx.unwrap();
if std::env::args().any(|arg| arg == "--test") {
struct Visitor<'a, 'tcx: 'a>(
TyCtxt<'a, 'tcx, 'tcx>,
&'a CompileState<'a, 'tcx>
);
struct Visitor<'a, 'tcx: 'a> {
tcx: TyCtxt<'a, 'tcx, 'tcx>,
state: &'a CompileState<'a, 'tcx>,
validate: bool,
};
impl<'a, 'tcx: 'a, 'hir> itemlikevisit::ItemLikeVisitor<'hir> for Visitor<'a, 'tcx> {
fn visit_item(&mut self, i: &'hir hir::Item) {
if let hir::ItemKind::Fn(.., body_id) = i.node {
@ -118,13 +129,13 @@ fn visit_item(&mut self, i: &'hir hir::Item) {
attr.name() == "test"
})
{
let did = self.0.hir.body_owner_def_id(body_id);
let did = self.tcx.hir.body_owner_def_id(body_id);
println!(
"running test: {}",
self.0.def_path_debug_str(did),
self.tcx.def_path_debug_str(did),
);
miri::eval_main(self.0, did, None);
self.1.session.abort_if_errors();
miri::eval_main(self.tcx, did, None, self.validate);
self.state.session.abort_if_errors();
}
}
}
@ -132,7 +143,7 @@ fn visit_trait_item(&mut self, _trait_item: &'hir hir::TraitItem) {}
fn visit_impl_item(&mut self, _impl_item: &'hir hir::ImplItem) {}
}
state.hir_crate.unwrap().visit_all_item_likes(
&mut Visitor(tcx, state),
&mut Visitor { tcx, state, validate }
);
} else if let Some((entry_node_id, _, _)) = *state.session.entry_fn.borrow() {
let entry_def_id = tcx.hir.local_def_id(entry_node_id);
@ -142,7 +153,7 @@ fn visit_impl_item(&mut self, _impl_item: &'hir hir::ImplItem) {}
} else {
None
};
miri::eval_main(tcx, entry_def_id, start_wrapper);
miri::eval_main(tcx, entry_def_id, start_wrapper, validate);
state.session.abort_if_errors();
} else {
@ -221,12 +232,18 @@ fn main() {
}
let mut start_fn = false;
let mut validate = true;
args.retain(|arg| {
if arg == "-Zmiri-start-fn" {
start_fn = true;
false
} else {
true
match arg.as_str() {
"-Zmiri-start-fn" => {
start_fn = true;
false
},
"-Zmiri-disable-validation" => {
validate = false;
false
},
_ => true
}
});
@ -235,6 +252,7 @@ fn main() {
rustc_driver::run_compiler(&args, Box::new(MiriCompilerCalls {
default: Box::new(RustcDefaultCalls),
start_fn,
validate,
}), None, None)
});
std::process::exit(result as i32);

View File

@ -252,12 +252,12 @@ fn emulate_foreign_item(
// Now we make a function call. TODO: Consider making this re-usable? EvalContext::step does sth. similar for the TLS dtors,
// and of course eval_main.
let mir = self.load_mir(f_instance.def)?;
let closure_dest = Place::null(&self);
let ret_place = MPlaceTy::dangling(self.layout_of(self.tcx.mk_unit())?, &self).into();
self.push_stack_frame(
f_instance,
mir.span,
mir,
closure_dest,
Some(ret_place),
StackPopCleanup::Goto(Some(ret)), // directly return to caller
)?;
let mut args = self.frame().mir.args_iter();

View File

@ -238,7 +238,10 @@ fn call_intrinsic(
"init" => {
// Check fast path: we don't want to force an allocation in case the destination is a simple value,
// but we also do not want to create a new allocation with 0s and then copy that over.
if !dest.layout.is_zst() { // notzhing to do for ZST
// FIXME: We do not properly validate in case of ZSTs and when doing it in memory!
// However, this only affects direct calls of the intrinsic; calls to the stable
// functions wrapping them do get their validation.
if !dest.layout.is_zst() { // nothing to do for ZST
match dest.layout.abi {
layout::Abi::Scalar(ref s) => {
let x = Scalar::from_int(0, s.value.size(&self));
@ -338,7 +341,8 @@ fn call_intrinsic(
"size_of_val" => {
let mplace = self.ref_to_mplace(self.read_value(args[0])?)?;
let (size, _) = self.size_and_align_of_mplace(mplace)?;
let (size, _) = self.size_and_align_of_mplace(mplace)?
.expect("size_of_val called on extern type");
let ptr_size = self.pointer_size();
self.write_scalar(
Scalar::from_uint(size.bytes() as u128, ptr_size),
@ -349,7 +353,8 @@ fn call_intrinsic(
"min_align_of_val" |
"align_of_val" => {
let mplace = self.ref_to_mplace(self.read_value(args[0])?)?;
let (_, align) = self.size_and_align_of_mplace(mplace)?;
let (_, align) = self.size_and_align_of_mplace(mplace)?
.expect("size_of_val called on extern type");
let ptr_size = self.pointer_size();
self.write_scalar(
Scalar::from_uint(align.abi(), ptr_size),
@ -397,6 +402,9 @@ fn call_intrinsic(
"uninit" => {
// Check fast path: we don't want to force an allocation in case the destination is a simple value,
// but we also do not want to create a new allocation with 0s and then copy that over.
// FIXME: We do not properly validate in case of ZSTs and when doing it in memory!
// However, this only affects direct calls of the intrinsic; calls to the stable
// functions wrapping them do get their validation.
if !dest.layout.is_zst() { // nothing to do for ZST
match dest.layout.abi {
layout::Abi::Scalar(..) => {

View File

@ -50,11 +50,12 @@ pub fn create_ecx<'a, 'mir: 'a, 'tcx: 'mir>(
tcx: TyCtxt<'a, 'tcx, 'tcx>,
main_id: DefId,
start_wrapper: Option<DefId>,
validate: bool,
) -> EvalResult<'tcx, EvalContext<'a, 'mir, 'tcx, Evaluator<'tcx>>> {
let mut ecx = EvalContext::new(
tcx.at(syntax::source_map::DUMMY_SP),
ty::ParamEnv::reveal_all(),
Default::default(),
Evaluator::new(validate),
Default::default(),
);
@ -67,7 +68,6 @@ pub fn create_ecx<'a, 'mir: 'a, 'tcx: 'mir>(
.to_owned(),
));
}
let ptr_size = ecx.memory.pointer_size();
if let Some(start_id) = start_wrapper {
let main_ret_ty = ecx.tcx.fn_sig(main_id).output();
@ -89,16 +89,15 @@ pub fn create_ecx<'a, 'mir: 'a, 'tcx: 'mir>(
}
// Return value (in static memory so that it does not count as leak)
let size = ecx.tcx.data_layout.pointer_size;
let align = ecx.tcx.data_layout.pointer_align;
let ret_ptr = ecx.memory_mut().allocate(size, align, MiriMemoryKind::MutStatic.into())?;
let ret = ecx.layout_of(start_mir.return_ty())?;
let ret_ptr = ecx.allocate(ret, MiriMemoryKind::MutStatic.into())?;
// Push our stack frame
ecx.push_stack_frame(
start_instance,
start_mir.span,
start_mir,
Place::from_ptr(ret_ptr, align),
Some(ret_ptr.into()),
StackPopCleanup::None { cleanup: true },
)?;
@ -126,11 +125,12 @@ pub fn create_ecx<'a, 'mir: 'a, 'tcx: 'mir>(
assert!(args.next().is_none(), "start lang item has more arguments than expected");
} else {
let ret_place = MPlaceTy::dangling(ecx.layout_of(tcx.mk_unit())?, &ecx).into();
ecx.push_stack_frame(
main_instance,
main_mir.span,
main_mir,
Place::from_scalar_ptr(Scalar::from_int(1, ptr_size).into(), ty::layout::Align::from_bytes(1, 1).unwrap()),
Some(ret_place),
StackPopCleanup::None { cleanup: true },
)?;
@ -146,8 +146,9 @@ pub fn eval_main<'a, 'tcx: 'a>(
tcx: TyCtxt<'a, 'tcx, 'tcx>,
main_id: DefId,
start_wrapper: Option<DefId>,
validate: bool,
) {
let mut ecx = create_ecx(tcx, main_id, start_wrapper).expect("Couldn't create ecx");
let mut ecx = create_ecx(tcx, main_id, start_wrapper, validate).expect("Couldn't create ecx");
let res: EvalResult = (|| {
ecx.run()?;
@ -222,7 +223,7 @@ fn into(self) -> MemoryKind<MiriMemoryKind> {
}
#[derive(Clone, Default, PartialEq, Eq)]
#[derive(Clone, PartialEq, Eq)]
pub struct Evaluator<'tcx> {
/// Environment variables set by `setenv`
/// Miri does not expose env vars from the host to the emulated program
@ -230,6 +231,19 @@ pub struct Evaluator<'tcx> {
/// TLS state
pub(crate) tls: TlsData<'tcx>,
/// Whether to enforce the validity invariant
pub(crate) validate: bool,
}
impl<'tcx> Evaluator<'tcx> {
fn new(validate: bool) -> Self {
Evaluator {
env_vars: HashMap::default(),
tls: TlsData::default(),
validate,
}
}
}
impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> {
@ -240,7 +254,29 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> {
type MemoryMap = MonoHashMap<AllocId, (MemoryKind<MiriMemoryKind>, Allocation<()>)>;
const STATIC_KIND: Option<MiriMemoryKind> = Some(MiriMemoryKind::MutStatic);
const ENFORCE_VALIDITY: bool = false; // this is still WIP
fn enforce_validity(ecx: &EvalContext<'a, 'mir, 'tcx, Self>) -> bool {
if !ecx.machine.validate {
return false;
}
// Some functions are whitelisted until we figure out how to fix them.
// We walk up the stack a few frames to also cover their callees.
const WHITELIST: &[&str] = &[
// Uses mem::uninitialized
"std::ptr::read",
"std::sys::windows::mutex::Mutex::",
];
for frame in ecx.stack().iter()
.rev().take(3)
{
let name = frame.instance.to_string();
if WHITELIST.iter().any(|white| name.starts_with(white)) {
return false;
}
}
true
}
/// Returns Ok() when the function was handled, fail otherwise
fn find_fn(
@ -286,7 +322,7 @@ fn box_alloc(
malloc,
malloc_mir.span,
malloc_mir,
*dest,
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
// exchange_malloc returns, we go on evaluating exactly where we want to be.

View File

@ -1,9 +1,12 @@
use std::collections::BTreeMap;
use rustc_target::abi::LayoutOf;
use rustc::{ty, ty::layout::HasDataLayout, mir};
use super::{EvalResult, EvalErrorKind, Scalar, Evaluator,
Place, StackPopCleanup, EvalContext};
use super::{
EvalResult, EvalErrorKind, StackPopCleanup, EvalContext, Evaluator,
MPlaceTy, Scalar,
};
pub type TlsKey = u128;
@ -139,12 +142,12 @@ fn run_tls_dtors(&mut self) -> EvalResult<'tcx> {
// TODO: Potentially, this has to support all the other possible instances?
// See eval_fn_call in interpret/terminator/mod.rs
let mir = self.load_mir(instance.def)?;
let ret = Place::null(&self);
let ret_place = MPlaceTy::dangling(self.layout_of(self.tcx.mk_unit())?, &self).into();
self.push_stack_frame(
instance,
mir.span,
mir,
ret,
Some(ret_place),
StackPopCleanup::None { cleanup: true },
)?;
let arg_local = self.frame().mir.args_iter().next().ok_or_else(

View File

@ -1,5 +1,5 @@
// Validation makes this fail in the wrong place
// compile-flags: -Zmir-emit-validate=0
// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation
fn main() {
let b = Box::new(42);

View File

@ -1,5 +1,5 @@
// Validation makes this fail in the wrong place
// compile-flags: -Zmir-emit-validate=0
// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation
fn main() {
let g = unsafe {

View File

@ -1,5 +1,5 @@
// Validation makes this fail in the wrong place
// compile-flags: -Zmir-emit-validate=0
// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation
#![feature(box_syntax)]

View File

@ -1,5 +1,5 @@
// Validation makes this fail in the wrong place
// compile-flags: -Zmir-emit-validate=0
// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation
use std::mem;

View File

@ -1,9 +1,3 @@
//ignore-test FIXME: do some basic validation of invariants for all values in flight
//This does currently not get caught becuase it compiles to SwitchInt, which
//has no knowledge about data invariants.
fn main() {
let b = unsafe { std::mem::transmute::<u8, bool>(2) };
if b { unreachable!() } else { unreachable!() } //~ ERROR constant evaluation error
//~^ NOTE invalid boolean value read
let _b = unsafe { std::mem::transmute::<u8, bool>(2) }; //~ ERROR encountered 2, but expected something in the range 0..=1
}

View File

@ -1,3 +1,6 @@
// Validation makes this fail in the wrong place
// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation
fn main() {
let b = unsafe { std::mem::transmute::<u8, bool>(2) };
let _x = b == true; //~ ERROR invalid boolean value read

View File

@ -0,0 +1,8 @@
fn main() {
assert!(std::char::from_u32(-1_i32 as u32).is_none());
let _ = match unsafe { std::mem::transmute::<i32, char>(-1) } { //~ ERROR encountered 4294967295, but expected something in the range 0..=1114111
'a' => {true},
'b' => {false},
_ => {true},
};
}

View File

@ -1,3 +1,6 @@
// Validation makes this fail in the wrong place
// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation
fn main() {
assert!(std::char::from_u32(-1_i32 as u32).is_none());
let c = unsafe { std::mem::transmute::<i32, char>(-1) };

View File

@ -1,17 +1,8 @@
// Validation makes this fail in the wrong place
// compile-flags: -Zmir-emit-validate=0
#[repr(C)]
pub enum Foo {
A, B, C, D
}
fn main() {
let f = unsafe { std::mem::transmute::<i32, Foo>(42) };
match f {
Foo::A => {}, //~ ERROR invalid enum discriminant
Foo::B => {},
Foo::C => {},
Foo::D => {},
}
let _f = unsafe { std::mem::transmute::<i32, Foo>(42) }; //~ ERROR encountered invalid enum discriminant 42
}

View File

@ -1,5 +1,5 @@
// Validation makes this fail in the wrong place
// compile-flags: -Zmir-emit-validate=0
// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation
// error-pattern: invalid enum discriminant

View File

@ -1,13 +0,0 @@
//ignore-test FIXME: do some basic validation of invariants for all values in flight
//This does currently not get caught becuase it compiles to SwitchInt, which
//has no knowledge about data invariants.
fn main() {
assert!(std::char::from_u32(-1_i32 as u32).is_none());
let _ = match unsafe { std::mem::transmute::<i32, char>(-1) } { //~ ERROR constant evaluation error
//~^ NOTE tried to interpret an invalid 32-bit value as a char: 4294967295
'a' => {true},
'b' => {false},
_ => {true},
};
}

View File

@ -1,5 +1,5 @@
// This should fail even without validation
// compile-flags: -Zmir-emit-validate=0
// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation
#![feature(never_type)]
#![allow(unreachable_code)]

View File

@ -1,5 +1,5 @@
// This should fail even without validation
// compile-flags: -Zmir-emit-validate=0
// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation
#![feature(never_type)]
#![allow(unreachable_code)]

View File

@ -1,5 +1,5 @@
// This should fail even without validation
// compile-flags: -Zmir-emit-validate=0
// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation
#![feature(never_type)]
#![allow(unreachable_code)]

View File

@ -1,5 +1,5 @@
// This should fail even without validation
// compile-flags: -Zmir-emit-validate=0
// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation
#![allow(dead_code, unused_variables)]

View File

@ -1,3 +1,6 @@
// This should fail even without validation
// compile-flags: -Zmir-emit-validate=0 -Zmiri-disable-validation
static mut LEAK: usize = 0;
fn fill(v: &mut i32) {

View File

@ -0,0 +1,10 @@
fn main() {
// Cast a function pointer such that on a call, the argument gets transmuted
// from raw ptr to reference. This is ABI-compatible, so it's not the call that
// should fail, but validation should.
fn f(_x: &i32) { }
let g: fn(*const i32) = unsafe { std::mem::transmute(f as fn(&i32)) };
g(0usize as *const i32) //~ ERROR encountered 0, but expected something greater or equal to 1
}

View File

@ -0,0 +1,10 @@
fn main() {
// Cast a function pointer such that when returning, the return value gets transmuted
// from raw ptr to reference. This is ABI-compatible, so it's not the call that
// should fail, but validation should.
fn f() -> *const i32 { 0usize as *const i32 }
let g: fn() -> &'static i32 = unsafe { std::mem::transmute(f as fn() -> *const i32) };
let _x = g(); //~ ERROR encountered 0, but expected something greater or equal to 1
}

View File

@ -1,5 +1,5 @@
// mir validation can't cope with `mem::uninitialized()`, so this test fails with validation & full-MIR.
// compile-flags: -Zmir-emit-validate=0
// FIXME: Validation disabled due to https://github.com/rust-lang/rust/issues/54957
// compile-flags: -Zmiri-disable-validation
#[derive(PartialEq, Eq, PartialOrd, Ord)]
pub enum Foo {
@ -14,4 +14,6 @@ pub fn main() {
b.insert(Foo::A("/="));
b.insert(Foo::A("#"));
b.insert(Foo::A("0o"));
assert!(b.remove(&Foo::A("/=")));
assert!(!b.remove(&Foo::A("/=")));
}

View File

@ -1,3 +1,6 @@
// FIXME: Validation disabled due to https://github.com/rust-lang/rust/issues/54908
// compile-flags: -Zmiri-disable-validation
use std::cell::RefCell;
use std::rc::Rc;

View File

@ -1,7 +1,12 @@
// FIXME validation disabled because it checks these references too eagerly
// compile-flags: -Zmiri-disable-validation
fn main() {
let x = 2usize as *const u32;
// this is not aligned, but we immediately cast it to a raw ptr so that must be okay
let _y = unsafe { &*x as *const u32 };
let x = 0usize as *const u32;
// this is NULL, but we immediately cast it to a raw ptr so that must be okay
let _y = unsafe { &*x as *const u32 };
}