From 655f9af7fe315a9f8e881d94b7c221c965a7caae Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Tue, 6 Aug 2019 10:28:50 -0500 Subject: [PATCH 01/12] Add flag to enable communication --- benches/helpers/miri_helper.rs | 2 +- src/bin/miri-rustc-tests.rs | 5 +++-- src/bin/miri.rs | 6 +++++- src/eval.rs | 5 +++-- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/benches/helpers/miri_helper.rs b/benches/helpers/miri_helper.rs index 203a8b1133a5..003c93f7464d 100644 --- a/benches/helpers/miri_helper.rs +++ b/benches/helpers/miri_helper.rs @@ -25,7 +25,7 @@ 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); }); }); diff --git a/src/bin/miri-rustc-tests.rs b/src/bin/miri-rustc-tests.rs index dae5189937a3..a1bc53f69b80 100644 --- a/src/bin/miri-rustc-tests.rs +++ b/src/bin/miri-rustc-tests.rs @@ -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(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); @@ -61,7 +61,8 @@ 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(); diff --git a/src/bin/miri.rs b/src/bin/miri.rs index 43cb19b2658e..875afd42af44 100644 --- a/src/bin/miri.rs +++ b/src/bin/miri.rs @@ -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 = None; let mut rustc_args = vec![]; let mut miri_args = vec![]; @@ -147,6 +148,9 @@ fn main() { "-Zmiri-disable-validation" => { validate = false; }, + "-Zmiri-enable-communication" => { + communicate = true; + }, "--" => { after_dashdash = true; } @@ -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); diff --git a/src/eval.rs b/src/eval.rs index bc9d97b0280f..681bad01d23c 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -18,10 +18,11 @@ use crate::{ #[derive(Clone)] pub struct MiriConfig { pub validate: bool, + pub communicate: bool, pub args: Vec, // The seed to use when non-determinism is required (e.g. getrandom()) - pub seed: Option + pub seed: Option, } // Used by priroda. @@ -158,7 +159,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) From 068c448832c466e3eb6297125a12e5a83a450e2d Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Tue, 6 Aug 2019 15:32:57 -0500 Subject: [PATCH 02/12] Add communicate field to evaluator and fix formatting --- benches/helpers/miri_helper.rs | 7 ++++++- src/bin/miri-rustc-tests.rs | 14 ++++++++++++-- src/eval.rs | 5 +++-- src/machine.rs | 6 +++++- 4 files changed, 26 insertions(+), 6 deletions(-) diff --git a/benches/helpers/miri_helper.rs b/benches/helpers/miri_helper.rs index 003c93f7464d..5cb938659a6c 100644 --- a/benches/helpers/miri_helper.rs +++ b/benches/helpers/miri_helper.rs @@ -25,7 +25,12 @@ impl rustc_driver::Callbacks for MiriCompilerCalls<'_> { ); self.bencher.iter(|| { - let config = miri::MiriConfig { validate: true, communicate: false, args: vec![], seed: None }; + let config = miri::MiriConfig { + validate: true, + communicate: false, + args: vec![], + seed: None, + }; eval_main(tcx, entry_def_id, config); }); }); diff --git a/src/bin/miri-rustc-tests.rs b/src/bin/miri-rustc-tests.rs index a1bc53f69b80..0cf171f52e7e 100644 --- a/src/bin/miri-rustc-tests.rs +++ b/src/bin/miri-rustc-tests.rs @@ -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, communicate: false, 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); @@ -62,7 +67,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, communicate: false, 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(); diff --git a/src/eval.rs b/src/eval.rs index 681bad01d23c..e80162ec1796 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -18,10 +18,11 @@ use crate::{ #[derive(Clone)] pub struct MiriConfig { pub validate: bool, + /// Determines if communication with the host environment is enabled. pub communicate: bool, pub args: Vec, - // The seed to use when non-determinism is required (e.g. getrandom()) + /// The seed to use when non-determinism is required (e.g. getrandom()) pub seed: Option, } @@ -34,7 +35,7 @@ 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), ); diff --git a/src/machine.rs b/src/machine.rs index f2c81c92d3d0..ed9a8a1c4634 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -93,10 +93,13 @@ 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, } impl<'tcx> Evaluator<'tcx> { - pub(crate) fn new() -> Self { + pub(crate) fn new(communicate: bool) -> Self { Evaluator { env_vars: HashMap::default(), argc: None, @@ -104,6 +107,7 @@ impl<'tcx> Evaluator<'tcx> { cmd_line: None, last_error: 0, tls: TlsData::default(), + communicate, } } } From 0df7a728c684606a3ee275ca32d3d56c17902c76 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Tue, 6 Aug 2019 15:47:57 -0500 Subject: [PATCH 03/12] Update readme --- README.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/README.md b/README.md index 8c5a29880c7c..a9cd9ee938c5 100644 --- a/README.md +++ b/README.md @@ -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. From b731a6a15f3ac426ca29f5bdbe4e3d7bd68d9685 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Tue, 6 Aug 2019 17:40:07 -0500 Subject: [PATCH 04/12] Add support for env communication --- src/eval.rs | 8 ++++++++ src/shims/env.rs | 23 +++++++++++++++++++++++ src/shims/foreign_items.rs | 23 +++-------------------- src/shims/mod.rs | 1 + tests/run-pass/communication.rs | 6 ++++++ 5 files changed, 41 insertions(+), 20 deletions(-) create mode 100644 src/shims/env.rs create mode 100644 tests/run-pass/communication.rs diff --git a/src/eval.rs b/src/eval.rs index e80162ec1796..b171b38a6cbd 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -13,6 +13,7 @@ use crate::{ Scalar, Tag, Pointer, FnVal, MemoryExtra, MiriMemoryKind, Evaluator, TlsEvalContextExt, HelpersEvalContextExt, }; +use crate::shims::env::alloc_env_value; /// Configuration needed to spawn a Miri instance. #[derive(Clone)] @@ -163,6 +164,13 @@ pub fn create_ecx<'mir, 'tcx: 'mir>( assert!(args.next().is_none(), "start lang item has more arguments than expected"); + if config.communicate { + for (name, value) in std::env::vars() { + let value = alloc_env_value(value.as_bytes(), ecx.memory_mut(), &tcx); + ecx.machine.env_vars.insert(name.into_bytes(), value); + } + } + Ok(ecx) } diff --git a/src/shims/env.rs b/src/shims/env.rs new file mode 100644 index 000000000000..0cb8c9c370c3 --- /dev/null +++ b/src/shims/env.rs @@ -0,0 +1,23 @@ +use rustc::ty::{layout::{Size, Align}, TyCtxt}; +use rustc_mir::interpret::Memory; + +use crate::*; + +pub(crate) fn alloc_env_value<'mir, 'tcx>(bytes: &[u8], memory: &mut Memory<'mir, 'tcx, Evaluator<'tcx>>, tcx: &TyCtxt<'tcx>) -> Pointer { + 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 +} diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index 37e5fe42c3de..b1554395b692 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -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> { @@ -465,26 +466,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx } } 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(), tcx); + if let Some(var) = this.machine.env_vars.insert(name.to_owned(), value_copy) { this.memory_mut().deallocate(var, None, MiriMemoryKind::Env.into())?; } this.write_null(dest)?; diff --git a/src/shims/mod.rs b/src/shims/mod.rs index c06373005ff9..96a1f34152b3 100644 --- a/src/shims/mod.rs +++ b/src/shims/mod.rs @@ -2,6 +2,7 @@ pub mod foreign_items; pub mod intrinsics; pub mod tls; pub mod dlsym; +pub mod env; use rustc::{ty, mir}; diff --git a/tests/run-pass/communication.rs b/tests/run-pass/communication.rs new file mode 100644 index 000000000000..08c6bd88fd68 --- /dev/null +++ b/tests/run-pass/communication.rs @@ -0,0 +1,6 @@ +// ignore-windows: TODO env var emulation stubbed out on Windows +// compile-flags: -Zmiri-enable-communication + +fn main() { + assert!(std::env::var("PWD").is_ok()); +} From af623dede2437244eabe7a6833b28ec2535de82a Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Wed, 7 Aug 2019 09:09:13 -0500 Subject: [PATCH 05/12] Add env var test variable in compiletest --- tests/compiletest.rs | 3 +++ tests/run-pass/communication.rs | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/compiletest.rs b/tests/compiletest.rs index eaf7f8531b8f..c5fba3d46e2f 100644 --- a/tests/compiletest.rs +++ b/tests/compiletest.rs @@ -37,6 +37,9 @@ fn run_tests(mode: &str, path: &str, target: &str, mut flags: Vec) { flags.push(format!("--sysroot {}", sysroot)); } + // Add a test env var to do evironment communication tests + std::env::set_var("MIRI_ENV_VAR_TEST", "0"); + // The rest of the configuration. let mut config = compiletest::Config::default().tempdir(); config.mode = mode.parse().expect("Invalid mode"); diff --git a/tests/run-pass/communication.rs b/tests/run-pass/communication.rs index 08c6bd88fd68..e3fb0c5bd5e0 100644 --- a/tests/run-pass/communication.rs +++ b/tests/run-pass/communication.rs @@ -2,5 +2,5 @@ // compile-flags: -Zmiri-enable-communication fn main() { - assert!(std::env::var("PWD").is_ok()); + assert_eq!(std::env::var("MIRI_ENV_VAR_TEST"), Ok("0".to_owned())); } From 253af9692afebc1a4b460fd2ac2cc842d007d573 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Wed, 7 Aug 2019 09:10:39 -0500 Subject: [PATCH 06/12] Fix formatting --- src/shims/env.rs | 6 +++++- tests/compiletest.rs | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/shims/env.rs b/src/shims/env.rs index 0cb8c9c370c3..56d2bd4b3d3e 100644 --- a/src/shims/env.rs +++ b/src/shims/env.rs @@ -3,7 +3,11 @@ use rustc_mir::interpret::Memory; use crate::*; -pub(crate) fn alloc_env_value<'mir, 'tcx>(bytes: &[u8], memory: &mut Memory<'mir, 'tcx, Evaluator<'tcx>>, tcx: &TyCtxt<'tcx>) -> Pointer { +pub(crate) fn alloc_env_value<'mir, 'tcx>( + bytes: &[u8], + memory: &mut Memory<'mir, 'tcx, Evaluator<'tcx>>, + tcx: &TyCtxt<'tcx>, +) -> Pointer { let length = bytes.len() as u64; // `+1` for the null terminator. let ptr = memory.allocate( diff --git a/tests/compiletest.rs b/tests/compiletest.rs index c5fba3d46e2f..f61faa9fcc61 100644 --- a/tests/compiletest.rs +++ b/tests/compiletest.rs @@ -37,7 +37,7 @@ fn run_tests(mode: &str, path: &str, target: &str, mut flags: Vec) { flags.push(format!("--sysroot {}", sysroot)); } - // Add a test env var to do evironment communication tests + // Add a test env var to do environment communication tests std::env::set_var("MIRI_ENV_VAR_TEST", "0"); // The rest of the configuration. From 666cd22fa64ee318edc3b6292efa507442eb2554 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Tue, 13 Aug 2019 11:34:43 -0500 Subject: [PATCH 07/12] Wrap hashmap for env vars in its own type --- src/eval.rs | 11 ++++------- src/machine.rs | 6 +++--- src/shims/env.rs | 35 +++++++++++++++++++++++++++++++++-- src/shims/foreign_items.rs | 4 ++-- 4 files changed, 42 insertions(+), 14 deletions(-) diff --git a/src/eval.rs b/src/eval.rs index b171b38a6cbd..5e6c8129fb31 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -13,17 +13,17 @@ use crate::{ Scalar, Tag, Pointer, FnVal, MemoryExtra, MiriMemoryKind, Evaluator, TlsEvalContextExt, HelpersEvalContextExt, }; -use crate::shims::env::alloc_env_value; +use crate::shims::env::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, /// Determines if communication with the host environment is enabled. pub communicate: bool, pub args: Vec, - - /// The seed to use when non-determinism is required (e.g. getrandom()) + /// The seed to use when non-determinism or randomness are required (e.g. ptr-to-int cast, `getrandom()`). pub seed: Option, } @@ -165,10 +165,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>( assert!(args.next().is_none(), "start lang item has more arguments than expected"); if config.communicate { - for (name, value) in std::env::vars() { - let value = alloc_env_value(value.as_bytes(), ecx.memory_mut(), &tcx); - ecx.machine.env_vars.insert(name.into_bytes(), value); - } + EnvVars::init(&mut ecx, &tcx); } Ok(ecx) diff --git a/src/machine.rs b/src/machine.rs index ed9a8a1c4634..cc0c85d6603d 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -3,7 +3,6 @@ use std::rc::Rc; use std::borrow::Cow; -use std::collections::HashMap; use std::cell::RefCell; use rand::rngs::StdRng; @@ -15,6 +14,7 @@ use rustc::ty::{self, layout::{Size, LayoutOf}, TyCtxt}; use rustc::mir; use crate::*; +use crate::shims::env::EnvVars; // Some global facts about the emulated machine. pub const PAGE_SIZE: u64 = 4*1024; // FIXME: adjust to target architecture @@ -79,7 +79,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, Pointer>, + 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. @@ -101,7 +101,7 @@ pub struct Evaluator<'tcx> { impl<'tcx> Evaluator<'tcx> { pub(crate) fn new(communicate: bool) -> Self { Evaluator { - env_vars: HashMap::default(), + env_vars: EnvVars::default(), argc: None, argv: None, cmd_line: None, diff --git a/src/shims/env.rs b/src/shims/env.rs index 56d2bd4b3d3e..09d87d27ebc2 100644 --- a/src/shims/env.rs +++ b/src/shims/env.rs @@ -1,8 +1,39 @@ -use rustc::ty::{layout::{Size, Align}, TyCtxt}; -use rustc_mir::interpret::Memory; +use std::collections::HashMap; +use rustc::ty::{layout::{Size, Align}, TyCtxt}; +use rustc_mir::interpret::{Pointer, Memory}; +use crate::stacked_borrows::Tag; use crate::*; +#[derive(Default)] +pub struct EnvVars { + map: HashMap, Pointer>, +} + +impl EnvVars { + pub(crate) fn init<'mir, 'tcx>( + ecx: &mut InterpCx<'mir, 'tcx, Evaluator<'tcx>>, + tcx: &TyCtxt<'tcx>, + ) { + for (name, value) in std::env::vars() { + let value = alloc_env_value(value.as_bytes(), ecx.memory_mut(), tcx); + ecx.machine.env_vars.map.insert(name.into_bytes(), value); + } + } + + pub(crate) fn get(&self, name: &[u8]) -> Option<&Pointer> { + self.map.get(name) + } + + pub(crate) fn unset(&mut self, name: &[u8]) -> Option> { + self.map.remove(name) + } + + pub(crate) fn set(&mut self, name: Vec, ptr: Pointer) -> Option>{ + self.map.insert(name, ptr) + } +} + pub(crate) fn alloc_env_value<'mir, 'tcx>( bytes: &[u8], memory: &mut Memory<'mir, 'tcx, Evaluator<'tcx>>, diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index b1554395b692..6705183da47f 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -438,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)); } } } @@ -467,7 +467,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx } if let Some((name, value)) = new { let value_copy = alloc_env_value(&value, this.memory_mut(), tcx); - if let Some(var) = this.machine.env_vars.insert(name.to_owned(), value_copy) { + 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)?; From 67d13577aac2e47a0f40292c5c966469cbc610d7 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Tue, 13 Aug 2019 12:10:24 -0500 Subject: [PATCH 08/12] Move test env var to test_runner --- src/bin/miri-rustc-tests.rs | 1 - tests/compiletest.rs | 6 +++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/bin/miri-rustc-tests.rs b/src/bin/miri-rustc-tests.rs index 0cf171f52e7e..9ef64c38638b 100644 --- a/src/bin/miri-rustc-tests.rs +++ b/src/bin/miri-rustc-tests.rs @@ -66,7 +66,6 @@ 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, communicate: false, diff --git a/tests/compiletest.rs b/tests/compiletest.rs index f61faa9fcc61..702c6026eced 100644 --- a/tests/compiletest.rs +++ b/tests/compiletest.rs @@ -37,9 +37,6 @@ fn run_tests(mode: &str, path: &str, target: &str, mut flags: Vec) { flags.push(format!("--sysroot {}", sysroot)); } - // Add a test env var to do environment communication tests - std::env::set_var("MIRI_ENV_VAR_TEST", "0"); - // The rest of the configuration. let mut config = compiletest::Config::default().tempdir(); config.mode = mode.parse().expect("Invalid mode"); @@ -126,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); From afc6713e4178fc4c375e7acea8898534f2e6fba3 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Tue, 13 Aug 2019 16:17:41 -0500 Subject: [PATCH 09/12] Reorganize shims::env::EnvVars --- src/eval.rs | 8 +++----- src/lib.rs | 1 + src/machine.rs | 7 ++++--- src/shims/env.rs | 9 ++++++--- 4 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/eval.rs b/src/eval.rs index 5e6c8129fb31..936ae5b89532 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -12,8 +12,8 @@ use crate::{ InterpResult, InterpError, InterpCx, StackPopCleanup, struct_error, Scalar, Tag, Pointer, FnVal, MemoryExtra, MiriMemoryKind, Evaluator, TlsEvalContextExt, HelpersEvalContextExt, + ShimsEnvVars, }; -use crate::shims::env::EnvVars; /// Configuration needed to spawn a Miri instance. #[derive(Clone)] @@ -40,6 +40,8 @@ pub fn create_ecx<'mir, 'tcx: 'mir>( MemoryExtra::new(StdRng::seed_from_u64(config.seed.unwrap_or(0)), config.validate), ); + ShimsEnvVars::init(config.communicate, &mut ecx, &tcx); + let main_instance = ty::Instance::mono(ecx.tcx.tcx, main_id); let main_mir = ecx.load_mir(main_instance.def)?; @@ -164,10 +166,6 @@ pub fn create_ecx<'mir, 'tcx: 'mir>( assert!(args.next().is_none(), "start lang item has more arguments than expected"); - if config.communicate { - EnvVars::init(&mut ecx, &tcx); - } - Ok(ecx) } diff --git a/src/lib.rs b/src/lib.rs index 58f572bf7011..216e41d4f838 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -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 as ShimsEnvVars}; pub use crate::operator::EvalContextExt as OperatorEvalContextExt; pub use crate::range_map::RangeMap; pub use crate::helpers::{EvalContextExt as HelpersEvalContextExt}; diff --git a/src/machine.rs b/src/machine.rs index cc0c85d6603d..635b46bcdb04 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -14,7 +14,6 @@ use rustc::ty::{self, layout::{Size, LayoutOf}, TyCtxt}; use rustc::mir; use crate::*; -use crate::shims::env::EnvVars; // Some global facts about the emulated machine. pub const PAGE_SIZE: u64 = 4*1024; // FIXME: adjust to target architecture @@ -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: EnvVars, + pub(crate) env_vars: ShimsEnvVars, /// Program arguments (`Option` because we can only initialize them after creating the ecx). /// These are *pointers* to argc/argv because macOS. @@ -101,7 +100,9 @@ pub struct Evaluator<'tcx> { impl<'tcx> Evaluator<'tcx> { pub(crate) fn new(communicate: bool) -> Self { Evaluator { - env_vars: EnvVars::default(), + // `env_vars` could be initialized properly here if `Memory` were available before + // calling this method. + env_vars: ShimsEnvVars::default(), argc: None, argv: None, cmd_line: None, diff --git a/src/shims/env.rs b/src/shims/env.rs index 09d87d27ebc2..4a15eb4cfb48 100644 --- a/src/shims/env.rs +++ b/src/shims/env.rs @@ -12,12 +12,15 @@ pub struct EnvVars { impl EnvVars { pub(crate) fn init<'mir, 'tcx>( + communicate: bool, ecx: &mut InterpCx<'mir, 'tcx, Evaluator<'tcx>>, tcx: &TyCtxt<'tcx>, ) { - for (name, value) in std::env::vars() { - let value = alloc_env_value(value.as_bytes(), ecx.memory_mut(), tcx); - ecx.machine.env_vars.map.insert(name.into_bytes(), value); + if communicate { + for (name, value) in std::env::vars() { + let value = alloc_env_value(value.as_bytes(), ecx.memory_mut(), tcx); + ecx.machine.env_vars.map.insert(name.into_bytes(), value); + } } } From f451fe21bdae10709b98406544c61689e1691163 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Tue, 13 Aug 2019 16:17:53 -0500 Subject: [PATCH 10/12] Test env isolation --- tests/run-pass/env.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/run-pass/env.rs b/tests/run-pass/env.rs index 91e15f249d45..faf947420347 100644 --- a/tests/run-pass/env.rs +++ b/tests/run-pass/env.rs @@ -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()); } From 46f902b67dfeaeab45424dd7e24771676ef30b58 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Wed, 14 Aug 2019 10:24:35 -0500 Subject: [PATCH 11/12] Rename export for shims::env::EnvVars --- src/eval.rs | 8 ++++---- src/lib.rs | 2 +- src/machine.rs | 4 ++-- src/shims/env.rs | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/eval.rs b/src/eval.rs index 936ae5b89532..496bcf990adc 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -12,7 +12,7 @@ use crate::{ InterpResult, InterpError, InterpCx, StackPopCleanup, struct_error, Scalar, Tag, Pointer, FnVal, MemoryExtra, MiriMemoryKind, Evaluator, TlsEvalContextExt, HelpersEvalContextExt, - ShimsEnvVars, + EnvVars, }; /// Configuration needed to spawn a Miri instance. @@ -39,9 +39,9 @@ pub fn create_ecx<'mir, 'tcx: 'mir>( Evaluator::new(config.communicate), MemoryExtra::new(StdRng::seed_from_u64(config.seed.unwrap_or(0)), config.validate), ); - - ShimsEnvVars::init(config.communicate, &mut ecx, &tcx); - + // Complete initialization. + EnvVars::init(&mut ecx, &tcx, config.communicate); + // Setup first stack-frame let main_instance = ty::Instance::mono(ecx.tcx.tcx, main_id); let main_mir = ecx.load_mir(main_instance.def)?; diff --git a/src/lib.rs b/src/lib.rs index 216e41d4f838..738419c2498b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -33,7 +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 as ShimsEnvVars}; +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}; diff --git a/src/machine.rs b/src/machine.rs index 635b46bcdb04..b4aac147f94f 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -78,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: ShimsEnvVars, + 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. @@ -102,7 +102,7 @@ impl<'tcx> Evaluator<'tcx> { Evaluator { // `env_vars` could be initialized properly here if `Memory` were available before // calling this method. - env_vars: ShimsEnvVars::default(), + env_vars: EnvVars::default(), argc: None, argv: None, cmd_line: None, diff --git a/src/shims/env.rs b/src/shims/env.rs index 4a15eb4cfb48..d283cde8e3e9 100644 --- a/src/shims/env.rs +++ b/src/shims/env.rs @@ -12,9 +12,9 @@ pub struct EnvVars { impl EnvVars { pub(crate) fn init<'mir, 'tcx>( - communicate: bool, ecx: &mut InterpCx<'mir, 'tcx, Evaluator<'tcx>>, tcx: &TyCtxt<'tcx>, + communicate: bool, ) { if communicate { for (name, value) in std::env::vars() { From 451a09a6857f7e7dabc918f09c78932b3de2e8f4 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Wed, 14 Aug 2019 11:22:47 -0500 Subject: [PATCH 12/12] Remove tcx parameter for EnvVars::alloc_env_value --- src/eval.rs | 4 +++- src/shims/env.rs | 13 ++++++------- src/shims/foreign_items.rs | 2 +- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/eval.rs b/src/eval.rs index 496bcf990adc..0970edb2b75f 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -39,8 +39,10 @@ pub fn create_ecx<'mir, 'tcx: 'mir>( Evaluator::new(config.communicate), MemoryExtra::new(StdRng::seed_from_u64(config.seed.unwrap_or(0)), config.validate), ); + // Complete initialization. - EnvVars::init(&mut ecx, &tcx, config.communicate); + EnvVars::init(&mut ecx, config.communicate); + // Setup first stack-frame let main_instance = ty::Instance::mono(ecx.tcx.tcx, main_id); let main_mir = ecx.load_mir(main_instance.def)?; diff --git a/src/shims/env.rs b/src/shims/env.rs index d283cde8e3e9..05c5fbb04309 100644 --- a/src/shims/env.rs +++ b/src/shims/env.rs @@ -1,6 +1,6 @@ use std::collections::HashMap; -use rustc::ty::{layout::{Size, Align}, TyCtxt}; +use rustc::ty::layout::{Size, Align}; use rustc_mir::interpret::{Pointer, Memory}; use crate::stacked_borrows::Tag; use crate::*; @@ -13,12 +13,11 @@ pub struct EnvVars { impl EnvVars { pub(crate) fn init<'mir, 'tcx>( ecx: &mut InterpCx<'mir, 'tcx, Evaluator<'tcx>>, - tcx: &TyCtxt<'tcx>, communicate: bool, ) { if communicate { for (name, value) in std::env::vars() { - let value = alloc_env_value(value.as_bytes(), ecx.memory_mut(), tcx); + let value = alloc_env_value(value.as_bytes(), ecx.memory_mut()); ecx.machine.env_vars.map.insert(name.into_bytes(), value); } } @@ -40,8 +39,8 @@ impl EnvVars { pub(crate) fn alloc_env_value<'mir, 'tcx>( bytes: &[u8], memory: &mut Memory<'mir, 'tcx, Evaluator<'tcx>>, - tcx: &TyCtxt<'tcx>, ) -> Pointer { + let tcx = {memory.tcx.tcx}; let length = bytes.len() as u64; // `+1` for the null terminator. let ptr = memory.allocate( @@ -51,11 +50,11 @@ pub(crate) fn alloc_env_value<'mir, 'tcx>( ); // 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(); + alloc.write_bytes(&tcx, ptr, &bytes).unwrap(); let trailing_zero_ptr = ptr.offset( Size::from_bytes(length), - tcx, + &tcx, ).unwrap(); - alloc.write_bytes(tcx, trailing_zero_ptr, &[0]).unwrap(); + alloc.write_bytes(&tcx, trailing_zero_ptr, &[0]).unwrap(); ptr } diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index 6705183da47f..088077d5dfa2 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -466,7 +466,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx } } if let Some((name, value)) = new { - let value_copy = alloc_env_value(&value, this.memory_mut(), tcx); + 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())?; }