From d13fe01f824f9a58f94058948e57f4d8c6365866 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Wed, 4 Mar 2020 12:15:14 -0500 Subject: [PATCH 1/9] add working shim for environ --- src/eval.rs | 2 +- src/machine.rs | 14 ++++++++++++++ src/shims/env.rs | 15 +++++++++++++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/src/eval.rs b/src/eval.rs index 1981a8d1e0..dacd996c4a 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -77,8 +77,8 @@ pub fn create_ecx<'mir, 'tcx: 'mir>( ), ); // Complete initialization. - MemoryExtra::init_extern_statics(&mut ecx)?; EnvVars::init(&mut ecx, config.excluded_env_vars); + MemoryExtra::init_extern_statics(&mut ecx)?; // Setup first stack-frame let main_instance = ty::Instance::mono(tcx, main_id); diff --git a/src/machine.rs b/src/machine.rs index d15e290cbf..0e3cc58df7 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -84,6 +84,9 @@ pub struct MemoryExtra { /// An allocation ID to report when it is being allocated /// (helps for debugging memory leaks). tracked_alloc_id: Option, + + /// The `AllocId` for the `environ` static. + pub(crate) environ: Option>, } impl MemoryExtra { @@ -99,6 +102,7 @@ impl MemoryExtra { extern_statics: FxHashMap::default(), rng: RefCell::new(rng), tracked_alloc_id, + environ: None, } } @@ -118,6 +122,16 @@ impl MemoryExtra { .extern_statics .insert(Symbol::intern("__cxa_thread_atexit_impl"), place.ptr.assert_ptr().alloc_id) .unwrap_none(); + + // "environ" + let layout = this.layout_of(this.tcx.types.usize)?; + let place = this.allocate(layout, MiriMemoryKind::Machine.into()); + this.write_scalar(this.memory.extra.environ.unwrap(), place.into())?; + this.memory + .extra + .extern_statics + .insert(Symbol::intern("environ"), place.ptr.assert_ptr().alloc_id) + .unwrap_none(); } _ => {} // No "extern statics" supported on this platform } diff --git a/src/shims/env.rs b/src/shims/env.rs index 79f0c5b3d9..833fef69f2 100644 --- a/src/shims/env.rs +++ b/src/shims/env.rs @@ -2,6 +2,7 @@ use std::ffi::{OsString, OsStr}; use std::env; use crate::stacked_borrows::Tag; +use crate::rustc_target::abi::LayoutOf; use crate::*; use rustc_data_structures::fx::FxHashMap; @@ -20,15 +21,29 @@ impl EnvVars { ecx: &mut InterpCx<'mir, 'tcx, Evaluator<'tcx>>, excluded_env_vars: Vec, ) { + let mut vars = Vec::new(); if ecx.machine.communicate { for (name, value) in env::vars() { if !excluded_env_vars.contains(&name) { let var_ptr = alloc_env_var_as_c_str(name.as_ref(), value.as_ref(), ecx); ecx.machine.env_vars.map.insert(OsString::from(name), var_ptr); + vars.push(var_ptr.into()); } } } + // Add the trailing null pointer + vars.push(Scalar::from_int(0, ecx.pointer_size())); + // Make an array with all these pointers inside Miri. + let tcx = ecx.tcx; + let environ_layout = + ecx.layout_of(tcx.mk_array(tcx.mk_imm_ptr(tcx.types.u8), vars.len() as u64)).unwrap(); + let environ_place = ecx.allocate(environ_layout, MiriMemoryKind::Machine.into()); + for (idx, var) in vars.into_iter().enumerate() { + let place = ecx.mplace_field(environ_place, idx as u64).unwrap(); + ecx.write_scalar(var, place.into()).unwrap(); + } + ecx.memory.extra.environ = Some(environ_place.ptr.into()); } } From 4f5fdc581014ecb17a9aa14ada2a2f186c9a46a8 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Wed, 4 Mar 2020 13:24:01 -0500 Subject: [PATCH 2/9] update the environ shim when environment changes --- src/eval.rs | 2 +- src/machine.rs | 19 ++++++++++--------- src/shims/env.rs | 41 +++++++++++++++++++++++++++-------------- 3 files changed, 38 insertions(+), 24 deletions(-) diff --git a/src/eval.rs b/src/eval.rs index dacd996c4a..1981a8d1e0 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -77,8 +77,8 @@ pub fn create_ecx<'mir, 'tcx: 'mir>( ), ); // Complete initialization. - EnvVars::init(&mut ecx, config.excluded_env_vars); MemoryExtra::init_extern_statics(&mut ecx)?; + EnvVars::init(&mut ecx, config.excluded_env_vars); // Setup first stack-frame let main_instance = ty::Instance::mono(tcx, main_id); diff --git a/src/machine.rs b/src/machine.rs index 0e3cc58df7..ece4b62d3a 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -70,7 +70,7 @@ pub struct AllocExtra { /// Extra global memory data #[derive(Clone, Debug)] -pub struct MemoryExtra { +pub struct MemoryExtra<'tcx> { pub stacked_borrows: Option, pub intptrcast: intptrcast::MemoryExtra, @@ -85,11 +85,11 @@ pub struct MemoryExtra { /// (helps for debugging memory leaks). tracked_alloc_id: Option, - /// The `AllocId` for the `environ` static. - pub(crate) environ: Option>, + /// Place where the `environ` static is stored. + pub(crate) environ: Option>, } -impl MemoryExtra { +impl<'tcx> MemoryExtra<'tcx> { pub fn new(rng: StdRng, stacked_borrows: bool, tracked_pointer_tag: Option, tracked_alloc_id: Option) -> Self { let stacked_borrows = if stacked_borrows { Some(Rc::new(RefCell::new(stacked_borrows::GlobalState::new(tracked_pointer_tag)))) @@ -107,7 +107,7 @@ impl MemoryExtra { } /// Sets up the "extern statics" for this machine. - pub fn init_extern_statics<'mir, 'tcx>( + pub fn init_extern_statics<'mir>( this: &mut MiriEvalContext<'mir, 'tcx>, ) -> InterpResult<'tcx> { match this.tcx.sess.target.target.target_os.as_str() { @@ -126,12 +126,13 @@ impl MemoryExtra { // "environ" let layout = this.layout_of(this.tcx.types.usize)?; let place = this.allocate(layout, MiriMemoryKind::Machine.into()); - this.write_scalar(this.memory.extra.environ.unwrap(), place.into())?; + this.write_scalar(Scalar::from_machine_usize(0, &*this.tcx), place.into())?; this.memory .extra .extern_statics .insert(Symbol::intern("environ"), place.ptr.assert_ptr().alloc_id) .unwrap_none(); + this.memory.extra.environ = Some(place); } _ => {} // No "extern statics" supported on this platform } @@ -217,7 +218,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> { type MemoryKinds = MiriMemoryKind; type FrameExtra = FrameData<'tcx>; - type MemoryExtra = MemoryExtra; + type MemoryExtra = MemoryExtra<'tcx>; type AllocExtra = AllocExtra; type PointerTag = Tag; type ExtraFnVal = Dlsym; @@ -343,7 +344,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> { } fn init_allocation_extra<'b>( - memory_extra: &MemoryExtra, + memory_extra: &MemoryExtra<'tcx>, id: AllocId, alloc: Cow<'b, Allocation>, kind: Option>, @@ -380,7 +381,7 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for Evaluator<'tcx> { } #[inline(always)] - fn tag_static_base_pointer(memory_extra: &MemoryExtra, id: AllocId) -> Self::PointerTag { + fn tag_static_base_pointer(memory_extra: &MemoryExtra<'tcx>, id: AllocId) -> Self::PointerTag { if let Some(stacked_borrows) = memory_extra.stacked_borrows.as_ref() { stacked_borrows.borrow_mut().static_base_ptr(id) } else { diff --git a/src/shims/env.rs b/src/shims/env.rs index 833fef69f2..05c098bc4e 100644 --- a/src/shims/env.rs +++ b/src/shims/env.rs @@ -21,29 +21,16 @@ impl EnvVars { ecx: &mut InterpCx<'mir, 'tcx, Evaluator<'tcx>>, excluded_env_vars: Vec, ) { - let mut vars = Vec::new(); if ecx.machine.communicate { for (name, value) in env::vars() { if !excluded_env_vars.contains(&name) { let var_ptr = alloc_env_var_as_c_str(name.as_ref(), value.as_ref(), ecx); ecx.machine.env_vars.map.insert(OsString::from(name), var_ptr); - vars.push(var_ptr.into()); } } } - // Add the trailing null pointer - vars.push(Scalar::from_int(0, ecx.pointer_size())); - // Make an array with all these pointers inside Miri. - let tcx = ecx.tcx; - let environ_layout = - ecx.layout_of(tcx.mk_array(tcx.mk_imm_ptr(tcx.types.u8), vars.len() as u64)).unwrap(); - let environ_place = ecx.allocate(environ_layout, MiriMemoryKind::Machine.into()); - for (idx, var) in vars.into_iter().enumerate() { - let place = ecx.mplace_field(environ_place, idx as u64).unwrap(); - ecx.write_scalar(var, place.into()).unwrap(); - } - ecx.memory.extra.environ = Some(environ_place.ptr.into()); + ecx.update_environ().unwrap(); } } @@ -94,6 +81,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx if let Some((name, value)) = new { let var_ptr = alloc_env_var_as_c_str(&name, &value, &mut this); if let Some(var) = this.machine.env_vars.map.insert(name.to_owned(), var_ptr) { + this.update_environ()?; this.memory .deallocate(var, None, MiriMemoryKind::Machine.into())?; } @@ -112,6 +100,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let name = this.read_os_str_from_c_str(name_ptr)?.to_owned(); if !name.is_empty() && !name.to_string_lossy().contains('=') { success = Some(this.machine.env_vars.map.remove(&name)); + this.update_environ()?; } } if let Some(old) = success { @@ -165,4 +154,28 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx } } } + + fn update_environ(&mut self) -> InterpResult<'tcx> { + let this = self.eval_context_mut(); + // Collect all the pointers to each variable in a vector. + let mut vars: Vec> = this.machine.env_vars.map.values().map(|&ptr| ptr.into()).collect(); + // Add the trailing null pointer. + vars.push(Scalar::from_int(0, this.pointer_size())); + // Make an array with all these pointers inside Miri. + let tcx = this.tcx; + let vars_layout = + this.layout_of(tcx.mk_array(tcx.types.usize, vars.len() as u64))?; + let vars_place = this.allocate(vars_layout, MiriMemoryKind::Machine.into()); + for (idx, var) in vars.into_iter().enumerate() { + let place = this.mplace_field(vars_place, idx as u64)?; + this.write_scalar(var, place.into())?; + } + + this.write_scalar( + vars_place.ptr, + this.memory.extra.environ.unwrap().into(), + )?; + + Ok(()) + } } From a28330febb07e46b5f5bc73356eb4b36b549c51e Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Thu, 5 Mar 2020 09:41:35 -0500 Subject: [PATCH 3/9] add testcase for `environ` shim --- 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 faf9474203..0b7ee8adc1 100644 --- a/tests/run-pass/env.rs +++ b/tests/run-pass/env.rs @@ -8,4 +8,6 @@ fn main() { 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()); + // Test that the new variable is in the `std::env::Vars` iterator + assert!(env::vars().any(|(name, value)| name == "MIRI_TEST")) } From 7882dfb3f599df93d1e0d0a643e41072c6bf5683 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 5 Mar 2020 23:25:55 +0100 Subject: [PATCH 4/9] fix env update, and expand test --- src/shims/env.rs | 4 ++-- tests/run-pass/env.rs | 23 +++++++++++++++++++---- tests/run-pass/env.stdout | 14 ++++++++++++++ 3 files changed, 35 insertions(+), 6 deletions(-) create mode 100644 tests/run-pass/env.stdout diff --git a/src/shims/env.rs b/src/shims/env.rs index 05c098bc4e..3469beb971 100644 --- a/src/shims/env.rs +++ b/src/shims/env.rs @@ -81,10 +81,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx if let Some((name, value)) = new { let var_ptr = alloc_env_var_as_c_str(&name, &value, &mut this); if let Some(var) = this.machine.env_vars.map.insert(name.to_owned(), var_ptr) { - this.update_environ()?; this.memory .deallocate(var, None, MiriMemoryKind::Machine.into())?; } + this.update_environ()?; Ok(0) } else { Ok(-1) @@ -100,7 +100,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let name = this.read_os_str_from_c_str(name_ptr)?.to_owned(); if !name.is_empty() && !name.to_string_lossy().contains('=') { success = Some(this.machine.env_vars.map.remove(&name)); - this.update_environ()?; } } if let Some(old) = success { @@ -108,6 +107,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx this.memory .deallocate(var, None, MiriMemoryKind::Machine.into())?; } + this.update_environ()?; Ok(0) } else { Ok(-1) diff --git a/tests/run-pass/env.rs b/tests/run-pass/env.rs index 0b7ee8adc1..c7506b23c1 100644 --- a/tests/run-pass/env.rs +++ b/tests/run-pass/env.rs @@ -3,11 +3,26 @@ use std::env; fn main() { + // Test that miri environment is isolated when communication is disabled. + // (`MIRI_ENV_VAR_TEST` is set by the test harness.) + assert_eq!(env::var("MIRI_ENV_VAR_TEST"), Err(env::VarError::NotPresent)); + + // Test base state. + println!("{:#?}", env::vars().collect::>()); assert_eq!(env::var("MIRI_TEST"), Err(env::VarError::NotPresent)); + + // Set the variable. 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()); - // Test that the new variable is in the `std::env::Vars` iterator - assert!(env::vars().any(|(name, value)| name == "MIRI_TEST")) + println!("{:#?}", env::vars().collect::>()); + + // Change the variable. + env::set_var("MIRI_TEST", "42"); + assert_eq!(env::var("MIRI_TEST"), Ok("42".to_owned())); + println!("{:#?}", env::vars().collect::>()); + + // Remove the variable. + env::remove_var("MIRI_TEST"); + assert_eq!(env::var("MIRI_TEST"), Err(env::VarError::NotPresent)); + println!("{:#?}", env::vars().collect::>()); } diff --git a/tests/run-pass/env.stdout b/tests/run-pass/env.stdout new file mode 100644 index 0000000000..9a8f979598 --- /dev/null +++ b/tests/run-pass/env.stdout @@ -0,0 +1,14 @@ +[] +[ + ( + "MIRI_TEST", + "the answer", + ), +] +[ + ( + "MIRI_TEST", + "42", + ), +] +[] From aedc34c6e5ee8d675212bb60d54e1394e9ee9964 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Fri, 6 Mar 2020 09:38:16 -0500 Subject: [PATCH 5/9] deallocate old environ --- src/machine.rs | 2 +- src/shims/env.rs | 10 +++++++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/machine.rs b/src/machine.rs index ece4b62d3a..188001c170 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -85,7 +85,7 @@ pub struct MemoryExtra<'tcx> { /// (helps for debugging memory leaks). tracked_alloc_id: Option, - /// Place where the `environ` static is stored. + /// Place where the `environ` static is stored. Its value should not change after initialization. pub(crate) environ: Option>, } diff --git a/src/shims/env.rs b/src/shims/env.rs index 3469beb971..0408741cf2 100644 --- a/src/shims/env.rs +++ b/src/shims/env.rs @@ -155,8 +155,17 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx } } + /// Updates the `environ` static. It should not be called before + /// `MemoryExtra::init_extern_statics`. fn update_environ(&mut self) -> InterpResult<'tcx> { let this = self.eval_context_mut(); + // Deallocate the old environ value. + let old_vars_ptr = this.read_scalar(this.memory.extra.environ.unwrap().into())?.not_undef()?; + // The pointer itself can be null because `MemoryExtra::init_extern_statics` only + // initializes the place for the static but not the static itself. + if !this.is_null(old_vars_ptr)? { + this.memory.deallocate(this.force_ptr(old_vars_ptr)?, None, MiriMemoryKind::Machine.into())?; + } // Collect all the pointers to each variable in a vector. let mut vars: Vec> = this.machine.env_vars.map.values().map(|&ptr| ptr.into()).collect(); // Add the trailing null pointer. @@ -170,7 +179,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let place = this.mplace_field(vars_place, idx as u64)?; this.write_scalar(var, place.into())?; } - this.write_scalar( vars_place.ptr, this.memory.extra.environ.unwrap().into(), From 6eccc809f21c11f583ec237dffdbebc7e6b66038 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Sat, 7 Mar 2020 09:26:04 -0500 Subject: [PATCH 6/9] test that `environ` gets deallocated on changes --- tests/compile-fail/environ-gets-deallocated.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 tests/compile-fail/environ-gets-deallocated.rs diff --git a/tests/compile-fail/environ-gets-deallocated.rs b/tests/compile-fail/environ-gets-deallocated.rs new file mode 100644 index 0000000000..014c8c431e --- /dev/null +++ b/tests/compile-fail/environ-gets-deallocated.rs @@ -0,0 +1,10 @@ +extern "C" { + static environ: *const *const u8; +} + +fn main() { + let pointer = unsafe { environ }; + let _x = unsafe { *pointer }; + std::env::set_var("FOO", "BAR"); + let _y = unsafe { *pointer }; //~ ERROR dangling pointer was dereferenced +} From e31b8b3342f4fc97198609f5e7e13e484969c8b1 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Sat, 7 Mar 2020 09:39:44 -0500 Subject: [PATCH 7/9] add `_NSGetEnviron` foreign function for macos --- src/machine.rs | 28 ++++++++++++++------------ src/shims/foreign_items/posix/macos.rs | 5 ++++- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/machine.rs b/src/machine.rs index 188001c170..5a50a76dfb 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -110,19 +110,21 @@ impl<'tcx> MemoryExtra<'tcx> { pub fn init_extern_statics<'mir>( this: &mut MiriEvalContext<'mir, 'tcx>, ) -> InterpResult<'tcx> { - match this.tcx.sess.target.target.target_os.as_str() { - "linux" => { - // "__cxa_thread_atexit_impl" - // This should be all-zero, pointer-sized. - let layout = this.layout_of(this.tcx.types.usize)?; - let place = this.allocate(layout, MiriMemoryKind::Machine.into()); - this.write_scalar(Scalar::from_machine_usize(0, &*this.tcx), place.into())?; - this.memory - .extra - .extern_statics - .insert(Symbol::intern("__cxa_thread_atexit_impl"), place.ptr.assert_ptr().alloc_id) - .unwrap_none(); - + let target_os = this.tcx.sess.target.target.target_os.as_str(); + match target_os { + "linux" | "macos" => { + if target_os == "linux" { + // "__cxa_thread_atexit_impl" + // This should be all-zero, pointer-sized. + let layout = this.layout_of(this.tcx.types.usize)?; + let place = this.allocate(layout, MiriMemoryKind::Machine.into()); + this.write_scalar(Scalar::from_machine_usize(0, &*this.tcx), place.into())?; + this.memory + .extra + .extern_statics + .insert(Symbol::intern("__cxa_thread_atexit_impl"), place.ptr.assert_ptr().alloc_id) + .unwrap_none(); + } // "environ" let layout = this.layout_of(this.tcx.types.usize)?; let place = this.allocate(layout, MiriMemoryKind::Machine.into()); diff --git a/src/shims/foreign_items/posix/macos.rs b/src/shims/foreign_items/posix/macos.rs index cb6cd9ba44..d167191279 100644 --- a/src/shims/foreign_items/posix/macos.rs +++ b/src/shims/foreign_items/posix/macos.rs @@ -41,7 +41,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let result = this.macos_fstat(args[0], args[1])?; this.write_scalar(Scalar::from_int(result, dest.layout.size), dest)?; } - + // Environment related shims + "_NSGetEnviron" => { + this.write_scalar(this.memory.extra.environ.unwrap().ptr, dest)?; + } // The only reason this is not in the `posix` module is because the `linux` item has a // different name. "opendir$INODE64" => { From 18a71ef7b36fa8ed2031b69e414261e086fd711c Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Sat, 7 Mar 2020 11:35:00 -0500 Subject: [PATCH 8/9] minor corrections --- src/eval.rs | 2 +- src/machine.rs | 2 +- src/shims/env.rs | 4 ++-- tests/compile-fail/environ-gets-deallocated.rs | 18 ++++++++++++++++-- 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/eval.rs b/src/eval.rs index 1981a8d1e0..13b55a93f2 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -78,7 +78,7 @@ pub fn create_ecx<'mir, 'tcx: 'mir>( ); // Complete initialization. MemoryExtra::init_extern_statics(&mut ecx)?; - EnvVars::init(&mut ecx, config.excluded_env_vars); + EnvVars::init(&mut ecx, config.excluded_env_vars)?; // Setup first stack-frame let main_instance = ty::Instance::mono(tcx, main_id); diff --git a/src/machine.rs b/src/machine.rs index 5a50a76dfb..0f0c443001 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -85,7 +85,7 @@ pub struct MemoryExtra<'tcx> { /// (helps for debugging memory leaks). tracked_alloc_id: Option, - /// Place where the `environ` static is stored. Its value should not change after initialization. + /// Place where the `environ` static is stored. Lazily initialized, but then never changes. pub(crate) environ: Option>, } diff --git a/src/shims/env.rs b/src/shims/env.rs index 0408741cf2..1962ca026c 100644 --- a/src/shims/env.rs +++ b/src/shims/env.rs @@ -20,7 +20,7 @@ impl EnvVars { pub(crate) fn init<'mir, 'tcx>( ecx: &mut InterpCx<'mir, 'tcx, Evaluator<'tcx>>, excluded_env_vars: Vec, - ) { + ) -> InterpResult<'tcx> { if ecx.machine.communicate { for (name, value) in env::vars() { if !excluded_env_vars.contains(&name) { @@ -30,7 +30,7 @@ impl EnvVars { } } } - ecx.update_environ().unwrap(); + ecx.update_environ() } } diff --git a/tests/compile-fail/environ-gets-deallocated.rs b/tests/compile-fail/environ-gets-deallocated.rs index 014c8c431e..a3d82a8842 100644 --- a/tests/compile-fail/environ-gets-deallocated.rs +++ b/tests/compile-fail/environ-gets-deallocated.rs @@ -1,9 +1,23 @@ -extern "C" { +//ignore-windows: TODO env var emulation stubbed out on Windows + +#[cfg(target_os="linux")] +fn get_environ() -> *const *const u8 { + extern "C" { static environ: *const *const u8; + } + environ +} + +#[cfg(target_os="macos")] +fn get_environ() -> *const *const u8 { + extern "C" { + fn _NSGetEnviron() -> *mut *const *const u8; + } + unsafe { *_NSGetEnviron() } } fn main() { - let pointer = unsafe { environ }; + let pointer = get_environ(); let _x = unsafe { *pointer }; std::env::set_var("FOO", "BAR"); let _y = unsafe { *pointer }; //~ ERROR dangling pointer was dereferenced From 8392a0c589461e998a28a52a070b5fa5a143cf77 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Sat, 7 Mar 2020 15:33:27 -0500 Subject: [PATCH 9/9] only expose environ on linux --- src/eval.rs | 2 +- src/machine.rs | 26 +++++++------------ src/shims/env.rs | 9 +++++-- src/shims/foreign_items/posix/macos.rs | 10 ++++--- .../compile-fail/environ-gets-deallocated.rs | 4 +-- 5 files changed, 26 insertions(+), 25 deletions(-) diff --git a/src/eval.rs b/src/eval.rs index 13b55a93f2..a82c40a99e 100644 --- a/src/eval.rs +++ b/src/eval.rs @@ -77,8 +77,8 @@ pub fn create_ecx<'mir, 'tcx: 'mir>( ), ); // Complete initialization. - MemoryExtra::init_extern_statics(&mut ecx)?; EnvVars::init(&mut ecx, config.excluded_env_vars)?; + MemoryExtra::init_extern_statics(&mut ecx)?; // Setup first stack-frame let main_instance = ty::Instance::mono(tcx, main_id); diff --git a/src/machine.rs b/src/machine.rs index 0f0c443001..d21ff32897 100644 --- a/src/machine.rs +++ b/src/machine.rs @@ -112,29 +112,23 @@ impl<'tcx> MemoryExtra<'tcx> { ) -> InterpResult<'tcx> { let target_os = this.tcx.sess.target.target.target_os.as_str(); match target_os { - "linux" | "macos" => { - if target_os == "linux" { - // "__cxa_thread_atexit_impl" - // This should be all-zero, pointer-sized. - let layout = this.layout_of(this.tcx.types.usize)?; - let place = this.allocate(layout, MiriMemoryKind::Machine.into()); - this.write_scalar(Scalar::from_machine_usize(0, &*this.tcx), place.into())?; - this.memory - .extra - .extern_statics - .insert(Symbol::intern("__cxa_thread_atexit_impl"), place.ptr.assert_ptr().alloc_id) - .unwrap_none(); - } - // "environ" + "linux" => { + // "__cxa_thread_atexit_impl" + // This should be all-zero, pointer-sized. let layout = this.layout_of(this.tcx.types.usize)?; let place = this.allocate(layout, MiriMemoryKind::Machine.into()); this.write_scalar(Scalar::from_machine_usize(0, &*this.tcx), place.into())?; this.memory .extra .extern_statics - .insert(Symbol::intern("environ"), place.ptr.assert_ptr().alloc_id) + .insert(Symbol::intern("__cxa_thread_atexit_impl"), place.ptr.assert_ptr().alloc_id) + .unwrap_none(); + // "environ" + this.memory + .extra + .extern_statics + .insert(Symbol::intern("environ"), this.memory.extra.environ.unwrap().ptr.assert_ptr().alloc_id) .unwrap_none(); - this.memory.extra.environ = Some(place); } _ => {} // No "extern statics" supported on this platform } diff --git a/src/shims/env.rs b/src/shims/env.rs index 1962ca026c..aaecbebc36 100644 --- a/src/shims/env.rs +++ b/src/shims/env.rs @@ -30,6 +30,11 @@ impl EnvVars { } } } + // Initialize the `environ` static + let layout = ecx.layout_of(ecx.tcx.types.usize)?; + let place = ecx.allocate(layout, MiriMemoryKind::Machine.into()); + ecx.write_scalar(Scalar::from_machine_usize(0, &*ecx.tcx), place.into())?; + ecx.memory.extra.environ = Some(place); ecx.update_environ() } } @@ -156,12 +161,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx } /// Updates the `environ` static. It should not be called before - /// `MemoryExtra::init_extern_statics`. + /// `EnvVars::init`. fn update_environ(&mut self) -> InterpResult<'tcx> { let this = self.eval_context_mut(); // Deallocate the old environ value. let old_vars_ptr = this.read_scalar(this.memory.extra.environ.unwrap().into())?.not_undef()?; - // The pointer itself can be null because `MemoryExtra::init_extern_statics` only + // The pointer itself can be null because `EnvVars::init` only // initializes the place for the static but not the static itself. if !this.is_null(old_vars_ptr)? { this.memory.deallocate(this.force_ptr(old_vars_ptr)?, None, MiriMemoryKind::Machine.into())?; diff --git a/src/shims/foreign_items/posix/macos.rs b/src/shims/foreign_items/posix/macos.rs index d167191279..c5c6423e85 100644 --- a/src/shims/foreign_items/posix/macos.rs +++ b/src/shims/foreign_items/posix/macos.rs @@ -41,10 +41,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let result = this.macos_fstat(args[0], args[1])?; this.write_scalar(Scalar::from_int(result, dest.layout.size), dest)?; } - // Environment related shims - "_NSGetEnviron" => { - this.write_scalar(this.memory.extra.environ.unwrap().ptr, dest)?; - } + // The only reason this is not in the `posix` module is because the `linux` item has a // different name. "opendir$INODE64" => { @@ -59,6 +56,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx this.write_scalar(Scalar::from_int(result, dest.layout.size), dest)?; } + // Environment related shims + "_NSGetEnviron" => { + this.write_scalar(this.memory.extra.environ.unwrap().ptr, dest)?; + } + // Time related shims "gettimeofday" => { let result = this.gettimeofday(args[0], args[1])?; diff --git a/tests/compile-fail/environ-gets-deallocated.rs b/tests/compile-fail/environ-gets-deallocated.rs index a3d82a8842..6131613fc0 100644 --- a/tests/compile-fail/environ-gets-deallocated.rs +++ b/tests/compile-fail/environ-gets-deallocated.rs @@ -3,9 +3,9 @@ #[cfg(target_os="linux")] fn get_environ() -> *const *const u8 { extern "C" { - static environ: *const *const u8; + static mut environ: *const *const u8; } - environ + unsafe { environ } } #[cfg(target_os="macos")]