Auto merge of #3591 - RalfJung:win-symlink-trouble, r=RalfJung

do not run symlink tests on Windows hosts

Fixes https://github.com/rust-lang/miri/issues/3587

src/tools/miri/tests/pass-dep/libc/libc-fs-readlink.rs

@@ -0,0 +1,51 @@
+// Symlink tests are separate since they don't in general work on a Windows host.
+//@ignore-host-windows: creating symlinks requires admin permissions on Windows
+//@ignore-target-windows: File handling is not implemented yet
+//@compile-flags: -Zmiri-disable-isolation
+
+use std::ffi::CString;
+use std::io::{Error, ErrorKind};
+use std::os::unix::ffi::OsStrExt;
+
+#[path = "../../utils/mod.rs"]
+mod utils;
+
+fn main() {
+    let bytes = b"Hello, World!\n";
+    let path = utils::prepare_with_content("miri_test_fs_link_target.txt", bytes);
+    let expected_path = path.as_os_str().as_bytes();
+
+    let symlink_path = utils::prepare("miri_test_fs_symlink.txt");
+    std::os::unix::fs::symlink(&path, &symlink_path).unwrap();
+
+    // Test that the expected string gets written to a buffer of proper
+    // length, and that a trailing null byte is not written.
+    let symlink_c_str = CString::new(symlink_path.as_os_str().as_bytes()).unwrap();
+    let symlink_c_ptr = symlink_c_str.as_ptr();
+
+    // Make the buf one byte larger than it needs to be,
+    // and check that the last byte is not overwritten.
+    let mut large_buf = vec![0xFF; expected_path.len() + 1];
+    let res =
+        unsafe { libc::readlink(symlink_c_ptr, large_buf.as_mut_ptr().cast(), large_buf.len()) };
+    // Check that the resolved path was properly written into the buf.
+    assert_eq!(&large_buf[..(large_buf.len() - 1)], expected_path);
+    assert_eq!(large_buf.last(), Some(&0xFF));
+    assert_eq!(res, large_buf.len() as isize - 1);
+
+    // Test that the resolved path is truncated if the provided buffer
+    // is too small.
+    let mut small_buf = [0u8; 2];
+    let res =
+        unsafe { libc::readlink(symlink_c_ptr, small_buf.as_mut_ptr().cast(), small_buf.len()) };
+    assert_eq!(small_buf, &expected_path[..small_buf.len()]);
+    assert_eq!(res, small_buf.len() as isize);
+
+    // Test that we report a proper error for a missing path.
+    let bad_path = CString::new("MIRI_MISSING_FILE_NAME").unwrap();
+    let res = unsafe {
+        libc::readlink(bad_path.as_ptr(), small_buf.as_mut_ptr().cast(), small_buf.len())
+    };
+    assert_eq!(res, -1);
+    assert_eq!(Error::last_os_error().kind(), ErrorKind::NotFound);
+}

src/tools/miri/tests/pass-dep/libc/libc-fs-with-isolation.rs

@@ -1,4 +1,4 @@
-//@ignore-target-windows: no libc on Windows
+//@ignore-target-windows: File handling is not implemented yet
 //@compile-flags: -Zmiri-isolation-error=warn-nobacktrace
 //@normalize-stderr-test: "(stat(x)?)" -> "$$STAT"
 

src/tools/miri/tests/pass-dep/libc/libc-fs.rs

@@ -1,11 +1,11 @@
-//@ignore-target-windows: no libc on Windows
+//@ignore-target-windows: File handling is not implemented yet
 //@compile-flags: -Zmiri-disable-isolation
 
 #![feature(io_error_more)]
 #![feature(io_error_uncategorized)]
 
 use std::ffi::{CStr, CString, OsString};
-use std::fs::{canonicalize, remove_dir_all, remove_file, File};
+use std::fs::{canonicalize, remove_file, File};
 use std::io::{Error, ErrorKind, Write};
 use std::os::unix::ffi::OsStrExt;
 use std::os::unix::io::AsRawFd;
@@ -21,7 +21,6 @@ fn main() {
     test_ftruncate::<libc::off_t>(libc::ftruncate);
     #[cfg(target_os = "linux")]
     test_ftruncate::<libc::off64_t>(libc::ftruncate64);
-    test_readlink();
     test_file_open_unix_allow_two_args();
     test_file_open_unix_needs_three_args();
     test_file_open_unix_extra_third_arg();
@@ -38,33 +37,8 @@ fn main() {
     test_isatty();
 }
 
-/// Prepare: compute filename and make sure the file does not exist.
-fn prepare(filename: &str) -> PathBuf {
-    let path = utils::tmp().join(filename);
-    // Clean the paths for robustness.
-    remove_file(&path).ok();
-    path
-}
-
-/// Prepare directory: compute directory name and make sure it does not exist.
-#[allow(unused)]
-fn prepare_dir(dirname: &str) -> PathBuf {
-    let path = utils::tmp().join(&dirname);
-    // Clean the directory for robustness.
-    remove_dir_all(&path).ok();
-    path
-}
-
-/// Prepare like above, and also write some initial content to the file.
-fn prepare_with_content(filename: &str, content: &[u8]) -> PathBuf {
-    let path = prepare(filename);
-    let mut file = File::create(&path).unwrap();
-    file.write(content).unwrap();
-    path
-}
-
 fn test_file_open_unix_allow_two_args() {
-    let path = prepare_with_content("test_file_open_unix_allow_two_args.txt", &[]);
+    let path = utils::prepare_with_content("test_file_open_unix_allow_two_args.txt", &[]);
 
     let mut name = path.into_os_string();
     name.push("\0");
@@ -73,7 +47,7 @@ fn test_file_open_unix_allow_two_args() {
 }
 
 fn test_file_open_unix_needs_three_args() {
-    let path = prepare_with_content("test_file_open_unix_needs_three_args.txt", &[]);
+    let path = utils::prepare_with_content("test_file_open_unix_needs_three_args.txt", &[]);
 
     let mut name = path.into_os_string();
     name.push("\0");
@@ -82,7 +56,7 @@ fn test_file_open_unix_needs_three_args() {
 }
 
 fn test_file_open_unix_extra_third_arg() {
-    let path = prepare_with_content("test_file_open_unix_extra_third_arg.txt", &[]);
+    let path = utils::prepare_with_content("test_file_open_unix_extra_third_arg.txt", &[]);
 
     let mut name = path.into_os_string();
     name.push("\0");
@@ -106,49 +80,9 @@ fn test_canonicalize_too_long() {
     assert!(canonicalize(too_long).is_err());
 }
 
-fn test_readlink() {
-    let bytes = b"Hello, World!\n";
-    let path = prepare_with_content("miri_test_fs_link_target.txt", bytes);
-    let expected_path = path.as_os_str().as_bytes();
-
-    let symlink_path = prepare("miri_test_fs_symlink.txt");
-    std::os::unix::fs::symlink(&path, &symlink_path).unwrap();
-
-    // Test that the expected string gets written to a buffer of proper
-    // length, and that a trailing null byte is not written.
-    let symlink_c_str = CString::new(symlink_path.as_os_str().as_bytes()).unwrap();
-    let symlink_c_ptr = symlink_c_str.as_ptr();
-
-    // Make the buf one byte larger than it needs to be,
-    // and check that the last byte is not overwritten.
-    let mut large_buf = vec![0xFF; expected_path.len() + 1];
-    let res =
-        unsafe { libc::readlink(symlink_c_ptr, large_buf.as_mut_ptr().cast(), large_buf.len()) };
-    // Check that the resolved path was properly written into the buf.
-    assert_eq!(&large_buf[..(large_buf.len() - 1)], expected_path);
-    assert_eq!(large_buf.last(), Some(&0xFF));
-    assert_eq!(res, large_buf.len() as isize - 1);
-
-    // Test that the resolved path is truncated if the provided buffer
-    // is too small.
-    let mut small_buf = [0u8; 2];
-    let res =
-        unsafe { libc::readlink(symlink_c_ptr, small_buf.as_mut_ptr().cast(), small_buf.len()) };
-    assert_eq!(small_buf, &expected_path[..small_buf.len()]);
-    assert_eq!(res, small_buf.len() as isize);
-
-    // Test that we report a proper error for a missing path.
-    let bad_path = CString::new("MIRI_MISSING_FILE_NAME").unwrap();
-    let res = unsafe {
-        libc::readlink(bad_path.as_ptr(), small_buf.as_mut_ptr().cast(), small_buf.len())
-    };
-    assert_eq!(res, -1);
-    assert_eq!(Error::last_os_error().kind(), ErrorKind::NotFound);
-}
-
 fn test_rename() {
-    let path1 = prepare("miri_test_libc_fs_source.txt");
-    let path2 = prepare("miri_test_libc_fs_rename_destination.txt");
+    let path1 = utils::prepare("miri_test_libc_fs_source.txt");
+    let path2 = utils::prepare("miri_test_libc_fs_rename_destination.txt");
 
     let file = File::create(&path1).unwrap();
     drop(file);
@@ -178,7 +112,7 @@ fn test_ftruncate<T: From<i32>>(
     // https://docs.rs/libc/latest/i686-unknown-linux-gnu/libc/type.off_t.html
 
     let bytes = b"hello";
-    let path = prepare("miri_test_libc_fs_ftruncate.txt");
+    let path = utils::prepare("miri_test_libc_fs_ftruncate.txt");
     let mut file = File::create(&path).unwrap();
     file.write(bytes).unwrap();
     file.sync_all().unwrap();
@@ -209,7 +143,7 @@ fn test_ftruncate<T: From<i32>>(
 fn test_o_tmpfile_flag() {
     use std::fs::{create_dir, OpenOptions};
     use std::os::unix::fs::OpenOptionsExt;
-    let dir_path = prepare_dir("miri_test_fs_dir");
+    let dir_path = utils::prepare_dir("miri_test_fs_dir");
     create_dir(&dir_path).unwrap();
     // test that the `O_TMPFILE` custom flag gracefully errors instead of stopping execution
     assert_eq!(

src/tools/miri/tests/pass-dep/libc/libc-time.rs

@@ -1,4 +1,4 @@
-//@ignore-target-windows: no libc on Windows
+//@ignore-target-windows: no libc time APIs on Windows
 //@compile-flags: -Zmiri-disable-isolation
 use std::ffi::CStr;
 use std::{env, mem, ptr};

src/tools/miri/tests/pass/shims/fs-symlink.rs

@@ -0,0 +1,50 @@
+// Symlink tests are separate since they don't in general work on a Windows host.
+//@ignore-host-windows: creating symlinks requires admin permissions on Windows
+//@ignore-target-windows: File handling is not implemented yet
+//@compile-flags: -Zmiri-disable-isolation
+
+use std::fs::{read_link, remove_file, File};
+use std::io::{Read, Result};
+use std::path::Path;
+
+#[path = "../../utils/mod.rs"]
+mod utils;
+
+fn check_metadata(bytes: &[u8], path: &Path) -> Result<()> {
+    // Test that the file metadata is correct.
+    let metadata = path.metadata()?;
+    // `path` should point to a file.
+    assert!(metadata.is_file());
+    // The size of the file must be equal to the number of written bytes.
+    assert_eq!(bytes.len() as u64, metadata.len());
+    Ok(())
+}
+
+fn main() {
+    let bytes = b"Hello, World!\n";
+    let path = utils::prepare_with_content("miri_test_fs_link_target.txt", bytes);
+    let symlink_path = utils::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();
+    symlink_file.read_to_end(&mut contents).unwrap();
+    assert_eq!(bytes, contents.as_slice());
+
+    // Test that metadata of a symbolic link (i.e., the file it points to) is correct.
+    check_metadata(bytes, &symlink_path).unwrap();
+    // Test that the metadata of a symbolic link is correct when not following it.
+    assert!(symlink_path.symlink_metadata().unwrap().file_type().is_symlink());
+    // Check that we can follow the link.
+    assert_eq!(read_link(&symlink_path).unwrap(), path);
+    // Removing symbolic link should succeed.
+    remove_file(&symlink_path).unwrap();
+
+    // Removing file should succeed.
+    remove_file(&path).unwrap();
+}

src/tools/miri/tests/pass/shims/fs.rs

@@ -1,21 +1,17 @@
 //@ignore-target-windows: File handling is not implemented yet
 //@compile-flags: -Zmiri-disable-isolation
 
-// If this test is failing for you locally, you can try
-// 1. Deleting the files `/tmp/miri_*`
-// 2. Setting `MIRI_TEMP` or `TMPDIR` to a different directory, without the `miri_*` files
-
 #![feature(io_error_more)]
 #![feature(io_error_uncategorized)]
 
 use std::collections::HashMap;
 use std::ffi::OsString;
 use std::fs::{
-    canonicalize, create_dir, read_dir, read_link, remove_dir, remove_dir_all, remove_file, rename,
-    File, OpenOptions,
+    canonicalize, create_dir, read_dir, remove_dir, remove_dir_all, remove_file, rename, File,
+    OpenOptions,
 };
 use std::io::{Error, ErrorKind, IsTerminal, Read, Result, Seek, SeekFrom, Write};
-use std::path::{Path, PathBuf};
+use std::path::Path;
 
 #[path = "../../utils/mod.rs"]
 mod utils;
@@ -29,7 +25,6 @@ fn main() {
     test_metadata();
     test_file_set_len();
     test_file_sync();
-    test_symlink();
     test_errors();
     test_rename();
     test_directory();
@@ -37,30 +32,6 @@ fn main() {
     test_from_raw_os_error();
 }
 
-/// Prepare: compute filename and make sure the file does not exist.
-fn prepare(filename: &str) -> PathBuf {
-    let path = utils::tmp().join(filename);
-    // Clean the paths for robustness.
-    remove_file(&path).ok();
-    path
-}
-
-/// Prepare directory: compute directory name and make sure it does not exist.
-fn prepare_dir(dirname: &str) -> PathBuf {
-    let path = utils::tmp().join(&dirname);
-    // Clean the directory for robustness.
-    remove_dir_all(&path).ok();
-    path
-}
-
-/// Prepare like above, and also write some initial content to the file.
-fn prepare_with_content(filename: &str, content: &[u8]) -> PathBuf {
-    let path = prepare(filename);
-    let mut file = File::create(&path).unwrap();
-    file.write(content).unwrap();
-    path
-}
-
 fn test_path_conversion() {
     let tmp = utils::tmp();
     assert!(tmp.is_absolute(), "{:?} is not absolute", tmp);
@@ -69,7 +40,7 @@ fn test_path_conversion() {
 
 fn test_file() {
     let bytes = b"Hello, World!\n";
-    let path = prepare("miri_test_fs_file.txt");
+    let path = utils::prepare("miri_test_fs_file.txt");
 
     // Test creating, writing and closing a file (closing is tested when `file` is dropped).
     let mut file = File::create(&path).unwrap();
@@ -96,7 +67,7 @@ fn test_file() {
 
 fn test_file_clone() {
     let bytes = b"Hello, World!\n";
-    let path = prepare_with_content("miri_test_fs_file_clone.txt", bytes);
+    let path = utils::prepare_with_content("miri_test_fs_file_clone.txt", bytes);
 
     // Cloning a file should be successful.
     let file = File::open(&path).unwrap();
@@ -111,7 +82,7 @@ fn test_file_clone() {
 }
 
 fn test_file_create_new() {
-    let path = prepare("miri_test_fs_file_create_new.txt");
+    let path = utils::prepare("miri_test_fs_file_create_new.txt");
 
     // Creating a new file that doesn't yet exist should succeed.
     OpenOptions::new().write(true).create_new(true).open(&path).unwrap();
@@ -129,7 +100,7 @@ fn test_file_create_new() {
 
 fn test_seek() {
     let bytes = b"Hello, entire World!\n";
-    let path = prepare_with_content("miri_test_fs_seek.txt", bytes);
+    let path = utils::prepare_with_content("miri_test_fs_seek.txt", bytes);
 
     let mut file = File::open(&path).unwrap();
     let mut contents = Vec::new();
@@ -168,7 +139,7 @@ fn check_metadata(bytes: &[u8], path: &Path) -> Result<()> {
 
 fn test_metadata() {
     let bytes = b"Hello, meta-World!\n";
-    let path = prepare_with_content("miri_test_fs_metadata.txt", bytes);
+    let path = utils::prepare_with_content("miri_test_fs_metadata.txt", bytes);
 
     // Test that metadata of an absolute path is correct.
     check_metadata(bytes, &path).unwrap();
@@ -182,7 +153,7 @@ fn test_metadata() {
 
 fn test_file_set_len() {
     let bytes = b"Hello, World!\n";
-    let path = prepare_with_content("miri_test_fs_set_len.txt", bytes);
+    let path = utils::prepare_with_content("miri_test_fs_set_len.txt", bytes);
 
     // Test extending the file
     let mut file = OpenOptions::new().read(true).write(true).open(&path).unwrap();
@@ -208,7 +179,7 @@ fn test_file_set_len() {
 
 fn test_file_sync() {
     let bytes = b"Hello, World!\n";
-    let path = prepare_with_content("miri_test_fs_sync.txt", bytes);
+    let path = utils::prepare_with_content("miri_test_fs_sync.txt", bytes);
 
     // Test that we can call sync_data and sync_all (can't readily test effects of this operation)
     let file = OpenOptions::new().write(true).open(&path).unwrap();
@@ -223,38 +194,9 @@ fn test_file_sync() {
     remove_file(&path).unwrap();
 }
 
-fn test_symlink() {
-    let bytes = b"Hello, World!\n";
-    let path = prepare_with_content("miri_test_fs_link_target.txt", bytes);
-    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();
-    symlink_file.read_to_end(&mut contents).unwrap();
-    assert_eq!(bytes, contents.as_slice());
-
-    // Test that metadata of a symbolic link (i.e., the file it points to) is correct.
-    check_metadata(bytes, &symlink_path).unwrap();
-    // Test that the metadata of a symbolic link is correct when not following it.
-    assert!(symlink_path.symlink_metadata().unwrap().file_type().is_symlink());
-    // Check that we can follow the link.
-    assert_eq!(read_link(&symlink_path).unwrap(), path);
-    // Removing symbolic link should succeed.
-    remove_file(&symlink_path).unwrap();
-
-    // Removing file should succeed.
-    remove_file(&path).unwrap();
-}
-
 fn test_errors() {
     let bytes = b"Hello, World!\n";
-    let path = prepare("miri_test_fs_errors.txt");
+    let path = utils::prepare("miri_test_fs_errors.txt");
 
     // The following tests also check that the `__errno_location()` shim is working properly.
     // Opening a non-existing file should fail with a "not found" error.
@@ -269,8 +211,8 @@ fn test_errors() {
 
 fn test_rename() {
     // Renaming a file should succeed.
-    let path1 = prepare("miri_test_fs_rename_source.txt");
-    let path2 = prepare("miri_test_fs_rename_destination.txt");
+    let path1 = utils::prepare("miri_test_fs_rename_source.txt");
+    let path2 = utils::prepare("miri_test_fs_rename_destination.txt");
 
     let file = File::create(&path1).unwrap();
     drop(file);
@@ -289,7 +231,7 @@ fn test_rename() {
 }
 
 fn test_canonicalize() {
-    let dir_path = prepare_dir("miri_test_fs_dir");
+    let dir_path = utils::prepare_dir("miri_test_fs_dir");
     create_dir(&dir_path).unwrap();
     let path = dir_path.join("test_file");
     drop(File::create(&path).unwrap());
@@ -301,7 +243,7 @@ fn test_canonicalize() {
 }
 
 fn test_directory() {
-    let dir_path = prepare_dir("miri_test_fs_dir");
+    let dir_path = utils::prepare_dir("miri_test_fs_dir");
     // Creating a directory should succeed.
     create_dir(&dir_path).unwrap();
     // Test that the metadata of a directory is correct.

src/tools/miri/tests/utils/fs.rs

@@ -1,4 +1,5 @@
 use std::ffi::OsString;
+use std::fs;
 use std::path::PathBuf;
 
 use super::miri_extern;
@@ -27,3 +28,26 @@ pub fn tmp() -> PathBuf {
     // These are host paths. We need to convert them to the target.
     host_to_target_path(path)
 }
+
+/// Prepare: compute filename and make sure the file does not exist.
+pub fn prepare(filename: &str) -> PathBuf {
+    let path = tmp().join(filename);
+    // Clean the paths for robustness.
+    fs::remove_file(&path).ok();
+    path
+}
+
+/// Prepare like above, and also write some initial content to the file.
+pub fn prepare_with_content(filename: &str, content: &[u8]) -> PathBuf {
+    let path = prepare(filename);
+    fs::write(&path, content).unwrap();
+    path
+}
+
+/// Prepare directory: compute directory name and make sure it does not exist.
+pub fn prepare_dir(dirname: &str) -> PathBuf {
+    let path = tmp().join(&dirname);
+    // Clean the directory for robustness.
+    fs::remove_dir_all(&path).ok();
+    path
+}