From e565624465538ab3770d3856178cbdb5db413318 Mon Sep 17 00:00:00 2001
From: Ralf Jung <post@ralfj.de>
Date: Sun, 30 Jul 2023 17:08:06 +0200
Subject: [PATCH] refactor tests/utils a bit, and move some FS functions there

---
 .../reserved/cell-protected-write.rs          |  2 +-
 .../reserved/int-protected-write.rs           |  2 +-
 .../miri/tests/pass-dep/shims/libc-fs.rs      | 31 +++------------
 .../miri/tests/pass-dep/shims/libc-misc.rs    | 35 ++++-------------
 src/tools/miri/tests/pass/shims/fs.rs         | 39 ++++---------------
 .../tree_borrows/cell-alternate-writes.rs     |  2 +-
 .../pass/tree_borrows/end-of-protector.rs     |  2 +-
 .../tests/pass/tree_borrows/formatting.rs     |  2 +-
 .../pass/tree_borrows/reborrow-is-read.rs     |  2 +-
 .../miri/tests/pass/tree_borrows/reserved.rs  |  7 ++--
 .../miri/tests/pass/tree_borrows/unique.rs    |  2 +-
 .../tests/pass/tree_borrows/vec_unique.rs     |  2 +-
 src/tools/miri/tests/utils/fs.rs              | 29 ++++++++++++++
 src/tools/miri/tests/utils/macros.rs          | 18 ++++-----
 src/tools/miri/tests/utils/miri_extern.rs     |  2 -
 src/tools/miri/tests/utils/mod.rs             | 12 +++++-
 16 files changed, 77 insertions(+), 112 deletions(-)
 create mode 100644 src/tools/miri/tests/utils/fs.rs

diff --git a/src/tools/miri/tests/fail/tree_borrows/reserved/cell-protected-write.rs b/src/tools/miri/tests/fail/tree_borrows/reserved/cell-protected-write.rs
index 872efe3ad593..465679b72c34 100644
--- a/src/tools/miri/tests/fail/tree_borrows/reserved/cell-protected-write.rs
+++ b/src/tools/miri/tests/fail/tree_borrows/reserved/cell-protected-write.rs
@@ -3,8 +3,8 @@
 // Check how a Reserved with interior mutability
 // responds to a Foreign Write under a Protector
 #[path = "../../../utils/mod.rs"]
+#[macro_use]
 mod utils;
-use utils::macros::*;
 
 use std::cell::UnsafeCell;
 
diff --git a/src/tools/miri/tests/fail/tree_borrows/reserved/int-protected-write.rs b/src/tools/miri/tests/fail/tree_borrows/reserved/int-protected-write.rs
index 3a1205a84f72..1e6e2eebd26c 100644
--- a/src/tools/miri/tests/fail/tree_borrows/reserved/int-protected-write.rs
+++ b/src/tools/miri/tests/fail/tree_borrows/reserved/int-protected-write.rs
@@ -1,8 +1,8 @@
 //@compile-flags: -Zmiri-tree-borrows -Zmiri-tag-gc=0
 
 #[path = "../../../utils/mod.rs"]
+#[macro_use]
 mod utils;
-use utils::macros::*;
 
 // Check how a Reserved without interior mutability responds to a Foreign
 // Write when under a protector
diff --git a/src/tools/miri/tests/pass-dep/shims/libc-fs.rs b/src/tools/miri/tests/pass-dep/shims/libc-fs.rs
index fbdf27688a9c..767a4fdbede3 100644
--- a/src/tools/miri/tests/pass-dep/shims/libc-fs.rs
+++ b/src/tools/miri/tests/pass-dep/shims/libc-fs.rs
@@ -5,12 +5,15 @@
 #![feature(io_error_uncategorized)]
 
 use std::convert::TryInto;
-use std::ffi::{c_char, CStr, CString};
+use std::ffi::CString;
 use std::fs::{canonicalize, remove_dir_all, remove_file, File};
 use std::io::{Error, ErrorKind, Write};
 use std::os::unix::ffi::OsStrExt;
 use std::path::PathBuf;
 
+#[path = "../../utils/mod.rs"]
+mod utils;
+
 fn main() {
     test_dup_stdout_stderr();
     test_canonicalize_too_long();
@@ -22,31 +25,9 @@ fn main() {
     test_o_tmpfile_flag();
 }
 
-fn tmp() -> PathBuf {
-    let path = std::env::var("MIRI_TEMP")
-        .unwrap_or_else(|_| std::env::temp_dir().into_os_string().into_string().unwrap());
-    // These are host paths. We need to convert them to the target.
-    let path = CString::new(path).unwrap();
-    let mut out = Vec::with_capacity(1024);
-
-    unsafe {
-        extern "Rust" {
-            fn miri_host_to_target_path(
-                path: *const c_char,
-                out: *mut c_char,
-                out_size: usize,
-            ) -> usize;
-        }
-        let ret = miri_host_to_target_path(path.as_ptr(), out.as_mut_ptr(), out.capacity());
-        assert_eq!(ret, 0);
-        let out = CStr::from_ptr(out.as_ptr()).to_str().unwrap();
-        PathBuf::from(out)
-    }
-}
-
 /// Prepare: compute filename and make sure the file does not exist.
 fn prepare(filename: &str) -> PathBuf {
-    let path = tmp().join(filename);
+    let path = utils::tmp().join(filename);
     // Clean the paths for robustness.
     remove_file(&path).ok();
     path
@@ -55,7 +36,7 @@ fn prepare(filename: &str) -> PathBuf {
 /// Prepare directory: compute directory name and make sure it does not exist.
 #[allow(unused)]
 fn prepare_dir(dirname: &str) -> PathBuf {
-    let path = tmp().join(&dirname);
+    let path = utils::tmp().join(&dirname);
     // Clean the directory for robustness.
     remove_dir_all(&path).ok();
     path
diff --git a/src/tools/miri/tests/pass-dep/shims/libc-misc.rs b/src/tools/miri/tests/pass-dep/shims/libc-misc.rs
index 68504cb1c794..ebfeb863abfd 100644
--- a/src/tools/miri/tests/pass-dep/shims/libc-misc.rs
+++ b/src/tools/miri/tests/pass-dep/shims/libc-misc.rs
@@ -6,29 +6,8 @@ use std::fs::{remove_file, File};
 use std::os::unix::io::AsRawFd;
 use std::path::PathBuf;
 
-fn tmp() -> PathBuf {
-    use std::ffi::{c_char, CStr, CString};
-
-    let path = std::env::var("MIRI_TEMP")
-        .unwrap_or_else(|_| std::env::temp_dir().into_os_string().into_string().unwrap());
-    // These are host paths. We need to convert them to the target.
-    let path = CString::new(path).unwrap();
-    let mut out = Vec::with_capacity(1024);
-
-    unsafe {
-        extern "Rust" {
-            fn miri_host_to_target_path(
-                path: *const c_char,
-                out: *mut c_char,
-                out_size: usize,
-            ) -> usize;
-        }
-        let ret = miri_host_to_target_path(path.as_ptr(), out.as_mut_ptr(), out.capacity());
-        assert_eq!(ret, 0);
-        let out = CStr::from_ptr(out.as_ptr()).to_str().unwrap();
-        PathBuf::from(out)
-    }
-}
+#[path = "../../utils/mod.rs"]
+mod utils;
 
 /// Test allocating variant of `realpath`.
 fn test_posix_realpath_alloc() {
@@ -38,7 +17,7 @@ fn test_posix_realpath_alloc() {
     use std::os::unix::ffi::OsStringExt;
 
     let buf;
-    let path = tmp().join("miri_test_libc_posix_realpath_alloc");
+    let path = utils::tmp().join("miri_test_libc_posix_realpath_alloc");
     let c_path = CString::new(path.as_os_str().as_bytes()).expect("CString::new failed");
 
     // Cleanup before test.
@@ -63,7 +42,7 @@ fn test_posix_realpath_noalloc() {
     use std::ffi::{CStr, CString};
     use std::os::unix::ffi::OsStrExt;
 
-    let path = tmp().join("miri_test_libc_posix_realpath_noalloc");
+    let path = utils::tmp().join("miri_test_libc_posix_realpath_noalloc");
     let c_path = CString::new(path.as_os_str().as_bytes()).expect("CString::new failed");
 
     let mut v = vec![0; libc::PATH_MAX as usize];
@@ -103,7 +82,7 @@ fn test_posix_realpath_errors() {
 fn test_posix_fadvise() {
     use std::io::Write;
 
-    let path = tmp().join("miri_test_libc_posix_fadvise.txt");
+    let path = utils::tmp().join("miri_test_libc_posix_fadvise.txt");
     // Cleanup before test
     remove_file(&path).ok();
 
@@ -130,7 +109,7 @@ fn test_posix_fadvise() {
 fn test_sync_file_range() {
     use std::io::Write;
 
-    let path = tmp().join("miri_test_libc_sync_file_range.txt");
+    let path = utils::tmp().join("miri_test_libc_sync_file_range.txt");
     // Cleanup before test.
     remove_file(&path).ok();
 
@@ -243,7 +222,7 @@ fn test_isatty() {
         libc::isatty(libc::STDERR_FILENO);
 
         // But when we open a file, it is definitely not a TTY.
-        let path = tmp().join("notatty.txt");
+        let path = utils::tmp().join("notatty.txt");
         // Cleanup before test.
         remove_file(&path).ok();
         let file = File::create(&path).unwrap();
diff --git a/src/tools/miri/tests/pass/shims/fs.rs b/src/tools/miri/tests/pass/shims/fs.rs
index af245aa89aa3..6ba39c1f563f 100644
--- a/src/tools/miri/tests/pass/shims/fs.rs
+++ b/src/tools/miri/tests/pass/shims/fs.rs
@@ -5,7 +5,7 @@
 #![feature(io_error_uncategorized)]
 
 use std::collections::HashMap;
-use std::ffi::{c_char, OsString};
+use std::ffi::OsString;
 use std::fs::{
     canonicalize, create_dir, read_dir, read_link, remove_dir, remove_dir_all, remove_file, rename,
     File, OpenOptions,
@@ -13,6 +13,9 @@ use std::fs::{
 use std::io::{Error, ErrorKind, IsTerminal, Read, Result, Seek, SeekFrom, Write};
 use std::path::{Path, PathBuf};
 
+#[path = "../../utils/mod.rs"]
+mod utils;
+
 fn main() {
     test_path_conversion();
     test_file();
@@ -30,37 +33,9 @@ fn main() {
     test_from_raw_os_error();
 }
 
-fn host_to_target_path(path: String) -> PathBuf {
-    use std::ffi::{CStr, CString};
-
-    let path = CString::new(path).unwrap();
-    let mut out = Vec::with_capacity(1024);
-
-    unsafe {
-        extern "Rust" {
-            fn miri_host_to_target_path(
-                path: *const c_char,
-                out: *mut c_char,
-                out_size: usize,
-            ) -> usize;
-        }
-        let ret = miri_host_to_target_path(path.as_ptr(), out.as_mut_ptr(), out.capacity());
-        assert_eq!(ret, 0);
-        let out = CStr::from_ptr(out.as_ptr()).to_str().unwrap();
-        PathBuf::from(out)
-    }
-}
-
-fn tmp() -> PathBuf {
-    let path = std::env::var("MIRI_TEMP")
-        .unwrap_or_else(|_| std::env::temp_dir().into_os_string().into_string().unwrap());
-    // 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.
 fn prepare(filename: &str) -> PathBuf {
-    let path = tmp().join(filename);
+    let path = utils::tmp().join(filename);
     // Clean the paths for robustness.
     remove_file(&path).ok();
     path
@@ -68,7 +43,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 path = tmp().join(&dirname);
+    let path = utils::tmp().join(&dirname);
     // Clean the directory for robustness.
     remove_dir_all(&path).ok();
     path
@@ -83,7 +58,7 @@ fn prepare_with_content(filename: &str, content: &[u8]) -> PathBuf {
 }
 
 fn test_path_conversion() {
-    let tmp = tmp();
+    let tmp = utils::tmp();
     assert!(tmp.is_absolute(), "{:?} is not absolute", tmp);
     assert!(tmp.is_dir(), "{:?} is not a directory", tmp);
 }
diff --git a/src/tools/miri/tests/pass/tree_borrows/cell-alternate-writes.rs b/src/tools/miri/tests/pass/tree_borrows/cell-alternate-writes.rs
index 1bd94c6df67c..398b542ed4c2 100644
--- a/src/tools/miri/tests/pass/tree_borrows/cell-alternate-writes.rs
+++ b/src/tools/miri/tests/pass/tree_borrows/cell-alternate-writes.rs
@@ -1,7 +1,7 @@
 //@compile-flags: -Zmiri-tree-borrows -Zmiri-tag-gc=0
 #[path = "../../utils/mod.rs"]
+#[macro_use]
 mod utils;
-use utils::macros::*;
 
 use std::cell::UnsafeCell;
 
diff --git a/src/tools/miri/tests/pass/tree_borrows/end-of-protector.rs b/src/tools/miri/tests/pass/tree_borrows/end-of-protector.rs
index 76bbc73e662d..fecc3360434d 100644
--- a/src/tools/miri/tests/pass/tree_borrows/end-of-protector.rs
+++ b/src/tools/miri/tests/pass/tree_borrows/end-of-protector.rs
@@ -3,8 +3,8 @@
 // Check that a protector goes back to normal behavior when the function
 // returns.
 #[path = "../../utils/mod.rs"]
+#[macro_use]
 mod utils;
-use utils::macros::*;
 
 fn main() {
     unsafe {
diff --git a/src/tools/miri/tests/pass/tree_borrows/formatting.rs b/src/tools/miri/tests/pass/tree_borrows/formatting.rs
index 64697cac2619..f22c408ad253 100644
--- a/src/tools/miri/tests/pass/tree_borrows/formatting.rs
+++ b/src/tools/miri/tests/pass/tree_borrows/formatting.rs
@@ -1,8 +1,8 @@
 //@compile-flags: -Zmiri-tree-borrows -Zmiri-tag-gc=0
 
 #[path = "../../utils/mod.rs"]
+#[macro_use]
 mod utils;
-use utils::macros::*;
 
 // Check the formatting of the trees.
 fn main() {
diff --git a/src/tools/miri/tests/pass/tree_borrows/reborrow-is-read.rs b/src/tools/miri/tests/pass/tree_borrows/reborrow-is-read.rs
index e3f3f2d4032f..a38cd6d28941 100644
--- a/src/tools/miri/tests/pass/tree_borrows/reborrow-is-read.rs
+++ b/src/tools/miri/tests/pass/tree_borrows/reborrow-is-read.rs
@@ -1,8 +1,8 @@
 //@compile-flags: -Zmiri-tree-borrows -Zmiri-tag-gc=0
 
 #[path = "../../utils/mod.rs"]
+#[macro_use]
 mod utils;
-use utils::macros::*;
 
 // To check that a reborrow is counted as a Read access, we use a reborrow
 // with no additional Read to Freeze an Active pointer.
diff --git a/src/tools/miri/tests/pass/tree_borrows/reserved.rs b/src/tools/miri/tests/pass/tree_borrows/reserved.rs
index d8a8c27568d4..8d0beab66f40 100644
--- a/src/tools/miri/tests/pass/tree_borrows/reserved.rs
+++ b/src/tools/miri/tests/pass/tree_borrows/reserved.rs
@@ -1,9 +1,8 @@
 //@compile-flags: -Zmiri-tree-borrows -Zmiri-tag-gc=0
 
 #[path = "../../utils/mod.rs"]
+#[macro_use]
 mod utils;
-use utils::macros::*;
-use utils::miri_extern::miri_write_to_stderr;
 
 use std::cell::UnsafeCell;
 
@@ -28,8 +27,8 @@ fn main() {
 }
 
 unsafe fn print(msg: &str) {
-    miri_write_to_stderr(msg.as_bytes());
-    miri_write_to_stderr("\n".as_bytes());
+    utils::miri_write_to_stderr(msg.as_bytes());
+    utils::miri_write_to_stderr("\n".as_bytes());
 }
 
 unsafe fn read_second<T>(x: &mut T, y: *mut u8) {
diff --git a/src/tools/miri/tests/pass/tree_borrows/unique.rs b/src/tools/miri/tests/pass/tree_borrows/unique.rs
index d0c3d133da50..44e2e8136252 100644
--- a/src/tools/miri/tests/pass/tree_borrows/unique.rs
+++ b/src/tools/miri/tests/pass/tree_borrows/unique.rs
@@ -5,8 +5,8 @@
 #![feature(ptr_internals)]
 
 #[path = "../../utils/mod.rs"]
+#[macro_use]
 mod utils;
-use utils::macros::*;
 
 use core::ptr::Unique;
 
diff --git a/src/tools/miri/tests/pass/tree_borrows/vec_unique.rs b/src/tools/miri/tests/pass/tree_borrows/vec_unique.rs
index 3516f8d2ebfb..e5d0a683a723 100644
--- a/src/tools/miri/tests/pass/tree_borrows/vec_unique.rs
+++ b/src/tools/miri/tests/pass/tree_borrows/vec_unique.rs
@@ -5,8 +5,8 @@
 #![feature(vec_into_raw_parts)]
 
 #[path = "../../utils/mod.rs"]
+#[macro_use]
 mod utils;
-use utils::macros::*;
 
 // Check general handling of `Unique`:
 // there is no *explicit* `Unique` being used here, but there is one
diff --git a/src/tools/miri/tests/utils/fs.rs b/src/tools/miri/tests/utils/fs.rs
new file mode 100644
index 000000000000..47904926b48b
--- /dev/null
+++ b/src/tools/miri/tests/utils/fs.rs
@@ -0,0 +1,29 @@
+use std::ffi::OsString;
+use std::path::PathBuf;
+
+use super::miri_extern;
+
+pub fn host_to_target_path(path: OsString) -> PathBuf {
+    use std::ffi::{CStr, CString};
+
+    // Once into_os_str_bytes is stable we can use it here.
+    // (Unstable features would need feature flags in each test...)
+    let path = CString::new(path.into_string().unwrap()).unwrap();
+    let mut out = Vec::with_capacity(1024);
+
+    unsafe {
+        let ret =
+            miri_extern::miri_host_to_target_path(path.as_ptr(), out.as_mut_ptr(), out.capacity());
+        assert_eq!(ret, 0);
+        // Here we panic if it's not UTF-8... but that is hard to avoid with OsStr APIs.
+        let out = CStr::from_ptr(out.as_ptr()).to_str().unwrap();
+        PathBuf::from(out)
+    }
+}
+
+pub fn tmp() -> PathBuf {
+    let path =
+        std::env::var_os("MIRI_TEMP").unwrap_or_else(|| std::env::temp_dir().into_os_string());
+    // These are host paths. We need to convert them to the target.
+    host_to_target_path(path)
+}
diff --git a/src/tools/miri/tests/utils/macros.rs b/src/tools/miri/tests/utils/macros.rs
index 28b40954306f..3f5b9f78ee01 100644
--- a/src/tools/miri/tests/utils/macros.rs
+++ b/src/tools/miri/tests/utils/macros.rs
@@ -9,7 +9,7 @@
 /// The id obtained can be passed directly to `print_state!`.
 macro_rules! alloc_id {
     ($ptr:expr) => {
-        crate::utils::miri_extern::miri_get_alloc_id($ptr as *const u8 as *const ())
+        $crate::utils::miri_get_alloc_id($ptr as *const u8 as *const ())
     };
 }
 
@@ -22,10 +22,10 @@ macro_rules! alloc_id {
 /// tags that have not been given a name. Defaults to `false`.
 macro_rules! print_state {
     ($alloc_id:expr) => {
-        crate::utils::macros::print_state!($alloc_id, false);
+        print_state!($alloc_id, false);
     };
     ($alloc_id:expr, $show:expr) => {
-        crate::utils::miri_extern::miri_print_borrow_state($alloc_id, $show);
+        $crate::utils::miri_print_borrow_state($alloc_id, $show);
     };
 }
 
@@ -42,20 +42,16 @@ macro_rules! print_state {
 /// `stringify!($ptr)` the name of `ptr` in the source code.
 macro_rules! name {
     ($ptr:expr, $name:expr) => {
-        crate::utils::macros::name!($ptr => 0, $name);
+        name!($ptr => 0, $name);
     };
     ($ptr:expr) => {
-        crate::utils::macros::name!($ptr => 0, stringify!($ptr));
+        name!($ptr => 0, stringify!($ptr));
     };
     ($ptr:expr => $nth_parent:expr) => {
-        crate::utils::macros::name!($ptr => $nth_parent, stringify!($ptr));
+        name!($ptr => $nth_parent, stringify!($ptr));
     };
     ($ptr:expr => $nth_parent:expr, $name:expr) => {
         let name = $name.as_bytes();
-        crate::utils::miri_extern::miri_pointer_name($ptr as *const u8 as *const (), $nth_parent, name);
+        $crate::utils::miri_pointer_name($ptr as *const u8 as *const (), $nth_parent, name);
     };
 }
-
-pub(crate) use alloc_id;
-pub(crate) use name;
-pub(crate) use print_state;
diff --git a/src/tools/miri/tests/utils/miri_extern.rs b/src/tools/miri/tests/utils/miri_extern.rs
index 55f3c1cc33e8..c0ef2c506413 100644
--- a/src/tools/miri/tests/utils/miri_extern.rs
+++ b/src/tools/miri/tests/utils/miri_extern.rs
@@ -1,5 +1,3 @@
-#![allow(dead_code)]
-
 #[repr(C)]
 /// Layout of the return value of `miri_resolve_frame`,
 /// with fields in the exact same order.
diff --git a/src/tools/miri/tests/utils/mod.rs b/src/tools/miri/tests/utils/mod.rs
index e1ea77e4df8c..593f82910c6f 100644
--- a/src/tools/miri/tests/utils/mod.rs
+++ b/src/tools/miri/tests/utils/mod.rs
@@ -1,2 +1,10 @@
-pub mod macros;
-pub mod miri_extern;
+#![allow(dead_code)]
+
+#[macro_use]
+mod macros;
+
+mod fs;
+mod miri_extern;
+
+pub use fs::*;
+pub use miri_extern::*;