From 8ddc72eca4bbb6412639cd52db5d51b9840f3011 Mon Sep 17 00:00:00 2001 From: The Miri Conjob Bot Date: Sun, 30 Jul 2023 05:36:38 +0000 Subject: [PATCH 01/30] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 1d5dd4d3f6351..dde8d86765513 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -d150dbb067e66f351a0b33a54e7d4b464ef51e47 +fb53384c94b87adebceb6048865c9fe305e71b92 From a4bfcbe4d3f2a1080a70c15c1f681b9f341c882e Mon Sep 17 00:00:00 2001 From: The Miri Conjob Bot Date: Sun, 30 Jul 2023 05:45:01 +0000 Subject: [PATCH 02/30] fmt --- src/tools/miri/tests/compiletest.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/tests/compiletest.rs b/src/tools/miri/tests/compiletest.rs index fa17923446fd0..8d82e6b8f9c65 100644 --- a/src/tools/miri/tests/compiletest.rs +++ b/src/tools/miri/tests/compiletest.rs @@ -72,7 +72,7 @@ fn test_config(target: &str, path: &str, mode: Mode, with_dependencies: bool) -> program.args.push(flag); } - let bless = env::var_os("RUSTC_BLESS").is_some_and(|v| v !="0"); + let bless = env::var_os("RUSTC_BLESS").is_some_and(|v| v != "0"); let skip_ui_checks = env::var_os("MIRI_SKIP_UI_CHECKS").is_some(); let output_conflict_handling = match (bless, skip_ui_checks) { From e565624465538ab3770d3856178cbdb5db413318 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 30 Jul 2023 17:08:06 +0200 Subject: [PATCH 03/30] 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 872efe3ad5933..465679b72c343 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 3a1205a84f726..1e6e2eebd26ce 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 fbdf27688a9c8..767a4fdbede38 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 68504cb1c794f..ebfeb863abfd9 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 af245aa89aa36..6ba39c1f563f9 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 1bd94c6df67c8..398b542ed4c24 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 76bbc73e662d3..fecc3360434d2 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 64697cac26196..f22c408ad2538 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 e3f3f2d4032fd..a38cd6d289416 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 d8a8c27568d4d..8d0beab66f406 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(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 d0c3d133da50d..44e2e8136252e 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 3516f8d2ebfbf..e5d0a683a7237 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 0000000000000..47904926b48b9 --- /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 28b40954306f3..3f5b9f78ee01e 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 55f3c1cc33e89..c0ef2c506413d 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 e1ea77e4df8c7..593f82910c6fe 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::*; From 7f2eca6a34da3e03f0ced71858bc76a986d5d886 Mon Sep 17 00:00:00 2001 From: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com> Date: Wed, 17 May 2023 10:50:47 +0200 Subject: [PATCH 04/30] rewrite miri script in Rust --- src/tools/miri/miri | 361 +----------- src/tools/miri/miri-script/Cargo.lock | 459 +++++++++++++++ src/tools/miri/miri-script/Cargo.toml | 23 + src/tools/miri/miri-script/src/arg.rs | 104 ++++ src/tools/miri/miri-script/src/commands.rs | 629 +++++++++++++++++++++ src/tools/miri/miri-script/src/main.rs | 58 ++ 6 files changed, 1277 insertions(+), 357 deletions(-) create mode 100644 src/tools/miri/miri-script/Cargo.lock create mode 100644 src/tools/miri/miri-script/Cargo.toml create mode 100644 src/tools/miri/miri-script/src/arg.rs create mode 100644 src/tools/miri/miri-script/src/commands.rs create mode 100644 src/tools/miri/miri-script/src/main.rs diff --git a/src/tools/miri/miri b/src/tools/miri/miri index bccf6d835ff8f..7412df69bd67c 100755 --- a/src/tools/miri/miri +++ b/src/tools/miri/miri @@ -1,359 +1,6 @@ #!/bin/bash set -e -USAGE=$(cat <<"EOF" - COMMANDS - -./miri install : -Installs the miri driver and cargo-miri. are passed to `cargo -install`. Sets up the rpath such that the installed binary should work in any -working directory. Note that the binaries are placed in the `miri` toolchain -sysroot, to prevent conflicts with other toolchains. - -./miri build : -Just build miri. are passed to `cargo build`. - -./miri check : -Just check miri. are passed to `cargo check`. - -./miri test : -Build miri, set up a sysroot and then run the test suite. are passed -to the final `cargo test` invocation. - -./miri run : -Build miri, set up a sysroot and then run the driver with the given . -(Also respects MIRIFLAGS environment variable.) - -./miri fmt : -Format all sources and tests. are passed to `rustfmt`. - -./miri clippy : -Runs clippy on all sources. are passed to `cargo clippy`. - -./miri cargo : -Runs just `cargo ` with the Miri-specific environment variables. -Mainly meant to be invoked by rust-analyzer. - -./miri many-seeds : -Runs over and over again with different seeds for Miri. The MIRIFLAGS -variable is set to its original value appended with ` -Zmiri-seed=$SEED` for -many different seeds. The MIRI_SEEDS variable controls how many seeds are being -tried; MIRI_SEED_START controls the first seed to try. - -./miri bench : -Runs the benchmarks from bench-cargo-miri in hyperfine. hyperfine needs to be installed. - can explicitly list the benchmarks to run; by default, all of them are run. - -./miri toolchain : -Update and activate the rustup toolchain 'miri' to the commit given in the -`rust-version` file. -`rustup-toolchain-install-master` must be installed for this to work. Any extra -flags are passed to `rustup-toolchain-install-master`. - -./miri rustc-pull : -Pull and merge Miri changes from the rustc repo. Defaults to fetching the latest -rustc commit. The fetched commit is stored in the `rust-version` file, so the -next `./miri toolchain` will install the rustc that just got pulled. - -./miri rustc-push : -Push Miri changes back to the rustc repo. This will pull a copy of the rustc -history into the Miri repo, unless you set the RUSTC_GIT env var to an existing -clone of the rustc repo. - - ENVIRONMENT VARIABLES - -MIRI_SYSROOT: -If already set, the "sysroot setup" step is skipped. - -CARGO_EXTRA_FLAGS: -Pass extra flags to all cargo invocations. (Ignored by `./miri cargo`.) -EOF -) - -## We need to know which command to run and some global constants. -COMMAND="$1" -if [ -z "$COMMAND" ]; then - echo "$USAGE" - exit 1 -fi -shift -# macOS does not have a useful readlink/realpath so we have to use Python instead... -MIRIDIR=$(python3 -c 'import pathlib, sys; print(pathlib.Path(sys.argv[1]).resolve().parent.as_posix())' "$0") -# Used for rustc syncs. -JOSH_FILTER=":rev(75dd959a3a40eb5b4574f8d2e23aa6efbeb33573:prefix=src/tools/miri):/src/tools/miri" -# Needed for `./miri bench`. -TOOLCHAIN=$(cd "$MIRIDIR"; rustup show active-toolchain | head -n 1 | cut -d ' ' -f 1) - -## Early commands, that don't do auto-things and don't want the environment-altering things happening below. -case "$COMMAND" in -toolchain) - cd "$MIRIDIR" - NEW_COMMIT=$(cat rust-version) - # Make sure rustup-toolchain-install-master is installed. - if ! which rustup-toolchain-install-master >/dev/null; then - echo "Please install rustup-toolchain-install-master by running 'cargo install rustup-toolchain-install-master'" - exit 1 - fi - # Check if we already are at that commit. - CUR_COMMIT=$(rustc +miri --version -v 2>/dev/null | grep "^commit-hash: " | cut -d " " -f 2) - if [[ "$CUR_COMMIT" == "$NEW_COMMIT" ]]; then - echo "miri toolchain is already at commit $CUR_COMMIT." - if [[ "$TOOLCHAIN" != "miri" ]]; then - rustup override set miri - fi - exit 0 - fi - # Install and setup new toolchain. - rustup toolchain uninstall miri - rustup-toolchain-install-master -n miri -c cargo -c rust-src -c rustc-dev -c llvm-tools -c rustfmt -c clippy "$@" -- "$NEW_COMMIT" - rustup override set miri - # Cleanup. - cargo clean - # Call 'cargo metadata' on the sources in case that changes the lockfile - # (which fails under some setups when it is done from inside vscode). - cargo metadata --format-version 1 --manifest-path "$(rustc --print sysroot)/lib/rustlib/rustc-src/rust/compiler/rustc/Cargo.toml" >/dev/null - # Done! - exit 0 - ;; -rustc-pull) - cd "$MIRIDIR" - FETCH_COMMIT="$1" - if [ -z "$FETCH_COMMIT" ]; then - FETCH_COMMIT=$(git ls-remote https://github.com/rust-lang/rust/ HEAD | cut -f 1) - fi - # Update rust-version file. As a separate commit, since making it part of - # the merge has confused the heck out of josh in the past. - echo "$FETCH_COMMIT" > rust-version - git commit rust-version -m "Preparing for merge from rustc" || (echo "FAILED to commit rust-version file, something went wrong"; exit 1) - # Fetch given rustc commit and note down which one that was - git fetch http://localhost:8000/rust-lang/rust.git@$FETCH_COMMIT$JOSH_FILTER.git || (echo "FAILED to fetch new commits, something went wrong"; exit 1) - git merge FETCH_HEAD --no-ff -m "Merge from rustc" || (echo "FAILED to merge new commits ($(git rev-parse FETCH_HEAD)), something went wrong"; exit 1) - exit 0 - ;; -rustc-push) - USER="$1" - BRANCH="$2" - if [ -z "$USER" ] || [ -z "$BRANCH" ]; then - echo "Usage: $0 rustc-push " - exit 1 - fi - if [ -n "$RUSTC_GIT" ]; then - # Use an existing fork for the branch updates. - cd "$RUSTC_GIT" - else - # Do this in the local Miri repo. - echo "This will pull a copy of the rust-lang/rust history into this Miri checkout, growing it by about 1GB." - read -r -p "To avoid that, abort now and set the RUSTC_GIT environment variable to an existing rustc checkout. Proceed? [y/N] " - if [[ ! $REPLY =~ ^[Yy]$ ]]; then - exit 1 - fi - cd "$MIRIDIR" - fi - # Prepare the branch. Pushing works much better if we use as base exactly - # the commit that we pulled from last time, so we use the `rust-version` - # file as a good approximation of that. - BASE=$(cat "$MIRIDIR/rust-version") - echo "Preparing $USER/rust (base: $BASE)..." - if git fetch "https://github.com/$USER/rust" "$BRANCH" &>/dev/null; then - echo "The branch '$BRANCH' seems to already exist in 'https://github.com/$USER/rust'. Please delete it and try again." - exit 1 - fi - git fetch https://github.com/rust-lang/rust $BASE - git push https://github.com/$USER/rust $BASE:refs/heads/$BRANCH -f - echo - # Do the actual push. - cd "$MIRIDIR" - echo "Pushing Miri changes..." - git push http://localhost:8000/$USER/rust.git$JOSH_FILTER.git HEAD:$BRANCH - # Do a round-trip check to make sure the push worked as expected. - echo - git fetch http://localhost:8000/$USER/rust.git@$JOSH_FILTER.git $BRANCH &>/dev/null - if [[ $(git rev-parse HEAD) != $(git rev-parse FETCH_HEAD) ]]; then - echo "ERROR: Josh created a non-roundtrip push! Do NOT merge this into rustc!" - exit 1 - else - echo "Confirmed that the push round-trips back to Miri properly. Please create a rustc PR:" - echo " https://github.com/$USER/rust/pull/new/$BRANCH" - exit 0 - fi - ;; -many-seeds) - MIRI_SEED_START=${MIRI_SEED_START:-0} # default to 0 - MIRI_SEEDS=${MIRI_SEEDS:-256} # default to 256 - for SEED in $(seq $MIRI_SEED_START $(( $MIRI_SEED_START + $MIRI_SEEDS - 1 )) ); do - echo "Trying seed: $SEED" - MIRIFLAGS="$MIRIFLAGS -Zlayout-seed=$SEED -Zmiri-seed=$SEED" $@ || { echo "Failing seed: $SEED"; break; } - done - exit 0 - ;; -bench) - # The hyperfine to use - HYPERFINE=${HYPERFINE:-hyperfine -w 1 -m 5 --shell=none} - # Make sure we have an up-to-date Miri installed - "$0" install - # Run the requested benchmarks - if [ -z "${1+exists}" ]; then - BENCHES=( $(ls "$MIRIDIR/bench-cargo-miri" ) ) - else - BENCHES=("$@") - fi - for BENCH in "${BENCHES[@]}"; do - $HYPERFINE "cargo +$TOOLCHAIN miri run --manifest-path $MIRIDIR/bench-cargo-miri/$BENCH/Cargo.toml" - done - exit 0 - ;; -esac - -## Run the auto-things. -if [ -z "$MIRI_AUTO_OPS" ]; then - export MIRI_AUTO_OPS=42 - - # Run this first, so that the toolchain doesn't change after - # other code has run. - if [ -f "$MIRIDIR/.auto-everything" ] || [ -f "$MIRIDIR/.auto-toolchain" ] ; then - $0 toolchain - # Let's make sure to actually use that toolchain, too. - TOOLCHAIN=miri - fi - - if [ -f "$MIRIDIR/.auto-everything" ] || [ -f "$MIRIDIR/.auto-fmt" ] ; then - $0 fmt - fi - - if [ -f "$MIRIDIR/.auto-everything" ] || [ -f "$MIRIDIR/.auto-clippy" ] ; then - $0 clippy -- -D warnings - fi -fi - -## Prepare the environment -# Determine some toolchain properties -TARGET=$(rustc +$TOOLCHAIN --version --verbose | grep "^host:" | cut -d ' ' -f 2) -SYSROOT=$(rustc +$TOOLCHAIN --print sysroot) -LIBDIR=$SYSROOT/lib/rustlib/$TARGET/lib -if ! test -d "$LIBDIR"; then - echo "Something went wrong determining the library dir." - echo "I got $LIBDIR but that does not exist." - echo "Please report a bug at https://github.com/rust-lang/miri/issues." - exit 2 -fi - -# Prepare flags for cargo and rustc. -CARGO="cargo +$TOOLCHAIN" -# Share target dir between `miri` and `cargo-miri`. -if [ -z "$CARGO_TARGET_DIR" ]; then - export CARGO_TARGET_DIR="$MIRIDIR/target" -fi -# We configure dev builds to not be unusably slow. -if [ -z "$CARGO_PROFILE_DEV_OPT_LEVEL" ]; then - export CARGO_PROFILE_DEV_OPT_LEVEL=2 -fi -# Enable rustc-specific lints (ignored without `-Zunstable-options`). -export RUSTFLAGS="-Zunstable-options -Wrustc::internal -Wrust_2018_idioms -Wunused_lifetimes -Wsemicolon_in_expressions_from_macros $RUSTFLAGS" -# We set the rpath so that Miri finds the private rustc libraries it needs. -export RUSTFLAGS="-C link-args=-Wl,-rpath,$LIBDIR $RUSTFLAGS" - -## Helper functions - -# Build a sysroot and set MIRI_SYSROOT to use it. Arguments are passed to `cargo miri setup`. -build_sysroot() { - if ! MIRI_SYSROOT="$($CARGO run $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/cargo-miri/Cargo.toml -- miri setup --print-sysroot "$@")"; then - # Run it again so the user can see the error. - $CARGO run $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/cargo-miri/Cargo.toml -- miri setup "$@" - echo "'cargo miri setup' failed" - exit 1 - fi - export MIRI_SYSROOT -} - -# Prepare and set MIRI_SYSROOT. Respects `MIRI_TEST_TARGET` and takes into account -# locally built vs. distributed rustc. -find_sysroot() { - if [ -n "$MIRI_SYSROOT" ]; then - # Sysroot already set, use that. - return 0 - fi - # We need to build a sysroot. - if [ -n "$MIRI_TEST_TARGET" ]; then - build_sysroot --target "$MIRI_TEST_TARGET" - else - build_sysroot - fi -} - -## Main - -# Run command. -case "$COMMAND" in -install) - # Install binaries to the miri toolchain's sysroot so they do not interact with other toolchains. - $CARGO install $CARGO_EXTRA_FLAGS --path "$MIRIDIR" --force --root "$SYSROOT" "$@" - $CARGO install $CARGO_EXTRA_FLAGS --path "$MIRIDIR"/cargo-miri --force --root "$SYSROOT" "$@" - ;; -check) - # Check, and let caller control flags. - $CARGO check $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml --all-targets "$@" - $CARGO check $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/cargo-miri/Cargo.toml "$@" - ;; -build) - # Build, and let caller control flags. - $CARGO build $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml "$@" - $CARGO build $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/cargo-miri/Cargo.toml "$@" - ;; -test|bless) - # First build and get a sysroot. - $CARGO build $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml - find_sysroot - if [ "$COMMAND" = "bless" ]; then - export RUSTC_BLESS="Gesundheit" - fi - # Then test, and let caller control flags. - # Only in root project as `cargo-miri` has no tests. - $CARGO test $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml "$@" - ;; -run|run-dep) - # Scan for "--target" to overwrite the "MIRI_TEST_TARGET" env var so - # that we set the MIRI_SYSROOT up the right way. - FOUND_TARGET_OPT=0 - for ARG in "$@"; do - if [ "$LAST_ARG" = "--target" ]; then - # Found it! - export MIRI_TEST_TARGET="$ARG" - FOUND_TARGET_OPT=1 - break - fi - LAST_ARG="$ARG" - done - if [ "$FOUND_TARGET_OPT" = "0" ] && [ -n "$MIRI_TEST_TARGET" ]; then - # Make sure Miri actually uses this target. - MIRIFLAGS="$MIRIFLAGS --target $MIRI_TEST_TARGET" - fi - - CARGO="$CARGO --quiet" - # First build and get a sysroot. - $CARGO build $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml - find_sysroot - # Then run the actual command. - - if [ "$COMMAND" = "run-dep" ]; then - exec $CARGO test --test compiletest $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml -- --miri-run-dep-mode $MIRIFLAGS "$@" - else - exec $CARGO run $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml -- $MIRIFLAGS "$@" - fi - ;; -fmt) - find "$MIRIDIR" -not \( -name target -prune \) -name '*.rs' \ - | xargs rustfmt +$TOOLCHAIN --edition=2021 --config-path "$MIRIDIR/rustfmt.toml" "$@" - ;; -clippy) - $CARGO clippy $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/Cargo.toml --all-targets "$@" - $CARGO clippy $CARGO_EXTRA_FLAGS --manifest-path "$MIRIDIR"/cargo-miri/Cargo.toml "$@" - ;; -cargo) - # We carefully kept the working dir intact, so this will run cargo *on the workspace in the - # current working dir*, not on the main Miri workspace. That is exactly what RA needs. - $CARGO "$@" - ;; -*) - echo "Unknown command: $COMMAND" - exit 1 - ;; -esac +# Instead of doing just `cargo run --manifest-path .. $@`, we invoke miri-script binary directly. Invoking `cargo run` goes through +# rustup (that sets it's own environmental variables), which is undesirable. +cargo build --manifest-path "$(dirname "$0")"/miri-script/Cargo.toml +"$(dirname "$0")"/miri-script/target/debug/miri-script $@ \ No newline at end of file diff --git a/src/tools/miri/miri-script/Cargo.lock b/src/tools/miri/miri-script/Cargo.lock new file mode 100644 index 0000000000000..be6eea0ed5d98 --- /dev/null +++ b/src/tools/miri/miri-script/Cargo.lock @@ -0,0 +1,459 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. +version = 3 + +[[package]] +name = "anstream" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0ca84f3628370c59db74ee214b3263d58f9aadd9b4fe7e711fd87dc452b7f163" +dependencies = [ + "anstyle", + "anstyle-parse", + "anstyle-query", + "anstyle-wincon", + "colorchoice", + "is-terminal", + "utf8parse", +] + +[[package]] +name = "anstyle" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "41ed9a86bf92ae6580e0a31281f65a1b1d867c0cc68d5346e2ae128dddfa6a7d" + +[[package]] +name = "anstyle-parse" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e765fd216e48e067936442276d1d57399e37bce53c264d6fefbe298080cb57ee" +dependencies = [ + "utf8parse", +] + +[[package]] +name = "anstyle-query" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5ca11d4be1bab0c8bc8734a9aa7bf4ee8316d462a08c6ac5052f888fef5b494b" +dependencies = [ + "windows-sys", +] + +[[package]] +name = "anstyle-wincon" +version = "1.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "180abfa45703aebe0093f79badacc01b8fd4ea2e35118747e5811127f926e188" +dependencies = [ + "anstyle", + "windows-sys", +] + +[[package]] +name = "anyhow" +version = "1.0.71" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9c7d0618f0e0b7e8ff11427422b64564d5fb0be1940354bfe2e0529b18a9d9b8" + +[[package]] +name = "bitflags" +version = "1.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" + +[[package]] +name = "cc" +version = "1.0.79" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "50d30906286121d95be3d479533b458f87493b30a4b5f79a607db8f5d11aa91f" + +[[package]] +name = "clap" +version = "4.2.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "34d21f9bf1b425d2968943631ec91202fe5e837264063503708b83013f8fc938" +dependencies = [ + "clap_builder", + "clap_derive", + "once_cell", +] + +[[package]] +name = "clap_builder" +version = "4.2.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "914c8c79fb560f238ef6429439a30023c862f7a28e688c58f7203f12b29970bd" +dependencies = [ + "anstream", + "anstyle", + "bitflags", + "clap_lex", + "strsim", +] + +[[package]] +name = "clap_derive" +version = "4.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3f9644cd56d6b87dbe899ef8b053e331c0637664e9e21a33dfcdc36093f5c5c4" +dependencies = [ + "heck", + "proc-macro2", + "quote", + "syn", +] + +[[package]] +name = "clap_lex" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8a2dd5a6fe8c6e3502f568a6353e5273bbb15193ad9a89e457b9970798efbea1" + +[[package]] +name = "colorchoice" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "acbf1af155f9b9ef647e42cdc158db4b64a1b61f743629225fde6f3e0be2a7c7" + +[[package]] +name = "dunce" +version = "1.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "56ce8c6da7551ec6c462cbaf3bfbc75131ebbfa1c944aeaa9dab51ca1c5f0c3b" + +[[package]] +name = "either" +version = "1.8.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7fcaabb2fef8c910e7f4c7ce9f67a1283a1715879a7c230ca9d6d1ae31f16d91" + +[[package]] +name = "errno" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4bcfec3a70f97c962c307b2d2c56e358cf1d00b558d74262b5f929ee8cc7e73a" +dependencies = [ + "errno-dragonfly", + "libc", + "windows-sys", +] + +[[package]] +name = "errno-dragonfly" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "aa68f1b12764fab894d2755d2518754e71b4fd80ecfb822714a1206c2aab39bf" +dependencies = [ + "cc", + "libc", +] + +[[package]] +name = "heck" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "95505c38b4572b2d910cecb0281560f54b440a19336cbbcb27bf6ce6adc6f5a8" + +[[package]] +name = "hermit-abi" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fed44880c466736ef9a5c5b5facefb5ed0785676d0c02d612db14e54f0d84286" + +[[package]] +name = "io-lifetimes" +version = "1.0.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9c66c74d2ae7e79a5a8f7ac924adbe38ee42a859c6539ad869eb51f0b52dc220" +dependencies = [ + "hermit-abi", + "libc", + "windows-sys", +] + +[[package]] +name = "is-terminal" +version = "0.4.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "adcf93614601c8129ddf72e2d5633df827ba6551541c6d8c59520a371475be1f" +dependencies = [ + "hermit-abi", + "io-lifetimes", + "rustix", + "windows-sys", +] + +[[package]] +name = "itertools" +version = "0.10.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b0fd2260e829bddf4cb6ea802289de2f86d6a7a690192fbe91b3f46e0f2c8473" +dependencies = [ + "either", +] + +[[package]] +name = "libc" +version = "0.2.144" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2b00cc1c228a6782d0f076e7b232802e0c5689d41bb5df366f2a6b6621cfdfe1" + +[[package]] +name = "linux-raw-sys" +version = "0.3.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ece97ea872ece730aed82664c424eb4c8291e1ff2480247ccf7409044bc6479f" + +[[package]] +name = "miri-script" +version = "0.1.0" +dependencies = [ + "anyhow", + "clap", + "dunce", + "itertools", + "path_macro", + "rustc_version", + "shell-words", + "walkdir", + "which", + "xshell", +] + +[[package]] +name = "once_cell" +version = "1.17.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b7e5500299e16ebb147ae15a00a942af264cf3688f47923b8fc2cd5858f23ad3" + +[[package]] +name = "path_macro" +version = "1.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a6e819bbd49d5939f682638fa54826bf1650abddcd65d000923de8ad63cc7d15" + +[[package]] +name = "proc-macro2" +version = "1.0.60" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "dec2b086b7a862cf4de201096214fa870344cf922b2b30c167badb3af3195406" +dependencies = [ + "unicode-ident", +] + +[[package]] +name = "quote" +version = "1.0.27" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8f4f29d145265ec1c483c7c654450edde0bfe043d3938d6972630663356d9500" +dependencies = [ + "proc-macro2", +] + +[[package]] +name = "rustc_version" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bfa0f585226d2e68097d4f95d113b15b83a82e819ab25717ec0590d9584ef366" +dependencies = [ + "semver", +] + +[[package]] +name = "rustix" +version = "0.37.19" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "acf8729d8542766f1b2cf77eb034d52f40d375bb8b615d0b147089946e16613d" +dependencies = [ + "bitflags", + "errno", + "io-lifetimes", + "libc", + "linux-raw-sys", + "windows-sys", +] + +[[package]] +name = "same-file" +version = "1.0.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "93fc1dc3aaa9bfed95e02e6eadabb4baf7e3078b0bd1b4d7b6b0b68378900502" +dependencies = [ + "winapi-util", +] + +[[package]] +name = "semver" +version = "1.0.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "bebd363326d05ec3e2f532ab7660680f3b02130d780c299bca73469d521bc0ed" + +[[package]] +name = "shell-words" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "24188a676b6ae68c3b2cb3a01be17fbf7240ce009799bb56d5b1409051e78fde" + +[[package]] +name = "strsim" +version = "0.10.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "73473c0e59e6d5812c5dfe2a064a6444949f089e20eec9a2e5506596494e4623" + +[[package]] +name = "syn" +version = "2.0.15" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a34fcf3e8b60f57e6a14301a2e916d323af98b0ea63c599441eec8558660c822" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + +[[package]] +name = "unicode-ident" +version = "1.0.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e5464a87b239f13a63a501f2701565754bae92d243d4bb7eb12f6d57d2269bf4" + +[[package]] +name = "utf8parse" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "711b9620af191e0cdc7468a8d14e709c3dcdb115b36f838e601583af800a370a" + +[[package]] +name = "walkdir" +version = "2.3.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "36df944cda56c7d8d8b7496af378e6b16de9284591917d307c9b4d313c44e698" +dependencies = [ + "same-file", + "winapi-util", +] + +[[package]] +name = "which" +version = "4.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2441c784c52b289a054b7201fc93253e288f094e2f4be9058343127c4226a269" +dependencies = [ + "either", + "libc", + "once_cell", +] + +[[package]] +name = "winapi" +version = "0.3.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c839a674fcd7a98952e593242ea400abe93992746761e38641405d28b00f419" +dependencies = [ + "winapi-i686-pc-windows-gnu", + "winapi-x86_64-pc-windows-gnu", +] + +[[package]] +name = "winapi-i686-pc-windows-gnu" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" + +[[package]] +name = "winapi-util" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "70ec6ce85bb158151cae5e5c87f95a8e97d2c0c4b001223f33a334e3ce5de178" +dependencies = [ + "winapi", +] + +[[package]] +name = "winapi-x86_64-pc-windows-gnu" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" + +[[package]] +name = "windows-sys" +version = "0.48.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "677d2418bec65e3338edb076e806bc1ec15693c5d0104683f2efe857f61056a9" +dependencies = [ + "windows-targets", +] + +[[package]] +name = "windows-targets" +version = "0.48.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7b1eb6f0cd7c80c79759c929114ef071b87354ce476d9d94271031c0497adfd5" +dependencies = [ + "windows_aarch64_gnullvm", + "windows_aarch64_msvc", + "windows_i686_gnu", + "windows_i686_msvc", + "windows_x86_64_gnu", + "windows_x86_64_gnullvm", + "windows_x86_64_msvc", +] + +[[package]] +name = "windows_aarch64_gnullvm" +version = "0.48.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "91ae572e1b79dba883e0d315474df7305d12f569b400fcf90581b06062f7e1bc" + +[[package]] +name = "windows_aarch64_msvc" +version = "0.48.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b2ef27e0d7bdfcfc7b868b317c1d32c641a6fe4629c171b8928c7b08d98d7cf3" + +[[package]] +name = "windows_i686_gnu" +version = "0.48.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "622a1962a7db830d6fd0a69683c80a18fda201879f0f447f065a3b7467daa241" + +[[package]] +name = "windows_i686_msvc" +version = "0.48.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4542c6e364ce21bf45d69fdd2a8e455fa38d316158cfd43b3ac1c5b1b19f8e00" + +[[package]] +name = "windows_x86_64_gnu" +version = "0.48.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ca2b8a661f7628cbd23440e50b05d705db3686f894fc9580820623656af974b1" + +[[package]] +name = "windows_x86_64_gnullvm" +version = "0.48.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7896dbc1f41e08872e9d5e8f8baa8fdd2677f29468c4e156210174edc7f7b953" + +[[package]] +name = "windows_x86_64_msvc" +version = "0.48.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1a515f5799fe4961cb532f983ce2b23082366b898e52ffbce459c86f67c8378a" + +[[package]] +name = "xshell" +version = "0.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "962c039b3a7b16cf4e9a4248397c6585c07547412e7d6a6e035389a802dcfe90" +dependencies = [ + "xshell-macros", +] + +[[package]] +name = "xshell-macros" +version = "0.2.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1dbabb1cbd15a1d6d12d9ed6b35cc6777d4af87ab3ba155ea37215f20beab80c" diff --git a/src/tools/miri/miri-script/Cargo.toml b/src/tools/miri/miri-script/Cargo.toml new file mode 100644 index 0000000000000..197f6abd99039 --- /dev/null +++ b/src/tools/miri/miri-script/Cargo.toml @@ -0,0 +1,23 @@ +[package] +authors = ["Miri Team"] +description = "Helpers for miri maintenance" +license = "MIT OR Apache-2.0" +name = "miri-script" +repository = "https://github.com/rust-lang/miri" +version = "0.1.0" +default-run = "miri-script" +edition = "2021" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[dependencies] +clap = {version = "4.2", features = ["derive", "env"]} +which = "4.4" +walkdir = "2.3" +itertools = "0.10" +path_macro = "1.0" +shell-words = "1.1" +anyhow = "1.0" +xshell = "0.2" +rustc_version = "0.4" +dunce = "1.0.4" diff --git a/src/tools/miri/miri-script/src/arg.rs b/src/tools/miri/miri-script/src/arg.rs new file mode 100644 index 0000000000000..24a5204e04397 --- /dev/null +++ b/src/tools/miri/miri-script/src/arg.rs @@ -0,0 +1,104 @@ +use clap::{Parser, Subcommand}; +use std::ffi::OsString; + +#[derive(Parser, Clone, Debug)] +#[command(author, about, long_about = None)] +pub struct Cli { + #[command(subcommand)] + pub commands: Subcommands, +} + +#[derive(Subcommand, Clone, Debug)] +pub enum Subcommands { + /// Installs the miri driver and cargo-miri. + /// Sets up the rpath such that the installed binary should work in any + /// working directory. Note that the binaries are placed in the `miri` toolchain + /// sysroot, to prevent conflicts with other toolchains. + Install { + /// Flags that are passed through to `cargo install`. + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + flags: Vec, + }, + /// Just build miri. + Build { + /// Flags that are passed through to `cargo build`. + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + flags: Vec, + }, + /// Just check miri. + Check { + /// Flags that are passed through to `cargo check`. + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + flags: Vec, + }, + /// Build miri, set up a sysroot and then run the test suite. + Test { + #[arg(long, default_value_t = false)] + bless: bool, + /// Flags that are passed through to `cargo test`. + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + flags: Vec, + }, + /// Build miri, set up a sysroot and then run the driver with the given . + /// (Also respects MIRIFLAGS environment variable.) + Run { + #[arg(long, default_value_t = false)] + dep: bool, + /// Flags that are passed through to `miri` + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + flags: Vec, + }, + /// Format all sources and tests. + Fmt { + /// Flags that are passed through to `rustfmt`. + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + flags: Vec, + }, + /// Runs clippy on all sources. + Clippy { + /// Flags that are passed through to `cargo clippy`. + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + flags: Vec, + }, + /// Runs just `cargo ` with the Miri-specific environment variables. + /// Mainly meant to be invoked by rust-analyzer. + Cargo { + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + flags: Vec, + }, + /// Runs over and over again with different seeds for Miri. The MIRIFLAGS + /// variable is set to its original value appended with ` -Zmiri-seed=$SEED` for + /// many different seeds. + ManySeeds { + /// Starting seed. + #[clap(long, env("MIRI_SEED_START"), default_value_t = 0)] + seed_start: u64, + #[clap(long, env("MIRI_SEEDS"), default_value_t = 256)] + /// Amount of seeds to try. + seeds: u64, + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + command: Vec, + }, + /// Runs the benchmarks from bench-cargo-miri in hyperfine. hyperfine needs to be installed. + Bench { + /// List of benchmarks to run. By default all benchmarks are run. + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + benches: Vec, + }, + /// Update and activate the rustup toolchain 'miri' to the commit given in the + /// `rust-version` file. + /// `rustup-toolchain-install-master` must be installed for this to work. Any extra + /// flags are passed to `rustup-toolchain-install-master`. + Toolchain { + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + flags: Vec, + }, + /// Pull and merge Miri changes from the rustc repo. Defaults to fetching the latest + /// rustc commit. The fetched commit is stored in the `rust-version` file, so the + /// next `./miri toolchain` will install the rustc that just got pulled. + RustcPull { commit: Option }, + /// Push Miri changes back to the rustc repo. This will pull a copy of the rustc + /// history into the Miri repo, unless you set the RUSTC_GIT env var to an existing + /// clone of the rustc repo. + RustcPush { github_user: String, branch: String }, +} diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs new file mode 100644 index 0000000000000..121b50678045b --- /dev/null +++ b/src/tools/miri/miri-script/src/commands.rs @@ -0,0 +1,629 @@ +use std::collections::BTreeMap; + +use std::ffi::{OsStr, OsString}; +use std::path::{Path, PathBuf}; + +use anyhow::{anyhow, bail, Context, Result}; +use dunce::canonicalize; +use path_macro::path; +use xshell::{cmd, Shell}; + +use walkdir::WalkDir; + +use crate::arg::Subcommands; + +/// Used for rustc syncs. +const JOSH_FILTER: &str = + ":rev(75dd959a3a40eb5b4574f8d2e23aa6efbeb33573:prefix=src/tools/miri):/src/tools/miri"; + +fn detect_miri_dir() -> std::io::Result { + const MIRI_SCRIPT_ROOT_DIR: &str = env!("CARGO_MANIFEST_DIR"); + Ok(canonicalize(MIRI_SCRIPT_ROOT_DIR)?.parent().unwrap().into()) +} + +/// Queries an active toolchain for `dir` via `rustup`. +fn get_active_toolchain(dir: &Path) -> Result { + let sh = Shell::new()?; + sh.change_dir(dir); + let stdout = cmd!(sh, "rustup show active-toolchain").read()?; + Ok(stdout.split_whitespace().next().context("Could not obtain active Rust toolchain")?.into()) +} + +#[derive(Clone, Debug)] +pub(super) struct MiriRunner<'a> { + /// miri_dir is the root of the miri repository checkout we are working in. + miri_dir: PathBuf, + /// active_toolchain is passed as `+toolchain` argument to cargo/rustc invocations. + active_toolchain: String, + cargo_extra_flags: Vec, + command: &'a super::Subcommands, + /// Environment variables passed to child processes. + env: BTreeMap, + /// Additional variables used by environment-altering commands. + /// These should be accessed by corresponding methods (e.g. `sysroot()`) and not directly. + sysroot: Option, +} + +fn shell_with_parent_env() -> Result { + let sh = Shell::new()?; + // xshell does not propagate parent's env variables by default. + for (k, v) in std::env::vars_os() { + sh.set_var(k, v); + } + Ok(sh) +} + +impl MiriRunner<'_> { + pub(super) fn exec(command: &super::Subcommands) -> Result<()> { + Self::exec_inner(command, true) + } + fn exec_inner(command: &super::Subcommands, run_auto_things: bool) -> Result<()> { + let miri_dir = detect_miri_dir()?; + let active_toolchain = get_active_toolchain(&miri_dir)?; + let config = command.get_config(&miri_dir); + // CARGO_EXTRA_FLAGS do not have to be a valid UTF-8, but that's what shell_words' expects. + let cargo_extra_flags = std::env::var("CARGO_EXTRA_FLAGS").unwrap_or_default(); + let cargo_extra_flags = shell_words::split(&cargo_extra_flags)?; + let env = BTreeMap::new(); + + let mut runner = MiriRunner { + miri_dir, + active_toolchain, + command, + env, + cargo_extra_flags, + sysroot: None, + }; + if let Some(config) = config { + // Run the auto-things. + if run_auto_things { + if config.toolchain { + // Run this first, so that the toolchain doesn't change after + // other code has run. + let command = Subcommands::Toolchain { flags: vec![] }; + Self::exec_inner(&command, false)?; + // Let's make sure to actually use that toolchain, too. + runner.active_toolchain = "miri".to_owned(); + } + if config.fmt { + let command = Subcommands::Fmt { flags: vec![] }; + Self::exec_inner(&command, false)?; + } + if config.clippy { + let command = Subcommands::Clippy { + flags: ["--", "-D", "warnings"].into_iter().map(OsString::from).collect(), + }; + Self::exec_inner(&command, false)?; + } + } + + // Prepare the environment + // Determine some toolchain properties + let libdir = runner.libdir()?; + if !libdir.exists() { + println!("Something went wrong determining the library dir."); + println!("I got {} but that does not exist.", libdir.display()); + println!("Please report a bug at https://github.com/rust-lang/miri/issues."); + std::process::exit(2); + } + // Share target dir between `miri` and `cargo-miri`. + let target_dir = std::env::var_os("CARGO_TARGET_DIR") + .filter(|val| !val.is_empty()) + .unwrap_or_else(|| { + let target_dir = path!(runner.miri_dir / "target"); + target_dir.into() + }); + runner.set_env("CARGO_TARGET_DIR", target_dir); + + // We configure dev builds to not be unusably slow. + let devel_opt_level = std::env::var_os("CARGO_PROFILE_DEV_OPT_LEVEL") + .filter(|val| !val.is_empty()) + .unwrap_or_else(|| "2".into()); + runner.set_env("CARGO_PROFILE_DEV_OPT_LEVEL", devel_opt_level); + let rustflags = { + let env = std::env::var_os("RUSTFLAGS"); + let mut flags_with_warnings = OsString::from( + "-Zunstable-options -Wrustc::internal -Wrust_2018_idioms -Wunused_lifetimes -Wsemicolon_in_expressions_from_macros ", + ); + if let Some(value) = env { + flags_with_warnings.push(value); + } + // We set the rpath so that Miri finds the private rustc libraries it needs. + let mut flags_with_compiler_settings = OsString::from("-C link-args=-Wl,-rpath,"); + flags_with_compiler_settings.push(&libdir); + flags_with_compiler_settings.push(flags_with_warnings); + flags_with_compiler_settings + }; + runner.set_env("RUSTFLAGS", rustflags); + } + runner.execute() + } + fn execute(&mut self) -> Result<()> { + // Run command. + match self.command { + Subcommands::Install { flags } => self.install(flags), + Subcommands::Build { flags } => self.build(flags), + Subcommands::Check { flags } => self.check(flags), + Subcommands::Test { bless, flags } => self.test(*bless, flags), + Subcommands::Run { dep, flags } => self.run(*dep, flags), + Subcommands::Fmt { flags } => self.fmt(flags), + Subcommands::Clippy { flags } => self.clippy(flags), + Subcommands::Cargo { flags } => self.cargo(flags), + Subcommands::ManySeeds { command, seed_start, seeds } => + self.many_seeds(command, *seed_start, *seeds), + Subcommands::Bench { benches } => self.bench(benches), + Subcommands::Toolchain { flags } => self.toolchain(flags), + Subcommands::RustcPull { commit } => self.rustc_pull(commit.clone()), + Subcommands::RustcPush { github_user, branch } => self.rustc_push(github_user, branch), + } + } + + fn set_env( + &mut self, + key: impl Into, + value: impl Into, + ) -> Option { + self.env.insert(key.into(), value.into()) + } + + /// Prepare and set MIRI_SYSROOT. Respects `MIRI_TEST_TARGET` and takes into account + /// locally built vs. distributed rustc. + fn find_miri_sysroot(&mut self) -> Result<()> { + let current_sysroot = std::env::var_os("MIRI_SYSROOT").unwrap_or_default(); + + if !current_sysroot.is_empty() { + // Sysroot already set, use that. + let current_value = self.set_env("MIRI_SYSROOT", ¤t_sysroot); + assert!(current_value.is_none() || current_value.unwrap() == current_sysroot); + return Ok(()); + } + // We need to build a sysroot. + let target = std::env::var_os("MIRI_TEST_TARGET").filter(|target| !target.is_empty()); + let sysroot = self.build_miri_sysroot(target.as_deref())?; + self.set_env("MIRI_SYSROOT", sysroot); + Ok(()) + } + + /// Build a sysroot and set MIRI_SYSROOT to use it. Arguments are passed to `cargo miri setup`. + fn build_miri_sysroot(&self, target: Option<&OsStr>) -> Result { + let manifest_path = path!(self.miri_dir / "cargo-miri" / "Cargo.toml"); + let Self { active_toolchain, cargo_extra_flags, .. } = &self; + let target_prefix: Option<&OsStr> = target.map(|_| "--target".as_ref()); + let sh = self.shell()?; + let output = cmd!(sh, "cargo +{active_toolchain} --quiet run {cargo_extra_flags...} --manifest-path {manifest_path} -- miri setup --print-sysroot {target_prefix...} {target...}").read(); + if output.is_err() { + // Run it again (without `--print-sysroot`) so the user can see the error. + cmd!(sh, "cargo +{active_toolchain} --quiet run {cargo_extra_flags...} --manifest-path {manifest_path} -- miri setup {target_prefix...} {target...}").run().with_context(|| "`cargo miri setup` failed")?; + } + + Ok(output?) + } + fn build_package( + // Path to Cargo.toml file of a package to build. + path: &OsStr, + toolchain: impl AsRef, + extra_flags: &[String], + args: impl IntoIterator>, + ) -> Result<()> { + let sh = Shell::new()?; + cmd!(sh, "cargo +{toolchain} build {extra_flags...} --manifest-path {path} {args...}") + .run()?; + Ok(()) + } + fn shell(&self) -> Result { + let sh = shell_with_parent_env()?; + for (k, v) in &self.env { + sh.set_var(k, v); + } + + Ok(sh) + } + + fn libdir(&self) -> Result { + let sh = shell_with_parent_env()?; + let toolchain = &self.active_toolchain; + let target_output = cmd!(sh, "rustc +{toolchain} --version --verbose").read()?; + let rustc_meta = rustc_version::version_meta_for(&target_output)?; + let target = rustc_meta.host; + + let sysroot = cmd!(sh, "rustc +{toolchain} --print sysroot").read()?; + + let sysroot = PathBuf::from(sysroot); + let libdir = path!(sysroot / "lib" / "rustlib" / target / "lib"); + Ok(libdir) + } + fn sysroot(&mut self) -> Result { + if let Some(sysroot) = self.sysroot.as_ref() { + Ok(sysroot.clone()) + } else { + let sh = shell_with_parent_env()?; + let toolchain = &self.active_toolchain; + + let sysroot: PathBuf = cmd!(sh, "rustc +{toolchain} --print sysroot").read()?.into(); + self.sysroot = Some(sysroot.clone()); + Ok(sysroot) + } + } + fn install_to_dir( + &mut self, + sh: &Shell, + path: PathBuf, + args: impl IntoIterator>, + ) -> Result<()> { + let sysroot = self.sysroot()?; + let toolchain = &self.active_toolchain; + let extra_flags = &self.cargo_extra_flags; + // "--locked" to respect the Cargo.lock file if it exists. + // Install binaries to the miri toolchain's sysroot so they do not interact with other toolchains. + cmd!(sh, "cargo +{toolchain} install {extra_flags...} --path {path} --force --root {sysroot} {args...}").run()?; + Ok(()) + } +} + +impl MiriRunner<'_> { + fn bench(&self, benches: &[OsString]) -> Result<()> { + // The hyperfine to use + let hyperfine = std::env::var("HYPERFINE"); + let hyperfine = hyperfine.as_deref().unwrap_or("hyperfine -w 1 -m 5 --shell=none"); + let hyperfine = shell_words::split(hyperfine).unwrap(); + let Some((program_name, args)) = hyperfine.split_first() else { + bail!("Expected HYPERFINE environment variable to be non-empty"); + }; + // Make sure we have an up-to-date Miri installed + Self::exec_inner(&Subcommands::Install { flags: vec![] }, false)?; + let benches_dir = path!(self.miri_dir / "bench-cargo-miri"); + let benches = if benches.is_empty() { + std::fs::read_dir(&benches_dir)? + .filter_map(|path| { + path.ok() + .filter(|dir| dir.file_type().map(|t| t.is_dir()).unwrap_or(false)) + .map(|p| p.file_name()) + }) + .collect() + } else { + benches.to_owned() + }; + let sh = shell_with_parent_env()?; + let toolchain = &self.active_toolchain; + // Run the requested benchmarks + for bench in benches { + let current_bench_dir = path!(benches_dir / bench / "Cargo.toml"); + cmd!( + sh, + "{program_name} {args...} 'cargo +'{toolchain}' miri run --manifest-path \"'{current_bench_dir}'\"'" + ) + .run()?; + } + Ok(()) + } + + fn toolchain(&self, flags: &[OsString]) -> Result<()> { + // Make sure rustup-toolchain-install-master is installed. + which::which("rustup-toolchain-install-master").context("Please install rustup-toolchain-install-master by running 'cargo install rustup-toolchain-install-master'")?; + let sh = shell_with_parent_env()?; + sh.change_dir(&self.miri_dir); + let new_commit = Some(sh.read_file("rust-version")?.trim().to_owned()); + let current_commit = { + let rustc_info = cmd!(sh, "rustc +miri --version -v").read(); + if rustc_info.is_err() { + None + } else { + let metadata = rustc_version::version_meta_for(&rustc_info.unwrap())?; + Some( + metadata + .commit_hash + .ok_or_else(|| anyhow!("rustc metadata did not contain commit hash"))?, + ) + } + }; + // Check if we already are at that commit. + if current_commit == new_commit { + println!("miri toolchain is already at commit {}.", current_commit.unwrap()); + cmd!(sh, "rustup override set miri").run()?; + return Ok(()); + } + // Install and setup new toolchain. + cmd!(sh, "rustup toolchain uninstall miri").run()?; + + cmd!(sh, "rustup-toolchain-install-master -n miri -c cargo -c rust-src -c rustc-dev -c llvm-tools -c rustfmt -c clippy {flags...} -- {new_commit...}").run()?; + cmd!(sh, "rustup override set miri").run()?; + // Cleanup. + cmd!(sh, "cargo clean").run()?; + // Call `cargo metadata` on the sources in case that changes the lockfile + // (which fails under some setups when it is done from inside vscode). + let sysroot = cmd!(sh, "rustc --print sysroot").read()?; + let sysroot = sysroot.trim(); + cmd!(sh, "cargo metadata --format-version 1 --manifest-path {sysroot}/lib/rustlib/rustc-src/rust/compiler/rustc/Cargo.toml").ignore_stdout().run()?; + Ok(()) + } + + fn rustc_pull(&self, commit: Option) -> Result<()> { + let sh = shell_with_parent_env()?; + sh.change_dir(&self.miri_dir); + let commit: String = commit.map(Result::Ok).unwrap_or_else(|| { + let rust_repo_head = + cmd!(sh, "git ls-remote https://github.com/rust-lang/rust/ HEAD").read()?; + rust_repo_head + .split_whitespace() + .next() + .map(|front| front.trim().to_owned()) + .ok_or_else(|| anyhow!("Could not obtain Rust repo HEAD from remote.")) + })?; + // Update rust-version file. As a separate commit, since making it part of + // the merge has confused the heck out of josh in the past. + sh.write_file(path!(self.miri_dir / "rust-version"), &commit)?; + const PREPARING_COMMIT_MESSAGE: &str = "Preparing for merge from rustc"; + cmd!(sh, "git commit rust-version -m {PREPARING_COMMIT_MESSAGE}") + .run() + .context("FAILED to commit rust-version file, something went wrong")?; + // Fetch given rustc commit and note down which one that was + cmd!(sh, "git fetch http://localhost:8000/rust-lang/rust.git@{commit}{JOSH_FILTER}.git") + .run() + .context("FAILED to fetch new commits, something went wrong")?; + const MERGE_COMMIT_MESSAGE: &str = "Merge from rustc"; + cmd!(sh, "git merge FETCH_HEAD --no-ff -m {MERGE_COMMIT_MESSAGE}") + .run() + .context("FAILED to merge new commits, something went wrong")?; + Ok(()) + } + + fn rustc_push(&self, github_user: &str, branch: &str) -> Result<()> { + let rustc_git = std::env::var_os("RUSTC_GIT"); + let working_directory = if let Some(rustc_git) = rustc_git { + rustc_git + } else { + // If rustc_git is `Some`, we'll use an existing fork for the branch updates. + // Otherwise, do this in the local Miri repo. + println!( + "This will pull a copy of the rust-lang/rust history into this Miri checkout, growing it by about 1GB." + ); + println!( + "To avoid that, abort now and set the RUSTC_GIT environment variable to an existing rustc checkout. Proceed? [y/N] " + ); + let mut answer = String::new(); + std::io::stdin().read_line(&mut answer)?; + if answer.trim().to_lowercase() != "y" { + std::process::exit(1); + } + self.miri_dir.clone().into() + }; + // Prepare the branch. Pushing works much better if we use as base exactly + // the commit that we pulled from last time, so we use the `rust-version` + // file as a good approximation of that. + let rust_version_path = path!(self.miri_dir / "rust-version"); + let base = std::fs::read_to_string(rust_version_path)?.trim().to_owned(); + println!("Preparing {github_user}/rust (base: {base})...)"); + let sh = shell_with_parent_env()?; + sh.change_dir(working_directory); + + if cmd!(sh, "git fetch https://github.com/{github_user}").read().is_ok() { + println!( + "The branch '{branch}' seems to already exist in 'https://github.com/{github_user}'. Please delete it and try again." + ); + std::process::exit(1); + } + + cmd!(sh, "git fetch https://github.com/rust-lang/rust {base}").run()?; + + cmd!(sh, "git push https://github.com/{github_user}/rust {base}:refs/heads/{branch}") + .run()?; + println!(); + // Do the actual push. + sh.change_dir(&self.miri_dir); + println!("Pushing miri changes..."); + cmd!( + sh, + "git push http://localhost:8000/{github_user}/rust.git{JOSH_FILTER}.git HEAD:{branch}" + ) + .run()?; + // Do a round-trip check to make sure the push worked as expected. + println!(); + cmd!( + sh, + "git fetch http://localhost:8000/{github_user}/rust.git{JOSH_FILTER}.git {branch}" + ) + .read()?; + let head = cmd!(sh, "git rev-parse HEAD").read()?; + let fetch_head = cmd!(sh, "git rev-parse FETCH_HEAD").read()?; + if head != fetch_head { + println!("ERROR: Josh created a non-roundtrip push! Do NOT merge this into rustc!"); + std::process::exit(1); + } + println!( + "Confirmed that the push round-trips back to Miri properly. Please create a rustc PR:" + ); + println!(" https://github.com/{github_user}/rust/pull/new/{branch}"); + Ok(()) + } + + fn install(&mut self, flags: &[OsString]) -> Result<()> { + let sh = self.shell()?; + self.install_to_dir(&sh, self.miri_dir.clone(), flags)?; + let cargo_miri_dir = path!(self.miri_dir / "cargo-miri"); + self.install_to_dir(&sh, cargo_miri_dir, flags)?; + Ok(()) + } + + fn build(&self, flags: &[OsString]) -> Result<()> { + // Build, and let caller control flags. + let miri_manifest = path!(self.miri_dir / "Cargo.toml"); + let cargo_miri_manifest = path!(self.miri_dir / "cargo-miri" / "Cargo.toml"); + Self::build_package( + miri_manifest.as_ref(), + &self.active_toolchain, + &self.cargo_extra_flags, + flags, + )?; + Self::build_package( + cargo_miri_manifest.as_ref(), + &self.active_toolchain, + &self.cargo_extra_flags, + flags, + )?; + Ok(()) + } + + fn check(&self, flags: &[OsString]) -> Result<()> { + fn check_package( + // Path to Cargo.toml file of a package to check. + path: &OsStr, + toolchain: impl AsRef, + extra_flags: &[String], + all_targets: bool, + args: impl IntoIterator>, + ) -> Result<()> { + let all_targets: Option<&OsStr> = all_targets.then_some("--all-targets".as_ref()); + let sh = Shell::new()?; + cmd!(sh, "cargo +{toolchain} check {extra_flags...} --manifest-path {path} {all_targets...} {args...}").run()?; + Ok(()) + } + // Check, and let caller control flags. + let miri_manifest = path!(self.miri_dir / "Cargo.toml"); + let cargo_miri_manifest = path!(self.miri_dir / "cargo-miri" / "Cargo.toml"); + check_package( + miri_manifest.as_ref(), + &self.active_toolchain, + &self.cargo_extra_flags, + true, + flags, + )?; + check_package( + cargo_miri_manifest.as_ref(), + &self.active_toolchain, + &self.cargo_extra_flags, + false, + flags, + )?; + Ok(()) + } + + fn test(&mut self, bless: bool, flags: &[OsString]) -> Result<()> { + let miri_manifest = path!(self.miri_dir / "Cargo.toml"); + // First build and get a sysroot. + Self::build_package( + miri_manifest.as_ref(), + &self.active_toolchain, + &self.cargo_extra_flags, + std::iter::empty::(), + )?; + self.find_miri_sysroot()?; + let extra_flags = &self.cargo_extra_flags; + // Then test, and let caller control flags. + // Only in root project as `cargo-miri` has no tests. + let sh = self.shell()?; + if bless { + sh.set_var("MIRI_BLESS", "Gesundheit"); + } + let toolchain: &OsStr = self.active_toolchain.as_ref(); + cmd!( + sh, + "cargo +{toolchain} test {extra_flags...} --manifest-path {miri_manifest} -- {flags...}" + ) + .run()?; + Ok(()) + } + + fn run(&mut self, dep: bool, flags: &[OsString]) -> Result<()> { + use itertools::Itertools; + // Scan for "--target" to overwrite the "MIRI_TEST_TARGET" env var so + // that we set the MIRI_SYSROOT up the right way. + let target = flags.iter().tuple_windows().find(|(first, _)| first == &"--target"); + if let Some((_, target)) = target { + // Found it! + self.set_env("MIRI_TEST_TARGET", target); + } else if let Some(var) = + std::env::var_os("MIRI_TEST_TARGET").filter(|target| !target.is_empty()) + { + // Make sure miri actually uses this target. + let entry = self.env.entry("MIRIFLAGS".into()).or_default(); + entry.push(" --target "); + entry.push(var); + } + // First build and get a sysroot. + let miri_manifest = path!(self.miri_dir / "Cargo.toml"); + Self::build_package( + miri_manifest.as_ref(), + &self.active_toolchain, + &self.cargo_extra_flags, + std::iter::empty::(), + )?; + self.find_miri_sysroot()?; + // Then run the actual command. + let miri_flags = self.env.get(&OsString::from("MIRIFLAGS")).cloned().unwrap_or_default(); + let miri_flags: &OsStr = miri_flags.as_ref(); + let extra_flags = &self.cargo_extra_flags; + let sh = self.shell()?; + let toolchain: &OsStr = self.active_toolchain.as_ref(); + if dep { + cmd!( + sh, + "cargo +{toolchain} --quiet test --test compiletest {extra_flags...} --manifest-path {miri_manifest} -- --miri-run-dep-mode {miri_flags} {flags...}" + ).run()?; + } else { + cmd!( + sh, + "cargo +{toolchain} --quiet run {extra_flags...} --manifest-path {miri_manifest} -- {miri_flags} {flags...}" + ).run()?; + } + Ok(()) + } + + fn fmt(&self, flags: &[OsString]) -> Result<()> { + let toolchain = &self.active_toolchain; + let config_path = path!(self.miri_dir / "rustfmt.toml"); + let sh = self.shell()?; + for item in WalkDir::new(&self.miri_dir).into_iter().filter_entry(|entry| { + let name: String = entry.file_name().to_string_lossy().into(); + let ty = entry.file_type(); + if ty.is_file() { + name.ends_with(".rs") + } else { + // dir or symlink + &name != "target" + } + }) { + let item = item.unwrap(); // Should never panic as we've already filtered out failed entries. + if item.file_type().is_file() { + let path = item.path(); + cmd!(sh, "rustfmt +{toolchain} --edition=2021 --config-path {config_path} {flags...} {path}").quiet().run()?; + } + } + Ok(()) + } + + fn clippy(&self, flags: &[OsString]) -> Result<()> { + let toolchain_modifier = &self.active_toolchain; + let extra_flags = &self.cargo_extra_flags; + let miri_manifest = path!(self.miri_dir / "Cargo.toml"); + let sh = self.shell()?; + cmd!(sh, "cargo +{toolchain_modifier} clippy {extra_flags...} --manifest-path {miri_manifest} --all-targets -- {flags...}").run()?; + Ok(()) + } + + fn cargo(&self, flags: &[OsString]) -> Result<()> { + // We carefully kept the working dir intact, so this will run cargo *on the workspace in the + // current working dir*, not on the main Miri workspace. That is exactly what RA needs. + let toolchain_modifier = &self.active_toolchain; + let sh = self.shell()?; + cmd!(sh, "cargo +{toolchain_modifier} {flags...}").run()?; + Ok(()) + } + fn many_seeds(&self, command: &[OsString], seed_start: u64, seed_count: u64) -> Result<()> { + let seed_end = seed_start + seed_count; + assert!(!command.is_empty()); + let (command_name, trailing_args) = command.split_first().unwrap(); + let sh = shell_with_parent_env()?; + for seed in seed_start..seed_end { + println!("Trying seed: {seed}"); + let mut miriflags = std::env::var_os("MIRIFLAGS").unwrap_or_default(); + miriflags.push(format!(" -Zlayout-seed={seed} -Zmiri-seed={seed}")); + let status = + cmd!(sh, "{command_name} {trailing_args...}").env("MIRIFLAGS", miriflags).run(); + if status.is_err() { + println!("Failing seed: {seed}"); + break; + } + } + Ok(()) + } +} diff --git a/src/tools/miri/miri-script/src/main.rs b/src/tools/miri/miri-script/src/main.rs new file mode 100644 index 0000000000000..dbee923d48f8a --- /dev/null +++ b/src/tools/miri/miri-script/src/main.rs @@ -0,0 +1,58 @@ +pub(crate) mod arg; +mod commands; + +use std::path::Path; + +use anyhow::Result; +use clap::Parser; +use path_macro::path; + +use arg::Subcommands; + +struct AutoConfig { + toolchain: bool, + fmt: bool, + clippy: bool, +} + +impl Subcommands { + fn run_auto_things(&self) -> bool { + use Subcommands::*; + match self { + // Early commands, that don't do auto-things and don't want the environment-altering things happening below. + Toolchain { .. } + | RustcPull { .. } + | RustcPush { .. } + | ManySeeds { .. } + | Bench { .. } => false, + Install { .. } + | Check { .. } + | Build { .. } + | Test { .. } + | Run { .. } + | Fmt { .. } + | Clippy { .. } + | Cargo { .. } => true, + } + } + fn get_config(&self, miri_dir: &Path) -> Option { + let skip_auto_ops = std::env::var_os("MIRI_AUTO_OPS").is_some(); + if !self.run_auto_things() { + return None; + } + if skip_auto_ops { + return Some(AutoConfig { toolchain: false, fmt: false, clippy: false }); + } + + let auto_everything = path!(miri_dir / ".auto_everything").exists(); + let toolchain = auto_everything || path!(miri_dir / ".auto-toolchain").exists(); + let fmt = auto_everything || path!(miri_dir / ".auto-fmt").exists(); + let clippy = auto_everything || path!(miri_dir / ".auto-clippy").exists(); + Some(AutoConfig { toolchain, fmt, clippy }) + } +} +fn main() -> Result<()> { + let args = arg::Cli::parse(); + commands::MiriRunner::exec(&args.commands)?; + Ok(()) +} From 3f952f4508c0216575b246e8279357cf99475e01 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 30 Jul 2023 17:28:23 +0200 Subject: [PATCH 05/30] miri-script refactor --- src/tools/miri/.github/workflows/ci.yml | 2 +- src/tools/miri/CONTRIBUTING.md | 2 +- src/tools/miri/cargo-miri/src/phases.rs | 3 +- src/tools/miri/cargo-miri/src/util.rs | 5 + src/tools/miri/miri | 4 +- src/tools/miri/miri-script/miri | 4 + src/tools/miri/miri-script/src/arg.rs | 104 --- src/tools/miri/miri-script/src/commands.rs | 734 ++++++++------------- src/tools/miri/miri-script/src/main.rs | 156 +++-- src/tools/miri/miri-script/src/util.rs | 153 +++++ 10 files changed, 553 insertions(+), 614 deletions(-) create mode 100755 src/tools/miri/miri-script/miri delete mode 100644 src/tools/miri/miri-script/src/arg.rs create mode 100644 src/tools/miri/miri-script/src/util.rs diff --git a/src/tools/miri/.github/workflows/ci.yml b/src/tools/miri/.github/workflows/ci.yml index 042bb9bbd2ca9..8ced9fa86bee4 100644 --- a/src/tools/miri/.github/workflows/ci.yml +++ b/src/tools/miri/.github/workflows/ci.yml @@ -130,7 +130,7 @@ jobs: - name: clippy run: ./miri clippy -- -D warnings - name: rustdoc - run: RUSTDOCFLAGS="-Dwarnings" cargo doc --document-private-items + run: RUSTDOCFLAGS="-Dwarnings" ./miri cargo doc --document-private-items # These jobs doesn't actually test anything, but they're only used to tell # bors the build completed, as there is no practical way to detect when a diff --git a/src/tools/miri/CONTRIBUTING.md b/src/tools/miri/CONTRIBUTING.md index bcdb623b090e4..b67e7103fd095 100644 --- a/src/tools/miri/CONTRIBUTING.md +++ b/src/tools/miri/CONTRIBUTING.md @@ -107,7 +107,7 @@ evaluation error was originally raised. ### UI testing We use ui-testing in Miri, meaning we generate `.stderr` and `.stdout` files for the output -produced by Miri. You can use `./miri bless` to automatically (re)generate these files when +produced by Miri. You can use `./miri test --bless` to automatically (re)generate these files when you add new tests or change how Miri presents certain output. Note that when you also use `MIRIFLAGS` to change optimizations and similar, the ui output diff --git a/src/tools/miri/cargo-miri/src/phases.rs b/src/tools/miri/cargo-miri/src/phases.rs index 465e4a1b2d2b2..d74e0c5157d35 100644 --- a/src/tools/miri/cargo-miri/src/phases.rs +++ b/src/tools/miri/cargo-miri/src/phases.rs @@ -538,8 +538,7 @@ pub fn phase_runner(mut binary_args: impl Iterator, phase: Runner } // Respect `MIRIFLAGS`. if let Ok(a) = env::var("MIRIFLAGS") { - // This code is taken from `RUSTFLAGS` handling in cargo. - let args = a.split(' ').map(str::trim).filter(|s| !s.is_empty()).map(str::to_string); + let args = flagsplit(&a); cmd.args(args); } diff --git a/src/tools/miri/cargo-miri/src/util.rs b/src/tools/miri/cargo-miri/src/util.rs index 4c19ed97fb810..0e3b04c0d8817 100644 --- a/src/tools/miri/cargo-miri/src/util.rs +++ b/src/tools/miri/cargo-miri/src/util.rs @@ -114,6 +114,11 @@ pub fn cargo() -> Command { Command::new(env::var_os("CARGO").unwrap_or_else(|| OsString::from("cargo"))) } +pub fn flagsplit(flags: &str) -> Vec { + // This code is taken from `RUSTFLAGS` handling in cargo. + flags.split(' ').map(str::trim).filter(|s| !s.is_empty()).map(str::to_string).collect() +} + /// Execute the `Command`, where possible by replacing the current process with a new process /// described by the `Command`. Then exit this process with the exit code of the new process. pub fn exec(mut cmd: Command) -> ! { diff --git a/src/tools/miri/miri b/src/tools/miri/miri index 7412df69bd67c..c816a4bb06b16 100755 --- a/src/tools/miri/miri +++ b/src/tools/miri/miri @@ -2,5 +2,5 @@ set -e # Instead of doing just `cargo run --manifest-path .. $@`, we invoke miri-script binary directly. Invoking `cargo run` goes through # rustup (that sets it's own environmental variables), which is undesirable. -cargo build --manifest-path "$(dirname "$0")"/miri-script/Cargo.toml -"$(dirname "$0")"/miri-script/target/debug/miri-script $@ \ No newline at end of file +cargo build -q --manifest-path "$(dirname "$0")"/miri-script/Cargo.toml +"$(dirname "$0")"/miri-script/target/debug/miri-script "$@" diff --git a/src/tools/miri/miri-script/miri b/src/tools/miri/miri-script/miri new file mode 100755 index 0000000000000..cf3ad06788ab1 --- /dev/null +++ b/src/tools/miri/miri-script/miri @@ -0,0 +1,4 @@ +#!/bin/sh +# RA invokes `./miri cargo ...` for each workspace, so we need to forward that to the main `miri` +# script. See . +exec "$(dirname "$0")"/../miri "$@" diff --git a/src/tools/miri/miri-script/src/arg.rs b/src/tools/miri/miri-script/src/arg.rs deleted file mode 100644 index 24a5204e04397..0000000000000 --- a/src/tools/miri/miri-script/src/arg.rs +++ /dev/null @@ -1,104 +0,0 @@ -use clap::{Parser, Subcommand}; -use std::ffi::OsString; - -#[derive(Parser, Clone, Debug)] -#[command(author, about, long_about = None)] -pub struct Cli { - #[command(subcommand)] - pub commands: Subcommands, -} - -#[derive(Subcommand, Clone, Debug)] -pub enum Subcommands { - /// Installs the miri driver and cargo-miri. - /// Sets up the rpath such that the installed binary should work in any - /// working directory. Note that the binaries are placed in the `miri` toolchain - /// sysroot, to prevent conflicts with other toolchains. - Install { - /// Flags that are passed through to `cargo install`. - #[arg(trailing_var_arg = true, allow_hyphen_values = true)] - flags: Vec, - }, - /// Just build miri. - Build { - /// Flags that are passed through to `cargo build`. - #[arg(trailing_var_arg = true, allow_hyphen_values = true)] - flags: Vec, - }, - /// Just check miri. - Check { - /// Flags that are passed through to `cargo check`. - #[arg(trailing_var_arg = true, allow_hyphen_values = true)] - flags: Vec, - }, - /// Build miri, set up a sysroot and then run the test suite. - Test { - #[arg(long, default_value_t = false)] - bless: bool, - /// Flags that are passed through to `cargo test`. - #[arg(trailing_var_arg = true, allow_hyphen_values = true)] - flags: Vec, - }, - /// Build miri, set up a sysroot and then run the driver with the given . - /// (Also respects MIRIFLAGS environment variable.) - Run { - #[arg(long, default_value_t = false)] - dep: bool, - /// Flags that are passed through to `miri` - #[arg(trailing_var_arg = true, allow_hyphen_values = true)] - flags: Vec, - }, - /// Format all sources and tests. - Fmt { - /// Flags that are passed through to `rustfmt`. - #[arg(trailing_var_arg = true, allow_hyphen_values = true)] - flags: Vec, - }, - /// Runs clippy on all sources. - Clippy { - /// Flags that are passed through to `cargo clippy`. - #[arg(trailing_var_arg = true, allow_hyphen_values = true)] - flags: Vec, - }, - /// Runs just `cargo ` with the Miri-specific environment variables. - /// Mainly meant to be invoked by rust-analyzer. - Cargo { - #[arg(trailing_var_arg = true, allow_hyphen_values = true)] - flags: Vec, - }, - /// Runs over and over again with different seeds for Miri. The MIRIFLAGS - /// variable is set to its original value appended with ` -Zmiri-seed=$SEED` for - /// many different seeds. - ManySeeds { - /// Starting seed. - #[clap(long, env("MIRI_SEED_START"), default_value_t = 0)] - seed_start: u64, - #[clap(long, env("MIRI_SEEDS"), default_value_t = 256)] - /// Amount of seeds to try. - seeds: u64, - #[arg(trailing_var_arg = true, allow_hyphen_values = true)] - command: Vec, - }, - /// Runs the benchmarks from bench-cargo-miri in hyperfine. hyperfine needs to be installed. - Bench { - /// List of benchmarks to run. By default all benchmarks are run. - #[arg(trailing_var_arg = true, allow_hyphen_values = true)] - benches: Vec, - }, - /// Update and activate the rustup toolchain 'miri' to the commit given in the - /// `rust-version` file. - /// `rustup-toolchain-install-master` must be installed for this to work. Any extra - /// flags are passed to `rustup-toolchain-install-master`. - Toolchain { - #[arg(trailing_var_arg = true, allow_hyphen_values = true)] - flags: Vec, - }, - /// Pull and merge Miri changes from the rustc repo. Defaults to fetching the latest - /// rustc commit. The fetched commit is stored in the `rust-version` file, so the - /// next `./miri toolchain` will install the rustc that just got pulled. - RustcPull { commit: Option }, - /// Push Miri changes back to the rustc repo. This will pull a copy of the rustc - /// history into the Miri repo, unless you set the RUSTC_GIT env var to an existing - /// clone of the rustc repo. - RustcPush { github_user: String, branch: String }, -} diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index 121b50678045b..33e407a65dab5 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -1,307 +1,100 @@ -use std::collections::BTreeMap; - -use std::ffi::{OsStr, OsString}; -use std::path::{Path, PathBuf}; +use std::env; +use std::ffi::OsString; +use std::io::Write; +use std::ops::Not; use anyhow::{anyhow, bail, Context, Result}; -use dunce::canonicalize; use path_macro::path; -use xshell::{cmd, Shell}; - use walkdir::WalkDir; +use xshell::cmd; -use crate::arg::Subcommands; +use crate::util::*; +use crate::Command; /// Used for rustc syncs. const JOSH_FILTER: &str = ":rev(75dd959a3a40eb5b4574f8d2e23aa6efbeb33573:prefix=src/tools/miri):/src/tools/miri"; -fn detect_miri_dir() -> std::io::Result { - const MIRI_SCRIPT_ROOT_DIR: &str = env!("CARGO_MANIFEST_DIR"); - Ok(canonicalize(MIRI_SCRIPT_ROOT_DIR)?.parent().unwrap().into()) -} - -/// Queries an active toolchain for `dir` via `rustup`. -fn get_active_toolchain(dir: &Path) -> Result { - let sh = Shell::new()?; - sh.change_dir(dir); - let stdout = cmd!(sh, "rustup show active-toolchain").read()?; - Ok(stdout.split_whitespace().next().context("Could not obtain active Rust toolchain")?.into()) -} - -#[derive(Clone, Debug)] -pub(super) struct MiriRunner<'a> { - /// miri_dir is the root of the miri repository checkout we are working in. - miri_dir: PathBuf, - /// active_toolchain is passed as `+toolchain` argument to cargo/rustc invocations. - active_toolchain: String, - cargo_extra_flags: Vec, - command: &'a super::Subcommands, - /// Environment variables passed to child processes. - env: BTreeMap, - /// Additional variables used by environment-altering commands. - /// These should be accessed by corresponding methods (e.g. `sysroot()`) and not directly. - sysroot: Option, -} - -fn shell_with_parent_env() -> Result { - let sh = Shell::new()?; - // xshell does not propagate parent's env variables by default. - for (k, v) in std::env::vars_os() { - sh.set_var(k, v); - } - Ok(sh) -} - -impl MiriRunner<'_> { - pub(super) fn exec(command: &super::Subcommands) -> Result<()> { - Self::exec_inner(command, true) - } - fn exec_inner(command: &super::Subcommands, run_auto_things: bool) -> Result<()> { - let miri_dir = detect_miri_dir()?; - let active_toolchain = get_active_toolchain(&miri_dir)?; - let config = command.get_config(&miri_dir); - // CARGO_EXTRA_FLAGS do not have to be a valid UTF-8, but that's what shell_words' expects. - let cargo_extra_flags = std::env::var("CARGO_EXTRA_FLAGS").unwrap_or_default(); - let cargo_extra_flags = shell_words::split(&cargo_extra_flags)?; - let env = BTreeMap::new(); - - let mut runner = MiriRunner { - miri_dir, - active_toolchain, - command, - env, - cargo_extra_flags, - sysroot: None, - }; - if let Some(config) = config { - // Run the auto-things. - if run_auto_things { - if config.toolchain { - // Run this first, so that the toolchain doesn't change after - // other code has run. - let command = Subcommands::Toolchain { flags: vec![] }; - Self::exec_inner(&command, false)?; - // Let's make sure to actually use that toolchain, too. - runner.active_toolchain = "miri".to_owned(); - } - if config.fmt { - let command = Subcommands::Fmt { flags: vec![] }; - Self::exec_inner(&command, false)?; - } - if config.clippy { - let command = Subcommands::Clippy { - flags: ["--", "-D", "warnings"].into_iter().map(OsString::from).collect(), - }; - Self::exec_inner(&command, false)?; - } - } - - // Prepare the environment - // Determine some toolchain properties - let libdir = runner.libdir()?; - if !libdir.exists() { - println!("Something went wrong determining the library dir."); - println!("I got {} but that does not exist.", libdir.display()); - println!("Please report a bug at https://github.com/rust-lang/miri/issues."); - std::process::exit(2); - } - // Share target dir between `miri` and `cargo-miri`. - let target_dir = std::env::var_os("CARGO_TARGET_DIR") - .filter(|val| !val.is_empty()) - .unwrap_or_else(|| { - let target_dir = path!(runner.miri_dir / "target"); - target_dir.into() - }); - runner.set_env("CARGO_TARGET_DIR", target_dir); - - // We configure dev builds to not be unusably slow. - let devel_opt_level = std::env::var_os("CARGO_PROFILE_DEV_OPT_LEVEL") - .filter(|val| !val.is_empty()) - .unwrap_or_else(|| "2".into()); - runner.set_env("CARGO_PROFILE_DEV_OPT_LEVEL", devel_opt_level); - let rustflags = { - let env = std::env::var_os("RUSTFLAGS"); - let mut flags_with_warnings = OsString::from( - "-Zunstable-options -Wrustc::internal -Wrust_2018_idioms -Wunused_lifetimes -Wsemicolon_in_expressions_from_macros ", - ); - if let Some(value) = env { - flags_with_warnings.push(value); - } - // We set the rpath so that Miri finds the private rustc libraries it needs. - let mut flags_with_compiler_settings = OsString::from("-C link-args=-Wl,-rpath,"); - flags_with_compiler_settings.push(&libdir); - flags_with_compiler_settings.push(flags_with_warnings); - flags_with_compiler_settings - }; - runner.set_env("RUSTFLAGS", rustflags); - } - runner.execute() - } - fn execute(&mut self) -> Result<()> { - // Run command. - match self.command { - Subcommands::Install { flags } => self.install(flags), - Subcommands::Build { flags } => self.build(flags), - Subcommands::Check { flags } => self.check(flags), - Subcommands::Test { bless, flags } => self.test(*bless, flags), - Subcommands::Run { dep, flags } => self.run(*dep, flags), - Subcommands::Fmt { flags } => self.fmt(flags), - Subcommands::Clippy { flags } => self.clippy(flags), - Subcommands::Cargo { flags } => self.cargo(flags), - Subcommands::ManySeeds { command, seed_start, seeds } => - self.many_seeds(command, *seed_start, *seeds), - Subcommands::Bench { benches } => self.bench(benches), - Subcommands::Toolchain { flags } => self.toolchain(flags), - Subcommands::RustcPull { commit } => self.rustc_pull(commit.clone()), - Subcommands::RustcPush { github_user, branch } => self.rustc_push(github_user, branch), - } - } - - fn set_env( - &mut self, - key: impl Into, - value: impl Into, - ) -> Option { - self.env.insert(key.into(), value.into()) - } - - /// Prepare and set MIRI_SYSROOT. Respects `MIRI_TEST_TARGET` and takes into account - /// locally built vs. distributed rustc. - fn find_miri_sysroot(&mut self) -> Result<()> { - let current_sysroot = std::env::var_os("MIRI_SYSROOT").unwrap_or_default(); - - if !current_sysroot.is_empty() { +impl MiriEnv { + fn build_miri_sysroot(&mut self) -> Result<()> { + if self.sh.var("MIRI_SYSROOT").is_ok() { // Sysroot already set, use that. - let current_value = self.set_env("MIRI_SYSROOT", ¤t_sysroot); - assert!(current_value.is_none() || current_value.unwrap() == current_sysroot); return Ok(()); } - // We need to build a sysroot. - let target = std::env::var_os("MIRI_TEST_TARGET").filter(|target| !target.is_empty()); - let sysroot = self.build_miri_sysroot(target.as_deref())?; - self.set_env("MIRI_SYSROOT", sysroot); + let manifest_path = path!(self.miri_dir / "cargo-miri" / "Cargo.toml"); + let Self { toolchain, cargo_extra_flags, .. } = &self; + let target = &match self.sh.var("MIRI_TEST_TARGET") { + Ok(target) => vec!["--target".into(), target], + Err(_) => vec![], + }; + let output = cmd!(self.sh, + "cargo +{toolchain} --quiet run {cargo_extra_flags...} --manifest-path {manifest_path} -- + miri setup --print-sysroot {target...}" + ).read(); + let Ok(output) = output else { + // Run it again (without `--print-sysroot` or `--quiet`) so the user can see the error. + cmd!( + self.sh, + "cargo +{toolchain} run {cargo_extra_flags...} --manifest-path {manifest_path} -- + miri setup {target...}" + ) + .run() + .with_context(|| "`cargo miri setup` failed")?; + panic!("`cargo miri setup` didn't fail again the 2nd time?"); + }; + self.sh.set_var("MIRI_SYSROOT", output); Ok(()) } +} - /// Build a sysroot and set MIRI_SYSROOT to use it. Arguments are passed to `cargo miri setup`. - fn build_miri_sysroot(&self, target: Option<&OsStr>) -> Result { - let manifest_path = path!(self.miri_dir / "cargo-miri" / "Cargo.toml"); - let Self { active_toolchain, cargo_extra_flags, .. } = &self; - let target_prefix: Option<&OsStr> = target.map(|_| "--target".as_ref()); - let sh = self.shell()?; - let output = cmd!(sh, "cargo +{active_toolchain} --quiet run {cargo_extra_flags...} --manifest-path {manifest_path} -- miri setup --print-sysroot {target_prefix...} {target...}").read(); - if output.is_err() { - // Run it again (without `--print-sysroot`) so the user can see the error. - cmd!(sh, "cargo +{active_toolchain} --quiet run {cargo_extra_flags...} --manifest-path {manifest_path} -- miri setup {target_prefix...} {target...}").run().with_context(|| "`cargo miri setup` failed")?; +impl Command { + fn auto_actions() -> Result<()> { + let miri_dir = miri_dir()?; + let auto_everything = path!(miri_dir / ".auto_everything").exists(); + let auto_toolchain = auto_everything || path!(miri_dir / ".auto-toolchain").exists(); + let auto_fmt = auto_everything || path!(miri_dir / ".auto-fmt").exists(); + let auto_clippy = auto_everything || path!(miri_dir / ".auto-clippy").exists(); + + // `toolchain` goes first as it could affect the others + if auto_toolchain { + Self::toolchain(vec![])?; } - - Ok(output?) - } - fn build_package( - // Path to Cargo.toml file of a package to build. - path: &OsStr, - toolchain: impl AsRef, - extra_flags: &[String], - args: impl IntoIterator>, - ) -> Result<()> { - let sh = Shell::new()?; - cmd!(sh, "cargo +{toolchain} build {extra_flags...} --manifest-path {path} {args...}") - .run()?; - Ok(()) - } - fn shell(&self) -> Result { - let sh = shell_with_parent_env()?; - for (k, v) in &self.env { - sh.set_var(k, v); + if auto_fmt { + Self::fmt(vec![])?; } - - Ok(sh) - } - - fn libdir(&self) -> Result { - let sh = shell_with_parent_env()?; - let toolchain = &self.active_toolchain; - let target_output = cmd!(sh, "rustc +{toolchain} --version --verbose").read()?; - let rustc_meta = rustc_version::version_meta_for(&target_output)?; - let target = rustc_meta.host; - - let sysroot = cmd!(sh, "rustc +{toolchain} --print sysroot").read()?; - - let sysroot = PathBuf::from(sysroot); - let libdir = path!(sysroot / "lib" / "rustlib" / target / "lib"); - Ok(libdir) - } - fn sysroot(&mut self) -> Result { - if let Some(sysroot) = self.sysroot.as_ref() { - Ok(sysroot.clone()) - } else { - let sh = shell_with_parent_env()?; - let toolchain = &self.active_toolchain; - - let sysroot: PathBuf = cmd!(sh, "rustc +{toolchain} --print sysroot").read()?.into(); - self.sysroot = Some(sysroot.clone()); - Ok(sysroot) + if auto_clippy { + Self::clippy(vec![])?; } - } - fn install_to_dir( - &mut self, - sh: &Shell, - path: PathBuf, - args: impl IntoIterator>, - ) -> Result<()> { - let sysroot = self.sysroot()?; - let toolchain = &self.active_toolchain; - let extra_flags = &self.cargo_extra_flags; - // "--locked" to respect the Cargo.lock file if it exists. - // Install binaries to the miri toolchain's sysroot so they do not interact with other toolchains. - cmd!(sh, "cargo +{toolchain} install {extra_flags...} --path {path} --force --root {sysroot} {args...}").run()?; + Ok(()) } -} -impl MiriRunner<'_> { - fn bench(&self, benches: &[OsString]) -> Result<()> { - // The hyperfine to use - let hyperfine = std::env::var("HYPERFINE"); - let hyperfine = hyperfine.as_deref().unwrap_or("hyperfine -w 1 -m 5 --shell=none"); - let hyperfine = shell_words::split(hyperfine).unwrap(); - let Some((program_name, args)) = hyperfine.split_first() else { - bail!("Expected HYPERFINE environment variable to be non-empty"); - }; - // Make sure we have an up-to-date Miri installed - Self::exec_inner(&Subcommands::Install { flags: vec![] }, false)?; - let benches_dir = path!(self.miri_dir / "bench-cargo-miri"); - let benches = if benches.is_empty() { - std::fs::read_dir(&benches_dir)? - .filter_map(|path| { - path.ok() - .filter(|dir| dir.file_type().map(|t| t.is_dir()).unwrap_or(false)) - .map(|p| p.file_name()) - }) - .collect() - } else { - benches.to_owned() - }; - let sh = shell_with_parent_env()?; - let toolchain = &self.active_toolchain; - // Run the requested benchmarks - for bench in benches { - let current_bench_dir = path!(benches_dir / bench / "Cargo.toml"); - cmd!( - sh, - "{program_name} {args...} 'cargo +'{toolchain}' miri run --manifest-path \"'{current_bench_dir}'\"'" - ) - .run()?; + pub fn exec(self) -> Result<()> { + match self { + Command::Install { flags } => Self::install(flags), + Command::Build { flags } => Self::build(flags), + Command::Check { flags } => Self::check(flags), + Command::Test { bless, flags } => Self::test(bless, flags), + Command::Run { dep, flags } => Self::run(dep, flags), + Command::Fmt { flags } => Self::fmt(flags), + Command::Clippy { flags } => Self::clippy(flags), + Command::Cargo { flags } => Self::cargo(flags), + Command::ManySeeds { command, seed_start, seeds } => + Self::many_seeds(command, seed_start, seeds), + Command::Bench { benches } => Self::bench(benches), + Command::Toolchain { flags } => Self::toolchain(flags), + Command::RustcPull { commit } => Self::rustc_pull(commit.clone()), + Command::RustcPush { rustc_git, github_user, branch } => + Self::rustc_push(rustc_git, github_user, branch), } - Ok(()) } - fn toolchain(&self, flags: &[OsString]) -> Result<()> { + fn toolchain(flags: Vec) -> Result<()> { // Make sure rustup-toolchain-install-master is installed. - which::which("rustup-toolchain-install-master").context("Please install rustup-toolchain-install-master by running 'cargo install rustup-toolchain-install-master'")?; - let sh = shell_with_parent_env()?; - sh.change_dir(&self.miri_dir); + which::which("rustup-toolchain-install-master") + .context("Please install rustup-toolchain-install-master by running 'cargo install rustup-toolchain-install-master'")?; + let sh = shell()?; + sh.change_dir(miri_dir()?); let new_commit = Some(sh.read_file("rust-version")?.trim().to_owned()); let current_commit = { let rustc_info = cmd!(sh, "rustc +miri --version -v").read(); @@ -318,8 +111,9 @@ impl MiriRunner<'_> { }; // Check if we already are at that commit. if current_commit == new_commit { - println!("miri toolchain is already at commit {}.", current_commit.unwrap()); - cmd!(sh, "rustup override set miri").run()?; + if active_toolchain()? != "miri" { + cmd!(sh, "rustup override set miri").run()?; + } return Ok(()); } // Install and setup new toolchain. @@ -337,10 +131,10 @@ impl MiriRunner<'_> { Ok(()) } - fn rustc_pull(&self, commit: Option) -> Result<()> { - let sh = shell_with_parent_env()?; - sh.change_dir(&self.miri_dir); - let commit: String = commit.map(Result::Ok).unwrap_or_else(|| { + fn rustc_pull(commit: Option) -> Result<()> { + let sh = shell()?; + sh.change_dir(miri_dir()?); + let commit = commit.map(Result::Ok).unwrap_or_else(|| { let rust_repo_head = cmd!(sh, "git ls-remote https://github.com/rust-lang/rust/ HEAD").read()?; rust_repo_head @@ -349,85 +143,109 @@ impl MiriRunner<'_> { .map(|front| front.trim().to_owned()) .ok_or_else(|| anyhow!("Could not obtain Rust repo HEAD from remote.")) })?; + // Make sure the repo is clean. + if cmd!(sh, "git status --untracked-files=no --porcelain").read()?.is_empty().not() { + bail!("working directory must be clean before running `./miri rustc-pull`"); + } + // Update rust-version file. As a separate commit, since making it part of // the merge has confused the heck out of josh in the past. - sh.write_file(path!(self.miri_dir / "rust-version"), &commit)?; + // We pass `--no-verify` to avoid running git hooks like `./miri fmt` that could in turn + // trigger auto-actions. + sh.write_file("rust-version", &commit)?; const PREPARING_COMMIT_MESSAGE: &str = "Preparing for merge from rustc"; - cmd!(sh, "git commit rust-version -m {PREPARING_COMMIT_MESSAGE}") + cmd!(sh, "git commit rust-version --no-verify -m {PREPARING_COMMIT_MESSAGE}") .run() .context("FAILED to commit rust-version file, something went wrong")?; - // Fetch given rustc commit and note down which one that was + + // Fetch given rustc commit. cmd!(sh, "git fetch http://localhost:8000/rust-lang/rust.git@{commit}{JOSH_FILTER}.git") .run() - .context("FAILED to fetch new commits, something went wrong")?; + .map_err(|e| { + // Try to un-do the previous `git commit`, to leave the repo in the state we found it it. + cmd!(sh, "git reset --hard HEAD^") + .run() + .expect("FAILED to clean up again after failed `git fetch`, sorry for that"); + e + }) + .context("FAILED to fetch new commits, something went wrong (committing the rust-version file has been undone)")?; + + // Merge the fetched commit. const MERGE_COMMIT_MESSAGE: &str = "Merge from rustc"; - cmd!(sh, "git merge FETCH_HEAD --no-ff -m {MERGE_COMMIT_MESSAGE}") + cmd!(sh, "git merge FETCH_HEAD --no-verify --no-ff -m {MERGE_COMMIT_MESSAGE}") .run() .context("FAILED to merge new commits, something went wrong")?; Ok(()) } - fn rustc_push(&self, github_user: &str, branch: &str) -> Result<()> { - let rustc_git = std::env::var_os("RUSTC_GIT"); - let working_directory = if let Some(rustc_git) = rustc_git { - rustc_git - } else { + fn rustc_push(rustc_git: Option, github_user: String, branch: String) -> Result<()> { + let sh = shell()?; + sh.change_dir(miri_dir()?); + let base = sh.read_file("rust-version")?.trim().to_owned(); + // Make sure the repo is clean. + if cmd!(sh, "git status --untracked-files=no --porcelain").read()?.is_empty().not() { + bail!("working directory must be clean before running `./miri rustc-push`"); + } + + // Find a repo we can do our preparation in. + if let Some(rustc_git) = rustc_git { // If rustc_git is `Some`, we'll use an existing fork for the branch updates. + sh.change_dir(rustc_git); + } else { // Otherwise, do this in the local Miri repo. println!( "This will pull a copy of the rust-lang/rust history into this Miri checkout, growing it by about 1GB." ); - println!( - "To avoid that, abort now and set the RUSTC_GIT environment variable to an existing rustc checkout. Proceed? [y/N] " + print!( + "To avoid that, abort now and set the `--rustc-git` flag to an existing rustc checkout. Proceed? [y/N] " ); + std::io::stdout().flush()?; let mut answer = String::new(); std::io::stdin().read_line(&mut answer)?; if answer.trim().to_lowercase() != "y" { std::process::exit(1); } - self.miri_dir.clone().into() }; // Prepare the branch. Pushing works much better if we use as base exactly // the commit that we pulled from last time, so we use the `rust-version` // file as a good approximation of that. - let rust_version_path = path!(self.miri_dir / "rust-version"); - let base = std::fs::read_to_string(rust_version_path)?.trim().to_owned(); - println!("Preparing {github_user}/rust (base: {base})...)"); - let sh = shell_with_parent_env()?; - sh.change_dir(working_directory); - - if cmd!(sh, "git fetch https://github.com/{github_user}").read().is_ok() { + println!("Preparing {github_user}/rust (base: {base})..."); + if cmd!(sh, "git fetch https://github.com/{github_user}/rust {branch}") + .ignore_stderr() + .read() + .is_ok() + { println!( - "The branch '{branch}' seems to already exist in 'https://github.com/{github_user}'. Please delete it and try again." + "The branch '{branch}' seems to already exist in 'https://github.com/{github_user}/rust'. Please delete it and try again." ); std::process::exit(1); } - cmd!(sh, "git fetch https://github.com/rust-lang/rust {base}").run()?; - cmd!(sh, "git push https://github.com/{github_user}/rust {base}:refs/heads/{branch}") .run()?; println!(); + // Do the actual push. - sh.change_dir(&self.miri_dir); + sh.change_dir(miri_dir()?); println!("Pushing miri changes..."); cmd!( sh, "git push http://localhost:8000/{github_user}/rust.git{JOSH_FILTER}.git HEAD:{branch}" ) .run()?; - // Do a round-trip check to make sure the push worked as expected. println!(); + + // Do a round-trip check to make sure the push worked as expected. cmd!( sh, "git fetch http://localhost:8000/{github_user}/rust.git{JOSH_FILTER}.git {branch}" ) + .ignore_stderr() .read()?; let head = cmd!(sh, "git rev-parse HEAD").read()?; let fetch_head = cmd!(sh, "git rev-parse FETCH_HEAD").read()?; if head != fetch_head { - println!("ERROR: Josh created a non-roundtrip push! Do NOT merge this into rustc!"); - std::process::exit(1); + bail!("Josh created a non-roundtrip push! Do NOT merge this into rustc!"); } println!( "Confirmed that the push round-trips back to Miri properly. Please create a rustc PR:" @@ -436,194 +254,196 @@ impl MiriRunner<'_> { Ok(()) } - fn install(&mut self, flags: &[OsString]) -> Result<()> { - let sh = self.shell()?; - self.install_to_dir(&sh, self.miri_dir.clone(), flags)?; - let cargo_miri_dir = path!(self.miri_dir / "cargo-miri"); - self.install_to_dir(&sh, cargo_miri_dir, flags)?; + fn many_seeds(command: Vec, seed_start: u64, seed_count: u64) -> Result<()> { + let seed_end = seed_start + seed_count; + let Some((command_name, trailing_args)) = command.split_first() else { + bail!("expected many-seeds command to be non-empty"); + }; + let sh = shell()?; + for seed in seed_start..seed_end { + println!("Trying seed: {seed}"); + let mut miriflags = env::var_os("MIRIFLAGS").unwrap_or_default(); + miriflags.push(format!(" -Zlayout-seed={seed} -Zmiri-seed={seed}")); + let status = cmd!(sh, "{command_name} {trailing_args...}") + .env("MIRIFLAGS", miriflags) + .quiet() + .run(); + if status.is_err() { + println!("Failing seed: {seed}"); + break; + } + } Ok(()) } - fn build(&self, flags: &[OsString]) -> Result<()> { - // Build, and let caller control flags. - let miri_manifest = path!(self.miri_dir / "Cargo.toml"); - let cargo_miri_manifest = path!(self.miri_dir / "cargo-miri" / "Cargo.toml"); - Self::build_package( - miri_manifest.as_ref(), - &self.active_toolchain, - &self.cargo_extra_flags, - flags, - )?; - Self::build_package( - cargo_miri_manifest.as_ref(), - &self.active_toolchain, - &self.cargo_extra_flags, - flags, - )?; + fn bench(benches: Vec) -> Result<()> { + // The hyperfine to use + let hyperfine = env::var("HYPERFINE"); + let hyperfine = hyperfine.as_deref().unwrap_or("hyperfine -w 1 -m 5 --shell=none"); + let hyperfine = shell_words::split(hyperfine)?; + let Some((program_name, args)) = hyperfine.split_first() else { + bail!("expected HYPERFINE environment variable to be non-empty"); + }; + // Make sure we have an up-to-date Miri installed and selected the right toolchain. + Self::install(vec![])?; + + let sh = shell()?; + sh.change_dir(miri_dir()?); + let benches_dir = "bench-cargo-miri"; + let benches = if benches.is_empty() { + sh.read_dir(benches_dir)? + .into_iter() + .filter(|path| path.is_dir()) + .map(Into::into) + .collect() + } else { + benches.to_owned() + }; + // Run the requested benchmarks + for bench in benches { + let current_bench = path!(benches_dir / bench / "Cargo.toml"); + // We don't attempt to escape `current_bench`, but we wrap it in quotes. + // That seems to make Windows CI happy. + cmd!( + sh, + "{program_name} {args...} 'cargo miri run --manifest-path \"'{current_bench}'\"'" + ) + .run()?; + } Ok(()) } - fn check(&self, flags: &[OsString]) -> Result<()> { - fn check_package( - // Path to Cargo.toml file of a package to check. - path: &OsStr, - toolchain: impl AsRef, - extra_flags: &[String], - all_targets: bool, - args: impl IntoIterator>, - ) -> Result<()> { - let all_targets: Option<&OsStr> = all_targets.then_some("--all-targets".as_ref()); - let sh = Shell::new()?; - cmd!(sh, "cargo +{toolchain} check {extra_flags...} --manifest-path {path} {all_targets...} {args...}").run()?; - Ok(()) - } - // Check, and let caller control flags. - let miri_manifest = path!(self.miri_dir / "Cargo.toml"); - let cargo_miri_manifest = path!(self.miri_dir / "cargo-miri" / "Cargo.toml"); - check_package( - miri_manifest.as_ref(), - &self.active_toolchain, - &self.cargo_extra_flags, - true, - flags, - )?; - check_package( - cargo_miri_manifest.as_ref(), - &self.active_toolchain, - &self.cargo_extra_flags, - false, - flags, - )?; + fn install(flags: Vec) -> Result<()> { + Self::auto_actions()?; + let e = MiriEnv::new()?; + e.install_to_sysroot(e.miri_dir.clone(), &flags)?; + e.install_to_sysroot(path!(e.miri_dir / "cargo-miri"), &flags)?; + Ok(()) + } + + fn build(flags: Vec) -> Result<()> { + Self::auto_actions()?; + let e = MiriEnv::new()?; + e.build(path!(e.miri_dir / "Cargo.toml"), &flags, /* quiet */ false)?; + e.build(path!(e.miri_dir / "cargo-miri" / "Cargo.toml"), &flags, /* quiet */ false)?; + Ok(()) + } + + fn check(flags: Vec) -> Result<()> { + Self::auto_actions()?; + let e = MiriEnv::new()?; + e.check(path!(e.miri_dir / "Cargo.toml"), &flags)?; + e.check(path!(e.miri_dir / "cargo-miri" / "Cargo.toml"), &flags)?; + Ok(()) + } + + fn clippy(flags: Vec) -> Result<()> { + Self::auto_actions()?; + let e = MiriEnv::new()?; + e.clippy(path!(e.miri_dir / "Cargo.toml"), &flags)?; + e.clippy(path!(e.miri_dir / "cargo-miri" / "Cargo.toml"), &flags)?; + e.clippy(path!(e.miri_dir / "miri-script" / "Cargo.toml"), &flags)?; + Ok(()) + } + + fn cargo(flags: Vec) -> Result<()> { + Self::auto_actions()?; + let e = MiriEnv::new()?; + let toolchain = &e.toolchain; + // We carefully kept the working dir intact, so this will run cargo *on the workspace in the + // current working dir*, not on the main Miri workspace. That is exactly what RA needs. + cmd!(e.sh, "cargo +{toolchain} {flags...}").run()?; Ok(()) } - fn test(&mut self, bless: bool, flags: &[OsString]) -> Result<()> { - let miri_manifest = path!(self.miri_dir / "Cargo.toml"); - // First build and get a sysroot. - Self::build_package( - miri_manifest.as_ref(), - &self.active_toolchain, - &self.cargo_extra_flags, - std::iter::empty::(), - )?; - self.find_miri_sysroot()?; - let extra_flags = &self.cargo_extra_flags; + fn test(bless: bool, flags: Vec) -> Result<()> { + Self::auto_actions()?; + let mut e = MiriEnv::new()?; + // First build, and get a sysroot. + e.build(path!(e.miri_dir / "Cargo.toml"), &[], /* quiet */ true)?; + e.build_miri_sysroot()?; + // Then test, and let caller control flags. // Only in root project as `cargo-miri` has no tests. - let sh = self.shell()?; if bless { - sh.set_var("MIRI_BLESS", "Gesundheit"); + e.sh.set_var("RUSTC_BLESS", "Gesundheit"); } - let toolchain: &OsStr = self.active_toolchain.as_ref(); - cmd!( - sh, - "cargo +{toolchain} test {extra_flags...} --manifest-path {miri_manifest} -- {flags...}" - ) - .run()?; + e.test(path!(e.miri_dir / "Cargo.toml"), &flags)?; Ok(()) } - fn run(&mut self, dep: bool, flags: &[OsString]) -> Result<()> { - use itertools::Itertools; + fn run(dep: bool, flags: Vec) -> Result<()> { + Self::auto_actions()?; + let mut e = MiriEnv::new()?; // Scan for "--target" to overwrite the "MIRI_TEST_TARGET" env var so // that we set the MIRI_SYSROOT up the right way. + use itertools::Itertools; let target = flags.iter().tuple_windows().find(|(first, _)| first == &"--target"); if let Some((_, target)) = target { // Found it! - self.set_env("MIRI_TEST_TARGET", target); - } else if let Some(var) = - std::env::var_os("MIRI_TEST_TARGET").filter(|target| !target.is_empty()) - { + e.sh.set_var("MIRI_TEST_TARGET", target); + } else if let Ok(target) = std::env::var("MIRI_TEST_TARGET") { // Make sure miri actually uses this target. - let entry = self.env.entry("MIRIFLAGS".into()).or_default(); - entry.push(" --target "); - entry.push(var); + let miriflags = e.sh.var("MIRIFLAGS").unwrap_or_default(); + e.sh.set_var("MIRIFLAGS", format!("{miriflags} --target {target}")); } - // First build and get a sysroot. - let miri_manifest = path!(self.miri_dir / "Cargo.toml"); - Self::build_package( - miri_manifest.as_ref(), - &self.active_toolchain, - &self.cargo_extra_flags, - std::iter::empty::(), - )?; - self.find_miri_sysroot()?; + // First build, and get a sysroot. + let miri_manifest = path!(e.miri_dir / "Cargo.toml"); + e.build(&miri_manifest, &[], /* quiet */ true)?; + e.build_miri_sysroot()?; + // Then run the actual command. - let miri_flags = self.env.get(&OsString::from("MIRIFLAGS")).cloned().unwrap_or_default(); - let miri_flags: &OsStr = miri_flags.as_ref(); - let extra_flags = &self.cargo_extra_flags; - let sh = self.shell()?; - let toolchain: &OsStr = self.active_toolchain.as_ref(); + let miri_flags = e.sh.var("MIRIFLAGS").unwrap_or_default(); + let miri_flags = flagsplit(&miri_flags); + let toolchain = &e.toolchain; + let extra_flags = &e.cargo_extra_flags; if dep { cmd!( - sh, - "cargo +{toolchain} --quiet test --test compiletest {extra_flags...} --manifest-path {miri_manifest} -- --miri-run-dep-mode {miri_flags} {flags...}" - ).run()?; + e.sh, + "cargo +{toolchain} --quiet test --test compiletest {extra_flags...} --manifest-path {miri_manifest} -- --miri-run-dep-mode {miri_flags...} {flags...}" + ).quiet().run()?; } else { cmd!( - sh, - "cargo +{toolchain} --quiet run {extra_flags...} --manifest-path {miri_manifest} -- {miri_flags} {flags...}" - ).run()?; + e.sh, + "cargo +{toolchain} --quiet run {extra_flags...} --manifest-path {miri_manifest} -- {miri_flags...} {flags...}" + ).quiet().run()?; } Ok(()) } - fn fmt(&self, flags: &[OsString]) -> Result<()> { - let toolchain = &self.active_toolchain; - let config_path = path!(self.miri_dir / "rustfmt.toml"); - let sh = self.shell()?; - for item in WalkDir::new(&self.miri_dir).into_iter().filter_entry(|entry| { - let name: String = entry.file_name().to_string_lossy().into(); + fn fmt(flags: Vec) -> Result<()> { + Self::auto_actions()?; + let e = MiriEnv::new()?; + let toolchain = &e.toolchain; + let config_path = path!(e.miri_dir / "rustfmt.toml"); + + let mut cmd = cmd!( + e.sh, + "rustfmt +{toolchain} --edition=2021 --config-path {config_path} {flags...}" + ); + eprintln!("$ {cmd} ..."); + + // Add all the filenames to the command. + // FIXME: `rustfmt` will follow the `mod` statements in these files, so we get a bunch of + // duplicate diffs. + for item in WalkDir::new(&e.miri_dir).into_iter().filter_entry(|entry| { + let name = entry.file_name().to_string_lossy(); let ty = entry.file_type(); if ty.is_file() { name.ends_with(".rs") } else { - // dir or symlink - &name != "target" + // dir or symlink. skip `target` and `.git`. + &name != "target" && &name != ".git" } }) { - let item = item.unwrap(); // Should never panic as we've already filtered out failed entries. + let item = item?; if item.file_type().is_file() { - let path = item.path(); - cmd!(sh, "rustfmt +{toolchain} --edition=2021 --config-path {config_path} {flags...} {path}").quiet().run()?; + cmd = cmd.arg(item.into_path()); } } - Ok(()) - } - - fn clippy(&self, flags: &[OsString]) -> Result<()> { - let toolchain_modifier = &self.active_toolchain; - let extra_flags = &self.cargo_extra_flags; - let miri_manifest = path!(self.miri_dir / "Cargo.toml"); - let sh = self.shell()?; - cmd!(sh, "cargo +{toolchain_modifier} clippy {extra_flags...} --manifest-path {miri_manifest} --all-targets -- {flags...}").run()?; - Ok(()) - } - fn cargo(&self, flags: &[OsString]) -> Result<()> { - // We carefully kept the working dir intact, so this will run cargo *on the workspace in the - // current working dir*, not on the main Miri workspace. That is exactly what RA needs. - let toolchain_modifier = &self.active_toolchain; - let sh = self.shell()?; - cmd!(sh, "cargo +{toolchain_modifier} {flags...}").run()?; - Ok(()) - } - fn many_seeds(&self, command: &[OsString], seed_start: u64, seed_count: u64) -> Result<()> { - let seed_end = seed_start + seed_count; - assert!(!command.is_empty()); - let (command_name, trailing_args) = command.split_first().unwrap(); - let sh = shell_with_parent_env()?; - for seed in seed_start..seed_end { - println!("Trying seed: {seed}"); - let mut miriflags = std::env::var_os("MIRIFLAGS").unwrap_or_default(); - miriflags.push(format!(" -Zlayout-seed={seed} -Zmiri-seed={seed}")); - let status = - cmd!(sh, "{command_name} {trailing_args...}").env("MIRIFLAGS", miriflags).run(); - if status.is_err() { - println!("Failing seed: {seed}"); - break; - } - } + // We want our own error message, repeating the command is too much. + cmd.quiet().run().map_err(|_| anyhow!("`rustfmt` failed"))?; Ok(()) } } diff --git a/src/tools/miri/miri-script/src/main.rs b/src/tools/miri/miri-script/src/main.rs index dbee923d48f8a..ce00de5ac542d 100644 --- a/src/tools/miri/miri-script/src/main.rs +++ b/src/tools/miri/miri-script/src/main.rs @@ -1,58 +1,120 @@ -pub(crate) mod arg; mod commands; +mod util; -use std::path::Path; +use std::ffi::OsString; use anyhow::Result; -use clap::Parser; -use path_macro::path; +use clap::{Parser, Subcommand}; -use arg::Subcommands; - -struct AutoConfig { - toolchain: bool, - fmt: bool, - clippy: bool, +#[derive(Parser, Clone, Debug)] +#[command(author, about, long_about = None)] +pub struct Cli { + #[command(subcommand)] + pub command: Command, } -impl Subcommands { - fn run_auto_things(&self) -> bool { - use Subcommands::*; - match self { - // Early commands, that don't do auto-things and don't want the environment-altering things happening below. - Toolchain { .. } - | RustcPull { .. } - | RustcPush { .. } - | ManySeeds { .. } - | Bench { .. } => false, - Install { .. } - | Check { .. } - | Build { .. } - | Test { .. } - | Run { .. } - | Fmt { .. } - | Clippy { .. } - | Cargo { .. } => true, - } - } - fn get_config(&self, miri_dir: &Path) -> Option { - let skip_auto_ops = std::env::var_os("MIRI_AUTO_OPS").is_some(); - if !self.run_auto_things() { - return None; - } - if skip_auto_ops { - return Some(AutoConfig { toolchain: false, fmt: false, clippy: false }); - } - - let auto_everything = path!(miri_dir / ".auto_everything").exists(); - let toolchain = auto_everything || path!(miri_dir / ".auto-toolchain").exists(); - let fmt = auto_everything || path!(miri_dir / ".auto-fmt").exists(); - let clippy = auto_everything || path!(miri_dir / ".auto-clippy").exists(); - Some(AutoConfig { toolchain, fmt, clippy }) - } +#[derive(Subcommand, Clone, Debug)] +pub enum Command { + /// Installs the miri driver and cargo-miri. + /// Sets up the rpath such that the installed binary should work in any + /// working directory. Note that the binaries are placed in the `miri` toolchain + /// sysroot, to prevent conflicts with other toolchains. + Install { + /// Flags that are passed through to `cargo install`. + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + flags: Vec, + }, + /// Just build miri. + Build { + /// Flags that are passed through to `cargo build`. + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + flags: Vec, + }, + /// Just check miri. + Check { + /// Flags that are passed through to `cargo check`. + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + flags: Vec, + }, + /// Build miri, set up a sysroot and then run the test suite. + Test { + #[arg(long, default_value_t = false)] + bless: bool, + /// Flags that are passed through to `cargo test`. + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + flags: Vec, + }, + /// Build miri, set up a sysroot and then run the driver with the given . + /// (Also respects MIRIFLAGS environment variable.) + Run { + #[arg(long, default_value_t = false)] + dep: bool, + /// Flags that are passed through to `miri` + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + flags: Vec, + }, + /// Format all sources and tests. + Fmt { + /// Flags that are passed through to `rustfmt`. + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + flags: Vec, + }, + /// Runs clippy on all sources. + Clippy { + /// Flags that are passed through to `cargo clippy`. + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + flags: Vec, + }, + /// Runs just `cargo ` with the Miri-specific environment variables. + /// Mainly meant to be invoked by rust-analyzer. + Cargo { + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + flags: Vec, + }, + /// Runs over and over again with different seeds for Miri. The MIRIFLAGS + /// variable is set to its original value appended with ` -Zmiri-seed=$SEED` for + /// many different seeds. + ManySeeds { + /// Starting seed. + #[arg(long, env = "MIRI_SEED_START", default_value_t = 0)] + seed_start: u64, + /// Amount of seeds to try. + #[arg(long, env = "MIRI_SEEDS", default_value_t = 256)] + seeds: u64, + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + command: Vec, + }, + /// Runs the benchmarks from bench-cargo-miri in hyperfine. hyperfine needs to be installed. + Bench { + /// List of benchmarks to run. By default all benchmarks are run. + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + benches: Vec, + }, + /// Update and activate the rustup toolchain 'miri' to the commit given in the + /// `rust-version` file. + /// `rustup-toolchain-install-master` must be installed for this to work. Any extra + /// flags are passed to `rustup-toolchain-install-master`. + Toolchain { + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + flags: Vec, + }, + /// Pull and merge Miri changes from the rustc repo. Defaults to fetching the latest + /// rustc commit. The fetched commit is stored in the `rust-version` file, so the + /// next `./miri toolchain` will install the rustc that just got pulled. + RustcPull { commit: Option }, + /// Push Miri changes back to the rustc repo. This will pull a copy of the rustc + /// history into the Miri repo, unless you set the RUSTC_GIT env var to an existing + /// clone of the rustc repo. + RustcPush { + #[arg(long, env = "RUSTC_GIT")] + rustc_git: Option, + github_user: String, + branch: String, + }, } + fn main() -> Result<()> { - let args = arg::Cli::parse(); - commands::MiriRunner::exec(&args.commands)?; + let args = Cli::parse(); + args.command.exec()?; Ok(()) } diff --git a/src/tools/miri/miri-script/src/util.rs b/src/tools/miri/miri-script/src/util.rs new file mode 100644 index 0000000000000..caebd88b14b7a --- /dev/null +++ b/src/tools/miri/miri-script/src/util.rs @@ -0,0 +1,153 @@ +use std::ffi::{OsStr, OsString}; +use std::path::PathBuf; + +use anyhow::{Context, Result}; +use dunce::canonicalize; +use path_macro::path; +use xshell::{cmd, Shell}; + +pub fn miri_dir() -> std::io::Result { + const MIRI_SCRIPT_ROOT_DIR: &str = env!("CARGO_MANIFEST_DIR"); + Ok(canonicalize(MIRI_SCRIPT_ROOT_DIR)?.parent().unwrap().into()) +} + +/// Queries the active toolchain for the Miri dir. +pub fn active_toolchain() -> Result { + let sh = shell()?; + sh.change_dir(miri_dir()?); + let stdout = cmd!(sh, "rustup show active-toolchain").read()?; + Ok(stdout.split_whitespace().next().context("Could not obtain active Rust toolchain")?.into()) +} + +pub fn shell() -> Result { + let sh = Shell::new()?; + // xshell does not propagate parent's env variables by default. + for (k, v) in std::env::vars_os() { + sh.set_var(k, v); + } + Ok(sh) +} + +pub fn flagsplit(flags: &str) -> Vec { + // This code is taken from `RUSTFLAGS` handling in cargo. + flags.split(' ').map(str::trim).filter(|s| !s.is_empty()).map(str::to_string).collect() +} + +/// Some extra state we track for building Miri, such as the right RUSTFLAGS. +pub struct MiriEnv { + /// miri_dir is the root of the miri repository checkout we are working in. + pub miri_dir: PathBuf, + /// active_toolchain is passed as `+toolchain` argument to cargo/rustc invocations. + pub toolchain: String, + /// Extra flags to pass to cargo. + pub cargo_extra_flags: Vec, + /// The rustc sysroot + pub sysroot: PathBuf, + /// The shell we use. + pub sh: Shell, +} + +impl MiriEnv { + pub fn new() -> Result { + let sh = shell()?; + let toolchain = active_toolchain()?; + let miri_dir = miri_dir()?; + + let sysroot = cmd!(sh, "rustc +{toolchain} --print sysroot").read()?.into(); + let target_output = cmd!(sh, "rustc +{toolchain} --version --verbose").read()?; + let rustc_meta = rustc_version::version_meta_for(&target_output)?; + let libdir = path!(sysroot / "lib" / "rustlib" / rustc_meta.host / "lib"); + + // Determine some toolchain properties + if !libdir.exists() { + println!("Something went wrong determining the library dir."); + println!("I got {} but that does not exist.", libdir.display()); + println!("Please report a bug at https://github.com/rust-lang/miri/issues."); + std::process::exit(2); + } + // Share target dir between `miri` and `cargo-miri`. + let target_dir = std::env::var_os("CARGO_TARGET_DIR") + .unwrap_or_else(|| path!(miri_dir / "target").into()); + sh.set_var("CARGO_TARGET_DIR", target_dir); + + // We configure dev builds to not be unusably slow. + let devel_opt_level = + std::env::var_os("CARGO_PROFILE_DEV_OPT_LEVEL").unwrap_or_else(|| "2".into()); + sh.set_var("CARGO_PROFILE_DEV_OPT_LEVEL", devel_opt_level); + + // Compute rustflags. + let rustflags = { + let env = std::env::var_os("RUSTFLAGS"); + let mut flags_with_warnings = OsString::from( + "-Zunstable-options -Wrustc::internal -Wrust_2018_idioms -Wunused_lifetimes -Wsemicolon_in_expressions_from_macros ", + ); + if let Some(value) = env { + flags_with_warnings.push(value); + } + // We set the rpath so that Miri finds the private rustc libraries it needs. + let mut flags_with_compiler_settings = OsString::from("-C link-args=-Wl,-rpath,"); + flags_with_compiler_settings.push(&libdir); + flags_with_compiler_settings.push(flags_with_warnings); + flags_with_compiler_settings + }; + sh.set_var("RUSTFLAGS", rustflags); + + // Get extra flags for cargo. + let cargo_extra_flags = std::env::var("CARGO_EXTRA_FLAGS").unwrap_or_default(); + let cargo_extra_flags = flagsplit(&cargo_extra_flags); + + Ok(MiriEnv { miri_dir, toolchain, sh, sysroot, cargo_extra_flags }) + } + + pub fn install_to_sysroot( + &self, + path: impl AsRef, + args: impl IntoIterator>, + ) -> Result<()> { + let MiriEnv { sysroot, toolchain, cargo_extra_flags, .. } = self; + // Install binaries to the miri toolchain's `sysroot` so they do not interact with other toolchains. + cmd!(self.sh, "cargo +{toolchain} install {cargo_extra_flags...} --path {path} --force --root {sysroot} {args...}").run()?; + Ok(()) + } + + pub fn build( + &self, + manifest_path: impl AsRef, + args: &[OsString], + quiet: bool, + ) -> Result<()> { + let MiriEnv { toolchain, cargo_extra_flags, .. } = self; + let quiet_flag = if quiet { Some("--quiet") } else { None }; + let mut cmd = cmd!( + self.sh, + "cargo +{toolchain} build {cargo_extra_flags...} --manifest-path {manifest_path} {quiet_flag...} {args...}" + ); + cmd.set_quiet(quiet); + cmd.run()?; + Ok(()) + } + + pub fn check(&self, manifest_path: impl AsRef, args: &[OsString]) -> Result<()> { + let MiriEnv { toolchain, cargo_extra_flags, .. } = self; + cmd!(self.sh, "cargo +{toolchain} check {cargo_extra_flags...} --manifest-path {manifest_path} --all-targets {args...}") + .run()?; + Ok(()) + } + + pub fn clippy(&self, manifest_path: impl AsRef, args: &[OsString]) -> Result<()> { + let MiriEnv { toolchain, cargo_extra_flags, .. } = self; + cmd!(self.sh, "cargo +{toolchain} clippy {cargo_extra_flags...} --manifest-path {manifest_path} --all-targets {args...}") + .run()?; + Ok(()) + } + + pub fn test(&self, manifest_path: impl AsRef, args: &[OsString]) -> Result<()> { + let MiriEnv { toolchain, cargo_extra_flags, .. } = self; + cmd!( + self.sh, + "cargo +{toolchain} test {cargo_extra_flags...} --manifest-path {manifest_path} {args...}" + ) + .run()?; + Ok(()) + } +} From efbd2a508c3acd1dcf722808f118341e48bea9cc Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sun, 30 Jul 2023 22:58:03 +0200 Subject: [PATCH 06/30] port to hand-rolled parser, since clap doesn't behave just the right way --- src/tools/miri/miri-script/Cargo.lock | 299 --------------------- src/tools/miri/miri-script/Cargo.toml | 1 - src/tools/miri/miri-script/src/commands.rs | 20 +- src/tools/miri/miri-script/src/main.rs | 184 +++++++++---- 4 files changed, 150 insertions(+), 354 deletions(-) diff --git a/src/tools/miri/miri-script/Cargo.lock b/src/tools/miri/miri-script/Cargo.lock index be6eea0ed5d98..cf6062d7d7f58 100644 --- a/src/tools/miri/miri-script/Cargo.lock +++ b/src/tools/miri/miri-script/Cargo.lock @@ -2,121 +2,12 @@ # It is not intended for manual editing. version = 3 -[[package]] -name = "anstream" -version = "0.3.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0ca84f3628370c59db74ee214b3263d58f9aadd9b4fe7e711fd87dc452b7f163" -dependencies = [ - "anstyle", - "anstyle-parse", - "anstyle-query", - "anstyle-wincon", - "colorchoice", - "is-terminal", - "utf8parse", -] - -[[package]] -name = "anstyle" -version = "1.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "41ed9a86bf92ae6580e0a31281f65a1b1d867c0cc68d5346e2ae128dddfa6a7d" - -[[package]] -name = "anstyle-parse" -version = "0.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e765fd216e48e067936442276d1d57399e37bce53c264d6fefbe298080cb57ee" -dependencies = [ - "utf8parse", -] - -[[package]] -name = "anstyle-query" -version = "1.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5ca11d4be1bab0c8bc8734a9aa7bf4ee8316d462a08c6ac5052f888fef5b494b" -dependencies = [ - "windows-sys", -] - -[[package]] -name = "anstyle-wincon" -version = "1.0.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "180abfa45703aebe0093f79badacc01b8fd4ea2e35118747e5811127f926e188" -dependencies = [ - "anstyle", - "windows-sys", -] - [[package]] name = "anyhow" version = "1.0.71" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9c7d0618f0e0b7e8ff11427422b64564d5fb0be1940354bfe2e0529b18a9d9b8" -[[package]] -name = "bitflags" -version = "1.3.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" - -[[package]] -name = "cc" -version = "1.0.79" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "50d30906286121d95be3d479533b458f87493b30a4b5f79a607db8f5d11aa91f" - -[[package]] -name = "clap" -version = "4.2.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "34d21f9bf1b425d2968943631ec91202fe5e837264063503708b83013f8fc938" -dependencies = [ - "clap_builder", - "clap_derive", - "once_cell", -] - -[[package]] -name = "clap_builder" -version = "4.2.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "914c8c79fb560f238ef6429439a30023c862f7a28e688c58f7203f12b29970bd" -dependencies = [ - "anstream", - "anstyle", - "bitflags", - "clap_lex", - "strsim", -] - -[[package]] -name = "clap_derive" -version = "4.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3f9644cd56d6b87dbe899ef8b053e331c0637664e9e21a33dfcdc36093f5c5c4" -dependencies = [ - "heck", - "proc-macro2", - "quote", - "syn", -] - -[[package]] -name = "clap_lex" -version = "0.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8a2dd5a6fe8c6e3502f568a6353e5273bbb15193ad9a89e457b9970798efbea1" - -[[package]] -name = "colorchoice" -version = "1.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "acbf1af155f9b9ef647e42cdc158db4b64a1b61f743629225fde6f3e0be2a7c7" - [[package]] name = "dunce" version = "1.0.4" @@ -129,62 +20,6 @@ version = "1.8.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7fcaabb2fef8c910e7f4c7ce9f67a1283a1715879a7c230ca9d6d1ae31f16d91" -[[package]] -name = "errno" -version = "0.3.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4bcfec3a70f97c962c307b2d2c56e358cf1d00b558d74262b5f929ee8cc7e73a" -dependencies = [ - "errno-dragonfly", - "libc", - "windows-sys", -] - -[[package]] -name = "errno-dragonfly" -version = "0.1.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "aa68f1b12764fab894d2755d2518754e71b4fd80ecfb822714a1206c2aab39bf" -dependencies = [ - "cc", - "libc", -] - -[[package]] -name = "heck" -version = "0.4.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "95505c38b4572b2d910cecb0281560f54b440a19336cbbcb27bf6ce6adc6f5a8" - -[[package]] -name = "hermit-abi" -version = "0.3.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fed44880c466736ef9a5c5b5facefb5ed0785676d0c02d612db14e54f0d84286" - -[[package]] -name = "io-lifetimes" -version = "1.0.10" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9c66c74d2ae7e79a5a8f7ac924adbe38ee42a859c6539ad869eb51f0b52dc220" -dependencies = [ - "hermit-abi", - "libc", - "windows-sys", -] - -[[package]] -name = "is-terminal" -version = "0.4.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "adcf93614601c8129ddf72e2d5633df827ba6551541c6d8c59520a371475be1f" -dependencies = [ - "hermit-abi", - "io-lifetimes", - "rustix", - "windows-sys", -] - [[package]] name = "itertools" version = "0.10.5" @@ -200,18 +35,11 @@ version = "0.2.144" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2b00cc1c228a6782d0f076e7b232802e0c5689d41bb5df366f2a6b6621cfdfe1" -[[package]] -name = "linux-raw-sys" -version = "0.3.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ece97ea872ece730aed82664c424eb4c8291e1ff2480247ccf7409044bc6479f" - [[package]] name = "miri-script" version = "0.1.0" dependencies = [ "anyhow", - "clap", "dunce", "itertools", "path_macro", @@ -234,24 +62,6 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a6e819bbd49d5939f682638fa54826bf1650abddcd65d000923de8ad63cc7d15" -[[package]] -name = "proc-macro2" -version = "1.0.60" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "dec2b086b7a862cf4de201096214fa870344cf922b2b30c167badb3af3195406" -dependencies = [ - "unicode-ident", -] - -[[package]] -name = "quote" -version = "1.0.27" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8f4f29d145265ec1c483c7c654450edde0bfe043d3938d6972630663356d9500" -dependencies = [ - "proc-macro2", -] - [[package]] name = "rustc_version" version = "0.4.0" @@ -261,20 +71,6 @@ dependencies = [ "semver", ] -[[package]] -name = "rustix" -version = "0.37.19" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "acf8729d8542766f1b2cf77eb034d52f40d375bb8b615d0b147089946e16613d" -dependencies = [ - "bitflags", - "errno", - "io-lifetimes", - "libc", - "linux-raw-sys", - "windows-sys", -] - [[package]] name = "same-file" version = "1.0.6" @@ -296,35 +92,6 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "24188a676b6ae68c3b2cb3a01be17fbf7240ce009799bb56d5b1409051e78fde" -[[package]] -name = "strsim" -version = "0.10.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "73473c0e59e6d5812c5dfe2a064a6444949f089e20eec9a2e5506596494e4623" - -[[package]] -name = "syn" -version = "2.0.15" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a34fcf3e8b60f57e6a14301a2e916d323af98b0ea63c599441eec8558660c822" -dependencies = [ - "proc-macro2", - "quote", - "unicode-ident", -] - -[[package]] -name = "unicode-ident" -version = "1.0.8" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e5464a87b239f13a63a501f2701565754bae92d243d4bb7eb12f6d57d2269bf4" - -[[package]] -name = "utf8parse" -version = "0.2.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "711b9620af191e0cdc7468a8d14e709c3dcdb115b36f838e601583af800a370a" - [[package]] name = "walkdir" version = "2.3.3" @@ -377,72 +144,6 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" -[[package]] -name = "windows-sys" -version = "0.48.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "677d2418bec65e3338edb076e806bc1ec15693c5d0104683f2efe857f61056a9" -dependencies = [ - "windows-targets", -] - -[[package]] -name = "windows-targets" -version = "0.48.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7b1eb6f0cd7c80c79759c929114ef071b87354ce476d9d94271031c0497adfd5" -dependencies = [ - "windows_aarch64_gnullvm", - "windows_aarch64_msvc", - "windows_i686_gnu", - "windows_i686_msvc", - "windows_x86_64_gnu", - "windows_x86_64_gnullvm", - "windows_x86_64_msvc", -] - -[[package]] -name = "windows_aarch64_gnullvm" -version = "0.48.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "91ae572e1b79dba883e0d315474df7305d12f569b400fcf90581b06062f7e1bc" - -[[package]] -name = "windows_aarch64_msvc" -version = "0.48.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b2ef27e0d7bdfcfc7b868b317c1d32c641a6fe4629c171b8928c7b08d98d7cf3" - -[[package]] -name = "windows_i686_gnu" -version = "0.48.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "622a1962a7db830d6fd0a69683c80a18fda201879f0f447f065a3b7467daa241" - -[[package]] -name = "windows_i686_msvc" -version = "0.48.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4542c6e364ce21bf45d69fdd2a8e455fa38d316158cfd43b3ac1c5b1b19f8e00" - -[[package]] -name = "windows_x86_64_gnu" -version = "0.48.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ca2b8a661f7628cbd23440e50b05d705db3686f894fc9580820623656af974b1" - -[[package]] -name = "windows_x86_64_gnullvm" -version = "0.48.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7896dbc1f41e08872e9d5e8f8baa8fdd2677f29468c4e156210174edc7f7b953" - -[[package]] -name = "windows_x86_64_msvc" -version = "0.48.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1a515f5799fe4961cb532f983ce2b23082366b898e52ffbce459c86f67c8378a" - [[package]] name = "xshell" version = "0.2.3" diff --git a/src/tools/miri/miri-script/Cargo.toml b/src/tools/miri/miri-script/Cargo.toml index 197f6abd99039..c0414a2fe374a 100644 --- a/src/tools/miri/miri-script/Cargo.toml +++ b/src/tools/miri/miri-script/Cargo.toml @@ -11,7 +11,6 @@ edition = "2021" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] -clap = {version = "4.2", features = ["derive", "env"]} which = "4.4" walkdir = "2.3" itertools = "0.10" diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index 33e407a65dab5..66c2a4b0fd665 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -79,13 +79,11 @@ impl Command { Command::Fmt { flags } => Self::fmt(flags), Command::Clippy { flags } => Self::clippy(flags), Command::Cargo { flags } => Self::cargo(flags), - Command::ManySeeds { command, seed_start, seeds } => - Self::many_seeds(command, seed_start, seeds), + Command::ManySeeds { command } => Self::many_seeds(command), Command::Bench { benches } => Self::bench(benches), Command::Toolchain { flags } => Self::toolchain(flags), Command::RustcPull { commit } => Self::rustc_pull(commit.clone()), - Command::RustcPush { rustc_git, github_user, branch } => - Self::rustc_push(rustc_git, github_user, branch), + Command::RustcPush { github_user, branch } => Self::rustc_push(github_user, branch), } } @@ -178,7 +176,7 @@ impl Command { Ok(()) } - fn rustc_push(rustc_git: Option, github_user: String, branch: String) -> Result<()> { + fn rustc_push(github_user: String, branch: String) -> Result<()> { let sh = shell()?; sh.change_dir(miri_dir()?); let base = sh.read_file("rust-version")?.trim().to_owned(); @@ -188,7 +186,7 @@ impl Command { } // Find a repo we can do our preparation in. - if let Some(rustc_git) = rustc_git { + if let Ok(rustc_git) = env::var("RUSTC_GIT") { // If rustc_git is `Some`, we'll use an existing fork for the branch updates. sh.change_dir(rustc_git); } else { @@ -254,7 +252,15 @@ impl Command { Ok(()) } - fn many_seeds(command: Vec, seed_start: u64, seed_count: u64) -> Result<()> { + fn many_seeds(command: Vec) -> Result<()> { + let seed_start: u64 = env::var("MIRI_SEED_START") + .unwrap_or_else(|_| "0".into()) + .parse() + .context("failed to parse MIRI_SEED_START")?; + let seed_count: u64 = env::var("MIRI_SEEDS") + .unwrap_or_else(|_| "256".into()) + .parse() + .context("failed to parse MIRI_SEEDS")?; let seed_end = seed_start + seed_count; let Some((command_name, trailing_args)) = command.split_first() else { bail!("expected many-seeds command to be non-empty"); diff --git a/src/tools/miri/miri-script/src/main.rs b/src/tools/miri/miri-script/src/main.rs index ce00de5ac542d..849a9168028f2 100644 --- a/src/tools/miri/miri-script/src/main.rs +++ b/src/tools/miri/miri-script/src/main.rs @@ -1,19 +1,12 @@ mod commands; mod util; +use std::env; use std::ffi::OsString; -use anyhow::Result; -use clap::{Parser, Subcommand}; +use anyhow::{anyhow, bail, Result}; -#[derive(Parser, Clone, Debug)] -#[command(author, about, long_about = None)] -pub struct Cli { - #[command(subcommand)] - pub command: Command, -} - -#[derive(Subcommand, Clone, Debug)] +#[derive(Clone, Debug)] pub enum Command { /// Installs the miri driver and cargo-miri. /// Sets up the rpath such that the installed binary should work in any @@ -21,83 +14,58 @@ pub enum Command { /// sysroot, to prevent conflicts with other toolchains. Install { /// Flags that are passed through to `cargo install`. - #[arg(trailing_var_arg = true, allow_hyphen_values = true)] flags: Vec, }, /// Just build miri. Build { /// Flags that are passed through to `cargo build`. - #[arg(trailing_var_arg = true, allow_hyphen_values = true)] flags: Vec, }, /// Just check miri. Check { /// Flags that are passed through to `cargo check`. - #[arg(trailing_var_arg = true, allow_hyphen_values = true)] flags: Vec, }, /// Build miri, set up a sysroot and then run the test suite. Test { - #[arg(long, default_value_t = false)] bless: bool, /// Flags that are passed through to `cargo test`. - #[arg(trailing_var_arg = true, allow_hyphen_values = true)] flags: Vec, }, /// Build miri, set up a sysroot and then run the driver with the given . /// (Also respects MIRIFLAGS environment variable.) Run { - #[arg(long, default_value_t = false)] dep: bool, - /// Flags that are passed through to `miri` - #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + /// Flags that are passed through to `miri`. flags: Vec, }, /// Format all sources and tests. Fmt { /// Flags that are passed through to `rustfmt`. - #[arg(trailing_var_arg = true, allow_hyphen_values = true)] flags: Vec, }, /// Runs clippy on all sources. Clippy { /// Flags that are passed through to `cargo clippy`. - #[arg(trailing_var_arg = true, allow_hyphen_values = true)] flags: Vec, }, /// Runs just `cargo ` with the Miri-specific environment variables. /// Mainly meant to be invoked by rust-analyzer. - Cargo { - #[arg(trailing_var_arg = true, allow_hyphen_values = true)] - flags: Vec, - }, + Cargo { flags: Vec }, /// Runs over and over again with different seeds for Miri. The MIRIFLAGS /// variable is set to its original value appended with ` -Zmiri-seed=$SEED` for /// many different seeds. - ManySeeds { - /// Starting seed. - #[arg(long, env = "MIRI_SEED_START", default_value_t = 0)] - seed_start: u64, - /// Amount of seeds to try. - #[arg(long, env = "MIRI_SEEDS", default_value_t = 256)] - seeds: u64, - #[arg(trailing_var_arg = true, allow_hyphen_values = true)] - command: Vec, - }, + ManySeeds { command: Vec }, /// Runs the benchmarks from bench-cargo-miri in hyperfine. hyperfine needs to be installed. Bench { /// List of benchmarks to run. By default all benchmarks are run. - #[arg(trailing_var_arg = true, allow_hyphen_values = true)] benches: Vec, }, /// Update and activate the rustup toolchain 'miri' to the commit given in the /// `rust-version` file. /// `rustup-toolchain-install-master` must be installed for this to work. Any extra /// flags are passed to `rustup-toolchain-install-master`. - Toolchain { - #[arg(trailing_var_arg = true, allow_hyphen_values = true)] - flags: Vec, - }, + Toolchain { flags: Vec }, /// Pull and merge Miri changes from the rustc repo. Defaults to fetching the latest /// rustc commit. The fetched commit is stored in the `rust-version` file, so the /// next `./miri toolchain` will install the rustc that just got pulled. @@ -105,16 +73,138 @@ pub enum Command { /// Push Miri changes back to the rustc repo. This will pull a copy of the rustc /// history into the Miri repo, unless you set the RUSTC_GIT env var to an existing /// clone of the rustc repo. - RustcPush { - #[arg(long, env = "RUSTC_GIT")] - rustc_git: Option, - github_user: String, - branch: String, - }, + RustcPush { github_user: String, branch: String }, } +const HELP: &str = r#" COMMANDS + +./miri build : +Just build miri. are passed to `cargo build`. + +./miri check : +Just check miri. are passed to `cargo check`. + +./miri test [--bless] : +Build miri, set up a sysroot and then run the test suite. are passed +to the final `cargo test` invocation. + +./miri run [--dep] : +Build miri, set up a sysroot and then run the driver with the given . +(Also respects MIRIFLAGS environment variable.) + +./miri fmt : +Format all sources and tests. are passed to `rustfmt`. + +./miri clippy : +Runs clippy on all sources. are passed to `cargo clippy`. + +./miri cargo : +Runs just `cargo ` with the Miri-specific environment variables. +Mainly meant to be invoked by rust-analyzer. + +./miri install : +Installs the miri driver and cargo-miri. are passed to `cargo +install`. Sets up the rpath such that the installed binary should work in any +working directory. Note that the binaries are placed in the `miri` toolchain +sysroot, to prevent conflicts with other toolchains. + +./miri many-seeds : +Runs over and over again with different seeds for Miri. The MIRIFLAGS +variable is set to its original value appended with ` -Zmiri-seed=$SEED` for +many different seeds. The MIRI_SEEDS variable controls how many seeds are being +tried; MIRI_SEED_START controls the first seed to try. + +./miri bench : +Runs the benchmarks from bench-cargo-miri in hyperfine. hyperfine needs to be installed. + can explicitly list the benchmarks to run; by default, all of them are run. + +./miri toolchain : +Update and activate the rustup toolchain 'miri' to the commit given in the +`rust-version` file. +`rustup-toolchain-install-master` must be installed for this to work. Any extra +flags are passed to `rustup-toolchain-install-master`. + +./miri rustc-pull : +Pull and merge Miri changes from the rustc repo. Defaults to fetching the latest +rustc commit. The fetched commit is stored in the `rust-version` file, so the +next `./miri toolchain` will install the rustc that just got pulled. + +./miri rustc-push : +Push Miri changes back to the rustc repo. This will pull a copy of the rustc +history into the Miri repo, unless you set the RUSTC_GIT env var to an existing +clone of the rustc repo. + + ENVIRONMENT VARIABLES + +MIRI_SYSROOT: +If already set, the "sysroot setup" step is skipped. + +CARGO_EXTRA_FLAGS: +Pass extra flags to all cargo invocations. (Ignored by `./miri cargo`.)"#; + fn main() -> Result<()> { - let args = Cli::parse(); - args.command.exec()?; + // We are hand-rolling our own argument parser, since `clap` can't express what we need + // (https://github.com/clap-rs/clap/issues/5055). + let mut args = env::args_os().peekable(); + args.next().unwrap(); // skip program name + let command = match args.next().and_then(|s| s.into_string().ok()).as_deref() { + Some("build") => Command::Build { flags: args.collect() }, + Some("check") => Command::Check { flags: args.collect() }, + Some("test") => { + let bless = args.peek().is_some_and(|a| a.to_str() == Some("--bless")); + if bless { + // Consume the flag. + args.next().unwrap(); + } + Command::Test { bless, flags: args.collect() } + } + Some("run") => { + let dep = args.peek().is_some_and(|a| a.to_str() == Some("--dep")); + if dep { + // Consume the flag. + args.next().unwrap(); + } + Command::Run { dep, flags: args.collect() } + } + Some("fmt") => Command::Fmt { flags: args.collect() }, + Some("clippy") => Command::Clippy { flags: args.collect() }, + Some("cargo") => Command::Cargo { flags: args.collect() }, + Some("install") => Command::Install { flags: args.collect() }, + Some("many-seeds") => Command::ManySeeds { command: args.collect() }, + Some("bench") => Command::Bench { benches: args.collect() }, + Some("toolchain") => Command::Toolchain { flags: args.collect() }, + Some("rustc-pull") => { + let commit = args.next().map(|a| a.to_string_lossy().into_owned()); + if args.next().is_some() { + bail!("Too many arguments for `./miri rustc-pull`"); + } + Command::RustcPull { commit } + } + Some("rustc-push") => { + let github_user = args + .next() + .ok_or_else(|| { + anyhow!("Missing first argument for `./miri rustc-push GITHUB_USER BRANCH`") + })? + .to_string_lossy() + .into_owned(); + let branch = args + .next() + .ok_or_else(|| { + anyhow!("Missing second argument for `./miri rustc-push GITHUB_USER BRANCH`") + })? + .to_string_lossy() + .into_owned(); + if args.next().is_some() { + bail!("Too many arguments for `./miri rustc-push GITHUB_USER BRANCH`"); + } + Command::RustcPush { github_user, branch } + } + _ => { + eprintln!("Unknown or missing command. Usage:\n\n{HELP}"); + std::process::exit(1); + } + }; + command.exec()?; Ok(()) } From a63db6de5ae034bd006be663e760b9f81226b44c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 31 Jul 2023 16:00:48 +0200 Subject: [PATCH 07/30] add some interesting tests for alignment corner cases --- .../field_requires_parent_struct_alignment.rs | 24 +++++++++++++++++++ ...ld_requires_parent_struct_alignment.stderr | 20 ++++++++++++++++ .../align_strange_enum_discriminant_offset.rs | 23 ++++++++++++++++++ .../miri/tests/pass/issues/issue-53728.rs | 18 -------------- 4 files changed, 67 insertions(+), 18 deletions(-) create mode 100644 src/tools/miri/tests/fail/unaligned_pointers/field_requires_parent_struct_alignment.rs create mode 100644 src/tools/miri/tests/fail/unaligned_pointers/field_requires_parent_struct_alignment.stderr create mode 100644 src/tools/miri/tests/pass/align_strange_enum_discriminant_offset.rs delete mode 100644 src/tools/miri/tests/pass/issues/issue-53728.rs diff --git a/src/tools/miri/tests/fail/unaligned_pointers/field_requires_parent_struct_alignment.rs b/src/tools/miri/tests/fail/unaligned_pointers/field_requires_parent_struct_alignment.rs new file mode 100644 index 0000000000000..fa1812adc2961 --- /dev/null +++ b/src/tools/miri/tests/fail/unaligned_pointers/field_requires_parent_struct_alignment.rs @@ -0,0 +1,24 @@ +/// This tests that when a field sits at offset 0 in a 4-aligned struct, accessing the field +/// requires alignment 4 even if the field type has lower alignment requirements. + +#[repr(C)] +pub struct S { + x: u8, + y: u32, +} + +unsafe fn foo(x: *const S) -> u8 { + unsafe { (*x).x } //~ERROR: accessing memory with alignment 1, but alignment 4 is required +} + +fn main() { + unsafe { + let mem = [0u64; 16]; + let odd_ptr = std::ptr::addr_of!(mem).cast::().add(1); + // `odd_ptr` is now not aligned enough for `S`. + // If accessing field `x` can exploit that it is at offset 0 + // in a 4-aligned struct, that field access requires alignment 4, + // thus making this UB. + foo(odd_ptr.cast()); + } +} diff --git a/src/tools/miri/tests/fail/unaligned_pointers/field_requires_parent_struct_alignment.stderr b/src/tools/miri/tests/fail/unaligned_pointers/field_requires_parent_struct_alignment.stderr new file mode 100644 index 0000000000000..0f030a6e27ce0 --- /dev/null +++ b/src/tools/miri/tests/fail/unaligned_pointers/field_requires_parent_struct_alignment.stderr @@ -0,0 +1,20 @@ +error: Undefined Behavior: accessing memory with alignment ALIGN, but alignment ALIGN is required + --> $DIR/field_requires_parent_struct_alignment.rs:LL:CC + | +LL | unsafe { (*x).x } + | ^^^^^^ accessing memory with alignment ALIGN, but alignment ALIGN is required + | + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = note: BACKTRACE: + = note: inside `foo` at $DIR/field_requires_parent_struct_alignment.rs:LL:CC +note: inside `main` + --> $DIR/field_requires_parent_struct_alignment.rs:LL:CC + | +LL | foo(odd_ptr.cast()); + | ^^^^^^^^^^^^^^^^^^^ + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + diff --git a/src/tools/miri/tests/pass/align_strange_enum_discriminant_offset.rs b/src/tools/miri/tests/pass/align_strange_enum_discriminant_offset.rs new file mode 100644 index 0000000000000..e0d05e0b65abf --- /dev/null +++ b/src/tools/miri/tests/pass/align_strange_enum_discriminant_offset.rs @@ -0,0 +1,23 @@ +#![allow(unused)] + +#[repr(u16)] +enum DeviceKind { + Nil = 0, +} + +#[repr(C, packed)] +struct DeviceInfo { + endianness: u8, + device_kind: DeviceKind, +} + +fn main() { + // The layout of `Option<(DeviceInfo, u64)>` is funny: it uses the + // `DeviceKind` enum as niche, so that is offset 1, but the niche type is u16! + // So despite the type having alignment 8 and the field type alignment 2, + // the actual alignment is 1. + let x = None::<(DeviceInfo, u8)>; + let y = None::<(DeviceInfo, u16)>; + let z = None::<(DeviceInfo, u64)>; + format!("{} {} {}", x.is_some(), y.is_some(), y.is_some()); +} diff --git a/src/tools/miri/tests/pass/issues/issue-53728.rs b/src/tools/miri/tests/pass/issues/issue-53728.rs deleted file mode 100644 index 0c858d3444fb3..0000000000000 --- a/src/tools/miri/tests/pass/issues/issue-53728.rs +++ /dev/null @@ -1,18 +0,0 @@ -#[repr(u16)] -#[allow(dead_code)] -enum DeviceKind { - Nil = 0, -} - -#[repr(packed)] -#[allow(dead_code)] -struct DeviceInfo { - endianness: u8, - device_kind: DeviceKind, -} - -fn main() { - let _x = None::<(DeviceInfo, u8)>; - let _y = None::<(DeviceInfo, u16)>; - let _z = None::<(DeviceInfo, u64)>; -} From 26fc26f521076d498946ec616d036882958911d1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 31 Jul 2023 16:39:43 +0200 Subject: [PATCH 08/30] fix oversight from new miri-script --- src/tools/miri/tests/compiletest.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/tests/compiletest.rs b/src/tools/miri/tests/compiletest.rs index 8d82e6b8f9c65..4006fe139dbac 100644 --- a/src/tools/miri/tests/compiletest.rs +++ b/src/tools/miri/tests/compiletest.rs @@ -76,7 +76,7 @@ fn test_config(target: &str, path: &str, mode: Mode, with_dependencies: bool) -> let skip_ui_checks = env::var_os("MIRI_SKIP_UI_CHECKS").is_some(); let output_conflict_handling = match (bless, skip_ui_checks) { - (false, false) => OutputConflictHandling::Error("./miri bless".into()), + (false, false) => OutputConflictHandling::Error("./miri test --bless".into()), (true, false) => OutputConflictHandling::Bless, (false, true) => OutputConflictHandling::Ignore, (true, true) => panic!("cannot use RUSTC_BLESS and MIRI_SKIP_UI_CHECKS at the same time"), From 552700aa762d2f18f3f873501fbb103491ab8382 Mon Sep 17 00:00:00 2001 From: The Miri Conjob Bot Date: Wed, 2 Aug 2023 05:34:47 +0000 Subject: [PATCH 09/30] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index dde8d86765513..9de3b2ec9884b 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -fb53384c94b87adebceb6048865c9fe305e71b92 +90bb4184f89a24d26787a9eada781bf3c4dd3dc6 \ No newline at end of file From 2d01258c122f2218ca79bfd75ae6cd2354e12cb1 Mon Sep 17 00:00:00 2001 From: The Miri Conjob Bot Date: Wed, 2 Aug 2023 05:45:02 +0000 Subject: [PATCH 10/30] fmt --- .../pass/align_repeat_into_packed_field.rs | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/tools/miri/tests/pass/align_repeat_into_packed_field.rs b/src/tools/miri/tests/pass/align_repeat_into_packed_field.rs index 3affb20420509..fb028627d9d45 100644 --- a/src/tools/miri/tests/pass/align_repeat_into_packed_field.rs +++ b/src/tools/miri/tests/pass/align_repeat_into_packed_field.rs @@ -2,17 +2,21 @@ use std::intrinsics::mir::*; #[repr(packed)] -struct S { field: [u32; 2] } +struct S { + field: [u32; 2], +} #[custom_mir(dialect = "runtime", phase = "optimized")] -fn test() { mir! { - let s: S; - { - // Store a repeat expression directly into a field of a packed struct. - s.field = [0; 2]; - Return() +fn test() { + mir! { + let s: S; + { + // Store a repeat expression directly into a field of a packed struct. + s.field = [0; 2]; + Return() + } } -} } +} fn main() { // Run this a bunch of time to make sure it doesn't pass by chance. From c9512084bd250c92d898137863bc7d1f6374a6a3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 2 Aug 2023 18:26:55 +0200 Subject: [PATCH 11/30] add write_does_not_invalidate_all_aliases test, and enable direct_mut_to_const_raw test in TB --- .../pass/stacked-borrows/stacked-borrows.rs | 37 +++++++++++++++--- .../tests/pass/tree_borrows/tree-borrows.rs | 39 ++++++++++++++++--- 2 files changed, 65 insertions(+), 11 deletions(-) diff --git a/src/tools/miri/tests/pass/stacked-borrows/stacked-borrows.rs b/src/tools/miri/tests/pass/stacked-borrows/stacked-borrows.rs index 43ae7d6f522eb..d2ba184184485 100644 --- a/src/tools/miri/tests/pass/stacked-borrows/stacked-borrows.rs +++ b/src/tools/miri/tests/pass/stacked-borrows/stacked-borrows.rs @@ -10,7 +10,7 @@ fn main() { mut_raw_mut(); partially_invalidate_mut(); drop_after_sharing(); - direct_mut_to_const_raw(); + // direct_mut_to_const_raw(); two_raw(); shr_and_raw(); disjoint_mutable_subborrows(); @@ -19,6 +19,7 @@ fn main() { mut_below_shr(); wide_raw_ptr_in_tuple(); not_unpin_not_protected(); + write_does_not_invalidate_all_aliases(); } // Make sure that reading from an `&mut` does, like reborrowing to `&`, @@ -110,14 +111,13 @@ fn drop_after_sharing() { } // Make sure that coercing &mut T to *const T produces a writeable pointer. -fn direct_mut_to_const_raw() { - // TODO: This is currently disabled, waiting on a decision on - /*let x = &mut 0; +// TODO: This is currently disabled, waiting on a decision on +/*fn direct_mut_to_const_raw() { + let x = &mut 0; let y: *const i32 = x; unsafe { *(y as *mut i32) = 1; } assert_eq!(*x, 1); - */ -} +}*/ // Make sure that we can create two raw pointers from a mutable reference and use them both. fn two_raw() { @@ -238,3 +238,28 @@ fn not_unpin_not_protected() { drop(unsafe { Box::from_raw(raw) }); }); } + +fn write_does_not_invalidate_all_aliases() { + mod other { + /// Some private memory to store stuff in. + static mut S: *mut i32 = 0 as *mut i32; + + pub fn lib1(x: &&mut i32) { + unsafe { + S = (x as *const &mut i32).cast::<*mut i32>().read(); + } + } + + pub fn lib2() { + unsafe { + *S = 1337; + } + } + } + + let x = &mut 0; + other::lib1(&x); + *x = 42; // a write to x -- invalidates other pointers? + other::lib2(); + assert_eq!(*x, 1337); // oops, the value changed! I guess not all pointers were invalidated +} diff --git a/src/tools/miri/tests/pass/tree_borrows/tree-borrows.rs b/src/tools/miri/tests/pass/tree_borrows/tree-borrows.rs index 0d50d54faf6ac..476a4c857401b 100644 --- a/src/tools/miri/tests/pass/tree_borrows/tree-borrows.rs +++ b/src/tools/miri/tests/pass/tree_borrows/tree-borrows.rs @@ -10,6 +10,7 @@ fn main() { aliasing_read_only_mutable_refs(); string_as_mut_ptr(); two_mut_protected_same_alloc(); + direct_mut_to_const_raw(); // Stacked Borrows tests read_does_not_invalidate1(); @@ -19,7 +20,6 @@ fn main() { mut_raw_mut(); partially_invalidate_mut(); drop_after_sharing(); - direct_mut_to_const_raw(); two_raw(); shr_and_raw(); disjoint_mutable_subborrows(); @@ -28,6 +28,7 @@ fn main() { mut_below_shr(); wide_raw_ptr_in_tuple(); not_unpin_not_protected(); + write_does_not_invalidate_all_aliases(); } // Tree Borrows has no issue with several mutable references existing @@ -172,12 +173,12 @@ fn drop_after_sharing() { // Make sure that coercing &mut T to *const T produces a writeable pointer. fn direct_mut_to_const_raw() { - // TODO: This is currently disabled, waiting on a decision on - /*let x = &mut 0; + let x = &mut 0; let y: *const i32 = x; - unsafe { *(y as *mut i32) = 1; } + unsafe { + *(y as *mut i32) = 1; + } assert_eq!(*x, 1); - */ } // Make sure that we can create two raw pointers from a mutable reference and use them both. @@ -298,3 +299,31 @@ fn not_unpin_not_protected() { drop(unsafe { Box::from_raw(raw) }); }); } + +fn write_does_not_invalidate_all_aliases() { + // In TB there are other ways to do that (`addr_of!(*x)` has the same tag as `x`), + // but let's still make sure this SB test keeps working. + + mod other { + /// Some private memory to store stuff in. + static mut S: *mut i32 = 0 as *mut i32; + + pub fn lib1(x: &&mut i32) { + unsafe { + S = (x as *const &mut i32).cast::<*mut i32>().read(); + } + } + + pub fn lib2() { + unsafe { + *S = 1337; + } + } + } + + let x = &mut 0; + other::lib1(&x); + *x = 42; // a write to x -- invalidates other pointers? + other::lib2(); + assert_eq!(*x, 1337); // oops, the value changed! I guess not all pointers were invalidated +} From 42269c35afdb113b0b826d50293a0afe64c17826 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 2 Aug 2023 18:25:13 +0200 Subject: [PATCH 12/30] miri-script: simplify flag computation a bit --- src/tools/miri/miri-script/src/util.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/tools/miri/miri-script/src/util.rs b/src/tools/miri/miri-script/src/util.rs index caebd88b14b7a..dfbffa2ae911f 100644 --- a/src/tools/miri/miri-script/src/util.rs +++ b/src/tools/miri/miri-script/src/util.rs @@ -49,8 +49,8 @@ pub struct MiriEnv { impl MiriEnv { pub fn new() -> Result { - let sh = shell()?; let toolchain = active_toolchain()?; + let sh = shell()?; // we are preserving the current_dir on this one, so paths resolve properly! let miri_dir = miri_dir()?; let sysroot = cmd!(sh, "rustc +{toolchain} --print sysroot").read()?.into(); @@ -77,18 +77,18 @@ impl MiriEnv { // Compute rustflags. let rustflags = { - let env = std::env::var_os("RUSTFLAGS"); - let mut flags_with_warnings = OsString::from( - "-Zunstable-options -Wrustc::internal -Wrust_2018_idioms -Wunused_lifetimes -Wsemicolon_in_expressions_from_macros ", - ); - if let Some(value) = env { - flags_with_warnings.push(value); - } + let mut flags = OsString::new(); // We set the rpath so that Miri finds the private rustc libraries it needs. - let mut flags_with_compiler_settings = OsString::from("-C link-args=-Wl,-rpath,"); - flags_with_compiler_settings.push(&libdir); - flags_with_compiler_settings.push(flags_with_warnings); - flags_with_compiler_settings + flags.push("-C link-args=-Wl,-rpath,"); + flags.push(libdir); + // Enable rustc-specific lints (ignored without `-Zunstable-options`). + flags.push(" -Zunstable-options -Wrustc::internal -Wrust_2018_idioms -Wunused_lifetimes -Wsemicolon_in_expressions_from_macros"); + // Add user-defined flags. + if let Some(value) = std::env::var_os("RUSTFLAGS") { + flags.push(" "); + flags.push(value); + } + flags }; sh.set_var("RUSTFLAGS", rustflags); From df3b25f38605c65737c08bac684bf84dde56b2f1 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 2 Aug 2023 18:31:54 +0200 Subject: [PATCH 13/30] add local_addr_of_mut test --- .../miri/tests/pass/tree_borrows/tree-borrows.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/tools/miri/tests/pass/tree_borrows/tree-borrows.rs b/src/tools/miri/tests/pass/tree_borrows/tree-borrows.rs index 476a4c857401b..531543441c253 100644 --- a/src/tools/miri/tests/pass/tree_borrows/tree-borrows.rs +++ b/src/tools/miri/tests/pass/tree_borrows/tree-borrows.rs @@ -11,6 +11,7 @@ fn main() { string_as_mut_ptr(); two_mut_protected_same_alloc(); direct_mut_to_const_raw(); + local_addr_of_mut(); // Stacked Borrows tests read_does_not_invalidate1(); @@ -31,6 +32,17 @@ fn main() { write_does_not_invalidate_all_aliases(); } +#[allow(unused_assignments)] +fn local_addr_of_mut() { + let mut local = 0; + let ptr = ptr::addr_of_mut!(local); + // In SB, `local` and `*ptr` would have different tags, but in TB they have the same tag. + local = 1; + unsafe { *ptr = 2 }; + local = 3; + unsafe { *ptr = 4 }; +} + // Tree Borrows has no issue with several mutable references existing // at the same time, as long as they are used only immutably. // I.e. multiple Reserved can coexist. From 751cfa83f84d8f05e91c58ceacf2e46ffb17f3ba Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 2 Aug 2023 20:14:19 +0200 Subject: [PATCH 14/30] no need to forward all env vars --- src/tools/miri/miri-script/src/commands.rs | 12 ++++++------ src/tools/miri/miri-script/src/util.rs | 13 ++----------- 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index 66c2a4b0fd665..e8bd3ed903c5e 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -6,7 +6,7 @@ use std::ops::Not; use anyhow::{anyhow, bail, Context, Result}; use path_macro::path; use walkdir::WalkDir; -use xshell::cmd; +use xshell::{cmd, Shell}; use crate::util::*; use crate::Command; @@ -91,7 +91,7 @@ impl Command { // Make sure rustup-toolchain-install-master is installed. which::which("rustup-toolchain-install-master") .context("Please install rustup-toolchain-install-master by running 'cargo install rustup-toolchain-install-master'")?; - let sh = shell()?; + let sh = Shell::new()?; sh.change_dir(miri_dir()?); let new_commit = Some(sh.read_file("rust-version")?.trim().to_owned()); let current_commit = { @@ -130,7 +130,7 @@ impl Command { } fn rustc_pull(commit: Option) -> Result<()> { - let sh = shell()?; + let sh = Shell::new()?; sh.change_dir(miri_dir()?); let commit = commit.map(Result::Ok).unwrap_or_else(|| { let rust_repo_head = @@ -177,7 +177,7 @@ impl Command { } fn rustc_push(github_user: String, branch: String) -> Result<()> { - let sh = shell()?; + let sh = Shell::new()?; sh.change_dir(miri_dir()?); let base = sh.read_file("rust-version")?.trim().to_owned(); // Make sure the repo is clean. @@ -265,7 +265,7 @@ impl Command { let Some((command_name, trailing_args)) = command.split_first() else { bail!("expected many-seeds command to be non-empty"); }; - let sh = shell()?; + let sh = Shell::new()?; for seed in seed_start..seed_end { println!("Trying seed: {seed}"); let mut miriflags = env::var_os("MIRIFLAGS").unwrap_or_default(); @@ -293,7 +293,7 @@ impl Command { // Make sure we have an up-to-date Miri installed and selected the right toolchain. Self::install(vec![])?; - let sh = shell()?; + let sh = Shell::new()?; sh.change_dir(miri_dir()?); let benches_dir = "bench-cargo-miri"; let benches = if benches.is_empty() { diff --git a/src/tools/miri/miri-script/src/util.rs b/src/tools/miri/miri-script/src/util.rs index dfbffa2ae911f..64e780b61a7c7 100644 --- a/src/tools/miri/miri-script/src/util.rs +++ b/src/tools/miri/miri-script/src/util.rs @@ -13,21 +13,12 @@ pub fn miri_dir() -> std::io::Result { /// Queries the active toolchain for the Miri dir. pub fn active_toolchain() -> Result { - let sh = shell()?; + let sh = Shell::new()?; sh.change_dir(miri_dir()?); let stdout = cmd!(sh, "rustup show active-toolchain").read()?; Ok(stdout.split_whitespace().next().context("Could not obtain active Rust toolchain")?.into()) } -pub fn shell() -> Result { - let sh = Shell::new()?; - // xshell does not propagate parent's env variables by default. - for (k, v) in std::env::vars_os() { - sh.set_var(k, v); - } - Ok(sh) -} - pub fn flagsplit(flags: &str) -> Vec { // This code is taken from `RUSTFLAGS` handling in cargo. flags.split(' ').map(str::trim).filter(|s| !s.is_empty()).map(str::to_string).collect() @@ -50,7 +41,7 @@ pub struct MiriEnv { impl MiriEnv { pub fn new() -> Result { let toolchain = active_toolchain()?; - let sh = shell()?; // we are preserving the current_dir on this one, so paths resolve properly! + let sh = Shell::new()?; // we are preserving the current_dir on this one, so paths resolve properly! let miri_dir = miri_dir()?; let sysroot = cmd!(sh, "rustc +{toolchain} --print sysroot").read()?.into(); From 27377cbd8b53a7db693323ea66057357772cc22f Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 2 Aug 2023 20:43:05 +0200 Subject: [PATCH 15/30] fix miri-script being silent when running './miri test' --- src/tools/miri/miri-script/src/commands.rs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index e8bd3ed903c5e..3e0d4c2c1d027 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -16,17 +16,25 @@ const JOSH_FILTER: &str = ":rev(75dd959a3a40eb5b4574f8d2e23aa6efbeb33573:prefix=src/tools/miri):/src/tools/miri"; impl MiriEnv { - fn build_miri_sysroot(&mut self) -> Result<()> { + fn build_miri_sysroot(&mut self, quiet: bool) -> Result<()> { if self.sh.var("MIRI_SYSROOT").is_ok() { // Sysroot already set, use that. return Ok(()); } let manifest_path = path!(self.miri_dir / "cargo-miri" / "Cargo.toml"); let Self { toolchain, cargo_extra_flags, .. } = &self; + + // Make sure everything is built. Also Miri itself. + self.build(path!(self.miri_dir / "Cargo.toml"), &[], quiet)?; + self.build(&manifest_path, &[], quiet)?; + let target = &match self.sh.var("MIRI_TEST_TARGET") { Ok(target) => vec!["--target".into(), target], Err(_) => vec![], }; + if !quiet { + eprintln!("$ (buildig Miri sysroot)"); + } let output = cmd!(self.sh, "cargo +{toolchain} --quiet run {cargo_extra_flags...} --manifest-path {manifest_path} -- miri setup --print-sysroot {target...}" @@ -365,9 +373,8 @@ impl Command { fn test(bless: bool, flags: Vec) -> Result<()> { Self::auto_actions()?; let mut e = MiriEnv::new()?; - // First build, and get a sysroot. - e.build(path!(e.miri_dir / "Cargo.toml"), &[], /* quiet */ true)?; - e.build_miri_sysroot()?; + // Prepare a sysroot. + e.build_miri_sysroot(/* quiet */ false)?; // Then test, and let caller control flags. // Only in root project as `cargo-miri` has no tests. @@ -393,12 +400,11 @@ impl Command { let miriflags = e.sh.var("MIRIFLAGS").unwrap_or_default(); e.sh.set_var("MIRIFLAGS", format!("{miriflags} --target {target}")); } - // First build, and get a sysroot. - let miri_manifest = path!(e.miri_dir / "Cargo.toml"); - e.build(&miri_manifest, &[], /* quiet */ true)?; - e.build_miri_sysroot()?; + // Prepare a sysroot. + e.build_miri_sysroot(/* quiet */ true)?; // Then run the actual command. + let miri_manifest = path!(e.miri_dir / "Cargo.toml"); let miri_flags = e.sh.var("MIRIFLAGS").unwrap_or_default(); let miri_flags = flagsplit(&miri_flags); let toolchain = &e.toolchain; From 7fc0cbb02a273658312ceca24cc9e84d7ce2edaf Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 2 Aug 2023 20:43:30 +0200 Subject: [PATCH 16/30] Command debug printing prints the environment these days, we can kill some custom debugging code --- src/tools/miri/cargo-miri/src/util.rs | 40 +-------------------------- 1 file changed, 1 insertion(+), 39 deletions(-) diff --git a/src/tools/miri/cargo-miri/src/util.rs b/src/tools/miri/cargo-miri/src/util.rs index 0e3b04c0d8817..6381a4db86177 100644 --- a/src/tools/miri/cargo-miri/src/util.rs +++ b/src/tools/miri/cargo-miri/src/util.rs @@ -1,7 +1,5 @@ -use std::collections::HashMap; use std::env; use std::ffi::OsString; -use std::fmt::Write as _; use std::fs::File; use std::io::{self, BufWriter, Read, Write}; use std::ops::Not; @@ -247,46 +245,10 @@ pub fn local_crates(metadata: &Metadata) -> String { local_crates } -fn env_vars_from_cmd(cmd: &Command) -> Vec<(String, String)> { - let mut envs = HashMap::new(); - for (key, value) in std::env::vars() { - envs.insert(key, value); - } - for (key, value) in cmd.get_envs() { - if let Some(value) = value { - envs.insert(key.to_string_lossy().to_string(), value.to_string_lossy().to_string()); - } else { - envs.remove(&key.to_string_lossy().to_string()); - } - } - let mut envs: Vec<_> = envs.into_iter().collect(); - envs.sort(); - envs -} - /// Debug-print a command that is going to be run. pub fn debug_cmd(prefix: &str, verbose: usize, cmd: &Command) { if verbose == 0 { return; } - // We only do a single `eprintln!` call to minimize concurrency interactions. - let mut out = prefix.to_string(); - writeln!(out, " running command: env \\").unwrap(); - if verbose > 1 { - // Print the full environment this will be called in. - for (key, value) in env_vars_from_cmd(cmd) { - writeln!(out, "{key}={value:?} \\").unwrap(); - } - } else { - // Print only what has been changed for this `cmd`. - for (var, val) in cmd.get_envs() { - if let Some(val) = val { - writeln!(out, "{}={val:?} \\", var.to_string_lossy()).unwrap(); - } else { - writeln!(out, "--unset={}", var.to_string_lossy()).unwrap(); - } - } - } - write!(out, "{cmd:?}").unwrap(); - eprintln!("{out}"); + eprintln!("{prefix} running command: {cmd:?}"); } From 6e9479f4e04e633ce050fb6c80d123a62128bd32 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 2 Aug 2023 20:43:42 +0200 Subject: [PATCH 17/30] cargo-miri: avoid set_env --- src/tools/miri/cargo-miri/src/phases.rs | 4 +++- src/tools/miri/cargo-miri/src/setup.rs | 21 +++++++++++++++------ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/src/tools/miri/cargo-miri/src/phases.rs b/src/tools/miri/cargo-miri/src/phases.rs index d74e0c5157d35..11e7943ec8f69 100644 --- a/src/tools/miri/cargo-miri/src/phases.rs +++ b/src/tools/miri/cargo-miri/src/phases.rs @@ -94,7 +94,7 @@ pub fn phase_cargo_miri(mut args: impl Iterator) { let target = target.as_ref().unwrap_or(host); // We always setup. - setup(&subcommand, target, &rustc_version, verbose); + let miri_sysroot = setup(&subcommand, target, &rustc_version, verbose); // Invoke actual cargo for the job, but with different flags. // We re-use `cargo test` and `cargo run`, which makes target and binary handling very easy but @@ -159,6 +159,8 @@ pub fn phase_cargo_miri(mut args: impl Iterator) { // Forward all further arguments (not consumed by `ArgSplitFlagValue`) to cargo. cmd.args(args); + // Let it know where the Miri sysroot lives. + cmd.env("MIRI_SYSROOT", miri_sysroot); // Set `RUSTC_WRAPPER` to ourselves. Cargo will prepend that binary to its usual invocation, // i.e., the first argument is `rustc` -- which is what we use in `main` to distinguish // the two codepaths. (That extra argument is why we prefer this over setting `RUSTC`.) diff --git a/src/tools/miri/cargo-miri/src/setup.rs b/src/tools/miri/cargo-miri/src/setup.rs index 2e4f0a71013c0..55a97bafbcfa5 100644 --- a/src/tools/miri/cargo-miri/src/setup.rs +++ b/src/tools/miri/cargo-miri/src/setup.rs @@ -13,13 +13,20 @@ use crate::util::*; /// Performs the setup required to make `cargo miri` work: Getting a custom-built libstd. Then sets /// `MIRI_SYSROOT`. Skipped if `MIRI_SYSROOT` is already set, in which case we expect the user has /// done all this already. -pub fn setup(subcommand: &MiriCommand, target: &str, rustc_version: &VersionMeta, verbose: usize) { +pub fn setup( + subcommand: &MiriCommand, + target: &str, + rustc_version: &VersionMeta, + verbose: usize, +) -> PathBuf { let only_setup = matches!(subcommand, MiriCommand::Setup); let ask_user = !only_setup; let print_sysroot = only_setup && has_arg_flag("--print-sysroot"); // whether we just print the sysroot path - if !only_setup && std::env::var_os("MIRI_SYSROOT").is_some() { - // Skip setup step if MIRI_SYSROOT is explicitly set, *unless* we are `cargo miri setup`. - return; + if !only_setup { + if let Some(sysroot) = std::env::var_os("MIRI_SYSROOT") { + // Skip setup step if MIRI_SYSROOT is explicitly set, *unless* we are `cargo miri setup`. + return sysroot.into(); + } } // Determine where the rust sources are located. The env var trumps auto-detection. @@ -92,6 +99,8 @@ pub fn setup(subcommand: &MiriCommand, target: &str, rustc_version: &VersionMeta command.env("RUSTC", &cargo_miri_path); } command.env("MIRI_CALLED_FROM_SETUP", "1"); + // Miri expects `MIRI_SYSROOT` to be set when invoked in target mode. Even if that directory is empty. + command.env("MIRI_SYSROOT", &sysroot_dir); // Make sure there are no other wrappers getting in our way (Cc // https://github.com/rust-lang/miri/issues/1421, // https://github.com/rust-lang/miri/issues/2429). Looks like setting @@ -117,8 +126,6 @@ pub fn setup(subcommand: &MiriCommand, target: &str, rustc_version: &VersionMeta // the user might have set, which is consistent with normal `cargo build` that does // not apply `RUSTFLAGS` to the sysroot either. let rustflags = &["-Cdebug-assertions=off", "-Coverflow-checks=on"]; - // Make sure all target-level Miri invocations know their sysroot. - std::env::set_var("MIRI_SYSROOT", &sysroot_dir); // Do the build. if print_sysroot { @@ -159,4 +166,6 @@ pub fn setup(subcommand: &MiriCommand, target: &str, rustc_version: &VersionMeta // Print just the sysroot and nothing else to stdout; this way we do not need any escaping. println!("{}", sysroot_dir.display()); } + + sysroot_dir } From 072835472ae5794d533edafe9acd6d14082b31f2 Mon Sep 17 00:00:00 2001 From: The Miri Conjob Bot Date: Thu, 3 Aug 2023 05:25:09 +0000 Subject: [PATCH 18/30] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index 9de3b2ec9884b..b3e184cb2160f 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -90bb4184f89a24d26787a9eada781bf3c4dd3dc6 \ No newline at end of file +d8bbef50bbad789e26219f4ec88b5d73b05570a3 \ No newline at end of file From 9bb8b6691207f8f0072e7b58cddd8ee6f3141b5c Mon Sep 17 00:00:00 2001 From: The Miri Conjob Bot Date: Thu, 3 Aug 2023 05:34:22 +0000 Subject: [PATCH 19/30] fmt --- src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs index b2dbe8a70f0f6..51006ee97c98f 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs @@ -183,7 +183,12 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' ) -> InterpResult<'tcx, Option<(AllocId, BorTag)>> { let this = self.eval_context_mut(); // Ensure we bail out if the pointer goes out-of-bounds (see miri#1050). - this.check_ptr_access_align(place.ptr, ptr_size, Align::ONE, CheckInAllocMsg::InboundsTest)?; + this.check_ptr_access_align( + place.ptr, + ptr_size, + Align::ONE, + CheckInAllocMsg::InboundsTest, + )?; // It is crucial that this gets called on all code paths, to ensure we track tag creation. let log_creation = |this: &MiriInterpCx<'mir, 'tcx>, @@ -209,7 +214,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' // Unlike SB, we *do* a proper retag for size 0 if can identify the allocation. // After all, the pointer may be lazily initialized outside this initial range. data - }, + } Err(_) => { assert_eq!(ptr_size, Size::ZERO); // we did the deref check above, size has to be 0 here // This pointer doesn't come with an AllocId, so there's no From 7956dbdcd15c562d6fed7bc5cd58f62874a67112 Mon Sep 17 00:00:00 2001 From: Oli Scherer Date: Thu, 3 Aug 2023 11:37:43 +0000 Subject: [PATCH 20/30] Avoid infinite recursion for auto-fmt and auto-clippy --- src/tools/miri/miri-script/src/commands.rs | 25 ++++++++++++++-------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index 3e0d4c2c1d027..8e3e82854bf35 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -58,7 +58,7 @@ impl MiriEnv { impl Command { fn auto_actions() -> Result<()> { let miri_dir = miri_dir()?; - let auto_everything = path!(miri_dir / ".auto_everything").exists(); + let auto_everything = path!(miri_dir / ".auto-everything").exists(); let auto_toolchain = auto_everything || path!(miri_dir / ".auto-toolchain").exists(); let auto_fmt = auto_everything || path!(miri_dir / ".auto-fmt").exists(); let auto_clippy = auto_everything || path!(miri_dir / ".auto-clippy").exists(); @@ -78,6 +78,21 @@ impl Command { } pub fn exec(self) -> Result<()> { + match &self { + Command::Install { .. } + | Command::Build { .. } + | Command::Check { .. } + | Command::Test { .. } + | Command::Run { .. } + | Command::Fmt { .. } + | Command::Clippy { .. } + | Command::Cargo { .. } => Self::auto_actions()?, + | Command::ManySeeds { .. } + | Command::Toolchain { .. } + | Command::RustcPull { .. } + | Command::Bench { .. } + | Command::RustcPush { .. } => {} + } match self { Command::Install { flags } => Self::install(flags), Command::Build { flags } => Self::build(flags), @@ -328,7 +343,6 @@ impl Command { } fn install(flags: Vec) -> Result<()> { - Self::auto_actions()?; let e = MiriEnv::new()?; e.install_to_sysroot(e.miri_dir.clone(), &flags)?; e.install_to_sysroot(path!(e.miri_dir / "cargo-miri"), &flags)?; @@ -336,7 +350,6 @@ impl Command { } fn build(flags: Vec) -> Result<()> { - Self::auto_actions()?; let e = MiriEnv::new()?; e.build(path!(e.miri_dir / "Cargo.toml"), &flags, /* quiet */ false)?; e.build(path!(e.miri_dir / "cargo-miri" / "Cargo.toml"), &flags, /* quiet */ false)?; @@ -344,7 +357,6 @@ impl Command { } fn check(flags: Vec) -> Result<()> { - Self::auto_actions()?; let e = MiriEnv::new()?; e.check(path!(e.miri_dir / "Cargo.toml"), &flags)?; e.check(path!(e.miri_dir / "cargo-miri" / "Cargo.toml"), &flags)?; @@ -352,7 +364,6 @@ impl Command { } fn clippy(flags: Vec) -> Result<()> { - Self::auto_actions()?; let e = MiriEnv::new()?; e.clippy(path!(e.miri_dir / "Cargo.toml"), &flags)?; e.clippy(path!(e.miri_dir / "cargo-miri" / "Cargo.toml"), &flags)?; @@ -361,7 +372,6 @@ impl Command { } fn cargo(flags: Vec) -> Result<()> { - Self::auto_actions()?; let e = MiriEnv::new()?; let toolchain = &e.toolchain; // We carefully kept the working dir intact, so this will run cargo *on the workspace in the @@ -371,7 +381,6 @@ impl Command { } fn test(bless: bool, flags: Vec) -> Result<()> { - Self::auto_actions()?; let mut e = MiriEnv::new()?; // Prepare a sysroot. e.build_miri_sysroot(/* quiet */ false)?; @@ -386,7 +395,6 @@ impl Command { } fn run(dep: bool, flags: Vec) -> Result<()> { - Self::auto_actions()?; let mut e = MiriEnv::new()?; // Scan for "--target" to overwrite the "MIRI_TEST_TARGET" env var so // that we set the MIRI_SYSROOT up the right way. @@ -424,7 +432,6 @@ impl Command { } fn fmt(flags: Vec) -> Result<()> { - Self::auto_actions()?; let e = MiriEnv::new()?; let toolchain = &e.toolchain; let config_path = path!(e.miri_dir / "rustfmt.toml"); From 3c39dc2cba7434e88e580274d20e5d4ae0f8ab1c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 3 Aug 2023 14:14:14 +0200 Subject: [PATCH 21/30] add test checking that overlapping assignments work --- src/tools/miri/tests/pass/ptr_raw.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/tools/miri/tests/pass/ptr_raw.rs b/src/tools/miri/tests/pass/ptr_raw.rs index 3ba0fba9a9412..2f1843589072f 100644 --- a/src/tools/miri/tests/pass/ptr_raw.rs +++ b/src/tools/miri/tests/pass/ptr_raw.rs @@ -20,6 +20,15 @@ fn basic_raw() { assert_eq!(*x, 23); } +fn assign_overlapping() { + // Test an assignment where LHS and RHS alias. + // In Mir, that's UB (see `fail/overlapping_assignment.rs`), but in surface Rust this is allowed. + let mut mem = [0u32; 4]; + let ptr = &mut mem as *mut [u32; 4]; + unsafe { *ptr = *ptr }; +} + fn main() { basic_raw(); + assign_overlapping(); } From 3df2884c52f72425f888841f27e2b5dc7df87e1c Mon Sep 17 00:00:00 2001 From: Ben Kimock Date: Thu, 3 Aug 2023 17:59:57 -0400 Subject: [PATCH 22/30] A bit of spell-checking --- src/tools/miri/cargo-miri/src/main.rs | 2 +- src/tools/miri/cargo-miri/src/phases.rs | 2 +- src/tools/miri/cargo-miri/src/setup.rs | 2 +- src/tools/miri/miri-script/src/commands.rs | 2 +- src/tools/miri/src/helpers.rs | 2 +- src/tools/miri/src/machine.rs | 4 ++-- 6 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/tools/miri/cargo-miri/src/main.rs b/src/tools/miri/cargo-miri/src/main.rs index 6178670b4f034..c5fada6fe55b8 100644 --- a/src/tools/miri/cargo-miri/src/main.rs +++ b/src/tools/miri/cargo-miri/src/main.rs @@ -56,7 +56,7 @@ fn main() { return; } - // The way rustdoc invokes rustc is indistuingishable from the way cargo invokes rustdoc by the + // The way rustdoc invokes rustc is indistinguishable from the way cargo invokes rustdoc by the // arguments alone. `phase_cargo_rustdoc` sets this environment variable to let us disambiguate. if env::var_os("MIRI_CALLED_FROM_RUSTDOC").is_some() { // ...however, we then also see this variable when rustdoc invokes us as the testrunner! diff --git a/src/tools/miri/cargo-miri/src/phases.rs b/src/tools/miri/cargo-miri/src/phases.rs index 11e7943ec8f69..80ce63255826e 100644 --- a/src/tools/miri/cargo-miri/src/phases.rs +++ b/src/tools/miri/cargo-miri/src/phases.rs @@ -521,7 +521,7 @@ pub fn phase_runner(mut binary_args: impl Iterator, phase: Runner // `.rmeta`. // We also need to remove `--error-format` as cargo specifies that to be JSON, // but when we run here, cargo does not interpret the JSON any more. `--json` - // then also nees to be dropped. + // then also needs to be dropped. let mut args = info.args.into_iter(); let error_format_flag = "--error-format"; let json_flag = "--json"; diff --git a/src/tools/miri/cargo-miri/src/setup.rs b/src/tools/miri/cargo-miri/src/setup.rs index 55a97bafbcfa5..77cecddcb8b14 100644 --- a/src/tools/miri/cargo-miri/src/setup.rs +++ b/src/tools/miri/cargo-miri/src/setup.rs @@ -114,7 +114,7 @@ pub fn setup( command.arg("-v"); } } else { - // Supress output. + // Suppress output. command.stdout(process::Stdio::null()); command.stderr(process::Stdio::null()); } diff --git a/src/tools/miri/miri-script/src/commands.rs b/src/tools/miri/miri-script/src/commands.rs index 3e0d4c2c1d027..995efff5c47ba 100644 --- a/src/tools/miri/miri-script/src/commands.rs +++ b/src/tools/miri/miri-script/src/commands.rs @@ -33,7 +33,7 @@ impl MiriEnv { Err(_) => vec![], }; if !quiet { - eprintln!("$ (buildig Miri sysroot)"); + eprintln!("$ (building Miri sysroot)"); } let output = cmd!(self.sh, "cargo +{toolchain} --quiet run {cargo_extra_flags...} --manifest-path {manifest_path} -- diff --git a/src/tools/miri/src/helpers.rs b/src/tools/miri/src/helpers.rs index f9a8bad3a4f0e..e0f74d03ff6ba 100644 --- a/src/tools/miri/src/helpers.rs +++ b/src/tools/miri/src/helpers.rs @@ -337,7 +337,7 @@ pub trait EvalContextExt<'mir, 'tcx: 'mir>: crate::MiriInterpCxExt<'mir, 'tcx> { /// Call a function: Push the stack frame and pass the arguments. /// For now, arguments must be scalars (so that the caller does not have to know the layout). /// - /// If you do not provie a return place, a dangling zero-sized place will be created + /// If you do not provide a return place, a dangling zero-sized place will be created /// for your convenience. fn call_function( &mut self, diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 0c9c072b0511d..73035b01a1b8d 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -427,7 +427,7 @@ pub struct MiriMachine<'mir, 'tcx> { /// the emulated program. profiler: Option, /// Used with `profiler` to cache the `StringId`s for event names - /// uesd with `measureme`. + /// used with `measureme`. string_cache: FxHashMap, /// Cache of `Instance` exported under the given `Symbol` name. @@ -516,7 +516,7 @@ impl<'mir, 'tcx> MiriMachine<'mir, 'tcx> { let pid = process::id(); // We adopt the same naming scheme for the profiler output that rustc uses. In rustc, // the PID is padded so that the nondeterministic value of the PID does not spread - // nondeterminisim to the allocator. In Miri we are not aiming for such performance + // nondeterminism to the allocator. In Miri we are not aiming for such performance // control, we just pad for consistency with rustc. let filename = format!("{crate_name}-{pid:07}"); let path = Path::new(out).join(filename); From 325dc9288e1d15a9d25ec040ef8a568f46de505b Mon Sep 17 00:00:00 2001 From: The Miri Conjob Bot Date: Fri, 4 Aug 2023 05:27:12 +0000 Subject: [PATCH 23/30] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index b3e184cb2160f..f195bd5ac8a6c 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -d8bbef50bbad789e26219f4ec88b5d73b05570a3 \ No newline at end of file +a7caaae9fbef81325887aea060fc551da4589c6f \ No newline at end of file From b3a8e8eaa1c698f2decfffbf2ea2209766190485 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 5 Aug 2023 08:21:34 +0200 Subject: [PATCH 24/30] Preparing for merge from rustc --- src/tools/miri/rust-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tools/miri/rust-version b/src/tools/miri/rust-version index f195bd5ac8a6c..716b690daaa9b 100644 --- a/src/tools/miri/rust-version +++ b/src/tools/miri/rust-version @@ -1 +1 @@ -a7caaae9fbef81325887aea060fc551da4589c6f \ No newline at end of file +fca59ab5f0e7df7d816bed77a32abc0045ebe80b \ No newline at end of file From 621aeeb096d8e4f3eb656259fc286a1f32862ed3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 5 Aug 2023 08:22:15 +0200 Subject: [PATCH 25/30] fmt --- src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs | 6 +----- src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs | 6 +----- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs index 75e4b5f8466c5..ea7cba3f34640 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs @@ -13,11 +13,7 @@ use log::trace; use rustc_data_structures::fx::FxHashSet; use rustc_middle::mir::{Mutability, RetagKind}; -use rustc_middle::ty::{ - self, - layout::HasParamEnv, - Ty, -}; +use rustc_middle::ty::{self, layout::HasParamEnv, Ty}; use rustc_target::abi::{Abi, Align, Size}; use crate::borrow_tracker::{ diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs index c584eae220ed0..283bc94d3cf15 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs @@ -5,11 +5,7 @@ use rustc_target::abi::{Abi, Align, Size}; use crate::borrow_tracker::{AccessKind, GlobalStateInner, ProtectorKind, RetagFields}; use rustc_middle::{ mir::{Mutability, RetagKind}, - ty::{ - self, - layout::HasParamEnv, - Ty, - }, + ty::{self, layout::HasParamEnv, Ty}, }; use rustc_span::def_id::DefId; From a73c86d8a595a1b511392d54d5e2de9edfa2dc02 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 4 Aug 2023 13:59:44 +0200 Subject: [PATCH 26/30] fix return place protection when the place is given as a local --- src/tools/miri/src/machine.rs | 3 +- .../return_pointer_aliasing2.rs | 30 ++++++++++++++ .../return_pointer_aliasing2.stderr | 39 +++++++++++++++++++ 3 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.rs create mode 100644 src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.stderr diff --git a/src/tools/miri/src/machine.rs b/src/tools/miri/src/machine.rs index 73035b01a1b8d..e19be417b2261 100644 --- a/src/tools/miri/src/machine.rs +++ b/src/tools/miri/src/machine.rs @@ -1219,7 +1219,8 @@ impl<'mir, 'tcx> Machine<'mir, 'tcx> for MiriMachine<'mir, 'tcx> { // If we have a borrow tracker, we also have it set up protection so that all reads *and // writes* during this call are insta-UB. if ecx.machine.borrow_tracker.is_some() { - if let Either::Left(place) = place.as_mplace_or_local() { + // Have to do `to_op` first because a `Place::Local` doesn't imply the local doesn't have an address. + if let Either::Left(place) = ecx.place_to_op(place)?.as_mplace_or_imm() { ecx.protect_place(&place)?; } else { // Locals that don't have their address taken are as protected as they can ever be. diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.rs b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.rs new file mode 100644 index 0000000000000..7e9a632002615 --- /dev/null +++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.rs @@ -0,0 +1,30 @@ +//@compile-flags: -Zmiri-tree-borrows +#![feature(raw_ref_op)] +#![feature(core_intrinsics)] +#![feature(custom_mir)] + +use std::intrinsics::mir::*; + +#[custom_mir(dialect = "runtime", phase = "optimized")] +pub fn main() { + mir! { + { + let x = 0; + let ptr = &raw mut x; + // We arrange for `myfun` to have a pointer that aliases + // its return place. Even just reading from that pointer is UB. + Call(x, after_call, myfun(ptr)) + } + + after_call = { + Return() + } + } +} + +fn myfun(ptr: *mut i32) -> i32 { + // This overwrites the return place, which shouldn't be possible through another pointer. + unsafe { ptr.write(0) }; + //~^ ERROR: /write access .* forbidden/ + 13 +} diff --git a/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.stderr b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.stderr new file mode 100644 index 0000000000000..33a8a4b46bd05 --- /dev/null +++ b/src/tools/miri/tests/fail/function_calls/return_pointer_aliasing2.stderr @@ -0,0 +1,39 @@ +error: Undefined Behavior: write access through (root of the allocation) is forbidden + --> $DIR/return_pointer_aliasing2.rs:LL:CC + | +LL | unsafe { ptr.write(0) }; + | ^^^^^^^^^^^^ write access through (root of the allocation) is forbidden + | + = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental + = help: the accessed tag (root of the allocation) is foreign to the protected tag (i.e., it is not a child) + = help: this foreign write access would cause the protected tag (currently Active) to become Disabled + = help: protected tags must never be Disabled +help: the accessed tag was created here + --> $DIR/return_pointer_aliasing2.rs:LL:CC + | +LL | / mir! { +LL | | { +LL | | let x = 0; +LL | | let ptr = &raw mut x; +... | +LL | | } +LL | | } + | |_____^ +help: the protected tag was created here, in the initial state Active + --> $DIR/return_pointer_aliasing2.rs:LL:CC + | +LL | unsafe { ptr.write(0) }; + | ^^^^^^^^^^^^^^^^^^^^^^^ + = note: BACKTRACE (of the first span): + = note: inside `myfun` at $DIR/return_pointer_aliasing2.rs:LL:CC +note: inside `main` + --> $DIR/return_pointer_aliasing2.rs:LL:CC + | +LL | Call(x, after_call, myfun(ptr)) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: this error originates in the macro `::core::intrinsics::mir::__internal_remove_let` which comes from the expansion of the macro `mir` (in Nightly builds, run with -Z macro-backtrace for more info) + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + From e55f49415d412daa5c95989181ccbd424f8da01b Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 4 Aug 2023 14:33:09 +0200 Subject: [PATCH 27/30] borrow tracking: simplify provenance updating --- .../src/borrow_tracker/stacked_borrows/mod.rs | 37 ++++++------------ .../src/borrow_tracker/tree_borrows/mod.rs | 38 ++++--------------- 2 files changed, 20 insertions(+), 55 deletions(-) diff --git a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs index ea7cba3f34640..0351f586872eb 100644 --- a/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/stacked_borrows/mod.rs @@ -604,8 +604,7 @@ impl<'mir: 'ecx, 'tcx: 'mir, 'ecx> EvalContextPrivExt<'mir, 'tcx, 'ecx> { } trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'mir, 'tcx> { - /// Returns the `AllocId` the reborrow was done in, if some actual borrow stack manipulation - /// happened. + /// Returns the provenance that should be used henceforth. fn sb_reborrow( &mut self, place: &MPlaceTy<'tcx, Provenance>, @@ -613,7 +612,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' new_perm: NewPermission, new_tag: BorTag, retag_info: RetagInfo, // diagnostics info about this retag - ) -> InterpResult<'tcx, Option> { + ) -> InterpResult<'tcx, Option> { let this = self.eval_context_mut(); // Ensure we bail out if the pointer goes out-of-bounds (see miri#1050). this.check_ptr_access_align(place.ptr, size, Align::ONE, CheckInAllocMsg::InboundsTest)?; @@ -695,11 +694,14 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' // pointer tagging for example all calls to get_unchecked on them are invalid. if let Ok((alloc_id, base_offset, orig_tag)) = this.ptr_try_get_alloc_id(place.ptr) { log_creation(this, Some((alloc_id, base_offset, orig_tag)))?; - return Ok(Some(alloc_id)); + // Still give it the new provenance, it got retagged after all. + return Ok(Some(Provenance::Concrete { alloc_id, tag: new_tag })); + } else { + // This pointer doesn't come with an AllocId. :shrug: + log_creation(this, None)?; + // Provenance unchanged. + return Ok(place.ptr.provenance); } - // This pointer doesn't come with an AllocId. :shrug: - log_creation(this, None)?; - return Ok(None); } let (alloc_id, base_offset, orig_tag) = this.ptr_get_alloc_id(place.ptr)?; @@ -804,7 +806,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' } } - Ok(Some(alloc_id)) + Ok(Some(Provenance::Concrete { alloc_id, tag: new_tag })) } /// Retags an individual pointer, returning the retagged version. @@ -831,25 +833,10 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' let new_tag = this.machine.borrow_tracker.as_mut().unwrap().get_mut().new_ptr(); // Reborrow. - let alloc_id = this.sb_reborrow(&place, size, new_perm, new_tag, info)?; + let new_prov = this.sb_reborrow(&place, size, new_perm, new_tag, info)?; // Adjust pointer. - let new_place = place.map_provenance(|p| { - p.map(|prov| { - match alloc_id { - Some(alloc_id) => { - // If `reborrow` could figure out the AllocId of this ptr, hard-code it into the new one. - // Even if we started out with a wildcard, this newly retagged pointer is tied to that allocation. - Provenance::Concrete { alloc_id, tag: new_tag } - } - None => { - // Looks like this has to stay a wildcard pointer. - assert!(matches!(prov, Provenance::Wildcard)); - Provenance::Wildcard - } - } - }) - }); + let new_place = place.map_provenance(|_| new_prov); // Return new pointer. Ok(ImmTy::from_immediate(new_place.to_ref(this), val.layout)) diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs index 283bc94d3cf15..33141306d64dd 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs @@ -161,22 +161,14 @@ impl<'mir: 'ecx, 'tcx: 'mir, 'ecx> EvalContextPrivExt<'mir, 'tcx, 'ecx> { } trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<'mir, 'tcx> { - /// Returns the `AllocId` the reborrow was done in, if there is some actual - /// memory associated with this pointer. Returns `None` if there is no actual - /// memory allocated. Also checks that the reborrow of size `ptr_size` is - /// within bounds of the allocation. - /// - /// Also returns the tag that the pointer should get, which is essentially - /// `if new_perm.is_some() { new_tag } else { parent_tag }` along with - /// some logging (always) and fake reads (if `new_perm` is - /// `Some(NewPermission { perform_read_access: true }`). + /// Returns the provenance that should be used henceforth. fn tb_reborrow( &mut self, place: &MPlaceTy<'tcx, Provenance>, // parent tag extracted from here ptr_size: Size, new_perm: NewPermission, new_tag: BorTag, - ) -> InterpResult<'tcx, Option<(AllocId, BorTag)>> { + ) -> InterpResult<'tcx, Option> { let this = self.eval_context_mut(); // Ensure we bail out if the pointer goes out-of-bounds (see miri#1050). this.check_ptr_access_align( @@ -222,13 +214,14 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' place.layout.ty, ); log_creation(this, None)?; - return Ok(None); + // Keep original provenance. + return Ok(place.ptr.provenance); } }; log_creation(this, Some((alloc_id, base_offset, parent_prov)))?; let orig_tag = match parent_prov { - ProvenanceExtra::Wildcard => return Ok(None), // TODO: handle wildcard pointers + ProvenanceExtra::Wildcard => return Ok(place.ptr.provenance), // TODO: handle wildcard pointers ProvenanceExtra::Concrete(tag) => tag, }; @@ -279,7 +272,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' // Record the parent-child pair in the tree. tree_borrows.new_child(orig_tag, new_tag, new_perm.initial_state, range, span)?; - Ok(Some((alloc_id, new_tag))) + Ok(Some(Provenance::Concrete { alloc_id, tag: new_tag })) } /// Retags an individual pointer, returning the retagged version. @@ -315,25 +308,10 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' let new_tag = this.machine.borrow_tracker.as_mut().unwrap().get_mut().new_ptr(); // Compute the actual reborrow. - let reborrowed = this.tb_reborrow(&place, reborrow_size, new_perm, new_tag)?; + let new_prov = this.tb_reborrow(&place, reborrow_size, new_perm, new_tag)?; // Adjust pointer. - let new_place = place.map_provenance(|p| { - p.map(|prov| { - match reborrowed { - Some((alloc_id, actual_tag)) => { - // If `reborrow` could figure out the AllocId of this ptr, hard-code it into the new one. - // Even if we started out with a wildcard, this newly retagged pointer is tied to that allocation. - Provenance::Concrete { alloc_id, tag: actual_tag } - } - None => { - // Looks like this has to stay a wildcard pointer. - assert!(matches!(prov, Provenance::Wildcard)); - Provenance::Wildcard - } - } - }) - }); + let new_place = place.map_provenance(|_| new_prov); // Return new pointer. Ok(ImmTy::from_immediate(new_place.to_ref(this), val.layout)) From f0cbc7d3dc7b6f277ed3e7483a436d8e77b2039a Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 4 Aug 2023 14:50:45 +0200 Subject: [PATCH 28/30] ensure we allow zero-sized references to functions and vtables --- .../src/borrow_tracker/tree_borrows/mod.rs | 7 ++++++ .../miri/tests/pass/strange_references.rs | 25 +++++++++++++++++++ 2 files changed, 32 insertions(+) create mode 100644 src/tools/miri/tests/pass/strange_references.rs diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs index 33141306d64dd..a83dae8ae9835 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs @@ -248,6 +248,13 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' .insert(new_tag, protect); } + let alloc_kind = this.get_alloc_info(alloc_id).2; + if !matches!(alloc_kind, AllocKind::LiveData) { + // There's not actually any bytes here where accesses could even be tracked. + // Just produce the new provenance, nothing else to do. + return Ok(Some(Provenance::Concrete { alloc_id, tag: new_tag })); + } + let span = this.machine.current_span(); let alloc_extra = this.get_alloc_extra(alloc_id)?; let range = alloc_range(base_offset, ptr_size); diff --git a/src/tools/miri/tests/pass/strange_references.rs b/src/tools/miri/tests/pass/strange_references.rs new file mode 100644 index 0000000000000..fe5ff93a9ca8d --- /dev/null +++ b/src/tools/miri/tests/pass/strange_references.rs @@ -0,0 +1,25 @@ +//@revisions: stack tree +//@[tree]compile-flags: -Zmiri-tree-borrows + +// Create zero-sized references to vtables and function data. +// Just make sure nothing explodes. + +use std::{mem, ptr}; + +fn check_ref(x: &()) { + let _ptr = ptr::addr_of!(*x); +} + +fn main() { + check_ref({ + // Create reference to a function. + let fnptr: fn(&()) = check_ref; + unsafe { mem::transmute(fnptr) } + }); + check_ref({ + // Create reference to a vtable. + let wideptr: &dyn Send = &0; + let fields: (&i32, &()) = unsafe { mem::transmute(wideptr) }; + fields.1 + }) +} From ee674e79614679622306f3257b363f1f1bd39049 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Fri, 4 Aug 2023 15:49:37 +0200 Subject: [PATCH 29/30] tree borrows: consider some retags as writes for the purpose of data races --- .../tree_borrows/diagnostics.rs | 2 +- .../src/borrow_tracker/tree_borrows/mod.rs | 48 ++++++++++++------- .../src/borrow_tracker/tree_borrows/perms.rs | 21 +++++--- .../src/borrow_tracker/tree_borrows/tree.rs | 2 +- .../retag_data_race_protected_read.rs | 29 +++++++++++ ...etag_data_race_protected_read.stack.stderr | 20 ++++++++ ...retag_data_race_protected_read.tree.stderr | 25 ++++++++++ .../retag_data_race_write.rs | 4 +- .../retag_data_race_write.stack.stderr} | 0 .../retag_data_race_write.tree.stderr | 25 ++++++++++ .../retag_data_race_read.stack.stderr} | 20 ++++---- .../retag_data_race_read.tree.stderr | 26 ++++++++++ .../fail/tree_borrows/fragile-data-race.rs | 42 ---------------- .../tree_borrows/fragile-data-race.stderr | 32 ------------- .../fail/tree_borrows/retag-data-race.rs | 28 ----------- 15 files changed, 186 insertions(+), 138 deletions(-) create mode 100644 src/tools/miri/tests/fail/both_borrows/retag_data_race_protected_read.rs create mode 100644 src/tools/miri/tests/fail/both_borrows/retag_data_race_protected_read.stack.stderr create mode 100644 src/tools/miri/tests/fail/both_borrows/retag_data_race_protected_read.tree.stderr rename src/tools/miri/tests/fail/{stacked_borrows => both_borrows}/retag_data_race_write.rs (74%) rename src/tools/miri/tests/fail/{stacked_borrows/retag_data_race_write.stderr => both_borrows/retag_data_race_write.stack.stderr} (100%) create mode 100644 src/tools/miri/tests/fail/both_borrows/retag_data_race_write.tree.stderr rename src/tools/miri/tests/fail/{tree_borrows/retag-data-race.stderr => stacked_borrows/retag_data_race_read.stack.stderr} (54%) create mode 100644 src/tools/miri/tests/fail/stacked_borrows/retag_data_race_read.tree.stderr delete mode 100644 src/tools/miri/tests/fail/tree_borrows/fragile-data-race.rs delete mode 100644 src/tools/miri/tests/fail/tree_borrows/fragile-data-race.stderr delete mode 100644 src/tools/miri/tests/fail/tree_borrows/retag-data-race.rs diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs index 7723f06f29688..fd45671ba29d8 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/diagnostics.rs @@ -218,7 +218,7 @@ impl<'tcx> Tree { } } -#[derive(Debug, Clone, Copy, PartialEq)] +#[derive(Debug, Clone, Copy)] pub(super) enum TransitionError { /// This access is not allowed because some parent tag has insufficient permissions. /// For example, if a tag is `Frozen` and encounters a child write this will diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs index a83dae8ae9835..0fbe66360b2ab 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/mod.rs @@ -117,7 +117,7 @@ impl<'tcx> NewPermission { let ty_is_freeze = pointee.is_freeze(*cx.tcx, cx.param_env()); let ty_is_unpin = pointee.is_unpin(*cx.tcx, cx.param_env()); let initial_state = match mutability { - Mutability::Mut if ty_is_unpin => Permission::new_unique_2phase(ty_is_freeze), + Mutability::Mut if ty_is_unpin => Permission::new_reserved(ty_is_freeze), Mutability::Not if ty_is_freeze => Permission::new_frozen(), // Raw pointers never enter this function so they are not handled. // However raw pointers are not the only pointers that take the parent @@ -146,7 +146,7 @@ impl<'tcx> NewPermission { let ty_is_freeze = ty.is_freeze(*cx.tcx, cx.param_env()); Self { zero_size, - initial_state: Permission::new_unique_2phase(ty_is_freeze), + initial_state: Permission::new_reserved(ty_is_freeze), protector: (kind == RetagKind::FnEntry).then_some(ProtectorKind::WeakProtector), } }) @@ -250,6 +250,7 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' let alloc_kind = this.get_alloc_info(alloc_id).2; if !matches!(alloc_kind, AllocKind::LiveData) { + assert_eq!(ptr_size, Size::ZERO); // we did the deref check above, size has to be 0 here // There's not actually any bytes here where accesses could even be tracked. // Just produce the new provenance, nothing else to do. return Ok(Some(Provenance::Concrete { alloc_id, tag: new_tag })); @@ -261,24 +262,39 @@ trait EvalContextPrivExt<'mir: 'ecx, 'tcx: 'mir, 'ecx>: crate::MiriInterpCxExt<' let mut tree_borrows = alloc_extra.borrow_tracker_tb().borrow_mut(); // All reborrows incur a (possibly zero-sized) read access to the parent - { - let global = &this.machine.borrow_tracker.as_ref().unwrap(); - let span = this.machine.current_span(); - tree_borrows.perform_access( - AccessKind::Read, - orig_tag, - range, - global, - span, - diagnostics::AccessCause::Reborrow, - )?; + tree_borrows.perform_access( + AccessKind::Read, + orig_tag, + range, + this.machine.borrow_tracker.as_ref().unwrap(), + this.machine.current_span(), + diagnostics::AccessCause::Reborrow, + )?; + // Record the parent-child pair in the tree. + tree_borrows.new_child(orig_tag, new_tag, new_perm.initial_state, range, span)?; + drop(tree_borrows); + + // Also inform the data race model (but only if any bytes are actually affected). + if range.size.bytes() > 0 { if let Some(data_race) = alloc_extra.data_race.as_ref() { - data_race.read(alloc_id, range, &this.machine)?; + // We sometimes need to make it a write, since not all retags commute with reads! + // FIXME: Is that truly the semantics we want? Some optimizations are likely to be + // very unhappy without this. We'd tsill ge some UB just by picking a suitable + // interleaving, but wether UB happens can depend on whether a write occurs in the + // future... + let is_write = new_perm.initial_state.is_active() + || (new_perm.initial_state.is_resrved() && new_perm.protector.is_some()); + if is_write { + // Need to get mutable access to alloc_extra. + // (Cannot always do this as we can do read-only reborrowing on read-only allocations.) + let (alloc_extra, machine) = this.get_alloc_extra_mut(alloc_id)?; + alloc_extra.data_race.as_mut().unwrap().write(alloc_id, range, machine)?; + } else { + data_race.read(alloc_id, range, &this.machine)?; + } } } - // Record the parent-child pair in the tree. - tree_borrows.new_child(orig_tag, new_tag, new_perm.initial_state, range, span)?; Ok(Some(Provenance::Concrete { alloc_id, tag: new_tag })) } diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs index 051b209da176d..a81c6095a4f39 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs @@ -134,25 +134,32 @@ pub struct PermTransition { impl Permission { /// Default initial permission of the root of a new tree. - pub fn new_root() -> Self { + pub fn new_active() -> Self { Self { inner: Active } } /// Default initial permission of a reborrowed mutable reference. - pub fn new_unique_2phase(ty_is_freeze: bool) -> Self { + pub fn new_reserved(ty_is_freeze: bool) -> Self { Self { inner: Reserved { ty_is_freeze } } } - /// Default initial permission for return place. - pub fn new_active() -> Self { - Self { inner: Active } - } - /// Default initial permission of a reborrowed shared reference pub fn new_frozen() -> Self { Self { inner: Frozen } } + pub fn is_active(self) -> bool { + matches!(self.inner, Active) + } + + pub fn is_resrved(self) -> bool { + matches!(self.inner, Reserved { .. }) + } + + pub fn is_frozen(self) -> bool { + matches!(self.inner, Frozen) + } + /// Apply the transition to the inner PermissionPriv. pub fn perform_access( kind: AccessKind, diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs index 355356b743a7d..2e89036e061a8 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs @@ -376,7 +376,7 @@ where { impl Tree { /// Create a new tree, with only a root pointer. pub fn new(root_tag: BorTag, size: Size, span: Span) -> Self { - let root_perm = Permission::new_root(); + let root_perm = Permission::new_active(); let mut tag_mapping = UniKeyMap::default(); let root_idx = tag_mapping.insert(root_tag); let nodes = { diff --git a/src/tools/miri/tests/fail/both_borrows/retag_data_race_protected_read.rs b/src/tools/miri/tests/fail/both_borrows/retag_data_race_protected_read.rs new file mode 100644 index 0000000000000..f192e76de1350 --- /dev/null +++ b/src/tools/miri/tests/fail/both_borrows/retag_data_race_protected_read.rs @@ -0,0 +1,29 @@ +//@revisions: stack tree +//@compile-flags: -Zmiri-preemption-rate=0 +//@[tree]compile-flags: -Zmiri-tree-borrows +use std::thread; + +#[derive(Copy, Clone)] +struct SendPtr(*mut i32); +unsafe impl Send for SendPtr {} + +fn main() { + let mut mem = 0; + let ptr = SendPtr(&mut mem as *mut _); + + let t = thread::spawn(move || { + let ptr = ptr; + // We do a protected 2phase retag (but no write!) in this thread. + fn retag(_x: &mut i32) {} //~[tree]ERROR: Data race detected between (1) Read on thread `main` and (2) Write on thread `` + retag(unsafe { &mut *ptr.0 }); //~[stack]ERROR: Data race detected between (1) Read on thread `main` and (2) Write on thread `` + }); + + // We do a read in the main thread. + unsafe { ptr.0.read() }; + + // These two operations do not commute -- if the read happens after the retag, the retagged pointer + // gets frozen! So we want this to be considered UB so that we can still freely move the read around + // in this thread without worrying about reordering with retags in other threads. + + t.join().unwrap(); +} diff --git a/src/tools/miri/tests/fail/both_borrows/retag_data_race_protected_read.stack.stderr b/src/tools/miri/tests/fail/both_borrows/retag_data_race_protected_read.stack.stderr new file mode 100644 index 0000000000000..10fb1dece2afc --- /dev/null +++ b/src/tools/miri/tests/fail/both_borrows/retag_data_race_protected_read.stack.stderr @@ -0,0 +1,20 @@ +error: Undefined Behavior: Data race detected between (1) Read on thread `main` and (2) Write on thread `` at ALLOC. (2) just happened here + --> $DIR/retag_data_race_protected_read.rs:LL:CC + | +LL | retag(unsafe { &mut *ptr.0 }); + | ^^^^^^^^^^^ Data race detected between (1) Read on thread `main` and (2) Write on thread `` at ALLOC. (2) just happened here + | +help: and (1) occurred earlier here + --> $DIR/retag_data_race_protected_read.rs:LL:CC + | +LL | unsafe { ptr.0.read() }; + | ^^^^^^^^^^^^ + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = note: BACKTRACE (of the first span): + = note: inside closure at $DIR/retag_data_race_protected_read.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + diff --git a/src/tools/miri/tests/fail/both_borrows/retag_data_race_protected_read.tree.stderr b/src/tools/miri/tests/fail/both_borrows/retag_data_race_protected_read.tree.stderr new file mode 100644 index 0000000000000..173acf4b96c03 --- /dev/null +++ b/src/tools/miri/tests/fail/both_borrows/retag_data_race_protected_read.tree.stderr @@ -0,0 +1,25 @@ +error: Undefined Behavior: Data race detected between (1) Read on thread `main` and (2) Write on thread `` at ALLOC. (2) just happened here + --> $DIR/retag_data_race_protected_read.rs:LL:CC + | +LL | fn retag(_x: &mut i32) {} + | ^^ Data race detected between (1) Read on thread `main` and (2) Write on thread `` at ALLOC. (2) just happened here + | +help: and (1) occurred earlier here + --> $DIR/retag_data_race_protected_read.rs:LL:CC + | +LL | unsafe { ptr.0.read() }; + | ^^^^^^^^^^^^ + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = note: BACKTRACE (of the first span): + = note: inside `main::{closure#0}::retag` at $DIR/retag_data_race_protected_read.rs:LL:CC +note: inside closure + --> $DIR/retag_data_race_protected_read.rs:LL:CC + | +LL | ... retag(unsafe { &mut *ptr.0 }); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + diff --git a/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_write.rs b/src/tools/miri/tests/fail/both_borrows/retag_data_race_write.rs similarity index 74% rename from src/tools/miri/tests/fail/stacked_borrows/retag_data_race_write.rs rename to src/tools/miri/tests/fail/both_borrows/retag_data_race_write.rs index c1dded40d3c35..868b3beb53b7d 100644 --- a/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_write.rs +++ b/src/tools/miri/tests/fail/both_borrows/retag_data_race_write.rs @@ -1,5 +1,7 @@ //! Make sure that a retag acts like a write for the data race model. +//@revisions: stack tree //@compile-flags: -Zmiri-preemption-rate=0 +//@[tree]compile-flags: -Zmiri-tree-borrows #[derive(Copy, Clone)] struct SendPtr(*mut u8); @@ -15,7 +17,7 @@ fn thread_1(p: SendPtr) { fn thread_2(p: SendPtr) { let p = p.0; unsafe { - *p = 5; //~ ERROR: Data race detected between (1) Write on thread `` and (2) Write on thread `` + *p = 5; //~ ERROR: /Data race detected between \(1\) (Read|Write) on thread `` and \(2\) Write on thread ``/ } } diff --git a/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_write.stderr b/src/tools/miri/tests/fail/both_borrows/retag_data_race_write.stack.stderr similarity index 100% rename from src/tools/miri/tests/fail/stacked_borrows/retag_data_race_write.stderr rename to src/tools/miri/tests/fail/both_borrows/retag_data_race_write.stack.stderr diff --git a/src/tools/miri/tests/fail/both_borrows/retag_data_race_write.tree.stderr b/src/tools/miri/tests/fail/both_borrows/retag_data_race_write.tree.stderr new file mode 100644 index 0000000000000..37d216b9877be --- /dev/null +++ b/src/tools/miri/tests/fail/both_borrows/retag_data_race_write.tree.stderr @@ -0,0 +1,25 @@ +error: Undefined Behavior: Data race detected between (1) Read on thread `` and (2) Write on thread `` at ALLOC. (2) just happened here + --> $DIR/retag_data_race_write.rs:LL:CC + | +LL | *p = 5; + | ^^^^^^ Data race detected between (1) Read on thread `` and (2) Write on thread `` at ALLOC. (2) just happened here + | +help: and (1) occurred earlier here + --> $DIR/retag_data_race_write.rs:LL:CC + | +LL | let _r = &mut *p; + | ^^^^^^^ + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = note: BACKTRACE (of the first span): + = note: inside `thread_2` at $DIR/retag_data_race_write.rs:LL:CC +note: inside closure + --> $DIR/retag_data_race_write.rs:LL:CC + | +LL | let t2 = std::thread::spawn(move || thread_2(p)); + | ^^^^^^^^^^^ + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to previous error + diff --git a/src/tools/miri/tests/fail/tree_borrows/retag-data-race.stderr b/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_read.stack.stderr similarity index 54% rename from src/tools/miri/tests/fail/tree_borrows/retag-data-race.stderr rename to src/tools/miri/tests/fail/stacked_borrows/retag_data_race_read.stack.stderr index f2cdfe7c314d5..c53a495b5e18b 100644 --- a/src/tools/miri/tests/fail/tree_borrows/retag-data-race.stderr +++ b/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_read.stack.stderr @@ -1,23 +1,23 @@ error: Undefined Behavior: Data race detected between (1) Read on thread `` and (2) Write on thread `` at ALLOC. (2) just happened here - --> $DIR/retag-data-race.rs:LL:CC + --> $DIR/retag_data_race_read.rs:LL:CC | -LL | *p = 5; - | ^^^^^^ Data race detected between (1) Read on thread `` and (2) Write on thread `` at ALLOC. (2) just happened here +LL | *p = 5; + | ^^^^^^ Data race detected between (1) Read on thread `` and (2) Write on thread `` at ALLOC. (2) just happened here | help: and (1) occurred earlier here - --> $DIR/retag-data-race.rs:LL:CC + --> $DIR/retag_data_race_read.rs:LL:CC | -LL | let _r = &*p; - | ^^^ +LL | let _r = &*p; + | ^^^ = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information = note: BACKTRACE (of the first span): - = note: inside `thread_2` at $DIR/retag-data-race.rs:LL:CC + = note: inside `thread_2` at $DIR/retag_data_race_read.rs:LL:CC note: inside closure - --> $DIR/retag-data-race.rs:LL:CC + --> $DIR/retag_data_race_read.rs:LL:CC | -LL | let t2 = std::thread::spawn(move || unsafe { thread_2(p) }); - | ^^^^^^^^^^^ +LL | let t2 = std::thread::spawn(move || thread_2(p)); + | ^^^^^^^^^^^ note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace diff --git a/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_read.tree.stderr b/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_read.tree.stderr new file mode 100644 index 0000000000000..1e154eb0564a2 --- /dev/null +++ b/src/tools/miri/tests/fail/stacked_borrows/retag_data_race_read.tree.stderr @@ -0,0 +1,26 @@ +error: Undefined Behavior: reborrow through (root of the allocation) is forbidden + --> RUSTLIB/std/src/rt.rs:LL:CC + | +LL | panic::catch_unwind(move || unsafe { init(argc, argv, sigpipe) }).map_err(rt_abort)?; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ reborrow through (root of the allocation) is forbidden + | + = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental + = help: the accessed tag (root of the allocation) is foreign to the protected tag (i.e., it is not a child) + = help: this reborrow (acting as a foreign read access) would cause the protected tag (currently Active) to become Disabled + = help: protected tags must never be Disabled +help: the accessed tag was created here + --> RUSTLIB/std/src/rt.rs:LL:CC + | +LL | panic::catch_unwind(move || unsafe { init(argc, argv, sigpipe) }).map_err(rt_abort)?; + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +help: the protected tag was created here, in the initial state Active + --> RUSTLIB/std/src/panic.rs:LL:CC + | +LL | pub fn catch_unwind R + UnwindSafe, R>(f: F) -> Result { + | ^ + = note: BACKTRACE (of the first span): + = note: inside `std::rt::lang_start_internal` at RUSTLIB/std/src/rt.rs:LL:CC + = note: inside `std::rt::lang_start::<()>` at RUSTLIB/std/src/rt.rs:LL:CC + +error: aborting due to previous error + diff --git a/src/tools/miri/tests/fail/tree_borrows/fragile-data-race.rs b/src/tools/miri/tests/fail/tree_borrows/fragile-data-race.rs deleted file mode 100644 index 215100de0a13c..0000000000000 --- a/src/tools/miri/tests/fail/tree_borrows/fragile-data-race.rs +++ /dev/null @@ -1,42 +0,0 @@ -//! Race-condition-like interaction between a read and a reborrow. -//! Even though no write or fake write occurs, reads have an effect on protected -//! Reserved. This is a protected-retag/read data race, but is not *detected* as -//! a data race violation because reborrows are not writes. -//! -//! This test is sensitive to the exact schedule so we disable preemption. -//@compile-flags: -Zmiri-tree-borrows -Zmiri-preemption-rate=0 -use std::ptr::addr_of_mut; -use std::thread; - -#[derive(Copy, Clone)] -struct SendPtr(*mut u8); - -unsafe impl Send for SendPtr {} - -// First thread is just a reborrow, but for an instant `x` is -// protected and thus vulnerable to foreign reads. -fn thread_1(x: &mut u8) -> SendPtr { - thread::yield_now(); // make the other thread go first - SendPtr(x as *mut u8) -} - -// Second thread simply performs a read. -fn thread_2(x: &u8) { - let _val = *x; -} - -fn main() { - let mut x = 0u8; - let x_1 = unsafe { &mut *addr_of_mut!(x) }; - let xg = unsafe { &*addr_of_mut!(x) }; - - // The two threads are executed in parallel on aliasing pointers. - // UB occurs if the read of thread_2 occurs while the protector of thread_1 - // is in place. - let hf = thread::spawn(move || thread_1(x_1)); - let hg = thread::spawn(move || thread_2(xg)); - let SendPtr(p) = hf.join().unwrap(); - let () = hg.join().unwrap(); - - unsafe { *p = 1 }; //~ ERROR: /write access through .* is forbidden/ -} diff --git a/src/tools/miri/tests/fail/tree_borrows/fragile-data-race.stderr b/src/tools/miri/tests/fail/tree_borrows/fragile-data-race.stderr deleted file mode 100644 index 910f51ba8a3f5..0000000000000 --- a/src/tools/miri/tests/fail/tree_borrows/fragile-data-race.stderr +++ /dev/null @@ -1,32 +0,0 @@ -error: Undefined Behavior: write access through is forbidden - --> $DIR/fragile-data-race.rs:LL:CC - | -LL | unsafe { *p = 1 }; - | ^^^^^^ write access through is forbidden - | - = help: this indicates a potential bug in the program: it performed an invalid operation, but the Tree Borrows rules it violated are still experimental - = help: the accessed tag is a child of the conflicting tag - = help: the conflicting tag has state Frozen which forbids this child write access -help: the accessed tag was created here - --> $DIR/fragile-data-race.rs:LL:CC - | -LL | fn thread_1(x: &mut u8) -> SendPtr { - | ^ -help: the conflicting tag was created here, in the initial state Reserved - --> RUSTLIB/std/src/panic.rs:LL:CC - | -LL | pub fn catch_unwind R + UnwindSafe, R>(f: F) -> Result { - | ^ -help: the conflicting tag later transitioned to Frozen due to a reborrow (acting as a foreign read access) at offsets [0x0..0x1] - --> RUSTLIB/core/src/ptr/mod.rs:LL:CC - | -LL | crate::intrinsics::read_via_copy(src) - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = help: this transition corresponds to a loss of write permissions - = note: BACKTRACE (of the first span): - = note: inside `main` at $DIR/fragile-data-race.rs:LL:CC - -note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace - -error: aborting due to previous error - diff --git a/src/tools/miri/tests/fail/tree_borrows/retag-data-race.rs b/src/tools/miri/tests/fail/tree_borrows/retag-data-race.rs deleted file mode 100644 index 8ef3d23e804b7..0000000000000 --- a/src/tools/miri/tests/fail/tree_borrows/retag-data-race.rs +++ /dev/null @@ -1,28 +0,0 @@ -//! Make sure that a retag acts like a read for the data race model. -//! This is a retag/write race condition. -//! -//! This test is sensitive to the exact schedule so we disable preemption. -//@compile-flags: -Zmiri-tree-borrows -Zmiri-preemption-rate=0 -#[derive(Copy, Clone)] -struct SendPtr(*mut u8); - -unsafe impl Send for SendPtr {} - -unsafe fn thread_1(SendPtr(p): SendPtr) { - let _r = &*p; -} - -unsafe fn thread_2(SendPtr(p): SendPtr) { - *p = 5; //~ ERROR: Data race detected between (1) Read on thread `` and (2) Write on thread `` -} - -fn main() { - let mut x = 0; - let p = std::ptr::addr_of_mut!(x); - let p = SendPtr(p); - - let t1 = std::thread::spawn(move || unsafe { thread_1(p) }); - let t2 = std::thread::spawn(move || unsafe { thread_2(p) }); - let _ = t1.join(); - let _ = t2.join(); -} From 36716dc21c2a1a13736e99580bd546067a401b9c Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Sat, 5 Aug 2023 09:45:13 +0200 Subject: [PATCH 30/30] add a test ensuring that we enforce noalias on accesses --- .../src/borrow_tracker/tree_borrows/perms.rs | 2 +- .../src/borrow_tracker/tree_borrows/tree.rs | 42 ++++++++++++++++--- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs index a81c6095a4f39..b4a9a768e27ff 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/perms.rs @@ -445,7 +445,7 @@ mod propagation_optimization_checks { } #[test] - fn foreign_read_is_noop_after_write() { + fn foreign_read_is_noop_after_foreign_write() { use transition::*; let old_access = AccessKind::Write; let new_access = AccessKind::Read; diff --git a/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs b/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs index 2e89036e061a8..5abf13229bbcc 100644 --- a/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs +++ b/src/tools/miri/src/borrow_tracker/tree_borrows/tree.rs @@ -110,7 +110,7 @@ impl LocationState { // Helper to optimize the tree traversal. // The optimization here consists of observing thanks to the tests - // `foreign_read_is_noop_after_write` and `all_transitions_idempotent`, + // `foreign_read_is_noop_after_foreign_write` and `all_transitions_idempotent`, // that there are actually just three possible sequences of events that can occur // in between two child accesses that produce different results. // @@ -139,7 +139,7 @@ impl LocationState { let new_access_noop = match (self.latest_foreign_access, access_kind) { // Previously applied transition makes the new one a guaranteed // noop in the two following cases: - // (1) justified by `foreign_read_is_noop_after_write` + // (1) justified by `foreign_read_is_noop_after_foreign_write` (Some(AccessKind::Write), AccessKind::Read) => true, // (2) justified by `all_transitions_idempotent` (Some(old), new) if old == new => true, @@ -670,7 +670,8 @@ impl AccessRelatedness { mod commutation_tests { use super::*; impl LocationState { - pub fn all_without_access() -> impl Iterator { + pub fn all() -> impl Iterator { + // We keep `latest_foreign_access` at `None` as that's just a cache. Permission::all().flat_map(|permission| { [false, true].into_iter().map(move |initialized| { Self { permission, initialized, latest_foreign_access: None } @@ -695,12 +696,12 @@ mod commutation_tests { // Any protector state works, but we can't move reads across function boundaries // so the two read accesses occur under the same protector. for &protected in &[true, false] { - for loc in LocationState::all_without_access() { + for loc in LocationState::all() { // Apply 1 then 2. Failure here means that there is UB in the source // and we skip the check in the target. let mut loc12 = loc; - let Ok(_) = loc12.perform_access(kind, rel1, protected) else { continue; }; - let Ok(_) = loc12.perform_access(kind, rel2, protected) else { continue; }; + let Ok(_) = loc12.perform_access(kind, rel1, protected) else { continue }; + let Ok(_) = loc12.perform_access(kind, rel2, protected) else { continue }; // If 1 followed by 2 succeeded, then 2 followed by 1 must also succeed... let mut loc21 = loc; @@ -718,4 +719,33 @@ mod commutation_tests { } } } + + #[test] + #[rustfmt::skip] + // Ensure that of 2 accesses happen, one foreign and one a child, and we are protected, that we + // get UB unless they are both reads. + fn protected_enforces_noalias() { + for rel1 in AccessRelatedness::all() { + for rel2 in AccessRelatedness::all() { + if rel1.is_foreign() == rel2.is_foreign() { + // We want to check pairs of accesses where one is foreign and one is not. + continue; + } + for kind1 in AccessKind::all() { + for kind2 in AccessKind::all() { + for mut state in LocationState::all() { + let protected = true; + let Ok(_) = state.perform_access(kind1, rel1, protected) else { continue }; + let Ok(_) = state.perform_access(kind2, rel2, protected) else { continue }; + // If these were both allowed, it must have been two reads. + assert!( + kind1 == AccessKind::Read && kind2 == AccessKind::Read, + "failed to enforce noalias between two accesses that are not both reads" + ); + } + } + } + } + } + } }