Skip to content

rustup: Update for rustc validation fixes #473

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Oct 14, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion rust-toolchain
Original file line number Diff line number Diff line change
@@ -1 +1 @@
nightly-2018-10-11
nightly-2018-10-14
4 changes: 2 additions & 2 deletions rustc_tests/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ fn after_analysis<'a, 'tcx>(state: &mut CompileState<'a, 'tcx>) {
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();
}
}
Expand All @@ -106,7 +106,7 @@ fn after_analysis<'a, 'tcx>(state: &mut CompileState<'a, 'tcx>) {
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 {
Expand Down
56 changes: 37 additions & 19 deletions src/bin/miri.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,14 @@ use std::path::PathBuf;

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 {
Expand Down Expand Up @@ -87,7 +91,9 @@ impl<'a> CompilerCalls<'a> for MiriCompilerCalls {
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
}
Expand All @@ -101,38 +107,43 @@ 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 {
if i.attrs.iter().any(|attr| {
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();
}
}
}
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);
Expand All @@ -142,7 +153,7 @@ fn after_analysis<'a, 'tcx>(state: &mut CompileState<'a, 'tcx>, use_start_fn: bo
} 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 {
Expand Down Expand Up @@ -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
}
});

Expand All @@ -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);
Expand Down
4 changes: 2 additions & 2 deletions src/fn_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,12 +252,12 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx, 'mir> for EvalContext<'a, '
// 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();
Expand Down
14 changes: 11 additions & 3 deletions src/intrinsic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,10 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super:
"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));
Expand Down Expand Up @@ -338,7 +341,8 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super:

"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),
Expand All @@ -349,7 +353,8 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super:
"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),
Expand Down Expand Up @@ -397,6 +402,9 @@ impl<'a, 'mir, 'tcx> EvalContextExt<'tcx> for EvalContext<'a, 'mir, 'tcx, super:
"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(..) => {
Expand Down
58 changes: 47 additions & 11 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
);

Expand All @@ -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();
Expand All @@ -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 },
)?;

Expand Down Expand Up @@ -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 },
)?;

Expand All @@ -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()?;
Expand Down Expand Up @@ -222,14 +223,27 @@ impl Into<MemoryKind<MiriMemoryKind>> for 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
pub(crate) env_vars: HashMap<Vec<u8>, Pointer>,

/// 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> {
Expand All @@ -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(
Expand Down Expand Up @@ -286,7 +322,7 @@ impl<'a, 'mir, 'tcx> Machine<'a, 'mir, 'tcx> for Evaluator<'tcx> {
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.
Expand Down
11 changes: 7 additions & 4 deletions src/tls.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -139,12 +142,12 @@ impl<'a, 'mir, 'tcx: 'mir + 'a> EvalContextExt<'tcx> for EvalContext<'a, 'mir, '
// 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(
Expand Down
2 changes: 1 addition & 1 deletion tests/compile-fail/cast_box_int_to_fn_ptr.rs
Original file line number Diff line number Diff line change
@@ -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);
Expand Down
2 changes: 1 addition & 1 deletion tests/compile-fail/cast_int_to_fn_ptr.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion tests/compile-fail/execute_memory.rs
Original file line number Diff line number Diff line change
@@ -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)]

Expand Down
2 changes: 1 addition & 1 deletion tests/compile-fail/fn_ptr_offset.rs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down
Loading