From a481b8f261a196ec8b4107273705a750c545856b Mon Sep 17 00:00:00 2001 From: JOE1994 Date: Sat, 28 Mar 2020 14:42:22 -0400 Subject: [PATCH 1/7] partially implement 'set_last_error_from_io_error' for Windows --- src/helpers.rs | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/src/helpers.rs b/src/helpers.rs index d0a5fc95866..751d77a3897 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -71,6 +71,13 @@ fn eval_libc(&mut self, name: &str) -> InterpResult<'tcx, Scalar> { .not_undef() } + /// Helper function to get a `windows` constant as a `Scalar`. + fn eval_windows(&mut self, name: &str) -> InterpResult<'tcx, Scalar> { + self.eval_context_mut() + .eval_path_scalar(&["std", "sys", "windows", "c", name])? + .not_undef() + } + /// Helper function to get a `libc` constant as an `i32`. fn eval_libc_i32(&mut self, name: &str) -> InterpResult<'tcx, i32> { // TODO: Cache the result. @@ -428,11 +435,14 @@ fn set_last_error_from_io_error(&mut self, e: std::io::Error) -> InterpResult<'t } })? } else { - // FIXME: we have to implement the Windows equivalent of this. - throw_unsup_format!( - "setting the last OS error from an io::Error is unsupported for {}.", - target.target_os - ) + // FIXME: we have to finish implementing the Windows equivalent of this. + this.eval_windows(match e.kind() { + NotFound => "ERROR_FILE_NOT_FOUND", + _ => throw_unsup_format!( + "setting the last OS error from an io::Error is yet unsupported for {}.", + target.target_os + ) + })? }; this.set_last_error(last_error) } From 1141b21e50121976567cb54d6bd227bb7d5e1b61 Mon Sep 17 00:00:00 2001 From: JOE1994 Date: Sat, 28 Mar 2020 14:44:41 -0400 Subject: [PATCH 2/7] Windows shims for GetCurrentDirectoryW/SetCurrentDirectoryW --- src/shims/env.rs | 54 ++++++++++++- src/shims/foreign_items/windows.rs | 10 +++ src/shims/os_str.rs | 117 +++++++++++++++-------------- tests/run-pass/current_dir.rs | 7 +- 4 files changed, 127 insertions(+), 61 deletions(-) diff --git a/src/shims/env.rs b/src/shims/env.rs index 8a69e725abf..78e7c37d9ce 100644 --- a/src/shims/env.rs +++ b/src/shims/env.rs @@ -138,8 +138,8 @@ fn GetEnvironmentVariableW( } } None => { - let envvar_not_found = this.eval_path_scalar(&["std", "sys", "windows", "c", "ERROR_ENVVAR_NOT_FOUND"])?; - this.set_last_error(envvar_not_found.not_undef()?)?; + let envvar_not_found = this.eval_windows("ERROR_ENVVAR_NOT_FOUND")?; + this.set_last_error(envvar_not_found)?; 0 // return zero upon failure } }) @@ -289,6 +289,8 @@ fn getcwd( size_op: OpTy<'tcx, Tag>, ) -> InterpResult<'tcx, Scalar> { let this = self.eval_context_mut(); + let target_os = &this.tcx.sess.target.target.target_os; + assert!(target_os == "linux" || target_os == "macos", "`getcwd` is only available for the UNIX target family"); this.check_no_isolation("getcwd")?; @@ -308,8 +310,35 @@ fn getcwd( Ok(Scalar::null_ptr(&*this.tcx)) } + #[allow(non_snake_case)] + fn GetCurrentDirectoryW( + &mut self, + size_op: OpTy<'tcx, Tag>, // DWORD + buf_op: OpTy<'tcx, Tag>, // LPTSTR + ) -> InterpResult<'tcx, u32> { + let this = self.eval_context_mut(); + this.assert_target_os("windows", "GetCurrentDirectoryW"); + + this.check_no_isolation("GetCurrentDirectoryW")?; + + let size = u64::from(this.read_scalar(size_op)?.to_u32()?); + let buf = this.read_scalar(buf_op)?.not_undef()?; + + // If we cannot get the current directory, we return 0 + match env::current_dir() { + Ok(cwd) => { + let len = this.write_path_to_wide_str(&cwd, buf, size)?.1; + return Ok(u32::try_from(len).unwrap()) + } + Err(e) => this.set_last_error_from_io_error(e)?, + } + Ok(0) + } + fn chdir(&mut self, path_op: OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> { let this = self.eval_context_mut(); + let target_os = &this.tcx.sess.target.target.target_os; + assert!(target_os == "linux" || target_os == "macos", "`getcwd` is only available for the UNIX target family"); this.check_no_isolation("chdir")?; @@ -324,6 +353,27 @@ fn chdir(&mut self, path_op: OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> { } } + #[allow(non_snake_case)] + fn SetCurrentDirectoryW ( + &mut self, + path_op: OpTy<'tcx, Tag> // LPCTSTR + ) -> InterpResult<'tcx, i32> { // Returns BOOL(i32 in Windows) + let this = self.eval_context_mut(); + this.assert_target_os("windows", "SetCurrentDirectoryW"); + + this.check_no_isolation("SetCurrentDirectoryW")?; + + let path = this.read_path_from_wide_str(this.read_scalar(path_op)?.not_undef()?)?; + + match env::set_current_dir(path) { + Ok(()) => Ok(1), + Err(e) => { + this.set_last_error_from_io_error(e)?; + Ok(0) + } + } + } + /// Updates the `environ` static. /// The first time it gets called, also initializes `extra.environ`. fn update_environ(&mut self) -> InterpResult<'tcx> { diff --git a/src/shims/foreign_items/windows.rs b/src/shims/foreign_items/windows.rs index 7f3f1d0c2d2..ecd1432df0c 100644 --- a/src/shims/foreign_items/windows.rs +++ b/src/shims/foreign_items/windows.rs @@ -40,6 +40,16 @@ fn emulate_foreign_item_by_name( this.write_scalar(Scalar::from_i32(result), dest)?; } + "GetCurrentDirectoryW" => { + let result = this.GetCurrentDirectoryW(args[0], args[1])?; + this.write_scalar(Scalar::from_u32(result), dest)?; + } + + "SetCurrentDirectoryW" => { + let result = this.SetCurrentDirectoryW(args[0])?; + this.write_scalar(Scalar::from_i32(result), dest)?; + } + // File related shims "GetStdHandle" => { let which = this.read_scalar(args[0])?.to_i32()?; diff --git a/src/shims/os_str.rs b/src/shims/os_str.rs index 316d5b0014a..c513be4a61f 100644 --- a/src/shims/os_str.rs +++ b/src/shims/os_str.rs @@ -177,36 +177,24 @@ fn read_path_from_c_str<'a>(&'a self, scalar: Scalar) -> InterpResult<'tcx, 'mir: 'a, { let this = self.eval_context_ref(); - let os_str = this.read_os_str_from_c_str(scalar)?; + let os_str: &'a OsStr = this.read_os_str_from_c_str(scalar)?; - #[cfg(windows)] - return Ok(if this.tcx.sess.target.target.target_os == "windows" { - // Windows-on-Windows, all fine. - Cow::Borrowed(Path::new(os_str)) - } else { - // Unix target, Windows host. Need to convert target '/' to host '\'. - let converted = os_str - .encode_wide() - .map(|wchar| if wchar == '/' as u16 { '\\' as u16 } else { wchar }) - .collect::>(); - Cow::Owned(PathBuf::from(OsString::from_wide(&converted))) - }); - #[cfg(unix)] - return Ok(if this.tcx.sess.target.target.target_os == "windows" { - // Windows target, Unix host. Need to convert target '\' to host '/'. - let converted = os_str - .as_bytes() - .iter() - .map(|&wchar| if wchar == '/' as u8 { '\\' as u8 } else { wchar }) - .collect::>(); - Cow::Owned(PathBuf::from(OsString::from_vec(converted))) - } else { - // Unix-on-Unix, all is fine. - Cow::Borrowed(Path::new(os_str)) - }); + Ok(match convert_path_separator(os_str, &this.tcx.sess.target.target.target_os) { + Cow::Borrowed(x) => Cow::Borrowed(Path::new(x)), + Cow::Owned(y) => Cow::Owned(PathBuf::from(y)), + }) } - /// Write a Path to the machine memory, adjusting path separators if needed. + /// Read a null-terminated sequence of `u16`s, and perform path separator conversion if needed. + fn read_path_from_wide_str(&self, scalar: Scalar) -> InterpResult<'tcx, PathBuf> { + let this = self.eval_context_ref(); + let os_str: OsString = this.read_os_str_from_wide_str(scalar)?; + + Ok(PathBuf::from(&convert_path_separator(&os_str, &this.tcx.sess.target.target.target_os))) + } + + /// Write a Path to the machine memory (as a null-terminated sequence of bytes), + /// adjusting path separators if needed. fn write_path_to_c_str( &mut self, path: &Path, @@ -214,35 +202,52 @@ fn write_path_to_c_str( size: u64, ) -> InterpResult<'tcx, (bool, u64)> { let this = self.eval_context_mut(); - - #[cfg(windows)] - let os_str = if this.tcx.sess.target.target.target_os == "windows" { - // Windows-on-Windows, all fine. - Cow::Borrowed(path.as_os_str()) - } else { - // Unix target, Windows host. Need to convert host '\\' to target '/'. - let converted = path - .as_os_str() - .encode_wide() - .map(|wchar| if wchar == '\\' as u16 { '/' as u16 } else { wchar }) - .collect::>(); - Cow::Owned(OsString::from_wide(&converted)) - }; - #[cfg(unix)] - let os_str = if this.tcx.sess.target.target.target_os == "windows" { - // Windows target, Unix host. Need to convert host '/' to target '\'. - let converted = path - .as_os_str() - .as_bytes() - .iter() - .map(|&wchar| if wchar == '/' as u8 { '\\' as u8 } else { wchar }) - .collect::>(); - Cow::Owned(OsString::from_vec(converted)) - } else { - // Unix-on-Unix, all is fine. - Cow::Borrowed(path.as_os_str()) - }; - + let os_str = convert_path_separator(path.as_os_str(), &this.tcx.sess.target.target.target_os); this.write_os_str_to_c_str(&os_str, scalar, size) } + + /// Write a Path to the machine memory (as a null-terminated sequence of `u16`s), + /// adjusting path separators if needed. + fn write_path_to_wide_str( + &mut self, + path: &Path, + scalar: Scalar, + size: u64, + ) -> InterpResult<'tcx, (bool, u64)> { + let this = self.eval_context_mut(); + let os_str = convert_path_separator(path.as_os_str(), &this.tcx.sess.target.target.target_os); + this.write_os_str_to_wide_str(&os_str, scalar, size) + } +} + +/// Perform path separator conversion if needed. +fn convert_path_separator<'a>( + os_str: &'a OsStr, + target_os: &str, +) -> Cow<'a, OsStr> { + #[cfg(windows)] + return if target_os == "windows" { + // Windows-on-Windows, all fine. + Cow::Borrowed(os_str) + } else { + // Unix target, Windows host. Need to convert host '\\' to target '/'. + let converted = os_str + .encode_wide() + .map(|wchar| if wchar == '\\' as u16 { '/' as u16 } else { wchar }) + .collect::>(); + Cow::Owned(OsString::from_wide(&converted)) + }; + #[cfg(unix)] + return if target_os == "windows" { + // Windows target, Unix host. Need to convert host '/' to target '\'. + let converted = os_str + .as_bytes() + .iter() + .map(|&wchar| if wchar == '/' as u8 { '\\' as u8 } else { wchar }) + .collect::>(); + Cow::Owned(OsString::from_vec(converted)) + } else { + // Unix-on-Unix, all is fine. + Cow::Borrowed(os_str) + }; } diff --git a/tests/run-pass/current_dir.rs b/tests/run-pass/current_dir.rs index 5e896659c85..a88f820951c 100644 --- a/tests/run-pass/current_dir.rs +++ b/tests/run-pass/current_dir.rs @@ -1,7 +1,6 @@ -// ignore-windows: TODO the windows hook is not done yet // compile-flags: -Zmiri-disable-isolation use std::env; -use std::path::Path; +use std::io::ErrorKind; fn main() { // Test that `getcwd` is available @@ -11,7 +10,9 @@ fn main() { // keep the current directory equal to `cwd`. let parent = cwd.parent().unwrap_or(&cwd); // Test that `chdir` is available - assert!(env::set_current_dir(&Path::new("..")).is_ok()); + assert!(env::set_current_dir("..").is_ok()); // Test that `..` goes to the parent directory assert_eq!(env::current_dir().unwrap(), parent); + // Test that `chdir` to a non-existing directory returns a proper error + assert_eq!(env::set_current_dir("thisdoesnotexist").unwrap_err().kind(), ErrorKind::NotFound); } From fe9ecb50d1dd1b8facd001e634449a9e8d52ad22 Mon Sep 17 00:00:00 2001 From: JOE1994 Date: Sun, 29 Mar 2020 09:46:13 -0400 Subject: [PATCH 3/7] Follow-up to reviews from RalfJung 1. Fix 'fn convert_path_separator' in src/shims/os_str.rs 2. Fix 'fn set_last_error_from_io_error' in src/helpers.rs 3. Minor comment fix for 'fn SetCurrentDirectoryW' in src/shims/env.rs --- src/helpers.rs | 10 +++++----- src/shims/env.rs | 2 +- src/shims/os_str.rs | 21 +++++++++++++-------- 3 files changed, 19 insertions(+), 14 deletions(-) diff --git a/src/helpers.rs b/src/helpers.rs index 751d77a3897..5407eb12051 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -414,6 +414,7 @@ fn set_last_error_from_io_error(&mut self, e: std::io::Error) -> InterpResult<'t use std::io::ErrorKind::*; let this = self.eval_context_mut(); let target = &this.tcx.sess.target.target; + let target_os = &target.target_os; let last_error = if target.options.target_family == Some("unix".to_owned()) { this.eval_libc(match e.kind() { ConnectionRefused => "ECONNREFUSED", @@ -434,15 +435,14 @@ fn set_last_error_from_io_error(&mut self, e: std::io::Error) -> InterpResult<'t throw_unsup_format!("io error {} cannot be transformed into a raw os error", e) } })? - } else { + } else if target_os == "windows" { // FIXME: we have to finish implementing the Windows equivalent of this. this.eval_windows(match e.kind() { NotFound => "ERROR_FILE_NOT_FOUND", - _ => throw_unsup_format!( - "setting the last OS error from an io::Error is yet unsupported for {}.", - target.target_os - ) + _ => throw_unsup_format!("io error {} cannot be transformed into a raw os error", e) })? + } else { + throw_unsup_format!("setting the last OS error from an io::Error is unsupported for {}.", target_os) }; this.set_last_error(last_error) } diff --git a/src/shims/env.rs b/src/shims/env.rs index 78e7c37d9ce..ce1be90ce69 100644 --- a/src/shims/env.rs +++ b/src/shims/env.rs @@ -357,7 +357,7 @@ fn chdir(&mut self, path_op: OpTy<'tcx, Tag>) -> InterpResult<'tcx, i32> { fn SetCurrentDirectoryW ( &mut self, path_op: OpTy<'tcx, Tag> // LPCTSTR - ) -> InterpResult<'tcx, i32> { // Returns BOOL(i32 in Windows) + ) -> InterpResult<'tcx, i32> { // Returns BOOL (i32 in Windows) let this = self.eval_context_mut(); this.assert_target_os("windows", "SetCurrentDirectoryW"); diff --git a/src/shims/os_str.rs b/src/shims/os_str.rs index c513be4a61f..a182950f006 100644 --- a/src/shims/os_str.rs +++ b/src/shims/os_str.rs @@ -179,7 +179,7 @@ fn read_path_from_c_str<'a>(&'a self, scalar: Scalar) -> InterpResult<'tcx, let this = self.eval_context_ref(); let os_str: &'a OsStr = this.read_os_str_from_c_str(scalar)?; - Ok(match convert_path_separator(os_str, &this.tcx.sess.target.target.target_os) { + Ok(match convert_path_separator(os_str, &this.tcx.sess.target.target.target_os, false) { Cow::Borrowed(x) => Cow::Borrowed(Path::new(x)), Cow::Owned(y) => Cow::Owned(PathBuf::from(y)), }) @@ -190,7 +190,7 @@ fn read_path_from_wide_str(&self, scalar: Scalar) -> InterpResult<'tcx, Pat let this = self.eval_context_ref(); let os_str: OsString = this.read_os_str_from_wide_str(scalar)?; - Ok(PathBuf::from(&convert_path_separator(&os_str, &this.tcx.sess.target.target.target_os))) + Ok(PathBuf::from(&convert_path_separator(&os_str, &this.tcx.sess.target.target.target_os, false))) } /// Write a Path to the machine memory (as a null-terminated sequence of bytes), @@ -202,7 +202,7 @@ fn write_path_to_c_str( size: u64, ) -> InterpResult<'tcx, (bool, u64)> { let this = self.eval_context_mut(); - let os_str = convert_path_separator(path.as_os_str(), &this.tcx.sess.target.target.target_os); + let os_str = convert_path_separator(path.as_os_str(), &this.tcx.sess.target.target.target_os, true); this.write_os_str_to_c_str(&os_str, scalar, size) } @@ -215,35 +215,40 @@ fn write_path_to_wide_str( size: u64, ) -> InterpResult<'tcx, (bool, u64)> { let this = self.eval_context_mut(); - let os_str = convert_path_separator(path.as_os_str(), &this.tcx.sess.target.target.target_os); + let os_str = convert_path_separator(path.as_os_str(), &this.tcx.sess.target.target.target_os, true); this.write_os_str_to_wide_str(&os_str, scalar, size) } } /// Perform path separator conversion if needed. +/// if direction == true, Convert from 'host' to 'target'. +/// if direction == false, Convert from 'target' to 'host'. fn convert_path_separator<'a>( os_str: &'a OsStr, target_os: &str, + direction: bool, ) -> Cow<'a, OsStr> { #[cfg(windows)] return if target_os == "windows" { // Windows-on-Windows, all fine. Cow::Borrowed(os_str) } else { - // Unix target, Windows host. Need to convert host '\\' to target '/'. + // Unix target, Windows host. + let (from, to) = if direction { ('\\', '/') } else { ('/', '\\') }; let converted = os_str .encode_wide() - .map(|wchar| if wchar == '\\' as u16 { '/' as u16 } else { wchar }) + .map(|wchar| if wchar == from as u16 { to as u16 } else { wchar }) .collect::>(); Cow::Owned(OsString::from_wide(&converted)) }; #[cfg(unix)] return if target_os == "windows" { - // Windows target, Unix host. Need to convert host '/' to target '\'. + // Windows target, Unix host. + let (from, to) = if direction { ('/', '\\') } else { ('\\', '/') }; let converted = os_str .as_bytes() .iter() - .map(|&wchar| if wchar == '/' as u8 { '\\' as u8 } else { wchar }) + .map(|&wchar| if wchar == from as u8 { to as u8 } else { wchar }) .collect::>(); Cow::Owned(OsString::from_vec(converted)) } else { From 7e0cc8307e80821cdc76c379805245b256071a89 Mon Sep 17 00:00:00 2001 From: JOE1994 Date: Sun, 29 Mar 2020 10:21:02 -0400 Subject: [PATCH 4/7] fix 'magic boolean' to enum --- src/shims/os_str.rs | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/src/shims/os_str.rs b/src/shims/os_str.rs index a182950f006..59cedc6e9df 100644 --- a/src/shims/os_str.rs +++ b/src/shims/os_str.rs @@ -179,7 +179,7 @@ fn read_path_from_c_str<'a>(&'a self, scalar: Scalar) -> InterpResult<'tcx, let this = self.eval_context_ref(); let os_str: &'a OsStr = this.read_os_str_from_c_str(scalar)?; - Ok(match convert_path_separator(os_str, &this.tcx.sess.target.target.target_os, false) { + Ok(match convert_path_separator(os_str, &this.tcx.sess.target.target.target_os, PathConversionDirection::TargetToHost) { Cow::Borrowed(x) => Cow::Borrowed(Path::new(x)), Cow::Owned(y) => Cow::Owned(PathBuf::from(y)), }) @@ -190,7 +190,7 @@ fn read_path_from_wide_str(&self, scalar: Scalar) -> InterpResult<'tcx, Pat let this = self.eval_context_ref(); let os_str: OsString = this.read_os_str_from_wide_str(scalar)?; - Ok(PathBuf::from(&convert_path_separator(&os_str, &this.tcx.sess.target.target.target_os, false))) + Ok(PathBuf::from(&convert_path_separator(&os_str, &this.tcx.sess.target.target.target_os, PathConversionDirection::TargetToHost))) } /// Write a Path to the machine memory (as a null-terminated sequence of bytes), @@ -202,7 +202,7 @@ fn write_path_to_c_str( size: u64, ) -> InterpResult<'tcx, (bool, u64)> { let this = self.eval_context_mut(); - let os_str = convert_path_separator(path.as_os_str(), &this.tcx.sess.target.target.target_os, true); + let os_str = convert_path_separator(path.as_os_str(), &this.tcx.sess.target.target.target_os, PathConversionDirection::HostToTarget); this.write_os_str_to_c_str(&os_str, scalar, size) } @@ -215,18 +215,21 @@ fn write_path_to_wide_str( size: u64, ) -> InterpResult<'tcx, (bool, u64)> { let this = self.eval_context_mut(); - let os_str = convert_path_separator(path.as_os_str(), &this.tcx.sess.target.target.target_os, true); + let os_str = convert_path_separator(path.as_os_str(), &this.tcx.sess.target.target.target_os, PathConversionDirection::HostToTarget); this.write_os_str_to_wide_str(&os_str, scalar, size) } } +enum PathConversionDirection { + HostToTarget, + TargetToHost, +} + /// Perform path separator conversion if needed. -/// if direction == true, Convert from 'host' to 'target'. -/// if direction == false, Convert from 'target' to 'host'. fn convert_path_separator<'a>( os_str: &'a OsStr, target_os: &str, - direction: bool, + direction: PathConversionDirection, ) -> Cow<'a, OsStr> { #[cfg(windows)] return if target_os == "windows" { @@ -234,7 +237,10 @@ fn convert_path_separator<'a>( Cow::Borrowed(os_str) } else { // Unix target, Windows host. - let (from, to) = if direction { ('\\', '/') } else { ('/', '\\') }; + let (from, to) = match direction { + PathConversionDirection::HostToTarget => ('\\', '/'), + PathConversionDirection::TargetToHost => ('/', '\\'), + }; let converted = os_str .encode_wide() .map(|wchar| if wchar == from as u16 { to as u16 } else { wchar }) @@ -244,7 +250,10 @@ fn convert_path_separator<'a>( #[cfg(unix)] return if target_os == "windows" { // Windows target, Unix host. - let (from, to) = if direction { ('/', '\\') } else { ('\\', '/') }; + let (from, to) = match direction { + PathConversionDirection::HostToTarget => ('/', '\\'), + PathConversionDirection::TargetToHost => ('\\', '/'), + }; let converted = os_str .as_bytes() .iter() From 1667ded0d2bc0044f84f2e56aa77f76986199058 Mon Sep 17 00:00:00 2001 From: JOE1994 Date: Sun, 29 Mar 2020 11:15:53 -0400 Subject: [PATCH 5/7] fix fn GetCurrentDirectoryW + clarify return types of Windows shims --- src/shims/env.rs | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/src/shims/env.rs b/src/shims/env.rs index ce1be90ce69..dfbeabf2a12 100644 --- a/src/shims/env.rs +++ b/src/shims/env.rs @@ -105,10 +105,10 @@ fn getenv(&mut self, name_op: OpTy<'tcx, Tag>) -> InterpResult<'tcx, Scalar #[allow(non_snake_case)] fn GetEnvironmentVariableW( &mut self, - name_op: OpTy<'tcx, Tag>, // LPCWSTR - buf_op: OpTy<'tcx, Tag>, // LPWSTR - size_op: OpTy<'tcx, Tag>, // DWORD - ) -> InterpResult<'tcx, u32> { + name_op: OpTy<'tcx, Tag>, // LPCWSTR + buf_op: OpTy<'tcx, Tag>, // LPWSTR + size_op: OpTy<'tcx, Tag>, // DWORD + ) -> InterpResult<'tcx, u32> { // Returns DWORD (u32 in Windows) let this = self.eval_context_mut(); this.assert_target_os("windows", "GetEnvironmentVariableW"); @@ -125,17 +125,7 @@ fn GetEnvironmentVariableW( let buf_ptr = this.read_scalar(buf_op)?.not_undef()?; // `buf_size` represents the size in characters. let buf_size = u64::from(this.read_scalar(size_op)?.to_u32()?); - let (success, len) = this.write_os_str_to_wide_str(&var, buf_ptr, buf_size)?; - - if success { - // If the function succeeds, the return value is the number of characters stored in the buffer pointed to by lpBuffer, - // not including the terminating null character. - u32::try_from(len).unwrap() - } else { - // If lpBuffer is not large enough to hold the data, the return value is the buffer size, in characters, - // required to hold the string and its terminating null character and the contents of lpBuffer are undefined. - u32::try_from(len).unwrap().checked_add(1).unwrap() - } + HowWasBufferSize(this.write_os_str_to_wide_str(&var, buf_ptr, buf_size)?) } None => { let envvar_not_found = this.eval_windows("ERROR_ENVVAR_NOT_FOUND")?; @@ -326,10 +316,8 @@ fn GetCurrentDirectoryW( // If we cannot get the current directory, we return 0 match env::current_dir() { - Ok(cwd) => { - let len = this.write_path_to_wide_str(&cwd, buf, size)?.1; - return Ok(u32::try_from(len).unwrap()) - } + Ok(cwd) => + return Ok(HowWasBufferSize(this.write_path_to_wide_str(&cwd, buf, size)?)), Err(e) => this.set_last_error_from_io_error(e)?, } Ok(0) @@ -411,3 +399,17 @@ fn update_environ(&mut self) -> InterpResult<'tcx> { Ok(()) } } + +// Local helper function to be used in Windows shims +#[allow(non_snake_case)] +fn HowWasBufferSize((success, len): (bool, u64)) -> u32 { + if success { + // If the function succeeds, the return value is the number of characters stored in the buffer pointed to by lpBuffer, + // not including the terminating null character. + u32::try_from(len).unwrap() + } else { + // If lpBuffer is not large enough to hold the data, the return value is the buffer size, in characters, + // required to hold the string and its terminating null character and the contents of lpBuffer are undefined. + u32::try_from(len.checked_add(1).unwrap()).unwrap() + } +} From 1b0abc5797fa4e3d32a74b80ed653da1a9ab98ef Mon Sep 17 00:00:00 2001 From: JOE1994 Date: Sun, 29 Mar 2020 13:10:23 -0400 Subject: [PATCH 6/7] small refactorings to 'src/shims/os_str.rs' & 'src/shims/env.rs' --- src/shims/env.rs | 17 +++---- src/shims/os_str.rs | 105 ++++++++++++++++++++++---------------------- 2 files changed, 62 insertions(+), 60 deletions(-) diff --git a/src/shims/env.rs b/src/shims/env.rs index dfbeabf2a12..350d76bc9da 100644 --- a/src/shims/env.rs +++ b/src/shims/env.rs @@ -125,7 +125,7 @@ fn GetEnvironmentVariableW( let buf_ptr = this.read_scalar(buf_op)?.not_undef()?; // `buf_size` represents the size in characters. let buf_size = u64::from(this.read_scalar(size_op)?.to_u32()?); - HowWasBufferSize(this.write_os_str_to_wide_str(&var, buf_ptr, buf_size)?) + windows_check_buffer_size(this.write_os_str_to_wide_str(&var, buf_ptr, buf_size)?) } None => { let envvar_not_found = this.eval_windows("ERROR_ENVVAR_NOT_FOUND")?; @@ -317,7 +317,7 @@ fn GetCurrentDirectoryW( // If we cannot get the current directory, we return 0 match env::current_dir() { Ok(cwd) => - return Ok(HowWasBufferSize(this.write_path_to_wide_str(&cwd, buf, size)?)), + return Ok(windows_check_buffer_size(this.write_path_to_wide_str(&cwd, buf, size)?)), Err(e) => this.set_last_error_from_io_error(e)?, } Ok(0) @@ -400,16 +400,17 @@ fn update_environ(&mut self) -> InterpResult<'tcx> { } } -// Local helper function to be used in Windows shims -#[allow(non_snake_case)] -fn HowWasBufferSize((success, len): (bool, u64)) -> u32 { +/// Check whether an operation that writes to a target buffer was successful. +/// Accordingly select return value. +/// Local helper function to be used in Windows shims. +fn windows_check_buffer_size((success, len): (bool, u64)) -> u32 { if success { - // If the function succeeds, the return value is the number of characters stored in the buffer pointed to by lpBuffer, + // If the function succeeds, the return value is the number of characters stored in the target buffer, // not including the terminating null character. u32::try_from(len).unwrap() } else { - // If lpBuffer is not large enough to hold the data, the return value is the buffer size, in characters, - // required to hold the string and its terminating null character and the contents of lpBuffer are undefined. + // If the target buffer was not large enough to hold the data, the return value is the buffer size, in characters, + // required to hold the string and its terminating null character. u32::try_from(len.checked_add(1).unwrap()).unwrap() } } diff --git a/src/shims/os_str.rs b/src/shims/os_str.rs index 59cedc6e9df..74932ef6ca4 100644 --- a/src/shims/os_str.rs +++ b/src/shims/os_str.rs @@ -13,6 +13,53 @@ use crate::*; +/// Represent how path separator conversion should be done. +enum Pathconversion { + HostToTarget, + TargetToHost, +} + +/// Perform path separator conversion if needed. +fn convert_path_separator<'a>( + os_str: &'a OsStr, + target_os: &str, + direction: Pathconversion, +) -> Cow<'a, OsStr> { + #[cfg(windows)] + return if target_os == "windows" { + // Windows-on-Windows, all fine. + Cow::Borrowed(os_str) + } else { + // Unix target, Windows host. + let (from, to) = match direction { + Pathconversion::HostToTarget => ('\\', '/'), + Pathconversion::TargetToHost => ('/', '\\'), + }; + let converted = os_str + .encode_wide() + .map(|wchar| if wchar == from as u16 { to as u16 } else { wchar }) + .collect::>(); + Cow::Owned(OsString::from_wide(&converted)) + }; + #[cfg(unix)] + return if target_os == "windows" { + // Windows target, Unix host. + let (from, to) = match direction { + Pathconversion::HostToTarget => ('/', '\\'), + Pathconversion::TargetToHost => ('\\', '/'), + }; + let converted = os_str + .as_bytes() + .iter() + .map(|&wchar| if wchar == from as u8 { to as u8 } else { wchar }) + .collect::>(); + Cow::Owned(OsString::from_vec(converted)) + } else { + // Unix-on-Unix, all is fine. + Cow::Borrowed(os_str) + }; +} + impl<'mir, 'tcx> EvalContextExt<'mir, 'tcx> for crate::MiriEvalContext<'mir, 'tcx> {} pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx> { /// Helper function to read an OsString from a null-terminated sequence of bytes, which is what @@ -177,9 +224,9 @@ fn read_path_from_c_str<'a>(&'a self, scalar: Scalar) -> InterpResult<'tcx, 'mir: 'a, { let this = self.eval_context_ref(); - let os_str: &'a OsStr = this.read_os_str_from_c_str(scalar)?; + let os_str = this.read_os_str_from_c_str(scalar)?; - Ok(match convert_path_separator(os_str, &this.tcx.sess.target.target.target_os, PathConversionDirection::TargetToHost) { + Ok(match convert_path_separator(os_str, &this.tcx.sess.target.target.target_os, Pathconversion::TargetToHost) { Cow::Borrowed(x) => Cow::Borrowed(Path::new(x)), Cow::Owned(y) => Cow::Owned(PathBuf::from(y)), }) @@ -188,9 +235,9 @@ fn read_path_from_c_str<'a>(&'a self, scalar: Scalar) -> InterpResult<'tcx, /// Read a null-terminated sequence of `u16`s, and perform path separator conversion if needed. fn read_path_from_wide_str(&self, scalar: Scalar) -> InterpResult<'tcx, PathBuf> { let this = self.eval_context_ref(); - let os_str: OsString = this.read_os_str_from_wide_str(scalar)?; + let os_str = this.read_os_str_from_wide_str(scalar)?; - Ok(PathBuf::from(&convert_path_separator(&os_str, &this.tcx.sess.target.target.target_os, PathConversionDirection::TargetToHost))) + Ok(PathBuf::from(&convert_path_separator(&os_str, &this.tcx.sess.target.target.target_os, Pathconversion::TargetToHost))) } /// Write a Path to the machine memory (as a null-terminated sequence of bytes), @@ -202,7 +249,7 @@ fn write_path_to_c_str( size: u64, ) -> InterpResult<'tcx, (bool, u64)> { let this = self.eval_context_mut(); - let os_str = convert_path_separator(path.as_os_str(), &this.tcx.sess.target.target.target_os, PathConversionDirection::HostToTarget); + let os_str = convert_path_separator(path.as_os_str(), &this.tcx.sess.target.target.target_os, Pathconversion::HostToTarget); this.write_os_str_to_c_str(&os_str, scalar, size) } @@ -215,53 +262,7 @@ fn write_path_to_wide_str( size: u64, ) -> InterpResult<'tcx, (bool, u64)> { let this = self.eval_context_mut(); - let os_str = convert_path_separator(path.as_os_str(), &this.tcx.sess.target.target.target_os, PathConversionDirection::HostToTarget); + let os_str = convert_path_separator(path.as_os_str(), &this.tcx.sess.target.target.target_os, Pathconversion::HostToTarget); this.write_os_str_to_wide_str(&os_str, scalar, size) } } - -enum PathConversionDirection { - HostToTarget, - TargetToHost, -} - -/// Perform path separator conversion if needed. -fn convert_path_separator<'a>( - os_str: &'a OsStr, - target_os: &str, - direction: PathConversionDirection, -) -> Cow<'a, OsStr> { - #[cfg(windows)] - return if target_os == "windows" { - // Windows-on-Windows, all fine. - Cow::Borrowed(os_str) - } else { - // Unix target, Windows host. - let (from, to) = match direction { - PathConversionDirection::HostToTarget => ('\\', '/'), - PathConversionDirection::TargetToHost => ('/', '\\'), - }; - let converted = os_str - .encode_wide() - .map(|wchar| if wchar == from as u16 { to as u16 } else { wchar }) - .collect::>(); - Cow::Owned(OsString::from_wide(&converted)) - }; - #[cfg(unix)] - return if target_os == "windows" { - // Windows target, Unix host. - let (from, to) = match direction { - PathConversionDirection::HostToTarget => ('/', '\\'), - PathConversionDirection::TargetToHost => ('\\', '/'), - }; - let converted = os_str - .as_bytes() - .iter() - .map(|&wchar| if wchar == from as u8 { to as u8 } else { wchar }) - .collect::>(); - Cow::Owned(OsString::from_vec(converted)) - } else { - // Unix-on-Unix, all is fine. - Cow::Borrowed(os_str) - }; -} From 9bdb4bbbbf17c2d94eece96ddc6114a3c9104f73 Mon Sep 17 00:00:00 2001 From: JOE1994 Date: Sun, 29 Mar 2020 13:13:42 -0400 Subject: [PATCH 7/7] Move definition of 'fn windows_check_buffer_size' to top of 'src/shims/env.rs' --- src/shims/env.rs | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/shims/env.rs b/src/shims/env.rs index 350d76bc9da..5846f51b904 100644 --- a/src/shims/env.rs +++ b/src/shims/env.rs @@ -10,6 +10,21 @@ use rustc::ty::layout::Size; use rustc_mir::interpret::Pointer; +/// Check whether an operation that writes to a target buffer was successful. +/// Accordingly select return value. +/// Local helper function to be used in Windows shims. +fn windows_check_buffer_size((success, len): (bool, u64)) -> u32 { + if success { + // If the function succeeds, the return value is the number of characters stored in the target buffer, + // not including the terminating null character. + u32::try_from(len).unwrap() + } else { + // If the target buffer was not large enough to hold the data, the return value is the buffer size, in characters, + // required to hold the string and its terminating null character. + u32::try_from(len.checked_add(1).unwrap()).unwrap() + } +} + #[derive(Default)] pub struct EnvVars<'tcx> { /// Stores pointers to the environment variables. These variables must be stored as @@ -399,18 +414,3 @@ fn update_environ(&mut self) -> InterpResult<'tcx> { Ok(()) } } - -/// Check whether an operation that writes to a target buffer was successful. -/// Accordingly select return value. -/// Local helper function to be used in Windows shims. -fn windows_check_buffer_size((success, len): (bool, u64)) -> u32 { - if success { - // If the function succeeds, the return value is the number of characters stored in the target buffer, - // not including the terminating null character. - u32::try_from(len).unwrap() - } else { - // If the target buffer was not large enough to hold the data, the return value is the buffer size, in characters, - // required to hold the string and its terminating null character. - u32::try_from(len.checked_add(1).unwrap()).unwrap() - } -}