Environ shim

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<stacked_borrows::MemoryExtra>,
     pub intptrcast: intptrcast::MemoryExtra,
 
@@ -84,9 +84,12 @@ pub struct MemoryExtra {
     /// An allocation ID to report when it is being allocated
     /// (helps for debugging memory leaks).
     tracked_alloc_id: Option<AllocId>,
+
+    /// Place where the `environ` static is stored. Its value should not change after initialization.
+    pub(crate) environ: Option<MPlaceTy<'tcx, Tag>>,
 }
 
-impl MemoryExtra {
+impl<'tcx> MemoryExtra<'tcx> {
     pub fn new(rng: StdRng, stacked_borrows: bool, tracked_pointer_tag: Option<PtrId>, tracked_alloc_id: Option<AllocId>) -> Self {
         let stacked_borrows = if stacked_borrows {
             Some(Rc::new(RefCell::new(stacked_borrows::GlobalState::new(tracked_pointer_tag))))
@@ -99,25 +102,39 @@ impl MemoryExtra {
             extern_statics: FxHashMap::default(),
             rng: RefCell::new(rng),
             tracked_alloc_id,
+            environ: None,
         }
     }
 
     /// 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() {
-            "linux" => {
-                // "__cxa_thread_atexit_impl"
-                // This should be all-zero, pointer-sized.
+        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());
                 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)
+                    .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
         }
@@ -203,7 +220,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;
@@ -329,7 +346,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<MemoryKind<Self::MemoryKinds>>,
@@ -366,7 +383,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 {

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;
@@ -29,6 +30,7 @@ impl EnvVars {
                 }
             }
         }
+        ecx.update_environ().unwrap();
     }
 }
 
@@ -82,6 +84,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)
@@ -104,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)
@@ -150,4 +154,36 @@ 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<Scalar<Tag>> = 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(())
+    }
 }

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" => {

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
+}

tests/run-pass/env.rs

@@ -3,9 +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::<Vec<_>>());
     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());
+    println!("{:#?}", env::vars().collect::<Vec<_>>());
+
+    // Change the variable.
+    env::set_var("MIRI_TEST", "42");
+    assert_eq!(env::var("MIRI_TEST"), Ok("42".to_owned()));
+    println!("{:#?}", env::vars().collect::<Vec<_>>());
+
+    // Remove the variable.
+    env::remove_var("MIRI_TEST");
+    assert_eq!(env::var("MIRI_TEST"), Err(env::VarError::NotPresent));
+    println!("{:#?}", env::vars().collect::<Vec<_>>());
 }

tests/run-pass/env.stdout

@@ -0,0 +1,14 @@
+[]
+[
+    (
+        "MIRI_TEST",
+        "the answer",
+    ),
+]
+[
+    (
+        "MIRI_TEST",
+        "42",
+    ),
+]
+[]