Auto merge of #1268 - JOE1994:dir, r=RalfJung

Windows shims for GetCurrentDirectoryW/SetCurrentDirectoryW

Implemented shims for Windows
* [GetCurrentDirectoryW](https://github.com/rust-lang/rust/blob/75208942f6144daac669e8e382029fc33bdce841/src/libstd/sys/windows/os.rs#L242) (`getcwd` for Windows)
* [SetCurrentDirectoryW](https://github.com/rust-lang/rust/blob/75208942f6144daac669e8e382029fc33bdce841/src/libstd/sys/windows/os.rs#L250) (`chdir` for Windows)

Currently passes test :
`./miri run tests/run-pass/current_dir.rs -Zmiri-disable-isolation`

Author: bors

src/helpers.rs

@@ -71,6 +71,13 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             .not_undef()
     }
 
+    /// Helper function to get a `windows` constant as a `Scalar`.
+    fn eval_windows(&mut self, name: &str) -> InterpResult<'tcx, Scalar<Tag>> {
+        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.
@@ -407,6 +414,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         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",
@@ -427,12 +435,14 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                     throw_unsup_format!("io error {} cannot be transformed into a raw os error", e)
                 }
             })?
+        } 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!("io error {} cannot be transformed into a raw os error", e)
+            })?
         } 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
-            )
+            throw_unsup_format!("setting the last OS error from an io::Error is unsupported for {}.", target_os)
         };
         this.set_last_error(last_error)
     }

src/shims/env.rs

@@ -10,6 +10,21 @@ use rustc_data_structures::fx::FxHashMap;
 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
@@ -105,10 +120,10 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
     #[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,21 +140,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 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()
-                }
+                windows_check_buffer_size(this.write_os_str_to_wide_str(&var, buf_ptr, buf_size)?)
             }
             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 +294,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         size_op: OpTy<'tcx, Tag>,
     ) -> InterpResult<'tcx, Scalar<Tag>> {
         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 +315,33 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         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) =>
+                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)
+    }
+
     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 +356,27 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         }
     }
 
+    #[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> {

src/shims/foreign_items/windows.rs

@@ -40,6 +40,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 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()?;

src/shims/os_str.rs

@@ -13,6 +13,53 @@ use rustc::ty::layout::LayoutOf;
 
 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::<Vec<_>>();
+        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::<Vec<_>>();
+        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
@@ -179,70 +226,43 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         let this = self.eval_context_ref();
         let os_str = 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::<Vec<_>>();
-            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::<Vec<_>>();
-            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, Pathconversion::TargetToHost) {
+            Cow::Borrowed(x) => Cow::Borrowed(Path::new(x)),
+            Cow::Owned(y) => Cow::Owned(PathBuf::from(y)),
+        })
+    }
+
+    /// Read a null-terminated sequence of `u16`s, and perform path separator conversion if needed.
+    fn read_path_from_wide_str(&self, scalar: Scalar<Tag>) -> InterpResult<'tcx, PathBuf> {
+        let this = self.eval_context_ref();
+        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, Pathconversion::TargetToHost)))
     }
 
-    /// Write a Path to the machine memory, adjusting path separators if needed.
+    /// 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,
         scalar: Scalar<Tag>,
         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::<Vec<_>>();
-            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::<Vec<_>>();
-            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, Pathconversion::HostToTarget);
         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<Tag>,
+        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, Pathconversion::HostToTarget);
+        this.write_os_str_to_wide_str(&os_str, scalar, size)
+    }
 }

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);
 }