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

Enable env communication #894

Merged
merged 12 commits into from
Aug 14, 2019
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,9 @@ Several `-Z` flags are relevant for Miri:
is enforced by default. This is mostly useful for debugging; it means Miri
will miss bugs in your program. However, this can also help to make Miri run
faster.
* `-Zmiri-enable-communication` enables communication between the host
environment and Miri, i.e., all the host environment variables are available
during Miri runtime.
* `-Zmir-opt-level` controls how many MIR optimizations are performed. Miri
overrides the default to be `0`; be advised that using any higher level can
make Miri miss bugs in your program because they got optimized away.
Expand Down
7 changes: 6 additions & 1 deletion benches/helpers/miri_helper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,12 @@ impl rustc_driver::Callbacks for MiriCompilerCalls<'_> {
);

self.bencher.iter(|| {
let config = miri::MiriConfig { validate: true, args: vec![], seed: None };
let config = miri::MiriConfig {
validate: true,
communicate: false,
args: vec![],
seed: None,
};
eval_main(tcx, entry_def_id, config);
});
});
Expand Down
14 changes: 12 additions & 2 deletions src/bin/miri-rustc-tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,12 @@ 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(syntax::symbol::sym::test)) {
let config = MiriConfig { validate: true, args: vec![], seed: None };
let config = MiriConfig {
validate: true,
communicate: false,
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 +66,12 @@ 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![], seed: None };
let config = MiriConfig {
validate: true,
communicate: false,
args: vec![],
seed: None
};
miri::eval_main(tcx, entry_def_id, config);

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

// Parse our arguments and split them across `rustc` and `miri`.
let mut validate = true;
let mut communicate = false;
let mut seed: Option<u64> = None;
let mut rustc_args = vec![];
let mut miri_args = vec![];
Expand All @@ -147,6 +148,9 @@ fn main() {
"-Zmiri-disable-validation" => {
validate = false;
},
"-Zmiri-enable-communication" => {
communicate = true;
},
"--" => {
after_dashdash = true;
}
Expand Down Expand Up @@ -196,7 +200,7 @@ fn main() {

debug!("rustc arguments: {:?}", rustc_args);
debug!("miri arguments: {:?}", miri_args);
let miri_config = miri::MiriConfig { validate, args: miri_args, seed };
let miri_config = miri::MiriConfig { validate, communicate, 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
17 changes: 12 additions & 5 deletions src/eval.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,19 @@ use crate::{
InterpResult, InterpError, InterpCx, StackPopCleanup, struct_error,
Scalar, Tag, Pointer, FnVal,
MemoryExtra, MiriMemoryKind, Evaluator, TlsEvalContextExt, HelpersEvalContextExt,
EnvVars,
};

/// Configuration needed to spawn a Miri instance.
#[derive(Clone)]
pub struct MiriConfig {
/// Determine if validity checking and Stacked Borrows are enabled.
pub validate: bool,
pvdrz marked this conversation as resolved.
Show resolved Hide resolved
/// Determines if communication with the host environment is enabled.
pub communicate: bool,
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
pub args: Vec<String>,

// The seed to use when non-determinism is required (e.g. getrandom())
pub seed: Option<u64>
/// The seed to use when non-determinism or randomness are required (e.g. ptr-to-int cast, `getrandom()`).
pub seed: Option<u64>,
}

// Used by priroda.
Expand All @@ -33,10 +36,14 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
let mut ecx = InterpCx::new(
tcx.at(syntax::source_map::DUMMY_SP),
ty::ParamEnv::reveal_all(),
Evaluator::new(),
Evaluator::new(config.communicate),
MemoryExtra::new(StdRng::seed_from_u64(config.seed.unwrap_or(0)), config.validate),
);

// Complete initialization.
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
EnvVars::init(&mut ecx, config.communicate);

// Setup first stack-frame
pvdrz marked this conversation as resolved.
Show resolved Hide resolved
let main_instance = ty::Instance::mono(ecx.tcx.tcx, main_id);
pvdrz marked this conversation as resolved.
Show resolved Hide resolved
let main_mir = ecx.load_mir(main_instance.def)?;

Expand Down Expand Up @@ -158,7 +165,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>(
cur_ptr = cur_ptr.offset(char_size, tcx)?;
}
}

assert!(args.next().is_none(), "start lang item has more arguments than expected");

Ok(ecx)
Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ pub use crate::shims::foreign_items::EvalContextExt as ForeignItemsEvalContextEx
pub use crate::shims::intrinsics::EvalContextExt as IntrinsicsEvalContextExt;
pub use crate::shims::tls::{EvalContextExt as TlsEvalContextExt, TlsData};
pub use crate::shims::dlsym::{Dlsym, EvalContextExt as DlsymEvalContextExt};
pub use crate::shims::env::EnvVars;
pub use crate::operator::EvalContextExt as OperatorEvalContextExt;
pub use crate::range_map::RangeMap;
pub use crate::helpers::{EvalContextExt as HelpersEvalContextExt};
Expand Down
13 changes: 9 additions & 4 deletions src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

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

use rand::rngs::StdRng;
Expand Down Expand Up @@ -79,7 +78,7 @@ impl MemoryExtra {
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<Tag>>,
pub(crate) env_vars: EnvVars,

/// Program arguments (`Option` because we can only initialize them after creating the ecx).
/// These are *pointers* to argc/argv because macOS.
Expand All @@ -93,17 +92,23 @@ pub struct Evaluator<'tcx> {

/// TLS state.
pub(crate) tls: TlsData<'tcx>,

/// If enabled, the `env_vars` field is populated with the host env vars during initialization.
pub(crate) communicate: bool,
pvdrz marked this conversation as resolved.
Show resolved Hide resolved
}

impl<'tcx> Evaluator<'tcx> {
pub(crate) fn new() -> Self {
pub(crate) fn new(communicate: bool) -> Self {
Evaluator {
env_vars: HashMap::default(),
// `env_vars` could be initialized properly here if `Memory` were available before
// calling this method.
env_vars: EnvVars::default(),
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
argc: None,
argv: None,
cmd_line: None,
last_error: 0,
tls: TlsData::default(),
communicate,
}
}
}
Expand Down
60 changes: 60 additions & 0 deletions src/shims/env.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
use std::collections::HashMap;

use rustc::ty::layout::{Size, Align};
use rustc_mir::interpret::{Pointer, Memory};
use crate::stacked_borrows::Tag;
use crate::*;

pvdrz marked this conversation as resolved.
Show resolved Hide resolved
#[derive(Default)]
pub struct EnvVars {
map: HashMap<Vec<u8>, Pointer<Tag>>,
}

impl EnvVars {
pub(crate) fn init<'mir, 'tcx>(
ecx: &mut InterpCx<'mir, 'tcx, Evaluator<'tcx>>,
communicate: bool,
Copy link
Member

Choose a reason for hiding this comment

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

Ah, I entirely forgot that the machine has this flag. So can you remove this parameter, and use ecx.machine.communicate instead?

Copy link
Member

@RalfJung RalfJung Aug 15, 2019

Choose a reason for hiding this comment

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

Since you are probably asleep, I'll do this as part of #909.

) {
if communicate {
for (name, value) in std::env::vars() {
let value = alloc_env_value(value.as_bytes(), ecx.memory_mut());
ecx.machine.env_vars.map.insert(name.into_bytes(), value);
}
}
}

pub(crate) fn get(&self, name: &[u8]) -> Option<&Pointer<Tag>> {
self.map.get(name)
}

pub(crate) fn unset(&mut self, name: &[u8]) -> Option<Pointer<Tag>> {
self.map.remove(name)
}

pub(crate) fn set(&mut self, name: Vec<u8>, ptr: Pointer<Tag>) -> Option<Pointer<Tag>>{
self.map.insert(name, ptr)
}
}

pub(crate) fn alloc_env_value<'mir, 'tcx>(
bytes: &[u8],
memory: &mut Memory<'mir, 'tcx, Evaluator<'tcx>>,
) -> Pointer<Tag> {
let tcx = {memory.tcx.tcx};
let length = bytes.len() as u64;
// `+1` for the null terminator.
let ptr = memory.allocate(
Size::from_bytes(length + 1),
Align::from_bytes(1).unwrap(),
MiriMemoryKind::Env.into(),
);
// We just allocated these, so the write cannot fail.
let alloc = memory.get_mut(ptr.alloc_id).unwrap();
alloc.write_bytes(&tcx, ptr, &bytes).unwrap();
let trailing_zero_ptr = ptr.offset(
Size::from_bytes(length),
&tcx,
).unwrap();
alloc.write_bytes(&tcx, trailing_zero_ptr, &[0]).unwrap();
ptr
}
pvdrz marked this conversation as resolved.
Show resolved Hide resolved
25 changes: 4 additions & 21 deletions src/shims/foreign_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use syntax::attr;
use syntax::symbol::sym;

use crate::*;
use crate::shims::env::alloc_env_value;

impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {}
pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> {
Expand Down Expand Up @@ -437,7 +438,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
if !this.is_null(name_ptr)? {
let name = this.memory().read_c_str(name_ptr)?.to_owned();
if !name.is_empty() && !name.contains(&b'=') {
success = Some(this.machine.env_vars.remove(&name));
success = Some(this.machine.env_vars.unset(&name));
}
}
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
}
Expand Down Expand Up @@ -465,26 +466,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
}
RalfJung marked this conversation as resolved.
Show resolved Hide resolved
}
if let Some((name, value)) = new {
// `+1` for the null terminator.
let value_copy = this.memory_mut().allocate(
Size::from_bytes((value.len() + 1) as u64),
Align::from_bytes(1).unwrap(),
MiriMemoryKind::Env.into(),
);
// We just allocated these, so the write cannot fail.
let alloc = this.memory_mut().get_mut(value_copy.alloc_id).unwrap();
alloc.write_bytes(tcx, value_copy, &value).unwrap();
let trailing_zero_ptr = value_copy.offset(
Size::from_bytes(value.len() as u64),
tcx,
).unwrap();
alloc.write_bytes(tcx, trailing_zero_ptr, &[0]).unwrap();

if let Some(var) = this.machine.env_vars.insert(
name.to_owned(),
value_copy,
)
{
let value_copy = alloc_env_value(&value, this.memory_mut());
if let Some(var) = this.machine.env_vars.set(name.to_owned(), value_copy) {
this.memory_mut().deallocate(var, None, MiriMemoryKind::Env.into())?;
}
this.write_null(dest)?;
Expand Down
1 change: 1 addition & 0 deletions src/shims/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ pub mod foreign_items;
pub mod intrinsics;
pub mod tls;
pub mod dlsym;
pub mod env;

use rustc::{ty, mir};

Expand Down
3 changes: 3 additions & 0 deletions tests/compiletest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,9 @@ fn compile_fail_miri(opt: bool) {
}

fn test_runner(_tests: &[&()]) {
// Add a test env var to do environment communication tests
std::env::set_var("MIRI_ENV_VAR_TEST", "0");

run_pass_miri(false);
run_pass_miri(true);

Expand Down
6 changes: 6 additions & 0 deletions tests/run-pass/communication.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// ignore-windows: TODO env var emulation stubbed out on Windows
// compile-flags: -Zmiri-enable-communication

fn main() {
assert_eq!(std::env::var("MIRI_ENV_VAR_TEST"), Ok("0".to_owned()));
pvdrz marked this conversation as resolved.
Show resolved Hide resolved
}
2 changes: 2 additions & 0 deletions tests/run-pass/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,6 @@ fn main() {
assert_eq!(env::var("MIRI_TEST"), Err(env::VarError::NotPresent));
env::set_var("MIRI_TEST", "the answer");
assert_eq!(env::var("MIRI_TEST"), Ok("the answer".to_owned()));
// Test that miri environment is isolated when communication is disabled.
assert!(env::var("MIRI_ENV_VAR_TEST").is_err());
}