Skip to content

Environ shim #1208

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

Merged
merged 9 commits into from
Mar 8, 2020
Merged
Changes from all 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: 1 addition & 1 deletion src/eval.rs
Original file line number Diff line number Diff line change
@@ -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);
25 changes: 18 additions & 7 deletions src/machine.rs
Original file line number Diff line number Diff line change
@@ -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. Lazily initialized, but then never changes.
pub(crate) environ: Option<MPlaceTy<'tcx, Tag>>,
Comment on lines +88 to +89
Copy link
Member

@RalfJung RalfJung Mar 8, 2020

Choose a reason for hiding this comment

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

I just realized we already have the EnvVars struct for extra state for environment variables... @christianpoveda would you mind submitting a follow-up PR to move environ over to that struct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure

}

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,14 +102,16 @@ 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() {
let target_os = this.tcx.sess.target.target.target_os.as_str();
match target_os {
"linux" => {
// "__cxa_thread_atexit_impl"
// This should be all-zero, pointer-sized.
@@ -118,6 +123,12 @@ impl MemoryExtra {
.extern_statics
.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();
}
_ => {} // No "extern statics" supported on this platform
}
@@ -203,7 +214,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 +340,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 +377,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 {
43 changes: 42 additions & 1 deletion src/shims/env.rs
Original file line number Diff line number Diff line change
@@ -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;
@@ -19,7 +20,7 @@ impl EnvVars {
pub(crate) fn init<'mir, 'tcx>(
ecx: &mut InterpCx<'mir, 'tcx, Evaluator<'tcx>>,
excluded_env_vars: Vec<String>,
) {
) -> InterpResult<'tcx> {
if ecx.machine.communicate {
for (name, value) in env::vars() {
if !excluded_env_vars.contains(&name) {
@@ -29,6 +30,12 @@ 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()
}
}

@@ -82,6 +89,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 +112,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 +159,36 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
}
}
}

/// Updates the `environ` static. It should not be called before
/// `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 `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())?;
}
// 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(())
}
}
5 changes: 5 additions & 0 deletions src/shims/foreign_items/posix/macos.rs
Original file line number Diff line number Diff line change
@@ -56,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])?;
24 changes: 24 additions & 0 deletions tests/compile-fail/environ-gets-deallocated.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
//ignore-windows: TODO env var emulation stubbed out on Windows

#[cfg(target_os="linux")]
fn get_environ() -> *const *const u8 {
extern "C" {
static mut environ: *const *const u8;
}
unsafe { 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 = get_environ();
let _x = unsafe { *pointer };
std::env::set_var("FOO", "BAR");
let _y = unsafe { *pointer }; //~ ERROR dangling pointer was dereferenced
}
21 changes: 19 additions & 2 deletions tests/run-pass/env.rs
Original file line number Diff line number Diff line change
@@ -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<_>>());
}
14 changes: 14 additions & 0 deletions tests/run-pass/env.stdout
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[]
[
(
"MIRI_TEST",
"the answer",
),
]
[
(
"MIRI_TEST",
"42",
),
]
[]