From da721233d705b6f011747d75e7ada23e948b820e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 22 Mar 2020 20:08:00 +0100 Subject: [PATCH 1/7] cross-test on a Windows host --- .appveyor.yml | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/.appveyor.yml b/.appveyor.yml index 4281fe0180f0..919802d12abe 100644 --- a/.appveyor.yml +++ b/.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,33 @@ 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 - # Test cargo integration - cd test-cargo-miri - '"C:\msys64\mingw64\bin\python3.exe" run-test.py' + - cd .. + # 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 .. + # 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 + - cd test-cargo-miri + - '"C:\msys64\mingw64\bin\python3.exe" run-test.py' + - cd .. after_test: # Don't cache "master" toolchain, it's a waste From 3517ce66abe20263316830f13a164aff78abc5a2 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 22 Mar 2020 20:32:42 +0100 Subject: [PATCH 2/7] need to unset MIRI_SYSROOT before calling 'cargo miri setup' --- .appveyor.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.appveyor.yml b/.appveyor.yml index 919802d12abe..e5c2f4fb7294 100644 --- a/.appveyor.yml +++ b/.appveyor.yml @@ -53,6 +53,7 @@ test_script: - 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 @@ -61,6 +62,7 @@ test_script: - 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 @@ -69,6 +71,7 @@ test_script: - 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 From 4ac91384ff124d5b6258a648df5176909ff1b780 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 23 Mar 2020 19:24:16 +0100 Subject: [PATCH 3/7] route all path reading/writing through central read/write methods --- src/helpers.rs | 24 +++++++++++++++++++++++- src/shims/env.rs | 4 ++-- src/shims/fs.rs | 42 +++++++++++++++++++++--------------------- 3 files changed, 46 insertions(+), 24 deletions(-) diff --git a/src/helpers.rs b/src/helpers.rs index 76c5308cfd35..d8731b537719 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -1,6 +1,8 @@ use std::ffi::{OsStr, OsString}; +use std::path::Path; use std::{iter, mem}; use std::convert::TryFrom; +use std::borrow::Cow; use rustc::mir; use rustc::ty::{ @@ -491,6 +493,16 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx bytes_to_os_str(bytes) } + /// 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) -> InterpResult<'tcx, Cow<'a, Path>> + where + 'tcx: 'a, + 'mir: 'a, + { + let os_str = self.read_os_str_from_c_str(scalar)?; + Ok(Cow::Borrowed(Path::new(os_str))) + } + /// Helper function to read an OsString from a 0x0000-terminated sequence of u16, /// which is what the Windows APIs usually handle. fn read_os_str_from_wide_str<'a>(&'a self, scalar: Scalar) -> InterpResult<'tcx, OsString> @@ -513,7 +525,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 @@ -553,6 +564,17 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx Ok((true, string_length)) } + /// Write a Path to the machine memory, adjusting path separators if needed. + fn write_path_to_c_str( + &mut self, + path: &Path, + scalar: Scalar, + size: u64, + ) -> InterpResult<'tcx, (bool, u64)> { + let os_str = path.as_os_str(); + self.write_os_str_to_c_str(os_str, scalar, size) + } + /// Helper function to write an OsStr as a 0x0000-terminated u16-sequence, which is what /// the Windows 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 diff --git a/src/shims/env.rs b/src/shims/env.rs index 6b18cd25a1b0..192fc0d47abe 100644 --- a/src/shims/env.rs +++ b/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), diff --git a/src/shims/fs.rs b/src/shims/fs.rs index 16d83188dbc2..9778775fca64 100644 --- a/src/shims/fs.rs +++ b/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> { let metadata = if follow_symlink { From c4e29c8646e3836cec79bd17a73ac13a2b90e6e4 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 23 Mar 2020 19:57:40 +0100 Subject: [PATCH 4/7] convert dir separators on path load/store --- src/helpers.rs | 78 ++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 69 insertions(+), 9 deletions(-) diff --git a/src/helpers.rs b/src/helpers.rs index d8731b537719..abfe3253ce7b 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -1,9 +1,14 @@ use std::ffi::{OsStr, OsString}; -use std::path::Path; +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::{ self, @@ -479,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> { @@ -499,8 +504,34 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx 'tcx: 'a, 'mir: 'a, { - let os_str = self.read_os_str_from_c_str(scalar)?; - Ok(Cow::Borrowed(Path::new(os_str))) + 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::>(); + 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)) + }); } /// Helper function to read an OsString from a 0x0000-terminated sequence of u16, @@ -512,7 +543,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx { #[cfg(windows)] pub fn u16vec_to_osstring<'tcx, 'a>(u16_vec: Vec) -> 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) -> InterpResult<'tcx, OsString> { @@ -538,7 +569,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]> { @@ -571,8 +602,37 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx scalar: Scalar, size: u64, ) -> InterpResult<'tcx, (bool, u64)> { - let os_str = path.as_os_str(); - self.write_os_str_to_c_str(os_str, scalar, size) + 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()) + }; + + this.write_os_str_to_c_str(&os_str, scalar, size) } /// Helper function to write an OsStr as a 0x0000-terminated u16-sequence, which is what @@ -588,7 +648,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> { - 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> { From 3ba588db496c99eb6a9280400271af4cb1958a91 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 23 Mar 2020 23:48:12 +0100 Subject: [PATCH 5/7] make fs.rs at least build under Windows --- tests/run-pass/fs.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/run-pass/fs.rs b/tests/run-pass/fs.rs index 0f3512b36605..b6d460f7a981 100644 --- a/tests/run-pass/fs.rs +++ b/tests/run-pass/fs.rs @@ -142,7 +142,10 @@ fn test_symlink() { let symlink_path = prepare("miri_test_fs_symlink.txt"); // Creating a symbolic link should succeed. + #[cfg(unix)] std::os::unix::fs::symlink(&path, &symlink_path).unwrap(); + #[cfg(windows)] + std::os::windows::fs::symlink_file(&path, &symlink_path).unwrap(); // Test that the symbolic link has the same contents as the file. let mut symlink_file = File::open(&symlink_path).unwrap(); let mut contents = Vec::new(); From 8817397828f2e4aedf4251bdc02c6299d1d1654c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 23 Mar 2020 23:42:03 +0100 Subject: [PATCH 6/7] test harness informs tests about suitable temp dir --- tests/compiletest.rs | 2 ++ tests/run-pass/fs.rs | 10 ++++++---- tests/run-pass/libc.rs | 11 ++++++++--- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/tests/compiletest.rs b/tests/compiletest.rs index f43ff8ca349d..d082a2cc484b 100644 --- a/tests/compiletest.rs +++ b/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); diff --git a/tests/run-pass/fs.rs b/tests/run-pass/fs.rs index b6d460f7a981..104ba46c3e45 100644 --- a/tests/run-pass/fs.rs +++ b/tests/run-pass/fs.rs @@ -16,10 +16,13 @@ fn main() { test_directory(); } +fn tmp() -> PathBuf { + std::env::var("MIRI_TEMP").map(PathBuf::from).unwrap_or_else(|_| std::env::temp_dir()) +} + /// Prepare: compute filename and make sure the file does not exist. fn prepare(filename: &str) -> PathBuf { - let tmp = std::env::temp_dir(); - let path = tmp.join(filename); + let path = tmp().join(filename); // Clean the paths for robustness. remove_file(&path).ok(); path @@ -27,8 +30,7 @@ fn prepare(filename: &str) -> PathBuf { /// Prepare directory: compute directory name and make sure it does not exist. fn prepare_dir(dirname: &str) -> PathBuf { - let tmp = std::env::temp_dir(); - let path = tmp.join(&dirname); + let path = tmp().join(&dirname); // Clean the directory for robustness. remove_dir_all(&path).ok(); path diff --git a/tests/run-pass/libc.rs b/tests/run-pass/libc.rs index 5b7b37db327b..064c00e81bb8 100644 --- a/tests/run-pass/libc.rs +++ b/tests/run-pass/libc.rs @@ -2,19 +2,24 @@ // compile-flags: -Zmiri-disable-isolation #![feature(rustc_private)] +#![allow(unused)] // necessary on macos due to conditional compilation + +use std::path::PathBuf; -#[allow(unused)] // necessary on macos due to conditional compilation extern crate libc; +fn tmp() -> PathBuf { + std::env::var("MIRI_TEMP").map(PathBuf::from).unwrap_or_else(|_| std::env::temp_dir()) +} + #[cfg(not(target_os = "macos"))] fn test_posix_fadvise() { use std::convert::TryInto; - use std::env::temp_dir; use std::fs::{File, remove_file}; use std::io::Write; use std::os::unix::io::AsRawFd; - let path = temp_dir().join("miri_test_libc.txt"); + let path = tmp().join("miri_test_libc.txt"); // Cleanup before test remove_file(&path).ok(); From e9e04e56fc6767f63458de1c137b5231cfd77b33 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Tue, 24 Mar 2020 08:38:05 +0100 Subject: [PATCH 7/7] move path methods together, to the bottom of the string helpers --- src/helpers.rs | 156 +++++++++++++++++++++++++------------------------ 1 file changed, 79 insertions(+), 77 deletions(-) diff --git a/src/helpers.rs b/src/helpers.rs index abfe3253ce7b..7ca2b238218a 100644 --- a/src/helpers.rs +++ b/src/helpers.rs @@ -498,42 +498,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx bytes_to_os_str(bytes) } - /// 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) -> 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::>(); - 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)) - }); - } - /// Helper function to read an OsString from a 0x0000-terminated sequence of u16, /// which is what the Windows APIs usually handle. fn read_os_str_from_wide_str<'a>(&'a self, scalar: Scalar) -> InterpResult<'tcx, OsString> @@ -595,46 +559,6 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriEvalContextExt<'mir, 'tcx Ok((true, string_length)) } - /// Write a Path to the machine memory, adjusting path separators if needed. - fn write_path_to_c_str( - &mut self, - path: &Path, - scalar: Scalar, - 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()) - }; - - this.write_os_str_to_c_str(&os_str, scalar, size) - } - /// Helper function to write an OsStr as a 0x0000-terminated u16-sequence, which is what /// the Windows 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 @@ -674,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)) @@ -695,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, @@ -709,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, @@ -722,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) -> 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::>(); + 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)) + }); + } + + /// Write a Path to the machine memory, adjusting path separators if needed. + fn write_path_to_c_str( + &mut self, + path: &Path, + scalar: Scalar, + 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()) + }; + + this.write_os_str_to_c_str(&os_str, scalar, size) + } } pub fn immty_from_int_checked<'tcx>(