Skip to content
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

Implement non-deterministc mode #683

Merged
merged 21 commits into from
Apr 10, 2019
Merged
Show file tree
Hide file tree
Changes from 17 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: 2 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ shell-escape = "0.1.4"
# See the `src/tools/rustc-workspace-hack/README.md` file in `rust-lang/rust`
# for more information.
rustc-workspace-hack = "1.0.0"
hex = "0.3.2"
rand = "0.6.5"

[build-dependencies]
vergen = "3"
Expand Down
2 changes: 1 addition & 1 deletion benches/helpers/miri_helper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ impl rustc_driver::Callbacks for MiriCompilerCalls<'_> {
);

self.bencher.iter(|| {
let config = miri::MiriConfig { validate: true, args: vec![] };
let config = miri::MiriConfig { validate: true, args: vec![], seed: None };
eval_main(tcx, entry_def_id, config);
});
});
Expand Down
4 changes: 2 additions & 2 deletions src/bin/miri-rustc-tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ impl rustc_driver::Callbacks for MiriCompilerCalls {
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.check_name("test")) {
let config = MiriConfig { validate: true, args: vec![] };
let config = MiriConfig { validate: true, args: vec![], seed: None };
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, config);
Expand All @@ -61,7 +61,7 @@ impl rustc_driver::Callbacks for MiriCompilerCalls {
}
tcx.hir().krate().visit_all_item_likes(&mut Visitor(tcx));
} else if let Some((entry_def_id, _)) = tcx.entry_fn(LOCAL_CRATE) {
let config = MiriConfig { validate: true, args: vec![] };
let config = MiriConfig { validate: true, args: vec![], seed: None };
miri::eval_main(tcx, entry_def_id, config);

compiler.session().abort_if_errors();
Expand Down
17 changes: 16 additions & 1 deletion src/bin/miri.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ fn main() {

// Parse our arguments and split them across `rustc` and `miri`.
let mut validate = true;
let mut seed: Option<u64> = None;
let mut rustc_args = vec![];
let mut miri_args = vec![];
let mut after_dashdash = false;
Expand All @@ -145,6 +146,20 @@ fn main() {
"--" => {
after_dashdash = true;
}
arg if arg.starts_with("-Zmiri-seed=") => {
if seed.is_some() {
panic!("Cannot specify -Zmiri-seed multiple times!");
}
let seed_raw = hex::decode(arg.trim_start_matches("-Zmiri-seed=")).unwrap();
if seed_raw.len() > 8 {
panic!(format!("-Zmiri-seed must be at most 8 bytes, was {}", seed_raw.len()));
}

let mut bytes = [0; 8];
bytes[..seed_raw.len()].copy_from_slice(&seed_raw);
seed = Some(u64::from_be_bytes(bytes));

},
_ => {
rustc_args.push(arg);
}
Expand All @@ -163,7 +178,7 @@ fn main() {

debug!("rustc arguments: {:?}", rustc_args);
debug!("miri arguments: {:?}", miri_args);
let miri_config = miri::MiriConfig { validate, args: miri_args };
let miri_config = miri::MiriConfig { validate, args: miri_args, seed };
let result = rustc_driver::report_ices_to_stderr_if_any(move || {
rustc_driver::run_compiler(&rustc_args, &mut MiriCompilerCalls { miri_config }, None, None)
}).and_then(|result| result);
Expand Down
90 changes: 72 additions & 18 deletions src/fn_call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ use rustc::hir::def_id::DefId;
use rustc::mir;
use syntax::attr;

use rand::RngCore;

use crate::*;

impl<'a, 'mir, 'tcx> EvalContextExt<'a, 'mir, 'tcx> for crate::MiriEvalContext<'a, 'mir, 'tcx> {}
Expand Down Expand Up @@ -209,16 +211,26 @@ pub trait EvalContextExt<'a, 'mir, 'tcx: 'a + 'mir>: crate::MiriEvalContextExt<'
}

"syscall" => {
// TODO: read `syscall` IDs like `sysconf` IDs and
// figure out some way to actually process some of them.
//
let sys_getrandom = this.eval_path_scalar(&["libc", "SYS_getrandom"])?
.expect("Failed to get libc::SYS_getrandom")
.to_usize(this)? as i64;
RalfJung marked this conversation as resolved.
Show resolved Hide resolved

// `libc::syscall(NR_GETRANDOM, buf.as_mut_ptr(), buf.len(), GRND_NONBLOCK)`
// is called if a `HashMap` is created the regular way.
match this.read_scalar(args[0])?.to_usize(this)? {
318 | 511 => {
return err!(Unimplemented(
"miri does not support random number generators".to_owned(),
))
// is called if a `HashMap` is created the regular way (e.g. HashMap<K, V>).
match this.read_scalar(args[0])?.to_usize(this)? as i64 {
id if id == sys_getrandom => {
let ptr = this.read_scalar(args[1])?.to_ptr()?;
let len = this.read_scalar(args[2])?.to_usize(this)?;

// The only supported flags are GRND_RANDOM and GRND_NONBLOCK,
// neither of which have any effect on our current PRNG
let _flags = this.read_scalar(args[3])?.to_i32()?;

let data = gen_random(this, len as usize)?;
this.memory_mut().get_mut(ptr.alloc_id)?
.write_bytes(tcx, ptr, &data)?;
RalfJung marked this conversation as resolved.
Show resolved Hide resolved

this.write_scalar(Scalar::from_uint(len, dest.layout.size), dest)?;
}
id => {
return err!(Unimplemented(
Expand Down Expand Up @@ -484,18 +496,13 @@ pub trait EvalContextExt<'a, 'mir, 'tcx: 'a + 'mir>: crate::MiriEvalContextExt<'
];
let mut result = None;
for &(path, path_value) in paths {
if let Ok(instance) = this.resolve_path(path) {
let cid = GlobalId {
instance,
promoted: None,
};
let const_val = this.const_eval_raw(cid)?;
let const_val = this.read_scalar(const_val.into())?;
let value = const_val.to_i32()?;
if value == name {
if let Some(val) = this.eval_path_scalar(path)? {
let val = val.to_i32()?;
if val == name {
result = Some(path_value);
break;
}

}
}
if let Some(result) = result {
Expand Down Expand Up @@ -742,6 +749,17 @@ pub trait EvalContextExt<'a, 'mir, 'tcx: 'a + 'mir>: crate::MiriEvalContextExt<'
"GetCommandLineW" => {
this.write_scalar(Scalar::Ptr(this.machine.cmd_line.unwrap()), dest)?;
}
// The actual name of 'RtlGenRandom'
"SystemFunction036" => {
let ptr = this.read_scalar(args[0])?.to_ptr()?;
let len = this.read_scalar(args[1])?.to_usize(this)?;

let data = gen_random(this, len as usize)?;
this.memory_mut().get_mut(ptr.alloc_id)?
.write_bytes(tcx, ptr, &data)?;

this.write_scalar(Scalar::from_bool(true), dest)?;
}

// We can't execute anything else.
_ => {
Expand All @@ -759,4 +777,40 @@ pub trait EvalContextExt<'a, 'mir, 'tcx: 'a + 'mir>: crate::MiriEvalContextExt<'
fn write_null(&mut self, dest: PlaceTy<'tcx, Borrow>) -> EvalResult<'tcx> {
self.eval_context_mut().write_scalar(Scalar::from_int(0, dest.layout.size), dest)
}

/// Evaluates the scalar at the specified path. Returns Some(val)
/// if the path could be resolved, and None otherwise
fn eval_path_scalar(&mut self, path: &[&str]) -> EvalResult<'tcx, Option<ScalarMaybeUndef<stacked_borrows::Borrow>>> {
let this = self.eval_context_mut();
if let Ok(instance) = this.resolve_path(path) {
let cid = GlobalId {
instance,
promoted: None,
};
let const_val = this.const_eval_raw(cid)?;
let const_val = this.read_scalar(const_val.into())?;
return Ok(Some(const_val));
}
return Ok(None);
}
}

fn gen_random<'a, 'mir, 'tcx>(this: &mut MiriEvalContext<'a, 'mir, 'tcx>,
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
len: usize) -> Result<Vec<u8>, EvalError<'tcx>> {

match this.machine.rng.as_ref() {
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
Some(rng) => {
let mut data = vec![0; len];
rng.borrow_mut().fill_bytes(&mut data);
Ok(data)
}
None => {
err!(Unimplemented(
"miri does not support gathering system entropy in deterministic mode!
Use '-Zmiri-seed=<seed>' to enable random number generation.
WARNING: Miri does *not* generate cryptographically secure entropy -
do not use Miri to run any program that need secure random number generation".to_owned(),
Aaron1011 marked this conversation as resolved.
Show resolved Hide resolved
))
}
}
}
16 changes: 14 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ mod stacked_borrows;

use std::collections::HashMap;
use std::borrow::Cow;
use std::cell::RefCell;

use rand::rngs::StdRng;
use rand::SeedableRng;

use rustc::ty::{self, TyCtxt, query::TyCtxtAt};
use rustc::ty::layout::{LayoutOf, Size, Align};
Expand Down Expand Up @@ -60,6 +64,9 @@ pub fn miri_default_args() -> &'static [&'static str] {
pub struct MiriConfig {
pub validate: bool,
pub args: Vec<String>,

// The seed to use when non-determinism is required (e.g. getrandom())
pub seed: Option<u64>
}

// Used by priroda.
Expand All @@ -71,7 +78,7 @@ pub fn create_ecx<'a, 'mir: 'a, 'tcx: 'mir>(
let mut ecx = InterpretCx::new(
tcx.at(syntax::source_map::DUMMY_SP),
ty::ParamEnv::reveal_all(),
Evaluator::new(config.validate),
Evaluator::new(config.validate, config.seed),
);

let main_instance = ty::Instance::mono(ecx.tcx.tcx, main_id);
Expand Down Expand Up @@ -326,10 +333,14 @@ pub struct Evaluator<'tcx> {

/// Stacked Borrows state.
pub(crate) stacked_borrows: stacked_borrows::State,

/// The random number generator to use if Miri
/// is running in non-deterministic mode
pub(crate) rng: Option<RefCell<StdRng>>
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
}

impl<'tcx> Evaluator<'tcx> {
fn new(validate: bool) -> Self {
fn new(validate: bool, seed: Option<u64>) -> Self {
Evaluator {
env_vars: HashMap::default(),
argc: None,
Expand All @@ -339,6 +350,7 @@ impl<'tcx> Evaluator<'tcx> {
tls: TlsData::default(),
validate,
stacked_borrows: stacked_borrows::State::default(),
rng: seed.map(|s| RefCell::new(StdRng::seed_from_u64(s)))
}
}
}
Expand Down
13 changes: 13 additions & 0 deletions tests/compile-fail/getrandom.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// ignore-macos
// ignore-windows

#![feature(rustc_private)]
extern crate libc;

fn main() {
let mut buf = [0u8; 5];
unsafe {
libc::syscall(libc::SYS_getrandom, buf.as_mut_ptr() as *mut libc::c_void, 5 as libc::size_t, 0 as libc::c_uint);
//~^ ERROR constant evaluation error: miri does not support gathering system entropy in deterministic mode!
}
}
20 changes: 17 additions & 3 deletions tests/run-pass/hashmap.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
// compile-flags: -Zmiri-seed=0000000000000000

use std::collections::{self, HashMap};
use std::hash::BuildHasherDefault;
use std::hash::{BuildHasherDefault, BuildHasher};

fn main() {
let mut map : HashMap<i32, i32, BuildHasherDefault<collections::hash_map::DefaultHasher>> = Default::default();
fn test_map<S: BuildHasher>(mut map: HashMap<i32, i32, S>) {
map.insert(0, 0);
assert_eq!(map.values().fold(0, |x, y| x+y), 0);

Expand All @@ -22,4 +23,17 @@ fn main() {
assert_eq!(map.values().fold(0, |x, y| x+y), num*(num-1)/2);

// TODO: Test Entry API, Iterators, ...

}

fn main() {
let _map : HashMap<i32, i32, BuildHasherDefault<collections::hash_map::DefaultHasher>> = Default::default();

// TODO: Implement random number generation on OS X
if cfg!(not(target_os = "macos")) {
let map_normal: HashMap<i32, i32> = HashMap::new();
test_map(map_normal);
} else {
test_map(_map);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move the let _map into this conditional branch to avoid the unused variable on other platforms? Then you can also avoid the _ in the name.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to test creation of both types of maps on all platforms, even though only one has the full set of tests run on it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TBH I don't see the point, this code just looks weird. Why test creation on both but not the rest of the test?

The purpose of this is mostly to test Miri, not to test the HashMap. For that, HashBrown actually runs its test suite in Miri. I don't see a good reason to use multiple different hash functions here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My intention was to ensure that Miri's test coverage didn't regress (e.g. ensure that we catch any regressions with creating a HashMap with a custom hasher).

I'll clean it up.

}
}