From 5cf90bc7866cd8f24178f2696e4fcbb7bfad0171 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Wed, 18 Sep 2019 16:10:13 -0500 Subject: [PATCH 1/7] Add getcwd shim --- src/shims/env.rs | 28 +++++++++++++++++++++++++++- src/shims/foreign_items.rs | 5 +++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/src/shims/env.rs b/src/shims/env.rs index 1cffc60bc4..73627086e0 100644 --- a/src/shims/env.rs +++ b/src/shims/env.rs @@ -1,4 +1,5 @@ use std::collections::HashMap; +use std::env; use rustc::ty::layout::{Size}; use rustc_mir::interpret::{Pointer, Memory}; @@ -21,7 +22,7 @@ impl EnvVars { excluded_env_vars.push("TERM".to_owned()); if ecx.machine.communicate { - for (name, value) in std::env::vars() { + for (name, value) in env::vars() { if !excluded_env_vars.contains(&name) { let var_ptr = alloc_env_var(name.as_bytes(), value.as_bytes(), ecx.memory_mut()); ecx.machine.env_vars.map.insert(name.into_bytes(), var_ptr); @@ -111,4 +112,29 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx Ok(-1) } } + + fn getcwd( + &mut self, + buf_op: OpTy<'tcx, Tag>, + size_op: OpTy<'tcx, Tag>, + ) -> InterpResult<'tcx, Scalar> { + let this = self.eval_context_mut(); + let tcx = &{this.tcx.tcx}; + + let buf = this.force_ptr(this.read_scalar(buf_op)?.not_undef()?)?; + let size = this.read_scalar(size_op)?.to_usize(&*this.tcx)?; + // If we cannot get the current directory, we return null + if let Ok(cwd) = env::current_dir() { + // It is not clear what happens with non-utf8 paths here + let mut bytes = cwd.display().to_string().into_bytes(); + // If the buffer is smaller than the path, we return null + if bytes.len() as u64 <= size { + // We need `size` bytes exactly + bytes.resize(size as usize, 0); + this.memory_mut().get_mut(buf.alloc_id)?.write_bytes(tcx, buf, &bytes)?; + return Ok(Scalar::Ptr(buf)) + } + } + Ok(Scalar::ptr_null(&*this.tcx)) + } } diff --git a/src/shims/foreign_items.rs b/src/shims/foreign_items.rs index 90c18265fc..45f167bea5 100644 --- a/src/shims/foreign_items.rs +++ b/src/shims/foreign_items.rs @@ -436,6 +436,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx this.write_scalar(Scalar::from_int(result, dest.layout.size), dest)?; } + "getcwd" => { + let result = this.getcwd(args[0], args[1])?; + this.write_scalar(result, dest)?; + } + "write" => { let fd = this.read_scalar(args[0])?.to_i32()?; let buf = this.read_scalar(args[1])?.not_undef()?; From 6593563e46d025aa0eb7a12a34085b7337b62c5b Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Wed, 18 Sep 2019 16:46:41 -0500 Subject: [PATCH 2/7] Check that getcwd does not error --- tests/run-pass/get_current_dir.rs | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 tests/run-pass/get_current_dir.rs diff --git a/tests/run-pass/get_current_dir.rs b/tests/run-pass/get_current_dir.rs new file mode 100644 index 0000000000..d8e6c225e9 --- /dev/null +++ b/tests/run-pass/get_current_dir.rs @@ -0,0 +1,5 @@ +// ignore-windows: TODO the windows hook is not done yet + +fn main() { + std::env::current_dir().unwrap(); +} From 133c2b39db73843e6a5ee0bce325cca034012579 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Thu, 19 Sep 2019 10:32:18 -0500 Subject: [PATCH 3/7] Only use getcwd without isolation --- src/shims/env.rs | 32 +++++++++++++++++-------------- tests/run-pass/get_current_dir.rs | 1 + 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/shims/env.rs b/src/shims/env.rs index 73627086e0..debeab894d 100644 --- a/src/shims/env.rs +++ b/src/shims/env.rs @@ -119,22 +119,26 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx size_op: OpTy<'tcx, Tag>, ) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - let tcx = &{this.tcx.tcx}; - let buf = this.force_ptr(this.read_scalar(buf_op)?.not_undef()?)?; - let size = this.read_scalar(size_op)?.to_usize(&*this.tcx)?; - // If we cannot get the current directory, we return null - if let Ok(cwd) = env::current_dir() { - // It is not clear what happens with non-utf8 paths here - let mut bytes = cwd.display().to_string().into_bytes(); - // If the buffer is smaller than the path, we return null - if bytes.len() as u64 <= size { - // We need `size` bytes exactly - bytes.resize(size as usize, 0); - this.memory_mut().get_mut(buf.alloc_id)?.write_bytes(tcx, buf, &bytes)?; - return Ok(Scalar::Ptr(buf)) + if this.machine.communicate { + let tcx = &{this.tcx.tcx}; + + let buf = this.force_ptr(this.read_scalar(buf_op)?.not_undef()?)?; + let size = this.read_scalar(size_op)?.to_usize(&*this.tcx)?; + // If we cannot get the current directory, we return null + if let Ok(cwd) = env::current_dir() { + // It is not clear what happens with non-utf8 paths here + let mut bytes = cwd.display().to_string().into_bytes(); + // If the buffer is smaller than the path, we return null + if bytes.len() as u64 <= size { + // We need `size` bytes exactly + bytes.resize(size as usize, 0); + this.memory_mut().get_mut(buf.alloc_id)?.write_bytes(tcx, buf, &bytes)?; + return Ok(Scalar::Ptr(buf)) + } } + return Ok(Scalar::ptr_null(&*this.tcx)); } - Ok(Scalar::ptr_null(&*this.tcx)) + throw_unsup_format!("Function not available when isolation is enabled") } } diff --git a/tests/run-pass/get_current_dir.rs b/tests/run-pass/get_current_dir.rs index d8e6c225e9..45efd06d3f 100644 --- a/tests/run-pass/get_current_dir.rs +++ b/tests/run-pass/get_current_dir.rs @@ -1,4 +1,5 @@ // ignore-windows: TODO the windows hook is not done yet +// compile-flags: -Zmiri-disable-isolation fn main() { std::env::current_dir().unwrap(); From 49275d42690318e410f65e3a38d04f2c4ccfbd0c Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Fri, 20 Sep 2019 02:13:48 -0500 Subject: [PATCH 4/7] Avoid writing more bytes than necessary --- src/shims/env.rs | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/src/shims/env.rs b/src/shims/env.rs index debeab894d..5e81b94e69 100644 --- a/src/shims/env.rs +++ b/src/shims/env.rs @@ -120,25 +120,29 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx ) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); - if this.machine.communicate { - let tcx = &{this.tcx.tcx}; + if !this.machine.communicate { + throw_unsup_format!("Function not available when isolation is enabled") + } - let buf = this.force_ptr(this.read_scalar(buf_op)?.not_undef()?)?; - let size = this.read_scalar(size_op)?.to_usize(&*this.tcx)?; - // If we cannot get the current directory, we return null - if let Ok(cwd) = env::current_dir() { - // It is not clear what happens with non-utf8 paths here - let mut bytes = cwd.display().to_string().into_bytes(); - // If the buffer is smaller than the path, we return null - if bytes.len() as u64 <= size { - // We need `size` bytes exactly - bytes.resize(size as usize, 0); - this.memory_mut().get_mut(buf.alloc_id)?.write_bytes(tcx, buf, &bytes)?; - return Ok(Scalar::Ptr(buf)) - } + let tcx = &{this.tcx.tcx}; + + let buf = this.force_ptr(this.read_scalar(buf_op)?.not_undef()?)?; + let size = this.read_scalar(size_op)?.to_usize(&*this.tcx)?; + // If we cannot get the current directory, we return null + // FIXME: Technically we have to set the `errno` global too + if let Ok(cwd) = env::current_dir() { + // It is not clear what happens with non-utf8 paths here + let mut bytes = cwd.display().to_string().into_bytes(); + // If the buffer is smaller or equal than the path, we return null. + // FIXME: Technically we have to set the `errno` global too + if (bytes.len() as u64) < size { + // We add a `/0` terminator + bytes.push(0); + // This is ok because the buffer is larger than the path with the null terminator. + this.memory_mut().get_mut(buf.alloc_id)?.write_bytes(tcx, buf, &bytes)?; + return Ok(Scalar::Ptr(buf)) } - return Ok(Scalar::ptr_null(&*this.tcx)); } - throw_unsup_format!("Function not available when isolation is enabled") + Ok(Scalar::ptr_null(&*this.tcx)) } } From c0a6b5ff69580325e590d1062570be16c0a93f14 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Fri, 20 Sep 2019 03:30:55 -0500 Subject: [PATCH 5/7] Set errno when getcwd fails --- src/shims/env.rs | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/shims/env.rs b/src/shims/env.rs index 5e81b94e69..d766d8f0c1 100644 --- a/src/shims/env.rs +++ b/src/shims/env.rs @@ -130,18 +130,21 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let size = this.read_scalar(size_op)?.to_usize(&*this.tcx)?; // If we cannot get the current directory, we return null // FIXME: Technically we have to set the `errno` global too - if let Ok(cwd) = env::current_dir() { - // It is not clear what happens with non-utf8 paths here - let mut bytes = cwd.display().to_string().into_bytes(); - // If the buffer is smaller or equal than the path, we return null. - // FIXME: Technically we have to set the `errno` global too - if (bytes.len() as u64) < size { - // We add a `/0` terminator - bytes.push(0); - // This is ok because the buffer is larger than the path with the null terminator. - this.memory_mut().get_mut(buf.alloc_id)?.write_bytes(tcx, buf, &bytes)?; - return Ok(Scalar::Ptr(buf)) + match env::current_dir() { + Ok(cwd) =>{ + // It is not clear what happens with non-utf8 paths here + let mut bytes = cwd.display().to_string().into_bytes(); + // If the buffer is smaller or equal than the path, we return null. + if (bytes.len() as u64) < size { + // We add a `/0` terminator + bytes.push(0); + // This is ok because the buffer is larger than the path with the null terminator. + this.memory_mut().get_mut(buf.alloc_id)?.write_bytes(tcx, buf, &bytes)?; + return Ok(Scalar::Ptr(buf)) + } + this.machine.last_error = 34; // ERANGE } + Err(e) => this.machine.last_error = e.raw_os_error().unwrap() as u32, } Ok(Scalar::ptr_null(&*this.tcx)) } From 0f58289b3d799e04aa2b0027c4a7ff571c7a7511 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Fri, 20 Sep 2019 10:25:43 -0500 Subject: [PATCH 6/7] fetch ERANGE value from libc --- src/shims/env.rs | 42 +++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/src/shims/env.rs b/src/shims/env.rs index d766d8f0c1..784df0a50f 100644 --- a/src/shims/env.rs +++ b/src/shims/env.rs @@ -1,10 +1,10 @@ use std::collections::HashMap; use std::env; -use rustc::ty::layout::{Size}; -use rustc_mir::interpret::{Pointer, Memory}; use crate::stacked_borrows::Tag; use crate::*; +use rustc::ty::layout::Size; +use rustc_mir::interpret::{Memory, Pointer}; #[derive(Default)] pub struct EnvVars { @@ -24,7 +24,8 @@ impl EnvVars { if ecx.machine.communicate { for (name, value) in env::vars() { if !excluded_env_vars.contains(&name) { - let var_ptr = alloc_env_var(name.as_bytes(), value.as_bytes(), ecx.memory_mut()); + let var_ptr = + alloc_env_var(name.as_bytes(), value.as_bytes(), ecx.memory_mut()); ecx.machine.env_vars.map.insert(name.into_bytes(), var_ptr); } } @@ -46,17 +47,16 @@ fn alloc_env_var<'mir, 'tcx>( impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {} pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> { - fn getenv( - &mut self, - name_op: OpTy<'tcx, Tag>, - ) -> InterpResult<'tcx, Scalar> { + fn getenv(&mut self, name_op: OpTy<'tcx, Tag>) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); let name_ptr = this.read_scalar(name_op)?.not_undef()?; let name = this.memory().read_c_str(name_ptr)?; Ok(match this.machine.env_vars.map.get(name) { // The offset is used to strip the "{name}=" part of the string. - Some(var_ptr) => Scalar::Ptr(var_ptr.offset(Size::from_bytes(name.len() as u64 + 1), this)?), + Some(var_ptr) => { + Scalar::Ptr(var_ptr.offset(Size::from_bytes(name.len() as u64 + 1), this)?) + } None => Scalar::ptr_null(&*this.tcx), }) } @@ -81,7 +81,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx if let Some((name, value)) = new { let var_ptr = alloc_env_var(&name, &value, this.memory_mut()); if let Some(var) = this.machine.env_vars.map.insert(name.to_owned(), var_ptr) { - this.memory_mut().deallocate(var, None, MiriMemoryKind::Env.into())?; + this.memory_mut() + .deallocate(var, None, MiriMemoryKind::Env.into())?; } Ok(0) } else { @@ -89,10 +90,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx } } - fn unsetenv( - &mut self, - name_op: OpTy<'tcx, Tag>, - ) -> InterpResult<'tcx, i32> { + fn unsetenv(&mut self, name_op: OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> { let this = self.eval_context_mut(); let name_ptr = this.read_scalar(name_op)?.not_undef()?; @@ -105,7 +103,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx } if let Some(old) = success { if let Some(var) = old { - this.memory_mut().deallocate(var, None, MiriMemoryKind::Env.into())?; + this.memory_mut() + .deallocate(var, None, MiriMemoryKind::Env.into())?; } Ok(0) } else { @@ -124,14 +123,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx throw_unsup_format!("Function not available when isolation is enabled") } - let tcx = &{this.tcx.tcx}; + let tcx = &{ this.tcx.tcx }; let buf = this.force_ptr(this.read_scalar(buf_op)?.not_undef()?)?; let size = this.read_scalar(size_op)?.to_usize(&*this.tcx)?; // If we cannot get the current directory, we return null // FIXME: Technically we have to set the `errno` global too match env::current_dir() { - Ok(cwd) =>{ + Ok(cwd) => { // It is not clear what happens with non-utf8 paths here let mut bytes = cwd.display().to_string().into_bytes(); // If the buffer is smaller or equal than the path, we return null. @@ -139,10 +138,15 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx // We add a `/0` terminator bytes.push(0); // This is ok because the buffer is larger than the path with the null terminator. - this.memory_mut().get_mut(buf.alloc_id)?.write_bytes(tcx, buf, &bytes)?; - return Ok(Scalar::Ptr(buf)) + this.memory_mut() + .get_mut(buf.alloc_id)? + .write_bytes(tcx, buf, &bytes)?; + return Ok(Scalar::Ptr(buf)); } - this.machine.last_error = 34; // ERANGE + this.machine.last_error = this + .eval_path_scalar(&["libc", "ERANGE"])? + .unwrap() + .to_u32()?; } Err(e) => this.machine.last_error = e.raw_os_error().unwrap() as u32, } From 02261e4be2c5c8d569ac98f6d7c7623c6d26fe47 Mon Sep 17 00:00:00 2001 From: Christian Poveda Date: Tue, 24 Sep 2019 09:29:16 -0500 Subject: [PATCH 7/7] Fix comments --- src/shims/env.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/shims/env.rs b/src/shims/env.rs index 784df0a50f..e2a9452045 100644 --- a/src/shims/env.rs +++ b/src/shims/env.rs @@ -120,7 +120,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let this = self.eval_context_mut(); if !this.machine.communicate { - throw_unsup_format!("Function not available when isolation is enabled") + throw_unsup_format!("`getcwd` not available when isolation is enabled") } let tcx = &{ this.tcx.tcx }; @@ -128,7 +128,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx let buf = this.force_ptr(this.read_scalar(buf_op)?.not_undef()?)?; let size = this.read_scalar(size_op)?.to_usize(&*this.tcx)?; // If we cannot get the current directory, we return null - // FIXME: Technically we have to set the `errno` global too match env::current_dir() { Ok(cwd) => { // It is not clear what happens with non-utf8 paths here