Auto merge of #1251 - RalfJung:win-cross, r=RalfJung

cross-test on a Windows host

.appveyor.yml

@@ -2,7 +2,6 @@ environment:
     global:
         PROJECT_NAME: miri
     matrix:
-        - TARGET: x86_64-pc-windows-msvc
         - TARGET: i686-pc-windows-msvc
 
 # branches to build
@@ -43,18 +42,36 @@ build_script:
     # Build and install miri
     - cargo build --release --all-features --all-targets --locked
     - cargo install --all-features --force --path . --locked --offline
-    # Get ourselves a MIR-full libstd, and use it henceforth
-    - cargo miri setup
-    - set MIRI_SYSROOT=%USERPROFILE%\AppData\Local\rust-lang\miri\cache\HOST
 
 test_script:
     - set RUST_TEST_NOCAPTURE=1
     - set RUST_BACKTRACE=1
-    # Test miri
+    # Test host miri: 32bit Windows
+    - cargo miri setup
+    - set MIRI_SYSROOT=%USERPROFILE%\AppData\Local\rust-lang\miri\cache\HOST
+    - cargo test --release --all-features --locked
+    - cd test-cargo-miri
+    - '"C:\msys64\mingw64\bin\python3.exe" run-test.py'
+    - cd ..
+    - ps: $env:MIRI_SYSROOT = ""
+    # Test foreign miri: 64bit Linux
+    - cargo miri setup --target x86_64-unknown-linux-gnu
+    - set MIRI_SYSROOT=%USERPROFILE%\AppData\Local\rust-lang\miri\cache
+    - set MIRI_TEST_TARGET=x86_64-unknown-linux-gnu
+    - cargo test --release --all-features --locked
+    - cd test-cargo-miri
+    - '"C:\msys64\mingw64\bin\python3.exe" run-test.py'
+    - cd ..
+    - ps: $env:MIRI_SYSROOT = ""
+    # Test foreign miri: 64bit macOS
+    - cargo miri setup --target x86_64-apple-darwin
+    - set MIRI_SYSROOT=%USERPROFILE%\AppData\Local\rust-lang\miri\cache
+    - set MIRI_TEST_TARGET=x86_64-apple-darwin
     - cargo test --release --all-features --locked
-    # Test cargo integration
     - cd test-cargo-miri
     - '"C:\msys64\mingw64\bin\python3.exe" run-test.py'
+    - cd ..
+    - ps: $env:MIRI_SYSROOT = ""
 
 after_test:
     # Don't cache "master" toolchain, it's a waste

src/helpers.rs

@@ -1,6 +1,13 @@
 use std::ffi::{OsStr, OsString};
+use std::path::{Path, PathBuf};
 use std::{iter, mem};
 use std::convert::TryFrom;
+use std::borrow::Cow;
+
+#[cfg(unix)]
+use std::os::unix::ffi::{OsStrExt, OsStringExt};
+#[cfg(windows)]
+use std::os::windows::ffi::{OsStrExt, OsStringExt};
 
 use rustc::mir;
 use rustc::ty::{
@@ -477,7 +484,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
     {
         #[cfg(unix)]
         fn bytes_to_os_str<'tcx, 'a>(bytes: &'a [u8]) -> InterpResult<'tcx, &'a OsStr> {
-            Ok(std::os::unix::ffi::OsStrExt::from_bytes(bytes))
+            Ok(OsStr::from_bytes(bytes))
         }
         #[cfg(not(unix))]
         fn bytes_to_os_str<'tcx, 'a>(bytes: &'a [u8]) -> InterpResult<'tcx, &'a OsStr> {
@@ -500,7 +507,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
     {
         #[cfg(windows)]
         pub fn u16vec_to_osstring<'tcx, 'a>(u16_vec: Vec<u16>) -> InterpResult<'tcx, OsString> {
-            Ok(std::os::windows::ffi::OsStringExt::from_wide(&u16_vec[..]))
+            Ok(OsString::from_wide(&u16_vec[..]))
         }
         #[cfg(not(windows))]
         pub fn u16vec_to_osstring<'tcx, 'a>(u16_vec: Vec<u16>) -> InterpResult<'tcx, OsString> {
@@ -513,7 +520,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         u16vec_to_osstring(u16_vec)
     }
 
-
     /// Helper function to write an OsStr as a null-terminated sequence of bytes, which is what
     /// the Unix APIs usually handle. This function returns `Ok((false, length))` without trying
     /// to write if `size` is not large enough to fit the contents of `os_string` plus a null
@@ -527,7 +533,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
     ) -> InterpResult<'tcx, (bool, u64)> {
         #[cfg(unix)]
         fn os_str_to_bytes<'tcx, 'a>(os_str: &'a OsStr) -> InterpResult<'tcx, &'a [u8]> {
-            Ok(std::os::unix::ffi::OsStrExt::as_bytes(os_str))
+            Ok(os_str.as_bytes())
         }
         #[cfg(not(unix))]
         fn os_str_to_bytes<'tcx, 'a>(os_str: &'a OsStr) -> InterpResult<'tcx, &'a [u8]> {
@@ -566,7 +572,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
     ) -> InterpResult<'tcx, (bool, u64)> {
         #[cfg(windows)]
         fn os_str_to_u16vec<'tcx>(os_str: &OsStr) -> InterpResult<'tcx, Vec<u16>> {
-            Ok(std::os::windows::ffi::OsStrExt::encode_wide(os_str).collect())
+            Ok(os_str.encode_wide().collect())
         }
         #[cfg(not(windows))]
         fn os_str_to_u16vec<'tcx>(os_str: &OsStr) -> InterpResult<'tcx, Vec<u16>> {
@@ -592,7 +598,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         // Store the UTF-16 string.
         let char_size = Size::from_bytes(2);
         for (idx, c) in u16_vec.into_iter().chain(iter::once(0x0000)).enumerate() {
-            let place = this.mplace_field(mplace, idx as u64)?; 
+            let place = this.mplace_field(mplace, u64::try_from(idx).unwrap())?; 
             this.write_scalar(Scalar::from_uint(c, char_size), place.into())?;
         }
         Ok((true, string_length))
@@ -613,6 +619,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         }
     }
 
+    /// Allocate enough memory to store the given `OsStr` as a null-terminated sequence of bytes.
     fn alloc_os_str_as_c_str(
         &mut self,
         os_str: &OsStr,
@@ -627,6 +634,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         arg_place.ptr.assert_ptr()
     }
 
+    /// Allocate enough memory to store the given `OsStr` as a null-terminated sequence of `u16`.
     fn alloc_os_str_as_wide_str(
         &mut self,
         os_str: &OsStr,
@@ -640,6 +648,82 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         assert!(self.write_os_str_to_wide_str(os_str, arg_place, size).unwrap().0);
         arg_place.ptr.assert_ptr()
     }
+
+    /// Read a null-terminated sequence of bytes, and perform path separator conversion if needed.
+    fn read_path_from_c_str<'a>(&'a self, scalar: Scalar<Tag>) -> InterpResult<'tcx, Cow<'a, Path>>
+    where
+        'tcx: 'a,
+        'mir: 'a,
+    {
+        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))
+        });
+    }
+
+    /// Write a Path to the machine memory, 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())
+        };
+
+        this.write_os_str_to_c_str(&os_str, scalar, size)
+    }
 }
 
 pub fn immty_from_int_checked<'tcx>(

src/shims/env.rs

@@ -134,7 +134,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         // If we cannot get the current directory, we return null
         match env::current_dir() {
             Ok(cwd) => {
-                if this.write_os_str_to_c_str(&OsString::from(cwd), buf, size)?.0 {
+                if this.write_path_to_c_str(&cwd, buf, size)?.0 {
                     return Ok(buf);
                 }
                 let erange = this.eval_libc("ERANGE")?;
@@ -150,7 +150,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
         this.check_no_isolation("chdir")?;
 
-        let path = this.read_os_str_from_c_str(this.read_scalar(path_op)?.not_undef()?)?;
+        let path = this.read_path_from_c_str(this.read_scalar(path_op)?.not_undef()?)?;
 
         match env::set_current_dir(path) {
             Ok(()) => Ok(0),

src/shims/fs.rs

@@ -2,7 +2,7 @@ use std::collections::BTreeMap;
 use std::convert::{TryFrom, TryInto};
 use std::fs::{read_dir, remove_dir, remove_file, rename, DirBuilder, File, FileType, OpenOptions, ReadDir};
 use std::io::{Read, Seek, SeekFrom, Write};
-use std::path::PathBuf;
+use std::path::Path;
 use std::time::SystemTime;
 
 use rustc_data_structures::fx::FxHashMap;
@@ -79,9 +79,9 @@ trait EvalContextExtPrivate<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, '
         let this = self.eval_context_mut();
 
         let path_scalar = this.read_scalar(path_op)?.not_undef()?;
-        let path: PathBuf = this.read_os_str_from_c_str(path_scalar)?.into();
+        let path = this.read_path_from_c_str(path_scalar)?.into_owned();
 
-        let metadata = match FileMetadata::from_path(this, path, follow_symlink)? {
+        let metadata = match FileMetadata::from_path(this, &path, follow_symlink)? {
             Some(metadata) => metadata,
             None => return Ok(-1),
         };
@@ -303,7 +303,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             throw_unsup_format!("unsupported flags {:#x}", flag & !mirror);
         }
 
-        let path = this.read_os_str_from_c_str(this.read_scalar(path_op)?.not_undef()?)?;
+        let path = this.read_path_from_c_str(this.read_scalar(path_op)?.not_undef()?)?;
 
         let fd = options.open(&path).map(|file| {
             let fh = &mut this.machine.file_handler;
@@ -524,10 +524,9 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
         this.check_no_isolation("unlink")?;
 
-        let path = this.read_os_str_from_c_str(this.read_scalar(path_op)?.not_undef()?)?;
+        let path = this.read_path_from_c_str(this.read_scalar(path_op)?.not_undef()?)?;
 
         let result = remove_file(path).map(|_| 0);
-
         this.try_unwrap_io_result(result)
     }
 
@@ -537,12 +536,12 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         linkpath_op: OpTy<'tcx, Tag>
     ) -> InterpResult<'tcx, i32> {
         #[cfg(target_family = "unix")]
-        fn create_link(src: PathBuf, dst: PathBuf) -> std::io::Result<()> {
+        fn create_link(src: &Path, dst: &Path) -> std::io::Result<()> {
             std::os::unix::fs::symlink(src, dst)
         }
 
         #[cfg(target_family = "windows")]
-        fn create_link(src: PathBuf, dst: PathBuf) -> std::io::Result<()> {
+        fn create_link(src: &Path, dst: &Path) -> std::io::Result<()> {
             use std::os::windows::fs;
             if src.is_dir() {
                 fs::symlink_dir(src, dst)
@@ -555,10 +554,11 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
         this.check_no_isolation("symlink")?;
 
-        let target = this.read_os_str_from_c_str(this.read_scalar(target_op)?.not_undef()?)?.into();
-        let linkpath = this.read_os_str_from_c_str(this.read_scalar(linkpath_op)?.not_undef()?)?.into();
+        let target = this.read_path_from_c_str(this.read_scalar(target_op)?.not_undef()?)?;
+        let linkpath = this.read_path_from_c_str(this.read_scalar(linkpath_op)?.not_undef()?)?;
 
-        this.try_unwrap_io_result(create_link(target, linkpath).map(|_| 0))
+        let result = create_link(&target, &linkpath).map(|_| 0);
+        this.try_unwrap_io_result(result)
     }
 
     fn macos_stat(
@@ -644,7 +644,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             this.ref_to_mplace(statxbuf_imm)?
         };
 
-        let path: PathBuf = this.read_os_str_from_c_str(pathname_scalar)?.into();
+        let path = this.read_path_from_c_str(pathname_scalar)?.into_owned();
         // `flags` should be a `c_int` but the `syscall` function provides an `isize`.
         let flags: i32 =
             this.read_scalar(flags_op)?.to_machine_isize(&*this.tcx)?.try_into().map_err(|e| {
@@ -691,7 +691,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
         let metadata = if path.as_os_str().is_empty() && empty_path_flag {
             FileMetadata::from_fd(this, dirfd)?
         } else {
-            FileMetadata::from_path(this, path, follow_symlink)?
+            FileMetadata::from_path(this, &path, follow_symlink)?
         };
         let metadata = match metadata {
             Some(metadata) => metadata,
@@ -785,8 +785,8 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             return Ok(-1);
         }
 
-        let oldpath = this.read_os_str_from_c_str(oldpath_scalar)?;
-        let newpath = this.read_os_str_from_c_str(newpath_scalar)?;
+        let oldpath = this.read_path_from_c_str(oldpath_scalar)?;
+        let newpath = this.read_path_from_c_str(newpath_scalar)?;
 
         let result = rename(oldpath, newpath).map(|_| 0);
 
@@ -808,7 +808,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
             this.read_scalar(mode_op)?.to_u32()?
         };
 
-        let path = this.read_os_str_from_c_str(this.read_scalar(path_op)?.not_undef()?)?;
+        let path = this.read_path_from_c_str(this.read_scalar(path_op)?.not_undef()?)?;
 
         let mut builder = DirBuilder::new();
 
@@ -833,7 +833,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
         this.check_no_isolation("rmdir")?;
 
-        let path = this.read_os_str_from_c_str(this.read_scalar(path_op)?.not_undef()?)?;
+        let path = this.read_path_from_c_str(this.read_scalar(path_op)?.not_undef()?)?;
 
         let result = remove_dir(path).map(|_| 0i32);
 
@@ -845,7 +845,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
 
         this.check_no_isolation("opendir")?;
 
-        let name = this.read_os_str_from_c_str(this.read_scalar(name_op)?.not_undef()?)?;
+        let name = this.read_path_from_c_str(this.read_scalar(name_op)?.not_undef()?)?;
 
         let result = read_dir(name);
 
@@ -899,7 +899,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 let entry_place = this.deref_operand(entry_op)?;
                 let name_place = this.mplace_field(entry_place, 4)?;
 
-                let file_name = dir_entry.file_name();
+                let file_name = dir_entry.file_name(); // not a Path as there are no separators!
                 let (name_fits, _) = this.write_os_str_to_c_str(
                     &file_name,
                     name_place.ptr,
@@ -987,7 +987,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx
                 let entry_place = this.deref_operand(entry_op)?;
                 let name_place = this.mplace_field(entry_place, 5)?;
 
-                let file_name = dir_entry.file_name();
+                let file_name = dir_entry.file_name(); // not a Path as there are no separators!
                 let (name_fits, file_name_len) = this.write_os_str_to_c_str(
                     &file_name,
                     name_place.ptr,
@@ -1082,7 +1082,7 @@ struct FileMetadata {
 impl FileMetadata {
     fn from_path<'tcx, 'mir>(
         ecx: &mut MiriEvalContext<'mir, 'tcx>,
-        path: PathBuf,
+        path: &Path,
         follow_symlink: bool
     ) -> InterpResult<'tcx, Option<FileMetadata>> {
         let metadata = if follow_symlink {

tests/compiletest.rs

@@ -110,6 +110,8 @@ fn get_target() -> String {
 fn test_runner(_tests: &[&()]) {
     // Add a test env var to do environment communication tests.
     std::env::set_var("MIRI_ENV_VAR_TEST", "0");
+    // Let the tests know where to store temp files (they might run for a different target, which can make this hard to find).
+    std::env::set_var("MIRI_TEMP", std::env::temp_dir());
 
     let target = get_target();
     miri_pass("tests/run-pass", &target);